diff --git a/Dockerfile b/Dockerfile index b482401..8c29fcf 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,9 @@ -FROM python:3.11 -RUN apt-get update && apt-get install -y libpq-dev +FROM python:3.11-slim +RUN apt-get update && apt-get install -y --no-install-recommends libpq-dev && rm -rf /var/lib/apt/lists/* COPY . /app WORKDIR /app RUN pip install -r requirements.txt +RUN useradd -r appuser && chown -R appuser /app +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", "warning"] 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/models.py b/models.py index bd73c60..e7cb87e 100644 --- a/models.py +++ b/models.py @@ -1,14 +1,13 @@ +import re 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,6 +17,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(17), nullable=False) + # Pydantic models class BookIn(BaseModel): @@ -25,6 +26,19 @@ 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 and hyphens, correct length).""" + # Strip hyphens for length/digit checks + digits = value.replace("-", "") + if not re.fullmatch(r"[0-9X]+", digits, re.IGNORECASE): + raise ValueError("ISBN must contain only digits, hyphens, or a trailing X") + if len(digits) not in (10, 13): + raise ValueError("ISBN must be 10 or 13 digits (excluding hyphens)") + return value class BookOut(BaseModel): @@ -33,5 +47,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..e867471 100644 --- a/routers.py +++ b/routers.py @@ -1,10 +1,12 @@ -from fastapi import APIRouter, Depends, HTTPException, status +import logging +from fastapi import APIRouter, Depends, HTTPException, Query, status from sqlalchemy.orm import Session -from typing import List import models import repositories from dependencies import get_db +logger = logging.getLogger(__name__) + # Create router with prefix router = APIRouter() @@ -16,19 +18,29 @@ def create_book(book: models.BookIn, db: Session = Depends(get_db)): try: return repositories.create_book(db=db, book=book) except Exception as e: - raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) + logger.exception("Error creating book: %s", e) + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Could not create book", + ) 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)): +def get_books( + skip: int = Query(default=0, ge=0), + limit: int = Query(default=10, ge=1, le=100), + db: Session = Depends(get_db), +): try: return repositories.get_books(db=db, skip=skip, limit=limit) except Exception as e: + logger.exception("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..f59f134 100644 --- a/test_main.py +++ b/test_main.py @@ -1,14 +1,17 @@ +import pytest +from sqlalchemy import inspect +from fastapi import status 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 data constants — include isbn field (valid ISBN-13 values) 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 +22,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 +30,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,10 +39,25 @@ 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) + def test_get_books_pagination(self, test_db): + """Test pagination parameters (skip / limit) in get_books""" + create_book(test_db, BookIn(**TEST_BOOKS[0])) + create_book(test_db, BookIn(**TEST_BOOKS[1])) + + # limit=1 should return exactly one book + books = get_books(test_db, skip=0, limit=1) + assert len(books) == 1 + + # skip=1, limit=1 should return the second book only + all_books = get_books(test_db) + books_skipped = get_books(test_db, skip=1, limit=1) + assert len(books_skipped) == 1 + assert books_skipped[0].id != books[0].id + def test_get_book(self, test_db): """Test getting a specific book""" created_book = create_book(test_db, BookIn(**TEST_BOOKS[0])) @@ -45,6 +65,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 +75,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="9780307887436")) deleted_book = delete_book(test_db, book.id) assert deleted_book is not None @@ -67,5 +89,118 @@ 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="9780307887436")) is None assert delete_book(test_db, 999999) is None + + +# ISBN validation tests +class TestIsbnValidation: + def test_valid_isbn13(self): + """BookIn accepts a valid ISBN-13""" + book = BookIn(title="T", author="A", isbn="9780385533225") + assert book.isbn == "9780385533225" + + def test_valid_isbn13_with_hyphens(self): + """BookIn accepts a hyphenated ISBN-13""" + book = BookIn(title="T", author="A", isbn="978-0-385-53322-5") + assert book.isbn == "978-0-385-53322-5" + + def test_valid_isbn10(self): + """BookIn accepts a valid ISBN-10""" + book = BookIn(title="T", author="A", isbn="0306406152") + assert book.isbn == "0306406152" + + def test_invalid_isbn_too_short(self): + """BookIn rejects an ISBN that is too short""" + with pytest.raises(Exception): + BookIn(title="T", author="A", isbn="12345") + + def test_invalid_isbn_too_long(self): + """BookIn rejects an ISBN that is too long""" + with pytest.raises(Exception): + BookIn(title="T", author="A", isbn="12345678901234") + + def test_invalid_isbn_non_numeric(self): + """BookIn rejects an ISBN with invalid characters""" + with pytest.raises(Exception): + BookIn(title="T", author="A", isbn="97803854HELLO") + + +# Router / HTTP integration tests +class TestBookRoutes: + def test_create_book_returns_201(self, client): + """POST /api/books/ with valid payload returns 201 and the created book""" + payload = {"title": "Dune", "author": "Frank Herbert", "isbn": "9780441013593"} + response = client.post("/api/books/", json=payload) + assert response.status_code == status.HTTP_201_CREATED + 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_isbn_returns_422(self, client): + """POST /api/books/ without isbn returns 422 Unprocessable Entity""" + payload = {"title": "Dune", "author": "Frank Herbert"} + response = client.post("/api/books/", json=payload) + assert response.status_code == 422 + + def test_create_book_invalid_isbn_returns_422(self, client): + """POST /api/books/ with an invalid isbn returns 422""" + payload = {"title": "Dune", "author": "Frank Herbert", "isbn": "NOTANISBN"} + response = client.post("/api/books/", json=payload) + assert response.status_code == 422 + + def test_get_books_returns_200(self, client): + """GET /api/books/ returns 200 and a list""" + response = client.get("/api/books/") + assert response.status_code == status.HTTP_200_OK + assert isinstance(response.json(), list) + + def test_get_books_limit_cap(self, client): + """GET /api/books/ with limit > 100 returns 422""" + response = client.get("/api/books/?limit=10000000") + assert response.status_code == 422 + + def test_get_book_not_found_returns_404(self, client): + """GET /api/books/{id} for unknown id returns 404""" + response = client.get("/api/books/999999") + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_get_book_returns_isbn(self, client): + """GET /api/books/{id} response includes isbn field""" + payload = {"title": "Foundation", "author": "Isaac Asimov", "isbn": "9780553293357"} + created = client.post("/api/books/", json=payload).json() + response = client.get(f"/api/books/{created['id']}") + assert response.status_code == status.HTTP_200_OK + assert response.json()["isbn"] == payload["isbn"] + + def test_update_book_returns_updated_data(self, client): + """PUT /api/books/{id} updates and returns the book""" + payload = {"title": "Old Title", "author": "Old Author", "isbn": "9780385533225"} + created = client.post("/api/books/", json=payload).json() + update_payload = {"title": "New Title", "author": "New Author", "isbn": "9780441013593"} + response = client.put(f"/api/books/{created['id']}", json=update_payload) + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert data["title"] == "New Title" + assert data["isbn"] == "9780441013593" + + def test_update_book_not_found_returns_404(self, client): + """PUT /api/books/{id} for unknown id returns 404""" + payload = {"title": "T", "author": "A", "isbn": "9780385533225"} + response = client.put("/api/books/999999", json=payload) + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_delete_book_returns_deleted_book(self, client): + """DELETE /api/books/{id} returns the deleted book""" + payload = {"title": "To Delete", "author": "Author", "isbn": "9780441013593"} + created = client.post("/api/books/", json=payload).json() + response = client.delete(f"/api/books/{created['id']}") + assert response.status_code == status.HTTP_200_OK + assert response.json()["id"] == created["id"] + + def test_delete_book_not_found_returns_404(self, client): + """DELETE /api/books/{id} for unknown id returns 404""" + response = client.delete("/api/books/999999") + assert response.status_code == status.HTTP_404_NOT_FOUND