fix: automated review of PR #24

.env.example

@@ -0,0 +1,5 @@
+# Copy this file to .env and fill in real values. Do NOT commit .env to version control.
+POSTGRES_DB=app
+POSTGRES_USER=user
+POSTGRES_PASSWORD=change_me_before_use
+DATABASE_URL=postgresql://user:change_me_before_use@db/app

.gitignore

@@ -9,3 +9,9 @@
 
 # macOS
 .DS_Store
+
+# Environment / secrets
+.env
+
+# Test artifacts
+test.db

Dockerfile

@@ -3,5 +3,7 @@ RUN apt-get update && apt-get install -y libpq-dev
 COPY . /app
 WORKDIR /app
 RUN pip install -r requirements.txt
+RUN useradd -m appuser
+USER appuser
 EXPOSE 8000
-CMD ["uvicorn", "main:app", "--host", "0.0.0.0", "--port", "8000", "--log-level", "trace"]
+CMD ["uvicorn", "main:app", "--host", "0.0.0.0", "--port", "8000", "--log-level", "info"]

conftest.py

@@ -1,28 +1,41 @@
+import os
 import pytest
-from sqlalchemy import create_engine, inspect, text
+from sqlalchemy import create_engine, inspect
 from sqlalchemy.orm import sessionmaker
 from fastapi.testclient import TestClient
-from main import create_app
-from dependencies import get_db, database_url
-from models import Base
+
+# Use an in-process SQLite database for all tests so no external Postgres is needed.
+TEST_DATABASE_URL = "sqlite:///./test.db"
+
+# Set DATABASE_URL before importing application modules so dependencies.py does not raise.
+os.environ.setdefault("DATABASE_URL", TEST_DATABASE_URL)
+
+from main import create_app  # noqa: E402  (import after env setup)
+from dependencies import get_db  # noqa: E402
+from models import Base  # noqa: E402
+
 
 @pytest.fixture(scope="session")
 def test_engine():
-    """Create test database engine"""
-    test_engine = create_engine(database_url)
-    Base.metadata.create_all(bind=test_engine)
-    yield test_engine
-    Base.metadata.drop_all(bind=test_engine)
+    """Create an in-memory SQLite engine for the test session."""
+    engine = create_engine(
+        TEST_DATABASE_URL, connect_args={"check_same_thread": False}
+    )
+    Base.metadata.create_all(bind=engine)
+    yield engine
+    Base.metadata.drop_all(bind=engine)
 
 
 @pytest.fixture(scope="function")
 def test_db(test_engine):
-    """Create test database session that rolls back after each test"""
+    """Create test database session that rolls back after each test."""
     connection = test_engine.connect()
     transaction = connection.begin()
     db = sessionmaker(
-        autocommit=False, autoflush=False, bind=connection,
-        join_transaction_mode="create_savepoint"
+        autocommit=False,
+        autoflush=False,
+        bind=connection,
+        join_transaction_mode="create_savepoint",
     )()
     try:
         yield db
@@ -34,13 +47,13 @@ def test_db(test_engine):
 
 @pytest.fixture
 def test_app(test_engine):
-    """Create test FastAPI application with test database"""
+    """Create test FastAPI application with test database override."""
 
     def override_get_db():
-        TestingSessionLocal = sessionmaker(
+        session_factory = sessionmaker(
             autocommit=False, autoflush=False, bind=test_engine
         )
-        db = TestingSessionLocal()
+        db = session_factory()
         try:
             yield db
         finally:
@@ -53,5 +66,5 @@ def override_get_db():
 
 @pytest.fixture
 def client(test_app):
-    """Create test client"""
+    """Create test HTTP client."""
     return TestClient(test_app)

docker-compose.yml

@@ -24,7 +24,7 @@ services:
       POSTGRES_USER: ${POSTGRES_USER}
       POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}
     ports:
-      - "5432:5432"
+      - "127.0.0.1:5432:5432"
     volumes:
       - postgres_data:/var/lib/postgresql/data
       - ./db.sql:/docker-entrypoint-initdb.d/db.sql

main.py

@@ -1,6 +1,6 @@
-from fastapi import FastAPI, Depends
+from fastapi import FastAPI
 from routers import router
-from dependencies import init_db, get_db
+from dependencies import init_db
 
 
 def create_app() -> FastAPI:
@@ -10,8 +10,8 @@ def create_app() -> FastAPI:
     # Initialize database
     init_db()
 
-    # Include routers
-    app.include_router(router, prefix="/api", dependencies=[Depends(get_db)])
+    # Include routers (each route declares its own db dependency)
+    app.include_router(router, prefix="/api")
 
     return app
 

models.py

@@ -7,8 +7,6 @@
 class Base(DeclarativeBase):
     """Base class for all database models"""
 
-    pass
-
 
 class Book(Base):
     """Book model"""
@@ -18,13 +16,16 @@ class Book(Base):
     id: Mapped[int] = mapped_column(primary_key=True, index=True)
     title: Mapped[str] = mapped_column(String(255), index=True)
     author: Mapped[str] = mapped_column(String(255))
+    isbn: Mapped[str] = mapped_column(String(255))
+
 
 # Pydantic models
 class BookIn(BaseModel):
     """Pydantic model for book input"""
 
     title: str
     author: str
+    isbn: str
 
 
 class BookOut(BaseModel):
@@ -33,5 +34,6 @@ class BookOut(BaseModel):
     id: int
     title: str
     author: str
+    isbn: str
 
     model_config = ConfigDict(from_attributes=True)

repositories.py

@@ -4,7 +4,7 @@
 
 # Create a new book
 def create_book(db: Session, book: models.BookIn):
-    db_book = models.Book(title=book.title, author=book.author)
+    db_book = models.Book(title=book.title, author=book.author, isbn=book.isbn)
     db.add(db_book)
     db.commit()
     db.refresh(db_book)
@@ -27,6 +27,7 @@ def update_book(db: Session, book_id: int, book: models.BookIn):
     if db_book:
         db_book.title = book.title
         db_book.author = book.author
+        db_book.isbn = book.isbn
         db.commit()
         db.refresh(db_book)
         return db_book

routers.py

@@ -1,11 +1,17 @@
+import logging
+from typing import List
+
 from fastapi import APIRouter, Depends, HTTPException, status
+from sqlalchemy.exc import IntegrityError, SQLAlchemyError
 from sqlalchemy.orm import Session
-from typing import List
+
 import models
 import repositories
 from dependencies import get_db
 
-# Create router with prefix
+logger = logging.getLogger(__name__)
+
+# Router for book endpoints
 router = APIRouter()
 
 
@@ -15,20 +21,38 @@
 def create_book(book: models.BookIn, db: Session = Depends(get_db)):
     try:
         return repositories.create_book(db=db, book=book)
+    except IntegrityError as e:
+        logger.exception("Integrity error creating book")
+        raise HTTPException(
+            status_code=status.HTTP_409_CONFLICT, detail="Book already exists"
+        ) from e
+    except SQLAlchemyError as e:
+        logger.exception("Database error creating book")
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail="Internal server error",
+        ) from e
     except Exception as e:
-        raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))
+        logger.exception("Unexpected error creating book")
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid request data"
+        ) from e
 
 
 @router.get(
     "/books/", response_model=List[models.BookOut], status_code=status.HTTP_200_OK
 )
 def get_books(skip: int = 0, limit: int = 10, db: Session = Depends(get_db)):
+    # Cap limit to prevent resource exhaustion
+    limit = min(limit, 100)
     try:
         return repositories.get_books(db=db, skip=skip, limit=limit)
     except Exception as e:
+        logger.exception("Error retrieving books")
         raise HTTPException(
-            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=str(e)
-        )
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail="Internal server error",
+        ) from e
 
 
 @router.get(