Define the 'published feed' predicate once and add a repository conformance test #29

New issue

Closed

opened 2026-07-01 14:49:30 +00:00 by rosa · 0 comments

rosa commented

2026-07-01 14:49:30 +00:00

Owner

Problem

The ADR-0004 rule for "a live, public, published Post" exists three times, in three languages:

SQL — crates/infra/src/repositories/postgres.rs WHERE clauses (find_posts_published_by_user ~804-808, find_drafts_by_user ~783-786, find_posts_published_with_tag ~827-831). The drafts query even hand-encodes status = 'draft' OR published_at IS NULL OR published_at > NOW().
Mock Rust filter — crates/domain/src/mocks.rs (find_posts_published_by_user ~407, find_drafts_by_user ~380, find_posts_published_with_tag ~435-437), using post.is_live(now) && post.visibility().is_public().
The model — crates/domain/src/models/post.rs: is_live (:442) and is_viewable_by (:449) already define liveness/visibility.

The two AppRepository adapters are supposed to be behaviourally identical, but nothing enforces it: the mock filters with the domain's own is_live, while Postgres re-expresses the same rule as free-form SQL. They can silently drift. A change to what "published" means is a three-file edit with no test that would catch a mismatch.

Deletion test: you can't delete the SQL — but you can delete the duplicated policy. If the predicate were one named concept the SQL and mock both had to satisfy, deleting the ad-hoc copies would concentrate the rule, not lose it.

Solution

Two moves, either or both:

Name the collections. "The published feed" and "drafts & scheduled" are domain concepts (already named in CONTEXT.md: Published, Scheduled, Draft). Make the predicate a single domain function and have the mock filter through it, so the model and the test adapter share one definition rather than two.
Add a repository conformance test. One shared test suite run against both PostgresAppRepository and MemoryAppRepo pins the two adapters to identical behaviour — the "interface is the test surface" principle applied to a seam that currently has two untethered adapters.

Benefits

Locality: the definition of "published" changes in one place; the SQL and the mock are checked against it rather than re-stating it.
Testability: a conformance suite turns the mock from "a thing that might behave like Postgres" into "a thing proven to." Today the mock's filters are untested against real SQL.
Confidence in fast tests: the whole point of MemoryAppRepo is that service tests don't need a database — that payoff is only sound if the two adapters agree.

Constraints preserved

ADR-0004 (post viewability) becomes the single source of truth it is meant to be, rather than being re-stated per adapter.

Architecture review candidate 2 of 4 (Worth exploring). Cheap, high-confidence — a conformance test is almost pure upside.

## Problem The ADR-0004 rule for "a live, public, published Post" exists three times, in three languages: - **SQL** — `crates/infra/src/repositories/postgres.rs` WHERE clauses (`find_posts_published_by_user` ~804-808, `find_drafts_by_user` ~783-786, `find_posts_published_with_tag` ~827-831). The drafts query even hand-encodes `status = 'draft' OR published_at IS NULL OR published_at > NOW()`. - **Mock Rust filter** — `crates/domain/src/mocks.rs` (`find_posts_published_by_user` ~407, `find_drafts_by_user` ~380, `find_posts_published_with_tag` ~435-437), using `post.is_live(now) && post.visibility().is_public()`. - **The model** — `crates/domain/src/models/post.rs`: `is_live` (:442) and `is_viewable_by` (:449) already define liveness/visibility. The two `AppRepository` adapters are supposed to be behaviourally identical, but nothing enforces it: the mock filters with the domain's own `is_live`, while Postgres re-expresses the same rule as free-form SQL. They can silently drift. A change to what "published" means is a three-file edit with no test that would catch a mismatch. **Deletion test:** you can't delete the SQL — but you *can* delete the duplicated *policy*. If the predicate were one named concept the SQL and mock both had to satisfy, deleting the ad-hoc copies would concentrate the rule, not lose it. ## Solution Two moves, either or both: 1. **Name the collections.** "The published feed" and "drafts & scheduled" are domain concepts (already named in `CONTEXT.md`: Published, Scheduled, Draft). Make the predicate a single domain function and have the mock filter through it, so the model and the test adapter share one definition rather than two. 2. **Add a repository conformance test.** One shared test suite run against both `PostgresAppRepository` and `MemoryAppRepo` pins the two adapters to identical behaviour — the "interface is the test surface" principle applied to a seam that currently has two untethered adapters. ## Benefits - **Locality:** the definition of "published" changes in one place; the SQL and the mock are checked against it rather than re-stating it. - **Testability:** a conformance suite turns the mock from "a thing that might behave like Postgres" into "a thing proven to." Today the mock's filters are untested against real SQL. - **Confidence in fast tests:** the whole point of `MemoryAppRepo` is that service tests don't need a database — that payoff is only sound if the two adapters agree. ## Constraints preserved - ADR-0004 (post viewability) becomes the single source of truth it is meant to be, rather than being re-stated per adapter. _Architecture review candidate 2 of 4 (Worth exploring). Cheap, high-confidence — a conformance test is almost pure upside._

rosa referenced this issue from a commit

2026-07-01 16:14:01 +00:00

refactor: Name the published-feed predicate and add repository conformance tests

rosa closed this issue

2026-07-01 16:14:01 +00:00

rosa referenced this issue

2026-07-03 21:46:25 +00:00

test: Extract shared conformance-test scaffolding into a common module #78