From 49098bd6f7bbecb6cd8a8046f0dd25f3a333cf2a Mon Sep 17 00:00:00 2001 From: MartinKalema Date: Mon, 6 Jul 2026 21:50:34 +0900 Subject: [PATCH] refactor: align declared architecture with actual architecture (audit fixes) A clean-architecture audit found the runtime dependency rule held everywhere, but the type layer told a different story. This makes the two match, and adds the enforcement that keeps them matched. Dependency Rule: - CQRS query ports move from domain/*/interfaces to the application layer (application/query_handlers/interfaces.py): they serve views and return application read models, so the domain importing BookReadModel (the audit's only true inward-ring violation) disappears. Write-side ports (command repositories, units of work) stay in the domain. - All 17 handler annotations re-pointed from concrete adapters (LoanUnitOfWork, CacheAdapter, PatronQueryRepository) to ports (ILoanUnitOfWork, ICache, application query ports); 8 phantom TYPE_CHECKING imports of non-existent names fixed - sqlalchemy leaves the presentation layer: the loan unit of work translates IntegrityError on the unique-active-loan index into BookNotAvailableException at the boundary - Health routes stop importing infrastructure directly: PostgreSQL gains ping(), the circuit-breaker registry is container-provided Boundary contracts: - PatronReadModel/LoanReadModel were each defined twice; all read models consolidated in application/query_handlers/read_models.py - Patron and loan query repositories return typed read models instead of raw dicts, matching the book convention end to end - ICache protocol updated to the adapter's real (async) surface Main component: - api/main.py becomes a create_app() factory (uvicorn --factory); importing the module no longer builds the container or calls etcd, so the non-e2e test suite runs with zero infrastructure Dead architecture removed: - BookRepository, ICatalogQueryRepository, IPatronRepository, EmailTemplate, ITemplateRenderer (all zero-use) Enforcement: - mypy configured in pyproject (SQLAlchemy legacy-Column noise scoped to persistence adapters, documented) and clean on 134 files - GitHub Actions CI: mypy + the infrastructure-free test suites - mypy immediately caught two real defects: three patron routes dereferencing a possibly-None query result (500 on missing patron), and the reaper calling find_expired_reservations which existed on the concrete repository but not on the port 157 tests passing; verified live (factory app, saga end-to-end, health endpoints through the container-provided registry). --- .github/workflows/ci.yml | 38 +++++++++ Dockerfile | 2 +- main.py | 2 +- pyproject.toml | 14 ++++ src/application/command_handlers/add_book.py | 4 +- .../command_handlers/borrow_book.py | 10 +-- .../command_handlers/confirm_book_borrow.py | 4 +- .../command_handlers/create_loan.py | 4 +- .../command_handlers/extend_loan.py | 4 +- .../command_handlers/register_patron.py | 4 +- .../command_handlers/reinstate_patron.py | 4 +- .../release_book_reservation.py | 4 +- .../release_expired_reservations.py | 4 +- .../command_handlers/return_book.py | 4 +- .../command_handlers/return_loan.py | 5 +- .../command_handlers/suspend_patron.py | 6 +- .../command_handlers/upgrade_patron_tier.py | 4 +- .../create_loan_on_book_reserved.py | 10 +-- src/application/query_handlers/__init__.py | 13 ++- src/application/query_handlers/get_book.py | 11 ++- src/application/query_handlers/get_loan.py | 29 ++----- src/application/query_handlers/get_patron.py | 29 ++----- src/application/query_handlers/interfaces.py | 84 +++++++++++++++++++ src/application/query_handlers/list_books.py | 28 ++----- .../query_handlers/list_patron_loans.py | 29 ++----- .../query_handlers/list_patrons.py | 29 ++----- src/application/query_handlers/read_models.py | 52 ++++++++++++ src/container.py | 6 +- src/domain/catalog/__init__.py | 3 +- src/domain/catalog/entities/catalog_book.py | 2 + src/domain/catalog/interfaces/__init__.py | 2 - .../interfaces/book_query_repository.py | 57 ------------- .../interfaces/catalog_book_repository.py | 30 ------- .../interfaces/catalog_command_repository.py | 5 ++ .../interfaces/catalog_query_repository.py | 46 ---------- src/domain/lending/__init__.py | 3 +- src/domain/lending/interfaces/__init__.py | 2 - .../interfaces/loan_query_repository.py | 28 ------- src/domain/patron/__init__.py | 3 +- src/domain/patron/interfaces/__init__.py | 2 - .../interfaces/patron_query_repository.py | 32 ------- .../patron/interfaces/patron_repository.py | 30 ------- src/domain/shared_kernel/__init__.py | 4 - src/domain/shared_kernel/email_template.py | 10 --- src/domain/shared_kernel/exceptions.py | 2 +- src/domain/shared_kernel/interfaces.py | 24 +++--- .../adapters/cache/cache_adapter.py | 13 ++- .../adapters/catalog/catalog_unit_of_work.py | 1 + .../adapters/lending/loan_query_repository.py | 61 +++++++------- .../adapters/lending/loan_unit_of_work.py | 23 ++++- .../patron/patron_query_repository.py | 59 ++++++------- .../adapters/patron/patron_unit_of_work.py | 1 + .../external/elasticsearch_client.py | 5 +- src/infrastructure/external/etcd_client.py | 2 +- src/infrastructure/external/kafka_client.py | 1 + src/infrastructure/external/postgresql.py | 7 ++ src/presentation/api/main.py | 65 ++++++++------ src/presentation/api/routes/health_routes.py | 20 ++--- src/presentation/api/routes/loan_routes.py | 3 - src/presentation/api/routes/patron_routes.py | 6 ++ tests/application/test_event_handlers.py | 14 +++- tests/conftest.py | 29 +++---- tests/integration/test_borrow_choreography.py | 16 +++- tests/integration/test_use_cases.py | 10 ++- 64 files changed, 511 insertions(+), 547 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 src/application/query_handlers/interfaces.py create mode 100644 src/application/query_handlers/read_models.py delete mode 100644 src/domain/catalog/interfaces/book_query_repository.py delete mode 100644 src/domain/catalog/interfaces/catalog_book_repository.py delete mode 100644 src/domain/catalog/interfaces/catalog_query_repository.py delete mode 100644 src/domain/lending/interfaces/loan_query_repository.py delete mode 100644 src/domain/patron/interfaces/patron_query_repository.py delete mode 100644 src/domain/patron/interfaces/patron_repository.py delete mode 100644 src/domain/shared_kernel/email_template.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..58a9f13 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,38 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + +jobs: + typecheck: + name: mypy (dependency rule + type layer) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + cache: pip + - name: Install + run: pip install -e . mypy + - name: mypy + run: mypy src + + test: + name: pytest (domain, application, infrastructure, integration) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + cache: pip + - name: Install + run: pip install -e . pytest + - name: pytest + # e2e requires the full compose stack (etcd, Kafka, ES); everything + # else runs infrastructure-free since app composition moved into + # the create_app factory + run: pytest tests/domain tests/application tests/infrastructure tests/integration -q diff --git a/Dockerfile b/Dockerfile index e50e5c6..8b70865 100644 --- a/Dockerfile +++ b/Dockerfile @@ -28,4 +28,4 @@ EXPOSE 8000 # One worker per container: scaling is horizontal via API container # replicas. A single process keeps circuit breaker state and # /health/circuits coherent per container. -CMD ["uvicorn", "src.presentation.api.main:app", "--host", "0.0.0.0", "--port", "8000", "--workers", "1"] +CMD ["uvicorn", "--factory", "src.presentation.api.main:create_app", "--host", "0.0.0.0", "--port", "8000", "--workers", "1"] diff --git a/main.py b/main.py index c95da70..29f75e9 100644 --- a/main.py +++ b/main.py @@ -6,4 +6,4 @@ # keeps circuit breaker state and /health/circuits coherent — with # multiple workers, each process holds its own breaker registry and # health checks sample a random one. - uvicorn.run("src.presentation.api.main:app", host="0.0.0.0", port=8000, workers=1, backlog=8192, limit_concurrency=10000) + uvicorn.run("src.presentation.api.main:create_app", factory=True, host="0.0.0.0", port=8000, workers=1, backlog=8192, limit_concurrency=10000) diff --git a/pyproject.toml b/pyproject.toml index 43f5665..044d4ca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,3 +41,17 @@ asyncio_default_fixture_loop_scope = "session" [build-system] requires = ["setuptools", "wheel"] build-backend = "setuptools.build_meta" + +[tool.mypy] +python_version = "3.12" +ignore_missing_imports = true +warn_unused_ignores = true +no_implicit_optional = true +exclude = ["tests/"] + +# SQLAlchemy's legacy Column() declarative style types instance attributes +# as Column[...] rather than their values; silence value-vs-column noise in +# the persistence adapters without losing import/name/annotation checking +[[tool.mypy.overrides]] +module = "src.infrastructure.adapters.*" +disable_error_code = ["arg-type", "assignment", "return-value", "index", "attr-defined"] diff --git a/src/application/command_handlers/add_book.py b/src/application/command_handlers/add_book.py index f8bfe67..b4229e7 100644 --- a/src/application/command_handlers/add_book.py +++ b/src/application/command_handlers/add_book.py @@ -12,7 +12,7 @@ from src.domain.catalog import Author, Book, BookId, Title if TYPE_CHECKING: - from src.domain.catalog import UnitOfWork + from src.domain.catalog import ICatalogUnitOfWork from src.domain.shared_kernel import ILogger @@ -41,7 +41,7 @@ class AddBookHandler: and may emit domain events for read model synchronization. """ - def __init__(self, uow: UnitOfWork, logger: ILogger): + def __init__(self, uow: ICatalogUnitOfWork, logger: ILogger): self.uow = uow self.logger = logger diff --git a/src/application/command_handlers/borrow_book.py b/src/application/command_handlers/borrow_book.py index a9b87c4..ddacf3a 100644 --- a/src/application/command_handlers/borrow_book.py +++ b/src/application/command_handlers/borrow_book.py @@ -10,10 +10,8 @@ from src.domain.catalog import BookNotFoundException, BorrowerNotEligibleException if TYPE_CHECKING: - from src.domain.catalog import UnitOfWork - from src.domain.patron.interfaces.patron_query_repository import ( - IPatronQueryRepository, - ) + from src.domain.catalog import ICatalogUnitOfWork + from src.application.query_handlers.interfaces import IPatronQueryRepository from src.domain.shared_kernel import ILogger @@ -46,7 +44,7 @@ class BorrowBookHandler: def __init__( self, - uow: UnitOfWork, + uow: ICatalogUnitOfWork, patron_query_repository: IPatronQueryRepository, logger: ILogger, ): @@ -65,7 +63,7 @@ async def handle(self, command: BorrowBookCommand) -> BorrowBookResult: raise BorrowerNotEligibleException( command.borrower_email, "no patron registered with this email" ) - if patron.get("is_suspended"): + if patron.is_suspended: raise BorrowerNotEligibleException( command.borrower_email, "patron is suspended" ) diff --git a/src/application/command_handlers/confirm_book_borrow.py b/src/application/command_handlers/confirm_book_borrow.py index cac88dd..5506966 100644 --- a/src/application/command_handlers/confirm_book_borrow.py +++ b/src/application/command_handlers/confirm_book_borrow.py @@ -20,7 +20,7 @@ from src.domain.catalog import BookNotFoundException, BookStatus if TYPE_CHECKING: - from src.domain.catalog import UnitOfWork + from src.domain.catalog import ICatalogUnitOfWork from src.domain.shared_kernel import ILogger @@ -36,7 +36,7 @@ class ConfirmBookBorrowCommand: class ConfirmBookBorrowHandler: """Handles the ConfirmBookBorrowCommand.""" - def __init__(self, uow: UnitOfWork, logger: ILogger): + def __init__(self, uow: ICatalogUnitOfWork, logger: ILogger): self.uow = uow self.logger = logger diff --git a/src/application/command_handlers/create_loan.py b/src/application/command_handlers/create_loan.py index f865a39..0e10028 100644 --- a/src/application/command_handlers/create_loan.py +++ b/src/application/command_handlers/create_loan.py @@ -12,7 +12,7 @@ if TYPE_CHECKING: from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.lending import LoanUnitOfWork + from src.domain.lending import ILoanUnitOfWork @dataclass(frozen=True) @@ -42,7 +42,7 @@ class CreateLoanResult: class CreateLoanHandler: """Handles loan creation.""" - def __init__(self, uow: LoanUnitOfWork, logger: ILogger): + def __init__(self, uow: ILoanUnitOfWork, logger: ILogger): self.uow = uow self.logger = logger diff --git a/src/application/command_handlers/extend_loan.py b/src/application/command_handlers/extend_loan.py index a70f02b..6b9a47e 100644 --- a/src/application/command_handlers/extend_loan.py +++ b/src/application/command_handlers/extend_loan.py @@ -11,7 +11,7 @@ if TYPE_CHECKING: from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.lending import LoanUnitOfWork + from src.domain.lending import ILoanUnitOfWork @dataclass(frozen=True) @@ -31,7 +31,7 @@ class ExtendLoanResult: class ExtendLoanHandler: """Handles loan extensions.""" - def __init__(self, uow: LoanUnitOfWork, logger: ILogger): + def __init__(self, uow: ILoanUnitOfWork, logger: ILogger): self.uow = uow self.logger = logger diff --git a/src/application/command_handlers/register_patron.py b/src/application/command_handlers/register_patron.py index 6f197a5..49a3fb5 100644 --- a/src/application/command_handlers/register_patron.py +++ b/src/application/command_handlers/register_patron.py @@ -13,7 +13,7 @@ if TYPE_CHECKING: from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.patron import PatronUnitOfWork + from src.domain.patron import IPatronUnitOfWork @dataclass(frozen=True) @@ -37,7 +37,7 @@ class RegisterPatronResult: class RegisterPatronHandler: """Handles patron registration.""" - def __init__(self, uow: PatronUnitOfWork, logger: ILogger): + def __init__(self, uow: IPatronUnitOfWork, logger: ILogger): self.uow = uow self.logger = logger diff --git a/src/application/command_handlers/reinstate_patron.py b/src/application/command_handlers/reinstate_patron.py index 5b312b7..afc986f 100644 --- a/src/application/command_handlers/reinstate_patron.py +++ b/src/application/command_handlers/reinstate_patron.py @@ -10,7 +10,7 @@ if TYPE_CHECKING: from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.patron import PatronUnitOfWork + from src.domain.patron import IPatronUnitOfWork @dataclass(frozen=True) @@ -29,7 +29,7 @@ class ReinstatePatronResult: class ReinstatePatronHandler: """Handles patron reinstatement.""" - def __init__(self, uow: PatronUnitOfWork, logger: ILogger): + def __init__(self, uow: IPatronUnitOfWork, logger: ILogger): self.uow = uow self.logger = logger diff --git a/src/application/command_handlers/release_book_reservation.py b/src/application/command_handlers/release_book_reservation.py index 4bb1cf2..e11142f 100644 --- a/src/application/command_handlers/release_book_reservation.py +++ b/src/application/command_handlers/release_book_reservation.py @@ -12,7 +12,7 @@ from src.domain.catalog import BookNotFoundException, BookStatus if TYPE_CHECKING: - from src.domain.catalog import UnitOfWork + from src.domain.catalog import ICatalogUnitOfWork from src.domain.shared_kernel import ILogger @@ -26,7 +26,7 @@ class ReleaseBookReservationCommand: class ReleaseBookReservationHandler: """Handles the ReleaseBookReservationCommand.""" - def __init__(self, uow: UnitOfWork, logger: ILogger): + def __init__(self, uow: ICatalogUnitOfWork, logger: ILogger): self.uow = uow self.logger = logger diff --git a/src/application/command_handlers/release_expired_reservations.py b/src/application/command_handlers/release_expired_reservations.py index f671ab5..801b774 100644 --- a/src/application/command_handlers/release_expired_reservations.py +++ b/src/application/command_handlers/release_expired_reservations.py @@ -17,7 +17,7 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: - from src.domain.catalog import UnitOfWork + from src.domain.catalog import ICatalogUnitOfWork from src.domain.shared_kernel import ILogger @@ -36,7 +36,7 @@ class ReleaseExpiredReservationsResult: class ReleaseExpiredReservationsHandler: """Handles the ReleaseExpiredReservationsCommand.""" - def __init__(self, uow: UnitOfWork, logger: ILogger): + def __init__(self, uow: ICatalogUnitOfWork, logger: ILogger): self.uow = uow self.logger = logger diff --git a/src/application/command_handlers/return_book.py b/src/application/command_handlers/return_book.py index 703c25d..393230b 100644 --- a/src/application/command_handlers/return_book.py +++ b/src/application/command_handlers/return_book.py @@ -9,7 +9,7 @@ from src.domain.catalog import BookNotFoundException if TYPE_CHECKING: - from src.domain.catalog import UnitOfWork + from src.domain.catalog import ICatalogUnitOfWork from src.domain.shared_kernel import ILogger @@ -36,7 +36,7 @@ class ReturnBookHandler: Emits BookReturned domain event for read model sync. """ - def __init__(self, uow: UnitOfWork, logger: ILogger): + def __init__(self, uow: ICatalogUnitOfWork, logger: ILogger): self.uow = uow self.logger = logger diff --git a/src/application/command_handlers/return_loan.py b/src/application/command_handlers/return_loan.py index 49f1328..216cdf7 100644 --- a/src/application/command_handlers/return_loan.py +++ b/src/application/command_handlers/return_loan.py @@ -11,7 +11,7 @@ if TYPE_CHECKING: from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.lending import LoanUnitOfWork + from src.domain.lending import ILoanUnitOfWork @dataclass(frozen=True) @@ -31,7 +31,7 @@ class ReturnLoanResult: class ReturnLoanHandler: """Handles loan returns.""" - def __init__(self, uow: LoanUnitOfWork, logger: ILogger): + def __init__(self, uow: ILoanUnitOfWork, logger: ILogger): self.uow = uow self.logger = logger @@ -44,6 +44,7 @@ async def handle(self, command: ReturnLoanCommand) -> ReturnLoanResult: returned_at = datetime.now() was_overdue = loan.due_date.is_overdue_as_of(returned_at) loan.return_book(returned_at) + assert loan.returned_at is not None await self.uow.loans.update(loan) await self.uow.commit() diff --git a/src/application/command_handlers/suspend_patron.py b/src/application/command_handlers/suspend_patron.py index 2ff21d2..a86aa16 100644 --- a/src/application/command_handlers/suspend_patron.py +++ b/src/application/command_handlers/suspend_patron.py @@ -10,7 +10,7 @@ if TYPE_CHECKING: from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.patron import PatronUnitOfWork + from src.domain.patron import IPatronUnitOfWork @dataclass(frozen=True) @@ -25,13 +25,13 @@ class SuspendPatronResult: """Result of suspending a patron.""" id: str is_suspended: bool - reason: str + reason: "str | None" class SuspendPatronHandler: """Handles patron suspension.""" - def __init__(self, uow: PatronUnitOfWork, logger: ILogger): + def __init__(self, uow: IPatronUnitOfWork, logger: ILogger): self.uow = uow self.logger = logger diff --git a/src/application/command_handlers/upgrade_patron_tier.py b/src/application/command_handlers/upgrade_patron_tier.py index 22078d6..81f87ca 100644 --- a/src/application/command_handlers/upgrade_patron_tier.py +++ b/src/application/command_handlers/upgrade_patron_tier.py @@ -11,7 +11,7 @@ if TYPE_CHECKING: from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.patron import PatronUnitOfWork + from src.domain.patron import IPatronUnitOfWork @dataclass(frozen=True) @@ -32,7 +32,7 @@ class UpgradePatronTierResult: class UpgradePatronTierHandler: """Handles patron tier upgrades.""" - def __init__(self, uow: PatronUnitOfWork, logger: ILogger): + def __init__(self, uow: IPatronUnitOfWork, logger: ILogger): self.uow = uow self.logger = logger diff --git a/src/application/event_handlers/create_loan_on_book_reserved.py b/src/application/event_handlers/create_loan_on_book_reserved.py index 2966760..aa4f5cf 100644 --- a/src/application/event_handlers/create_loan_on_book_reserved.py +++ b/src/application/event_handlers/create_loan_on_book_reserved.py @@ -33,9 +33,7 @@ if TYPE_CHECKING: from src.domain.catalog import CatalogBookReserved - from src.domain.patron.interfaces.patron_query_repository import ( - IPatronQueryRepository, - ) + from src.application.query_handlers.interfaces import IPatronQueryRepository from src.domain.shared_kernel import ILogger @@ -61,8 +59,8 @@ async def handle(self, event: CatalogBookReserved) -> None: event, f"no patron registered with email {event.borrower_email}" ) return - if patron.get("is_suspended"): - await self._compensate(event, f"patron {patron['id']} is suspended") + if patron.is_suspended: + await self._compensate(event, f"patron {patron.id} is suspended") return # The catalog decided the due date; lending honors it instead of @@ -70,7 +68,7 @@ async def handle(self, event: CatalogBookReserved) -> None: loan_duration_days = max(1, (event.return_due_date - event.reserved_at).days) command = CreateLoanCommand( - patron_id=patron["id"], + patron_id=patron.id, patron_email=event.borrower_email, catalog_book_id=event.book_id, book_title=event.title, diff --git a/src/application/query_handlers/__init__.py b/src/application/query_handlers/__init__.py index d67e133..ce7a5f1 100644 --- a/src/application/query_handlers/__init__.py +++ b/src/application/query_handlers/__init__.py @@ -4,7 +4,13 @@ Queries read from optimized read models and never modify state. """ from .get_book import GetBookHandler, GetBookQuery -from .list_books import BookReadModel, ListBooksHandler, ListBooksQuery +from .interfaces import ( + IBookQueryRepository, + ILoanQueryRepository, + IPatronQueryRepository, +) +from .list_books import ListBooksHandler, ListBooksQuery +from .read_models import BookReadModel, LoanReadModel, PatronReadModel __all__ = [ "ListBooksQuery", @@ -12,4 +18,9 @@ "GetBookQuery", "GetBookHandler", "BookReadModel", + "PatronReadModel", + "LoanReadModel", + "IBookQueryRepository", + "IPatronQueryRepository", + "ILoanQueryRepository", ] diff --git a/src/application/query_handlers/get_book.py b/src/application/query_handlers/get_book.py index 95859e9..4d4af99 100644 --- a/src/application/query_handlers/get_book.py +++ b/src/application/query_handlers/get_book.py @@ -8,12 +8,11 @@ from src.domain.catalog import BookNotFoundException -from .list_books import BookReadModel +from .read_models import BookReadModel if TYPE_CHECKING: - from src.domain.catalog import BookQueryRepository - from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.cache import CacheAdapter + from src.application.query_handlers.interfaces import IBookQueryRepository + from src.domain.shared_kernel import ICache, ILogger @dataclass(frozen=True) @@ -33,8 +32,8 @@ class GetBookHandler: def __init__( self, - query_repository: BookQueryRepository, - cache: CacheAdapter, + query_repository: IBookQueryRepository, + cache: ICache, logger: ILogger, ): self.query_repository = query_repository diff --git a/src/application/query_handlers/get_loan.py b/src/application/query_handlers/get_loan.py index 93cd857..918ea5a 100644 --- a/src/application/query_handlers/get_loan.py +++ b/src/application/query_handlers/get_loan.py @@ -7,24 +7,11 @@ from datetime import datetime from typing import TYPE_CHECKING, Optional -if TYPE_CHECKING: - from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.cache import CacheAdapter - from src.infrastructure.adapters.lending import LoanQueryRepository - +from .read_models import LoanReadModel -@dataclass(frozen=True) -class LoanReadModel: - """Read model for Loan.""" - id: str - patron_id: str - patron_email: str - catalog_book_id: str - book_title: str - borrowed_at: datetime - due_date: datetime - returned_at: Optional[datetime] - status: str +if TYPE_CHECKING: + from src.application.query_handlers.interfaces import ILoanQueryRepository + from src.domain.shared_kernel import ICache, ILogger @dataclass(frozen=True) @@ -40,8 +27,8 @@ class GetLoanHandler: def __init__( self, - query_repository: LoanQueryRepository, - cache: CacheAdapter, + query_repository: ILoanQueryRepository, + cache: ICache, logger: ILogger, ): self.query_repository = query_repository @@ -60,5 +47,5 @@ async def handle(self, query: GetLoanQuery) -> Optional[LoanReadModel]: if not result: return None - await self.cache.set(cache_key, result) - return LoanReadModel(**result) + await self.cache.set(cache_key, result.__dict__) + return result diff --git a/src/application/query_handlers/get_patron.py b/src/application/query_handlers/get_patron.py index 7ad0a27..f34a106 100644 --- a/src/application/query_handlers/get_patron.py +++ b/src/application/query_handlers/get_patron.py @@ -7,24 +7,11 @@ from datetime import datetime from typing import TYPE_CHECKING, Optional -if TYPE_CHECKING: - from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.cache import CacheAdapter - from src.infrastructure.adapters.patron import PatronQueryRepository - +from .read_models import PatronReadModel -@dataclass(frozen=True) -class PatronReadModel: - """Read model for Patron.""" - id: str - name: str - first_name: str - last_name: str - email: str - membership_tier: str - is_suspended: bool - suspended_reason: Optional[str] - registered_at: datetime +if TYPE_CHECKING: + from src.application.query_handlers.interfaces import IPatronQueryRepository + from src.domain.shared_kernel import ICache, ILogger @dataclass(frozen=True) @@ -40,8 +27,8 @@ class GetPatronHandler: def __init__( self, - query_repository: PatronQueryRepository, - cache: CacheAdapter, + query_repository: IPatronQueryRepository, + cache: ICache, logger: ILogger, ): self.query_repository = query_repository @@ -60,5 +47,5 @@ async def handle(self, query: GetPatronQuery) -> Optional[PatronReadModel]: if not result: return None - await self.cache.set(cache_key, result) - return PatronReadModel(**result) + await self.cache.set(cache_key, result.__dict__) + return result diff --git a/src/application/query_handlers/interfaces.py b/src/application/query_handlers/interfaces.py new file mode 100644 index 0000000..2eb9b08 --- /dev/null +++ b/src/application/query_handlers/interfaces.py @@ -0,0 +1,84 @@ +""" +Query-repository ports (CQRS read side). + +These live in the application layer, not the domain: query repositories +exist to serve views and return application read models, so the domain +has no business knowing about them. (The write-side ports — command +repositories and units of work, which speak in aggregates — remain in +the domain, where they belong.) + +Implementations live in infrastructure/adapters/*. +""" +from typing import List, Optional, Protocol + +from src.application.query_handlers.read_models import ( + BookReadModel, + LoanReadModel, + PatronReadModel, +) + + +class IBookQueryRepository(Protocol): + """Read-side port for book queries.""" + + async def find_by_id(self, book_id: str) -> Optional[BookReadModel]: + ... + + async def find_all( + self, + only_available: bool = False, + only_borrowed: bool = False, + author_contains: Optional[str] = None, + title_contains: Optional[str] = None, + limit: int = 100, + offset: int = 0, + ) -> List[BookReadModel]: + ... + + async def count( + self, + only_available: bool = False, + only_borrowed: bool = False, + ) -> int: + ... + + +class IPatronQueryRepository(Protocol): + """Read-side port for patron queries.""" + + async def find_by_id(self, patron_id: str) -> Optional[PatronReadModel]: + ... + + async def find_by_email(self, email: str) -> Optional[PatronReadModel]: + ... + + async def find_all( + self, + only_suspended: bool = False, + membership_tier: Optional[str] = None, + limit: int = 100, + offset: int = 0, + ) -> List[PatronReadModel]: + ... + + async def count(self, only_suspended: bool = False) -> int: + ... + + +class ILoanQueryRepository(Protocol): + """Read-side port for loan queries.""" + + async def find_by_id(self, loan_id: str) -> Optional[LoanReadModel]: + ... + + async def find_by_patron( + self, + patron_id: str, + only_active: bool = False, + limit: int = 100, + offset: int = 0, + ) -> List[LoanReadModel]: + ... + + async def find_overdue(self, limit: int = 100) -> List[LoanReadModel]: + ... diff --git a/src/application/query_handlers/list_books.py b/src/application/query_handlers/list_books.py index 723c379..7617fca 100644 --- a/src/application/query_handlers/list_books.py +++ b/src/application/query_handlers/list_books.py @@ -10,27 +10,11 @@ from datetime import datetime from typing import TYPE_CHECKING, List, Optional -if TYPE_CHECKING: - from src.domain.catalog import BookQueryRepository - from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.cache import CacheAdapter - +from .read_models import BookReadModel -@dataclass(frozen=True) -class BookReadModel: - """ - Read-optimized book representation. - - This is a denormalized view optimized for display, - separate from the write model (domain aggregate). - """ - id: str - title: str - author: str - is_borrowed: bool - status: str = "available" - borrowed_at: Optional[datetime] = None - return_due_date: Optional[datetime] = None +if TYPE_CHECKING: + from src.application.query_handlers.interfaces import IBookQueryRepository + from src.domain.shared_kernel import ICache, ILogger @dataclass(frozen=True) @@ -63,8 +47,8 @@ class ListBooksHandler: def __init__( self, - query_repository: BookQueryRepository, - cache: CacheAdapter, + query_repository: IBookQueryRepository, + cache: ICache, logger: ILogger, ): self.query_repository = query_repository diff --git a/src/application/query_handlers/list_patron_loans.py b/src/application/query_handlers/list_patron_loans.py index 13634d3..d37adae 100644 --- a/src/application/query_handlers/list_patron_loans.py +++ b/src/application/query_handlers/list_patron_loans.py @@ -7,24 +7,11 @@ from datetime import datetime from typing import TYPE_CHECKING, List, Optional -if TYPE_CHECKING: - from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.cache import CacheAdapter - from src.infrastructure.adapters.lending import LoanQueryRepository - +from .read_models import LoanReadModel -@dataclass(frozen=True) -class LoanReadModel: - """Read model for Loan.""" - id: str - patron_id: str - patron_email: str - catalog_book_id: str - book_title: str - borrowed_at: datetime - due_date: datetime - returned_at: Optional[datetime] - status: str +if TYPE_CHECKING: + from src.application.query_handlers.interfaces import ILoanQueryRepository + from src.domain.shared_kernel import ICache, ILogger @dataclass(frozen=True) @@ -43,8 +30,8 @@ class ListPatronLoansHandler: def __init__( self, - query_repository: LoanQueryRepository, - cache: CacheAdapter, + query_repository: ILoanQueryRepository, + cache: ICache, logger: ILogger, ): self.query_repository = query_repository @@ -72,5 +59,5 @@ async def handle(self, query: ListPatronLoansQuery) -> List[LoanReadModel]: offset=query.offset, ) - await self.cache.set(cache_key, results) - return [LoanReadModel(**r) for r in results] + await self.cache.set(cache_key, [r.__dict__ for r in results]) + return results diff --git a/src/application/query_handlers/list_patrons.py b/src/application/query_handlers/list_patrons.py index 3933f98..0cb9377 100644 --- a/src/application/query_handlers/list_patrons.py +++ b/src/application/query_handlers/list_patrons.py @@ -7,24 +7,11 @@ from datetime import datetime from typing import TYPE_CHECKING, List, Optional -if TYPE_CHECKING: - from src.domain.shared_kernel import ILogger - from src.infrastructure.adapters.cache import CacheAdapter - from src.infrastructure.adapters.patron import PatronQueryRepository - +from .read_models import PatronReadModel -@dataclass(frozen=True) -class PatronReadModel: - """Read model for Patron.""" - id: str - name: str - first_name: str - last_name: str - email: str - membership_tier: str - is_suspended: bool - suspended_reason: Optional[str] - registered_at: datetime +if TYPE_CHECKING: + from src.application.query_handlers.interfaces import IPatronQueryRepository + from src.domain.shared_kernel import ICache, ILogger @dataclass(frozen=True) @@ -43,8 +30,8 @@ class ListPatronsHandler: def __init__( self, - query_repository: PatronQueryRepository, - cache: CacheAdapter, + query_repository: IPatronQueryRepository, + cache: ICache, logger: ILogger, ): self.query_repository = query_repository @@ -72,5 +59,5 @@ async def handle(self, query: ListPatronsQuery) -> List[PatronReadModel]: offset=query.offset, ) - await self.cache.set(cache_key, results) - return [PatronReadModel(**r) for r in results] + await self.cache.set(cache_key, [r.__dict__ for r in results]) + return results diff --git a/src/application/query_handlers/read_models.py b/src/application/query_handlers/read_models.py new file mode 100644 index 0000000..c601586 --- /dev/null +++ b/src/application/query_handlers/read_models.py @@ -0,0 +1,52 @@ +""" +Read models - the query side's output DTOs. + +These are denormalized views optimized for display, separate from the +write model (the domain aggregates). They are the data structures that +cross the boundary out of the application layer, so they are defined +once here and shared by the query handlers, the query-repository ports, +and their infrastructure implementations. +""" +from dataclasses import dataclass +from datetime import datetime +from typing import Optional + + +@dataclass(frozen=True) +class BookReadModel: + """Read model for Book.""" + id: str + title: str + author: str + is_borrowed: bool + status: str = "available" + borrowed_at: Optional[datetime] = None + return_due_date: Optional[datetime] = None + + +@dataclass(frozen=True) +class PatronReadModel: + """Read model for Patron.""" + id: str + name: str + first_name: str + last_name: str + email: str + membership_tier: str + is_suspended: bool + suspended_reason: Optional[str] + registered_at: datetime + + +@dataclass(frozen=True) +class LoanReadModel: + """Read model for Loan.""" + id: str + patron_id: str + patron_email: str + catalog_book_id: str + book_title: str + borrowed_at: datetime + due_date: datetime + returned_at: Optional[datetime] + status: str diff --git a/src/container.py b/src/container.py index 0efa383..72b9d86 100644 --- a/src/container.py +++ b/src/container.py @@ -59,7 +59,9 @@ from src.infrastructure.adapters.logger import LoggerFactory from src.infrastructure.adapters.patron import (PatronQueryRepository, PatronUnitOfWork) -from src.infrastructure.adapters.resilience import CircuitBreakerFactory +from src.infrastructure.adapters.resilience import (CircuitBreakerFactory, + circuit_breaker_registry as + breaker_registry) from src.infrastructure.external.elasticsearch_client import \ ElasticsearchClient from src.infrastructure.external.etcd_client import EtcdClient @@ -186,6 +188,8 @@ class Container(containers.DeclarativeContainer): logger=logger, ) + circuit_breaker_registry = providers.Object(breaker_registry) + elasticsearch_circuit_breaker = providers.Singleton( CircuitBreakerFactory, name=configurations.circuit_breakers.elasticsearch.name, diff --git a/src/domain/catalog/__init__.py b/src/domain/catalog/__init__.py index 814da88..e2bcd7a 100644 --- a/src/domain/catalog/__init__.py +++ b/src/domain/catalog/__init__.py @@ -24,7 +24,7 @@ CatalogException, ConcurrentModificationException, ) -from .interfaces import IBookCommandRepository, IBookQueryRepository, ICatalogUnitOfWork +from .interfaces import IBookCommandRepository, ICatalogUnitOfWork from .value_objects import ISBN, Author, BookId, BookStatus, Title __all__ = [ @@ -41,7 +41,6 @@ "CatalogBookReserved", "CatalogBookReturned", "IBookCommandRepository", - "IBookQueryRepository", "ICatalogUnitOfWork", "DomainException", "ValidationException", diff --git a/src/domain/catalog/entities/catalog_book.py b/src/domain/catalog/entities/catalog_book.py index 0567509..1aa6a57 100644 --- a/src/domain/catalog/entities/catalog_book.py +++ b/src/domain/catalog/entities/catalog_book.py @@ -80,6 +80,8 @@ def confirm_borrow(self, borrower_email: str): """Confirm the reservation into a final borrow (loan created).""" if self.status != BookStatus.RESERVED: raise BookNotReservedException(self.id.value, self.status.value) + # reserve() always sets these; RESERVED status guarantees them + assert self.borrowed_at is not None and self.return_due_date is not None self.status = BookStatus.BORROWED self.reserved_at = None diff --git a/src/domain/catalog/interfaces/__init__.py b/src/domain/catalog/interfaces/__init__.py index ad8f357..d6b50f5 100644 --- a/src/domain/catalog/interfaces/__init__.py +++ b/src/domain/catalog/interfaces/__init__.py @@ -1,9 +1,7 @@ from .catalog_command_repository import IBookCommandRepository -from .catalog_query_repository import IBookQueryRepository from .unit_of_work import ICatalogUnitOfWork __all__ = [ "IBookCommandRepository", - "IBookQueryRepository", "ICatalogUnitOfWork", ] diff --git a/src/domain/catalog/interfaces/book_query_repository.py b/src/domain/catalog/interfaces/book_query_repository.py deleted file mode 100644 index df61bf2..0000000 --- a/src/domain/catalog/interfaces/book_query_repository.py +++ /dev/null @@ -1,57 +0,0 @@ -""" -Book Query Repository Interface - CQRS Read Side. - -This interface defines read operations separate from the write model. -Implementations can be optimized for queries (denormalized, cached, etc.). -""" -from __future__ import annotations - -from typing import TYPE_CHECKING, List, Optional, Protocol, runtime_checkable - -if TYPE_CHECKING: - from src.application.query_handlers import BookReadModel - - -@runtime_checkable -class BookQueryRepository(Protocol): - """ - Repository interface for book queries (CQRS read side). - - This is separate from the write repository (BookRepository) - to allow independent optimization of reads vs writes. - - Implementations might: - - Use a separate read database - - Use denormalized views - - Use caching (Redis, etc.) - - Use search engines (Elasticsearch) - """ - - async def find_by_id(self, book_id: str) -> Optional[BookReadModel]: - """Find a book by its ID.""" - ... - - async def find_all( - self, - only_available: bool = False, - only_borrowed: bool = False, - author_contains: Optional[str] = None, - title_contains: Optional[str] = None, - limit: int = 100, - offset: int = 0, - ) -> List[BookReadModel]: - """ - Find books with optional filters. - - This method is optimized for read operations and can - support complex filtering without affecting write performance. - """ - ... - - async def count( - self, - only_available: bool = False, - only_borrowed: bool = False, - ) -> int: - """Count books matching criteria.""" - ... diff --git a/src/domain/catalog/interfaces/catalog_book_repository.py b/src/domain/catalog/interfaces/catalog_book_repository.py deleted file mode 100644 index fb585d6..0000000 --- a/src/domain/catalog/interfaces/catalog_book_repository.py +++ /dev/null @@ -1,30 +0,0 @@ -""" -Repository interface for the Catalog bounded context. -""" -from typing import List, Optional, Protocol - -from src.domain.catalog.entities import Book - - -class BookRepository(Protocol): - """Repository for Book aggregates.""" - - async def add(self, book: Book) -> Book: - """Add a new book to the catalog.""" - ... - - async def get_by_id(self, book_id: str) -> Optional[Book]: - """Find a book by its ID.""" - ... - - async def get_all(self) -> List[Book]: - """Get all books in the catalog.""" - ... - - async def update(self, book: Book) -> None: - """Update a book.""" - ... - - async def remove(self, book_id: str) -> None: - """Remove a book from the catalog.""" - ... diff --git a/src/domain/catalog/interfaces/catalog_command_repository.py b/src/domain/catalog/interfaces/catalog_command_repository.py index a11fdf6..3507d91 100644 --- a/src/domain/catalog/interfaces/catalog_command_repository.py +++ b/src/domain/catalog/interfaces/catalog_command_repository.py @@ -1,6 +1,7 @@ """ Command Repository interface for Book aggregate. """ +from datetime import datetime from typing import List, Optional, Protocol from src.domain.catalog.entities import Book @@ -17,6 +18,10 @@ async def get_by_id(self, book_id: str) -> Optional[Book]: """Find a book by its ID.""" ... + async def find_expired_reservations(self, cutoff: datetime) -> List[Book]: + """Find books whose reservation started before the cutoff.""" + ... + async def get_all(self) -> List[Book]: """Get all books in the catalog.""" ... diff --git a/src/domain/catalog/interfaces/catalog_query_repository.py b/src/domain/catalog/interfaces/catalog_query_repository.py deleted file mode 100644 index 911ea6c..0000000 --- a/src/domain/catalog/interfaces/catalog_query_repository.py +++ /dev/null @@ -1,46 +0,0 @@ -""" -Catalog Query Repository Interface - CQRS Read Side. - -This interface defines read operations separate from the write model. -Implementations can be optimized for queries (denormalized, cached, etc.). -""" -from __future__ import annotations - -from typing import TYPE_CHECKING, List, Optional, Protocol, runtime_checkable - -if TYPE_CHECKING: - from src.application.query_handlers import BookReadModel - - -@runtime_checkable -class IBookQueryRepository(Protocol): - """ - Query repository interface for Book (CQRS read side). - - This is separate from the command repository to allow - independent optimization of reads vs writes. - """ - - async def find_by_id(self, book_id: str) -> Optional[BookReadModel]: - """Find a book by its ID.""" - ... - - async def find_all( - self, - only_available: bool = False, - only_borrowed: bool = False, - author_contains: Optional[str] = None, - title_contains: Optional[str] = None, - limit: int = 100, - offset: int = 0, - ) -> List[BookReadModel]: - """Find books with optional filters.""" - ... - - async def count( - self, - only_available: bool = False, - only_borrowed: bool = False, - ) -> int: - """Count books matching criteria.""" - ... diff --git a/src/domain/lending/__init__.py b/src/domain/lending/__init__.py index a60054d..43cb729 100644 --- a/src/domain/lending/__init__.py +++ b/src/domain/lending/__init__.py @@ -39,7 +39,7 @@ LoanNotActiveException, LoanNotOverdueException, ) -from .interfaces import ILoanCommandRepository, ILoanQueryRepository, ILoanUnitOfWork +from .interfaces import ILoanCommandRepository, ILoanUnitOfWork from .value_objects.lending_vo import DueDate, LoanId, LoanStatus __all__ = [ @@ -53,7 +53,6 @@ "LoanExtended", "BookOverdue", "ILoanCommandRepository", - "ILoanQueryRepository", "ILoanUnitOfWork", "InvalidLoanIdException", "LoanAlreadyReturnedException", diff --git a/src/domain/lending/interfaces/__init__.py b/src/domain/lending/interfaces/__init__.py index 7e02d8f..a708f15 100644 --- a/src/domain/lending/interfaces/__init__.py +++ b/src/domain/lending/interfaces/__init__.py @@ -1,9 +1,7 @@ from .loan_command_repository import ILoanCommandRepository -from .loan_query_repository import ILoanQueryRepository from .loan_unit_of_work import ILoanUnitOfWork __all__ = [ "ILoanCommandRepository", - "ILoanQueryRepository", "ILoanUnitOfWork", ] diff --git a/src/domain/lending/interfaces/loan_query_repository.py b/src/domain/lending/interfaces/loan_query_repository.py deleted file mode 100644 index 6cb93cc..0000000 --- a/src/domain/lending/interfaces/loan_query_repository.py +++ /dev/null @@ -1,28 +0,0 @@ -""" -Loan Query Repository Interface - CQRS Read Side. -""" -from __future__ import annotations - -from typing import List, Optional, Protocol - - -class ILoanQueryRepository(Protocol): - """Query repository interface for loans (CQRS read side).""" - - async def find_by_id(self, loan_id: str) -> Optional[dict]: - """Find a loan by ID.""" - ... - - async def find_by_patron( - self, - patron_id: str, - only_active: bool = False, - limit: int = 100, - offset: int = 0, - ) -> List[dict]: - """Find loans for a patron.""" - ... - - async def find_overdue(self, limit: int = 100) -> List[dict]: - """Find overdue loans.""" - ... diff --git a/src/domain/patron/__init__.py b/src/domain/patron/__init__.py index 2752b93..8225ddd 100644 --- a/src/domain/patron/__init__.py +++ b/src/domain/patron/__init__.py @@ -31,7 +31,7 @@ PatronAlreadySuspendedException, PatronNotSuspendedException, ) -from .interfaces import IPatronCommandRepository, IPatronQueryRepository, IPatronUnitOfWork +from .interfaces import IPatronCommandRepository, IPatronUnitOfWork from .value_objects.patron_info import MembershipTier, PatronId, PatronName __all__ = [ @@ -43,7 +43,6 @@ "PatronSuspended", "PatronReinstated", "IPatronCommandRepository", - "IPatronQueryRepository", "IPatronUnitOfWork", "InvalidPatronIdException", "InvalidPatronNameException", diff --git a/src/domain/patron/interfaces/__init__.py b/src/domain/patron/interfaces/__init__.py index 6b1bdc6..1a35bf4 100644 --- a/src/domain/patron/interfaces/__init__.py +++ b/src/domain/patron/interfaces/__init__.py @@ -1,9 +1,7 @@ from .patron_command_repository import IPatronCommandRepository -from .patron_query_repository import IPatronQueryRepository from .patron_unit_of_work import IPatronUnitOfWork __all__ = [ "IPatronCommandRepository", - "IPatronQueryRepository", "IPatronUnitOfWork", ] diff --git a/src/domain/patron/interfaces/patron_query_repository.py b/src/domain/patron/interfaces/patron_query_repository.py deleted file mode 100644 index 806ded5..0000000 --- a/src/domain/patron/interfaces/patron_query_repository.py +++ /dev/null @@ -1,32 +0,0 @@ -""" -Patron Query Repository Interface - CQRS Read Side. -""" -from __future__ import annotations - -from typing import List, Optional, Protocol - - -class IPatronQueryRepository(Protocol): - """Query repository interface for patron (CQRS read side).""" - - async def find_by_id(self, patron_id: str) -> Optional[dict]: - """Find a patron by ID.""" - ... - - async def find_by_email(self, email: str) -> Optional[dict]: - """Find a patron by email.""" - ... - - async def find_all( - self, - only_suspended: bool = False, - membership_tier: Optional[str] = None, - limit: int = 100, - offset: int = 0, - ) -> List[dict]: - """Find patrons with optional filters.""" - ... - - async def count(self, only_suspended: bool = False) -> int: - """Count patrons matching criteria.""" - ... diff --git a/src/domain/patron/interfaces/patron_repository.py b/src/domain/patron/interfaces/patron_repository.py deleted file mode 100644 index 703ad08..0000000 --- a/src/domain/patron/interfaces/patron_repository.py +++ /dev/null @@ -1,30 +0,0 @@ -""" -Repository interface for the Patron bounded context. -""" -from typing import List, Optional, Protocol - -from src.domain.patron.entities import Patron - - -class PatronRepository(Protocol): - """Repository for Patron aggregates.""" - - async def add(self, patron: Patron) -> Patron: - """Register a new patron.""" - ... - - async def get_by_id(self, patron_id: str) -> Optional[Patron]: - """Find a patron by ID.""" - ... - - async def get_by_email(self, email: str) -> Optional[Patron]: - """Find a patron by email address.""" - ... - - async def get_all(self) -> List[Patron]: - """Get all patrons.""" - ... - - async def update(self, patron: Patron) -> None: - """Update patron information.""" - ... diff --git a/src/domain/shared_kernel/__init__.py b/src/domain/shared_kernel/__init__.py index 9d10100..317484b 100644 --- a/src/domain/shared_kernel/__init__.py +++ b/src/domain/shared_kernel/__init__.py @@ -7,7 +7,6 @@ """ from .aggregate_root import AggregateRoot from .domain_event import DomainEvent -from .email_template import EmailTemplate from .exceptions import ( DomainException, EmailDeliveryException, @@ -22,7 +21,6 @@ IEventDispatcher, IEventHandler, ILogger, - ITemplateRenderer, ) from .value_objects import EmailAddress @@ -36,8 +34,6 @@ "IEventDispatcher", "IEventHandler", "ILogger", - "ITemplateRenderer", - "EmailTemplate", "DomainException", "EmailDeliveryException", "ValidationException", diff --git a/src/domain/shared_kernel/email_template.py b/src/domain/shared_kernel/email_template.py deleted file mode 100644 index 6a6d0dd..0000000 --- a/src/domain/shared_kernel/email_template.py +++ /dev/null @@ -1,10 +0,0 @@ -""" -Email templates used across bounded contexts. -""" -from enum import Enum - - -class EmailTemplate(Enum): - BOOK_BORROWED = "book_borrowed" - BOOK_RETURNED = "book_returned" - BOOK_OVERDUE = "book_overdue" diff --git a/src/domain/shared_kernel/exceptions.py b/src/domain/shared_kernel/exceptions.py index 1304b7e..54bfa36 100644 --- a/src/domain/shared_kernel/exceptions.py +++ b/src/domain/shared_kernel/exceptions.py @@ -34,6 +34,6 @@ class EmailDeliveryException(Exception): from transient failures (timeouts, open circuit breaker), which propagate as other exception types and are worth retrying. """ - def __init__(self, message: str, original_exception: Exception = None): + def __init__(self, message: str, original_exception: "Exception | None" = None): super().__init__(message) self.original_exception = original_exception diff --git a/src/domain/shared_kernel/interfaces.py b/src/domain/shared_kernel/interfaces.py index c38c56d..57fb994 100644 --- a/src/domain/shared_kernel/interfaces.py +++ b/src/domain/shared_kernel/interfaces.py @@ -57,11 +57,6 @@ async def send_email(self, to_email: str, subject: str, content: str) -> None: ... -class ITemplateRenderer(Protocol): - def render(self, template: Any, context: Dict[str, Any]) -> str: - ... - - class IConfigurationProvider(Protocol): def get(self, key: str, default: Any = None) -> Any: ... @@ -77,21 +72,30 @@ def close(self) -> None: class ICache(Protocol): - def get(self, key: str) -> Optional[Any]: + async def get(self, key: str) -> Optional[Any]: ... - def set(self, key: str, value: Any, ttl: Optional[int] = None) -> bool: + async def set(self, key: str, value: Any, ttl: Optional[int] = None) -> bool: ... - def delete(self, key: str) -> bool: + async def delete(self, key: str) -> bool: ... - def invalidate_entity(self, entity_type: str, entity_id: str) -> None: + async def invalidate_entity(self, entity_type: str, entity_id: str) -> None: ... - def invalidate_all(self, entity_type: str) -> None: + async def invalidate_all(self, entity_type: str) -> None: ... @property def is_enabled(self) -> bool: ... + + def build_key(self, *parts: Any) -> str: + ... + + def build_list_key(self, entity_type: str, **filters: Any) -> str: + ... + + def build_count_key(self, entity_type: str, **filters: Any) -> str: + ... diff --git a/src/infrastructure/adapters/cache/cache_adapter.py b/src/infrastructure/adapters/cache/cache_adapter.py index f6610b3..a750a2c 100644 --- a/src/infrastructure/adapters/cache/cache_adapter.py +++ b/src/infrastructure/adapters/cache/cache_adapter.py @@ -6,7 +6,7 @@ """ from __future__ import annotations -from typing import TYPE_CHECKING, Any, Callable, Optional, TypeVar +from typing import TYPE_CHECKING, Any, Awaitable, Callable, Optional, TypeVar if TYPE_CHECKING: from src.infrastructure.external.redis_client import RedisClient @@ -42,7 +42,7 @@ async def delete(self, key: str) -> bool: async def get_or_set( self, key: str, - factory: Callable[[], T], + factory: Callable[[], Awaitable[T]], ttl: Optional[int] = None, ) -> T: """ @@ -86,19 +86,16 @@ def is_enabled(self) -> bool: """Check if caching is enabled.""" return self._client.is_enabled - @staticmethod - def build_key(*parts: Any) -> str: + def build_key(self, *parts: Any) -> str: """Build a cache key from parts.""" return ":".join(str(p) for p in parts) - @staticmethod - def build_list_key(entity_type: str, **filters: Any) -> str: + def build_list_key(self, entity_type: str, **filters: Any) -> str: """Build a cache key for list queries.""" filter_str = ":".join(f"{k}={v}" for k, v in sorted(filters.items())) return f"{entity_type}:list:{filter_str}" - @staticmethod - def build_count_key(entity_type: str, **filters: Any) -> str: + def build_count_key(self, entity_type: str, **filters: Any) -> str: """Build a cache key for count queries.""" filter_str = ":".join(f"{k}={v}" for k, v in sorted(filters.items())) return f"{entity_type}:count:{filter_str}" diff --git a/src/infrastructure/adapters/catalog/catalog_unit_of_work.py b/src/infrastructure/adapters/catalog/catalog_unit_of_work.py index ddabe88..e28e000 100644 --- a/src/infrastructure/adapters/catalog/catalog_unit_of_work.py +++ b/src/infrastructure/adapters/catalog/catalog_unit_of_work.py @@ -60,6 +60,7 @@ def _stage_domain_events(self) -> None: state change and its events commit (or roll back) atomically. Debezium picks the rows up from the WAL and publishes them to Kafka. """ + assert self._session is not None for aggregate in self.identity_map.values(): for event in aggregate.get_domain_events(): self._session.add( diff --git a/src/infrastructure/adapters/lending/loan_query_repository.py b/src/infrastructure/adapters/lending/loan_query_repository.py index e802956..ce587bb 100644 --- a/src/infrastructure/adapters/lending/loan_query_repository.py +++ b/src/infrastructure/adapters/lending/loan_query_repository.py @@ -17,6 +17,7 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import async_sessionmaker +from src.application.query_handlers.read_models import LoanReadModel from src.infrastructure.adapters.lending.loan_model import LoanModel from src.infrastructure.exceptions import ( CircuitBreakerOpenException, @@ -53,7 +54,7 @@ def __init__( self._circuit_breaker = circuit_breaker self._logger = logger - async def find_by_id(self, loan_id: str) -> Optional[dict]: + async def find_by_id(self, loan_id: str) -> Optional[LoanReadModel]: """Find a loan by ID (uses PostgreSQL for consistency).""" async with self._session_factory() as session: result = await session.execute( @@ -70,7 +71,7 @@ async def find_by_patron( only_active: bool = False, limit: int = 100, offset: int = 0, - ) -> List[dict]: + ) -> List[LoanReadModel]: """Find loans for a patron (uses Elasticsearch for search).""" query = self._build_es_query( patron_id=patron_id, @@ -102,7 +103,7 @@ async def find_by_patron( for hit in result["hits"] ] - async def find_overdue(self, limit: int = 100) -> List[dict]: + async def find_overdue(self, limit: int = 100) -> List[LoanReadModel]: """Find overdue loans (uses Elasticsearch for search).""" query = {"bool": {"filter": [{"term": {"is_overdue": True}}]}} @@ -131,7 +132,7 @@ async def _find_by_patron_from_db( only_active: bool = False, limit: int = 100, offset: int = 0, - ) -> List[dict]: + ) -> List[LoanReadModel]: """PostgreSQL fallback for find_by_patron.""" stmt = select(LoanModel).where(LoanModel.patron_id == patron_id) @@ -144,7 +145,7 @@ async def _find_by_patron_from_db( result = await session.execute(stmt) return [self._to_read_model_from_db(row) for row in result.scalars().all()] - async def _find_overdue_from_db(self, limit: int = 100) -> List[dict]: + async def _find_overdue_from_db(self, limit: int = 100) -> List[LoanReadModel]: """PostgreSQL fallback for find_overdue.""" stmt = ( select(LoanModel) @@ -176,30 +177,30 @@ def _build_es_query( return {"bool": {"filter": filter_clauses}} return {"match_all": {}} - def _to_read_model_from_db(self, loan: LoanModel) -> dict: + def _to_read_model_from_db(self, loan: LoanModel) -> LoanReadModel: """Convert database row to read model.""" - return { - "id": loan.id, - "patron_id": loan.patron_id, - "patron_email": loan.patron_email, - "catalog_book_id": loan.catalog_book_id, - "book_title": loan.book_title, - "borrowed_at": loan.borrowed_at, - "due_date": loan.due_date, - "returned_at": loan.returned_at, - "status": loan.status, - } - - def _to_read_model_from_es(self, hit: dict[str, Any]) -> dict: + return LoanReadModel( + id=loan.id, + patron_id=loan.patron_id, + patron_email=loan.patron_email, + catalog_book_id=loan.catalog_book_id, + book_title=loan.book_title, + borrowed_at=loan.borrowed_at, + due_date=loan.due_date, + returned_at=loan.returned_at, + status=loan.status, + ) + + def _to_read_model_from_es(self, hit: dict[str, Any]) -> LoanReadModel: """Convert Elasticsearch hit to read model.""" - return { - "id": hit["id"], - "patron_id": hit.get("patron_id", ""), - "patron_email": hit.get("patron_email", ""), - "catalog_book_id": hit.get("catalog_book_id", ""), - "book_title": hit.get("book_title", ""), - "borrowed_at": hit.get("borrowed_at"), - "due_date": hit.get("due_date"), - "returned_at": hit.get("returned_at"), - "status": hit.get("status", ""), - } + return LoanReadModel( + id=hit["id"], + patron_id=hit.get("patron_id", ""), + patron_email=hit.get("patron_email", ""), + catalog_book_id=hit.get("catalog_book_id", ""), + book_title=hit.get("book_title", ""), + borrowed_at=hit.get("borrowed_at"), + due_date=hit.get("due_date"), + returned_at=hit.get("returned_at"), + status=hit.get("status", ""), + ) diff --git a/src/infrastructure/adapters/lending/loan_unit_of_work.py b/src/infrastructure/adapters/lending/loan_unit_of_work.py index e01084c..96a9f3c 100644 --- a/src/infrastructure/adapters/lending/loan_unit_of_work.py +++ b/src/infrastructure/adapters/lending/loan_unit_of_work.py @@ -7,6 +7,9 @@ from typing import TYPE_CHECKING, Dict, Optional +from sqlalchemy.exc import IntegrityError + +from src.domain.lending.exceptions import BookNotAvailableException from src.infrastructure.adapters.lending.loan_command_repository import ( LoanCommandRepository, ) @@ -50,7 +53,24 @@ async def __aexit__(self, exc_type, _exc_val, _exc_tb): async def commit(self): if self._session: self._stage_domain_events() - await self._session.commit() + try: + await self._session.commit() + except IntegrityError as e: + # Translate the framework exception at the boundary: the + # partial unique index (one active loan per book) losing a + # race is a domain fact, and callers must not need + # SQLAlchemy to understand it + await self.rollback() + if "ix_loans_active_book_unique" in str(e.orig): + book_id = next( + ( + loan.catalog_book_id + for loan in self.identity_map.values() + ), + "unknown", + ) + raise BookNotAvailableException(book_id) from e + raise def _stage_domain_events(self) -> None: """ @@ -60,6 +80,7 @@ def _stage_domain_events(self) -> None: state change and its events commit (or roll back) atomically. Debezium picks the rows up from the WAL and publishes them to Kafka. """ + assert self._session is not None for aggregate in self.identity_map.values(): for event in aggregate.get_domain_events(): self._session.add( diff --git a/src/infrastructure/adapters/patron/patron_query_repository.py b/src/infrastructure/adapters/patron/patron_query_repository.py index 45a88c3..d9200c9 100644 --- a/src/infrastructure/adapters/patron/patron_query_repository.py +++ b/src/infrastructure/adapters/patron/patron_query_repository.py @@ -16,6 +16,7 @@ from sqlalchemy import func, select from sqlalchemy.ext.asyncio import async_sessionmaker +from src.application.query_handlers.read_models import PatronReadModel from src.infrastructure.adapters.patron.patron_model import PatronModel from src.infrastructure.exceptions import ( CircuitBreakerOpenException, @@ -52,7 +53,7 @@ def __init__( self._circuit_breaker = circuit_breaker self._logger = logger - async def find_by_id(self, patron_id: str) -> Optional[dict]: + async def find_by_id(self, patron_id: str) -> Optional[PatronReadModel]: """Find a patron by ID (uses PostgreSQL for consistency).""" async with self._session_factory() as session: result = await session.execute( @@ -63,7 +64,7 @@ async def find_by_id(self, patron_id: str) -> Optional[dict]: return None return self._to_read_model_from_db(patron) - async def find_by_email(self, email: str) -> Optional[dict]: + async def find_by_email(self, email: str) -> Optional[PatronReadModel]: """Find a patron by email (uses PostgreSQL for consistency).""" async with self._session_factory() as session: result = await session.execute( @@ -80,7 +81,7 @@ async def find_all( membership_tier: Optional[str] = None, limit: int = 100, offset: int = 0, - ) -> List[dict]: + ) -> List[PatronReadModel]: """Find patrons with optional filters (uses Elasticsearch for search).""" query = self._build_es_query( only_suspended=only_suspended, @@ -135,7 +136,7 @@ async def _find_all_from_db( membership_tier: Optional[str] = None, limit: int = 100, offset: int = 0, - ) -> List[dict]: + ) -> List[PatronReadModel]: """PostgreSQL fallback for find_all.""" stmt = select(PatronModel) @@ -179,30 +180,30 @@ def _build_es_query( return {"bool": {"filter": filter_clauses}} return {"match_all": {}} - def _to_read_model_from_db(self, patron: PatronModel) -> dict: + def _to_read_model_from_db(self, patron: PatronModel) -> PatronReadModel: """Convert database row to read model.""" - return { - "id": patron.id, - "first_name": patron.first_name, - "last_name": patron.last_name, - "name": f"{patron.first_name} {patron.last_name}", - "email": patron.email, - "membership_tier": patron.membership_tier, - "is_suspended": patron.is_suspended, - "suspended_reason": patron.suspended_reason, - "registered_at": patron.registered_at, - } - - def _to_read_model_from_es(self, hit: dict[str, Any]) -> dict: + return PatronReadModel( + id=patron.id, + first_name=patron.first_name, + last_name=patron.last_name, + name=f"{patron.first_name} {patron.last_name}", + email=patron.email, + membership_tier=patron.membership_tier, + is_suspended=patron.is_suspended, + suspended_reason=patron.suspended_reason, + registered_at=patron.registered_at, + ) + + def _to_read_model_from_es(self, hit: dict[str, Any]) -> PatronReadModel: """Convert Elasticsearch hit to read model.""" - return { - "id": hit["id"], - "first_name": hit.get("first_name", ""), - "last_name": hit.get("last_name", ""), - "name": hit.get("full_name", ""), - "email": hit.get("email", ""), - "membership_tier": hit.get("membership_tier"), - "is_suspended": hit.get("is_suspended", False), - "suspended_reason": hit.get("suspended_reason"), - "registered_at": hit.get("registered_at"), - } + return PatronReadModel( + id=hit["id"], + first_name=hit.get("first_name", ""), + last_name=hit.get("last_name", ""), + name=hit.get("full_name", ""), + email=hit.get("email", ""), + membership_tier=hit.get("membership_tier"), + is_suspended=hit.get("is_suspended", False), + suspended_reason=hit.get("suspended_reason"), + registered_at=hit.get("registered_at"), + ) diff --git a/src/infrastructure/adapters/patron/patron_unit_of_work.py b/src/infrastructure/adapters/patron/patron_unit_of_work.py index d40dccf..63f608b 100644 --- a/src/infrastructure/adapters/patron/patron_unit_of_work.py +++ b/src/infrastructure/adapters/patron/patron_unit_of_work.py @@ -60,6 +60,7 @@ def _stage_domain_events(self) -> None: state change and its events commit (or roll back) atomically. Debezium picks the rows up from the WAL and publishes them to Kafka. """ + assert self._session is not None for aggregate in self.identity_map.values(): for event in aggregate.get_domain_events(): self._session.add( diff --git a/src/infrastructure/external/elasticsearch_client.py b/src/infrastructure/external/elasticsearch_client.py index 35402ba..71886dd 100644 --- a/src/infrastructure/external/elasticsearch_client.py +++ b/src/infrastructure/external/elasticsearch_client.py @@ -3,7 +3,7 @@ """ from __future__ import annotations -from typing import TYPE_CHECKING, Any, Optional +from typing import TYPE_CHECKING, Any, NoReturn, Optional from elasticsearch import AsyncElasticsearch, NotFoundError @@ -85,9 +85,10 @@ async def _ensure_connected(self) -> AsyncElasticsearch: # Connection failures must honor the typed-exception contract # too, or they bypass the circuit breaker and PG fallback self._raise("connect", e) + assert self._client is not None return self._client - def _raise(self, operation: str, error: Exception) -> None: + def _raise(self, operation: str, error: Exception) -> NoReturn: if self._logger: self._logger.error(f"Elasticsearch {operation} error: {error}") raise SearchEngineException( diff --git a/src/infrastructure/external/etcd_client.py b/src/infrastructure/external/etcd_client.py index 8cb3bfe..8a7a0fe 100644 --- a/src/infrastructure/external/etcd_client.py +++ b/src/infrastructure/external/etcd_client.py @@ -55,7 +55,7 @@ def _ensure_connected(self) -> etcd3.Etcd3Client: """Ensure we have an active connection.""" if self._client is None: self.connect() - return self._client # type: ignore + return self._client def get(self, key: str) -> Optional[bytes]: """Get a value by key.""" diff --git a/src/infrastructure/external/kafka_client.py b/src/infrastructure/external/kafka_client.py index 5fb40d7..b4ae8cc 100644 --- a/src/infrastructure/external/kafka_client.py +++ b/src/infrastructure/external/kafka_client.py @@ -110,6 +110,7 @@ async def send( """Send a message to a Kafka topic.""" if not self._producer: await self.connect_producer() + assert self._producer is not None try: await self._producer.send_and_wait(topic, value=value, key=key) diff --git a/src/infrastructure/external/postgresql.py b/src/infrastructure/external/postgresql.py index 137eedb..8673b09 100644 --- a/src/infrastructure/external/postgresql.py +++ b/src/infrastructure/external/postgresql.py @@ -63,6 +63,13 @@ async def init_models(self): async with self.engine.begin() as conn: await conn.run_sync(Base.metadata.create_all) + async def ping(self) -> None: + """Verify connectivity; raises if the database is unreachable.""" + from sqlalchemy import text + + async with self.session_factory() as session: + await session.execute(text("SELECT 1")) + async def dispose(self): """Dispose of the connection pool.""" await self.engine.dispose() diff --git a/src/presentation/api/main.py b/src/presentation/api/main.py index ead96d1..a751118 100644 --- a/src/presentation/api/main.py +++ b/src/presentation/api/main.py @@ -18,36 +18,25 @@ from src.presentation.api.routes import book_routes, health_routes, loan_routes, patron_routes -container = Container() -etcd_adapter = container.etcd_adapter() -etcd_adapter.load() -container.configurations.from_dict(etcd_adapter.get_all()) +class App(FastAPI): + """FastAPI application carrying its composition root.""" + container: Container @asynccontextmanager -async def lifespan(app: FastAPI): - db = container.postgresql() +async def lifespan(app: App): + db = app.container.postgresql() await db.init_models() # Instantiate circuit breakers eagerly: they register with the global # registry on creation, so /health/circuits reports every breaker from # startup instead of only after the first protected call - container.sendgrid_circuit_breaker() - container.elasticsearch_circuit_breaker() + app.container.sendgrid_circuit_breaker() + app.container.elasticsearch_circuit_breaker() yield await db.engine.dispose() -app = FastAPI( - title="Library API", - description="Clean Architecture Library Management System", - version="1.0.0", - lifespan=lifespan -) -app.container = container - - -@app.exception_handler(InfrastructureException) -async def infrastructure_exception_handler(request: Request, exc: InfrastructureException): +async def infrastructure_exception_handler(request: Request, exc: Exception): # Search backend unavailable (and PostgreSQL fallback also failed): # temporary condition, not a server bug if isinstance(exc, (SearchEngineException, CircuitBreakerOpenException)): @@ -55,8 +44,7 @@ async def infrastructure_exception_handler(request: Request, exc: Infrastructure return JSONResponse(status_code=500, content={"message": "Internal Server Error"}) -@app.exception_handler(DomainException) -async def domain_exception_handler(request: Request, exc: DomainException): +async def domain_exception_handler(request: Request, exc: Exception): if isinstance(exc, BookNotFoundException): return JSONResponse(status_code=404, content={"message": str(exc)}) if isinstance(exc, BookAlreadyBorrowedException): @@ -66,7 +54,34 @@ async def domain_exception_handler(request: Request, exc: DomainException): return JSONResponse(status_code=400, content={"message": str(exc)}) -app.include_router(health_routes.router) -app.include_router(book_routes.router) -app.include_router(loan_routes.router) -app.include_router(patron_routes.router) +def create_app() -> App: + """ + Application factory - the Main component. + + All composition happens here, on explicit invocation: importing this + module has no side effects (no container build, no etcd calls), which + keeps tests and tooling free to import without infrastructure. + Run with: uvicorn --factory src.presentation.api.main:create_app + """ + container = Container() + etcd_adapter = container.etcd_adapter() + etcd_adapter.load() + container.configurations.from_dict(etcd_adapter.get_all()) + + app = App( + title="Library API", + description="Clean Architecture Library Management System", + version="1.0.0", + lifespan=lifespan + ) + app.container = container + + app.add_exception_handler(InfrastructureException, infrastructure_exception_handler) + app.add_exception_handler(DomainException, domain_exception_handler) + + app.include_router(health_routes.router) + app.include_router(book_routes.router) + app.include_router(loan_routes.router) + app.include_router(patron_routes.router) + + return app diff --git a/src/presentation/api/routes/health_routes.py b/src/presentation/api/routes/health_routes.py index 8520605..79d0b24 100644 --- a/src/presentation/api/routes/health_routes.py +++ b/src/presentation/api/routes/health_routes.py @@ -12,11 +12,8 @@ from dependency_injector.wiring import Provide, inject from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel -from sqlalchemy import text from src.container import Container -from src.infrastructure.adapters.resilience import circuit_breaker_registry -from src.infrastructure.external.postgresql import PostgreSQL router = APIRouter(prefix="/health", tags=["Health"]) @@ -67,7 +64,8 @@ async def live(): @router.get("/ready", response_model=HealthStatus) @inject async def readiness( - postgresql: PostgreSQL = Depends(Provide[Container.postgresql]) + postgresql=Depends(Provide[Container.postgresql]), + registry=Depends(Provide[Container.circuit_breaker_registry]), ): """ Readiness check with dependency verification. @@ -83,14 +81,13 @@ async def readiness( all_healthy = True try: - async with postgresql.session_factory() as session: - await session.execute(text("SELECT 1")) + await postgresql.ping() checks["postgresql"] = {"status": "healthy"} except Exception as e: checks["postgresql"] = {"status": "unhealthy", "error": str(e)} all_healthy = False - unhealthy_circuits = circuit_breaker_registry.get_unhealthy() + unhealthy_circuits = registry.get_unhealthy() if unhealthy_circuits: checks["circuit_breakers"] = { "status": "degraded", @@ -115,7 +112,10 @@ async def readiness( @router.get("/circuits", response_model=CircuitBreakerStatus) -async def circuit_breakers(): +@inject +async def circuit_breakers( + registry=Depends(Provide[Container.circuit_breaker_registry]), +): """ Get detailed status of all circuit breakers. @@ -130,8 +130,8 @@ async def circuit_breakers(): Returns 200 always (this is informational only). """ - all_status = circuit_breaker_registry.get_all_status() - unhealthy = circuit_breaker_registry.get_unhealthy() + all_status = registry.get_all_status() + unhealthy = registry.get_unhealthy() return CircuitBreakerStatus( timestamp=datetime.utcnow().isoformat(), diff --git a/src/presentation/api/routes/loan_routes.py b/src/presentation/api/routes/loan_routes.py index 86adb7e..9285132 100644 --- a/src/presentation/api/routes/loan_routes.py +++ b/src/presentation/api/routes/loan_routes.py @@ -7,7 +7,6 @@ from dependency_injector.wiring import Provide, inject from fastapi import APIRouter, Depends, HTTPException, Query from pydantic import BaseModel -from sqlalchemy.exc import IntegrityError from src.application.command_handlers.create_loan import ( CreateLoanCommand, @@ -109,8 +108,6 @@ async def create_loan( ) except BookNotAvailableException as e: raise HTTPException(status_code=409, detail=str(e)) - except IntegrityError: - raise HTTPException(status_code=409, detail=f"Book {loan.catalog_book_id} is no longer available") except LendingException as e: raise HTTPException(status_code=400, detail=str(e)) diff --git a/src/presentation/api/routes/patron_routes.py b/src/presentation/api/routes/patron_routes.py index debf9ae..c948dfd 100644 --- a/src/presentation/api/routes/patron_routes.py +++ b/src/presentation/api/routes/patron_routes.py @@ -198,6 +198,8 @@ async def suspend_patron( await handler.handle(command) patron = await get_handler.handle(GetPatronQuery(patron_id=patron_id)) + if patron is None: + raise HTTPException(status_code=404, detail=f"Patron {patron_id} not found") return PatronResponse( id=patron.id, name=patron.name, @@ -228,6 +230,8 @@ async def reinstate_patron( await handler.handle(command) patron = await get_handler.handle(GetPatronQuery(patron_id=patron_id)) + if patron is None: + raise HTTPException(status_code=404, detail=f"Patron {patron_id} not found") return PatronResponse( id=patron.id, name=patron.name, @@ -259,6 +263,8 @@ async def upgrade_patron_tier( await handler.handle(command) patron = await get_handler.handle(GetPatronQuery(patron_id=patron_id)) + if patron is None: + raise HTTPException(status_code=404, detail=f"Patron {patron_id} not found") return PatronResponse( id=patron.id, name=patron.name, diff --git a/tests/application/test_event_handlers.py b/tests/application/test_event_handlers.py index f867ad3..6f2ce41 100644 --- a/tests/application/test_event_handlers.py +++ b/tests/application/test_event_handlers.py @@ -7,6 +7,7 @@ import pytest from src.application.command_handlers.create_loan import CreateLoanResult +from src.application.query_handlers import PatronReadModel from src.application.event_handlers import ( ConfirmBorrowOnLoanCreatedHandler, CreateLoanOnBookReservedHandler, @@ -18,6 +19,15 @@ from src.domain.shared_kernel import EmailDeliveryException +def _patron_read_model(patron_id="patron-1", is_suspended=False): + return PatronReadModel( + id=patron_id, name="Test Patron", first_name="Test", last_name="Patron", + email="patron@example.com", membership_tier="regular", + is_suspended=is_suspended, suspended_reason=None, + registered_at=datetime(2026, 1, 1), + ) + + def _book_reserved() -> CatalogBookReserved: return CatalogBookReserved( book_id="book-1", @@ -53,7 +63,7 @@ def _reserved_handler(patron=...): release_handler = AsyncMock() patron_repository = AsyncMock() patron_repository.find_by_email.return_value = ( - {"id": "patron-1", "is_suspended": False} if patron is ... else patron + _patron_read_model() if patron is ... else patron ) handler = CreateLoanOnBookReservedHandler( create_loan_handler=create_loan_handler, @@ -93,7 +103,7 @@ async def test_unknown_patron_compensates_by_releasing_reservation(self): @pytest.mark.asyncio async def test_suspended_patron_compensates_by_releasing_reservation(self): handler, create_loan, release = _reserved_handler( - patron={"id": "patron-1", "is_suspended": True} + patron=_patron_read_model(is_suspended=True) ) await handler.handle(_book_reserved()) diff --git a/tests/conftest.py b/tests/conftest.py index d6b8e26..8b892f0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,9 +5,8 @@ from httpx import ASGITransport, AsyncClient from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker -from src.container import Container from src.infrastructure.external.postgresql import PostgreSQL -from src.presentation.api.main import app +from src.presentation.api.main import create_app # Use in-memory SQLite for tests TEST_DATABASE_URL = "sqlite:///:memory:" @@ -23,35 +22,29 @@ async def test_db(): async def db_session(test_db) -> AsyncGenerator[AsyncSession, None]: connection = await test_db.engine.connect() transaction = await connection.begin() - + session_factory = async_sessionmaker(bind=connection, class_=AsyncSession, expire_on_commit=False) async with session_factory() as session: yield session - + await transaction.rollback() await connection.close() @pytest_asyncio.fixture async def client(test_db) -> AsyncGenerator[AsyncClient, None]: - container = Container() - - # Load configuration from etcd (same bootstrap as src.presentation.api.main) - etcd_adapter = container.etcd_adapter() - etcd_adapter.load() - container.configurations.from_dict(etcd_adapter.get_all()) + # The factory composes the app (container build + etcd config load) + # on invocation; importing the module has no side effects + app = create_app() + container = app.container container.postgresql.override(providers.Object(test_db)) - + # Mock external services from unittest.mock import AsyncMock container.event_dispatcher.override(providers.Object(AsyncMock())) container.email_service.override(providers.Object(AsyncMock())) - - app.container = container - + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac: yield ac - - container.postgresql.reset_override() - container.event_dispatcher.reset_override() - container.email_service.reset_override() + + container.unwire() diff --git a/tests/integration/test_borrow_choreography.py b/tests/integration/test_borrow_choreography.py index 1b804a1..32aa060 100644 --- a/tests/integration/test_borrow_choreography.py +++ b/tests/integration/test_borrow_choreography.py @@ -32,6 +32,7 @@ ReleaseExpiredReservationsCommand, ReleaseExpiredReservationsHandler, ) +from src.application.query_handlers import PatronReadModel from src.application.event_handlers import ( ConfirmBorrowOnLoanCreatedHandler, CreateLoanOnBookReservedHandler, @@ -44,6 +45,15 @@ from src.infrastructure.adapters.outbox import OutboxMessageModel +def _patron_read_model(patron_id): + return PatronReadModel( + id=patron_id, name="Choreo Patron", first_name="Choreo", last_name="Patron", + email="choreo@example.com", membership_tier="regular", + is_suspended=False, suspended_reason=None, + registered_at=datetime(2026, 1, 1), + ) + + async def _outbox_event(session_factory, event_type, aggregate_id): async with session_factory() as session: result = await session.execute( @@ -67,9 +77,7 @@ async def _reserve_and_capture_event(test_db, title: str, email: str): # Pre-flight check passes (the read-model guess says the patron is # fine); the lending-side reaction remains the authority preflight_patron_repository = AsyncMock() - preflight_patron_repository.find_by_email.return_value = { - "id": "patron-guess", "is_suspended": False, - } + preflight_patron_repository.find_by_email.return_value = _patron_read_model("patron-guess") reserved = await BorrowBookHandler( catalog_uow, patron_query_repository=preflight_patron_repository, @@ -108,7 +116,7 @@ async def test_full_saga_reserve_loan_confirm(test_db): # Step 2: lending reacts to the reservation await _lending_reaction( - session_factory, catalog_uow, patron={"id": "patron-choreo-1", "is_suspended": False} + session_factory, catalog_uow, patron=_patron_read_model("patron-choreo-1") ).handle(event) loan_uow = LoanUnitOfWork(session_factory) diff --git a/tests/integration/test_use_cases.py b/tests/integration/test_use_cases.py index ed3ee4f..7d8496f 100644 --- a/tests/integration/test_use_cases.py +++ b/tests/integration/test_use_cases.py @@ -8,10 +8,17 @@ def _patron_repository(patron=...): """Stub patron read model for the borrow pre-flight check.""" repo = AsyncMock() repo.find_by_email.return_value = ( - {"id": "patron-uc-1", "is_suspended": False} if patron is ... else patron + PatronReadModel( + id="patron-uc-1", name="UC Patron", first_name="UC", last_name="Patron", + email="uc@example.com", membership_tier="regular", + is_suspended=False, suspended_reason=None, + registered_at=datetime(2026, 1, 1), + ) if patron is ... else patron ) return repo +from datetime import datetime + import pytest from sqlalchemy.ext.asyncio import async_sessionmaker @@ -23,6 +30,7 @@ def _patron_repository(patron=...): ) from src.domain.catalog import (BookAlreadyBorrowedException, BorrowerNotEligibleException) +from src.application.query_handlers import PatronReadModel from src.infrastructure.adapters.catalog import CatalogUnitOfWork