diff --git a/src/cartsnitch_api/cache.py b/src/cartsnitch_api/cache.py index 6766a8c..a5aa91b 100644 --- a/src/cartsnitch_api/cache.py +++ b/src/cartsnitch_api/cache.py @@ -40,7 +40,7 @@ class CacheClient: return None if isinstance(value, bytes): return value.decode("utf-8", errors="replace") - return value + return str(value) async def set(self, key: str, value: str, ttl_seconds: int = 300) -> None: if not self._client: diff --git a/src/cartsnitch_api/main.py b/src/cartsnitch_api/main.py index 5a72b6b..66c1854 100644 --- a/src/cartsnitch_api/main.py +++ b/src/cartsnitch_api/main.py @@ -25,6 +25,12 @@ from cartsnitch_api.routes.user import router as user_router @asynccontextmanager async def lifespan(app: FastAPI): + # Lazy import: keep `dispose_engine` out of the top-level imports so a + # stale or partially-built database.py never breaks module load on + # container start. The function is required for graceful pool cleanup + # on shutdown; if the import fails, the cache_client.close() that + # follows the yield would mask it. See CAR-1135 for the original + # ImportError that motivated this pattern. from cartsnitch_api.database import dispose_engine await cache_client.initialize() diff --git a/src/cartsnitch_api/middleware/rate_limit.py b/src/cartsnitch_api/middleware/rate_limit.py index c6d5f21..ff1b218 100644 --- a/src/cartsnitch_api/middleware/rate_limit.py +++ b/src/cartsnitch_api/middleware/rate_limit.py @@ -121,10 +121,6 @@ if settings.rate_limit_redis_enabled: logger.warning("Failed to connect to Redis for rate limiting, using in-memory: %s", e) _use_redis = False -_public_limiter: RateLimitBackend -_auth_limiter: RateLimitBackend -_auth_strict_limiter: RateLimitBackend - if _use_redis and _redis_client: _public_limiter = RedisSlidingWindow( _redis_client, settings.rate_limit_requests, settings.rate_limit_window_seconds @@ -151,8 +147,8 @@ def _get_client_ip(request: Request) -> str: """Extract client IP, respecting X-Forwarded-For behind a reverse proxy.""" forwarded = request.headers.get("x-forwarded-for") if forwarded: - return forwarded.split(",")[0].strip() - return request.client.host if request.client else "unknown" + return str(forwarded.split(",")[0].strip()) + return str(request.client.host) if request.client else "unknown" def _get_rate_limit_key(request: Request) -> tuple[str, RateLimitBackend]: diff --git a/tests/conftest.py b/tests/conftest.py index 1958022..133f726 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -117,7 +117,6 @@ def _register_event_listeners(): event.listen(cls, "before_insert", _set_timestamp_defaults) - TEST_JWT_SECRET = secrets.token_urlsafe(32) TEST_SERVICE_KEY = secrets.token_urlsafe(32) TEST_FERNET_KEY = "7reF42nmTwbdN21PBoubGp7h_FU8qSimstmlaMLoRK8=" diff --git a/tests/test_openapi.py b/tests/test_openapi.py index 2311567..1abea55 100644 --- a/tests/test_openapi.py +++ b/tests/test_openapi.py @@ -3,8 +3,22 @@ import pytest from httpx import ASGITransport, AsyncClient +from cartsnitch_api.database import dispose_engine from cartsnitch_api.main import app + +def test_dispose_engine_importable_from_database(): + """Regression for CAR-1135: api main.py used to import dispose_engine + at module level. A stale database.py (no dispose_engine) crashed the + container at import time with ImportError on line 9. The fix moved + the import inside the lifespan function, but `dispose_engine` must + still be importable from `cartsnitch_api.database` for the lifespan + teardown to actually close pooled connections. + """ + assert callable(dispose_engine) + assert dispose_engine.__name__ == "dispose_engine" + + EXPECTED_ROUTES = [ # Auth (3 — register/login/refresh are handled by Better-Auth service) ("get", "/auth/me"),