From 374ede9527db696641019f87eacd4c6961654bb5 Mon Sep 17 00:00:00 2001 From: SuperElectron Date: Thu, 27 Aug 2026 11:51:58 -0700 Subject: [PATCH 1/3] =?UTF-8?q?feat:=20v2=20modules=20=E2=80=94=20async=5F?= =?UTF-8?q?producer=5Fconsumer,=20context=5Fmanager?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate both units to the module template: pattern/ (WorkerPool with both shutdown disciplines; AtomicWrite + temporarily), docs/ (fundamentals with classic-form contrast, implementation guide, cited external examples), mini-projects (feed_fetcher with per-item failure capture; atomic_deploy with ExitStack rollback), isolated behavioral tests. Legacy variant files removed. Co-Authored-By: Claude Fable 5 --- .../modern/async_producer_consumer/README.md | 42 ++---- .../async_producer_consumer/__init__.py | 14 +- .../async_producer_consumer/docs/examples.md | 25 ++++ .../docs/fundamentals.md | 86 ++++++++++++ .../docs/implementation.md | 47 +++++++ .../examples/__init__.py | 1 + .../examples/feed_fetcher/__init__.py | 16 +++ .../examples/feed_fetcher/__main__.py | 29 ++++ .../examples/feed_fetcher/fetcher.py | 57 ++++++++ .../examples/feed_fetcher/models.py | 30 +++++ .../modern/async_producer_consumer/naive.py | 39 ------ .../pattern/__init__.py | 10 ++ .../async_producer_consumer/pattern/pool.py | 125 ++++++++++++++++++ .../async_producer_consumer/pythonic.py | 41 ------ .../async_producer_consumer/real_world.py | 49 ------- .../tests/test_async_producer_consumer.py | 36 ----- .../tests/test_feed_fetcher.py | 54 ++++++++ .../tests/test_pool.py | 74 +++++++++++ patterns/modern/context_manager/README.md | 41 ++---- patterns/modern/context_manager/__init__.py | 9 +- .../modern/context_manager/docs/examples.md | 28 ++++ .../context_manager/docs/fundamentals.md | 75 +++++++++++ .../context_manager/docs/implementation.md | 49 +++++++ .../context_manager/examples/__init__.py | 1 + .../examples/atomic_deploy/__init__.py | 13 ++ .../examples/atomic_deploy/__main__.py | 30 +++++ .../examples/atomic_deploy/deploy.py | 60 +++++++++ patterns/modern/context_manager/naive.py | 55 -------- .../context_manager/pattern/__init__.py | 5 + .../context_manager/pattern/managers.py | 67 ++++++++++ patterns/modern/context_manager/pythonic.py | 52 -------- patterns/modern/context_manager/real_world.py | 32 ----- .../tests/test_atomic_deploy.py | 47 +++++++ .../tests/test_context_manager.py | 55 -------- .../context_manager/tests/test_managers.py | 56 ++++++++ 35 files changed, 1033 insertions(+), 417 deletions(-) create mode 100644 patterns/modern/async_producer_consumer/docs/examples.md create mode 100644 patterns/modern/async_producer_consumer/docs/fundamentals.md create mode 100644 patterns/modern/async_producer_consumer/docs/implementation.md create mode 100644 patterns/modern/async_producer_consumer/examples/__init__.py create mode 100644 patterns/modern/async_producer_consumer/examples/feed_fetcher/__init__.py create mode 100644 patterns/modern/async_producer_consumer/examples/feed_fetcher/__main__.py create mode 100644 patterns/modern/async_producer_consumer/examples/feed_fetcher/fetcher.py create mode 100644 patterns/modern/async_producer_consumer/examples/feed_fetcher/models.py delete mode 100644 patterns/modern/async_producer_consumer/naive.py create mode 100644 patterns/modern/async_producer_consumer/pattern/__init__.py create mode 100644 patterns/modern/async_producer_consumer/pattern/pool.py delete mode 100644 patterns/modern/async_producer_consumer/pythonic.py delete mode 100644 patterns/modern/async_producer_consumer/real_world.py delete mode 100644 patterns/modern/async_producer_consumer/tests/test_async_producer_consumer.py create mode 100644 patterns/modern/async_producer_consumer/tests/test_feed_fetcher.py create mode 100644 patterns/modern/async_producer_consumer/tests/test_pool.py create mode 100644 patterns/modern/context_manager/docs/examples.md create mode 100644 patterns/modern/context_manager/docs/fundamentals.md create mode 100644 patterns/modern/context_manager/docs/implementation.md create mode 100644 patterns/modern/context_manager/examples/__init__.py create mode 100644 patterns/modern/context_manager/examples/atomic_deploy/__init__.py create mode 100644 patterns/modern/context_manager/examples/atomic_deploy/__main__.py create mode 100644 patterns/modern/context_manager/examples/atomic_deploy/deploy.py delete mode 100644 patterns/modern/context_manager/naive.py create mode 100644 patterns/modern/context_manager/pattern/__init__.py create mode 100644 patterns/modern/context_manager/pattern/managers.py delete mode 100644 patterns/modern/context_manager/pythonic.py delete mode 100644 patterns/modern/context_manager/real_world.py create mode 100644 patterns/modern/context_manager/tests/test_atomic_deploy.py delete mode 100644 patterns/modern/context_manager/tests/test_context_manager.py create mode 100644 patterns/modern/context_manager/tests/test_managers.py diff --git a/patterns/modern/async_producer_consumer/README.md b/patterns/modern/async_producer_consumer/README.md index 365aa39..0305c50 100644 --- a/patterns/modern/async_producer_consumer/README.md +++ b/patterns/modern/async_producer_consumer/README.md @@ -14,31 +14,17 @@ stdlib_sightings: [asyncio.Queue, asyncio.TaskGroup, queue.Queue] # Async Producer/Consumer -## Problem - -Producers generate work faster (or slower) than consumers process it. You -want N workers pulling from a shared source, bounded memory in between, and -a shutdown that neither drops items nor hangs. - -## Naive solution - -`naive.py` is the thread version: `threading.Thread` workers around a -`queue.Queue` with sentinels — fine, but each worker burns an OS thread and -coordination is manual. - -## Pythonic solution - -`asyncio.Queue` with `TaskGroup`-managed workers: `maxsize` gives -backpressure, `queue.join()` waits for completion, cancellation ends the -idle workers. All the coordination is in the queue. - -## In the wild - -This *is* the stdlib idiom — the asyncio docs' own queue example is this -pattern; `real_world.py` shapes it as a rate-limited fetch pipeline with -per-item results collected in completion order. - -## Verdict - -**Use with care.** The right tool for I/O-bound fan-out; get the shutdown -discipline right (and tested) or debug it forever. +Fan I/O-bound work out to N workers over a bounded queue — backpressure by +`maxsize`, shutdown as an explicit, tested choice. **Verdict: use with care** +— the right tool for async fan-out; the two caveats are where it bites. + +| Where | What | +|---|---| +| [`pattern/`](pattern/) | The importable code: `WorkerPool`, `Shutdown`, `process_all` | +| [`docs/`](docs/) | [Fundamentals](docs/fundamentals.md) · [Implementation guide](docs/implementation.md) · [External examples](docs/examples.md) | +| [`examples/feed_fetcher/`](examples/feed_fetcher/) | Mini-project: feed pipeline with per-item failure capture, both shutdown disciplines | +| [`tests/`](tests/) | Behavioral tests for the pool and the mini-project | + +```bash +uv run python -m patterns.modern.async_producer_consumer.examples.feed_fetcher +``` diff --git a/patterns/modern/async_producer_consumer/__init__.py b/patterns/modern/async_producer_consumer/__init__.py index 85e6399..499f74d 100644 --- a/patterns/modern/async_producer_consumer/__init__.py +++ b/patterns/modern/async_producer_consumer/__init__.py @@ -1 +1,13 @@ -"""Async Producer/Consumer: bounded queues between async workers.""" +"""Async Producer/Consumer — public API. + +>>> from patterns.modern.async_producer_consumer import WorkerPool +""" + +from patterns.modern.async_producer_consumer.pattern import ( + Processor, + Shutdown, + WorkerPool, + process_all, +) + +__all__ = ["Processor", "Shutdown", "WorkerPool", "process_all"] diff --git a/patterns/modern/async_producer_consumer/docs/examples.md b/patterns/modern/async_producer_consumer/docs/examples.md new file mode 100644 index 0000000..122a56c --- /dev/null +++ b/patterns/modern/async_producer_consumer/docs/examples.md @@ -0,0 +1,25 @@ +# Async Producer/Consumer — external examples + +Real embodiments of the pattern outside this repo, for deeper study. + +## Standard library + +- **`asyncio.Queue`** — the buffer itself; the docs include a worked + producer/consumer example that is this pattern verbatim. + +- **`asyncio.TaskGroup`** (3.11+) — structured lifetime for the worker + tasks; the reason the pool needs no manual join/cancel bookkeeping + beyond its shutdown discipline. + +- **`queue.Queue`** — the threaded flavor, with the same + `task_done()`/`join()` contract the JOIN_AND_CANCEL discipline uses. + +- **`concurrent.futures`** — the pool-shaped alternative when items are + independent and you want futures rather than a shared queue. + + +## Elsewhere + +- **aiohttp** client examples — crawler-style fan-out over a session is + this pattern with real HTTP in the processor seam. *(unverified)* + diff --git a/patterns/modern/async_producer_consumer/docs/fundamentals.md b/patterns/modern/async_producer_consumer/docs/fundamentals.md new file mode 100644 index 0000000..26bce16 --- /dev/null +++ b/patterns/modern/async_producer_consumer/docs/fundamentals.md @@ -0,0 +1,86 @@ +# Async Producer/Consumer — fundamentals + +## Intent + +Decouple work *generation* from work *processing*: producers enqueue, N +consumers dequeue and process, and a bounded buffer between them keeps a fast +side from drowning a slow one. Under asyncio the pattern is how you fan +I/O-bound work out to concurrent workers with bounded memory and a shutdown +that neither drops items nor hangs. + +## Participants + +| Role | Classic (threaded) form | asyncio form | +|---|---|---| +| Buffer | `queue.Queue` + locks/conditions | `asyncio.Queue(maxsize=...)` — backpressure built in | +| Producers | Threads calling `put` | Any coroutine calling `await queue.put(item)` | +| Consumers | Worker threads in a `get` loop | N worker tasks — `WorkerPool` in [`pattern/pool.py`](../pattern/pool.py) | +| Worker lifetime | Manual `start`/`join` | `asyncio.TaskGroup` owns the tasks structurally | +| Shutdown discipline | Ad hoc, often forgotten | An explicit choice: `Shutdown.SENTINEL` or `Shutdown.JOIN_AND_CANCEL` | + +## Mechanism + +1. A bounded queue is created; `maxsize` is the memory budget *and* the + backpressure valve — `put` blocks when the buffer is full. +2. N workers start, each looping `get → process`. +3. Producers enqueue items; slow consumers automatically slow the producers. +4. Shutdown, the part naive versions get wrong, is one of two disciplines: + - **Sentinel** — after the last item, enqueue one end-marker per worker; + each worker exits on dequeuing one. + - **Join and cancel** — workers mark `task_done()`; the coordinator awaits + `queue.join()` (every item fetched *and* finished), then cancels the + now-idle workers. + +## The classic form, and what Python absorbs + +Before asyncio this was the thread pattern — an OS thread per worker, a lock +around shared results, and hand-rolled sentinel plumbing: + +```python +def process_all(items: list[str], worker_count: int = 2) -> list[str]: + channel: queue.Queue[str | None] = queue.Queue() + results: list[str] = [] + lock = threading.Lock() + + def worker() -> None: + while (item := channel.get()) is not None: + with lock: + results.append(item.upper()) + + workers = [threading.Thread(target=worker) for _ in range(worker_count)] + for w in workers: + w.start() + for item in items: + channel.put(item) + for _ in workers: + channel.put(None) # one sentinel per worker — forget one and it hangs + for w in workers: + w.join() + return sorted(results) +``` + +`asyncio.Queue` absorbs the locking entirely and `TaskGroup` absorbs the +lifetime bookkeeping. What Python does *not* absorb is the design: choosing +`maxsize`, and choosing — then testing — the shutdown discipline. That +remainder is the pattern. + +## When to use it + +- Many I/O-bound items (fetches, uploads, API calls) and you want bounded + concurrency rather than a task per item. +- Producers and consumers run at different, varying speeds and you need + memory to stay bounded in between. + +## When not to use it + +- CPU-bound work — an event loop serializes it; use a process pool. +- Independent items with no need to bound in-flight memory — + `asyncio.gather` (or `TaskGroup` alone) over per-item tasks is simpler. +- One item, one consumer — that is just an awaited call. + +## Verdict: use with care + +The right tool for I/O fan-out, with two sharp edges the caveats name: an +unbounded queue turns a slow consumer into a memory leak, and an untested +shutdown path is where these systems hang. `WorkerPool` makes both choices +explicit arguments so they cannot be forgotten — only wrong on purpose. diff --git a/patterns/modern/async_producer_consumer/docs/implementation.md b/patterns/modern/async_producer_consumer/docs/implementation.md new file mode 100644 index 0000000..aac1e09 --- /dev/null +++ b/patterns/modern/async_producer_consumer/docs/implementation.md @@ -0,0 +1,47 @@ +# Async Producer/Consumer — implementation guide + +## The smell that calls for it + +An `async` code path does `for item in items: await do(item)` and the wall +clock shows it — sequential awaits over independent I/O. Or the opposite: +`gather` over ten thousand tasks and memory shows *that*. + +## Introducing it, step by step + +1. **Isolate the per-item coroutine.** One `async def process(item) -> result` + with no shared state. This is the seam everything else plugs into. +2. **Pick the memory budget.** `maxsize` is how many items may sit fetched- + but-unprocessed. Small (2–16) is almost always right; it exists to create + backpressure, not to be a cache. +3. **Pick the worker count.** Concurrency toward the slow resource — for + HTTP this is "how many connections is polite", not "how many items exist". +4. **Pick the shutdown discipline — and write the test the same day.** + - `Shutdown.JOIN_AND_CANCEL` when a coordinator knows the item set and + wants "everything finished" as a joinable event. + - `Shutdown.SENTINEL` when the producer itself signals the end of a + stream and workers should drain and stop. +5. **Decide the failure policy at the edge.** The pool is fail-fast (one bad + item cancels the run, surfacing as an `ExceptionGroup`). If a bad item + must not kill the batch, catch inside *your* processor and return an + outcome object — as the [feed_fetcher example](../examples/feed_fetcher/) + does with `FetchOutcome`. + +## Idioms + +- `async with asyncio.TaskGroup()` owns the workers; nothing outlives the + block, and worker exceptions propagate instead of vanishing. +- Results in completion order, ordering as the caller's last step — sorting + inside the pool would hide the concurrency it exists to provide. +- The same shape works threaded (`queue.Queue`, `concurrent.futures`) when + the work is blocking rather than async; the design choices carry over. + +## Pitfalls + +- **Unbounded queue.** `asyncio.Queue()` with no `maxsize` removes the + backpressure that is half the pattern's point. +- **Mixed disciplines.** `task_done()` bookkeeping *and* sentinels in the + same pool — each looks redundant, together they deadlock or exit early. +- **Sentinel miscounting.** Fewer end-markers than workers hangs the rest; + route all shutdown through one tested code path (the pool), not call sites. +- **CPU-bound processors.** The event loop runs one coroutine at a time; + workers give you interleaved I/O waits, never parallel computation. diff --git a/patterns/modern/async_producer_consumer/examples/__init__.py b/patterns/modern/async_producer_consumer/examples/__init__.py new file mode 100644 index 0000000..07461af --- /dev/null +++ b/patterns/modern/async_producer_consumer/examples/__init__.py @@ -0,0 +1 @@ +"""Mini-projects demonstrating the async producer/consumer pattern in practice.""" diff --git a/patterns/modern/async_producer_consumer/examples/feed_fetcher/__init__.py b/patterns/modern/async_producer_consumer/examples/feed_fetcher/__init__.py new file mode 100644 index 0000000..dd5ce1e --- /dev/null +++ b/patterns/modern/async_producer_consumer/examples/feed_fetcher/__init__.py @@ -0,0 +1,16 @@ +"""Feed-fetching pipeline built on the async producer/consumer pool. + +Run it: ``uv run python -m patterns.modern.async_producer_consumer.examples.feed_fetcher`` +""" + +from patterns.modern.async_producer_consumer.examples.feed_fetcher.fetcher import ( + fetch_all, + fetch_entries, + summarize, +) +from patterns.modern.async_producer_consumer.examples.feed_fetcher.models import ( + Feed, + FetchOutcome, +) + +__all__ = ["Feed", "FetchOutcome", "fetch_all", "fetch_entries", "summarize"] diff --git a/patterns/modern/async_producer_consumer/examples/feed_fetcher/__main__.py b/patterns/modern/async_producer_consumer/examples/feed_fetcher/__main__.py new file mode 100644 index 0000000..f561a4e --- /dev/null +++ b/patterns/modern/async_producer_consumer/examples/feed_fetcher/__main__.py @@ -0,0 +1,29 @@ +"""Demo: a batch of feeds through the pool, under both shutdown disciplines.""" + +from __future__ import annotations + +import asyncio + +from patterns.modern.async_producer_consumer.examples.feed_fetcher.fetcher import ( + fetch_all, + summarize, +) +from patterns.modern.async_producer_consumer.examples.feed_fetcher.models import Feed +from patterns.modern.async_producer_consumer.pattern import Shutdown + + +def main() -> None: + feeds = [ + Feed("python-insider", "https://feeds.example/python-insider"), + Feed("lwn", "https://feeds.example/lwn"), + Feed("hn", "https://feeds.example/hn"), + Feed("dead-blog", "https://unreachable.example/rss"), + Feed("release-notes", "https://feeds.example/releases"), + ] + for shutdown in Shutdown: + outcomes = asyncio.run(fetch_all(feeds, shutdown=shutdown)) + print(f"{shutdown.value}: {summarize(outcomes)}") + + +if __name__ == "__main__": + main() diff --git a/patterns/modern/async_producer_consumer/examples/feed_fetcher/fetcher.py b/patterns/modern/async_producer_consumer/examples/feed_fetcher/fetcher.py new file mode 100644 index 0000000..6632fce --- /dev/null +++ b/patterns/modern/async_producer_consumer/examples/feed_fetcher/fetcher.py @@ -0,0 +1,57 @@ +"""The pipeline: N workers pull feeds through a bounded queue. + +A fake network stands in for HTTP so the demo and tests run offline and +deterministically; swap ``fetch_entries`` for a real client and nothing +else changes. +""" + +from __future__ import annotations + +import asyncio +from collections.abc import Iterable + +from patterns.modern.async_producer_consumer.examples.feed_fetcher.models import ( + Feed, + FetchOutcome, +) +from patterns.modern.async_producer_consumer.pattern import Shutdown, WorkerPool + + +async def fetch_entries(feed: Feed) -> int: + """Pretend to fetch and parse one feed; raise on a bad host.""" + await asyncio.sleep(0) # stand-in for real async I/O + if "unreachable" in feed.url: + raise ConnectionError(f"cannot reach {feed.url}") + return len(feed.name) # deterministic stand-in for "entries parsed" + + +async def fetch_all( + feeds: Iterable[Feed], + *, + workers: int = 3, + maxsize: int = 2, + shutdown: Shutdown = Shutdown.JOIN_AND_CANCEL, +) -> list[FetchOutcome]: + """Fetch every feed; failures become recorded outcomes, not crashes.""" + + async def capture(feed: Feed) -> FetchOutcome: + try: + return FetchOutcome(feed, entries=await fetch_entries(feed)) + except ConnectionError as exc: + return FetchOutcome(feed, error=str(exc)) + + pool: WorkerPool[Feed, FetchOutcome] = WorkerPool( + capture, workers=workers, maxsize=maxsize, shutdown=shutdown + ) + return await pool.run(feeds) + + +def summarize(outcomes: Iterable[FetchOutcome]) -> str: + """One line a human can read at the end of a run.""" + outcomes = list(outcomes) + fetched = sum(o.entries for o in outcomes if o.ok) + failed = [o.feed.name for o in outcomes if not o.ok] + line = f"{fetched} entries from {sum(o.ok for o in outcomes)} feeds" + if failed: + line += f"; failed: {', '.join(sorted(failed))}" + return line diff --git a/patterns/modern/async_producer_consumer/examples/feed_fetcher/models.py b/patterns/modern/async_producer_consumer/examples/feed_fetcher/models.py new file mode 100644 index 0000000..9d87ed0 --- /dev/null +++ b/patterns/modern/async_producer_consumer/examples/feed_fetcher/models.py @@ -0,0 +1,30 @@ +"""Domain objects for the feed-fetching pipeline.""" + +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Feed: + """A feed to fetch: a name and its URL.""" + + name: str + url: str + + +@dataclass(frozen=True) +class FetchOutcome: + """What happened to one feed — success with entries, or a recorded error. + + The pool itself is fail-fast; capturing per-feed failures is this + project's policy, applied inside its processor. + """ + + feed: Feed + entries: int = 0 + error: str | None = None + + @property + def ok(self) -> bool: + return self.error is None diff --git a/patterns/modern/async_producer_consumer/naive.py b/patterns/modern/async_producer_consumer/naive.py deleted file mode 100644 index 0b7b138..0000000 --- a/patterns/modern/async_producer_consumer/naive.py +++ /dev/null @@ -1,39 +0,0 @@ -"""The thread version: queue.Queue, sentinel-per-worker shutdown. - -Works, but every worker is an OS thread and the coordination is manual. -""" - -from __future__ import annotations - -import queue -import threading - - -def process_all(items: list[str], worker_count: int = 2) -> list[str]: - channel: queue.Queue[str | None] = queue.Queue() - results: list[str] = [] - lock = threading.Lock() - - def worker() -> None: - while (item := channel.get()) is not None: - with lock: - results.append(item.upper()) - - workers = [threading.Thread(target=worker) for _ in range(worker_count)] - for w in workers: - w.start() - for item in items: - channel.put(item) - for _ in workers: - channel.put(None) # one sentinel per worker - for w in workers: - w.join() - return sorted(results) - - -def main() -> None: - print(process_all(["a", "b", "c", "d"])) - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/async_producer_consumer/pattern/__init__.py b/patterns/modern/async_producer_consumer/pattern/__init__.py new file mode 100644 index 0000000..d397876 --- /dev/null +++ b/patterns/modern/async_producer_consumer/pattern/__init__.py @@ -0,0 +1,10 @@ +"""The async producer/consumer pattern, importable as library code.""" + +from patterns.modern.async_producer_consumer.pattern.pool import ( + Processor, + Shutdown, + WorkerPool, + process_all, +) + +__all__ = ["Processor", "Shutdown", "WorkerPool", "process_all"] diff --git a/patterns/modern/async_producer_consumer/pattern/pool.py b/patterns/modern/async_producer_consumer/pattern/pool.py new file mode 100644 index 0000000..f07aa84 --- /dev/null +++ b/patterns/modern/async_producer_consumer/pattern/pool.py @@ -0,0 +1,125 @@ +"""Async producer/consumer as an importable, typed building block. + +``WorkerPool`` fans items out to N workers over a bounded ``asyncio.Queue``: +``maxsize`` gives backpressure, and the shutdown discipline — the part the +classic pattern leaves implicit — is an explicit, tested choice +(:class:`Shutdown`). Results are collected in completion order. +""" + +from __future__ import annotations + +import asyncio +from collections.abc import Awaitable, Callable, Iterable +from enum import Enum +from typing import Generic, TypeVar + +Item = TypeVar("Item") +Result = TypeVar("Result") + +Processor = Callable[[Item], Awaitable[Result]] + + +class Shutdown(Enum): + """How the pool tells its workers the work is over. + + ``SENTINEL``: one end-marker per worker is enqueued after the items; + each worker exits when it dequeues one. ``JOIN_AND_CANCEL``: workers + loop forever; the pool awaits ``queue.join()`` then cancels them. + Pick one and test it — mixing disciplines is where shutdown bugs live. + """ + + SENTINEL = "sentinel" + JOIN_AND_CANCEL = "join-and-cancel" + + +class _End: + """Private end-of-work marker for the sentinel discipline.""" + + +_END = _End() + + +class WorkerPool(Generic[Item, Result]): + """N workers processing items from a bounded queue. + + The pool is fail-fast: an exception in ``process`` cancels the run and + surfaces as an ``ExceptionGroup`` (via ``TaskGroup``). Callers who want + per-item failure capture wrap it in their processor. + """ + + def __init__( + self, + process: Processor[Item, Result], + *, + workers: int = 4, + maxsize: int = 8, + shutdown: Shutdown = Shutdown.JOIN_AND_CANCEL, + ) -> None: + if workers < 1: + raise ValueError("a pool needs at least one worker") + self._process = process + self._workers = workers + self._maxsize = maxsize + self._shutdown = shutdown + + async def run(self, items: Iterable[Item]) -> list[Result]: + """Process every item; return results in completion order.""" + if self._shutdown is Shutdown.SENTINEL: + return await self._run_sentinel(items) + return await self._run_join_and_cancel(items) + + async def _run_join_and_cancel(self, items: Iterable[Item]) -> list[Result]: + channel: asyncio.Queue[Item] = asyncio.Queue(maxsize=self._maxsize) + results: list[Result] = [] + + async def worker() -> None: + while True: + item = await channel.get() + try: + results.append(await self._process(item)) + finally: + channel.task_done() + + async with asyncio.TaskGroup() as group: + workers = [group.create_task(worker()) for _ in range(self._workers)] + for item in items: + await channel.put(item) # blocks when full: backpressure + await channel.join() # every item fetched AND task_done() + for w in workers: + w.cancel() # idle workers end; TaskGroup absorbs this + return results + + async def _run_sentinel(self, items: Iterable[Item]) -> list[Result]: + channel: asyncio.Queue[Item | _End] = asyncio.Queue(maxsize=self._maxsize) + results: list[Result] = [] + + async def worker() -> None: + while True: + got = await channel.get() + if isinstance(got, _End): + return # a worker consumes exactly one sentinel + results.append(await self._process(got)) + + async with asyncio.TaskGroup() as group: + for _ in range(self._workers): + group.create_task(worker()) + for item in items: + await channel.put(item) + for _ in range(self._workers): + await channel.put(_END) # one per worker, after the items + return results + + +async def process_all( + items: Iterable[Item], + process: Processor[Item, Result], + *, + workers: int = 4, + maxsize: int = 8, + shutdown: Shutdown = Shutdown.JOIN_AND_CANCEL, +) -> list[Result]: + """One-shot convenience over :class:`WorkerPool`.""" + pool: WorkerPool[Item, Result] = WorkerPool( + process, workers=workers, maxsize=maxsize, shutdown=shutdown + ) + return await pool.run(items) diff --git a/patterns/modern/async_producer_consumer/pythonic.py b/patterns/modern/async_producer_consumer/pythonic.py deleted file mode 100644 index fe5e6cb..0000000 --- a/patterns/modern/async_producer_consumer/pythonic.py +++ /dev/null @@ -1,41 +0,0 @@ -"""asyncio.Queue + TaskGroup workers. - -maxsize bounds memory (backpressure), join() waits for all items to be -processed, cancellation ends the idle workers. -""" - -from __future__ import annotations - -import asyncio - - -async def process_all(items: list[str], worker_count: int = 3) -> list[str]: - channel: asyncio.Queue[str] = asyncio.Queue(maxsize=2) # backpressure - results: list[str] = [] - - async def worker() -> None: - while True: - item = await channel.get() - try: - await asyncio.sleep(0) # stand-in for real async I/O - results.append(item.upper()) - finally: - channel.task_done() - - async with asyncio.TaskGroup() as group: - workers = [group.create_task(worker()) for _ in range(worker_count)] - for item in items: - await channel.put(item) # blocks when the queue is full - await channel.join() # all items fetched AND task_done() - for w in workers: - w.cancel() # idle workers end; TaskGroup absorbs the cancellation - - return sorted(results) - - -def main() -> None: - print(asyncio.run(process_all(["a", "b", "c", "d", "e"]))) - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/async_producer_consumer/real_world.py b/patterns/modern/async_producer_consumer/real_world.py deleted file mode 100644 index d52c3cf..0000000 --- a/patterns/modern/async_producer_consumer/real_world.py +++ /dev/null @@ -1,49 +0,0 @@ -"""The idiom shaped as a pipeline: N workers, bounded queue, ordered results. - -A fake fetcher stands in for HTTP so the demo and tests run offline; swap it -for a real client and nothing else changes. -""" - -from __future__ import annotations - -import asyncio -from collections.abc import Awaitable, Callable - -Fetcher = Callable[[str], Awaitable[str]] - - -async def fake_fetch(url: str) -> str: - await asyncio.sleep(0) - return f"body-of-{url}" - - -async def crawl(urls: list[str], fetch: Fetcher = fake_fetch, workers: int = 4) -> dict[str, str]: - """Fan URLs out to workers; collect {url: body} whatever the finish order.""" - channel: asyncio.Queue[str] = asyncio.Queue(maxsize=8) - pages: dict[str, str] = {} - - async def worker() -> None: - while True: - url = await channel.get() - try: - pages[url] = await fetch(url) - finally: - channel.task_done() - - async with asyncio.TaskGroup() as group: - tasks = [group.create_task(worker()) for _ in range(workers)] - for url in urls: - await channel.put(url) - await channel.join() - for t in tasks: - t.cancel() - return pages - - -def main() -> None: - urls = [f"https://example.com/{n}" for n in range(3)] - print(asyncio.run(crawl(urls))) - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/async_producer_consumer/tests/test_async_producer_consumer.py b/patterns/modern/async_producer_consumer/tests/test_async_producer_consumer.py deleted file mode 100644 index feffcf6..0000000 --- a/patterns/modern/async_producer_consumer/tests/test_async_producer_consumer.py +++ /dev/null @@ -1,36 +0,0 @@ -"""Behavioral tests for all three producer/consumer variants.""" - -from patterns.modern.async_producer_consumer import naive, pythonic, real_world - - -class TestNaive: - def test_thread_pool_processes_everything(self) -> None: - assert naive.process_all(["a", "b", "c", "d"]) == ["A", "B", "C", "D"] - - def test_zero_items(self) -> None: - assert naive.process_all([]) == [] - - -class TestPythonic: - async def test_all_items_processed_despite_backpressure(self) -> None: - items = [chr(ord("a") + n) for n in range(10)] # more items than maxsize - assert await pythonic.process_all(items) == [c.upper() for c in items] - - async def test_more_workers_than_items(self) -> None: - assert await pythonic.process_all(["x"], worker_count=5) == ["X"] - - async def test_zero_items_shuts_down_cleanly(self) -> None: - assert await pythonic.process_all([]) == [] - - -class TestRealWorld: - async def test_crawl_collects_every_url(self) -> None: - urls = [f"u{n}" for n in range(9)] - pages = await real_world.crawl(urls, workers=3) - assert pages == {u: f"body-of-{u}" for u in urls} - - async def test_injected_fetcher(self) -> None: - async def fetch(url: str) -> str: - return url[::-1] - - assert await real_world.crawl(["abc"], fetch=fetch) == {"abc": "cba"} diff --git a/patterns/modern/async_producer_consumer/tests/test_feed_fetcher.py b/patterns/modern/async_producer_consumer/tests/test_feed_fetcher.py new file mode 100644 index 0000000..56651ab --- /dev/null +++ b/patterns/modern/async_producer_consumer/tests/test_feed_fetcher.py @@ -0,0 +1,54 @@ +"""Behavioral tests for the feed_fetcher mini-project.""" + +from __future__ import annotations + +import pytest + +from patterns.modern.async_producer_consumer.examples.feed_fetcher import ( + Feed, + fetch_all, + summarize, +) +from patterns.modern.async_producer_consumer.examples.feed_fetcher.__main__ import main +from patterns.modern.async_producer_consumer.pattern import Shutdown + +FEEDS = [ + Feed("alpha", "https://feeds.example/alpha"), + Feed("beta", "https://feeds.example/beta"), + Feed("dead", "https://unreachable.example/rss"), +] + + +class TestFetchAll: + @pytest.mark.parametrize("shutdown", list(Shutdown)) + async def test_failures_are_captured_not_raised(self, shutdown: Shutdown) -> None: + outcomes = {o.feed.name: o for o in await fetch_all(FEEDS, shutdown=shutdown)} + assert len(outcomes) == 3 + assert outcomes["alpha"].ok and outcomes["alpha"].entries == len("alpha") + assert not outcomes["dead"].ok + assert outcomes["dead"].error is not None + assert "unreachable" in outcomes["dead"].error + + async def test_both_disciplines_agree_on_outcomes(self) -> None: + by_discipline = [ + sorted((o.feed.name, o.ok) for o in await fetch_all(FEEDS, shutdown=s)) + for s in Shutdown + ] + assert by_discipline[0] == by_discipline[1] + + +class TestSummarize: + async def test_reports_totals_and_failures(self) -> None: + line = summarize(await fetch_all(FEEDS)) + assert "2 feeds" in line + assert f"{len('alpha') + len('beta')} entries" in line + assert "failed: dead" in line + + +class TestDemo: + def test_demo_prints_one_line_per_discipline(self, capsys: pytest.CaptureFixture[str]) -> None: + main() + out = capsys.readouterr().out.strip().splitlines() + assert len(out) == len(Shutdown) + assert any(line.startswith("sentinel:") for line in out) + assert all("failed: dead-blog" in line for line in out) diff --git a/patterns/modern/async_producer_consumer/tests/test_pool.py b/patterns/modern/async_producer_consumer/tests/test_pool.py new file mode 100644 index 0000000..7e4883c --- /dev/null +++ b/patterns/modern/async_producer_consumer/tests/test_pool.py @@ -0,0 +1,74 @@ +"""Behavioral tests for the WorkerPool building block.""" + +from __future__ import annotations + +import asyncio + +import pytest + +from patterns.modern.async_producer_consumer.pattern import ( + Shutdown, + WorkerPool, + process_all, +) + + +async def upper(item: str) -> str: + await asyncio.sleep(0) + return item.upper() + + +class TestBothDisciplines: + @pytest.mark.parametrize("shutdown", list(Shutdown)) + async def test_processes_every_item_despite_backpressure(self, shutdown: Shutdown) -> None: + items = [chr(ord("a") + n) for n in range(10)] # far more than maxsize + results = await process_all(items, upper, maxsize=2, shutdown=shutdown) + assert sorted(results) == [c.upper() for c in items] + + @pytest.mark.parametrize("shutdown", list(Shutdown)) + async def test_zero_items(self, shutdown: Shutdown) -> None: + assert await process_all([], upper, shutdown=shutdown) == [] + + @pytest.mark.parametrize("shutdown", list(Shutdown)) + async def test_more_workers_than_items(self, shutdown: Shutdown) -> None: + assert await process_all(["x"], upper, workers=5, shutdown=shutdown) == ["X"] + + @pytest.mark.parametrize("shutdown", list(Shutdown)) + async def test_processor_error_fails_fast_as_exception_group(self, shutdown: Shutdown) -> None: + async def explode(item: str) -> str: + raise ValueError(f"bad item {item}") + + pool: WorkerPool[str, str] = WorkerPool(explode, shutdown=shutdown) + with pytest.raises(ExceptionGroup) as excinfo: + await pool.run(["a"]) + assert excinfo.group_contains(ValueError) + + +class TestConcurrencyContract: + async def test_in_flight_work_is_bounded_by_worker_count(self) -> None: + in_flight = 0 + seen_max = 0 + + async def track(item: int) -> int: + nonlocal in_flight, seen_max + in_flight += 1 + seen_max = max(seen_max, in_flight) + await asyncio.sleep(0) # yield so other workers get a turn + in_flight -= 1 + return item + + await process_all(range(20), track, workers=3, maxsize=2) + assert seen_max <= 3 + + async def test_results_are_not_sorted_by_the_pool(self) -> None: + async def slow_first(item: int) -> int: + await asyncio.sleep(0.02 if item == 0 else 0) + return item + + results = await process_all([0, 1, 2, 3], slow_first, workers=4) + assert sorted(results) == [0, 1, 2, 3] + assert results[-1] == 0 # the slow item finishes last, and stays last + + def test_pool_requires_at_least_one_worker(self) -> None: + with pytest.raises(ValueError): + WorkerPool(upper, workers=0) diff --git a/patterns/modern/context_manager/README.md b/patterns/modern/context_manager/README.md index 62e3b91..69b6b14 100644 --- a/patterns/modern/context_manager/README.md +++ b/patterns/modern/context_manager/README.md @@ -14,31 +14,16 @@ stdlib_sightings: [open, contextlib.contextmanager, contextlib.ExitStack, tempfi # Context Manager -## Problem - -Every acquired resource — file, lock, connection, temporary state — must be -released on *every* exit path. Hand-written `try/finally` scattered through a -codebase is where cleanup bugs live. - -## Naive solution - -`naive.py` is the try/finally discipline done by hand, including the nested -two-resource version that shows why it doesn't scale. - -## Pythonic solution - -The `with` statement makes the pairing structural: `pythonic.py` implements -the protocol both ways — a class with `__enter__`/`__exit__`, and the -generator form via `@contextmanager` where the `yield` splits acquire from -release. - -## In the wild - -`open`, locks, and sqlite transactions are all context managers; -`contextlib.ExitStack` manages a *dynamic* number of them, unwinding in -reverse on the way out — shown in `real_world.py`. - -## Verdict - -**Pythonic.** Python's own RAII; any acquire/release pair you write twice -deserves one. +Pair acquire with release on every exit path, structurally — Python's RAII. +**Verdict: pythonic** — any acquire/release pair you write twice deserves one. + +| Where | What | +|---|---| +| [`pattern/`](pattern/) | The importable code: `AtomicWrite` (protocol form), `temporarily` (generator form) | +| [`docs/`](docs/) | [Fundamentals](docs/fundamentals.md) · [Implementation guide](docs/implementation.md) · [External examples](docs/examples.md) | +| [`examples/atomic_deploy/`](examples/atomic_deploy/) | Mini-project: all-or-nothing config deployment via `ExitStack` | +| [`tests/`](tests/) | Behavioral tests for both managers and the mini-project | + +```bash +uv run python -m patterns.modern.context_manager.examples.atomic_deploy +``` diff --git a/patterns/modern/context_manager/__init__.py b/patterns/modern/context_manager/__init__.py index 85b6773..ac82640 100644 --- a/patterns/modern/context_manager/__init__.py +++ b/patterns/modern/context_manager/__init__.py @@ -1 +1,8 @@ -"""Context Manager: structural acquire/release pairing.""" +"""Context Manager — public API. + +>>> from patterns.modern.context_manager import AtomicWrite +""" + +from patterns.modern.context_manager.pattern import AtomicWrite, temporarily + +__all__ = ["AtomicWrite", "temporarily"] diff --git a/patterns/modern/context_manager/docs/examples.md b/patterns/modern/context_manager/docs/examples.md new file mode 100644 index 0000000..e7a8557 --- /dev/null +++ b/patterns/modern/context_manager/docs/examples.md @@ -0,0 +1,28 @@ +# Context Manager — external examples + +Real embodiments of the pattern outside this repo, for deeper study. + +## Origin + +- **PEP 343 — the `with` statement** — rationale and full semantics; + the pattern's founding document. + +## Standard library + +- **`contextlib`** — `contextmanager`, `ExitStack`, `suppress`, `closing`, + `ContextDecorator`: every construction form in one module. + +- **`open`, locks, `tempfile.TemporaryDirectory`** — the everyday managers; + a `with open(...)` is the pattern most Python code meets first. +- **`sqlite3.Connection`** — commit on clean exit, rollback on exception: + the branching-exit shape `AtomicWrite` mirrors. + + +## Elsewhere + +- **pytest yield fixtures** — setup/teardown expressed exactly as the + generator form: code before the `yield` is setup, after is teardown. + *(unverified)* +- **Django `transaction.atomic`** — one transaction seam usable as context + manager or decorator. *(unverified)* + diff --git a/patterns/modern/context_manager/docs/fundamentals.md b/patterns/modern/context_manager/docs/fundamentals.md new file mode 100644 index 0000000..cfd2e0d --- /dev/null +++ b/patterns/modern/context_manager/docs/fundamentals.md @@ -0,0 +1,75 @@ +# Context Manager — fundamentals + +## Intent + +Guarantee that acquire and release are paired around a block of code on +*every* exit path — normal return, early return, exception. The `with` +statement (PEP 343) makes the pairing structural instead of disciplinary: +cleanup lives with the acquisition, written once, not re-written correctly +at every call site. + +## Participants + +| Role | Form | Where | +|---|---|---| +| The protocol | `__enter__` / `__exit__` on a class | `AtomicWrite` in [`pattern/managers.py`](../pattern/managers.py) | +| The generator form | `@contextlib.contextmanager` around a `yield` | `temporarily` in the same module | +| Composition | `contextlib.ExitStack` — a dynamic pile of managers | the [atomic_deploy example](../examples/atomic_deploy/) | +| Client | `with manager as value:` | any block needing the guarantee | + +## Mechanism + +1. `with` calls `__enter__`; its return value binds to `as`. +2. The body runs. +3. `__exit__` runs *no matter how the body ended*, receiving the exception + triple (or three `None`s). Returning falsy re-raises; returning `True` + swallows the exception — do that only on purpose. +4. In the generator form, the `yield` is the seam: code before it is + `__enter__`, code after it is `__exit__` — which is why the `yield` must + sit inside `try/finally`, or an exception in the body skips the cleanup. + +## The classic form, and what Python absorbs + +Before `with`, the guarantee was hand-written `try/finally` at every call +site — correct, and unscalable: + +```python +def use_two(log: list[str]) -> None: + first = Resource("a", log) + try: + second = Resource("b", log) # every extra resource nests a level + try: + log.append("work") + finally: + second.close() + finally: + first.close() +``` + +The `with` statement absorbs the nesting and the discipline; `contextlib` +absorbs the boilerplate of writing managers; `ExitStack` absorbs the +"unknown number of resources" case. What remains — the pattern — is spotting +the acquire/release pair and choosing the right construction form for it. + +## Choosing the form + +- **Protocol class** when exit logic branches (commit vs discard, like + `AtomicWrite`), when the manager has state worth naming, or when it must + be re-entered. +- **Generator form** when cleanup is one unconditional restore + (`temporarily`) — three lines instead of a class. +- **`ExitStack`** when how many managers you need is a runtime fact, or + when you want callbacks-as-cleanup with `pop_all()` as the commit. + +## When not to use it + +- No release side exists — a plain function is enough. +- The "cleanup" must survive the process (a saga, a queued compensation) — + that is workflow logic, not block scoping. + +## Verdict: pythonic + +This *is* Python's RAII, made explicit. Any acquire/release pair you write +twice deserves a context manager; the two caveats (yield inside +`try/finally`; returning `True` from `__exit__` swallows) are the only +sharp edges. diff --git a/patterns/modern/context_manager/docs/implementation.md b/patterns/modern/context_manager/docs/implementation.md new file mode 100644 index 0000000..469a2a3 --- /dev/null +++ b/patterns/modern/context_manager/docs/implementation.md @@ -0,0 +1,49 @@ +# Context Manager — implementation guide + +## The smell that calls for it + +The same `try/finally` shape appears at more than one call site; a code +review comment says "don't forget to close/unlock/restore this"; a bug +report shows cleanup skipped on the exception path. + +## Introducing it, step by step + +1. **Name the pair.** What exactly is acquired, and what must run on exit? + If you cannot state the release in one sentence, the block is doing too + much to manage. +2. **Pick the form** (see [fundamentals](fundamentals.md)): branching exit + logic → protocol class; one unconditional cleanup → generator form; + a runtime-sized set of cleanups → `ExitStack`. +3. **Write the exception path first.** The manager exists for the failure + case: decide what a mid-block exception means (discard? restore? both?) + and test that before the happy path. +4. **Keep `__enter__` cheap and `__exit__` unconditional.** Acquisition + failures should raise *before* the body runs; release must not depend on + how far the body got. +5. **Replace the call sites** with `with`, deleting their hand-rolled + `try/finally`. The diff should only remove lines. + +## Idioms + +- Generator form: the `yield` inside `try/finally`, always — the unit's + first caveat exists because the failure is silent otherwise. +- `ExitStack.callback(undo, ...)` per step, then `pop_all()` on success: + transactional multi-step work where the commit is "don't run the undos" + (shown in [atomic_deploy](../examples/atomic_deploy/deploy.py)). +- A context manager that is also a decorator: subclass + `contextlib.ContextDecorator`, or stack `@contextmanager` functions. +- `contextlib.suppress(SomeError)` instead of `try/except: pass` — the + intent gets a name. + +## Pitfalls + +- **`yield` outside `try/finally`** in a generator manager: cleanup runs + only on the success path. The most common real-world defect in this + pattern. +- **Returning `True` from `__exit__`** (or swallowing in `finally`): + exceptions vanish. Only `contextlib.suppress`-style managers should ever + do it, and loudly. +- **Doing work in `__init__`.** Acquire in `__enter__`, or the manager + cannot be reused and fails before the `with` can protect it. +- **One giant manager** for several unrelated resources — compose small + ones with `ExitStack` instead; each stays testable alone. diff --git a/patterns/modern/context_manager/examples/__init__.py b/patterns/modern/context_manager/examples/__init__.py new file mode 100644 index 0000000..eaf1971 --- /dev/null +++ b/patterns/modern/context_manager/examples/__init__.py @@ -0,0 +1 @@ +"""Mini-projects demonstrating the context manager pattern in practice.""" diff --git a/patterns/modern/context_manager/examples/atomic_deploy/__init__.py b/patterns/modern/context_manager/examples/atomic_deploy/__init__.py new file mode 100644 index 0000000..2fed0ca --- /dev/null +++ b/patterns/modern/context_manager/examples/atomic_deploy/__init__.py @@ -0,0 +1,13 @@ +"""Atomic config deployment built on the context manager pattern. + +Run it: ``uv run python -m patterns.modern.context_manager.examples.atomic_deploy`` +""" + +from patterns.modern.context_manager.examples.atomic_deploy.deploy import ( + ReleaseError, + deploy, + no_validation, + require_nonempty, +) + +__all__ = ["ReleaseError", "deploy", "no_validation", "require_nonempty"] diff --git a/patterns/modern/context_manager/examples/atomic_deploy/__main__.py b/patterns/modern/context_manager/examples/atomic_deploy/__main__.py new file mode 100644 index 0000000..4f148de --- /dev/null +++ b/patterns/modern/context_manager/examples/atomic_deploy/__main__.py @@ -0,0 +1,30 @@ +"""Demo: a good release deploys; a bad one rolls back completely.""" + +from __future__ import annotations + +import tempfile +from pathlib import Path + +from patterns.modern.context_manager.examples.atomic_deploy.deploy import ( + ReleaseError, + deploy, + require_nonempty, +) + + +def main() -> None: + with tempfile.TemporaryDirectory() as tmp: + target = Path(tmp) + deploy({"app.toml": "retries = 3\n", "logging.toml": "level = 'info'\n"}, target) + print(f"v1 deployed: {sorted(p.name for p in target.iterdir())}") + + bad_release = {"app.toml": "retries = 5\n", "logging.toml": " "} + try: + deploy(bad_release, target, validate=require_nonempty) + except ReleaseError as exc: + print(f"v2 rejected ({exc})") + print(f"app.toml still reads: {(target / 'app.toml').read_text().strip()}") + + +if __name__ == "__main__": + main() diff --git a/patterns/modern/context_manager/examples/atomic_deploy/deploy.py b/patterns/modern/context_manager/examples/atomic_deploy/deploy.py new file mode 100644 index 0000000..8594888 --- /dev/null +++ b/patterns/modern/context_manager/examples/atomic_deploy/deploy.py @@ -0,0 +1,60 @@ +"""All-or-nothing config deployment, composed from context managers. + +Each file is written with ``AtomicWrite`` (old-or-new, never half). The +release as a whole is made transactional with ``ExitStack``: every written +file pushes a rollback callback, and only a fully validated release pops +them off uncalled — the commit *is* ``pop_all()``. Any exception on the way +unwinds the stack, restoring every file already touched. +""" + +from __future__ import annotations + +from collections.abc import Callable, Mapping +from contextlib import ExitStack +from pathlib import Path + +from patterns.modern.context_manager.pattern import AtomicWrite + +Validator = Callable[[str, str], None] + + +class ReleaseError(RuntimeError): + """A file in the release failed validation; nothing was deployed.""" + + +def _restore(path: Path, previous: str | None) -> None: + if previous is None: + path.unlink() # the file did not exist before this release + else: + path.write_text(previous, encoding="utf-8") + + +def no_validation(name: str, content: str) -> None: + """The default validator: accept everything.""" + + +def require_nonempty(name: str, content: str) -> None: + """A realistic validator: an empty config file is a broken release.""" + if not content.strip(): + raise ReleaseError(f"{name} is empty") + + +def deploy( + release: Mapping[str, str], + target: Path, + *, + validate: Validator = no_validation, +) -> list[Path]: + """Write every file in ``release`` into ``target``, or none of them.""" + written: list[Path] = [] + with ExitStack() as rollback: + for name, content in sorted(release.items()): + validate(name, content) + path = target / name + previous = path.read_text(encoding="utf-8") if path.exists() else None + with AtomicWrite(path) as handle: + handle.write(content) + rollback.callback(_restore, path, previous) # undo, if we unwind + written.append(path) + rollback.pop_all() # every file landed: cancel the rollbacks + return written diff --git a/patterns/modern/context_manager/naive.py b/patterns/modern/context_manager/naive.py deleted file mode 100644 index dbebb88..0000000 --- a/patterns/modern/context_manager/naive.py +++ /dev/null @@ -1,55 +0,0 @@ -"""Cleanup by hand: try/finally on every exit path. - -Correct -- and it must be re-written correctly at every call site. -The nested version shows why the discipline doesn't scale. -""" - -from __future__ import annotations - - -class Resource: - def __init__(self, name: str, log: list[str]) -> None: - self.name = name - self.log = log - self.log.append(f"open {name}") - - def close(self) -> None: - self.log.append(f"close {self.name}") - - -def use_one(log: list[str], *, explode: bool = False) -> None: - resource = Resource("a", log) - try: - log.append("work") - if explode: - raise RuntimeError("boom") - finally: - resource.close() - - -def use_two(log: list[str]) -> None: - first = Resource("a", log) - try: - second = Resource("b", log) # every extra resource nests another level - try: - log.append("work") - finally: - second.close() - finally: - first.close() - - -def main() -> None: - import contextlib - - log: list[str] = [] - with contextlib.suppress(RuntimeError): # itself a context manager! - use_one(log, explode=True) - print(f"cleanup survived the exception: {log}") - log.clear() - use_two(log) - print(f"nested by hand: {log}") - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/context_manager/pattern/__init__.py b/patterns/modern/context_manager/pattern/__init__.py new file mode 100644 index 0000000..2acc08b --- /dev/null +++ b/patterns/modern/context_manager/pattern/__init__.py @@ -0,0 +1,5 @@ +"""The context manager pattern, importable as library code.""" + +from patterns.modern.context_manager.pattern.managers import AtomicWrite, temporarily + +__all__ = ["AtomicWrite", "temporarily"] diff --git a/patterns/modern/context_manager/pattern/managers.py b/patterns/modern/context_manager/pattern/managers.py new file mode 100644 index 0000000..87d80dc --- /dev/null +++ b/patterns/modern/context_manager/pattern/managers.py @@ -0,0 +1,67 @@ +"""Context managers as importable, typed building blocks. + +Two general-purpose managers, one per construction form the pattern offers: +``AtomicWrite`` implements the protocol (``__enter__``/``__exit__``) because +its exit logic branches on the exception; ``temporarily`` uses the generator +form because its cleanup is one unconditional restore. Choosing the form to +fit the cleanup is itself part of the pattern. +""" + +from __future__ import annotations + +import os +import tempfile +from collections.abc import Iterator +from contextlib import contextmanager +from pathlib import Path +from types import TracebackType +from typing import IO, Any + + +class AtomicWrite: + """Write a file so readers see the old content or the new — never half. + + Text is written to a temp file beside ``path``; a clean exit renames it + over ``path`` (atomic on POSIX), an exception discards it and leaves any + previous content untouched. + """ + + def __init__(self, path: Path, *, encoding: str = "utf-8") -> None: + self._path = path + self._encoding = encoding + self._handle: IO[str] | None = None + self._tmp_name = "" + + def __enter__(self) -> IO[str]: + fd, self._tmp_name = tempfile.mkstemp(dir=self._path.parent, prefix=f".{self._path.name}.") + self._handle = os.fdopen(fd, "w", encoding=self._encoding) + return self._handle + + def __exit__( + self, + exc_type: type[BaseException] | None, + exc: BaseException | None, + tb: TracebackType | None, + ) -> None: + # Returning None (falsy): never swallow the body's exception. + if self._handle is not None: + self._handle.close() + if exc_type is None: + os.replace(self._tmp_name, self._path) # the atomic commit + else: + os.unlink(self._tmp_name) # discard; the old file stays intact + + +@contextmanager +def temporarily(obj: Any, attribute: str, value: object) -> Iterator[None]: + """Set ``obj.attribute = value`` for the block; restore on any exit. + + The ``yield`` sits inside ``try/finally`` — without that, an exception + in the body would skip the restore (the unit's first caveat). + """ + previous = getattr(obj, attribute) + setattr(obj, attribute, value) + try: + yield + finally: + setattr(obj, attribute, previous) diff --git a/patterns/modern/context_manager/pythonic.py b/patterns/modern/context_manager/pythonic.py deleted file mode 100644 index 516c048..0000000 --- a/patterns/modern/context_manager/pythonic.py +++ /dev/null @@ -1,52 +0,0 @@ -"""The protocol, both ways. - -A class with __enter__/__exit__, and the generator form where the yield is -the seam between acquire and release. Note the try/finally around the yield: -without it, an exception in the body skips cleanup. -""" - -from __future__ import annotations - -from collections.abc import Iterator -from contextlib import contextmanager -from types import TracebackType - - -class Managed: - def __init__(self, name: str, log: list[str]) -> None: - self.name = name - self.log = log - - def __enter__(self) -> Managed: - self.log.append(f"open {self.name}") - return self - - def __exit__( - self, - exc_type: type[BaseException] | None, - exc: BaseException | None, - tb: TracebackType | None, - ) -> None: - self.log.append(f"close {self.name}") # returning None: never swallow - - -@contextmanager -def managed(name: str, log: list[str]) -> Iterator[str]: - log.append(f"open {name}") - try: - yield name - finally: - log.append(f"close {name}") - - -def main() -> None: - log: list[str] = [] - with Managed("a", log): - log.append("work") - with managed("b", log): - log.append("more work") - print(log) - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/context_manager/real_world.py b/patterns/modern/context_manager/real_world.py deleted file mode 100644 index 0b88142..0000000 --- a/patterns/modern/context_manager/real_world.py +++ /dev/null @@ -1,32 +0,0 @@ -"""``contextlib.ExitStack``: a dynamic pile of context managers. - -Open N resources decided at runtime; the stack unwinds them all, in -reverse, on any exit. -""" - -from __future__ import annotations - -import tempfile -from contextlib import ExitStack -from pathlib import Path - - -def concatenate(paths: list[Path]) -> str: - """Open however many files there are; every handle closes on exit.""" - with ExitStack() as stack: - handles = [stack.enter_context(p.open()) for p in paths] - return "".join(h.read() for h in handles) - - -def main() -> None: - with tempfile.TemporaryDirectory() as tmp: # itself a context manager - paths = [] - for i, text in enumerate(["one ", "two ", "three"]): - path = Path(tmp) / f"{i}.txt" - path.write_text(text) - paths.append(path) - print(concatenate(paths)) - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/context_manager/tests/test_atomic_deploy.py b/patterns/modern/context_manager/tests/test_atomic_deploy.py new file mode 100644 index 0000000..a102446 --- /dev/null +++ b/patterns/modern/context_manager/tests/test_atomic_deploy.py @@ -0,0 +1,47 @@ +"""Behavioral tests for the atomic_deploy mini-project.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from patterns.modern.context_manager.examples.atomic_deploy import ( + ReleaseError, + deploy, + require_nonempty, +) +from patterns.modern.context_manager.examples.atomic_deploy.__main__ import main + +V1 = {"app.toml": "retries = 3\n", "logging.toml": "level = 'info'\n"} + + +class TestDeploy: + def test_good_release_writes_every_file(self, tmp_path: Path) -> None: + written = deploy(V1, tmp_path) + assert sorted(p.name for p in written) == ["app.toml", "logging.toml"] + assert (tmp_path / "app.toml").read_text() == V1["app.toml"] + + def test_failing_release_restores_previous_contents(self, tmp_path: Path) -> None: + deploy(V1, tmp_path) + bad_v2 = {"app.toml": "retries = 5\n", "logging.toml": " "} + with pytest.raises(ReleaseError, match=r"logging\.toml"): + deploy(bad_v2, tmp_path, validate=require_nonempty) + # app.toml sorts before logging.toml, so it WAS written — and rolled back. + assert (tmp_path / "app.toml").read_text() == V1["app.toml"] + assert (tmp_path / "logging.toml").read_text() == V1["logging.toml"] + + def test_failing_release_on_fresh_target_leaves_no_files(self, tmp_path: Path) -> None: + bad = {"a.toml": "ok = true\n", "z.toml": ""} + with pytest.raises(ReleaseError): + deploy(bad, tmp_path, validate=require_nonempty) + assert list(tmp_path.iterdir()) == [] # a.toml was created, then removed + + +class TestDemo: + def test_demo_shows_deploy_then_rollback(self, capsys: pytest.CaptureFixture[str]) -> None: + main() + out = capsys.readouterr().out + assert "v1 deployed: ['app.toml', 'logging.toml']" in out + assert "v2 rejected" in out + assert "app.toml still reads: retries = 3" in out diff --git a/patterns/modern/context_manager/tests/test_context_manager.py b/patterns/modern/context_manager/tests/test_context_manager.py deleted file mode 100644 index d122e90..0000000 --- a/patterns/modern/context_manager/tests/test_context_manager.py +++ /dev/null @@ -1,55 +0,0 @@ -"""Behavioral tests for all three context-manager variants.""" - -import tempfile -from pathlib import Path - -import pytest - -from patterns.modern.context_manager import naive, pythonic, real_world - - -class TestNaive: - def test_finally_cleans_up_on_exception(self) -> None: - log: list[str] = [] - with pytest.raises(RuntimeError): - naive.use_one(log, explode=True) - assert log == ["open a", "work", "close a"] - - def test_nested_resources_close_in_reverse(self) -> None: - log: list[str] = [] - naive.use_two(log) - assert log == ["open a", "open b", "work", "close b", "close a"] - - -class TestPythonic: - def test_class_form_pairs_enter_and_exit(self) -> None: - log: list[str] = [] - with pythonic.Managed("a", log): - log.append("work") - assert log == ["open a", "work", "close a"] - - def test_class_form_cleans_up_on_exception(self) -> None: - log: list[str] = [] - with pytest.raises(ValueError, match="boom"), pythonic.Managed("a", log): - raise ValueError("boom") - assert log == ["open a", "close a"] - - def test_generator_form_cleans_up_on_exception(self) -> None: - log: list[str] = [] - with pytest.raises(ValueError), pythonic.managed("g", log): - raise ValueError - assert log == ["open g", "close g"] - - -class TestRealWorld: - def test_exit_stack_handles_a_runtime_number_of_files(self) -> None: - with tempfile.TemporaryDirectory() as tmp: - paths = [] - for i, text in enumerate(["x", "y"]): - p = Path(tmp) / f"{i}.txt" - p.write_text(text) - paths.append(p) - assert real_world.concatenate(paths) == "xy" - - def test_empty_stack_is_fine(self) -> None: - assert real_world.concatenate([]) == "" diff --git a/patterns/modern/context_manager/tests/test_managers.py b/patterns/modern/context_manager/tests/test_managers.py new file mode 100644 index 0000000..6f2c8f0 --- /dev/null +++ b/patterns/modern/context_manager/tests/test_managers.py @@ -0,0 +1,56 @@ +"""Behavioral tests for the AtomicWrite and temporarily managers.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from patterns.modern.context_manager.pattern import AtomicWrite, temporarily + + +class TestAtomicWrite: + def test_clean_exit_commits(self, tmp_path: Path) -> None: + target = tmp_path / "config.toml" + with AtomicWrite(target) as handle: + handle.write("v2") + assert target.read_text() == "v2" + + def test_exception_discards_and_keeps_the_old_content(self, tmp_path: Path) -> None: + target = tmp_path / "config.toml" + target.write_text("v1") + with pytest.raises(RuntimeError, match="boom"), AtomicWrite(target) as handle: + handle.write("half-written v2") + raise RuntimeError("boom") + assert target.read_text() == "v1" # reader never sees the half-write + + def test_exception_on_a_fresh_path_leaves_nothing(self, tmp_path: Path) -> None: + target = tmp_path / "new.toml" + with pytest.raises(RuntimeError), AtomicWrite(target) as handle: + handle.write("partial") + raise RuntimeError("boom") + assert not target.exists() + assert list(tmp_path.iterdir()) == [] # no orphaned temp file either + + +class TestTemporarily: + class Settings: + retries = 3 + + def test_restores_after_the_block(self) -> None: + settings = self.Settings() + with temporarily(settings, "retries", 99): + assert settings.retries == 99 + assert settings.retries == 3 + + def test_restores_even_when_the_body_raises(self) -> None: + settings = self.Settings() + with pytest.raises(ValueError), temporarily(settings, "retries", 99): + raise ValueError("mid-block failure") + assert settings.retries == 3 + + def test_never_swallows_the_body_exception(self) -> None: + settings = self.Settings() + # The KeyError reaches pytest.raises: __exit__ returns falsy. + with pytest.raises(KeyError), temporarily(settings, "retries", 0): + raise KeyError("must propagate") From 9acd2f19e007ef47361c8b2dd99f71d484aa98b2 Mon Sep 17 00:00:00 2001 From: SuperElectron Date: Thu, 27 Aug 2026 11:53:42 -0700 Subject: [PATCH 2/3] =?UTF-8?q?feat:=20v2=20modules=20=E2=80=94=20dependen?= =?UTF-8?q?cy=5Finjection,=20registry,=20repository?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate three modern units to the patterns-as-modules template: - dependency_injection: ReminderService with Protocol seams in pattern/; invoice_reminders mini-project with adapters + a real composition root - registry: typed Registry[T] with explicit duplicate/unknown-key policies; export_plugins mini-project with a separate-module plugin demonstrating the import-time caveat - repository: Invoices Protocol port + InMemoryInvoices fake + domain services in pattern/; invoice_ledger mini-project with a sqlite adapter and one contract test suite parametrized over both backends Each: docs/{fundamentals,implementation,examples}.md, isolated behavioral tests, legacy variant files removed. Co-Authored-By: Claude Fable 5 --- .../modern/dependency_injection/README.md | 43 +++------ .../modern/dependency_injection/__init__.py | 15 ++- .../dependency_injection/docs/examples.md | 36 +++++++ .../dependency_injection/docs/fundamentals.md | 69 +++++++++++++ .../docs/implementation.md | 74 ++++++++++++++ .../dependency_injection/examples/__init__.py | 1 + .../examples/invoice_reminders/__init__.py | 15 +++ .../examples/invoice_reminders/__main__.py | 22 +++++ .../examples/invoice_reminders/adapters.py | 30 ++++++ .../examples/invoice_reminders/app.py | 41 ++++++++ patterns/modern/dependency_injection/naive.py | 31 ------ .../dependency_injection/pattern/__init__.py | 11 +++ .../dependency_injection/pattern/service.py | 72 ++++++++++++++ .../modern/dependency_injection/pythonic.py | 96 ------------------- .../modern/dependency_injection/real_world.py | 32 ------- .../tests/test_dependency_injection.py | 62 ------------ .../tests/test_invoice_reminders.py | 60 ++++++++++++ .../tests/test_service.py | 63 ++++++++++++ patterns/modern/registry/README.md | 39 +++----- patterns/modern/registry/__init__.py | 9 +- patterns/modern/registry/docs/examples.md | 41 ++++++++ patterns/modern/registry/docs/fundamentals.md | 68 +++++++++++++ .../modern/registry/docs/implementation.md | 81 ++++++++++++++++ patterns/modern/registry/examples/__init__.py | 1 + .../examples/export_plugins/__init__.py | 16 ++++ .../examples/export_plugins/__main__.py | 24 +++++ .../examples/export_plugins/exporters.py | 32 +++++++ .../examples/export_plugins/markdown.py | 25 +++++ patterns/modern/registry/naive.py | 26 ----- patterns/modern/registry/pattern/__init__.py | 5 + patterns/modern/registry/pattern/registry.py | 60 ++++++++++++ patterns/modern/registry/pythonic.py | 55 ----------- patterns/modern/registry/real_world.py | 28 ------ .../registry/tests/test_export_plugins.py | 52 ++++++++++ .../modern/registry/tests/test_registry.py | 76 +++++++++------ patterns/modern/repository/README.md | 41 +++----- patterns/modern/repository/__init__.py | 15 ++- patterns/modern/repository/docs/examples.md | 46 +++++++++ .../modern/repository/docs/fundamentals.md | 64 +++++++++++++ .../modern/repository/docs/implementation.md | 76 +++++++++++++++ .../modern/repository/examples/__init__.py | 1 + .../examples/invoice_ledger/__init__.py | 8 ++ .../examples/invoice_ledger/__main__.py | 38 ++++++++ .../examples/invoice_ledger/sqlite_repo.py | 47 +++++++++ patterns/modern/repository/naive.py | 23 ----- .../modern/repository/pattern/__init__.py | 11 +++ patterns/modern/repository/pattern/ledger.py | 61 ++++++++++++ patterns/modern/repository/pythonic.py | 54 ----------- patterns/modern/repository/real_world.py | 36 ------- .../repository/tests/test_invoice_ledger.py | 72 ++++++++++++++ .../modern/repository/tests/test_ledger.py | 38 ++++++++ .../repository/tests/test_repository.py | 41 -------- 52 files changed, 1487 insertions(+), 596 deletions(-) create mode 100644 patterns/modern/dependency_injection/docs/examples.md create mode 100644 patterns/modern/dependency_injection/docs/fundamentals.md create mode 100644 patterns/modern/dependency_injection/docs/implementation.md create mode 100644 patterns/modern/dependency_injection/examples/__init__.py create mode 100644 patterns/modern/dependency_injection/examples/invoice_reminders/__init__.py create mode 100644 patterns/modern/dependency_injection/examples/invoice_reminders/__main__.py create mode 100644 patterns/modern/dependency_injection/examples/invoice_reminders/adapters.py create mode 100644 patterns/modern/dependency_injection/examples/invoice_reminders/app.py delete mode 100644 patterns/modern/dependency_injection/naive.py create mode 100644 patterns/modern/dependency_injection/pattern/__init__.py create mode 100644 patterns/modern/dependency_injection/pattern/service.py delete mode 100644 patterns/modern/dependency_injection/pythonic.py delete mode 100644 patterns/modern/dependency_injection/real_world.py delete mode 100644 patterns/modern/dependency_injection/tests/test_dependency_injection.py create mode 100644 patterns/modern/dependency_injection/tests/test_invoice_reminders.py create mode 100644 patterns/modern/dependency_injection/tests/test_service.py create mode 100644 patterns/modern/registry/docs/examples.md create mode 100644 patterns/modern/registry/docs/fundamentals.md create mode 100644 patterns/modern/registry/docs/implementation.md create mode 100644 patterns/modern/registry/examples/__init__.py create mode 100644 patterns/modern/registry/examples/export_plugins/__init__.py create mode 100644 patterns/modern/registry/examples/export_plugins/__main__.py create mode 100644 patterns/modern/registry/examples/export_plugins/exporters.py create mode 100644 patterns/modern/registry/examples/export_plugins/markdown.py delete mode 100644 patterns/modern/registry/naive.py create mode 100644 patterns/modern/registry/pattern/__init__.py create mode 100644 patterns/modern/registry/pattern/registry.py delete mode 100644 patterns/modern/registry/pythonic.py delete mode 100644 patterns/modern/registry/real_world.py create mode 100644 patterns/modern/registry/tests/test_export_plugins.py create mode 100644 patterns/modern/repository/docs/examples.md create mode 100644 patterns/modern/repository/docs/fundamentals.md create mode 100644 patterns/modern/repository/docs/implementation.md create mode 100644 patterns/modern/repository/examples/__init__.py create mode 100644 patterns/modern/repository/examples/invoice_ledger/__init__.py create mode 100644 patterns/modern/repository/examples/invoice_ledger/__main__.py create mode 100644 patterns/modern/repository/examples/invoice_ledger/sqlite_repo.py delete mode 100644 patterns/modern/repository/naive.py create mode 100644 patterns/modern/repository/pattern/__init__.py create mode 100644 patterns/modern/repository/pattern/ledger.py delete mode 100644 patterns/modern/repository/pythonic.py delete mode 100644 patterns/modern/repository/real_world.py create mode 100644 patterns/modern/repository/tests/test_invoice_ledger.py create mode 100644 patterns/modern/repository/tests/test_ledger.py delete mode 100644 patterns/modern/repository/tests/test_repository.py diff --git a/patterns/modern/dependency_injection/README.md b/patterns/modern/dependency_injection/README.md index 4932b4e..104bc6a 100644 --- a/patterns/modern/dependency_injection/README.md +++ b/patterns/modern/dependency_injection/README.md @@ -14,32 +14,17 @@ stdlib_sightings: [json.dumps cls=, sorted key=, unittest.mock] # Dependency Injection -## Problem - -A class that builds its own collaborators — its clock, its store, its HTTP -client — can only ever be tested with the real things. The hidden `new` is -the coupling. - -## Naive solution - -`naive.py` hard-wires `datetime.now` and a concrete store inside the class. -Watch the test problem appear: the greeting depends on the actual wall -clock. - -## Pythonic solution - -Pass the collaborators in. `pythonic.py` is an overdue-invoice reminder -service with three seams — the clock, the invoice source, the mail transport — -each a `Protocol` or callable with a production default. Tests hand in a -frozen date and a capturing mailbox and become fully deterministic. No -container, no framework, no decorators. - -## In the wild - -Every `key=` argument is DI (`sorted`, `min`, `max`); `json.dumps(cls=...)` -injects the encoder; `unittest.mock` exists to be injected. The stdlib does -DI by keyword argument, and so should you. - -## Verdict - -**Pythonic.** The default-argument seam is the pattern, entire. +Hand an object its collaborators — clock, storage, transport — instead of +letting it construct them, so tests can swap in fakes. **Verdict: pythonic** — +a keyword argument with a production default is the whole mechanism. + +| Where | What | +|---|---| +| [`pattern/`](pattern/) | The importable code: `ReminderService`, `InvoiceSource`, `MailTransport`, `Clock` | +| [`docs/`](docs/) | [Fundamentals](docs/fundamentals.md) · [Implementation guide](docs/implementation.md) · [External examples](docs/examples.md) | +| [`examples/invoice_reminders/`](examples/invoice_reminders/) | Mini-project: adapters + a real composition root over `pattern/` | +| [`tests/`](tests/) | Behavioral tests for the pattern and the mini-project | + +```bash +uv run python -m patterns.modern.dependency_injection.examples.invoice_reminders +``` diff --git a/patterns/modern/dependency_injection/__init__.py b/patterns/modern/dependency_injection/__init__.py index d6ac82a..e8ceb59 100644 --- a/patterns/modern/dependency_injection/__init__.py +++ b/patterns/modern/dependency_injection/__init__.py @@ -1 +1,14 @@ -"""Dependency Injection: pass collaborators in; a kwarg default is the mechanism.""" +"""Dependency Injection — public API. + +>>> from patterns.modern.dependency_injection import ReminderService +""" + +from patterns.modern.dependency_injection.pattern import ( + Clock, + Invoice, + InvoiceSource, + MailTransport, + ReminderService, +) + +__all__ = ["Clock", "Invoice", "InvoiceSource", "MailTransport", "ReminderService"] diff --git a/patterns/modern/dependency_injection/docs/examples.md b/patterns/modern/dependency_injection/docs/examples.md new file mode 100644 index 0000000..066ca3a --- /dev/null +++ b/patterns/modern/dependency_injection/docs/examples.md @@ -0,0 +1,36 @@ +# Dependency Injection — where it lives outside this repo + +Cited, real implementations to study (or point an agent at) when designing or +reviewing injection seams. + +## Python standard library + +- **`sorted(key=...)` / `min` / `max`.** The ordering policy is injected as a + callable — micro-DI so idiomatic nobody calls it a pattern. + [docs.python.org/3/library/functions.html#sorted](https://docs.python.org/3/library/functions.html#sorted) +- **`json.dumps(cls=...)`.** The encoder is a constructor-injected + collaborator with a production default (`JSONEncoder`). + [docs.python.org/3/library/json.html](https://docs.python.org/3/library/json.html) +- **`unittest.mock`.** The other half of the pattern: the fakes that exist to + be injected through the seams you left. + [docs.python.org/3/library/unittest.mock.html](https://docs.python.org/3/library/unittest.mock.html) + +## Major ecosystems + +- **pytest fixtures.** Injection driven by argument *name*: declaring a + parameter called `tmp_path` is asking the framework to construct and pass + one — a composition root run per test. + [docs.pytest.org/en/stable/how-to/fixtures.html](https://docs.pytest.org/en/stable/how-to/fixtures.html) +- **FastAPI `Depends`.** Request-scoped DI as a framework feature; the + declared dependency graph is resolved per call, with overrides for tests. + [fastapi.tiangolo.com/tutorial/dependencies/](https://fastapi.tiangolo.com/tutorial/dependencies/) +- **Fowler's taxonomy.** Constructor vs setter vs interface injection, and why + containers exist at all — the vocabulary the industry still uses. + [martinfowler.com/articles/injection.html](https://martinfowler.com/articles/injection.html) + +## What to notice across all of them + +None of the Python examples involve a container: the language's keyword +arguments and structural typing carry the whole pattern. When reviewing, +look for the two failure directions — a seam that is missing (tests patch +internals) and seams that are gratuitous (constructors as wiring diagrams). diff --git a/patterns/modern/dependency_injection/docs/fundamentals.md b/patterns/modern/dependency_injection/docs/fundamentals.md new file mode 100644 index 0000000..7f3fef2 --- /dev/null +++ b/patterns/modern/dependency_injection/docs/fundamentals.md @@ -0,0 +1,69 @@ +# Dependency Injection — fundamentals + +## Intent + +Hand an object its collaborators instead of letting it construct them, so the +things that vary — the clock, the storage, the transport — can be swapped +without touching the object. Named and taxonomized by Fowler in +[Inversion of Control Containers and the Dependency Injection pattern](https://martinfowler.com/articles/injection.html) (2004). + +## Participants + +| Role | Framework-era form | Python form | +|---|---|---| +| Service | A class resolved from a container | A plain class taking collaborators as constructor arguments — `ReminderService` in [`pattern/service.py`](../pattern/service.py) | +| Seam contract | An interface registered with the container | A `Protocol` (or a bare callable type like `Clock`) | +| Adapters | Container-managed beans | Any object with the right methods — no base class, no registration | +| Composition root | XML / container configuration | The one ordinary function that builds the object graph ([`examples/invoice_reminders/app.py`](../examples/invoice_reminders/app.py)) | + +## Mechanism + +1. The service names what it needs as constructor parameters, typed by + `Protocol` so `mypy` checks any adapter structurally. +2. The composition root — one function, at the edge of the program — builds + the real collaborators and passes them in. +3. Tests build the same service with fakes: a frozen clock, a capturing + mailbox. No patching, no framework, no container. +4. A collaborator with one nearly-universal right answer keeps a **production + default** (`today: Clock = date.today`) — the seam is invisible until the + day a test needs it. + +## The hard-wired form, and what Python absorbs + +There is no GoF chapter for DI; the classic form here is the code you write +*before* the pattern — the service that builds its own collaborators: + +```python +class GreetingService: + def __init__(self) -> None: + self.sent: list[str] = [] # the "store", welded in + + def greet(self, name: str) -> str: + hour = datetime.now().hour # the clock, welded in + prefix = "good morning" if hour < 12 else "good day" + ... +``` + +This class can only be tested against the real wall clock; the hidden +construction *is* the coupling. Java grew containers, XML wiring, and +`@Autowired` to break it. Python absorbs all of that: keyword arguments are +the injection mechanism, defaults are the production wiring, `Protocol` is +the interface. What survives of the pattern is one design habit — **name your +seams, and construct nothing you might need to swap**. + +## When to use it + +- A collaborator must differ between production and tests (clock, randomness, + network, storage, transport). +- The same logic must run against interchangeable backends. + +## When not to use it + +- The collaborator never varies — `math.sqrt` needs no seam. +- Everything is injected on principle and constructors become wiring diagrams; + inject at the boundary that varies, not everywhere. + +## Verdict: pythonic + +A keyword argument with a production default is the entire mechanism; the +stdlib itself does DI this way (`sorted(key=...)`, `json.dumps(cls=...)`). diff --git a/patterns/modern/dependency_injection/docs/implementation.md b/patterns/modern/dependency_injection/docs/implementation.md new file mode 100644 index 0000000..dde2e7e --- /dev/null +++ b/patterns/modern/dependency_injection/docs/implementation.md @@ -0,0 +1,74 @@ +# Dependency Injection — putting it into a system + +## The smell it fixes + +A test that cannot run without the real world: + +```python +def test_greeting() -> None: + service = GreetingService() + assert service.greet("ada").startswith("good morning") # fails after noon +``` + +Whatever the class constructs for itself — `datetime.now`, `sqlite3.connect`, +`requests.Session()` — its tests drag along. The fix is to move construction +out of the class and pass the result in. + +## Steps + +1. **Find the seams.** List what the class reaches out to that varies by + environment: time, randomness, storage, network, transport. Those — and + only those — become parameters. +2. **Type each seam as a `Protocol`** (or a callable alias like + `Clock = Callable[[], date]`). Structural typing means adapters need no + base class: any object with a matching `send` method is a `MailTransport`. +3. **Take collaborators in the constructor.** Keep a default argument where + one implementation is right nearly always (`today: Clock = date.today`); + require the argument where the choice deserves to be visible. +4. **Build one composition root.** A single ordinary function at the program's + edge constructs adapters and assembles the graph + (`build_service(...)` in the mini-project). If wiring appears in more than + one place, it has leaked. +5. **Write the tests the seams were made for.** Freeze the clock with a + lambda, capture mail in a list; assert on behavior, not on mocks' innards. + +```python +from patterns.modern.dependency_injection import ReminderService + +service = ReminderService(invoices=source, mail=outbox, today=lambda: date(2026, 8, 27)) +assert service.send_reminders() == ["INV-1"] +``` + +## Python idioms that keep it small + +- **A lambda is a fine adapter** for one-method seams; `Protocol` earns its + keep from two methods up. +- **`functools.partial`** turns a configured function into an injectable + collaborator without a class. +- **No container.** When the graph grows past what one composition-root + function can hold readably, split the function — reach for a DI framework + only when you can name what the function can no longer do. + +## Pitfalls + +- **Injecting everything.** A constructor with ten parameters is a wiring + diagram; inject what varies, construct the rest. +- **Patching instead of injecting.** `unittest.mock.patch` reaches through + module internals to do what a seam would have offered openly — needing it + is the signal the seam is missing. +- **Wiring scattered through the code.** Construction belongs at the + composition root; a class that builds one collaborator and injects two + others has both problems. +- **Seams without contracts.** An untyped `mail=None` parameter accepts + anything and promises nothing; the `Protocol` is what makes the fake and + the real thing provably interchangeable. + +## Worked example + +[`examples/invoice_reminders/`](../examples/invoice_reminders/) wires the +service at a real composition root, with the demo pinning the clock through +the same seam the tests use: + +```bash +uv run python -m patterns.modern.dependency_injection.examples.invoice_reminders +``` diff --git a/patterns/modern/dependency_injection/examples/__init__.py b/patterns/modern/dependency_injection/examples/__init__.py new file mode 100644 index 0000000..f03c5a4 --- /dev/null +++ b/patterns/modern/dependency_injection/examples/__init__.py @@ -0,0 +1 @@ +"""Mini-projects demonstrating Dependency Injection in practice.""" diff --git a/patterns/modern/dependency_injection/examples/invoice_reminders/__init__.py b/patterns/modern/dependency_injection/examples/invoice_reminders/__init__.py new file mode 100644 index 0000000..aab3ef7 --- /dev/null +++ b/patterns/modern/dependency_injection/examples/invoice_reminders/__init__.py @@ -0,0 +1,15 @@ +"""Overdue-invoice reminders wired at a real composition root. + +Run it: ``uv run python -m patterns.modern.dependency_injection.examples.invoice_reminders`` +""" + +from patterns.modern.dependency_injection.examples.invoice_reminders.adapters import ( + ConsoleMail, + InMemoryInvoices, +) +from patterns.modern.dependency_injection.examples.invoice_reminders.app import ( + build_service, + sample_invoices, +) + +__all__ = ["ConsoleMail", "InMemoryInvoices", "build_service", "sample_invoices"] diff --git a/patterns/modern/dependency_injection/examples/invoice_reminders/__main__.py b/patterns/modern/dependency_injection/examples/invoice_reminders/__main__.py new file mode 100644 index 0000000..f811d84 --- /dev/null +++ b/patterns/modern/dependency_injection/examples/invoice_reminders/__main__.py @@ -0,0 +1,22 @@ +"""Demo: a morning's reminder run with a pinned clock.""" + +from __future__ import annotations + +from datetime import date + +from patterns.modern.dependency_injection.examples.invoice_reminders.app import ( + build_service, + sample_invoices, +) + + +def main() -> None: + # The demo pins the clock at the composition root — the same seam a test + # uses, exercised for reproducibility instead of assertion. + service = build_service(sample_invoices(), today=lambda: date(2026, 8, 27)) + reminded = service.send_reminders(grace_days=3) + print(f"reminded: {reminded}") + + +if __name__ == "__main__": + main() diff --git a/patterns/modern/dependency_injection/examples/invoice_reminders/adapters.py b/patterns/modern/dependency_injection/examples/invoice_reminders/adapters.py new file mode 100644 index 0000000..7a5a805 --- /dev/null +++ b/patterns/modern/dependency_injection/examples/invoice_reminders/adapters.py @@ -0,0 +1,30 @@ +"""Concrete collaborators satisfying the pattern's seams. + +Nothing here is imported by ``ReminderService`` — the service knows only the +protocols. These are what the composition root chooses to plug in. +""" + +from __future__ import annotations + +from patterns.modern.dependency_injection.pattern import Invoice + + +class InMemoryInvoices: + """An invoice source backed by a list; production would wrap a database.""" + + def __init__(self, invoices: list[Invoice] | None = None) -> None: + self._invoices = list(invoices or []) + + def unpaid(self) -> list[Invoice]: + return list(self._invoices) + + +class ConsoleMail: + """A mail transport that prints; production would speak SMTP.""" + + def __init__(self) -> None: + self.sent_count = 0 + + def send(self, to: str, subject: str, body: str) -> None: + self.sent_count += 1 + print(f"MAIL to={to} subject={subject!r}") diff --git a/patterns/modern/dependency_injection/examples/invoice_reminders/app.py b/patterns/modern/dependency_injection/examples/invoice_reminders/app.py new file mode 100644 index 0000000..caaeff5 --- /dev/null +++ b/patterns/modern/dependency_injection/examples/invoice_reminders/app.py @@ -0,0 +1,41 @@ +"""The composition root: the one place that knows every concrete choice. + +The service stays ignorant of these decisions; swapping SMTP for console +mail, or a database for a fixture list, edits only this file. +""" + +from __future__ import annotations + +from datetime import date + +from patterns.modern.dependency_injection.examples.invoice_reminders.adapters import ( + ConsoleMail, + InMemoryInvoices, +) +from patterns.modern.dependency_injection.pattern import ( + Clock, + Invoice, + MailTransport, + ReminderService, +) + + +def sample_invoices() -> list[Invoice]: + return [ + Invoice("INV-1", "ada@example.com", 120_00, date(2026, 8, 1)), + Invoice("INV-2", "grace@example.com", 80_00, date(2026, 8, 25)), + Invoice("INV-3", "linus@example.com", 45_50, date(2026, 7, 15)), + ] + + +def build_service( + invoices: list[Invoice], + mail: MailTransport | None = None, + today: Clock = date.today, +) -> ReminderService: + """Assemble the production object graph; every seam overridable for tests.""" + return ReminderService( + invoices=InMemoryInvoices(invoices), + mail=mail if mail is not None else ConsoleMail(), + today=today, + ) diff --git a/patterns/modern/dependency_injection/naive.py b/patterns/modern/dependency_injection/naive.py deleted file mode 100644 index d4db417..0000000 --- a/patterns/modern/dependency_injection/naive.py +++ /dev/null @@ -1,31 +0,0 @@ -"""Hard-wired dependencies: the class news up its own collaborators. - -The cost is invisible until you try to test it -- there is no seam to -substitute the clock or the store. -""" - -from __future__ import annotations - -from datetime import datetime - - -class GreetingService: - def __init__(self) -> None: - self.sent: list[str] = [] # the "store", welded in - - def greet(self, name: str) -> str: - hour = datetime.now().hour # the clock, welded in - prefix = "good morning" if hour < 12 else "good day" - message = f"{prefix}, {name}" - self.sent.append(message) - return message - - -def main() -> None: - service = GreetingService() - print(service.greet("ada")) - print(f"stored: {service.sent}") - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/dependency_injection/pattern/__init__.py b/patterns/modern/dependency_injection/pattern/__init__.py new file mode 100644 index 0000000..e68885f --- /dev/null +++ b/patterns/modern/dependency_injection/pattern/__init__.py @@ -0,0 +1,11 @@ +"""Dependency Injection as importable, typed building blocks.""" + +from patterns.modern.dependency_injection.pattern.service import ( + Clock, + Invoice, + InvoiceSource, + MailTransport, + ReminderService, +) + +__all__ = ["Clock", "Invoice", "InvoiceSource", "MailTransport", "ReminderService"] diff --git a/patterns/modern/dependency_injection/pattern/service.py b/patterns/modern/dependency_injection/pattern/service.py new file mode 100644 index 0000000..25418b8 --- /dev/null +++ b/patterns/modern/dependency_injection/pattern/service.py @@ -0,0 +1,72 @@ +"""Constructor injection with ``Protocol`` seams. + +The service names the collaborators that must vary — the invoice source, the +mail transport, the clock — as constructor parameters typed by ``Protocol`` +(or a plain callable). The composition root passes real adapters; tests pass +fakes. Where one implementation is right nearly always, a default argument +makes injection invisible until the day it is needed (``today=date.today``). +""" + +from __future__ import annotations + +from collections.abc import Callable +from dataclasses import dataclass +from datetime import date +from typing import Protocol + +Clock = Callable[[], date] + + +@dataclass(frozen=True) +class Invoice: + """One unpaid invoice, as the reminder policy sees it.""" + + number: str + customer_email: str + amount_cents: int + due: date + + +class InvoiceSource(Protocol): + """Where unpaid invoices come from — a database in production.""" + + def unpaid(self) -> list[Invoice]: ... + + +class MailTransport(Protocol): + """How reminders leave the system — SMTP in production.""" + + def send(self, to: str, subject: str, body: str) -> None: ... + + +class ReminderService: + """Remind customers about overdue invoices. + + Every collaborator arrives through the constructor; the service builds + nothing it depends on. The clock keeps a production default because + ``date.today`` is right everywhere except in a test. + """ + + def __init__( + self, + invoices: InvoiceSource, + mail: MailTransport, + today: Clock = date.today, + ) -> None: + self._invoices = invoices + self._mail = mail + self._today = today + + def send_reminders(self, grace_days: int = 3) -> list[str]: + """Mail every invoice more than ``grace_days`` overdue; return its numbers.""" + reminded: list[str] = [] + for invoice in self._invoices.unpaid(): + overdue = (self._today() - invoice.due).days + if overdue > grace_days: + self._mail.send( + to=invoice.customer_email, + subject=f"Invoice {invoice.number} is {overdue} days overdue", + body=f"Please pay {invoice.amount_cents / 100:.2f}.", + ) + reminded.append(invoice.number) + return reminded diff --git a/patterns/modern/dependency_injection/pythonic.py b/patterns/modern/dependency_injection/pythonic.py deleted file mode 100644 index 893d18d..0000000 --- a/patterns/modern/dependency_injection/pythonic.py +++ /dev/null @@ -1,96 +0,0 @@ -"""Constructor injection with Protocol seams and production defaults. - -A real service shape: overdue-invoice reminders. Three collaborators that -must be swappable in tests -- the clock, the invoice source, the mail -transport -- each behind a seam. Production passes nothing; tests pass -fakes and get deterministic behavior. -""" - -from __future__ import annotations - -from collections.abc import Callable -from dataclasses import dataclass -from datetime import date -from typing import Protocol - - -@dataclass(frozen=True) -class Invoice: - number: str - customer_email: str - amount_cents: int - due: date - - -class InvoiceSource(Protocol): - def unpaid(self) -> list[Invoice]: ... - - -class MailTransport(Protocol): - def send(self, to: str, subject: str, body: str) -> None: ... - - -class InMemoryInvoices: - """Production would wrap a database; the seam doesn't care.""" - - def __init__(self, invoices: list[Invoice] | None = None) -> None: - self._invoices = invoices or [] - - def unpaid(self) -> list[Invoice]: - return list(self._invoices) - - -class ConsoleMail: - """The production default transport (stand-in for SMTP).""" - - def send(self, to: str, subject: str, body: str) -> None: - print(f"MAIL to={to} subject={subject!r}") - - -class ReminderService: - def __init__( - self, - invoices: InvoiceSource, - mail: MailTransport | None = None, - today: Callable[[], date] = date.today, - ) -> None: - self.invoices = invoices - self.mail: MailTransport = mail if mail is not None else ConsoleMail() - self.today = today - - def send_reminders(self, grace_days: int = 3) -> list[str]: - """Remind every invoice more than grace_days overdue; return numbers.""" - reminded: list[str] = [] - for invoice in self.invoices.unpaid(): - overdue = (self.today() - invoice.due).days - if overdue > grace_days: - self.mail.send( - to=invoice.customer_email, - subject=f"Invoice {invoice.number} is {overdue} days overdue", - body=f"Please pay {invoice.amount_cents / 100:.2f}.", - ) - reminded.append(invoice.number) - return reminded - - -def main() -> None: - source = InMemoryInvoices( - [ - Invoice("INV-1", "ada@example.com", 120_00, date(2026, 8, 1)), - Invoice("INV-2", "grace@example.com", 80_00, date(2026, 8, 25)), - ] - ) - # Test wiring: frozen clock, captured mail -- fully deterministic. - outbox: list[str] = [] - - class CapturingMail: - def send(self, to: str, subject: str, body: str) -> None: - outbox.append(f"{to}: {subject}") - - service = ReminderService(source, mail=CapturingMail(), today=lambda: date(2026, 8, 26)) - print(f"reminded: {service.send_reminders()}") - print(f"outbox: {outbox}") - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/dependency_injection/real_world.py b/patterns/modern/dependency_injection/real_world.py deleted file mode 100644 index 9d44e84..0000000 --- a/patterns/modern/dependency_injection/real_world.py +++ /dev/null @@ -1,32 +0,0 @@ -"""The stdlib does DI by keyword argument. - -``sorted(key=...)`` injects the ordering; ``json.dumps(cls=...)`` injects -the encoder. Same seam, same benefit. -""" - -from __future__ import annotations - -import json -from typing import Any - - -class UpperEncoder(json.JSONEncoder): - def encode(self, o: Any) -> str: - return super().encode(o).upper() - - -def sort_by_injected_policy(words: list[str]) -> list[str]: - return sorted(words, key=str.casefold) - - -def dump_with_injected_encoder(data: dict[str, str]) -> str: - return json.dumps(data, cls=UpperEncoder) - - -def main() -> None: - print(sort_by_injected_policy(["b", "A", "c"])) - print(dump_with_injected_encoder({"k": "v"})) - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/dependency_injection/tests/test_dependency_injection.py b/patterns/modern/dependency_injection/tests/test_dependency_injection.py deleted file mode 100644 index 34da291..0000000 --- a/patterns/modern/dependency_injection/tests/test_dependency_injection.py +++ /dev/null @@ -1,62 +0,0 @@ -"""Behavioral tests for all three dependency-injection variants.""" - -from datetime import date - -from patterns.modern.dependency_injection import naive, pythonic, real_world - - -class TestNaive: - def test_works_but_depends_on_the_real_clock(self) -> None: - message = naive.GreetingService().greet("ada") - assert message.endswith(", ada") - assert message.startswith(("good morning", "good day")) - - -class CapturingMail: - def __init__(self) -> None: - self.outbox: list[tuple[str, str]] = [] - - def send(self, to: str, subject: str, body: str) -> None: - self.outbox.append((to, subject)) - - -def _service(mail: CapturingMail, today: date) -> pythonic.ReminderService: - source = pythonic.InMemoryInvoices( - [ - pythonic.Invoice("INV-1", "ada@example.com", 120_00, date(2026, 8, 1)), - pythonic.Invoice("INV-2", "grace@example.com", 80_00, date(2026, 8, 25)), - ] - ) - return pythonic.ReminderService(source, mail=mail, today=lambda: today) - - -class TestPythonic: - def test_frozen_clock_makes_reminders_deterministic(self) -> None: - mail = CapturingMail() - reminded = _service(mail, date(2026, 8, 26)).send_reminders(grace_days=3) - assert reminded == ["INV-1"] # 25 days overdue; INV-2 inside grace - assert mail.outbox == [("ada@example.com", "Invoice INV-1 is 25 days overdue")] - - def test_grace_period_is_respected(self) -> None: - mail = CapturingMail() - reminded = _service(mail, date(2026, 8, 26)).send_reminders(grace_days=30) - assert reminded == [] and mail.outbox == [] - - def test_every_seam_is_swappable(self) -> None: - # A different source, transport, and clock -- no monkeypatching anywhere. - source = pythonic.InMemoryInvoices([]) - mail = CapturingMail() - service = pythonic.ReminderService(source, mail=mail, today=lambda: date(2026, 1, 1)) - assert service.send_reminders() == [] - - def test_production_defaults_exist(self) -> None: - service = pythonic.ReminderService(pythonic.InMemoryInvoices([])) - assert isinstance(service.mail, pythonic.ConsoleMail) - - -class TestRealWorld: - def test_injected_sort_policy(self) -> None: - assert real_world.sort_by_injected_policy(["b", "A", "c"]) == ["A", "b", "c"] - - def test_injected_encoder(self) -> None: - assert real_world.dump_with_injected_encoder({"k": "v"}) == '{"K": "V"}' diff --git a/patterns/modern/dependency_injection/tests/test_invoice_reminders.py b/patterns/modern/dependency_injection/tests/test_invoice_reminders.py new file mode 100644 index 0000000..7a1b18d --- /dev/null +++ b/patterns/modern/dependency_injection/tests/test_invoice_reminders.py @@ -0,0 +1,60 @@ +"""Behavioral tests for the invoice-reminders mini-project.""" + +from __future__ import annotations + +from datetime import date + +import pytest + +from patterns.modern.dependency_injection.examples.invoice_reminders import ( + ConsoleMail, + InMemoryInvoices, + build_service, + sample_invoices, +) +from patterns.modern.dependency_injection.examples.invoice_reminders.__main__ import main +from patterns.modern.dependency_injection.pattern import Invoice + + +class TestAdapters: + def test_in_memory_source_returns_a_copy(self) -> None: + inv = Invoice("INV-1", "ada@example.com", 100, date(2026, 8, 1)) + source = InMemoryInvoices([inv]) + source.unpaid().clear() + assert source.unpaid() == [inv] + + def test_console_mail_prints_and_counts(self, capsys: pytest.CaptureFixture[str]) -> None: + mail = ConsoleMail() + mail.send("ada@example.com", "Invoice INV-1 is 5 days overdue", "Please pay.") + assert mail.sent_count == 1 + assert "ada@example.com" in capsys.readouterr().out + + +class TestCompositionRoot: + def test_build_service_defaults_to_production_adapters( + self, capsys: pytest.CaptureFixture[str] + ) -> None: + service = build_service(sample_invoices(), today=lambda: date(2026, 8, 27)) + reminded = service.send_reminders() + assert reminded == ["INV-1", "INV-3"] # INV-2 is inside the grace period + assert capsys.readouterr().out.count("MAIL") == 2 + + def test_every_seam_is_overridable_from_the_root(self) -> None: + captured: list[str] = [] + + class Outbox: + def send(self, to: str, subject: str, body: str) -> None: + captured.append(to) + + service = build_service(sample_invoices(), mail=Outbox(), today=lambda: date(2026, 8, 27)) + service.send_reminders() + assert captured == ["ada@example.com", "linus@example.com"] + + +class TestDemo: + def test_main_reports_the_pinned_day_reminders( + self, capsys: pytest.CaptureFixture[str] + ) -> None: + main() + out = capsys.readouterr().out + assert "reminded: ['INV-1', 'INV-3']" in out diff --git a/patterns/modern/dependency_injection/tests/test_service.py b/patterns/modern/dependency_injection/tests/test_service.py new file mode 100644 index 0000000..2289762 --- /dev/null +++ b/patterns/modern/dependency_injection/tests/test_service.py @@ -0,0 +1,63 @@ +"""Behavioral tests for the pattern's service — every seam exercised.""" + +from __future__ import annotations + +from datetime import date + +from patterns.modern.dependency_injection import Invoice, ReminderService + + +class FixedInvoices: + def __init__(self, invoices: list[Invoice]) -> None: + self._invoices = invoices + + def unpaid(self) -> list[Invoice]: + return list(self._invoices) + + +class CapturingMail: + def __init__(self) -> None: + self.outbox: list[tuple[str, str]] = [] + + def send(self, to: str, subject: str, body: str) -> None: + self.outbox.append((to, subject)) + + +def invoice(number: str, due: date, email: str = "ada@example.com") -> Invoice: + return Invoice(number, email, 100_00, due) + + +class TestReminderPolicy: + today = date(2026, 8, 27) + + def service(self, invoices: list[Invoice], mail: CapturingMail) -> ReminderService: + return ReminderService(FixedInvoices(invoices), mail, today=lambda: self.today) + + def test_overdue_past_grace_is_reminded(self) -> None: + mail = CapturingMail() + reminded = self.service([invoice("INV-1", date(2026, 8, 1))], mail).send_reminders() + assert reminded == ["INV-1"] + assert mail.outbox == [("ada@example.com", "Invoice INV-1 is 26 days overdue")] + + def test_exactly_at_grace_is_not_reminded(self) -> None: + mail = CapturingMail() + at_grace = invoice("INV-2", date(2026, 8, 24)) # 3 days overdue == grace + assert self.service([at_grace], mail).send_reminders(grace_days=3) == [] + assert mail.outbox == [] + + def test_not_yet_due_is_not_reminded(self) -> None: + mail = CapturingMail() + future = invoice("INV-3", date(2026, 9, 1)) + assert self.service([future], mail).send_reminders() == [] + + def test_the_clock_seam_controls_the_outcome(self) -> None: + """The same invoice flips from quiet to reminded by injecting a later day.""" + inv = invoice("INV-4", date(2026, 8, 25)) + + def at(day: date) -> ReminderService: + return ReminderService(FixedInvoices([inv]), CapturingMail(), today=lambda: day) + + early = at(date(2026, 8, 26)) + late = at(date(2026, 9, 26)) + assert early.send_reminders() == [] + assert late.send_reminders() == ["INV-4"] diff --git a/patterns/modern/registry/README.md b/patterns/modern/registry/README.md index 9879f37..319da0d 100644 --- a/patterns/modern/registry/README.md +++ b/patterns/modern/registry/README.md @@ -14,28 +14,17 @@ stdlib_sightings: [codecs.register, functools.singledispatch, atexit.register] # Registry -## Problem - -An exporter supports "csv", "json", "xml"… and every new format edits the -same `if/elif` ladder. The dispatcher has become a bottleneck every plugin -must patch. - -## Naive solution - -`naive.py` is that ladder: closed for extension, growing forever. - -## Pythonic solution - -A dict from name to callable, filled by a `@register("csv")` decorator — -defining a handler *is* registering it. Dispatch is a lookup; the unknown-key -policy lives in exactly one place. - -## In the wild - -`codecs.register` is a full plugin registry (every `.encode("rot13")` is a -lookup); `functools.singledispatch` is a registry keyed by type; -`atexit.register` collects callables to run at shutdown. - -## Verdict - -**Pythonic.** The standard cure for if/elif dispatch. +Implementations announce themselves by name; dispatch is a lookup, and adding +a case is writing one new function. **Verdict: pythonic** — the standard cure +for `if/elif` dispatch. + +| Where | What | +|---|---| +| [`pattern/`](pattern/) | The importable code: `Registry`, `UnknownKeyError` | +| [`docs/`](docs/) | [Fundamentals](docs/fundamentals.md) · [Implementation guide](docs/implementation.md) · [External examples](docs/examples.md) | +| [`examples/export_plugins/`](examples/export_plugins/) | Mini-project: self-registering exporters, one in a separate plugin module | +| [`tests/`](tests/) | Behavioral tests for the pattern and the mini-project | + +```bash +uv run python -m patterns.modern.registry.examples.export_plugins +``` diff --git a/patterns/modern/registry/__init__.py b/patterns/modern/registry/__init__.py index c1480b6..51eeae7 100644 --- a/patterns/modern/registry/__init__.py +++ b/patterns/modern/registry/__init__.py @@ -1 +1,8 @@ -"""Registry: implementations announce themselves; dispatch is a lookup.""" +"""Registry — public API. + +>>> from patterns.modern.registry import Registry +""" + +from patterns.modern.registry.pattern import Registry, UnknownKeyError + +__all__ = ["Registry", "UnknownKeyError"] diff --git a/patterns/modern/registry/docs/examples.md b/patterns/modern/registry/docs/examples.md new file mode 100644 index 0000000..67a151a --- /dev/null +++ b/patterns/modern/registry/docs/examples.md @@ -0,0 +1,41 @@ +# Registry — where it lives outside this repo + +Cited, real implementations to study (or point an agent at) when designing or +reviewing registry-shaped code. + +## Python standard library + +- **`codecs.register` / `codecs.lookup`.** The encodings machinery is a full + plugin registry: every `"text".encode(name)` is a lookup, and registered + search functions can serve entirely new names. + [docs.python.org/3/library/codecs.html](https://docs.python.org/3/library/codecs.html) +- **`functools.singledispatch`.** A registry keyed by *type* instead of name, + with the same decorator registration surface and MRO-aware lookup. + [docs.python.org/3/library/functools.html#functools.singledispatch](https://docs.python.org/3/library/functools.html#functools.singledispatch) +- **`atexit.register`.** A registry whose "dispatch" is the interpreter + shutting down — registration as decorator, in the stdlib since forever. + [docs.python.org/3/library/atexit.html](https://docs.python.org/3/library/atexit.html) + +## Major ecosystems + +- **Flask route decorators.** `@app.route("/users")` fills the URL map — a + registry populated at import time, which is why a views module that never + gets imported serves 404s (the caveat, in production form). + [flask.palletsprojects.com](https://flask.palletsprojects.com/) +- **Django `admin.site.register`.** The explicit-call flavor: the admin is a + registry of model → options, filled in each app's `admin.py` — a module + Django deliberately auto-imports, solving the import-time problem by + convention. + [docs.djangoproject.com/en/stable/ref/contrib/admin/](https://docs.djangoproject.com/en/stable/ref/contrib/admin/) +- **setuptools entry points.** Registration moved out of code into package + metadata, so plugins in *other distributions* are discoverable without any + import — the industrial-strength answer to "a plugin nobody imports". + [packaging.python.org/en/latest/specifications/entry-points/](https://packaging.python.org/en/latest/specifications/entry-points/) + +## What to notice across all of them + +Each one has an explicit answer to the two policy questions: unknown names +(`LookupError` from `codecs`, 404 from Flask) and registration time (import +side effects, auto-imported conventions, or metadata). When reviewing +registry code, find both answers; if either is implicit, that's the bug +waiting. diff --git a/patterns/modern/registry/docs/fundamentals.md b/patterns/modern/registry/docs/fundamentals.md new file mode 100644 index 0000000..8b633c8 --- /dev/null +++ b/patterns/modern/registry/docs/fundamentals.md @@ -0,0 +1,68 @@ +# Registry — fundamentals + +## Intent + +Let implementations announce themselves by name so dispatch becomes a lookup +instead of an `if/elif` ladder — and adding a case means writing one new +function, not editing the dispatcher. + +## Participants + +| Role | Ladder form | Python form | +|---|---|---| +| Dispatcher | One growing `if/elif` function | A mapping — `Registry` in [`pattern/registry.py`](../pattern/registry.py) | +| Cases | Arms of the ladder | Independent callables, possibly in other modules | +| Registration | Editing the ladder | A `@registry.register("name")` decorator at definition site | +| Lookup policy | The trailing `else`, per call site | `registry.get(name)` — one place, one decision | + +## Mechanism + +1. A module owns a `Registry` instance typed by what it stores + (`Registry[Exporter]`). +2. Each implementation registers itself where it is defined — the decorator + makes *defining* a handler and *announcing* it the same act. +3. Dispatch asks the registry by name. Unknown names raise `UnknownKeyError` + listing what is known; duplicate registrations are an error unless + explicitly replaced. +4. Because registration runs at import time, a plugin exists only once its + module has been imported — the pattern's one genuine sharp edge. + +## The classic form, and what Python absorbs + +The pre-pattern shape is the ladder every plugin must patch: + +```python +def export(rows, fmt): + if fmt == "csv": + ... # arm 1 + elif fmt == "keyvalue": + ... # arm 2 + else: + raise ValueError(...) # the unknown-name policy, re-decided per ladder +``` + +Closed for extension: format N+1 edits this function, and every parallel +ladder (validate, describe, …) drifts out of sync. Python absorbs the +machinery a plugin framework would add — a dict is the registry, a decorator +is the registration API, first-class functions are the plugins. What survives +is two policies the folk pattern leaves implicit: **what happens on an +unknown name**, and **what happens on a duplicate**. + +## When to use it + +- Open-ended families keyed by a value: exporters by format, handlers by + event name, commands by verb. +- Plugins live in modules the dispatcher must not know about. + +## When not to use it + +- The key is a *type* — `functools.singledispatch` is that registry, built in. +- The set of cases is small, closed, and local — a literal dict (or `match`) + says so more plainly. +- Cross-package plugins — reach for entry points, which solve the import-time + problem the plain registry cannot. + +## Verdict: pythonic + +The standard cure for `if/elif` dispatch; the stdlib itself ships registries +(`codecs.register`, `atexit.register`, `singledispatch.register`). diff --git a/patterns/modern/registry/docs/implementation.md b/patterns/modern/registry/docs/implementation.md new file mode 100644 index 0000000..8d4a481 --- /dev/null +++ b/patterns/modern/registry/docs/implementation.md @@ -0,0 +1,81 @@ +# Registry — putting it into a system + +## The smell it fixes + +A dispatcher that every new case must edit: + +```python +def export(rows, fmt): + if fmt == "csv": + ... + elif fmt == "json": + ... + elif fmt == "xml": + ... # this week's edit + else: + raise ValueError(...) +``` + +The ladder couples every format to one function, and the unknown-name policy +gets re-decided (differently) at every ladder in the codebase. + +## Steps + +1. **Name the contract.** A type alias for what the registry stores + (`Exporter = Callable[[Rows], str]`) turns "any function" into a checkable + promise. +2. **Create one registry instance in the module that owns dispatch** — + `EXPORTERS: Registry[Exporter] = Registry(kind="format")`. The `kind` + string buys readable errors for free. +3. **Convert each ladder arm into a decorated function.** Its condition + becomes its name: `@EXPORTERS.register("csv")`. +4. **Route all dispatch through one lookup.** `EXPORTERS.get(fmt)(rows)` — + the unknown-name policy now lives in the registry, once. +5. **Guarantee plugins are imported.** Registration is an import-time side + effect, so some module must import each plugin. The package `__init__` is + the honest place — with a comment saying the import is load-bearing. + +```python +from patterns.modern.registry import Registry + +EXPORTERS: Registry[Exporter] = Registry(kind="format") + + +@EXPORTERS.register("csv") +def to_csv(rows: Rows) -> str: ... +``` + +## Python idioms that keep it small + +- **The decorator returns its target unchanged**, so a registered function is + still an ordinary, individually-testable function. +- **Keep registries module-level and typed.** A registry passed around as a + parameter is usually dependency injection wearing the wrong hat. +- **For type-keyed dispatch, don't rebuild this** — `functools.singledispatch` + already is the registry, with MRO-aware lookup. + +## Pitfalls + +- **The plugin nobody imports.** The registry only knows what has run. + Symptom: works in the app (which imports everything), fails in a test that + imports one module. Fix: import plugins in the package `__init__`, or use + entry points for cross-package discovery. +- **Silent duplicate registration.** With a bare dict, two plugins claiming + `"csv"` is a last-import-wins race. `Registry` makes it an error; + `replace=True` makes an intentional override visible in the diff. +- **Unknown-name policy at call sites.** If callers wrap `get` in their own + `try/except KeyError` with their own fallbacks, the policy has leaked back + out — decide it once. +- **Registration with heavier side effects.** The decorator should record the + entry, nothing more; a plugin that opens connections at import time turns + every importer into an integration test. + +## Worked example + +[`examples/export_plugins/`](../examples/export_plugins/) applies every step, +including a plugin in its own module whose `__init__` import is the +documented fix for the import-time caveat: + +```bash +uv run python -m patterns.modern.registry.examples.export_plugins +``` diff --git a/patterns/modern/registry/examples/__init__.py b/patterns/modern/registry/examples/__init__.py new file mode 100644 index 0000000..bd1b838 --- /dev/null +++ b/patterns/modern/registry/examples/__init__.py @@ -0,0 +1 @@ +"""Mini-projects demonstrating the Registry in practice.""" diff --git a/patterns/modern/registry/examples/export_plugins/__init__.py b/patterns/modern/registry/examples/export_plugins/__init__.py new file mode 100644 index 0000000..fcbdedb --- /dev/null +++ b/patterns/modern/registry/examples/export_plugins/__init__.py @@ -0,0 +1,16 @@ +"""Self-registering exporters over a shared ``Registry``. + +Run it: ``uv run python -m patterns.modern.registry.examples.export_plugins`` + +The import below is load-bearing: ``markdown`` registers itself at import +time, and a plugin nobody imports doesn't exist (the pattern's sharpest +caveat). This package's ``__init__`` is where that import is guaranteed. +""" + +from patterns.modern.registry.examples.export_plugins import markdown as markdown +from patterns.modern.registry.examples.export_plugins.exporters import ( + EXPORTERS, + export, +) + +__all__ = ["EXPORTERS", "export", "markdown"] diff --git a/patterns/modern/registry/examples/export_plugins/__main__.py b/patterns/modern/registry/examples/export_plugins/__main__.py new file mode 100644 index 0000000..e69d680 --- /dev/null +++ b/patterns/modern/registry/examples/export_plugins/__main__.py @@ -0,0 +1,24 @@ +"""Demo: one dataset through every registered exporter.""" + +from __future__ import annotations + +from patterns.modern.registry.examples.export_plugins import EXPORTERS, export +from patterns.modern.registry.pattern import UnknownKeyError + + +def main() -> None: + rows = [ + {"name": "ada", "role": "eng"}, + {"name": "grace", "role": "ops"}, + ] + for fmt in EXPORTERS.names(): + print(f"--- {fmt} ---") + print(export(rows, fmt)) + try: + export(rows, "xml") + except UnknownKeyError as exc: + print(f"--- unknown format ---\n{exc}") + + +if __name__ == "__main__": + main() diff --git a/patterns/modern/registry/examples/export_plugins/exporters.py b/patterns/modern/registry/examples/export_plugins/exporters.py new file mode 100644 index 0000000..e181205 --- /dev/null +++ b/patterns/modern/registry/examples/export_plugins/exporters.py @@ -0,0 +1,32 @@ +"""The registry, the built-in exporters, and the one dispatch function.""" + +from __future__ import annotations + +import json +from collections.abc import Callable + +from patterns.modern.registry.pattern import Registry + +Rows = list[dict[str, str]] +Exporter = Callable[[Rows], str] + +EXPORTERS: Registry[Exporter] = Registry(kind="format") + + +@EXPORTERS.register("csv") +def to_csv(rows: Rows) -> str: + if not rows: + return "" + header = ",".join(rows[0]) + body = "\n".join(",".join(row.values()) for row in rows) + return f"{header}\n{body}" + + +@EXPORTERS.register("json") +def to_json(rows: Rows) -> str: + return json.dumps(rows, indent=2) + + +def export(rows: Rows, fmt: str) -> str: + """Dispatch is a lookup; the unknown-format policy lives in the registry, once.""" + return EXPORTERS.get(fmt)(rows) diff --git a/patterns/modern/registry/examples/export_plugins/markdown.py b/patterns/modern/registry/examples/export_plugins/markdown.py new file mode 100644 index 0000000..0406109 --- /dev/null +++ b/patterns/modern/registry/examples/export_plugins/markdown.py @@ -0,0 +1,25 @@ +"""A plugin in its own module — the import-time caveat, made concrete. + +Nothing imports this module for its names; it is imported (by the package +``__init__``) purely so the ``@EXPORTERS.register`` below runs. Comment out +that import and ``"markdown"`` vanishes from the registry without any other +code changing — which is exactly why real plugin systems pair registries +with entry points or explicit plugin loading. +""" + +from __future__ import annotations + +from patterns.modern.registry.examples.export_plugins.exporters import EXPORTERS, Rows + + +@EXPORTERS.register("markdown") +def to_markdown(rows: Rows) -> str: + if not rows: + return "" + headers = list(rows[0]) + lines = [ + "| " + " | ".join(headers) + " |", + "| " + " | ".join("---" for _ in headers) + " |", + ] + lines += ["| " + " | ".join(row[h] for h in headers) + " |" for row in rows] + return "\n".join(lines) diff --git a/patterns/modern/registry/naive.py b/patterns/modern/registry/naive.py deleted file mode 100644 index 7095db2..0000000 --- a/patterns/modern/registry/naive.py +++ /dev/null @@ -1,26 +0,0 @@ -"""Dispatch as an if/elif ladder: every new format edits this function.""" - -from __future__ import annotations - - -def export(rows: list[dict[str, str]], fmt: str) -> str: - if fmt == "csv": - if not rows: - return "" - header = ",".join(rows[0]) - body = "\n".join(",".join(row.values()) for row in rows) - return f"{header}\n{body}" - elif fmt == "keyvalue": - return "\n".join(f"{k}={v}" for row in rows for k, v in row.items()) - else: - raise ValueError(f"unknown format: {fmt}") - - -def main() -> None: - rows = [{"name": "ada", "role": "eng"}] - print(export(rows, "csv")) - print(export(rows, "keyvalue")) - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/registry/pattern/__init__.py b/patterns/modern/registry/pattern/__init__.py new file mode 100644 index 0000000..da2e9ec --- /dev/null +++ b/patterns/modern/registry/pattern/__init__.py @@ -0,0 +1,5 @@ +"""The Registry pattern, importable as library code.""" + +from patterns.modern.registry.pattern.registry import Registry, UnknownKeyError + +__all__ = ["Registry", "UnknownKeyError"] diff --git a/patterns/modern/registry/pattern/registry.py b/patterns/modern/registry/pattern/registry.py new file mode 100644 index 0000000..aaf56d9 --- /dev/null +++ b/patterns/modern/registry/pattern/registry.py @@ -0,0 +1,60 @@ +"""A typed plugin registry: a dict, a decorator, and one lookup policy. + +``Registry`` maps names to implementations. Defining a handler registers it +(``@registry.register("csv")``); dispatch is ``registry.get(name)``. The two +policies the folk pattern leaves implicit are explicit here: duplicate names +are an error unless ``replace=True``, and unknown names raise +``UnknownKeyError`` naming what *is* registered. +""" + +from __future__ import annotations + +from collections.abc import Callable +from typing import Generic, TypeVar + +T = TypeVar("T") + + +class UnknownKeyError(LookupError): + """The name is not registered; the message lists the names that are.""" + + +class Registry(Generic[T]): + """A name-to-implementation mapping filled by decorator.""" + + def __init__(self, kind: str = "entry") -> None: + self._kind = kind # names the registry's contents in error messages + self._entries: dict[str, T] = {} + + def register(self, name: str, *, replace: bool = False) -> Callable[[T], T]: + """Return a decorator that registers its target under ``name``. + + Duplicate names raise ``ValueError`` — a silent overwrite is how two + plugins fight over a name without anyone noticing — unless the caller + says ``replace=True``. + """ + + def decorator(entry: T) -> T: + if name in self._entries and not replace: + raise ValueError(f"{self._kind} {name!r} is already registered") + self._entries[name] = entry + return entry + + return decorator + + def get(self, name: str) -> T: + """Look up one entry; unknown names fail loudly, listing known ones.""" + try: + return self._entries[name] + except KeyError: + known = ", ".join(sorted(self._entries)) or "" + raise UnknownKeyError(f"unknown {self._kind} {name!r} (known: {known})") from None + + def names(self) -> tuple[str, ...]: + return tuple(sorted(self._entries)) + + def __contains__(self, name: object) -> bool: + return name in self._entries + + def __len__(self) -> int: + return len(self._entries) diff --git a/patterns/modern/registry/pythonic.py b/patterns/modern/registry/pythonic.py deleted file mode 100644 index 9021658..0000000 --- a/patterns/modern/registry/pythonic.py +++ /dev/null @@ -1,55 +0,0 @@ -"""The decorator-filled registry: defining a handler registers it. - -New formats are new functions -- possibly in other modules -- and the -dispatcher never changes again. -""" - -from __future__ import annotations - -from collections.abc import Callable - -Exporter = Callable[[list[dict[str, str]]], str] - -EXPORTERS: dict[str, Exporter] = {} - - -def register(name: str) -> Callable[[Exporter], Exporter]: - def decorator(func: Exporter) -> Exporter: - EXPORTERS[name] = func - return func - - return decorator - - -@register("csv") -def to_csv(rows: list[dict[str, str]]) -> str: - if not rows: - return "" - header = ",".join(rows[0]) - body = "\n".join(",".join(row.values()) for row in rows) - return f"{header}\n{body}" - - -@register("keyvalue") -def to_keyvalue(rows: list[dict[str, str]]) -> str: - return "\n".join(f"{k}={v}" for row in rows for k, v in row.items()) - - -def export(rows: list[dict[str, str]], fmt: str) -> str: - """Dispatch is a lookup; the unknown-key policy lives here, once.""" - try: - exporter = EXPORTERS[fmt] - except KeyError: - known = ", ".join(sorted(EXPORTERS)) - raise ValueError(f"unknown format {fmt!r} (known: {known})") from None - return exporter(rows) - - -def main() -> None: - rows = [{"name": "ada", "role": "eng"}] - print(export(rows, "csv")) - print(export(rows, "keyvalue")) - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/registry/real_world.py b/patterns/modern/registry/real_world.py deleted file mode 100644 index 705f901..0000000 --- a/patterns/modern/registry/real_world.py +++ /dev/null @@ -1,28 +0,0 @@ -"""``codecs``: the stdlib's plugin registry in daily use. - -Every str.encode(name) is a registry lookup; codecs.register() adds a -search function that can serve entirely new names. -""" - -from __future__ import annotations - -import codecs - - -def rot13(text: str) -> str: - """'rot13' resolves through the codec registry.""" - return codecs.encode(text, "rot13") - - -def lookup_is_the_registry(name: str) -> str: - """Ask the registry directly for a codec entry.""" - return codecs.lookup(name).name - - -def main() -> None: - print(rot13("gura fur fnvq")) - print(f"'UTF8' resolves to: {lookup_is_the_registry('UTF8')}") - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/registry/tests/test_export_plugins.py b/patterns/modern/registry/tests/test_export_plugins.py new file mode 100644 index 0000000..64d009a --- /dev/null +++ b/patterns/modern/registry/tests/test_export_plugins.py @@ -0,0 +1,52 @@ +"""Behavioral tests for the export-plugins mini-project.""" + +from __future__ import annotations + +import json + +import pytest + +from patterns.modern.registry.examples.export_plugins import EXPORTERS, export +from patterns.modern.registry.examples.export_plugins.__main__ import main +from patterns.modern.registry.pattern import UnknownKeyError + +ROWS = [{"name": "ada", "role": "eng"}, {"name": "grace", "role": "ops"}] + + +class TestExporters: + def test_csv_round_trips_headers_and_rows(self) -> None: + assert export(ROWS, "csv") == "name,role\nada,eng\ngrace,ops" + + def test_json_is_real_json(self) -> None: + assert json.loads(export(ROWS, "json")) == ROWS + + def test_markdown_renders_a_table(self) -> None: + out = export(ROWS, "markdown") + assert out.splitlines()[0] == "| name | role |" + assert "| ada | eng |" in out + + def test_empty_input_is_not_an_error(self) -> None: + assert export([], "csv") == "" + assert export([], "markdown") == "" + + +class TestPluginDiscovery: + def test_the_separate_module_plugin_registered_via_the_package_import(self) -> None: + # markdown.py is imported only by the package __init__ — its presence + # here is the import-time caveat's fix, working. + assert EXPORTERS.names() == ("csv", "json", "markdown") + + def test_unknown_format_policy_lives_in_one_place(self) -> None: + with pytest.raises(UnknownKeyError, match="unknown format 'xml'"): + export(ROWS, "xml") + + +class TestDemo: + def test_main_exports_every_format_and_shows_the_unknown_policy( + self, capsys: pytest.CaptureFixture[str] + ) -> None: + main() + out = capsys.readouterr().out + assert "--- csv ---" in out + assert "--- markdown ---" in out + assert "unknown format 'xml'" in out diff --git a/patterns/modern/registry/tests/test_registry.py b/patterns/modern/registry/tests/test_registry.py index 18fa282..7f74174 100644 --- a/patterns/modern/registry/tests/test_registry.py +++ b/patterns/modern/registry/tests/test_registry.py @@ -1,42 +1,62 @@ -"""Behavioral tests for all three registry variants.""" +"""Behavioral tests for the pattern's ``Registry``.""" + +from __future__ import annotations + +from collections.abc import Callable import pytest -from patterns.modern.registry import naive, pythonic, real_world +from patterns.modern.registry import Registry, UnknownKeyError + +Handler = Callable[[str], str] + -ROWS = [{"name": "ada", "role": "eng"}] +def make_registry() -> Registry[Handler]: + return Registry(kind="handler") -class TestNaive: - def test_ladder_dispatch_works(self) -> None: - assert naive.export(ROWS, "csv") == "name,role\nada,eng" +class TestRegistration: + def test_the_decorator_registers_and_returns_the_function_unchanged(self) -> None: + registry = make_registry() - def test_unknown_format(self) -> None: - with pytest.raises(ValueError, match="unknown format"): - naive.export(ROWS, "yaml") + @registry.register("upper") + def shout(text: str) -> str: + return text.upper() + assert registry.get("upper") is shout + assert shout("hi") == "HI" # still an ordinary function -class TestPythonic: - def test_registered_handlers_dispatch_by_name(self) -> None: - assert pythonic.export(ROWS, "csv") == "name,role\nada,eng" - assert pythonic.export(ROWS, "keyvalue") == "name=ada\nrole=eng" + def test_duplicate_names_are_an_error(self) -> None: + registry = make_registry() + registry.register("upper")(str.upper) + with pytest.raises(ValueError, match="handler 'upper' is already registered"): + registry.register("upper")(str.lower) + assert registry.get("upper")("hi") == "HI" # original untouched - def test_new_handler_registers_without_touching_the_dispatcher(self) -> None: - @pythonic.register("upper") - def to_upper(rows: list[dict[str, str]]) -> str: - return " ".join(v.upper() for row in rows for v in row.values()) + def test_replace_makes_an_override_explicit(self) -> None: + registry = make_registry() + registry.register("case")(str.upper) + registry.register("case", replace=True)(str.lower) + assert registry.get("case")("Hi") == "hi" - try: - assert pythonic.export(ROWS, "upper") == "ADA ENG" - finally: - del pythonic.EXPORTERS["upper"] - def test_unknown_format_names_the_known_ones(self) -> None: - with pytest.raises(ValueError, match="known: csv, keyvalue"): - pythonic.export(ROWS, "yaml") +class TestLookup: + def test_unknown_names_fail_loudly_and_list_known_ones(self) -> None: + registry = make_registry() + registry.register("upper")(str.upper) + registry.register("lower")(str.lower) + message = r"unknown handler 'title' \(known: lower, upper\)" + with pytest.raises(UnknownKeyError, match=message): + registry.get("title") + def test_an_empty_registry_says_so(self) -> None: + with pytest.raises(UnknownKeyError, match=""): + make_registry().get("anything") -class TestRealWorld: - def test_codec_registry_resolves_names(self) -> None: - assert real_world.rot13("gura fur fnvq") == "then she said" - assert real_world.lookup_is_the_registry("UTF8") == "utf-8" + def test_introspection_surface(self) -> None: + registry = make_registry() + registry.register("b")(str.upper) + registry.register("a")(str.lower) + assert registry.names() == ("a", "b") + assert "a" in registry and "z" not in registry + assert len(registry) == 2 diff --git a/patterns/modern/repository/README.md b/patterns/modern/repository/README.md index f4caf2f..5268897 100644 --- a/patterns/modern/repository/README.md +++ b/patterns/modern/repository/README.md @@ -14,30 +14,17 @@ stdlib_sightings: [sqlite3, shelve] # Repository -## Problem - -Pricing rules shouldn't know SQL. When persistence details soak into domain -logic, every business test drags a database behind it and every storage -change touches everything. - -## Naive solution - -`naive.py` inlines sqlite calls in the domain function — compact, and -welded shut. - -## Pythonic solution - -A `Protocol` names the collection-like operations the domain needs (`add`, -`get`, `list`); an in-memory dict repo serves tests, a sqlite repo serves -production, and the domain function accepts either. - -## In the wild - -`shelve` is a ready-made key-object repository over `dbm`; `sqlite3` with a -thin class over it is the standard hand-rolled form (shown in -`real_world.py`). - -## Verdict - -**Use with care.** Earn it with a real second implementation (the in-memory -fake counts); skip it for scripts that just need a query. +Domain logic speaks to storage through a small `Protocol` port; a fake and a +real adapter both satisfy it, held together by shared contract tests. +**Verdict: use with care** — earn it with a genuine second implementation. + +| Where | What | +|---|---| +| [`pattern/`](pattern/) | The importable code: `Invoice`, `Invoices` (port), `InMemoryInvoices` (fake), `total_owed`, `overdue` | +| [`docs/`](docs/) | [Fundamentals](docs/fundamentals.md) · [Implementation guide](docs/implementation.md) · [External examples](docs/examples.md) | +| [`examples/invoice_ledger/`](examples/invoice_ledger/) | Mini-project: sqlite adapter + identical answers from both backends | +| [`tests/`](tests/) | Domain tests on the fake; one contract suite parametrized over both adapters | + +```bash +uv run python -m patterns.modern.repository.examples.invoice_ledger +``` diff --git a/patterns/modern/repository/__init__.py b/patterns/modern/repository/__init__.py index 7f92ffb..39528de 100644 --- a/patterns/modern/repository/__init__.py +++ b/patterns/modern/repository/__init__.py @@ -1 +1,14 @@ -"""Repository: collection-like storage seam for domain logic.""" +"""Repository — public API. + +>>> from patterns.modern.repository import Invoices, InMemoryInvoices +""" + +from patterns.modern.repository.pattern import ( + InMemoryInvoices, + Invoice, + Invoices, + overdue, + total_owed, +) + +__all__ = ["InMemoryInvoices", "Invoice", "Invoices", "overdue", "total_owed"] diff --git a/patterns/modern/repository/docs/examples.md b/patterns/modern/repository/docs/examples.md new file mode 100644 index 0000000..1c30260 --- /dev/null +++ b/patterns/modern/repository/docs/examples.md @@ -0,0 +1,46 @@ +# Repository — where it lives outside this repo + +Cited, real implementations to study (or point an agent at) when designing or +reviewing repository-shaped code. + +## Canonical references + +- **Fowler, PoEAA — Repository.** The original catalog entry: a + collection-like interface mediating between domain and data mapping. + [martinfowler.com/eaaCatalog/repository.html](https://martinfowler.com/eaaCatalog/repository.html) +- **Percival & Gregory, *Architecture Patterns with Python*, ch. 2.** The + canonical Python worked example — `AbstractRepository`, a fake, SQLAlchemy + adapter, and the argument for contract tests. This unit is that chapter + in miniature, with `Protocol` instead of an ABC. + [cosmicpython.com/book/chapter_02_repository.html](https://www.cosmicpython.com/book/chapter_02_repository.html) + +## Python standard library + +- **`sqlite3`.** The stdlib backend a real adapter wraps — the mini-project's + `SqliteInvoices` is the standard hand-rolled form. + [docs.python.org/3/library/sqlite3.html](https://docs.python.org/3/library/sqlite3.html) +- **`shelve`.** A ready-made key→object repository over `dbm`: the smallest + possible repository surface (`__getitem__`/`__setitem__`), useful for + calibrating how little a port can be. + [docs.python.org/3/library/shelve.html](https://docs.python.org/3/library/shelve.html) + +## Major ecosystems — and a contrast + +- **Django `Manager`/`QuerySet`.** The *active-record* flavor: storage API + attached to the model class itself (`Invoice.objects.filter(...)`). + Convenient, and exactly what repository is **not** — the domain type and + the query surface are welded together, so there is no port to fake. + Knowing the difference is most of knowing when you need this pattern. + [docs.djangoproject.com/en/stable/topics/db/managers/](https://docs.djangoproject.com/en/stable/topics/db/managers/) +- **SQLAlchemy `Session`.** The data-mapper half the pattern assumes: domain + objects stay plain, the session maps them — a repository is a thin, + domain-vocabulary port over it. + [docs.sqlalchemy.org/en/latest/orm/session_basics.html](https://docs.sqlalchemy.org/en/latest/orm/session_basics.html) + +## What to notice across all of them + +The dividing line is always *who owns the interface*: repository puts the +domain in charge of a small port; active record puts the framework in charge +of a wide one. When reviewing, ask for the fake — if an in-memory +implementation would be laborious to write, the port has grown past the +domain's actual needs. diff --git a/patterns/modern/repository/docs/fundamentals.md b/patterns/modern/repository/docs/fundamentals.md new file mode 100644 index 0000000..53aeaeb --- /dev/null +++ b/patterns/modern/repository/docs/fundamentals.md @@ -0,0 +1,64 @@ +# Repository — fundamentals + +## Intent + +Keep domain logic ignorant of how objects are stored by mediating through a +collection-like interface. Named in Fowler's +[Patterns of Enterprise Application Architecture](https://martinfowler.com/eaaCatalog/repository.html); +given its canonical modern-Python treatment in +[Architecture Patterns with Python, ch. 2](https://www.cosmicpython.com/book/chapter_02_repository.html). + +## Participants + +| Role | Enterprise form | Python form | +|---|---|---| +| Domain objects | Mapped entities | Frozen dataclasses — `Invoice` in [`pattern/ledger.py`](../pattern/ledger.py) | +| The port | A repository interface | A `Protocol` naming only the operations the domain needs (`Invoices`) | +| Real adapter | ORM-backed repository class | Any class with the same methods (the mini-project's `SqliteInvoices`) | +| The fake | A mocking framework's job | `InMemoryInvoices` — a list with the port's methods, shipped *with* the pattern | +| Domain services | Methods on entities/services | Plain functions taking the port (`total_owed`, `overdue`) | + +## Mechanism + +1. The domain names its storage needs as a small `Protocol` — the operations + it actually uses, not a generic CRUD surface. +2. Domain logic takes the port as a parameter and never imports a driver. +3. Two adapters satisfy the port: an in-memory fake for tests and a real one + for production. Structural typing means neither declares anything. +4. One shared contract test suite runs against **both** adapters — that suite + is what makes "the fake behaves like production" a checked fact instead of + a hope. + +## The welded-shut form, and what Python absorbs + +The pre-pattern shape inlines storage into the domain question: + +```python +def total_owed(conn: sqlite3.Connection, customer: str) -> int: + rows = conn.execute("SELECT amount FROM invoices WHERE customer = ?", (customer,)).fetchall() + return sum(amount for (amount,) in rows) # domain math, welded to SQL +``` + +Compact — and every test of the *math* now drags a database, and every +storage change touches domain files. Enterprise stacks answered with +repository interfaces, unit-of-work classes, and ORMs. Python absorbs the +ceremony: `Protocol` gives the interface without inheritance, a list gives +the fake without a mocking framework. What survives is the discipline — +**the domain speaks only in its own types, through a port it owns**. + +## When to use it + +- Domain logic worth testing at speed, uncoupled from storage. +- A genuine second backend (and the in-memory fake counts as one). + +## When not to use it + +- Scripts that just need a query — the pattern's indirection buys nothing. +- One entity, one backend, no tests that hurt — wait for the pain. +- An ORM you're happy to couple to everywhere is itself a repository-shaped + boundary; wrapping it again adds a layer with no new seam. + +## Verdict: use with care + +Earn it with a real second implementation and shared contract tests; if your +tests still hit a database, the repository isn't earning its keep. diff --git a/patterns/modern/repository/docs/implementation.md b/patterns/modern/repository/docs/implementation.md new file mode 100644 index 0000000..1a68315 --- /dev/null +++ b/patterns/modern/repository/docs/implementation.md @@ -0,0 +1,76 @@ +# Repository — putting it into a system + +## The smell it fixes + +Domain tests that need infrastructure: + +```python +def test_total_owed() -> None: + conn = sqlite3.connect(TEST_DB) # schema setup, fixtures, teardown... + assert total_owed(conn, "ada") == 150 +``` + +SQL scattered through business logic means the business rules can't be +tested — or changed — without dragging storage along. + +## Steps + +1. **Write the domain type first.** A frozen dataclass in domain vocabulary + (`Invoice(number, customer, amount_cents, due)`) — no ORM base, no row + shapes. +2. **Name the port from the domain's demand side.** List the storage + operations domain code *actually performs* and put exactly those in a + `Protocol`. Three methods is a normal size; ten is a warning. +3. **Build the fake in the same module as the port.** A list with the port's + methods. It ships with the pattern, not buried in test helpers, because + it *is* the deliverable that makes domain tests instant. +4. **Move the SQL into a real adapter** that satisfies the same `Protocol` + (structurally — no base class), owning all row↔dataclass conversion. +5. **Write one contract test suite, parametrized over both adapters.** Same + assertions, both backends. This is the step most implementations skip, + and it is what keeps the fake honest. +6. **Pass the port into domain functions** — plain functions taking + `repo: Invoices` stay importable, testable, and driver-free. + +```python +from patterns.modern.repository import InMemoryInvoices, total_owed + +repo = InMemoryInvoices() +repo.add(Invoice("INV-1", "ada", 120_00, date(2026, 8, 1))) +assert total_owed(repo, "ada") == 120_00 +``` + +## Python idioms that keep it small + +- **`Protocol` over ABC**: adapters stay dependency-free; sqlite3's and the + fake's only relationship is behavioral. +- **Frozen dataclasses** make identity questions explicit and rows + hashable-by-value in tests. +- **Keep queries as methods, not a query language.** `for_customer(name)` + beats `find(spec)` until you have evidence otherwise (the caveat about + generic `Repository[T]` in this unit's frontmatter). + +## Pitfalls + +- **The port grows to mirror SQL.** If a method exists because a screen + needed a `JOIN`, the domain is no longer defining the port. Split read + models out rather than widening the port. +- **The fake drifts from production.** Without shared contract tests, the + fake quietly diverges (ordering, duplicates, missing rows) and domain + tests pass against behavior production doesn't have. +- **Leaking storage types** — returning rows, cursors, or ORM instances + through the port re-couples everything the pattern decoupled. +- **A repository per table** instead of per domain concept: the port serves + an aggregate, not a schema. +- **Transactions smeared across repositories.** Commit/rollback is its own + seam (unit of work); bolting `commit()` onto each repository hides it. + +## Worked example + +[`examples/invoice_ledger/`](../examples/invoice_ledger/) adds the sqlite +adapter and prints identical domain answers from both backends; the shared +contract tests live in [`tests/test_invoice_ledger.py`](../tests/test_invoice_ledger.py): + +```bash +uv run python -m patterns.modern.repository.examples.invoice_ledger +``` diff --git a/patterns/modern/repository/examples/__init__.py b/patterns/modern/repository/examples/__init__.py new file mode 100644 index 0000000..8bacf97 --- /dev/null +++ b/patterns/modern/repository/examples/__init__.py @@ -0,0 +1 @@ +"""Mini-projects demonstrating the Repository in practice.""" diff --git a/patterns/modern/repository/examples/invoice_ledger/__init__.py b/patterns/modern/repository/examples/invoice_ledger/__init__.py new file mode 100644 index 0000000..d20e332 --- /dev/null +++ b/patterns/modern/repository/examples/invoice_ledger/__init__.py @@ -0,0 +1,8 @@ +"""An invoice ledger over two interchangeable repositories. + +Run it: ``uv run python -m patterns.modern.repository.examples.invoice_ledger`` +""" + +from patterns.modern.repository.examples.invoice_ledger.sqlite_repo import SqliteInvoices + +__all__ = ["SqliteInvoices"] diff --git a/patterns/modern/repository/examples/invoice_ledger/__main__.py b/patterns/modern/repository/examples/invoice_ledger/__main__.py new file mode 100644 index 0000000..026f6d8 --- /dev/null +++ b/patterns/modern/repository/examples/invoice_ledger/__main__.py @@ -0,0 +1,38 @@ +"""Demo: identical domain answers from the fake and the sqlite adapter.""" + +from __future__ import annotations + +import sqlite3 +from datetime import date + +from patterns.modern.repository.examples.invoice_ledger.sqlite_repo import SqliteInvoices +from patterns.modern.repository.pattern import ( + InMemoryInvoices, + Invoice, + Invoices, + overdue, + total_owed, +) + +LEDGER = [ + Invoice("INV-1", "ada", 120_00, date(2026, 8, 1)), + Invoice("INV-2", "ada", 80_00, date(2026, 9, 15)), + Invoice("INV-3", "grace", 45_50, date(2026, 7, 15)), +] +TODAY = date(2026, 8, 27) + + +def report(label: str, repo: Invoices) -> None: + for invoice in LEDGER: + repo.add(invoice) + late = ", ".join(i.number for i in overdue(repo, TODAY)) or "none" + print(f"[{label}] ada owes {total_owed(repo, 'ada') / 100:.2f}; overdue: {late}") + + +def main() -> None: + report("memory", InMemoryInvoices()) + report("sqlite", SqliteInvoices(sqlite3.connect(":memory:"))) + + +if __name__ == "__main__": + main() diff --git a/patterns/modern/repository/examples/invoice_ledger/sqlite_repo.py b/patterns/modern/repository/examples/invoice_ledger/sqlite_repo.py new file mode 100644 index 0000000..c6813b6 --- /dev/null +++ b/patterns/modern/repository/examples/invoice_ledger/sqlite_repo.py @@ -0,0 +1,47 @@ +"""The real adapter: the same three methods over durable storage. + +The domain functions cannot tell this from ``InMemoryInvoices`` — the shared +contract tests in ``tests/test_invoice_ledger.py`` hold both to it. +""" + +from __future__ import annotations + +import sqlite3 +from datetime import date + +from patterns.modern.repository.pattern import Invoice + + +class SqliteInvoices: + """An ``Invoices`` adapter over sqlite3 (stdlib, durable when given a path).""" + + def __init__(self, conn: sqlite3.Connection) -> None: + self._conn = conn + self._conn.execute( + "CREATE TABLE IF NOT EXISTS invoices" + " (number TEXT PRIMARY KEY, customer TEXT, amount_cents INT, due TEXT)" + ) + + def add(self, invoice: Invoice) -> None: + self._conn.execute( + "INSERT INTO invoices VALUES (?, ?, ?, ?)", + (invoice.number, invoice.customer, invoice.amount_cents, invoice.due.isoformat()), + ) + + def for_customer(self, customer: str) -> list[Invoice]: + rows = self._conn.execute( + "SELECT number, customer, amount_cents, due FROM invoices WHERE customer = ?", + (customer,), + ).fetchall() + return [self._to_invoice(row) for row in rows] + + def list_all(self) -> list[Invoice]: + rows = self._conn.execute( + "SELECT number, customer, amount_cents, due FROM invoices" + ).fetchall() + return [self._to_invoice(row) for row in rows] + + @staticmethod + def _to_invoice(row: tuple[str, str, int, str]) -> Invoice: + number, customer, amount_cents, due = row + return Invoice(number, customer, amount_cents, date.fromisoformat(due)) diff --git a/patterns/modern/repository/naive.py b/patterns/modern/repository/naive.py deleted file mode 100644 index f01d053..0000000 --- a/patterns/modern/repository/naive.py +++ /dev/null @@ -1,23 +0,0 @@ -"""Persistence soaked into domain logic: SQL inline, everywhere.""" - -from __future__ import annotations - -import sqlite3 - - -def total_owed(conn: sqlite3.Connection, customer: str) -> int: - """Domain question, welded to storage details.""" - conn.execute("CREATE TABLE IF NOT EXISTS invoices (customer TEXT, amount INT)") - rows = conn.execute("SELECT amount FROM invoices WHERE customer = ?", (customer,)).fetchall() - return sum(amount for (amount,) in rows) - - -def main() -> None: - conn = sqlite3.connect(":memory:") - conn.execute("CREATE TABLE invoices (customer TEXT, amount INT)") - conn.executemany("INSERT INTO invoices VALUES (?, ?)", [("ada", 100), ("ada", 50)]) - print(f"ada owes {total_owed(conn, 'ada')}") - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/repository/pattern/__init__.py b/patterns/modern/repository/pattern/__init__.py new file mode 100644 index 0000000..7606de8 --- /dev/null +++ b/patterns/modern/repository/pattern/__init__.py @@ -0,0 +1,11 @@ +"""The Repository pattern, importable as library code.""" + +from patterns.modern.repository.pattern.ledger import ( + InMemoryInvoices, + Invoice, + Invoices, + overdue, + total_owed, +) + +__all__ = ["InMemoryInvoices", "Invoice", "Invoices", "overdue", "total_owed"] diff --git a/patterns/modern/repository/pattern/ledger.py b/patterns/modern/repository/pattern/ledger.py new file mode 100644 index 0000000..ea60dba --- /dev/null +++ b/patterns/modern/repository/pattern/ledger.py @@ -0,0 +1,61 @@ +"""The repository seam: a domain type, a ``Protocol`` port, and the fake. + +``Invoices`` names the collection-like operations the domain needs — three +methods, no more. ``InMemoryInvoices`` lives here rather than in a test +helper because the fake *is* the pattern's payoff: domain tests run against +it instantly, and any real adapter (see the mini-project's sqlite one) must +behave identically or the shared contract tests say so. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import date +from typing import Protocol + + +@dataclass(frozen=True) +class Invoice: + """One invoice, as the domain sees it — no storage details.""" + + number: str + customer: str + amount_cents: int + due: date + + +class Invoices(Protocol): + """The port: what the domain may ask of invoice storage.""" + + def add(self, invoice: Invoice) -> None: ... + + def for_customer(self, customer: str) -> list[Invoice]: ... + + def list_all(self) -> list[Invoice]: ... + + +class InMemoryInvoices: + """The fake that makes domain tests instant.""" + + def __init__(self) -> None: + self._items: list[Invoice] = [] + + def add(self, invoice: Invoice) -> None: + self._items.append(invoice) + + def for_customer(self, customer: str) -> list[Invoice]: + return [i for i in self._items if i.customer == customer] + + def list_all(self) -> list[Invoice]: + return list(self._items) + + +def total_owed(repo: Invoices, customer: str) -> int: + """Pure domain logic: no storage details anywhere in sight.""" + return sum(invoice.amount_cents for invoice in repo.for_customer(customer)) + + +def overdue(repo: Invoices, today: date, grace_days: int = 0) -> list[Invoice]: + """Every invoice more than ``grace_days`` past due, oldest first.""" + late = [i for i in repo.list_all() if (today - i.due).days > grace_days] + return sorted(late, key=lambda i: i.due) diff --git a/patterns/modern/repository/pythonic.py b/patterns/modern/repository/pythonic.py deleted file mode 100644 index 708117e..0000000 --- a/patterns/modern/repository/pythonic.py +++ /dev/null @@ -1,54 +0,0 @@ -"""The repository seam: a Protocol, a fake, and domain logic that can't tell. - -Tests use InMemoryInvoices; production wires something durable. The domain -function is identical either way. -""" - -from __future__ import annotations - -from dataclasses import dataclass -from typing import Protocol - - -@dataclass(frozen=True) -class Invoice: - customer: str - amount: int - - -class Invoices(Protocol): - """The collection-like operations the domain actually needs.""" - - def add(self, invoice: Invoice) -> None: ... - - def for_customer(self, customer: str) -> list[Invoice]: ... - - -class InMemoryInvoices: - """The fake that makes domain tests instant.""" - - def __init__(self) -> None: - self._items: list[Invoice] = [] - - def add(self, invoice: Invoice) -> None: - self._items.append(invoice) - - def for_customer(self, customer: str) -> list[Invoice]: - return [i for i in self._items if i.customer == customer] - - -def total_owed(repo: Invoices, customer: str) -> int: - """Pure domain logic: no storage details anywhere in sight.""" - return sum(invoice.amount for invoice in repo.for_customer(customer)) - - -def main() -> None: - repo = InMemoryInvoices() - repo.add(Invoice("ada", 100)) - repo.add(Invoice("ada", 50)) - repo.add(Invoice("grace", 9)) - print(f"ada owes {total_owed(repo, 'ada')}") - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/repository/real_world.py b/patterns/modern/repository/real_world.py deleted file mode 100644 index d1ed1d0..0000000 --- a/patterns/modern/repository/real_world.py +++ /dev/null @@ -1,36 +0,0 @@ -"""A sqlite3-backed repository satisfying the same protocol. - -Same domain function, durable storage -- the swap the pattern promises. -""" - -from __future__ import annotations - -import sqlite3 - -from patterns.modern.repository.pythonic import Invoice, total_owed - - -class SqliteInvoices: - def __init__(self, conn: sqlite3.Connection) -> None: - self._conn = conn - self._conn.execute("CREATE TABLE IF NOT EXISTS invoices (customer TEXT, amount INT)") - - def add(self, invoice: Invoice) -> None: - self._conn.execute("INSERT INTO invoices VALUES (?, ?)", (invoice.customer, invoice.amount)) - - def for_customer(self, customer: str) -> list[Invoice]: - rows = self._conn.execute( - "SELECT customer, amount FROM invoices WHERE customer = ?", (customer,) - ).fetchall() - return [Invoice(c, a) for c, a in rows] - - -def main() -> None: - repo = SqliteInvoices(sqlite3.connect(":memory:")) - repo.add(Invoice("ada", 100)) - repo.add(Invoice("ada", 50)) - print(f"ada owes {total_owed(repo, 'ada')} (from sqlite)") - - -if __name__ == "__main__": - main() diff --git a/patterns/modern/repository/tests/test_invoice_ledger.py b/patterns/modern/repository/tests/test_invoice_ledger.py new file mode 100644 index 0000000..2b6bbd0 --- /dev/null +++ b/patterns/modern/repository/tests/test_invoice_ledger.py @@ -0,0 +1,72 @@ +"""The mini-project's point, as tests: ONE contract suite, BOTH adapters. + +Every test here runs against the in-memory fake and the sqlite adapter via +the parametrized fixture — the fake stays honest because the same +assertions hold production storage to the same behavior. +""" + +from __future__ import annotations + +import sqlite3 +from collections.abc import Iterator +from datetime import date + +import pytest + +from patterns.modern.repository.examples.invoice_ledger import SqliteInvoices +from patterns.modern.repository.examples.invoice_ledger.__main__ import main +from patterns.modern.repository.pattern import ( + InMemoryInvoices, + Invoice, + Invoices, + overdue, + total_owed, +) + +TODAY = date(2026, 8, 27) + + +@pytest.fixture(params=["memory", "sqlite"]) +def repo(request: pytest.FixtureRequest) -> Iterator[Invoices]: + if request.param == "memory": + yield InMemoryInvoices() + else: + conn = sqlite3.connect(":memory:") + yield SqliteInvoices(conn) + conn.close() + + +class TestRepositoryContract: + """The port's behavior, pinned identically for fake and real adapter.""" + + def test_added_invoices_come_back_whole(self, repo: Invoices) -> None: + inv = Invoice("INV-1", "ada", 120_00, date(2026, 8, 1)) + repo.add(inv) + assert repo.for_customer("ada") == [inv] # round-trip preserves types + + def test_for_customer_filters(self, repo: Invoices) -> None: + repo.add(Invoice("INV-1", "ada", 100, TODAY)) + repo.add(Invoice("INV-2", "grace", 200, TODAY)) + assert [i.number for i in repo.for_customer("grace")] == ["INV-2"] + + def test_list_all_returns_everything(self, repo: Invoices) -> None: + repo.add(Invoice("INV-1", "ada", 100, TODAY)) + repo.add(Invoice("INV-2", "grace", 200, TODAY)) + assert {i.number for i in repo.list_all()} == {"INV-1", "INV-2"} + + def test_domain_logic_cannot_tell_the_adapters_apart(self, repo: Invoices) -> None: + repo.add(Invoice("INV-1", "ada", 120_00, date(2026, 8, 1))) + repo.add(Invoice("INV-2", "ada", 80_00, date(2026, 9, 15))) + assert total_owed(repo, "ada") == 200_00 + assert [i.number for i in overdue(repo, TODAY)] == ["INV-1"] + + +class TestDemo: + def test_main_reports_identical_answers_from_both_backends( + self, capsys: pytest.CaptureFixture[str] + ) -> None: + main() + lines = capsys.readouterr().out.strip().splitlines() + assert lines[0].replace("[memory]", "") == lines[1].replace("[sqlite]", "") + assert "ada owes 200.00" in lines[0] + assert "overdue: INV-3, INV-1" in lines[0] diff --git a/patterns/modern/repository/tests/test_ledger.py b/patterns/modern/repository/tests/test_ledger.py new file mode 100644 index 0000000..4089d4c --- /dev/null +++ b/patterns/modern/repository/tests/test_ledger.py @@ -0,0 +1,38 @@ +"""Behavioral tests for the pattern's domain logic, run against the fake.""" + +from __future__ import annotations + +from datetime import date + +from patterns.modern.repository import InMemoryInvoices, Invoice, overdue, total_owed + +TODAY = date(2026, 8, 27) + + +def invoice(number: str, customer: str = "ada", cents: int = 100_00, due: date = TODAY) -> Invoice: + return Invoice(number, customer, cents, due) + + +class TestDomainLogic: + def test_total_owed_sums_one_customer_only(self) -> None: + repo = InMemoryInvoices() + repo.add(invoice("INV-1", "ada", 100_00)) + repo.add(invoice("INV-2", "ada", 50_00)) + repo.add(invoice("INV-3", "grace", 9_00)) + assert total_owed(repo, "ada") == 150_00 + + def test_total_owed_for_an_unknown_customer_is_zero(self) -> None: + assert total_owed(InMemoryInvoices(), "nobody") == 0 + + def test_overdue_respects_grace_and_sorts_oldest_first(self) -> None: + repo = InMemoryInvoices() + repo.add(invoice("INV-1", due=date(2026, 8, 1))) + repo.add(invoice("INV-2", due=date(2026, 7, 1))) + repo.add(invoice("INV-3", due=date(2026, 8, 26))) # 1 day late + late = overdue(repo, TODAY, grace_days=5) + assert [i.number for i in late] == ["INV-2", "INV-1"] + + def test_due_today_is_not_overdue(self) -> None: + repo = InMemoryInvoices() + repo.add(invoice("INV-1", due=TODAY)) + assert overdue(repo, TODAY) == [] diff --git a/patterns/modern/repository/tests/test_repository.py b/patterns/modern/repository/tests/test_repository.py deleted file mode 100644 index 44ac74c..0000000 --- a/patterns/modern/repository/tests/test_repository.py +++ /dev/null @@ -1,41 +0,0 @@ -"""Behavioral tests for all three repository variants.""" - -import sqlite3 - -from patterns.modern.repository import naive, pythonic, real_world -from patterns.modern.repository.pythonic import Invoice - - -class TestNaive: - def test_inline_sql_works_but_needs_a_database(self) -> None: - conn = sqlite3.connect(":memory:") - conn.execute("CREATE TABLE invoices (customer TEXT, amount INT)") - conn.executemany("INSERT INTO invoices VALUES (?, ?)", [("ada", 100), ("ada", 50)]) - assert naive.total_owed(conn, "ada") == 150 - - -class TestPythonic: - def test_domain_logic_runs_on_the_fake(self) -> None: - repo = pythonic.InMemoryInvoices() - repo.add(Invoice("ada", 100)) - repo.add(Invoice("grace", 9)) - assert pythonic.total_owed(repo, "ada") == 100 - - def test_unknown_customer_owes_nothing(self) -> None: - assert pythonic.total_owed(pythonic.InMemoryInvoices(), "nobody") == 0 - - -class TestRealWorld: - def test_same_domain_function_over_sqlite(self) -> None: - repo = real_world.SqliteInvoices(sqlite3.connect(":memory:")) - repo.add(Invoice("ada", 100)) - repo.add(Invoice("ada", 50)) - assert pythonic.total_owed(repo, "ada") == 150 - - def test_the_two_repos_are_interchangeable(self) -> None: - for repo in ( - pythonic.InMemoryInvoices(), - real_world.SqliteInvoices(sqlite3.connect(":memory:")), - ): - repo.add(Invoice("x", 7)) - assert pythonic.total_owed(repo, "x") == 7 From 640c19152660e5783282d65a02c0d121ba7b8766 Mon Sep 17 00:00:00 2001 From: SuperElectron Date: Thu, 27 Aug 2026 13:17:34 -0700 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20modern=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20observable=20shutdown=20disciplines,=20durable=20sq?= =?UTF-8?q?lite,=20backpressure=20pinned?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mechanism tests kill both discipline-collapse mutations; SqliteInvoices commits (file-backed reopen test); pytest-timeout 30s; backpressure and sentinel accounting asserted; AtomicWrite atomicity via os.replace spy; duplicate-add contract decided (refused, both backends); docs snippets runnable as written. Co-Authored-By: Claude Fable 5 --- .../docs/fundamentals.md | 4 +- .../docs/implementation.md | 5 +- .../async_producer_consumer/pattern/pool.py | 12 ++- .../tests/test_pool.py | 76 +++++++++++++++++++ .../modern/context_manager/docs/examples.md | 2 +- .../context_manager/docs/fundamentals.md | 8 ++ .../examples/atomic_deploy/deploy.py | 4 +- .../context_manager/tests/test_managers.py | 30 ++++++++ .../docs/implementation.md | 17 ++++- .../tests/test_service.py | 15 +++- .../modern/registry/docs/implementation.md | 5 ++ patterns/modern/registry/pattern/registry.py | 2 +- .../modern/registry/tests/test_registry.py | 10 +++ .../modern/repository/docs/implementation.md | 6 +- .../examples/invoice_ledger/sqlite_repo.py | 20 +++-- patterns/modern/repository/pattern/ledger.py | 8 +- .../repository/tests/test_invoice_ledger.py | 28 ++++++- pyproject.toml | 2 + 18 files changed, 229 insertions(+), 25 deletions(-) diff --git a/patterns/modern/async_producer_consumer/docs/fundamentals.md b/patterns/modern/async_producer_consumer/docs/fundamentals.md index 26bce16..c9cc630 100644 --- a/patterns/modern/async_producer_consumer/docs/fundamentals.md +++ b/patterns/modern/async_producer_consumer/docs/fundamentals.md @@ -24,7 +24,7 @@ that neither drops items nor hangs. backpressure valve — `put` blocks when the buffer is full. 2. N workers start, each looping `get → process`. 3. Producers enqueue items; slow consumers automatically slow the producers. -4. Shutdown, the part naive versions get wrong, is one of two disciplines: +4. Shutdown, the part first attempts get wrong, is one of two disciplines: - **Sentinel** — after the last item, enqueue one end-marker per worker; each worker exits on dequeuing one. - **Join and cancel** — workers mark `task_done()`; the coordinator awaits @@ -37,7 +37,7 @@ Before asyncio this was the thread pattern — an OS thread per worker, a lock around shared results, and hand-rolled sentinel plumbing: ```python -def process_all(items: list[str], worker_count: int = 2) -> list[str]: +def process_all_threaded(items: list[str], worker_count: int = 2) -> list[str]: channel: queue.Queue[str | None] = queue.Queue() results: list[str] = [] lock = threading.Lock() diff --git a/patterns/modern/async_producer_consumer/docs/implementation.md b/patterns/modern/async_producer_consumer/docs/implementation.md index aac1e09..52115e9 100644 --- a/patterns/modern/async_producer_consumer/docs/implementation.md +++ b/patterns/modern/async_producer_consumer/docs/implementation.md @@ -18,8 +18,9 @@ clock shows it — sequential awaits over independent I/O. Or the opposite: 4. **Pick the shutdown discipline — and write the test the same day.** - `Shutdown.JOIN_AND_CANCEL` when a coordinator knows the item set and wants "everything finished" as a joinable event. - - `Shutdown.SENTINEL` when the producer itself signals the end of a - stream and workers should drain and stop. + - `Shutdown.SENTINEL` when workers should drain the queue and stop: + the pool enqueues one end-marker per worker after the items are + exhausted. 5. **Decide the failure policy at the edge.** The pool is fail-fast (one bad item cancels the run, surfacing as an `ExceptionGroup`). If a bad item must not kill the batch, catch inside *your* processor and return an diff --git a/patterns/modern/async_producer_consumer/pattern/pool.py b/patterns/modern/async_producer_consumer/pattern/pool.py index f07aa84..0372686 100644 --- a/patterns/modern/async_producer_consumer/pattern/pool.py +++ b/patterns/modern/async_producer_consumer/pattern/pool.py @@ -11,7 +11,7 @@ import asyncio from collections.abc import Awaitable, Callable, Iterable from enum import Enum -from typing import Generic, TypeVar +from typing import Any, Generic, TypeVar Item = TypeVar("Item") Result = TypeVar("Result") @@ -68,8 +68,14 @@ async def run(self, items: Iterable[Item]) -> list[Result]: return await self._run_sentinel(items) return await self._run_join_and_cancel(items) + def _make_channel(self, maxsize: int) -> asyncio.Queue[Any]: + """Observability seam: tests substitute a recording queue here to + assert the shutdown *mechanism* (sentinel count, task_done + bookkeeping, backpressure bound), not just the results.""" + return asyncio.Queue(maxsize=maxsize) + async def _run_join_and_cancel(self, items: Iterable[Item]) -> list[Result]: - channel: asyncio.Queue[Item] = asyncio.Queue(maxsize=self._maxsize) + channel: asyncio.Queue[Item] = self._make_channel(self._maxsize) results: list[Result] = [] async def worker() -> None: @@ -90,7 +96,7 @@ async def worker() -> None: return results async def _run_sentinel(self, items: Iterable[Item]) -> list[Result]: - channel: asyncio.Queue[Item | _End] = asyncio.Queue(maxsize=self._maxsize) + channel: asyncio.Queue[Item | _End] = self._make_channel(self._maxsize) results: list[Result] = [] async def worker() -> None: diff --git a/patterns/modern/async_producer_consumer/tests/test_pool.py b/patterns/modern/async_producer_consumer/tests/test_pool.py index 7e4883c..f54343d 100644 --- a/patterns/modern/async_producer_consumer/tests/test_pool.py +++ b/patterns/modern/async_producer_consumer/tests/test_pool.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +from typing import Any import pytest @@ -11,6 +12,7 @@ WorkerPool, process_all, ) +from patterns.modern.async_producer_consumer.pattern.pool import _End async def upper(item: str) -> str: @@ -72,3 +74,77 @@ async def slow_first(item: int) -> int: def test_pool_requires_at_least_one_worker(self) -> None: with pytest.raises(ValueError): WorkerPool(upper, workers=0) + + +class RecordingQueue(asyncio.Queue[Any]): + """An asyncio.Queue that logs puts, task_done calls, and peak backlog.""" + + def __init__(self, maxsize: int = 0) -> None: + super().__init__(maxsize) + self.created_maxsize = maxsize + self.put_log: list[Any] = [] + self.max_backlog = 0 + self.task_done_calls = 0 + + async def put(self, item: Any) -> None: + await super().put(item) + self.put_log.append(item) + self.max_backlog = max(self.max_backlog, self.qsize()) + + def task_done(self) -> None: + self.task_done_calls += 1 + super().task_done() + + +class ObservablePool(WorkerPool[str, str]): + """WorkerPool with the channel seam swapped for a RecordingQueue.""" + + channel: RecordingQueue + + def _make_channel(self, maxsize: int) -> asyncio.Queue[Any]: + self.channel = RecordingQueue(maxsize) + return self.channel + + +class TestShutdownMechanism: + """The disciplines must differ observably — not just agree on results. + + Collapsing ``run``'s switch to either branch fails one of these tests, + so the switch itself is pinned, not merely the outcomes. + """ + + async def test_sentinel_enqueues_one_marker_per_worker_and_drains(self) -> None: + pool = ObservablePool(upper, workers=3, shutdown=Shutdown.SENTINEL) + await pool.run(["a", "b", "c", "d", "e"]) + markers = [x for x in pool.channel.put_log if isinstance(x, _End)] + assert len(markers) == 3 # exactly one per worker, no orphans + assert pool.channel.qsize() == 0 # every marker consumed: clean drain + assert pool.channel.task_done_calls == 0 # no join bookkeeping here + + async def test_join_and_cancel_uses_task_done_and_no_sentinels(self) -> None: + pool = ObservablePool(upper, workers=3, shutdown=Shutdown.JOIN_AND_CANCEL) + await pool.run(["a", "b", "c", "d", "e"]) + assert pool.channel.task_done_calls == 5 # join() waits on these + assert not any(isinstance(x, _End) for x in pool.channel.put_log) + assert pool.channel.qsize() == 0 + + async def test_backpressure_bounds_the_queue(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Observes the REAL pool's queue (not the test seam), so making the + pool construct an unbounded queue fails here.""" + created: list[RecordingQueue] = [] + + class TrackingQueue(RecordingQueue): + def __init__(self, maxsize: int = 0) -> None: + super().__init__(maxsize) + created.append(self) + + monkeypatch.setattr(asyncio, "Queue", TrackingQueue) + + async def slow(item: str) -> str: + await asyncio.sleep(0.001) + return item.upper() + + await process_all([chr(ord("a") + n) for n in range(10)], slow, workers=2, maxsize=2) + (channel,) = created + assert channel.created_maxsize == 2 # the bound is actually passed through + assert channel.max_backlog <= 2 # and never exceeded during the run diff --git a/patterns/modern/context_manager/docs/examples.md b/patterns/modern/context_manager/docs/examples.md index e7a8557..799f045 100644 --- a/patterns/modern/context_manager/docs/examples.md +++ b/patterns/modern/context_manager/docs/examples.md @@ -25,4 +25,4 @@ Real embodiments of the pattern outside this repo, for deeper study. *(unverified)* - **Django `transaction.atomic`** — one transaction seam usable as context manager or decorator. *(unverified)* - + diff --git a/patterns/modern/context_manager/docs/fundamentals.md b/patterns/modern/context_manager/docs/fundamentals.md index cfd2e0d..ef4b890 100644 --- a/patterns/modern/context_manager/docs/fundamentals.md +++ b/patterns/modern/context_manager/docs/fundamentals.md @@ -34,6 +34,14 @@ Before `with`, the guarantee was hand-written `try/finally` at every call site — correct, and unscalable: ```python +class Resource: + def __init__(self, name: str, log: list[str]) -> None: + self.name, self.log = name, log + + def close(self) -> None: + self.log.append(f"closed {self.name}") + + def use_two(log: list[str]) -> None: first = Resource("a", log) try: diff --git a/patterns/modern/context_manager/examples/atomic_deploy/deploy.py b/patterns/modern/context_manager/examples/atomic_deploy/deploy.py index 8594888..0972981 100644 --- a/patterns/modern/context_manager/examples/atomic_deploy/deploy.py +++ b/patterns/modern/context_manager/examples/atomic_deploy/deploy.py @@ -4,7 +4,9 @@ release as a whole is made transactional with ``ExitStack``: every written file pushes a rollback callback, and only a fully validated release pops them off uncalled — the commit *is* ``pop_all()``. Any exception on the way -unwinds the stack, restoring every file already touched. +unwinds the stack, restoring every file already touched. (Each rollback +targets its own file, so the LIFO unwind order is deliberately not +load-bearing here; what matters is that every callback runs.) """ from __future__ import annotations diff --git a/patterns/modern/context_manager/tests/test_managers.py b/patterns/modern/context_manager/tests/test_managers.py index 6f2c8f0..4aca9cc 100644 --- a/patterns/modern/context_manager/tests/test_managers.py +++ b/patterns/modern/context_manager/tests/test_managers.py @@ -2,6 +2,7 @@ from __future__ import annotations +import os from pathlib import Path import pytest @@ -32,6 +33,35 @@ def test_exception_on_a_fresh_path_leaves_nothing(self, tmp_path: Path) -> None: assert not target.exists() assert list(tmp_path.iterdir()) == [] # no orphaned temp file either + def test_commit_is_one_atomic_replace_from_the_same_directory( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """The docstring's old-or-new promise rests on os.replace, and on the + temp file living beside the target (same filesystem, no EXDEV).""" + calls: list[tuple[str, Path]] = [] + real_replace = os.replace + + def spy(src: str | os.PathLike[str], dst: str | os.PathLike[str]) -> None: + calls.append((str(src), Path(dst))) + real_replace(src, dst) + + monkeypatch.setattr(os, "replace", spy) + target = tmp_path / "config.toml" + with AtomicWrite(target) as handle: + handle.write("v2") + assert target.read_text() == "v2" + assert len(calls) == 1 # exactly one atomic rename — never a rewrite + src, dst = calls[0] + assert dst == target + assert Path(src).parent == target.parent # beside the target, not /tmp + + def test_non_default_encoding_is_honored(self, tmp_path: Path) -> None: + target = tmp_path / "latin.txt" + with AtomicWrite(target, encoding="latin-1") as handle: + handle.write("café") + assert target.read_text(encoding="latin-1") == "café" + assert target.read_bytes() == b"caf\xe9" # actually latin-1, not utf-8 + class TestTemporarily: class Settings: diff --git a/patterns/modern/dependency_injection/docs/implementation.md b/patterns/modern/dependency_injection/docs/implementation.md index dde2e7e..bd6038f 100644 --- a/patterns/modern/dependency_injection/docs/implementation.md +++ b/patterns/modern/dependency_injection/docs/implementation.md @@ -33,10 +33,23 @@ out of the class and pass the result in. lambda, capture mail in a list; assert on behavior, not on mocks' innards. ```python -from patterns.modern.dependency_injection import ReminderService +from datetime import date +from patterns.modern.dependency_injection import Invoice, ReminderService +from patterns.modern.dependency_injection.examples.invoice_reminders import ( + ConsoleMail, + InMemoryInvoices, +) + +source = InMemoryInvoices( + [ + Invoice("INV-1", "ada@example.com", 120_00, date(2026, 8, 1)), + Invoice("INV-3", "sam@example.com", 60_00, date(2026, 7, 20)), + ] +) +outbox = ConsoleMail() service = ReminderService(invoices=source, mail=outbox, today=lambda: date(2026, 8, 27)) -assert service.send_reminders() == ["INV-1"] +assert service.send_reminders() == ["INV-1", "INV-3"] ``` ## Python idioms that keep it small diff --git a/patterns/modern/dependency_injection/tests/test_service.py b/patterns/modern/dependency_injection/tests/test_service.py index 2289762..703c000 100644 --- a/patterns/modern/dependency_injection/tests/test_service.py +++ b/patterns/modern/dependency_injection/tests/test_service.py @@ -17,10 +17,10 @@ def unpaid(self) -> list[Invoice]: class CapturingMail: def __init__(self) -> None: - self.outbox: list[tuple[str, str]] = [] + self.outbox: list[tuple[str, str, str]] = [] def send(self, to: str, subject: str, body: str) -> None: - self.outbox.append((to, subject)) + self.outbox.append((to, subject, body)) def invoice(number: str, due: date, email: str = "ada@example.com") -> Invoice: @@ -37,7 +37,16 @@ def test_overdue_past_grace_is_reminded(self) -> None: mail = CapturingMail() reminded = self.service([invoice("INV-1", date(2026, 8, 1))], mail).send_reminders() assert reminded == ["INV-1"] - assert mail.outbox == [("ada@example.com", "Invoice INV-1 is 26 days overdue")] + assert mail.outbox == [ + ("ada@example.com", "Invoice INV-1 is 26 days overdue", "Please pay 100.00.") + ] # cents rendered as currency, not 10000 + + def test_grace_days_parameter_actually_widens_the_grace(self) -> None: + mail = CapturingMail() + overdue_26_days = invoice("INV-1", date(2026, 8, 1)) + service = self.service([overdue_26_days], mail) + assert service.send_reminders(grace_days=30) == [] # 26 < 30: quiet + assert service.send_reminders(grace_days=25) == ["INV-1"] # 26 > 25 def test_exactly_at_grace_is_not_reminded(self) -> None: mail = CapturingMail() diff --git a/patterns/modern/registry/docs/implementation.md b/patterns/modern/registry/docs/implementation.md index 8d4a481..a9e9a7c 100644 --- a/patterns/modern/registry/docs/implementation.md +++ b/patterns/modern/registry/docs/implementation.md @@ -36,8 +36,13 @@ gets re-decided (differently) at every ladder in the codebase. the honest place — with a comment saying the import is load-bearing. ```python +from collections.abc import Callable + from patterns.modern.registry import Registry +Rows = list[dict[str, str]] +Exporter = Callable[[Rows], str] + EXPORTERS: Registry[Exporter] = Registry(kind="format") diff --git a/patterns/modern/registry/pattern/registry.py b/patterns/modern/registry/pattern/registry.py index aaf56d9..7610947 100644 --- a/patterns/modern/registry/pattern/registry.py +++ b/patterns/modern/registry/pattern/registry.py @@ -36,7 +36,7 @@ def register(self, name: str, *, replace: bool = False) -> Callable[[T], T]: def decorator(entry: T) -> T: if name in self._entries and not replace: - raise ValueError(f"{self._kind} {name!r} is already registered") + raise ValueError(f"{self._kind} {name!r} is already registered (pass replace=True)") self._entries[name] = entry return entry diff --git a/patterns/modern/registry/tests/test_registry.py b/patterns/modern/registry/tests/test_registry.py index 7f74174..74cc582 100644 --- a/patterns/modern/registry/tests/test_registry.py +++ b/patterns/modern/registry/tests/test_registry.py @@ -60,3 +60,13 @@ def test_introspection_surface(self) -> None: assert registry.names() == ("a", "b") assert "a" in registry and "z" not in registry assert len(registry) == 2 + + +class TestDefaultKind: + def test_default_kind_names_entries_in_errors(self) -> None: + registry: Registry[str] = Registry() # no kind given + registry.register("x")("value") + with pytest.raises(ValueError, match="entry 'x' is already registered"): + registry.register("x")("other") + with pytest.raises(UnknownKeyError, match="unknown entry 'y'"): + registry.get("y") diff --git a/patterns/modern/repository/docs/implementation.md b/patterns/modern/repository/docs/implementation.md index 1a68315..8772f17 100644 --- a/patterns/modern/repository/docs/implementation.md +++ b/patterns/modern/repository/docs/implementation.md @@ -33,7 +33,9 @@ tested — or changed — without dragging storage along. `repo: Invoices` stay importable, testable, and driver-free. ```python -from patterns.modern.repository import InMemoryInvoices, total_owed +from datetime import date + +from patterns.modern.repository import InMemoryInvoices, Invoice, total_owed repo = InMemoryInvoices() repo.add(Invoice("INV-1", "ada", 120_00, date(2026, 8, 1))) @@ -64,6 +66,8 @@ assert total_owed(repo, "ada") == 120_00 an aggregate, not a schema. - **Transactions smeared across repositories.** Commit/rollback is its own seam (unit of work); bolting `commit()` onto each repository hides it. + (The mini-project's sqlite adapter commits per write so its durability + claim stays true at demo scale; a production design lifts commit here.) ## Worked example diff --git a/patterns/modern/repository/examples/invoice_ledger/sqlite_repo.py b/patterns/modern/repository/examples/invoice_ledger/sqlite_repo.py index c6813b6..d3dd272 100644 --- a/patterns/modern/repository/examples/invoice_ledger/sqlite_repo.py +++ b/patterns/modern/repository/examples/invoice_ledger/sqlite_repo.py @@ -13,7 +13,12 @@ class SqliteInvoices: - """An ``Invoices`` adapter over sqlite3 (stdlib, durable when given a path).""" + """An ``Invoices`` adapter over sqlite3 (stdlib, durable when given a path). + + This demo commits per write so the durability claim is true of the code; + production designs often lift commit into a unit-of-work seam instead + (see the pitfalls in ``docs/implementation.md``). + """ def __init__(self, conn: sqlite3.Connection) -> None: self._conn = conn @@ -21,12 +26,17 @@ def __init__(self, conn: sqlite3.Connection) -> None: "CREATE TABLE IF NOT EXISTS invoices" " (number TEXT PRIMARY KEY, customer TEXT, amount_cents INT, due TEXT)" ) + self._conn.commit() def add(self, invoice: Invoice) -> None: - self._conn.execute( - "INSERT INTO invoices VALUES (?, ?, ?, ?)", - (invoice.number, invoice.customer, invoice.amount_cents, invoice.due.isoformat()), - ) + try: + self._conn.execute( + "INSERT INTO invoices VALUES (?, ?, ?, ?)", + (invoice.number, invoice.customer, invoice.amount_cents, invoice.due.isoformat()), + ) + except sqlite3.IntegrityError: + raise ValueError(f"invoice {invoice.number!r} already exists") from None + self._conn.commit() def for_customer(self, customer: str) -> list[Invoice]: rows = self._conn.execute( diff --git a/patterns/modern/repository/pattern/ledger.py b/patterns/modern/repository/pattern/ledger.py index ea60dba..8fd85f3 100644 --- a/patterns/modern/repository/pattern/ledger.py +++ b/patterns/modern/repository/pattern/ledger.py @@ -25,7 +25,11 @@ class Invoice: class Invoices(Protocol): - """The port: what the domain may ask of invoice storage.""" + """The port: what the domain may ask of invoice storage. + + Contract (held by the shared tests): ``add`` refuses a duplicate invoice + number with ``ValueError``; ``list_all`` returns insertion order. + """ def add(self, invoice: Invoice) -> None: ... @@ -41,6 +45,8 @@ def __init__(self) -> None: self._items: list[Invoice] = [] def add(self, invoice: Invoice) -> None: + if any(existing.number == invoice.number for existing in self._items): + raise ValueError(f"invoice {invoice.number!r} already exists") self._items.append(invoice) def for_customer(self, customer: str) -> list[Invoice]: diff --git a/patterns/modern/repository/tests/test_invoice_ledger.py b/patterns/modern/repository/tests/test_invoice_ledger.py index 2b6bbd0..2430ccc 100644 --- a/patterns/modern/repository/tests/test_invoice_ledger.py +++ b/patterns/modern/repository/tests/test_invoice_ledger.py @@ -10,6 +10,7 @@ import sqlite3 from collections.abc import Iterator from datetime import date +from pathlib import Path import pytest @@ -49,10 +50,16 @@ def test_for_customer_filters(self, repo: Invoices) -> None: repo.add(Invoice("INV-2", "grace", 200, TODAY)) assert [i.number for i in repo.for_customer("grace")] == ["INV-2"] - def test_list_all_returns_everything(self, repo: Invoices) -> None: + def test_list_all_returns_everything_in_insertion_order(self, repo: Invoices) -> None: + repo.add(Invoice("INV-2", "grace", 200, TODAY)) # non-alphabetical on purpose repo.add(Invoice("INV-1", "ada", 100, TODAY)) - repo.add(Invoice("INV-2", "grace", 200, TODAY)) - assert {i.number for i in repo.list_all()} == {"INV-1", "INV-2"} + assert [i.number for i in repo.list_all()] == ["INV-2", "INV-1"] + + def test_duplicate_invoice_numbers_are_refused(self, repo: Invoices) -> None: + repo.add(Invoice("INV-1", "ada", 100, TODAY)) + with pytest.raises(ValueError, match="INV-1"): + repo.add(Invoice("INV-1", "grace", 999, TODAY)) + assert len(repo.list_all()) == 1 # the original survives, nothing half-added def test_domain_logic_cannot_tell_the_adapters_apart(self, repo: Invoices) -> None: repo.add(Invoice("INV-1", "ada", 120_00, date(2026, 8, 1))) @@ -61,6 +68,21 @@ def test_domain_logic_cannot_tell_the_adapters_apart(self, repo: Invoices) -> No assert [i.number for i in overdue(repo, TODAY)] == ["INV-1"] +class TestSqliteDurability: + def test_writes_survive_closing_and_reopening_the_file(self, tmp_path: Path) -> None: + db = tmp_path / "ledger.db" + conn = sqlite3.connect(db) + SqliteInvoices(conn).add(Invoice("INV-1", "ada", 120_00, date(2026, 8, 1))) + conn.close() + + reopened = sqlite3.connect(db) + try: + rows = SqliteInvoices(reopened).list_all() + finally: + reopened.close() + assert [i.number for i in rows] == ["INV-1"] # durable, as the docstring claims + + class TestDemo: def test_main_reports_identical_answers_from_both_backends( self, capsys: pytest.CaptureFixture[str] diff --git a/pyproject.toml b/pyproject.toml index 07b2c0d..f2643ce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,6 +37,7 @@ dev = [ "mypy>=1.13", "types-pyyaml>=6.0", "pytest-asyncio>=0.24", + "pytest-timeout>=2.4.0", ] [build-system] @@ -64,6 +65,7 @@ testpaths = ["tests", "patterns"] pythonpath = ["."] asyncio_mode = "auto" addopts = "-q --cov=src --cov=patterns --cov-report=term-missing" +timeout = 30 # a shutdown regression must fail CI, not hang it [tool.coverage.report] skip_empty = true