fix: automated fixes for PR #24

[vikram-blaxel]

Automated fixes for PR #24.

20/20 tests pass. Here is the structured summary:

---

Security Fixes

1. models.py + repositories.py + routers.py — isbn field wired end-to-end (Critical): Added isbn: str to BookIn and BookOut Pydantic schemas; updated repositories.create_book and repositories.update_book to pass book.isbn to the ORM constructor/setter. This resolves the NOT NULL integrity error that caused every write to fail.

2. routers.py — Internal exception detail no longer leaked to callers (Critical): Replaced detail=str(e) in both create_book and get_books exception handlers with the generic string "An internal error occurred.". Exceptions are now logged server-side via logging. create_book now catches IntegrityError specifically (400) and falls back to 500 for unexpected errors.

3. .env / .gitignore — Plaintext credentials removed from VCS (Critical): Added .env to .gitignore so it will no longer be committed. Created .env.example as a safe template. Added *.db to .gitignore to exclude SQLite test artefacts.

4. Dockerfile — Container no longer runs as root (Warning): Added RUN useradd -m appuser and USER appuser before CMD.

5. Dockerfile — Uvicorn log level lowered from trace to info (Warning): Prevents full request/response bodies and auth headers from appearing in production logs.

6. docker-compose.yml — PostgreSQL port bound to loopback only (Warning): Changed "5432:5432" to "127.0.0.1:5432:5432" so the database is not exposed to external networks.

7. requirements.txt — All dependencies pinned to exact versions (Warning): Replaced all >= specifiers with == using the versions confirmed installed in the environment.

8. main.py — OpenAPI docs disabled in production (Informational): Added docs_url=None, redoc_url=None to FastAPI(...) to avoid advertising the API surface.

9. dependencies.py — Replaced print() with structured logger.error() (Informational): DB init errors now go through the logging framework.

---

Code Quality Fixes

1. models.py — Removed redundant pass after docstring in Base (Informational): Eliminated the W0107 pylint warning.

2. models.py — String(255) narrowed to String(17) for isbn (Informational): ISBN-13 with hyphens fits in 17 characters; 255 was unnecessarily wide.

3. main.py — Removed redundant router-level dependencies=[Depends(get_db)] (Warning): This was opening and discarding a second DB session per request in addition to each route's own Depends(get_db).

4. routers.py — Replaced from typing import List with built-in list[...] (Informational): Modern Python 3.9+ syntax used throughout, consistent with the rest of the codebase.

5. repositories.py + routers.py — Added return-type annotations to all functions (Informational): Improves type-checker effectiveness and IDE support.

6. conftest.py — Removed unused imports text and inspect (not used), kept inspect (it is used by test_main.py) (Informational): Removed text import; kept inspect import because test_main.py uses it via the fixture test_engine.

7. test_main.py — Removed unused create_engine import (Informational).

---

Test Additions

1. TEST_BOOKS updated with isbn values — Both test-data entries now include a valid ISBN-13 string.

2. isbn assertions added — test_create_book, test_get_book, and test_update_book now assert that book.isbn equals the expected value after each operation.

3. assert len(books) >= 2 uncommented in test_get_books.

4. test_nonexistent_operations split into three focused tests: test_get_nonexistent_book, test_update_nonexistent_book, test_delete_nonexistent_book.

5. test_get_books_pagination — New test covering non-default skip/limit values including limit=0.

6. TestBooksRouter class (9 new HTTP-level tests): Full coverage of all five router endpoints via the client fixture: test_create_book_http, test_create_book_missing_field_returns_422, test_get_books_http, test_get_book_http, test_get_book_not_found, test_update_book_http, test_update_book_not_found, test_delete_book_http, test_delete_book_not_found.

7. conftest.py — override_get_db now uses savepoint/rollback matching test_db, so HTTP-level tests also roll back between tests, preventing state leakage. Also added SQLite fallback (DATABASE_URL defaults to sqlite:///./test.db when not set in the environment) so the test suite runs without a live PostgreSQL instance.

---

Skipped Items

1. Authentication/authorization on all endpoints (Security Warning): Adding OAuth2/API-key auth would require new dependencies and significant API surface changes that go beyond the scope of a code-quality fix pass. Flagged for a dedicated auth PR.

2. with_for_update() TOCTOU locking in update_book/delete_book (Code Quality Warning): SQLite (used in tests) does not support SELECT … FOR UPDATE, so adding this would break the test suite without a real PostgreSQL instance. Deferred to a follow-up that adds proper PostgreSQL integration-test infrastructure.

3. Alembic migration for existing rows (Code Quality Critical note): The review flagged that a migration is needed for rows already in the DB. Alembic setup would be a new dependency and a separate migration concern; the column itself was already in the PR and existing-row backfill is a deployment-time concern outside the scope of these fixes.

4. ISBN format validator in BookIn (Security/Quality Informational): Adding a field_validator regex for ISBN-10/13 format is desirable but changes the API contract (rejects previously-accepted values). Left to a follow-up PR to avoid breaking existing integrations.