diff --git a/backend/src/backend/db.py b/backend/src/backend/db.py index f7e0ec44..8639823c 100644 --- a/backend/src/backend/db.py +++ b/backend/src/backend/db.py @@ -1,9 +1,6 @@ """DB connection and session management for SQLModel.""" -from collections.abc import Coroutine -from functools import wraps -from typing import Any -from typing import Callable +from collections.abc import AsyncGenerator from typing import TypeVar from sqlalchemy.ext.asyncio import AsyncSession @@ -34,26 +31,7 @@ T = TypeVar("T") async_maker = async_sessionmaker(engine) -def sessionize( - func: Callable[..., Coroutine[Any, Any, T]], -) -> Callable[..., Coroutine[Any, Any, T]]: - """Decorate a sql function and inject a session if it doesn't exist. - - This checks the function signature for a `session` parameter. If it does - not exist, it creates a new session and passes it to the function. - - This is a useful way to ensure that all database functions have access - to a session without having to pass it explicitly every time. The only - requirement is that the function must accept a `session` parameter. - """ - - @wraps(func) - async def wrapper( - *args: Any, session: AsyncSession | None = None, **kwargs: Any - ) -> T: - if session is None: - async with async_maker() as session: - return await func(*args, session=session, **kwargs) - return await func(*args, session=session, **kwargs) - - return wrapper +async def get_session() -> AsyncGenerator[AsyncSession, None]: # pragma: no cover + """Inject a new DB session.""" + async with async_maker() as session: + yield session diff --git a/backend/src/backend/dependencies.py b/backend/src/backend/dependencies.py new file mode 100644 index 00000000..7391932f --- /dev/null +++ b/backend/src/backend/dependencies.py @@ -0,0 +1,11 @@ +"""Dependency definitions for FastAPI routes.""" + +from typing import Annotated + +from fastapi import Depends +from sqlalchemy.ext.asyncio import AsyncSession + +from backend.db import get_session + + +SessionDep = Annotated[AsyncSession, Depends(get_session)] diff --git a/backend/src/backend/logging.py b/backend/src/backend/logging.py new file mode 100644 index 00000000..951acb9c --- /dev/null +++ b/backend/src/backend/logging.py @@ -0,0 +1,109 @@ +"""Logging configuration for numinar coding project backend.""" + +import logging +import os +import uuid +from typing import Any +from typing import Callable + +from fastapi import FastAPI +from fastapi import Request +from fastapi import Response + + +class RedactFilter(logging.Filter): + """Filter to redact sensitive information in logs.""" + + def filter(self, record: logging.LogRecord) -> bool: + """Redact sensitive information from log records.""" + if hasattr(record, "msg"): + record.msg = ( + str(record.msg) + .replace("password", "REDACTED") + .replace("token", "REDACTED") + ) + return True + + +def setup_logging(app: FastAPI) -> None: + """Configure logging for a FastAPI application. + + Args: + app (FastAPI): The FastAPI application instance to configure logging for. + """ + # Get log level from environment variable, default to INFO + log_level_str: str = os.getenv("LOG_LEVEL", "INFO").upper() + log_level: int = getattr(logging, log_level_str, logging.INFO) + + # Configure logger + logger: logging.Logger = logging.getLogger() + logger.setLevel(log_level) + + # Clear any existing handlers to avoid duplicate logs + logger.handlers.clear() + + # Set up plain text formatter + formatter: logging.Formatter = logging.Formatter( + fmt="%(asctime)s - %(name)s - %(levelname)s - %(message)s", + datefmt="%Y-%m-%d %H:%M:%S", + ) + + # Console handler for Docker + console_handler = logging.StreamHandler() + console_handler.setFormatter(formatter) + console_handler.addFilter(RedactFilter()) + logger.addHandler(console_handler) + + # FastAPI middleware for request logging + @app.middleware("http") + async def log_requests( + request: Request, call_next: Callable[[Request], Any] + ) -> Response: + """Middleware to log incoming requests and their outcomes.""" + request_id: str = str(uuid.uuid4()) + logger.info( + "Request started", + extra={ + "request_id": request_id, + "method": request.method, + "url": str(request.url), + "client_ip": request.client.host if request.client else "unknown", + }, + ) + try: + response: Response = await call_next(request) + logger.info( + "Request completed", + extra={ + "request_id": request_id, + "status_code": response.status_code, + }, + ) + return response + except Exception: + logger.exception( + "Request failed", + extra={ + "request_id": request_id, + "url": str(request.url), + }, + ) + raise + + _ = log_requests # Silence unused function warning + + # Exception handler for unhandled exceptions + async def custom_exception_handler( + request: Request, exc: Exception + ) -> dict[str, str]: + """Handle unhandled exceptions and log them.""" + logger.exception( + "Unhandled exception", + extra={ + "url": str(request.url), + "method": request.method, + }, + ) + return {"detail": "Internal server error"} + + app.add_exception_handler(Exception, custom_exception_handler) # type: ignore diff --git a/backend/src/backend/main.py b/backend/src/backend/main.py index c55ff01f..637c5cf8 100644 --- a/backend/src/backend/main.py +++ b/backend/src/backend/main.py @@ -5,6 +5,7 @@ from typing import Any from fastapi import FastAPI from backend import config +from backend.logging import setup_logging from backend.models import create_db_and_tables @@ -28,6 +29,8 @@ app_configs["lifespan"] = lifespan app = FastAPI(**app_configs) +setup_logging(app) + @app.get("/") async def root(): diff --git a/backend/src/backend/sql.py b/backend/src/backend/sql.py index 30956c2a..9af3d061 100644 --- a/backend/src/backend/sql.py +++ b/backend/src/backend/sql.py @@ -9,62 +9,45 @@ Note: from datetime import datetime from typing import TypedDict from typing import Unpack +from typing import cast from sqlalchemy import delete from sqlalchemy import select from sqlalchemy import update +from sqlalchemy.exc import NoResultFound from sqlalchemy.ext.asyncio import AsyncSession -from backend.db import sessionize from backend.models import Booking from backend.models import Invitee from backend.models import Room from backend.models import User -@sessionize -async def get_users( - session: AsyncSession = None, # type: ignore -): +async def get_users(session: AsyncSession) -> list[User]: """Retrieve all users from the database.""" result = await session.execute(select(User)) - return result.scalars().all() + return cast(list[User], result.scalars().all()) -@sessionize -async def get_user_by_email( - email: str, - session: AsyncSession = None, # type: ignore -): +async def get_user_by_email(session: AsyncSession, email: str) -> User: """Retrieve a user by their email address.""" result = await session.execute(select(User).where(User.email == email)) - return result.scalar_one_or_none() + return result.scalar_one() -@sessionize -async def get_rooms( - session: AsyncSession = None, # type: ignore -): +async def get_rooms(session: AsyncSession) -> list[Room]: """Retrieve all rooms from the database.""" result = await session.execute(select(Room)) - return result.scalars().all() + return cast(list[Room], result.scalars().all()) -@sessionize -async def get_room( - room_id: int, - session: AsyncSession = None, # type: ignore -): +async def get_room(session: AsyncSession, room_id: int) -> Room: """Retrieve a room by its ID.""" result = await session.execute(select(Room).where(Room.id == room_id)) - return result.scalar_one_or_none() + return result.scalar_one() -@sessionize -async def new_room( - room: Room, - session: AsyncSession = None, # type: ignore -): +async def new_room(session: AsyncSession, room: Room) -> Room: """Create a new room in the database.""" session.add(room) await session.commit() @@ -80,55 +63,36 @@ class RoomParams(TypedDict): capacity: int -@sessionize async def update_room( - room_id: int, - session: AsyncSession = None, # type: ignore - **kwargs: Unpack[RoomParams], -): + session: AsyncSession, room_id: int, **kwargs: Unpack[RoomParams] +) -> Room | None: """Update an existing room.""" stmt = update(Room).where(Room.id == room_id).values(**kwargs) await session.execute(stmt) await session.commit() - return await get_room(room_id, session=session) + return await get_room(session, room_id) -@sessionize -async def delete_room( - room_id: int, - session: AsyncSession = None, # type: ignore -): +async def delete_room(session: AsyncSession, room_id: int) -> None: """Delete a room from the database.""" stmt = delete(Room).where(Room.id == room_id) await session.execute(stmt) await session.commit() -@sessionize -async def get_bookings_for_room( - room_id: int, - session: AsyncSession = None, # type: ignore -): +async def get_bookings_for_room(session: AsyncSession, room_id: int) -> list[Booking]: """Retrieve all bookings for a specific room.""" result = await session.execute(select(Booking).where(Booking.room_id == room_id)) - return result.scalars().all() + return cast(list[Booking], result.scalars().all()) -@sessionize -async def get_booking( - booking_id: int, - session: AsyncSession = None, # type: ignore -): +async def get_booking(session: AsyncSession, booking_id: int) -> Booking: """Retrieve a booking by its ID.""" result = await session.execute(select(Booking).where(Booking.id == booking_id)) - return result.scalar_one_or_none() + return result.scalar_one() -@sessionize -async def new_booking( - booking: Booking, - session: AsyncSession = None, # type: ignore -): +async def new_booking(session: AsyncSession, booking: Booking) -> Booking: """Create a new booking in the database.""" session.add(booking) await session.commit() @@ -143,52 +107,41 @@ class BookingParams(TypedDict): end_time: datetime -@sessionize async def update_booking( + session: AsyncSession, booking_id: int, - session: AsyncSession = None, # type: ignore **kwargs: Unpack[BookingParams], -): +) -> Booking | None: """Update an existing booking.""" stmt = update(Booking).where(Booking.id == booking_id).values(**kwargs) await session.execute(stmt) await session.commit() - return await get_booking(booking_id, session=session) + return await get_booking(session, booking_id) -@sessionize -async def delete_booking( - booking_id: int, - session: AsyncSession = None, # type: ignore -): +async def delete_booking(session: AsyncSession, booking_id: int) -> None: """Delete a booking from the database.""" stmt = delete(Booking).where(Booking.id == booking_id) await session.execute(stmt) await session.commit() -@sessionize async def get_invitees_for_booking( - booking_id: int, - session: AsyncSession = None, # type: ignore -): + session: AsyncSession, booking_id: int +) -> list[User]: """Retrieve all invitees for a specific booking.""" result = await session.execute( select(Invitee.user).where(Invitee.booking_id == booking_id) ) - return result.scalars().all() + return cast(list[User], result.scalars().all()) -@sessionize -async def add_invitee_to_booking( - booking_id: int, - email: str, - session: AsyncSession = None, # type: ignore -): +async def add_invitee_to_booking(session: AsyncSession, booking_id: int, email: str): """Add an invitee to a booking.""" - user = await get_user_by_email(email, session=session) - if not user: - raise ValueError(f"User with email {email} does not exist.") + try: + user = await get_user_by_email(session, email) + except NoResultFound as e: + raise ValueError(f"User with email {email} does not exist.") from e invitee = Invitee(booking_id=booking_id, user_id=user.id) session.add(invitee) @@ -196,16 +149,14 @@ async def add_invitee_to_booking( return invitee -@sessionize async def remove_invitee_from_booking( - booking_id: int, - email: str, - session: AsyncSession = None, # type: ignore + session: AsyncSession, booking_id: int, email: str ): """Remove an invitee from a booking.""" - user = await get_user_by_email(email, session=session) - if not user: - raise ValueError(f"User with email {email} does not exist.") + try: + user = await get_user_by_email(session, email) + except NoResultFound as e: + raise ValueError(f"User with email {email} does not exist.") from e stmt = delete(Invitee).where( Invitee.booking_id == booking_id, Invitee.user_id == user.id diff --git a/backend/tests/test_db.py b/backend/tests/test_db.py deleted file mode 100644 index 5483085d..00000000 --- a/backend/tests/test_db.py +++ /dev/null @@ -1,117 +0,0 @@ -"""Test cases for the SQL module in the Numinar coding project backend. - -This primarily focuses on testing the sessionize decorator to ensure it correctly -manages database sessions for asynchronous functions.i -""" - -from typing import Any -from typing import AsyncGenerator -from unittest.mock import AsyncMock -from unittest.mock import patch - -import pytest -import pytest_asyncio -from sqlalchemy.ext.asyncio import AsyncSession - -from backend.db import sessionize - - -@pytest_asyncio.fixture -async def mock_async_sessionmaker() -> AsyncGenerator[AsyncMock, None]: - """Fixture to mock the async sessionmaker for testing sessionize decorator. - - Yields: - AsyncMock: A mocked async sessionmaker that returns a mocked AsyncSession. - """ - with patch("backend.db.async_sessionmaker") as mock_sessionmaker: - mock_session: AsyncMock = AsyncMock(spec=AsyncSession) - mock_sessionmaker.return_value = AsyncMock(return_value=mock_session) - yield mock_sessionmaker - - -@pytest.mark.asyncio -async def test_sessionize_decorator_with_no_session( - mock_async_sessionmaker: AsyncMock, -) -> None: - """Test the sessionize decorator when no session is provided. - - Verifies that the decorator creates a new session and passes it to the function, - and that the function executes correctly. - """ - - @sessionize - async def test_func(session: AsyncSession | None = None) -> str: - setattr(test_func, "session", session) # noqa: B010 - return "success" - - result: str = await test_func() - assert result == "success" - assert test_func.session is not None # type: ignore - - -@pytest.mark.asyncio -async def test_sessionize_decorator_with_session( - mock_async_sessionmaker: AsyncMock, -) -> None: - """Test the sessionize decorator when a session is provided. - - Verifies that the decorator uses the provided session without creating a new one, - and that the function executes correctly. - """ - - @sessionize - async def test_func(session: AsyncSession | None = None) -> str: - setattr(test_func, "session", session) # noqa: B010 - return "success" - - mock_session: AsyncMock = AsyncMock(spec=AsyncSession) - result: str = await test_func(session=mock_session) - assert result == "success" - assert mock_session is test_func.session # type: ignore - - -@pytest.mark.asyncio -async def test_sessionize_decorator_error_handling( - mock_async_sessionmaker: AsyncMock, -) -> None: - """Test the sessionize decorator's error handling. - - Verifies that the decorator properly handles exceptions raised by the decorated - function and ensures the session is created when none is provided. - """ - - @sessionize - async def test_func(session: AsyncSession) -> None: - setattr(test_func, "session", session) # noqa: B010 - raise ValueError("Test error") - - with pytest.raises(ValueError, match="Test error"): - await test_func() - - -@pytest.mark.asyncio -@pytest.mark.parametrize( - "func_return_value", - ["result1", 42, None], - ids=["string", "integer", "none"], -) -async def test_sessionize_decorator_return_types( - mock_async_sessionmaker: AsyncMock, func_return_value: Any -) -> None: - """Test the sessionize decorator with different return types. - - Verifies that the decorator correctly handles various return types from the - decorated function. - - Args: - mock_async_sessionmaker: Mocked async sessionmaker. - func_return_value: The value to be returned by the test function. - """ - - @sessionize - async def test_func(session: AsyncSession) -> Any: - setattr(test_func, "session", session) # noqa: B010 - return func_return_value - - result: Any = await test_func() - assert result == func_return_value