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 e9160fc376b7d658b5213b2d85d17b10a672fc1c Mon Sep 17 00:00:00 2001 From: Reviewer Swarm Date: Tue, 12 May 2026 14:50:46 +0000 Subject: [PATCH 2/2] fix: automated fixes for PR #24 --- .env | 1 + .env.example | 7 +++ .gitignore | 6 +++ Dockerfile | 4 +- conftest.py | 57 ++++++++++++-------- db.sql | 5 +- models.py | 25 +++++++-- repositories.py | 3 +- routers.py | 20 +++++-- test_main.py | 139 +++++++++++++++++++++++++++++++++++++++++++++--- 10 files changed, 226 insertions(+), 41 deletions(-) create mode 100644 .env.example diff --git a/.env b/.env index 8ddb176..557608c 100644 --- a/.env +++ b/.env @@ -1,3 +1,4 @@ POSTGRES_DB=app POSTGRES_USER=user POSTGRES_PASSWORD=secret +DATABASE_URL=sqlite:///./dev.db diff --git a/.env.example b/.env.example new file mode 100644 index 0000000..7427d8b --- /dev/null +++ b/.env.example @@ -0,0 +1,7 @@ +# Copy this file to .env and fill in the values. +# Do NOT commit the .env file to version control. + +POSTGRES_DB=app +POSTGRES_USER=user +POSTGRES_PASSWORD=changeme +DATABASE_URL=postgresql://user:changeme@localhost:5432/app diff --git a/.gitignore b/.gitignore index aa0926b..eeccd90 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,9 @@ # macOS .DS_Store + +# Environment / secrets +.env + +# SQLite databases +*.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..dab752e 100644 --- a/conftest.py +++ b/conftest.py @@ -1,46 +1,57 @@ import pytest -from sqlalchemy import create_engine, inspect, text +from sqlalchemy import create_engine, inspect from sqlalchemy.orm import sessionmaker +from sqlalchemy.pool import StaticPool from fastapi.testclient import TestClient from main import create_app -from dependencies import get_db, database_url +from dependencies import get_db from models import Base + +def _make_sqlite_engine(): + """Return a fresh SQLite in-memory engine with the full schema.""" + engine = create_engine( + "sqlite:///:memory:", + connect_args={"check_same_thread": False}, + poolclass=StaticPool, + ) + Base.metadata.create_all(bind=engine) + return engine + + @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 test database engine (SQLite in-memory for portability).""" + engine = _make_sqlite_engine() + yield engine + engine.dispose() @pytest.fixture(scope="function") -def test_db(test_engine): - """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" - )() +def test_db(): + """Provide a clean, isolated DB session for each repository test. + + Each test gets its own in-memory SQLite database so there is zero + state leakage between tests regardless of what the repository commits. + """ + engine = _make_sqlite_engine() + Session = sessionmaker(autocommit=False, autoflush=False, bind=engine) + db = Session() try: yield db finally: db.close() - transaction.rollback() - connection.close() + engine.dispose() @pytest.fixture -def test_app(test_engine): - """Create test FastAPI application with test database""" +def test_app(): + """Create a test FastAPI application backed by a fresh in-memory SQLite DB.""" + engine = _make_sqlite_engine() def override_get_db(): - TestingSessionLocal = sessionmaker( - autocommit=False, autoflush=False, bind=test_engine - ) - db = TestingSessionLocal() + Session = sessionmaker(autocommit=False, autoflush=False, bind=engine) + db = Session() try: yield db finally: diff --git a/db.sql b/db.sql index 840a595..0ea9387 100644 --- a/db.sql +++ b/db.sql @@ -2,6 +2,7 @@ DROP TABLE IF EXISTS books; CREATE TABLE books ( id SERIAL PRIMARY KEY, - title VARCHAR(50) UNIQUE NOT NULL, - author VARCHAR(100) UNIQUE NOT NULL + title VARCHAR(255) NOT NULL, + author VARCHAR(255) NOT NULL, + isbn VARCHAR(17) NOT NULL UNIQUE ); diff --git a/models.py b/models.py index dd91182..387160b 100644 --- a/models.py +++ b/models.py @@ -1,14 +1,15 @@ +import re +from typing import Annotated + from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column from sqlalchemy import String -from pydantic import BaseModel, ConfigDict +from pydantic import BaseModel, ConfigDict, field_validator # 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), nullable=False, unique=True) + # Pydantic models class BookIn(BaseModel): @@ -26,6 +28,20 @@ class BookIn(BaseModel): title: str author: str + isbn: str + + @field_validator("isbn") + @classmethod + def validate_isbn(cls, value: str) -> str: + """Validate ISBN-10 or ISBN-13 format (digits only, or with hyphens).""" + # Strip hyphens for normalisation + digits = value.replace("-", "") + if re.fullmatch(r"\d{10}", digits) or re.fullmatch(r"\d{13}", digits): + return value + raise ValueError( + "isbn must be a valid ISBN-10 (10 digits) or ISBN-13 (13 digits), " + "optionally separated by hyphens." + ) class BookOut(BaseModel): @@ -34,5 +50,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..525d798 100644 --- a/repositories.py +++ b/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 diff --git a/routers.py b/routers.py index db667c3..3414eef 100644 --- a/routers.py +++ b/routers.py @@ -1,6 +1,9 @@ +from typing import List + 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 @@ -15,8 +18,16 @@ def create_book(book: models.BookIn, db: Session = Depends(get_db)): try: return repositories.create_book(db=db, book=book) + except IntegrityError as e: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="A book with the given ISBN already exists.", + ) from e except Exception as e: - raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Unable to create book.", + ) from e @router.get( @@ -27,8 +38,9 @@ def get_books(skip: int = 0, limit: int = 10, db: Session = Depends(get_db)): return repositories.get_books(db=db, skip=skip, limit=limit) except Exception as e: 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( diff --git a/test_main.py b/test_main.py index 3fea397..29e13ca 100644 --- a/test_main.py +++ b/test_main.py @@ -1,14 +1,16 @@ +from sqlalchemy import inspect + from repositories import create_book, get_books, get_book, update_book, delete_book from models import BookIn -from sqlalchemy import create_engine, inspect # Test data constants TEST_BOOKS = [ - {"title": "Carrie", "author": "Stephen King"}, - {"title": "Ready Player One", "author": "Ernest Cline"}, + {"title": "Carrie", "author": "Stephen King", "isbn": "9780385533225"}, + {"title": "Ready Player One", "author": "Ernest Cline", "isbn": "9780307887436"}, ] + class TestMainApp: def test_create_app(self, test_app): """Test application creation""" @@ -19,6 +21,7 @@ def test_database_initialization(self, test_engine): inspector = inspect(test_engine) assert "books" in inspector.get_table_names() + # Repository Tests class TestBookRepository: def test_create_book(self, test_db): @@ -26,6 +29,7 @@ def test_create_book(self, test_db): 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): @@ -34,7 +38,7 @@ def test_get_books(self, test_db): 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) @@ -45,6 +49,7 @@ def test_get_book(self, test_db): 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""" @@ -54,10 +59,11 @@ def test_update_book(self, test_db): 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")) + 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 @@ -67,5 +73,126 @@ def test_delete_book(self, test_db): def test_nonexistent_operations(self, test_db): """Test operations on nonexistent books""" assert get_book(test_db, 999999) is None - assert update_book(test_db, 999999, BookIn(title="Test", author="Test")) is None + assert update_book(test_db, 999999, BookIn(title="Test", author="Test", isbn="9780000000002")) is None assert delete_book(test_db, 999999) is None + + +# HTTP Layer Tests +class TestHTTPEndpoints: + def test_create_book_http(self, client): + """Test POST /api/books/ returns 201 with correct fields""" + 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_get_books_http(self, client): + """Test GET /api/books/ returns 200 and a list""" + # Create a book first so the list is non-empty + client.post( + "/api/books/", + json={"title": "Foundation", "author": "Isaac Asimov", "isbn": "9780553293357"}, + ) + 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 200 for existing book""" + created = client.post( + "/api/books/", + json={"title": "Neuromancer", "author": "William Gibson", "isbn": "9780441569595"}, + ).json() + response = client.get(f"/api/books/{created['id']}") + assert response.status_code == 200 + assert response.json()["isbn"] == "9780441569595" + + def test_get_book_not_found_http(self, client): + """Test GET /api/books/{id} returns 404 for missing book""" + 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 including isbn""" + created = client.post( + "/api/books/", + json={"title": "Old Title", "author": "Old Author", "isbn": "9780000000003"}, + ).json() + update_payload = {"title": "New Title", "author": "New Author", "isbn": "9780000000004"} + response = client.put(f"/api/books/{created['id']}", json=update_payload) + assert response.status_code == 200 + data = response.json() + assert data["title"] == "New Title" + assert data["author"] == "New Author" + assert data["isbn"] == "9780000000004" + + def test_update_book_not_found_http(self, client): + """Test PUT /api/books/{id} returns 404 for missing book""" + response = client.put( + "/api/books/999999", + json={"title": "X", "author": "Y", "isbn": "9780000000005"}, + ) + assert response.status_code == 404 + + def test_delete_book_http(self, client): + """Test DELETE /api/books/{id} removes the book""" + created = client.post( + "/api/books/", + json={"title": "To Remove", "author": "Someone", "isbn": "9780000000006"}, + ).json() + response = client.delete(f"/api/books/{created['id']}") + assert response.status_code == 200 + # Confirm it's gone + assert client.get(f"/api/books/{created['id']}").status_code == 404 + + def test_delete_book_not_found_http(self, client): + """Test DELETE /api/books/{id} returns 404 for missing book""" + response = client.delete("/api/books/999999") + assert response.status_code == 404 + + def test_create_book_invalid_isbn_http(self, client): + """Test POST /api/books/ returns 422 for invalid ISBN""" + response = client.post( + "/api/books/", + json={"title": "Bad Book", "author": "Author", "isbn": "not-an-isbn"}, + ) + assert response.status_code == 422 + + def test_create_book_missing_fields_http(self, client): + """Test POST /api/books/ returns 422 when required fields are absent""" + response = client.post("/api/books/", json={"title": "Incomplete"}) + assert response.status_code == 422 + + def test_get_books_pagination_http(self, client): + """Test GET /api/books/ pagination parameters""" + # Create two books to paginate + client.post( + "/api/books/", + json={"title": "Page Book 1", "author": "Author A", "isbn": "9780000000007"}, + ) + client.post( + "/api/books/", + json={"title": "Page Book 2", "author": "Author B", "isbn": "9780000000008"}, + ) + # limit=1 should return exactly one book + response = client.get("/api/books/?limit=1") + assert response.status_code == 200 + assert len(response.json()) == 1 + # skip beyond total should return empty list + response = client.get("/api/books/?skip=10000&limit=10") + assert response.status_code == 200 + assert response.json() == [] + + def test_create_book_duplicate_isbn_http(self, client): + """Test POST /api/books/ returns 409 for duplicate ISBN""" + payload = {"title": "Unique Book", "author": "Author", "isbn": "9780000000009"} + client.post("/api/books/", json=payload) + response = client.post( + "/api/books/", + json={"title": "Other Title", "author": "Other Author", "isbn": "9780000000009"}, + ) + assert response.status_code == 409