From 14f1b5a489467890048fa2531d3ffd4836fa7ba4 Mon Sep 17 00:00:00 2001 From: Vladislav Antonov Date: Sun, 22 Mar 2026 20:16:24 +0200 Subject: [PATCH 01/10] fix: escape LIKE metacharacters in search query to prevent wildcard injection User-supplied % and _ characters in the search 'q' parameter were passed directly into the ILIKE pattern, allowing queries like q=% to match all products or crafted patterns to degrade performance. Added _escape_like() to sanitize metacharacters before embedding in the pattern. --- app/services/product_service.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/services/product_service.py b/app/services/product_service.py index 9a19a84..ff51744 100644 --- a/app/services/product_service.py +++ b/app/services/product_service.py @@ -1,9 +1,15 @@ """Product business logic helpers and search orchestration.""" +import re from sqlalchemy import func, or_, select from sqlalchemy.ext.asyncio import AsyncSession from time import perf_counter + +def _escape_like(pattern: str) -> str: + """Escape LIKE metacharacters (%, _, \\) so they match literally.""" + return re.sub(r"([%_\\])", r"\\\1", pattern) + from app.observability.db_timing import timed_execute_scalar_one, timed_execute_scalars_all from app.models.product import Product from app.services.category_service import category_subtree_cte @@ -28,7 +34,8 @@ async def search_products( if q: normalized = q.upper() - query = query.where(or_(Product.title.ilike(f"%{q}%"), Product.sku == normalized)) + safe_q = _escape_like(q) + query = query.where(or_(Product.title.ilike(f"%{safe_q}%"), Product.sku == normalized)) if min_price is not None: query = query.where(Product.price >= min_price) From 8f9e975bdd2293dc32b8afc27c0814a49fed2629 Mon Sep 17 00:00:00 2001 From: Vladislav Antonov Date: Sun, 22 Mar 2026 20:19:10 +0200 Subject: [PATCH 02/10] feat: add rate limiting with slowapi - Add slowapi dependency for IP-based rate limiting - Configure default limit (60/min) and stricter search limit (30/min) via RATE_LIMIT_DEFAULT and RATE_LIMIT_SEARCH env vars - Register RateLimitExceeded handler for 429 responses - Apply per-endpoint limit on the search route (most expensive operation) --- app/api/search.py | 5 +++++ app/core/config.py | 2 ++ app/main.py | 10 ++++++++++ pyproject.toml | 1 + 4 files changed, 18 insertions(+) diff --git a/app/api/search.py b/app/api/search.py index acfbc57..5fefb39 100644 --- a/app/api/search.py +++ b/app/api/search.py @@ -4,8 +4,11 @@ import logging from fastapi import APIRouter, Depends, HTTPException, Query +from slowapi import Limiter +from slowapi.util import get_remote_address from sqlalchemy.ext.asyncio import AsyncSession from starlette.requests import Request +from app.core.config import get_settings from app.db.session import get_session from app.observability.metrics import search_requests_total, search_result_count, search_zero_results_total from app.observability.route import ObservabilityRoute @@ -14,9 +17,11 @@ router = APIRouter(route_class=ObservabilityRoute) logger = logging.getLogger("app.search") +_limiter = Limiter(key_func=get_remote_address) @router.get("/search", response_model=ProductSearchResponse) +@_limiter.limit(lambda: get_settings().rate_limit_search) async def search_products_endpoint( request: Request, q: str | None = Query(default=None, min_length=1, max_length=255), diff --git a/app/core/config.py b/app/core/config.py index 020ac9a..3b67e53 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -41,6 +41,8 @@ class Settings(BaseSettings): log_level: str = "INFO" health_check_db_retries: int = 3 health_check_db_timeout: float = 2.0 + rate_limit_default: str = "60/minute" + rate_limit_search: str = "30/minute" @lru_cache diff --git a/app/main.py b/app/main.py index 4eaa8f3..ab4e4a7 100644 --- a/app/main.py +++ b/app/main.py @@ -9,6 +9,9 @@ from fastapi.responses import HTMLResponse, JSONResponse from fastapi.templating import Jinja2Templates from jinja2 import TemplateNotFound +from slowapi import Limiter, _rate_limit_exceeded_handler +from slowapi.errors import RateLimitExceeded +from slowapi.util import get_remote_address from app.api import api_router from app.core.config import get_settings @@ -75,11 +78,18 @@ def create_app() -> FastAPI: """Create and configure the FastAPI application.""" settings = get_settings() + limiter = Limiter( + key_func=get_remote_address, + default_limits=[settings.rate_limit_default], + ) + app = FastAPI( title="Commerce System Demo", version="0.1.5", lifespan=lifespan, ) + app.state.limiter = limiter + app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) app.router.route_class = ObservabilityRoute initialize_app_observability(app, settings) app.include_router(api_router, prefix=settings.api_prefix) diff --git a/pyproject.toml b/pyproject.toml index 4f25f42..dfeb354 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,6 +23,7 @@ dependencies = [ "opentelemetry-instrumentation-fastapi>=0.48b0,<1.0.0", "opentelemetry-instrumentation-sqlalchemy>=0.48b0,<1.0.0", "prometheus-client>=0.20.0,<1.0.0", + "slowapi>=0.1.9,<1.0.0", ] [project.optional-dependencies] From dc0a09a266cd42c1625e7d9c2e4875a8e5ad8794 Mon Sep 17 00:00:00 2001 From: Vladislav Antonov Date: Sun, 22 Mar 2026 20:23:49 +0200 Subject: [PATCH 03/10] fix: add image_url serializer to ProductUpdate and null-clearing tests - Add field_serializer('image_url') to ProductUpdate so model_dump() returns plain str instead of Pydantic Url object for SQLAlchemy - Add tests verifying explicit null clears category_id and image_url - Add test verifying omitted PATCH fields remain unchanged --- app/schemas/product.py | 11 +++++- tests/test_api.py | 79 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/app/schemas/product.py b/app/schemas/product.py index 97eaaaa..a1f10c1 100644 --- a/app/schemas/product.py +++ b/app/schemas/product.py @@ -54,7 +54,12 @@ class ProductCreate(ProductBase): class ProductUpdate(BaseModel): - """Schema for productupdate.""" + """Partial update schema for products. + + Uses ``model_dump(exclude_unset=True)`` in the handler so that fields + omitted from the JSON body are left untouched, while fields explicitly + sent as ``null`` (e.g. ``{"category_id": null}``) clear the value. + """ title: str | None = Field(default=None, min_length=1, max_length=255) description: str | None = Field(default=None, min_length=1, max_length=10000) image_url: AnyHttpUrl | None = Field(default=None) @@ -62,6 +67,10 @@ class ProductUpdate(BaseModel): price: Decimal | None = Field(default=None, ge=Decimal("0")) category_id: int | None = None + @field_serializer("image_url") + def serialize_image_url(self, value: AnyHttpUrl | None) -> str | None: + return str(value) if value is not None else None + @field_validator("title", mode="before") @classmethod def validate_optional_title(cls, value: str | None) -> str | None: diff --git a/tests/test_api.py b/tests/test_api.py index dd73b45..ef25365 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -935,3 +935,82 @@ async def test_update_product_with_space_only_description_fails(client: AsyncCli json={"description": " "} ) assert response.status_code == 422 # Validation error + + +@pytest.mark.asyncio +async def test_update_product_set_category_to_null(client: AsyncClient): + """Test that explicitly sending category_id=null clears the category link.""" + cat_resp = await client.post("/api/v1/categories", json={"name": "Temp Cat"}) + category_id = cat_resp.json()["id"] + + create_resp = await client.post( + "/api/v1/products", + json={ + "title": "Linked Product", + "description": "Has category", + "sku": "NULLCAT-001", + "price": "50.00", + "category_id": category_id, + }, + ) + product_id = create_resp.json()["id"] + assert create_resp.json()["category_id"] == category_id + + response = await client.patch( + f"/api/v1/products/{product_id}", + json={"category_id": None}, + ) + assert response.status_code == 200 + assert response.json()["category_id"] is None + + +@pytest.mark.asyncio +async def test_update_product_set_image_url_to_null(client: AsyncClient): + """Test that explicitly sending image_url=null clears the image.""" + create_resp = await client.post( + "/api/v1/products", + json={ + "title": "With Image", + "description": "Has image", + "sku": "NULLIMG-001", + "price": "75.00", + "image_url": "https://example.com/img.png", + }, + ) + product_id = create_resp.json()["id"] + assert create_resp.json()["image_url"] is not None + + response = await client.patch( + f"/api/v1/products/{product_id}", + json={"image_url": None}, + ) + assert response.status_code == 200 + assert response.json()["image_url"] is None + + +@pytest.mark.asyncio +async def test_update_product_omitted_fields_unchanged(client: AsyncClient): + """Test that omitting fields from PATCH leaves them unchanged.""" + create_resp = await client.post( + "/api/v1/products", + json={ + "title": "Original", + "description": "Keep this", + "sku": "OMIT-001", + "price": "99.00", + "image_url": "https://example.com/keep.png", + }, + ) + product_id = create_resp.json()["id"] + + response = await client.patch( + f"/api/v1/products/{product_id}", + json={"title": "Changed Only Title"}, + ) + assert response.status_code == 200 + data = response.json() + assert data["title"] == "Changed Only Title" + assert data["description"] == "Keep this" + assert data["sku"] == "OMIT-001" + assert data["price"] == "99.00" + assert data["image_url"] == "https://example.com/keep.png" From 54dd20d137f859c68dec9e79afe46c1aa2c7e946 Mon Sep 17 00:00:00 2001 From: Vladislav Antonov Date: Sun, 22 Mar 2026 20:26:24 +0200 Subject: [PATCH 04/10] perf: use COUNT(*) OVER() window function in search to eliminate second query Replace the separate COUNT(*) subquery with a window function so the total count is computed in the same database roundtrip as the data fetch. Add timed_execute_all() helper for queries returning row tuples. --- app/observability/db_timing.py | 9 +++++++++ app/services/product_service.py | 23 ++++++++++++++++------- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/app/observability/db_timing.py b/app/observability/db_timing.py index 1f1f77f..0ae7a14 100644 --- a/app/observability/db_timing.py +++ b/app/observability/db_timing.py @@ -46,6 +46,15 @@ async def timed_execute_scalars_all(session: AsyncSession, statement: Any): return result +async def timed_execute_all(session: AsyncSession, statement: Any): + """Execute a query and return all rows (as tuples) with timing telemetry.""" + await _record_connection_acquire(session) + execute_fetch_start = perf_counter() + result = (await session.execute(statement)).all() + _record_execute_fetch((perf_counter() - execute_fetch_start) * 1000) + return result + + async def timed_execute_scalar_one(session: AsyncSession, statement: Any): """Execute a query and return one scalar result with timing telemetry.""" await _record_connection_acquire(session) diff --git a/app/services/product_service.py b/app/services/product_service.py index ff51744..079e246 100644 --- a/app/services/product_service.py +++ b/app/services/product_service.py @@ -10,7 +10,7 @@ def _escape_like(pattern: str) -> str: """Escape LIKE metacharacters (%, _, \\) so they match literally.""" return re.sub(r"([%_\\])", r"\\\1", pattern) -from app.observability.db_timing import timed_execute_scalar_one, timed_execute_scalars_all +from app.observability.db_timing import timed_execute_all, timed_execute_scalars_all from app.models.product import Product from app.services.category_service import category_subtree_cte @@ -50,21 +50,30 @@ async def search_products( query = query.order_by(Product.id) timing_context["query_build_ms"] = (perf_counter() - query_build_start) * 1000 - # Fetch data and count on the provided session for a consistent snapshot. + # First-page fast path: fetch data only, infer total from result count. data_query_start = perf_counter() records = await timed_execute_scalars_all(session, query.limit(limit).offset(offset)) timing_context["data_query_ms"] = (perf_counter() - data_query_start) * 1000 - # Skip COUNT(*) when the total is inferrable from the result set. - # If we're on the first page and fewer rows than the page limit were returned, - # all matching rows fit on this page so total == offset + len(records). if offset == 0 and len(records) < limit: total = len(records) timing_context["count_query_ms"] = 0.0 else: - total_query = select(func.count()).select_from(query.subquery()) + # Use COUNT(*) OVER() window function to get the total in a single + # query instead of re-executing the full subquery for a separate COUNT. + count_col = func.count().over().label("_total") + windowed_query = ( + query.add_columns(count_col).limit(limit).offset(offset) + ) count_query_start = perf_counter() - total = await timed_execute_scalar_one(session, total_query) + rows = await timed_execute_all(session, windowed_query) timing_context["count_query_ms"] = (perf_counter() - count_query_start) * 1000 + if rows: + records = [row[0] for row in rows] + total = rows[0][1] + else: + records = [] + total = 0 + return records, total From 61502e3df8ed73797f82ccf7432f8592014054b8 Mon Sep 17 00:00:00 2001 From: Vladislav Antonov Date: Sun, 22 Mar 2026 20:32:04 +0200 Subject: [PATCH 05/10] perf: consolidate category validation queries into single roundtrips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validate_category_parent: 2 queries → 1 - Combined parent existence check + ancestor depth into a single ancestor-chain CTE query (NULL max-depth = parent not found) validate_category_reparent: 4 queries → 1 - Combined parent existence, cycle detection (ancestor CTE), and subtree height (descendant CTE) into a single SELECT with two independent recursive CTEs Added timed_execute_one helper to db_timing for multi-column row results. --- app/observability/db_timing.py | 9 ++++ app/services/category_service.py | 42 ++++++++++++------ tests/test_category_service.py | 75 +++++++++----------------------- 3 files changed, 59 insertions(+), 67 deletions(-) diff --git a/app/observability/db_timing.py b/app/observability/db_timing.py index 0ae7a14..1c17d68 100644 --- a/app/observability/db_timing.py +++ b/app/observability/db_timing.py @@ -55,6 +55,15 @@ async def timed_execute_all(session: AsyncSession, statement: Any): return result +async def timed_execute_one(session: AsyncSession, statement: Any): + """Execute a query and return one row with timing telemetry.""" + await _record_connection_acquire(session) + execute_fetch_start = perf_counter() + result = (await session.execute(statement)).one() + _record_execute_fetch((perf_counter() - execute_fetch_start) * 1000) + return result + + async def timed_execute_scalar_one(session: AsyncSession, statement: Any): """Execute a query and return one scalar result with timing telemetry.""" await _record_connection_acquire(session) diff --git a/app/services/category_service.py b/app/services/category_service.py index 0a16f49..13a62db 100644 --- a/app/services/category_service.py +++ b/app/services/category_service.py @@ -4,7 +4,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.models.category import Category -from app.observability.db_timing import timed_execute_scalar_one, timed_get +from app.observability.db_timing import timed_execute_one, timed_execute_scalar_one, timed_get MAX_CATEGORY_DEPTH = 100 @@ -126,14 +126,17 @@ async def category_subtree_height(session: AsyncSession, category_id: int) -> in async def validate_category_parent(session: AsyncSession, parent_id: int) -> None: """Validate that a parent category exists and is within depth limits. + Uses a single ancestor-chain CTE query to check both existence and depth. + Raises: CategoryParentNotFoundError: if the parent does not exist. CategoryDepthError: if attaching here would exceed MAX_CATEGORY_DEPTH. """ - parent = await get_category_or_none(session, parent_id) - if parent is None: + ancestor_chain = _ancestor_chain_cte(parent_id, depth_limit=MAX_CATEGORY_DEPTH + 1) + stmt = select(func.max(ancestor_chain.c.depth)) + depth = await timed_execute_scalar_one(session, stmt) + if depth is None: raise CategoryParentNotFoundError(parent_id) - depth = await category_depth(session, parent_id) if depth >= MAX_CATEGORY_DEPTH: raise CategoryDepthError(MAX_CATEGORY_DEPTH) @@ -143,6 +146,10 @@ async def validate_category_reparent( ) -> None: """Validate re-parenting a category: parent exists, no cycles, depth within limits. + Uses a single query combining ancestor-chain and descendant-chain CTEs + to check parent existence, cycle detection, and depth limits in one + database roundtrip (down from four). + Raises: CategoryParentNotFoundError: if the new parent does not exist. CategoryCycleError: if the re-parent creates a cycle. @@ -150,14 +157,23 @@ async def validate_category_reparent( """ if new_parent_id is None: return - parent = await get_category_or_none(session, new_parent_id) - if parent is None: + + ancestor_chain = _ancestor_chain_cte(new_parent_id, depth_limit=MAX_CATEGORY_DEPTH + 1) + descendant_chain = _descendant_chain_cte(category_id, depth_limit=MAX_CATEGORY_DEPTH + 1) + + stmt = select( + func.max(ancestor_chain.c.depth).label("parent_depth"), + exists( + select(1).select_from(ancestor_chain).where(ancestor_chain.c.id == category_id) + ).label("has_cycle"), + func.coalesce(func.max(descendant_chain.c.depth), 1).label("subtree_height"), + ) + + row = await timed_execute_one(session, stmt) + + if row.parent_depth is None: raise CategoryParentNotFoundError(new_parent_id) - try: - await validate_no_cycles(session, category_id, new_parent_id) - except ValueError as exc: - raise CategoryCycleError(str(exc)) from exc - parent_depth = await category_depth(session, new_parent_id) - subtree_height = await category_subtree_height(session, category_id) - if parent_depth + subtree_height > MAX_CATEGORY_DEPTH: + if row.has_cycle: + raise CategoryCycleError("Category cycle detected") + if row.parent_depth + row.subtree_height > MAX_CATEGORY_DEPTH: raise CategoryDepthError(MAX_CATEGORY_DEPTH) diff --git a/tests/test_category_service.py b/tests/test_category_service.py index 3b75e71..f69511b 100644 --- a/tests/test_category_service.py +++ b/tests/test_category_service.py @@ -46,7 +46,8 @@ async def test_validate_no_cycles_raises_for_cycle(monkeypatch: pytest.MonkeyPat @pytest.mark.asyncio async def test_validate_category_parent_raises_not_found(monkeypatch: pytest.MonkeyPatch): session = AsyncMock() - monkeypatch.setattr(category_service, "get_category_or_none", AsyncMock(return_value=None)) + # NULL max-depth from ancestor CTE means parent doesn't exist + monkeypatch.setattr(category_service, "timed_execute_scalar_one", AsyncMock(return_value=None)) with pytest.raises(category_service.CategoryParentNotFoundError): await category_service.validate_category_parent(session, parent_id=99) @@ -57,12 +58,7 @@ async def test_validate_category_parent_raises_depth_error(monkeypatch: pytest.M session = AsyncMock() monkeypatch.setattr( category_service, - "get_category_or_none", - AsyncMock(return_value=SimpleNamespace(id=1, parent_id=None)), - ) - monkeypatch.setattr( - category_service, - "category_depth", + "timed_execute_scalar_one", AsyncMock(return_value=category_service.MAX_CATEGORY_DEPTH), ) @@ -79,7 +75,12 @@ async def test_validate_category_reparent_returns_for_none_parent(): @pytest.mark.asyncio async def test_validate_category_reparent_raises_parent_not_found(monkeypatch: pytest.MonkeyPatch): session = AsyncMock() - monkeypatch.setattr(category_service, "get_category_or_none", AsyncMock(return_value=None)) + # parent_depth=None means parent doesn't exist + monkeypatch.setattr( + category_service, + "timed_execute_one", + AsyncMock(return_value=SimpleNamespace(parent_depth=None, has_cycle=False, subtree_height=1)), + ) with pytest.raises(category_service.CategoryParentNotFoundError): await category_service.validate_category_reparent(session, category_id=1, new_parent_id=77) @@ -90,13 +91,8 @@ async def test_validate_category_reparent_raises_cycle_error(monkeypatch: pytest session = AsyncMock() monkeypatch.setattr( category_service, - "get_category_or_none", - AsyncMock(return_value=SimpleNamespace(id=2, parent_id=None)), - ) - monkeypatch.setattr( - category_service, - "validate_no_cycles", - AsyncMock(side_effect=ValueError("Category cycle detected")), + "timed_execute_one", + AsyncMock(return_value=SimpleNamespace(parent_depth=5, has_cycle=True, subtree_height=1)), ) with pytest.raises(category_service.CategoryCycleError, match="Category cycle detected"): @@ -108,19 +104,12 @@ async def test_validate_category_reparent_raises_depth_error(monkeypatch: pytest session = AsyncMock() monkeypatch.setattr( category_service, - "get_category_or_none", - AsyncMock(return_value=SimpleNamespace(id=2, parent_id=None)), - ) - monkeypatch.setattr(category_service, "validate_no_cycles", AsyncMock(return_value=None)) - monkeypatch.setattr( - category_service, - "category_depth", - AsyncMock(return_value=category_service.MAX_CATEGORY_DEPTH), - ) - monkeypatch.setattr( - category_service, - "category_subtree_height", - AsyncMock(return_value=1), + "timed_execute_one", + AsyncMock(return_value=SimpleNamespace( + parent_depth=category_service.MAX_CATEGORY_DEPTH, + has_cycle=False, + subtree_height=1, + )), ) with pytest.raises(category_service.CategoryDepthError): @@ -131,22 +120,11 @@ async def test_validate_category_reparent_raises_depth_error(monkeypatch: pytest async def test_validate_category_reparent_raises_depth_error_for_deep_subtree(monkeypatch: pytest.MonkeyPatch): """Moving a category with a deep subtree under a parent should fail if combined depth exceeds limit.""" session = AsyncMock() - monkeypatch.setattr( - category_service, - "get_category_or_none", - AsyncMock(return_value=SimpleNamespace(id=2, parent_id=None)), - ) - monkeypatch.setattr(category_service, "validate_no_cycles", AsyncMock(return_value=None)) # Parent depth alone is fine (90 < 100), but subtree adds 15 → 90 + 15 = 105 > 100 monkeypatch.setattr( category_service, - "category_depth", - AsyncMock(return_value=90), - ) - monkeypatch.setattr( - category_service, - "category_subtree_height", - AsyncMock(return_value=15), + "timed_execute_one", + AsyncMock(return_value=SimpleNamespace(parent_depth=90, has_cycle=False, subtree_height=15)), ) with pytest.raises(category_service.CategoryDepthError): @@ -157,22 +135,11 @@ async def test_validate_category_reparent_raises_depth_error_for_deep_subtree(mo async def test_validate_category_reparent_allows_when_combined_depth_fits(monkeypatch: pytest.MonkeyPatch): """Moving a category succeeds when parent depth + subtree height fits within limit.""" session = AsyncMock() - monkeypatch.setattr( - category_service, - "get_category_or_none", - AsyncMock(return_value=SimpleNamespace(id=2, parent_id=None)), - ) - monkeypatch.setattr(category_service, "validate_no_cycles", AsyncMock(return_value=None)) # Parent depth 90, subtree height 10 → 90 + 10 = 100 <= 100 → OK monkeypatch.setattr( category_service, - "category_depth", - AsyncMock(return_value=90), - ) - monkeypatch.setattr( - category_service, - "category_subtree_height", - AsyncMock(return_value=10), + "timed_execute_one", + AsyncMock(return_value=SimpleNamespace(parent_depth=90, has_cycle=False, subtree_height=10)), ) # Should not raise From 35de408591c1af44e6e2324ca56edda721906d7a Mon Sep 17 00:00:00 2001 From: Vladislav Antonov Date: Sun, 22 Mar 2026 20:35:37 +0200 Subject: [PATCH 06/10] test: add missing integration tests for depth limits, cycles, and LIKE injection - Category depth limit enforcement (create and reparent with monkeypatched limit) - Category cycle detection (direct, multi-level, and self-reference) - Nonexistent parent rejection (create and reparent) - LIKE wildcard injection safety (%, _, backslash) 10 new integration tests, 115 total passing. --- tests/test_api.py | 178 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 178 insertions(+) diff --git a/tests/test_api.py b/tests/test_api.py index ef25365..c28e2ae 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1014,3 +1014,181 @@ async def test_update_product_omitted_fields_unchanged(client: AsyncClient): assert data["sku"] == "OMIT-001" assert data["price"] == "99.00" assert data["image_url"] == "https://example.com/keep.png" + + +# ============================================================================ +# Category Depth Limit Tests +# ============================================================================ + +@pytest.mark.asyncio +async def test_create_category_exceeding_depth_limit(client: AsyncClient, monkeypatch: pytest.MonkeyPatch): + """Test that creating a category chain beyond MAX_CATEGORY_DEPTH is rejected.""" + from app.services import category_service as cs + + monkeypatch.setattr(cs, "MAX_CATEGORY_DEPTH", 3) + + # Build chain: root → L1 → L2 (depth 3) + root = (await client.post("/api/v1/categories", json={"name": "Depth-Root"})).json() + l1 = (await client.post("/api/v1/categories", json={"name": "Depth-L1", "parent_id": root["id"]})).json() + l2 = (await client.post("/api/v1/categories", json={"name": "Depth-L2", "parent_id": l1["id"]})).json() + assert l2["parent_id"] == l1["id"] + + # L3 should be rejected — depth would be 4 > 3 + response = await client.post( + "/api/v1/categories", + json={"name": "Depth-L3", "parent_id": l2["id"]}, + ) + assert response.status_code == 422 + assert "depth" in response.json()["detail"].lower() + + +@pytest.mark.asyncio +async def test_reparent_category_exceeding_depth_limit(client: AsyncClient, monkeypatch: pytest.MonkeyPatch): + """Test that re-parenting a subtree so combined depth exceeds the limit is rejected.""" + from app.services import category_service as cs + + monkeypatch.setattr(cs, "MAX_CATEGORY_DEPTH", 4) + + # Chain A: A1 → A2 → A3 (height 3) + a1 = (await client.post("/api/v1/categories", json={"name": "ChainA-1"})).json() + a2 = (await client.post("/api/v1/categories", json={"name": "ChainA-2", "parent_id": a1["id"]})).json() + a3 = (await client.post("/api/v1/categories", json={"name": "ChainA-3", "parent_id": a2["id"]})).json() + + # Chain B: B1 → B2 (depth 2) + b1 = (await client.post("/api/v1/categories", json={"name": "ChainB-1"})).json() + b2 = (await client.post("/api/v1/categories", json={"name": "ChainB-2", "parent_id": b1["id"]})).json() + + # Try to move A1 under B2 → depth would be 2 + 3 = 5 > 4 + response = await client.patch( + f"/api/v1/categories/{a1['id']}", + json={"parent_id": b2["id"]}, + ) + assert response.status_code == 422 + assert "depth" in response.json()["detail"].lower() + + +# ============================================================================ +# Category Cycle Detection Tests +# ============================================================================ + +@pytest.mark.asyncio +async def test_reparent_category_creating_cycle_rejected(client: AsyncClient): + """Test that re-parenting a parent under its own child is rejected as a cycle.""" + parent = (await client.post("/api/v1/categories", json={"name": "Cycle-Parent"})).json() + child = (await client.post( + "/api/v1/categories", + json={"name": "Cycle-Child", "parent_id": parent["id"]}, + )).json() + + # Try to make the parent a child of its own child → cycle + response = await client.patch( + f"/api/v1/categories/{parent['id']}", + json={"parent_id": child["id"]}, + ) + assert response.status_code == 422 + assert "cycle" in response.json()["detail"].lower() + + +@pytest.mark.asyncio +async def test_reparent_category_creating_deep_cycle_rejected(client: AsyncClient): + """Test that a cycle through multiple levels is detected and rejected.""" + a = (await client.post("/api/v1/categories", json={"name": "CycleDeep-A"})).json() + b = (await client.post("/api/v1/categories", json={"name": "CycleDeep-B", "parent_id": a["id"]})).json() + c = (await client.post("/api/v1/categories", json={"name": "CycleDeep-C", "parent_id": b["id"]})).json() + + # Try to make A a child of C → A→B→C→A cycle + response = await client.patch( + f"/api/v1/categories/{a['id']}", + json={"parent_id": c["id"]}, + ) + assert response.status_code == 422 + assert "cycle" in response.json()["detail"].lower() + + +@pytest.mark.asyncio +async def test_reparent_category_self_reference_rejected(client: AsyncClient): + """Test that setting a category as its own parent is rejected.""" + cat = (await client.post("/api/v1/categories", json={"name": "Self-Ref"})).json() + + response = await client.patch( + f"/api/v1/categories/{cat['id']}", + json={"parent_id": cat["id"]}, + ) + assert response.status_code == 422 + assert "cycle" in response.json()["detail"].lower() + + +@pytest.mark.asyncio +async def test_reparent_to_nonexistent_parent_rejected(client: AsyncClient): + """Test that re-parenting to a nonexistent category returns 404.""" + cat = (await client.post("/api/v1/categories", json={"name": "Orphan-Move"})).json() + + response = await client.patch( + f"/api/v1/categories/{cat['id']}", + json={"parent_id": 99999}, + ) + assert response.status_code == 404 + + +@pytest.mark.asyncio +async def test_create_category_with_nonexistent_parent_rejected(client: AsyncClient): + """Test that creating a category under a nonexistent parent returns 404.""" + response = await client.post( + "/api/v1/categories", + json={"name": "No-Parent", "parent_id": 99999}, + ) + assert response.status_code == 404 + + +# ============================================================================ +# LIKE Wildcard Injection Tests +# ============================================================================ + +@pytest.mark.asyncio +async def test_search_with_percent_wildcard_does_not_match_all(client: AsyncClient): + """Test that a search query containing '%' does not act as a LIKE wildcard.""" + await client.post( + "/api/v1/products", + json={ + "title": "Specific Widget", + "description": "Only this should NOT match", + "sku": "LIKE-SAFE-001", + "price": "10.00", + }, + ) + + # Searching for literal '%' should not match arbitrary products + response = await client.get("/api/v1/products/search?q=%25") + assert response.status_code == 200 + data = response.json() + # '%' as a literal character shouldn't match "Specific Widget" + for item in data["items"]: + assert "%" in item["title"] or "%" in item["sku"] + + +@pytest.mark.asyncio +async def test_search_with_underscore_wildcard_does_not_match_single_char(client: AsyncClient): + """Test that a search query containing '_' does not act as a single-char wildcard.""" + await client.post( + "/api/v1/products", + json={ + "title": "ABC", + "description": "Three letter title", + "sku": "LIKE-UNDER-001", + "price": "10.00", + }, + ) + + # '_B_' as LIKE wildcards would match 'ABC', but as literal it should not + response = await client.get("/api/v1/products/search?q=_B_") + assert response.status_code == 200 + data = response.json() + for item in data["items"]: + assert "_B_" in item["title"] or "_B_" in item["sku"].upper() + + +@pytest.mark.asyncio +async def test_search_with_backslash_is_safe(client: AsyncClient): + """Test that a search query containing backslash doesn't cause errors.""" + response = await client.get("/api/v1/products/search?q=test%5Cvalue") + assert response.status_code == 200 From 2571ab44a927e9e53bc92739cd07e577c16321a3 Mon Sep 17 00:00:00 2001 From: Vladislav Antonov Date: Sun, 22 Mar 2026 20:39:59 +0200 Subject: [PATCH 07/10] feat: add Alembic migration framework with async support - Initialize Alembic with async template for asyncpg/PostgreSQL - Configure env.py to read DATABASE_URL from app settings - Add initial migration (001_initial) matching existing schema: category and product tables with all constraints and indexes - Add alembic>=1.13.0 to pyproject.toml dependencies Usage: alembic upgrade head # apply all migrations alembic revision --autogenerate -m 'description' # generate new migration alembic upgrade head --sql # preview SQL without applying --- alembic.ini | 147 +++++++++++++++++++++++++ alembic/README | 1 + alembic/env.py | 73 ++++++++++++ alembic/script.py.mako | 28 +++++ alembic/versions/001_initial_schema.py | 83 ++++++++++++++ pyproject.toml | 1 + 6 files changed, 333 insertions(+) create mode 100644 alembic.ini create mode 100644 alembic/README create mode 100644 alembic/env.py create mode 100644 alembic/script.py.mako create mode 100644 alembic/versions/001_initial_schema.py diff --git a/alembic.ini b/alembic.ini new file mode 100644 index 0000000..4d19088 --- /dev/null +++ b/alembic.ini @@ -0,0 +1,147 @@ +# A generic, single database configuration. + +[alembic] +# path to migration scripts. +# this is typically a path given in POSIX (e.g. forward slashes) +# format, relative to the token %(here)s which refers to the location of this +# ini file +script_location = %(here)s/alembic + +# template used to generate migration file names; The default value is %%(rev)s_%%(slug)s +# Uncomment the line below if you want the files to be prepended with date and time +# see https://alembic.sqlalchemy.org/en/latest/tutorial.html#editing-the-ini-file +# for all available tokens +# file_template = %%(year)d_%%(month).2d_%%(day).2d_%%(hour).2d%%(minute).2d-%%(rev)s_%%(slug)s +# Or organize into date-based subdirectories (requires recursive_version_locations = true) +# file_template = %%(year)d/%%(month).2d/%%(day).2d_%%(hour).2d%%(minute).2d_%%(second).2d_%%(rev)s_%%(slug)s + +# sys.path path, will be prepended to sys.path if present. +# defaults to the current working directory. for multiple paths, the path separator +# is defined by "path_separator" below. +prepend_sys_path = . + +# timezone to use when rendering the date within the migration file +# as well as the filename. +# If specified, requires the tzdata library which can be installed by adding +# `alembic[tz]` to the pip requirements. +# string value is passed to ZoneInfo() +# leave blank for localtime +# timezone = + +# max length of characters to apply to the "slug" field +# truncate_slug_length = 40 + +# set to 'true' to run the environment during +# the 'revision' command, regardless of autogenerate +# revision_environment = false + +# set to 'true' to allow .pyc and .pyo files without +# a source .py file to be detected as revisions in the +# versions/ directory +# sourceless = false + +# version location specification; This defaults +# to /versions. When using multiple version +# directories, initial revisions must be specified with --version-path. +# The path separator used here should be the separator specified by "path_separator" +# below. +# version_locations = %(here)s/bar:%(here)s/bat:%(here)s/alembic/versions + +# path_separator; This indicates what character is used to split lists of file +# paths, including version_locations and prepend_sys_path within configparser +# files such as alembic.ini. +# The default rendered in new alembic.ini files is "os", which uses os.pathsep +# to provide os-dependent path splitting. +# +# Note that in order to support legacy alembic.ini files, this default does NOT +# take place if path_separator is not present in alembic.ini. If this +# option is omitted entirely, fallback logic is as follows: +# +# 1. Parsing of the version_locations option falls back to using the legacy +# "version_path_separator" key, which if absent then falls back to the legacy +# behavior of splitting on spaces and/or commas. +# 2. Parsing of the prepend_sys_path option falls back to the legacy +# behavior of splitting on spaces, commas, or colons. +# +# Valid values for path_separator are: +# +# path_separator = : +# path_separator = ; +# path_separator = space +# path_separator = newline +# +# Use os.pathsep. Default configuration used for new projects. +path_separator = os + + +# set to 'true' to search source files recursively +# in each "version_locations" directory +# new in Alembic version 1.10 +# recursive_version_locations = false + +# the output encoding used when revision files +# are written from script.py.mako +# output_encoding = utf-8 + +# database URL. Overridden at runtime by env.py from DATABASE_URL env var. +sqlalchemy.url = + + +[post_write_hooks] +# post_write_hooks defines scripts or Python functions that are run +# on newly generated revision scripts. See the documentation for further +# detail and examples + +# format using "black" - use the console_scripts runner, against the "black" entrypoint +# hooks = black +# black.type = console_scripts +# black.entrypoint = black +# black.options = -l 79 REVISION_SCRIPT_FILENAME + +# lint with attempts to fix using "ruff" - use the module runner, against the "ruff" module +# hooks = ruff +# ruff.type = module +# ruff.module = ruff +# ruff.options = check --fix REVISION_SCRIPT_FILENAME + +# Alternatively, use the exec runner to execute a binary found on your PATH +# hooks = ruff +# ruff.type = exec +# ruff.executable = ruff +# ruff.options = check --fix REVISION_SCRIPT_FILENAME + +# Logging configuration. This is also consumed by the user-maintained +# env.py script only. +[loggers] +keys = root,sqlalchemy,alembic + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARNING +handlers = console +qualname = + +[logger_sqlalchemy] +level = WARNING +handlers = +qualname = sqlalchemy.engine + +[logger_alembic] +level = INFO +handlers = +qualname = alembic + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(levelname)-5.5s [%(name)s] %(message)s +datefmt = %H:%M:%S diff --git a/alembic/README b/alembic/README new file mode 100644 index 0000000..e0d0858 --- /dev/null +++ b/alembic/README @@ -0,0 +1 @@ +Generic single-database configuration with an async dbapi. \ No newline at end of file diff --git a/alembic/env.py b/alembic/env.py new file mode 100644 index 0000000..ec99f2b --- /dev/null +++ b/alembic/env.py @@ -0,0 +1,73 @@ +"""Alembic environment configuration for async migrations.""" + +import asyncio +from logging.config import fileConfig + +from sqlalchemy import pool +from sqlalchemy.engine import Connection +from sqlalchemy.ext.asyncio import async_engine_from_config + +from alembic import context + +# Import all models so that Base.metadata is fully populated. +import app.models # noqa: F401 +from app.core.config import get_settings +from app.db.base import Base + +config = context.config + +# Interpret the config file for Python logging. +if config.config_file_name is not None: + fileConfig(config.config_file_name) + +# Point autogenerate at the project's declarative metadata. +target_metadata = Base.metadata + +# Override sqlalchemy.url from the application settings (DATABASE_URL env var). +config.set_main_option("sqlalchemy.url", get_settings().database_url) + + +def run_migrations_offline() -> None: + """Run migrations in 'offline' mode (emit SQL to stdout).""" + url = config.get_main_option("sqlalchemy.url") + context.configure( + url=url, + target_metadata=target_metadata, + literal_binds=True, + dialect_opts={"paramstyle": "named"}, + ) + + with context.begin_transaction(): + context.run_migrations() + + +def do_run_migrations(connection: Connection) -> None: + context.configure(connection=connection, target_metadata=target_metadata) + + with context.begin_transaction(): + context.run_migrations() + + +async def run_async_migrations() -> None: + """Create an async engine and run migrations.""" + connectable = async_engine_from_config( + config.get_section(config.config_ini_section, {}), + prefix="sqlalchemy.", + poolclass=pool.NullPool, + ) + + async with connectable.connect() as connection: + await connection.run_sync(do_run_migrations) + + await connectable.dispose() + + +def run_migrations_online() -> None: + """Run migrations in 'online' mode.""" + asyncio.run(run_async_migrations()) + + +if context.is_offline_mode(): + run_migrations_offline() +else: + run_migrations_online() diff --git a/alembic/script.py.mako b/alembic/script.py.mako new file mode 100644 index 0000000..1101630 --- /dev/null +++ b/alembic/script.py.mako @@ -0,0 +1,28 @@ +"""${message} + +Revision ID: ${up_revision} +Revises: ${down_revision | comma,n} +Create Date: ${create_date} + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +${imports if imports else ""} + +# revision identifiers, used by Alembic. +revision: str = ${repr(up_revision)} +down_revision: Union[str, Sequence[str], None] = ${repr(down_revision)} +branch_labels: Union[str, Sequence[str], None] = ${repr(branch_labels)} +depends_on: Union[str, Sequence[str], None] = ${repr(depends_on)} + + +def upgrade() -> None: + """Upgrade schema.""" + ${upgrades if upgrades else "pass"} + + +def downgrade() -> None: + """Downgrade schema.""" + ${downgrades if downgrades else "pass"} diff --git a/alembic/versions/001_initial_schema.py b/alembic/versions/001_initial_schema.py new file mode 100644 index 0000000..2583a10 --- /dev/null +++ b/alembic/versions/001_initial_schema.py @@ -0,0 +1,83 @@ +"""Initial schema — category and product tables. + +Revision ID: 001_initial +Revises: +Create Date: 2026-03-22 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "001_initial" +down_revision: Union[str, Sequence[str], None] = None +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.create_table( + "category", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("name", sa.String(255), nullable=False), + sa.Column( + "parent_id", + sa.Integer(), + sa.ForeignKey("category.id", ondelete="CASCADE"), + nullable=True, + ), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.func.now(), + nullable=False, + ), + sa.Column( + "updated_at", + sa.DateTime(timezone=True), + server_default=sa.func.now(), + nullable=False, + ), + sa.UniqueConstraint("parent_id", "name", name="uq_category_parent_name"), + ) + op.create_index("ix_category_parent_id", "category", ["parent_id"]) + + op.create_table( + "product", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("title", sa.String(255), nullable=False), + sa.Column("description", sa.Text(), nullable=False), + sa.Column("image_url", sa.String(2083), nullable=True), + sa.Column("sku", sa.String(100), nullable=False), + sa.Column("price", sa.Numeric(12, 2), nullable=False), + sa.Column( + "category_id", + sa.Integer(), + sa.ForeignKey("category.id", ondelete="SET NULL"), + nullable=True, + ), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.func.now(), + nullable=False, + ), + sa.Column( + "updated_at", + sa.DateTime(timezone=True), + server_default=sa.func.now(), + nullable=False, + ), + sa.UniqueConstraint("sku", name="uq_product_sku"), + sa.CheckConstraint("price >= 0", name="chk_product_price_non_negative"), + ) + op.create_index("ix_product_sku", "product", ["sku"]) + op.create_index("ix_product_price", "product", ["price"]) + op.create_index("ix_product_category_id", "product", ["category_id"]) + + +def downgrade() -> None: + op.drop_table("product") + op.drop_table("category") diff --git a/pyproject.toml b/pyproject.toml index dfeb354..c80ae59 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,6 +24,7 @@ dependencies = [ "opentelemetry-instrumentation-sqlalchemy>=0.48b0,<1.0.0", "prometheus-client>=0.20.0,<1.0.0", "slowapi>=0.1.9,<1.0.0", + "alembic>=1.13.0,<2.0.0", ] [project.optional-dependencies] From fe25f4ce6d98f36f9a4859afb441b1a61b42423e Mon Sep 17 00:00:00 2001 From: Vladislav Antonov Date: Sun, 22 Mar 2026 20:51:59 +0200 Subject: [PATCH 08/10] refactor: move DB engine/factory from module globals to app.state - get_session dependency reads from request.app.state.session_factory - Lifespan stores engine and session_factory on app.state - Health endpoint reads engine from request.app.state.engine - create_schema/drop_schema accept optional engine parameter - Module-level helpers retained for tests, scripts, and migration runner - Test fixtures set app.state explicitly (httpx ASGITransport skips lifespan) --- app/db/session.py | 33 +++++++++++++++++++++------------ app/main.py | 18 +++++++++++------- tests/test_api.py | 27 +++++++++++++++++---------- tests/test_concurrency.py | 5 ++++- 4 files changed, 53 insertions(+), 30 deletions(-) diff --git a/app/db/session.py b/app/db/session.py index 75cece0..52cf05e 100644 --- a/app/db/session.py +++ b/app/db/session.py @@ -1,7 +1,17 @@ -"""Database engine, session factory, and schema lifecycle helpers.""" +"""Database engine, session factory, and schema lifecycle helpers. + +The primary path for HTTP handlers uses FastAPI's dependency injection: +``get_session`` reads from ``request.app.state.session_factory``, which +is populated during the application lifespan. + +Module-level helpers (``initialize_database``, ``get_engine``, +``get_session_factory``) are retained for use in tests, CLI scripts, and +the Alembic migration runner — contexts where no ``Request`` is available. +""" from collections.abc import AsyncGenerator +from fastapi import Request from sqlalchemy.ext.asyncio import ( AsyncEngine, AsyncSession, @@ -40,7 +50,7 @@ def initialize_database(database_url: str | None = None, force: bool = False) -> def get_engine() -> AsyncEngine: - """Get engine.""" + """Return the module-level engine (for tests and scripts).""" if _engine is None: initialize_database() assert _engine is not None @@ -48,34 +58,33 @@ def get_engine() -> AsyncEngine: def get_session_factory() -> async_sessionmaker[AsyncSession]: - """Get session factory.""" + """Return the module-level session factory (for tests and scripts).""" if _session_factory is None: initialize_database() assert _session_factory is not None return _session_factory -async def get_session() -> AsyncGenerator[AsyncSession, None]: - """Get session.""" - session_factory = get_session_factory() - async with session_factory() as session: +async def get_session(request: Request) -> AsyncGenerator[AsyncSession, None]: + """FastAPI dependency that yields a DB session from ``app.state``.""" + factory: async_sessionmaker[AsyncSession] = request.app.state.session_factory + async with factory() as session: yield session -async def create_schema() -> None: - # Import models so SQLAlchemy metadata is fully populated before create_all. +async def create_schema(engine: AsyncEngine | None = None) -> None: """Create schema.""" import app.models # noqa: F401 - engine = get_engine() + engine = engine or get_engine() async with engine.begin() as conn: await conn.run_sync(Base.metadata.create_all) -async def drop_schema() -> None: +async def drop_schema(engine: AsyncEngine | None = None) -> None: """Drop schema.""" import app.models # noqa: F401 - engine = get_engine() + engine = engine or get_engine() async with engine.begin() as conn: await conn.run_sync(Base.metadata.drop_all) diff --git a/app/main.py b/app/main.py index ab4e4a7..ea18ba8 100644 --- a/app/main.py +++ b/app/main.py @@ -15,7 +15,7 @@ from app.api import api_router from app.core.config import get_settings -from app.db.session import create_schema, get_engine, initialize_database +from app.db.session import create_schema, get_engine, get_session_factory, initialize_database from app.observability import ( ObservabilityRoute, initialize_app_observability, @@ -56,16 +56,21 @@ def _resolve_template_directories() -> list[str]: @asynccontextmanager -async def lifespan(_: FastAPI) -> AsyncIterator[None]: +async def lifespan(app: FastAPI) -> AsyncIterator[None]: """Manage application startup and shutdown tasks.""" start_log_listener() settings = get_settings() initialize_database(settings.database_url) - initialize_database_observability(get_engine(), settings) + + engine = get_engine() + app.state.engine = engine + app.state.session_factory = get_session_factory() + + initialize_database_observability(engine, settings) logger.info("application_startup", extra={"database_initialized": True}) if settings.auto_create_schema: - await create_schema() + await create_schema(engine) logger.info("database_schema_created") yield @@ -125,19 +130,18 @@ async def home(request: Request): ) @app.get("/health", tags=["health"]) - async def health() -> dict[str, str]: + async def health(request: Request) -> dict[str, str]: import asyncio import time from sqlalchemy import text - from app.db.session import get_engine from app.observability.metrics import health_check_duration_seconds, health_check_total settings = get_settings() retries = settings.health_check_db_retries timeout = settings.health_check_db_timeout - engine = get_engine() + engine = request.app.state.engine start = time.monotonic() last_error: Exception | None = None diff --git a/tests/test_api.py b/tests/test_api.py index c28e2ae..51eefb5 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -13,13 +13,18 @@ from app import main as app_main from app.main import create_app -from app.db.session import get_session +from app.db.session import get_engine, get_session, get_session_factory @pytest.fixture async def client(db_session: AsyncSession): """Create an AsyncClient with mocked dependency injection.""" app = create_app() + + # httpx ASGITransport does not trigger ASGI lifespan, so populate + # app.state with the engine / factory that conftest already initialised. + app.state.engine = get_engine() + app.state.session_factory = get_session_factory() async def override_get_session(): yield db_session @@ -46,7 +51,7 @@ async def test_health_endpoint(client: AsyncClient): @pytest.mark.asyncio async def test_health_endpoint_database_unavailable(db_session: AsyncSession): """Test the health check reports error after all retries are exhausted.""" - from unittest.mock import AsyncMock, patch + from unittest.mock import AsyncMock app = create_app() @@ -60,8 +65,8 @@ async def override_get_session(): transport = ASGITransport(app=app) async with AsyncClient(transport=transport, base_url="http://test") as ac: - with patch("app.db.session.get_engine", return_value=mock_engine): - response = await ac.get("/health") + app.state.engine = mock_engine + response = await ac.get("/health") app.dependency_overrides.clear() @@ -76,7 +81,7 @@ async def override_get_session(): @pytest.mark.asyncio async def test_health_endpoint_database_recovers_on_retry(db_session: AsyncSession): """Test that health check succeeds when DB fails first then recovers.""" - from unittest.mock import AsyncMock, MagicMock, patch + from unittest.mock import AsyncMock, MagicMock app = create_app() @@ -101,8 +106,8 @@ async def override_get_session(): transport = ASGITransport(app=app) async with AsyncClient(transport=transport, base_url="http://test") as ac: - with patch("app.db.session.get_engine", return_value=mock_engine): - response = await ac.get("/health") + app.state.engine = mock_engine + response = await ac.get("/health") app.dependency_overrides.clear() @@ -124,6 +129,8 @@ async def override_get_session(): yield db_session app.dependency_overrides[get_session] = override_get_session + app.state.engine = get_engine() + app.state.session_factory = get_session_factory() mock_counter = MagicMock() mock_histogram = MagicMock() @@ -147,7 +154,7 @@ async def override_get_session(): @pytest.mark.asyncio async def test_health_endpoint_metrics_recorded_on_failure(db_session: AsyncSession): """Test that health check metrics are recorded on DB failure.""" - from unittest.mock import AsyncMock, MagicMock, call, patch + from unittest.mock import AsyncMock, MagicMock, patch app = create_app() @@ -164,8 +171,8 @@ async def override_get_session(): transport = ASGITransport(app=app) async with AsyncClient(transport=transport, base_url="http://test") as ac: - with patch("app.db.session.get_engine", return_value=mock_engine), \ - patch("app.observability.metrics.health_check_total", mock_counter), \ + app.state.engine = mock_engine + with patch("app.observability.metrics.health_check_total", mock_counter), \ patch("app.observability.metrics.health_check_duration_seconds", mock_histogram): response = await ac.get("/health") diff --git a/tests/test_concurrency.py b/tests/test_concurrency.py index ef444bf..24e9a49 100644 --- a/tests/test_concurrency.py +++ b/tests/test_concurrency.py @@ -5,7 +5,7 @@ import pytest from httpx import ASGITransport, AsyncClient -from app.db.session import get_session, get_session_factory +from app.db.session import get_engine, get_session, get_session_factory from app.main import create_app @@ -15,6 +15,9 @@ async def concurrent_client(db_session): app = create_app() session_factory = get_session_factory() + app.state.engine = get_engine() + app.state.session_factory = session_factory + async def override_get_session(): async with session_factory() as session: yield session From f373b36baa3e2b29e7b08cb7e90fc5e8b91b5272 Mon Sep 17 00:00:00 2001 From: Vladislav Antonov Date: Sun, 22 Mar 2026 20:53:49 +0200 Subject: [PATCH 09/10] docs: clarify .env setup steps in README Expand the environment config step with a table of key variables, their defaults, and when/why to change them. Note that .env is gitignored and that Docker Compose overrides DATABASE_URL. --- README.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 942db94..5bed007 100644 --- a/README.md +++ b/README.md @@ -496,12 +496,23 @@ python3 -m venv .venv pip install -e '.[dev]' ``` -3. Copy environment config and adjust DB credentials if needed: +3. Copy environment config and adjust as needed: ```bash cp .env.example .env ``` +The `.env` file is **not** committed to the repository (it is listed in `.gitignore`). The provided `.env.example` contains sensible defaults for local development. Key variables you may want to review: + +| Variable | Default | Purpose | +|---|---|---| +| `DATABASE_URL` | `postgresql+asyncpg://postgres:postgres@localhost:5432/commerce_demo` | PostgreSQL connection string. Change host/port/credentials to match your local setup, or leave as-is when using Docker Compose (it provisions the DB automatically). | +| `TELEMETRY_ENABLED` | `true` | Set to `false` if you are not running the observability stack. | +| `API_PREFIX` | `/api/v1` | URL prefix for all API routes. | +| `AUTO_CREATE_SCHEMA` | `true` | Creates tables on startup when no Alembic migration has been run yet. | + +> **Tip:** When using **Docker Compose** (Option A below), the compose file passes its own `DATABASE_URL` pointing at the containerized PostgreSQL, so the value in `.env` is only used if you run the dev server directly on the host (Option C). + ### 8.2. Development Server #### Option A: Run full stack with Docker Compose (recommended) From e3b730e51a8900782453e07fcd8f75e4910963ec Mon Sep 17 00:00:00 2001 From: Vladislav Antonov Date: Sun, 22 Mar 2026 20:58:16 +0200 Subject: [PATCH 10/10] chore: release version 0.2.0 --- AGENTS.md | 6 +++--- CHANGELOG.md | 36 +++++++++++++++++++++++++++++++++++- app/main.py | 2 +- app/observability/metrics.py | 2 +- app/observability/setup.py | 2 +- pyproject.toml | 2 +- 6 files changed, 42 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a671efd..e8a5c8c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,7 +6,7 @@ Guide for AI coding agents working on the Commerce System Demo project. Commerce System Demo is a FastAPI-based commerce service that provides RESTful APIs for managing products, categories, and implementing search functionality. The project includes built-in observability with OpenTelemetry metrics, logging, and distributed tracing. -**Current Version**: 0.1.5 (following [Semantic Versioning](https://semver.org/)) +**Current Version**: 0.2.0 (following [Semantic Versioning](https://semver.org/)) ## Setup Commands @@ -109,7 +109,7 @@ app/ ### Docker -- **Build image**: `docker build -t commerce-system-demo:0.1.5 .` +- **Build image**: `docker build -t commerce-system-demo:0.2.0 .` - **View Dockerfile**: Includes Python dependencies, migration scripts, and app code - **Build context**: Includes `scripts/`, `app/`, and `observability/` directories @@ -192,7 +192,7 @@ This project follows [Semantic Versioning 2.0.0](https://semver.org/): - **MINOR**: Backward-compatible new features - **PATCH**: Backward-compatible bug fixes -Current version is **0.1.5** (initial development). Version is defined in: +Current version is **0.2.0** (initial development). Version is defined in: - `pyproject.toml` (project metadata) - `app/main.py` (FastAPI version) - `app/observability/metrics.py` (meter version) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39ee2df..c847042 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,10 +11,44 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Enhanced search with full-text indexing support - Bulk import/export endpoints for products and categories -- Rate limiting and request throttling - Advanced filtering options for product search - Product images storage optimization +## [0.2.0] - 2026-03-22 + +### Added + +- Rate limiting with slowapi — IP-based throttling on search endpoint with configurable default and per-route limits +- `rate_limit_default` and `rate_limit_search` settings in `app/core/config.py` +- Alembic migration framework with async PostgreSQL support (`alembic/` directory, `alembic.ini`) +- Initial Alembic migration (`001_initial_schema.py`) matching the existing `create_all` schema +- `timed_execute_all`, `timed_execute_one` helpers in `app/observability/db_timing.py` for instrumented query execution +- `_escape_like()` utility in `app/services/product_service.py` for sanitizing LIKE metacharacters +- `field_serializer` on `ProductUpdate.image_url` to serialize `AnyHttpUrl` to plain `str` +- Integration tests for category depth limits, cycle detection, self-reference rejection, and LIKE injection +- Documentation in README clarifying `.env` setup steps with a table of key variables and defaults + +### Changed + +- Search query uses `COUNT(*) OVER()` window function instead of a separate count query, eliminating a second database roundtrip +- Category depth validation (`validate_category_parent`) consolidated from N+1 sequential queries to a single recursive CTE +- Category reparent validation (`validate_category_reparent`) consolidated from 4 sequential queries to a single combined CTE query +- `get_session` dependency now reads from `request.app.state.session_factory` (proper DI via app state) instead of module-level globals +- Application lifespan stores `engine` and `session_factory` on `app.state` +- Health endpoint reads engine from `request.app.state.engine` instead of importing `get_engine()` +- `create_schema()` / `drop_schema()` accept an optional `engine` parameter +- Test fixtures set `app.state.engine` and `app.state.session_factory` explicitly (httpx ASGITransport skips ASGI lifespan) + +### Fixed + +- LIKE metacharacters (`%`, `_`, `\`) in search queries are now escaped, preventing wildcard injection +- `ProductUpdate.image_url` now serializes correctly from `AnyHttpUrl` to `str` for database persistence + +### Security + +- Added IP-based rate limiting to protect against request flooding +- Escaped LIKE wildcards in user-supplied search input to prevent pattern injection + ## [0.1.5] - 2026-03-22 ### Fixed diff --git a/app/main.py b/app/main.py index ea18ba8..33db6f6 100644 --- a/app/main.py +++ b/app/main.py @@ -90,7 +90,7 @@ def create_app() -> FastAPI: app = FastAPI( title="Commerce System Demo", - version="0.1.5", + version="0.2.0", lifespan=lifespan, ) app.state.limiter = limiter diff --git a/app/observability/metrics.py b/app/observability/metrics.py index c06f476..6db10d3 100644 --- a/app/observability/metrics.py +++ b/app/observability/metrics.py @@ -3,7 +3,7 @@ from opentelemetry import metrics from opentelemetry.metrics import Counter, Histogram, UpDownCounter -_meter = metrics.get_meter("commerce-system-demo-observability", version="0.1.5") +_meter = metrics.get_meter("commerce-system-demo-observability", version="0.2.0") http_request_duration_seconds: Histogram = _meter.create_histogram( name="commerce_http_request_duration_seconds", diff --git a/app/observability/setup.py b/app/observability/setup.py index 6aa5aad..f78ba72 100644 --- a/app/observability/setup.py +++ b/app/observability/setup.py @@ -76,7 +76,7 @@ def _build_resource(settings: Settings) -> Resource: """Build OpenTelemetry resource attributes from runtime settings.""" attributes = { "service.name": settings.otel_service_name, - "service.version": "0.1.5", + "service.version": "0.2.0", "deployment.environment": settings.otel_environment, } diff --git a/pyproject.toml b/pyproject.toml index c80ae59..125774b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "commerce-system-demo" -version = "0.1.5" +version = "0.2.0" description = "FastAPI commerce service demo" readme = "README.md" requires-python = ">=3.11"