From 3d4869501bf247175f4c7de74f9c89a0a84e13c7 Mon Sep 17 00:00:00 2001 From: vikram-blaxel Date: Mon, 11 May 2026 16:04:12 +0530 Subject: [PATCH 1/2] feat: add ISBN field to model --- models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/models.py b/models.py index bd73c60..dd91182 100644 --- a/models.py +++ b/models.py @@ -18,6 +18,7 @@ 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), nullable=False) # Pydantic models class BookIn(BaseModel): From a105ad1c880834d1f622b1a0d25aed1e6f852243 Mon Sep 17 00:00:00 2001 From: Reviewer Swarm Date: Mon, 11 May 2026 21:39:19 +0000 Subject: [PATCH 2/2] fix: automated fixes for PR #24 --- .env.example | 6 ++ .gitignore | 6 ++ Dockerfile | 4 +- conftest.py | 50 +++++++++++----- dependencies.py | 5 +- docker-compose.yml | 2 +- main.py | 8 +-- models.py | 10 +++- repositories.py | 13 ++-- requirements.txt | 14 ++--- routers.py | 25 ++++++-- test_main.py | 146 ++++++++++++++++++++++++++++++++++++++++----- 12 files changed, 231 insertions(+), 58 deletions(-) create mode 100644 .env.example diff --git a/.env.example b/.env.example new file mode 100644 index 0000000..085b7fe --- /dev/null +++ b/.env.example @@ -0,0 +1,6 @@ +# Copy this file to .env and fill in real values. +# Never commit .env to version control. +POSTGRES_DB=app +POSTGRES_USER=user +POSTGRES_PASSWORD=CHANGE_ME +DATABASE_URL=postgresql://user:CHANGE_ME@db/app diff --git a/.gitignore b/.gitignore index aa0926b..627cee4 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,9 @@ # macOS .DS_Store + +# Environment / secrets – never commit real credentials +.env + +# SQLite test database artefact +*.db diff --git a/Dockerfile b/Dockerfile index b482401..2fed3e5 100644 --- a/Dockerfile +++ b/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"] diff --git a/conftest.py b/conftest.py index b9cb0aa..e708a2c 100644 --- a/conftest.py +++ b/conftest.py @@ -1,18 +1,34 @@ -import pytest -from sqlalchemy import create_engine, inspect, text -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 +import os + +# Set a default DATABASE_URL for tests when one is not already provided. +# This allows the test suite to run without a live PostgreSQL instance +# by falling back to an in-process SQLite database. +if not os.getenv("DATABASE_URL"): + os.environ["DATABASE_URL"] = "sqlite:///./test.db" + +import pytest # noqa: E402 +from sqlalchemy import create_engine, inspect # noqa: E402, F401 +from sqlalchemy.orm import sessionmaker # noqa: E402 +from fastapi.testclient import TestClient # noqa: E402 +from main import create_app # noqa: E402 +from dependencies import get_db # noqa: E402 +from models import Base # noqa: E402 + +_DATABASE_URL = os.environ["DATABASE_URL"] + @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) + engine = create_engine( + _DATABASE_URL, + connect_args={"check_same_thread": False} + if _DATABASE_URL.startswith("sqlite") + else {}, + ) + Base.metadata.create_all(bind=engine) + yield engine + Base.metadata.drop_all(bind=engine) @pytest.fixture(scope="function") @@ -37,14 +53,18 @@ def test_app(test_engine): """Create test FastAPI application with test database""" def override_get_db(): - TestingSessionLocal = sessionmaker( - autocommit=False, autoflush=False, bind=test_engine - ) - db = TestingSessionLocal() + connection = test_engine.connect() + transaction = connection.begin() + db = sessionmaker( + autocommit=False, autoflush=False, bind=connection, + join_transaction_mode="create_savepoint" + )() try: yield db finally: db.close() + transaction.rollback() + connection.close() app = create_app() app.dependency_overrides[get_db] = override_get_db diff --git a/dependencies.py b/dependencies.py index eb51233..3f88207 100644 --- a/dependencies.py +++ b/dependencies.py @@ -1,3 +1,4 @@ +import logging from typing import Generator import os from dotenv import load_dotenv @@ -6,6 +7,8 @@ from sqlalchemy.exc import SQLAlchemyError from models import Base +logger = logging.getLogger(__name__) + load_dotenv() database_url = os.getenv("DATABASE_URL") if not database_url: @@ -23,7 +26,7 @@ def init_db(): try: Base.metadata.create_all(bind=engine) except SQLAlchemyError as e: - print(f"Error initializing the database: {e}") + logger.error("Error initializing the database: %s", e) raise diff --git a/docker-compose.yml b/docker-compose.yml index d222eda..e4222f3 100644 --- a/docker-compose.yml +++ b/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 diff --git a/main.py b/main.py index 5d31686..9d933f2 100644 --- a/main.py +++ b/main.py @@ -1,17 +1,17 @@ -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: """Create and configure the FastAPI application""" - app = FastAPI() + app = FastAPI(docs_url=None, redoc_url=None) # Initialize database init_db() # Include routers - app.include_router(router, prefix="/api", dependencies=[Depends(get_db)]) + app.include_router(router, prefix="/api") return app diff --git a/models.py b/models.py index dd91182..f87e6fc 100644 --- a/models.py +++ b/models.py @@ -1,14 +1,15 @@ +import logging from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column from sqlalchemy import String from pydantic import BaseModel, ConfigDict +logger = logging.getLogger(__name__) + # SQLAlchemy models class Base(DeclarativeBase): """Base class for all database models""" - pass - class Book(Base): """Book model""" @@ -18,7 +19,8 @@ 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), nullable=False) + isbn: Mapped[str] = mapped_column(String(17)) + # Pydantic models class BookIn(BaseModel): @@ -26,6 +28,7 @@ class BookIn(BaseModel): title: str author: str + isbn: str class BookOut(BaseModel): @@ -34,5 +37,6 @@ class BookOut(BaseModel): id: int title: str author: str + isbn: str model_config = ConfigDict(from_attributes=True) diff --git a/repositories.py b/repositories.py index dc9ff88..f946e03 100644 --- a/repositories.py +++ b/repositories.py @@ -3,8 +3,8 @@ # Create a new book -def create_book(db: Session, book: models.BookIn): - db_book = models.Book(title=book.title, author=book.author) +def create_book(db: Session, book: models.BookIn) -> models.Book: + db_book = models.Book(title=book.title, author=book.author, isbn=book.isbn) db.add(db_book) db.commit() db.refresh(db_book) @@ -12,21 +12,22 @@ def create_book(db: Session, book: models.BookIn): # Get all books -def get_books(db: Session, skip: int = 0, limit: int = 10): +def get_books(db: Session, skip: int = 0, limit: int = 10) -> list[models.Book]: return db.query(models.Book).offset(skip).limit(limit).all() # Get a book by ID -def get_book(db: Session, book_id: int): +def get_book(db: Session, book_id: int) -> models.Book | None: return db.query(models.Book).filter(models.Book.id == book_id).first() # Update a book -def update_book(db: Session, book_id: int, book: models.BookIn): +def update_book(db: Session, book_id: int, book: models.BookIn) -> models.Book | None: db_book = db.query(models.Book).filter(models.Book.id == book_id).first() 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 @@ -34,7 +35,7 @@ def update_book(db: Session, book_id: int, book: models.BookIn): # Delete a book -def delete_book(db: Session, book_id: int): +def delete_book(db: Session, book_id: int) -> models.Book | None: db_book = db.query(models.Book).filter(models.Book.id == book_id).first() if db_book: db.delete(db_book) diff --git a/requirements.txt b/requirements.txt index 395f82b..fd5e3a1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ -fastapi[standard]>=0.135.3 -sqlalchemy>=2.0.49 -psycopg2-binary>=2.9.0 -pydantic>=2.12.5 -pytest>=9.0.3 -python-dotenv>=1.2.2 -uvicorn[standard]>=0.44.0 +fastapi[standard]==0.136.1 +sqlalchemy==2.0.49 +psycopg2-binary==2.9.12 +pydantic==2.13.4 +pytest==9.0.3 +python-dotenv==1.2.2 +uvicorn[standard]==0.46.0 diff --git a/routers.py b/routers.py index db667c3..3a5008b 100644 --- a/routers.py +++ b/routers.py @@ -1,10 +1,13 @@ +import logging from fastapi import APIRouter, Depends, HTTPException, status from sqlalchemy.orm import Session -from typing import List +from sqlalchemy.exc import IntegrityError import models import repositories from dependencies import get_db +logger = logging.getLogger(__name__) + # Create router with prefix router = APIRouter() @@ -15,20 +18,32 @@ 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.error("IntegrityError creating book: %s", e) + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="An internal error occurred.", + ) from e except Exception as e: - raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) + logger.error("Unexpected error creating book: %s", e) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="An internal error occurred.", + ) from e @router.get( - "/books/", response_model=List[models.BookOut], status_code=status.HTTP_200_OK + "/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)): try: return repositories.get_books(db=db, skip=skip, limit=limit) except Exception as e: + logger.error("Unexpected error retrieving books: %s", e) raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=str(e) - ) + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="An internal error occurred.", + ) from e @router.get( diff --git a/test_main.py b/test_main.py index 3fea397..296cb0b 100644 --- a/test_main.py +++ b/test_main.py @@ -1,71 +1,187 @@ from repositories import create_book, get_books, get_book, update_book, delete_book from models import BookIn -from sqlalchemy import create_engine, inspect +from sqlalchemy import inspect # Test data constants TEST_BOOKS = [ - {"title": "Carrie", "author": "Stephen King"}, - {"title": "Ready Player One", "author": "Ernest Cline"}, + {"title": "Carrie", "author": "Stephen King", "isbn": "9780385533454"}, + {"title": "Ready Player One", "author": "Ernest Cline", "isbn": "9780307887443"}, ] + class TestMainApp: def test_create_app(self, test_app): - """Test application creation""" + """Test that the FastAPI application is created successfully""" assert test_app is not None def test_database_initialization(self, test_engine): - """Test database initialization""" + """Test that the books table is created during database initialization""" inspector = inspect(test_engine) assert "books" in inspector.get_table_names() + # Repository Tests class TestBookRepository: def test_create_book(self, test_db): - """Test creating a new book""" + """Test that create_book persists title, author, and isbn and returns an auto-assigned id""" book = create_book(test_db, BookIn(**TEST_BOOKS[0])) assert book.title == TEST_BOOKS[0]["title"] assert book.author == TEST_BOOKS[0]["author"] + assert book.isbn == TEST_BOOKS[0]["isbn"] assert book.id is not None def test_get_books(self, test_db): - """Test getting all books""" + """Test that get_books returns all previously created books""" book1 = create_book(test_db, BookIn(**TEST_BOOKS[0])) book2 = create_book(test_db, BookIn(**TEST_BOOKS[1])) books = get_books(test_db) - #assert len(books) >= 2 + assert len(books) >= 2 assert any(b.id == book1.id for b in books) assert any(b.id == book2.id for b in books) def test_get_book(self, test_db): - """Test getting a specific book""" + """Test that get_book retrieves the correct book by ID""" created_book = create_book(test_db, BookIn(**TEST_BOOKS[0])) retrieved_book = get_book(test_db, created_book.id) assert retrieved_book is not None assert retrieved_book.id == created_book.id assert retrieved_book.title == TEST_BOOKS[0]["title"] + assert retrieved_book.isbn == TEST_BOOKS[0]["isbn"] def test_update_book(self, test_db): - """Test updating a book""" + """Test that update_book modifies title, author, and isbn correctly""" book = create_book(test_db, BookIn(**TEST_BOOKS[0])) updated_data = BookIn(**TEST_BOOKS[1]) updated_book = update_book(test_db, book.id, updated_data) assert updated_book is not None assert updated_book.title == TEST_BOOKS[1]["title"] assert updated_book.author == TEST_BOOKS[1]["author"] + assert updated_book.isbn == TEST_BOOKS[1]["isbn"] def test_delete_book(self, test_db): - """Test deleting a book""" - book = create_book(test_db, BookIn(title="To Delete", author="Author")) + """Test that delete_book removes the record and it is no longer retrievable""" + book = create_book( + test_db, BookIn(title="To Delete", author="Author", isbn="9780000000001") + ) deleted_book = delete_book(test_db, book.id) assert deleted_book is not None assert deleted_book.id == book.id assert get_book(test_db, book.id) is None - def test_nonexistent_operations(self, test_db): - """Test operations on nonexistent books""" + def test_get_nonexistent_book(self, test_db): + """Test that get_book returns None for a nonexistent ID""" assert get_book(test_db, 999999) is None - assert update_book(test_db, 999999, BookIn(title="Test", author="Test")) is None + + def test_update_nonexistent_book(self, test_db): + """Test that update_book returns None for a nonexistent ID""" + assert update_book(test_db, 999999, BookIn(**TEST_BOOKS[0])) is None + + def test_delete_nonexistent_book(self, test_db): + """Test that delete_book returns None for a nonexistent ID""" assert delete_book(test_db, 999999) is None + + def test_get_books_pagination(self, test_db): + """Test that skip and limit pagination parameters work correctly""" + create_book(test_db, BookIn(**TEST_BOOKS[0])) + create_book(test_db, BookIn(**TEST_BOOKS[1])) + + books_page1 = get_books(test_db, skip=0, limit=1) + assert len(books_page1) == 1 + + books_page2 = get_books(test_db, skip=1, limit=1) + assert len(books_page2) == 1 + + assert books_page1[0].id != books_page2[0].id + + books_empty = get_books(test_db, skip=0, limit=0) + assert len(books_empty) == 0 + + +# HTTP / Router Tests +class TestBooksRouter: + def test_create_book_http(self, client): + """Test POST /api/books/ creates a book and returns 201""" + payload = {"title": "Dune", "author": "Frank Herbert", "isbn": "9780441013593"} + response = client.post("/api/books/", json=payload) + assert response.status_code == 201 + data = response.json() + assert data["title"] == payload["title"] + assert data["author"] == payload["author"] + assert data["isbn"] == payload["isbn"] + assert "id" in data + + def test_create_book_missing_field_returns_422(self, client): + """Test POST /api/books/ with missing required field returns 422""" + payload = {"title": "No Author or ISBN"} + response = client.post("/api/books/", json=payload) + assert response.status_code == 422 + + def test_get_books_http(self, client): + """Test GET /api/books/ returns a list of books""" + client.post( + "/api/books/", + json={"title": "Dune", "author": "Frank Herbert", "isbn": "9780441013593"}, + ) + response = client.get("/api/books/") + assert response.status_code == 200 + assert isinstance(response.json(), list) + + def test_get_book_http(self, client): + """Test GET /api/books/{id} returns the correct book""" + created = client.post( + "/api/books/", + json={"title": "Dune", "author": "Frank Herbert", "isbn": "9780441013593"}, + ).json() + response = client.get(f"/api/books/{created['id']}") + assert response.status_code == 200 + assert response.json()["id"] == created["id"] + + def test_get_book_not_found(self, client): + """Test GET /api/books/{id} returns 404 for nonexistent ID""" + response = client.get("/api/books/999999") + assert response.status_code == 404 + + def test_update_book_http(self, client): + """Test PUT /api/books/{id} updates all fields correctly""" + created = client.post( + "/api/books/", + json={"title": "Dune", "author": "Frank Herbert", "isbn": "9780441013593"}, + ).json() + update_payload = { + "title": "Dune Messiah", + "author": "Frank Herbert", + "isbn": "9780593098233", + } + response = client.put(f"/api/books/{created['id']}", json=update_payload) + assert response.status_code == 200 + data = response.json() + assert data["title"] == update_payload["title"] + assert data["isbn"] == update_payload["isbn"] + + def test_update_book_not_found(self, client): + """Test PUT /api/books/{id} returns 404 for nonexistent ID""" + response = client.put( + "/api/books/999999", + json={"title": "X", "author": "Y", "isbn": "9780000000000"}, + ) + assert response.status_code == 404 + + def test_delete_book_http(self, client): + """Test DELETE /api/books/{id} removes the book and returns 200""" + created = client.post( + "/api/books/", + json={"title": "Dune", "author": "Frank Herbert", "isbn": "9780441013593"}, + ).json() + response = client.delete(f"/api/books/{created['id']}") + assert response.status_code == 200 + + follow_up = client.get(f"/api/books/{created['id']}") + assert follow_up.status_code == 404 + + def test_delete_book_not_found(self, client): + """Test DELETE /api/books/{id} returns 404 for nonexistent ID""" + response = client.delete("/api/books/999999") + assert response.status_code == 404