From d8ef3255033932c4c01de73e91d6aaa5413ba57c Mon Sep 17 00:00:00 2001 From: KaliCZ Date: Thu, 16 Jul 2026 18:10:52 +0200 Subject: [PATCH 1/4] ADRs are living documents, not a change history Fold the single-project decision back into ADR-0001 and delete ADR-0009: an ADR is rewritten in place when its decision changes (git holds the history), never marked superseded or replaced by a new number. The rule is stated in docs/adr/README.md and CLAUDE.md. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 11 +++- ...0001-vertical-slices-and-project-layout.md | 58 ++++++++++--------- docs/adr/0009-single-api-project.md | 51 ---------------- docs/adr/README.md | 20 +++++-- docs/technical-requirements.md | 5 +- 5 files changed, 57 insertions(+), 88 deletions(-) delete mode 100644 docs/adr/0009-single-api-project.md diff --git a/CLAUDE.md b/CLAUDE.md index 5b8f55f..1dfe010 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,7 +12,7 @@ governs it: | ---------------------------------- | ---------- | | Anything at all | [docs/technical-requirements.md](docs/technical-requirements.md) | | User-facing behavior | [docs/business-requirements.md](docs/business-requirements.md) | -| A decision that feels reversible | The matching ADR in [docs/adr/](docs/adr/README.md) — supersede, never edit | +| A decision that feels reversible | The matching ADR in [docs/adr/](docs/adr/README.md) | | Validation, DTOs, domain types | [ADR-0002](docs/adr/0002-validation-lives-in-the-type-system.md), [ADR-0003](docs/adr/0003-business-errors-are-result-enums.md) | | API surface / frontend types | [ADR-0004](docs/adr/0004-openapi-is-the-frontend-contract.md) | | Auth | [ADR-0005](docs/adr/0005-auth-zitadel-oidc-pkce.md) | @@ -21,6 +21,13 @@ governs it: If a change contradicts a doc, change the doc in the same PR — or don't make the change. +**ADRs are the current design, never a change history.** Every ADR describes +how the application works *now*. Changing or rolling back a decision means +rewriting the matching ADR to the new thinking, in the same PR — git holds +the history. Never mark an ADR superseded, and never add a new ADR for an +aspect an existing one covers; new numbers are only for new aspects. A +rolled-back approach usually earns a line under "Alternatives considered". + ## Commands ```shell @@ -44,7 +51,7 @@ restart the AppHost. 1. **Vertical slices.** A feature lives in `Features//` — one folder, everything it owns inside: entities, handlers + error enums, controller, - DTOs, mapping (ADR-0009). This applies to infrastructure too: one concern + DTOs, mapping (ADR-0001). This applies to infrastructure too: one concern per file (`Infrastructure/RateLimits.cs`, `Health.cs`, …) with `Configure(...)`/`Use(...)` statics. No grab-bag `ServiceExtensions`. Entities and handlers still never touch HTTP — `HttpContext`, action diff --git a/docs/adr/0001-vertical-slices-and-project-layout.md b/docs/adr/0001-vertical-slices-and-project-layout.md index fc05ce5..cb42b34 100644 --- a/docs/adr/0001-vertical-slices-and-project-layout.md +++ b/docs/adr/0001-vertical-slices-and-project-layout.md @@ -1,6 +1,6 @@ -# ADR-0001 — Vertical feature slices in two projects; controllers, no MediatR +# ADR-0001 — Vertical feature slices in one API project; controllers, no MediatR -**Status:** Superseded by [ADR-0009](0009-single-api-project.md) (2026-07-16) +**Status:** Accepted (2026-07-15, revised 2026-07-16) ## Context @@ -8,51 +8,53 @@ The codebase has two jobs: demonstrate StrongTypes end to end, and serve as a starter template. Both jobs punish accidental complexity. A classic onion (`Api` → `Application` → `Domain` → `Infrastructure` + repositories + MediatR) scatters one feature across four projects and buries the -StrongTypes story under ceremony; a single flat project loses the -compiler-enforced rule that HTTP concerns never leak into business code. +StrongTypes story under ceremony — and even a two-project API/domain split +keeps every slice half-complete, with the feature folder always missing its +key domain objects. ## Decision -Two application projects with one compiler-enforced dependency edge, both +One backend project, `ProductReviews.Api` (next to the Aspire AppHost), organized **by feature, not by layer**: -- **`ProductReviews.Api`** — the HTTP boundary. `Features//` holds - the controller, its request/response DTOs, and the mapping between DTOs and - domain models. `Infrastructure/` holds one file per cross-cutting concern - (`Authentication.cs`, `OpenApi.cs`, `RateLimits.cs`, …), each a static class - with `Configure(...)`/`Use(...)` — `Program.cs` is a thin orchestrator and - there is no grab-bag `ServiceCollectionExtensions`. -- **`ProductReviews.Domain`** — everything else. `/` folders hold the - entities and one file per operation (`SubmitReview.cs` = handler class + - its error enum). `Persistence/` holds the `DbContext`, entity - configurations, migrations, and the seeder. EF Core is used directly as a - library — no repository interfaces wrapping it. +- **`Features//`** holds the whole slice: entities, handlers with + their error enums, the controller, request/response DTOs, and the mapping + between them. +- **`Persistence/`** holds the `DbContext`, entity configurations, migrations, + and the seeder. EF Core is used directly as a library — no repository + interfaces wrapping it. +- **`Infrastructure/`** holds one file per cross-cutting concern + (`Authentication.cs`, `OpenApi.cs`, `RateLimits.cs`, `Observability.cs`, + `Health.cs`, …), each a static class with `Configure(...)`/`Use(...)` — + `Program.cs` is a thin orchestrator and there is no grab-bag + `ServiceCollectionExtensions`. +- **HTTP stays out of the domain by review, not by compiler.** Entities and + handlers never touch `HttpContext`, action results, or DTOs; those live + only in controllers. - **Controllers, not minimal APIs** — controller classes group a feature's endpoints, give Swashbuckle first-class metadata, and keep binding, auth attributes, and `ProducesResponseType` declarations in one reviewable place per feature. - **No MediatR / no dispatch layer.** Controllers inject concrete handler - classes. A handler is a class with one public method; DI registration is a - per-project `AddDomain()` extension. + classes. A handler is a class with one public method; DI registration is + `Infrastructure/DomainServices.cs`. - **DTOs are a dedicated API-layer contract.** Domain entities and read models are never serialized. Handlers return domain models (`Result`); the controller maps success models - to response DTOs and error enums to HTTP — both directions live in the API - slice. + to response DTOs and error enums to HTTP. - **Read paths return proof-of-loading models, not entities.** A query handler owns a single query method with its `Include` chain and projects into an immutable record whose constructor requires every loaded - relationship (e.g. `ReviewWithViewerContext`). If it isn't loaded, the + relationship (e.g. `ReviewWithVotes`). If it isn't loaded, the model cannot be constructed — no lazy-loading surprises, no possibly-null navigations downstream. ## Consequences -- A feature is one folder in each of two projects; deleting a feature is - deleting two folders. Review diffs stay feature-local. -- The `Api → Domain` project reference makes "domain never sees HTTP" - structural; the missing reverse reference makes "HTTP never bypasses the - domain" reviewable at a glance. +- A feature is one folder; deleting a feature is deleting one folder plus its + EF configuration. Review diffs stay feature-local. +- "Domain never sees HTTP" needs review attention: no project edge enforces + it, only the rule that DTO mapping happens exclusively in controllers. - No runtime dispatch magic: navigation is F12, stack traces are honest, and the template stays approachable for readers new to the stack. - The cost of not abstracting EF: swapping the persistence technology means @@ -64,6 +66,10 @@ organized **by feature, not by layer**: - **Onion/Clean Architecture with repositories** — rejected: doubles the file count per feature and hides the `UseStrongTypes()` EF integration behind interfaces, which is the opposite of a showcase. +- **A separate domain project with a compiler-enforced dependency edge** — + tried first and rolled back: every feature lived in two half-folders, the + slice never owned its domain objects, and the edge protected a boundary + that review can hold. - **Minimal APIs** — rejected: endpoint metadata and per-route filters get re-invented per endpoint, and the controller + `ModelState` + `ValidationProblem` pipeline is exactly what the StrongTypes ASP.NET diff --git a/docs/adr/0009-single-api-project.md b/docs/adr/0009-single-api-project.md deleted file mode 100644 index 3f8eab1..0000000 --- a/docs/adr/0009-single-api-project.md +++ /dev/null @@ -1,51 +0,0 @@ -# ADR-0009 — One API project owns the domain; a feature folder owns everything - -**Status:** Accepted (2026-07-16) — supersedes [ADR-0001](0001-vertical-slices-and-project-layout.md) - -## Context - -ADR-0001 split the backend into `ProductReviews.Api` and `ProductReviews.Domain` -(plus a `ServiceDefaults` library), using the project reference to enforce -"domain never sees HTTP". In practice the split fought the vertical slices it -was meant to serve: every feature lived in two half-folders — entities and -handlers in one project, controller and DTOs in another — so the slice that -owned a feature was always missing its key domain objects. For a kick-start -template, two extra projects are ceremony that buries the StrongTypes story. - -## Decision - -One backend project, `ProductReviews.Api`, next to the AppHost: - -- **`Features//`** holds the *whole* slice: entities, handlers with - their error enums, the controller, request/response DTOs, and the mapping - between them. -- **`Persistence/`** holds the `DbContext`, entity configurations, migrations, - and the seeder. -- **`Infrastructure/`** holds one file per cross-cutting concern — including - `Observability.cs` (OpenTelemetry) and `Health.cs` (health checks), which - absorbed everything `ProductReviews.ServiceDefaults` used to provide. That - project is gone. - -Everything else in ADR-0001 carries forward unchanged: organized by feature -rather than layer, controllers not minimal APIs, no MediatR, DTOs as a -dedicated API-layer contract, proof-of-loading read models. - -## Consequences - -- A feature is one folder; deleting a feature is deleting one folder plus its - EF configuration. The slice is complete — nothing about a feature lives at - arm's length. -- "Domain never sees HTTP" is now a review rule, not a compiler rule: entities - and handlers must not touch `HttpContext`, action results, or DTOs. The - DTO mapping still happens only in controllers. -- Unit tests reference the API project directly - (`tests/ProductReviews.Api.UnitTests`, formerly `ProductReviews.Domain.Tests`). - -## Alternatives considered - -- **Keeping the two-project split** — rejected: the compiler-enforced edge - protected a boundary that review can hold, at the cost of scattering every - slice across projects. -- **Architecture tests to re-enforce the boundary** — rejected: an extra - harness is exactly the ceremony this ADR removes; the DTO rule plus review - suffice at this scale. diff --git a/docs/adr/README.md b/docs/adr/README.md index 68335e6..362854b 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -2,17 +2,26 @@ Short records of the decisions that shape how a specific aspect of this application is done — the ones a newcomer would otherwise question or -accidentally reverse. ADRs are **not** a change history: everything routine -lives in [technical-requirements.md](../technical-requirements.md). +accidentally reverse. Everything routine lives in +[technical-requirements.md](../technical-requirements.md). + +Two hard rules: + +- **ADRs are not a change history.** The set of ADRs is always the up-to-date + description of the current design; every file states how the application + works *now*. +- **When a decision changes, edit its ADR in place** — in the same PR as the + change — so it reflects the new thinking. Git holds the history; there are + no "superseded" ADRs, and a changed decision never gets a new number. A new + number is only for an aspect no existing ADR covers. A rolled-back approach + usually earns a line under "Alternatives considered". Format: **Status / Context / Decision / Consequences** (plus **Alternatives considered** where the rejected option is instructive). -An ADR is immutable once Accepted — change course by adding a superseding ADR, -never by editing an old one. | # | Decision | Status | | ---- | ------------------------------------------------------------------------------------------ | -------- | -| 0001 | [Vertical feature slices in two projects; controllers, no MediatR](0001-vertical-slices-and-project-layout.md) | Superseded by 0009 | +| 0001 | [Vertical feature slices in one API project; controllers, no MediatR](0001-vertical-slices-and-project-layout.md) | Accepted | | 0002 | [Validation lives in the type system, not in annotations or guards](0002-validation-lives-in-the-type-system.md) | Accepted | | 0003 | [Business failures are enum values in `Result`, not exceptions](0003-business-errors-are-result-enums.md) | Accepted | | 0004 | [The OpenAPI document is the frontend contract](0004-openapi-is-the-frontend-contract.md) | Accepted | @@ -20,4 +29,3 @@ never by editing an old one. | 0006 | [Seed data runs at startup, never inside migrations](0006-seeding-at-startup-not-migrations.md) | Accepted | | 0007 | [Tests exercise real dependencies; nothing is mocked](0007-tests-use-real-dependencies.md) | Accepted | | 0008 | [The frontend is a Vue 3 + Vite SPA, not a server-rendered app](0008-frontend-vue-spa.md) | Accepted | -| 0009 | [One API project owns the domain; a feature folder owns everything](0009-single-api-project.md) | Accepted | diff --git a/docs/technical-requirements.md b/docs/technical-requirements.md index a19a1e9..d3d08e3 100644 --- a/docs/technical-requirements.md +++ b/docs/technical-requirements.md @@ -51,7 +51,7 @@ flowchart LR ├── docs/ # this spec: requirements + ADRs ├── src/ │ ├── ProductReviews.AppHost/ # Aspire orchestration + Zitadel provisioning -│ └── ProductReviews.Api/ # the whole backend (ADR-0009) +│ └── ProductReviews.Api/ # the whole backend (ADR-0001) │ ├── Features/ # a folder owns its entire slice │ │ ├── Catalog/ # Product entity + queries + controller + DTOs │ │ ├── Reviews/ # Review, Rating, handlers + controller + DTOs @@ -77,8 +77,7 @@ flowchart LR Each of these is an ADR; the numbered file is the authority: - Feature slices in one API project, controllers, no MediatR, DTO layer owned - by the API, proof-of-loading read models — [ADR-0009](adr/0009-single-api-project.md) - (supersedes [ADR-0001](adr/0001-vertical-slices-and-project-layout.md)) + by the API, proof-of-loading read models — [ADR-0001](adr/0001-vertical-slices-and-project-layout.md) - Strong types are the only validation; no annotations, no guards — [ADR-0002](adr/0002-validation-lives-in-the-type-system.md) - `Result` + error enums; controllers map to HTTP — [ADR-0003](adr/0003-business-errors-are-result-enums.md) - OpenAPI is the frontend contract; client generated + drift-checked — [ADR-0004](adr/0004-openapi-is-the-frontend-contract.md) From d847e0f82c2dc6b4565799f97a614c99d642d044 Mon Sep 17 00:00:00 2001 From: KaliCZ Date: Thu, 16 Jul 2026 22:52:33 +0200 Subject: [PATCH 2/4] Consolidate ADRs: error enums fold into 0001, OpenAPI contract into 0002 One aspect, one ADR: 0001 now carries the whole feature-slice story (layout, controllers, Result error enums, DTO layer); 0002 carries the whole constraint pipeline (strong types -> EF -> OpenAPI -> generated frontend client). 0003 and 0004 are deleted; numbers are identity, so the gaps stay. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 4 +- ...0001-vertical-slices-and-project-layout.md | 47 +++++++++++++---- ...002-validation-lives-in-the-type-system.md | 52 ++++++++++++++----- .../0003-business-errors-are-result-enums.md | 38 -------------- .../0004-openapi-is-the-frontend-contract.md | 40 -------------- docs/adr/0008-frontend-vue-spa.md | 2 +- docs/adr/README.md | 8 +-- docs/technical-requirements.md | 18 +++---- frontend/src/api/client.ts | 2 +- frontend/tests/e2e/contract.spec.ts | 2 +- .../Infrastructure/ErrorHandling.cs | 2 +- .../Infrastructure/OpenApi.cs | 2 +- 12 files changed, 94 insertions(+), 123 deletions(-) delete mode 100644 docs/adr/0003-business-errors-are-result-enums.md delete mode 100644 docs/adr/0004-openapi-is-the-frontend-contract.md diff --git a/CLAUDE.md b/CLAUDE.md index 1dfe010..1d2466f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,8 +13,8 @@ governs it: | Anything at all | [docs/technical-requirements.md](docs/technical-requirements.md) | | User-facing behavior | [docs/business-requirements.md](docs/business-requirements.md) | | A decision that feels reversible | The matching ADR in [docs/adr/](docs/adr/README.md) | -| Validation, DTOs, domain types | [ADR-0002](docs/adr/0002-validation-lives-in-the-type-system.md), [ADR-0003](docs/adr/0003-business-errors-are-result-enums.md) | -| API surface / frontend types | [ADR-0004](docs/adr/0004-openapi-is-the-frontend-contract.md) | +| Validation, DTOs, domain types | [ADR-0002](docs/adr/0002-validation-lives-in-the-type-system.md), [ADR-0001](docs/adr/0001-vertical-slices-and-project-layout.md) | +| API surface / frontend types | [ADR-0002](docs/adr/0002-validation-lives-in-the-type-system.md) | | Auth | [ADR-0005](docs/adr/0005-auth-zitadel-oidc-pkce.md) | | Tests | [ADR-0007](docs/adr/0007-tests-use-real-dependencies.md) | diff --git a/docs/adr/0001-vertical-slices-and-project-layout.md b/docs/adr/0001-vertical-slices-and-project-layout.md index cb42b34..7d758fc 100644 --- a/docs/adr/0001-vertical-slices-and-project-layout.md +++ b/docs/adr/0001-vertical-slices-and-project-layout.md @@ -1,4 +1,4 @@ -# ADR-0001 — Vertical feature slices in one API project; controllers, no MediatR +# ADR-0001 — Vertical feature slices: one API project, controllers, `Result` error enums **Status:** Accepted (2026-07-15, revised 2026-07-16) @@ -12,6 +12,12 @@ StrongTypes story under ceremony — and even a two-project API/domain split keeps every slice half-complete, with the feature folder always missing its key domain objects. +Within a slice, domain operations can fail for legitimate business reasons: +the product does not exist, the reviewer already reviewed it, you cannot vote +on your own review. Modeling those as exceptions hides them from the +compiler — a caller cannot see from the signature which failures exist, and +controllers end up with `try/catch` ladders kept in sync by hand. + ## Decision One backend project, `ProductReviews.Api` (next to the Aspire AppHost), @@ -31,28 +37,42 @@ organized **by feature, not by layer**: - **HTTP stays out of the domain by review, not by compiler.** Entities and handlers never touch `HttpContext`, action results, or DTOs; those live only in controllers. -- **Controllers, not minimal APIs** — controller classes group a feature's - endpoints, give Swashbuckle first-class metadata, and keep binding, - auth attributes, and `ProducesResponseType` declarations in one reviewable - place per feature. -- **No MediatR / no dispatch layer.** Controllers inject concrete handler - classes. A handler is a class with one public method; DI registration is - `Infrastructure/DomainServices.cs`. +- **Handlers return `Result`, never throw for business + outcomes.** `TError` is a per-operation enum (`SubmitReviewError`, + `CastVoteError`, …); success and error are returned by implicit conversion + (`return review;` / `return SubmitReviewError.ProductNotFound;`). + Exceptions are reserved for bugs and infrastructure faults. +- **Controllers own the HTTP translation.** A controller consumes the result + with `result.Error is { } error` and maps each enum value in an exhaustive + `switch` (no `default` arm — a new enum value must break the build) to the + proper response: `ValidationProblem` with a field-scoped `ModelState` error + for input-shaped failures, `NotFound`/`Forbid`/`Conflict` where those + semantics fit. - **DTOs are a dedicated API-layer contract.** Domain entities and read - models are never serialized. Handlers return domain models - (`Result`); the controller maps success models - to response DTOs and error enums to HTTP. + models are never serialized. Handlers return domain models; the controller + maps success models to response DTOs and error enums to HTTP. - **Read paths return proof-of-loading models, not entities.** A query handler owns a single query method with its `Include` chain and projects into an immutable record whose constructor requires every loaded relationship (e.g. `ReviewWithVotes`). If it isn't loaded, the model cannot be constructed — no lazy-loading surprises, no possibly-null navigations downstream. +- **Controllers, not minimal APIs** — controller classes group a feature's + endpoints, give Swashbuckle first-class metadata, and keep binding, + auth attributes, and `ProducesResponseType` declarations in one reviewable + place per feature. +- **No MediatR / no dispatch layer.** Controllers inject concrete handler + classes. A handler is a class with one public method; DI registration is + `Infrastructure/DomainServices.cs`. ## Consequences - A feature is one folder; deleting a feature is deleting one folder plus its EF configuration. Review diffs stay feature-local. +- The set of possible failures of an operation is part of its signature, and + each error enum value appears in exactly one HTTP mapping — the API's error + behavior is reviewable in one file per feature. No `try/catch` in + controllers, no custom exception types in the domain. - "Domain never sees HTTP" needs review attention: no project edge enforces it, only the rule that DTO mapping happens exclusively in controllers. - No runtime dispatch magic: navigation is F12, stack traces are honest, and @@ -60,6 +80,8 @@ organized **by feature, not by layer**: - The cost of not abstracting EF: swapping the persistence technology means touching domain handlers. Accepted — the demo's point is EF storing strong types directly. +- Minor duplication: each feature carries its own small error enum instead of + a shared error catalogue — deliberate, so slices stay independent. ## Alternatives considered @@ -70,6 +92,9 @@ organized **by feature, not by layer**: tried first and rolled back: every feature lived in two half-folders, the slice never owned its domain objects, and the edge protected a boundary that review can hold. +- **Exceptions for business failures** — rejected: invisible in signatures, + and the compiler cannot force a controller to handle a newly added failure + the way an exhaustive `switch` over an enum does. - **Minimal APIs** — rejected: endpoint metadata and per-route filters get re-invented per endpoint, and the controller + `ModelState` + `ValidationProblem` pipeline is exactly what the StrongTypes ASP.NET diff --git a/docs/adr/0002-validation-lives-in-the-type-system.md b/docs/adr/0002-validation-lives-in-the-type-system.md index 730eac0..895cc03 100644 --- a/docs/adr/0002-validation-lives-in-the-type-system.md +++ b/docs/adr/0002-validation-lives-in-the-type-system.md @@ -1,24 +1,26 @@ -# ADR-0002 — Validation lives in the type system, not in annotations or guards +# ADR-0002 — Validation lives in the type system and flows end to end via OpenAPI -**Status:** Accepted (2026-07-15) +**Status:** Accepted (2026-07-15, revised 2026-07-16) ## Context The same input rule ("rating is 1–5", "title is not blank") tends to get -written three times: as a data annotation on the DTO, as a guard clause in the -controller, and again in the business layer because it can be called from -elsewhere. The three copies drift, and none of them changes the *type* — after -the check passes, the value is still a bare `string` or `int` and every layer -below has to trust or re-check it. +written many times over: as a data annotation on the DTO, as a guard clause in +the controller, again in the business layer, and once more as a hand-written +TypeScript interface that silently drifts from the API. The copies drift, and +none of them changes the *type* — after the check passes, the value is still a +bare `string` or `int` and every layer below has to trust or re-check it. This application exists to demonstrate the alternative: [Kalicz.StrongTypes](https://github.com/KaliCZ/StrongTypes) types that make -the invalid state unrepresentable ("parse, don't validate"). +the invalid state unrepresentable ("parse, don't validate") — and a pipeline +that carries the constraint to every consumer instead of restating it. ## Decision -Every constrained value is a strong type from the boundary inward, and the -constraint is expressed **nowhere else**: +Every constrained value is a strong type from the boundary inward, the +constraint is expressed **nowhere else**, and every other representation is +**generated** from it: - Request and response DTOs use `Email`, `NonEmptyString`, `Positive`, `NonNegative`, `Rating` (our own `[NumericWrapper]` type), … — never a @@ -34,18 +36,40 @@ constraint is expressed **nowhere else**: - Construction follows the library's intent split: `input.AsNonEmpty()` (nullable result) for external input where invalid means "reject", and `input.ToNonEmpty()` (throws) for internal values where invalid means "bug". +- The API renders its OpenAPI document via **Swashbuckle** with + `options.AddStrongTypes()` from `Kalicz.StrongTypes.OpenApi.Swashbuckle`, so + the schema carries the real constraints: `Email` → `format: email` + + `maxLength: 254`, `NonEmptyString` → `minLength: 1`, `Positive` → + `minimum: 0, exclusiveMinimum: true`, our `Rating` → `minimum: 1, maximum: 5`. +- **Nullability is truth.** A property is `nullable`/optional in the schema if + and only if it is optional in the C# DTO. Required DTO properties are + non-nullable and `required` in the schema; no information is lost between + C#, the schema, and TypeScript. +- The frontend's client types are **generated** from that document with + `openapi-typescript` and consumed through `openapi-fetch` — no hand-written + request/response types anywhere in the frontend. +- The generated file is committed. A test regenerates it against the running + API and fails on any diff, so contract drift breaks the build instead of + breaking users. - Rules the type system cannot carry (uniqueness, ownership, cross-entity - rules) are business logic and follow ADR-0003. + rules) are business logic and travel as `Result` error enums (ADR-0001). ## Consequences -- A rule exists in exactly one place — the type — and holds in the API, - the domain, the database, and (via ADR-0004) the generated frontend client. +- A rule exists in exactly one place — the type — and holds in the API, the + domain, the database, the OpenAPI document, and the generated frontend + client: change a DTO and the compiler + the drift test walk the change all + the way into the frontend. - Method signatures are the documentation: `Task> Handle(ProductId, AuthorId, Rating, NonEmptyString title, …)` states every - precondition. + precondition. Frontend code gets compile-time errors for wrong paths, wrong + verbs, missing parameters, and mis-typed bodies. - Unit tests for "what if the title is empty" disappear; the case does not type-check. Tests that remain are about behavior. +- Constraints are visible to API consumers in Swagger UI — the demo's payoff. + The OpenAPI document must stay accurate; anything that would degrade it + (untyped `IActionResult`, missing `ProducesResponseType`) is a review + blocker. - The cost: contributors must learn the small StrongTypes API surface (`Create` throws / `TryCreate` returns null / `As…`–`To…` extensions), and LINQ-to-SQL occasionally needs an explicit `.Unwrap()`. diff --git a/docs/adr/0003-business-errors-are-result-enums.md b/docs/adr/0003-business-errors-are-result-enums.md deleted file mode 100644 index f47c488..0000000 --- a/docs/adr/0003-business-errors-are-result-enums.md +++ /dev/null @@ -1,38 +0,0 @@ -# ADR-0003 — Business failures are enum values in `Result`, not exceptions - -**Status:** Accepted (2026-07-15) - -## Context - -Domain operations can fail for legitimate business reasons: the product does -not exist, the reviewer already reviewed it, you cannot vote on your own -review. Modeling those as exceptions hides them from the compiler — a caller -cannot see from the signature which failures exist, and controllers end up -with `try/catch` ladders that must be kept in sync with the service by hand. - -## Decision - -- Every domain handler that can fail returns - `Result` where `TError` is a **per-operation enum** - (`SubmitReviewError`, `CastVoteError`, …). Success and error are returned by - implicit conversion (`return review;` / `return SubmitReviewError.ProductNotFound;`). -- Exceptions are reserved for bugs and infrastructure faults — never for - business outcomes. -- **Controllers own the HTTP translation.** A controller consumes the result - with `result.Error is { } error` and maps each enum value in an exhaustive - `switch` to the proper response: `ValidationProblem` with a field-scoped - `ModelState` error for input-shaped failures, `NotFound`/`Forbid`/`Conflict` - where those semantics fit. The domain never sees HTTP. -- Handlers never return domain entities to the controller for serialization; - they return a success model, and the controller maps it to a response DTO - (see ADR-0001 for the DTO layer). - -## Consequences - -- The set of possible failures of an operation is part of its signature; a new - enum value fails compilation at every non-exhaustive `switch` that maps it. -- No `try/catch` in controllers; no custom exception types in the domain. -- Each error enum value appears in exactly one HTTP mapping, so the API's - error behavior is reviewable in one file per feature. -- Minor duplication: each feature carries its own small enum instead of a - shared error catalogue — deliberate, so slices stay independent. diff --git a/docs/adr/0004-openapi-is-the-frontend-contract.md b/docs/adr/0004-openapi-is-the-frontend-contract.md deleted file mode 100644 index 0742e13..0000000 --- a/docs/adr/0004-openapi-is-the-frontend-contract.md +++ /dev/null @@ -1,40 +0,0 @@ -# ADR-0004 — The OpenAPI document is the frontend contract - -**Status:** Accepted (2026-07-15) - -## Context - -The frontend needs types for every request and response. Hand-written -TypeScript interfaces drift silently from the API, and they cannot carry the -constraints the backend enforces. The backend already knows its exact wire -shape — including the strong-type constraints — because Swashbuckle renders it -into an OpenAPI document. - -## Decision - -- The API exposes its OpenAPI document via **Swashbuckle** with - `options.AddStrongTypes()` from `Kalicz.StrongTypes.OpenApi.Swashbuckle`, so - the schema carries the real constraints: `Email` → `format: email` + - `maxLength: 254`, `NonEmptyString` → `minLength: 1`, `Positive` → - `minimum: 0, exclusiveMinimum: true`, our `Rating` → `minimum: 1, maximum: 5`. -- **Nullability is truth.** A property is `nullable`/optional in the schema if - and only if it is optional in the C# DTO. Required DTO properties are - non-nullable and `required` in the schema; no information is lost between - C# and the schema. -- The frontend's client types are **generated** from that document with - `openapi-typescript` and consumed through `openapi-fetch` — no hand-written - request/response types anywhere in the frontend. -- The generated file is committed. A test regenerates it against the running - API and fails on any diff, so contract drift breaks the build instead of - breaking users. - -## Consequences - -- One source of truth: change a DTO and the compiler + the drift test walk the - change all the way into the frontend. -- Frontend code gets compile-time errors for wrong paths, wrong verbs, missing - parameters, and mis-typed bodies. -- Constraints are visible to API consumers in Swagger UI — the demo's payoff. -- The OpenAPI document must stay accurate; anything that would degrade it - (untyped `IActionResult`, missing `ProducesResponseType`) is a review - blocker. diff --git a/docs/adr/0008-frontend-vue-spa.md b/docs/adr/0008-frontend-vue-spa.md index 1935a2b..e01ea99 100644 --- a/docs/adr/0008-frontend-vue-spa.md +++ b/docs/adr/0008-frontend-vue-spa.md @@ -19,7 +19,7 @@ house preference (Vue reads closer to an MVC view layer). backend**, keeping the browser same-origin — no CORS configuration and no API URL in browser code. Aspire injects the proxy target as an environment variable. -- All API calls go through the generated `openapi-fetch` client (ADR-0004); +- All API calls go through the generated `openapi-fetch` client (ADR-0002); hand-written `fetch` calls to the API are a review blocker. - Frontend validation is derived from the generated schema types, not re-invented: the UI can pre-check what the schema states (required, min diff --git a/docs/adr/README.md b/docs/adr/README.md index 362854b..9580fc5 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -19,12 +19,12 @@ Two hard rules: Format: **Status / Context / Decision / Consequences** (plus **Alternatives considered** where the rejected option is instructive). +Numbers are identity, not sequence — a removed aspect leaves a gap. + | # | Decision | Status | | ---- | ------------------------------------------------------------------------------------------ | -------- | -| 0001 | [Vertical feature slices in one API project; controllers, no MediatR](0001-vertical-slices-and-project-layout.md) | Accepted | -| 0002 | [Validation lives in the type system, not in annotations or guards](0002-validation-lives-in-the-type-system.md) | Accepted | -| 0003 | [Business failures are enum values in `Result`, not exceptions](0003-business-errors-are-result-enums.md) | Accepted | -| 0004 | [The OpenAPI document is the frontend contract](0004-openapi-is-the-frontend-contract.md) | Accepted | +| 0001 | [Vertical feature slices: one API project, controllers, `Result` error enums](0001-vertical-slices-and-project-layout.md) | Accepted | +| 0002 | [Validation lives in the type system and flows end to end via OpenAPI](0002-validation-lives-in-the-type-system.md) | Accepted | | 0005 | [Auth is Zitadel OIDC + PKCE; author identity is a hash of `sub`](0005-auth-zitadel-oidc-pkce.md) | Accepted | | 0006 | [Seed data runs at startup, never inside migrations](0006-seeding-at-startup-not-migrations.md) | Accepted | | 0007 | [Tests exercise real dependencies; nothing is mocked](0007-tests-use-real-dependencies.md) | Accepted | diff --git a/docs/technical-requirements.md b/docs/technical-requirements.md index d3d08e3..f39e312 100644 --- a/docs/technical-requirements.md +++ b/docs/technical-requirements.md @@ -17,10 +17,10 @@ into the database and back out into the generated TypeScript client. | --------------- | ---------------------------------------------------------------------- | | Backend | ASP.NET Core Web API, controllers (ADR-0001), C# 14 / `net10.0` | | Validation | `Kalicz.StrongTypes` types only — no data annotations (ADR-0002) | -| Error handling | `Result` + per-operation error enums (ADR-0003) | +| Error handling | `Result` + per-operation error enums (ADR-0001) | | Persistence | PostgreSQL via EF Core (Npgsql), `.UseStrongTypes()` | | Auth | Zitadel (OIDC, PKCE SPA client) + `JwtBearer` (ADR-0005) | -| API docs | Swashbuckle + `Kalicz.StrongTypes.OpenApi.Swashbuckle` (ADR-0004) | +| API docs | Swashbuckle + `Kalicz.StrongTypes.OpenApi.Swashbuckle` (ADR-0002) | | Frontend | Vue 3 + Vite + TypeScript SPA (ADR-0008) | | Frontend client | Generated: `openapi-typescript` types + `openapi-fetch` runtime | | Orchestration | .NET Aspire AppHost: Postgres, Zitadel, API, frontend | @@ -76,11 +76,11 @@ flowchart LR Each of these is an ADR; the numbered file is the authority: -- Feature slices in one API project, controllers, no MediatR, DTO layer owned - by the API, proof-of-loading read models — [ADR-0001](adr/0001-vertical-slices-and-project-layout.md) -- Strong types are the only validation; no annotations, no guards — [ADR-0002](adr/0002-validation-lives-in-the-type-system.md) -- `Result` + error enums; controllers map to HTTP — [ADR-0003](adr/0003-business-errors-are-result-enums.md) -- OpenAPI is the frontend contract; client generated + drift-checked — [ADR-0004](adr/0004-openapi-is-the-frontend-contract.md) +- Feature slices in one API project, controllers, no MediatR, `Result` error + enums mapped to HTTP in controllers, DTO layer owned by the API, + proof-of-loading read models — [ADR-0001](adr/0001-vertical-slices-and-project-layout.md) +- Strong types are the only validation, and the constraints flow through + OpenAPI into the generated, drift-checked frontend client — [ADR-0002](adr/0002-validation-lives-in-the-type-system.md) - Zitadel OIDC + PKCE; `AuthorId` = SHA-256 of `sub` — [ADR-0005](adr/0005-auth-zitadel-oidc-pkce.md) - Seeding at startup, never in migrations — [ADR-0006](adr/0006-seeding-at-startup-not-migrations.md) - Real dependencies in tests, no mocks — [ADR-0007](adr/0007-tests-use-real-dependencies.md) @@ -151,7 +151,7 @@ wrappers with the same three-line recipe the library uses internally. One file per operation in its feature folder, named `.cs`, containing the handler class and, when the operation can fail, its error enum -(ADR-0003): +(ADR-0001): | Operation | Signature (conceptually) | Errors | | ---------------- | ----------------------------------------------------------------------- | --------------------------------- | @@ -206,7 +206,7 @@ DTO rules: - DTO properties are strong types; **optionality is encoded in nullability and nowhere else**. A required property is non-nullable in C#, `required` + non-nullable in the OpenAPI schema; an optional one is nullable in both. - The generated TypeScript mirrors this exactly (ADR-0004). + The generated TypeScript mirrors this exactly (ADR-0002). - API enums (`ReviewSort`) serialize as strings, are declared in the API layer, and map to domain enums with exhaustive switches — domain enums are never exposed on the wire. diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 3f45ab5..49197de 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -1,7 +1,7 @@ import createClient, { type Middleware } from "openapi-fetch"; import type { components, paths } from "./schema"; -// Convenience aliases over the generated schema — the ONLY source of API types (ADR-0004). +// Convenience aliases over the generated schema — the ONLY source of API types (ADR-0002). export type ProductSummary = components["schemas"]["ProductSummaryResponse"]; export type ProductDetail = components["schemas"]["ProductDetailResponse"]; export type Review = components["schemas"]["ReviewResponse"]; diff --git a/frontend/tests/e2e/contract.spec.ts b/frontend/tests/e2e/contract.spec.ts index 7ef74c8..c21a748 100644 --- a/frontend/tests/e2e/contract.spec.ts +++ b/frontend/tests/e2e/contract.spec.ts @@ -3,7 +3,7 @@ import { fileURLToPath } from "node:url"; import { expect, test } from "@playwright/test"; // The committed openapi.json is what the TypeScript client was generated from -// (ADR-0004). If the running API's document differs, the contract drifted: +// (ADR-0002). If the running API's document differs, the contract drifted: // refresh with `npm run refresh:api` (see README) and commit the result. test("the committed OpenAPI snapshot matches the running API", async ({ request }) => { const response = await request.get("/swagger/v1/swagger.json"); diff --git a/src/ProductReviews.Api/Infrastructure/ErrorHandling.cs b/src/ProductReviews.Api/Infrastructure/ErrorHandling.cs index ceab5d3..a1d858d 100644 --- a/src/ProductReviews.Api/Infrastructure/ErrorHandling.cs +++ b/src/ProductReviews.Api/Infrastructure/ErrorHandling.cs @@ -1,7 +1,7 @@ namespace ProductReviews.Api.Infrastructure; /// Unhandled exceptions become RFC 7807 responses. Business failures never get -/// here — they travel as Result error enums and are mapped by controllers (ADR-0003). +/// here — they travel as Result error enums and are mapped by controllers (ADR-0001). public static class ErrorHandling { public static void Configure(WebApplicationBuilder builder) diff --git a/src/ProductReviews.Api/Infrastructure/OpenApi.cs b/src/ProductReviews.Api/Infrastructure/OpenApi.cs index 4b12caa..79a24f1 100644 --- a/src/ProductReviews.Api/Infrastructure/OpenApi.cs +++ b/src/ProductReviews.Api/Infrastructure/OpenApi.cs @@ -6,7 +6,7 @@ namespace ProductReviews.Api.Infrastructure; /// Swashbuckle with AddStrongTypes(), so the document carries the real /// constraints (email format, minLength, exclusiveMinimum). The generated TypeScript -/// client is built from this document — it is the frontend contract (ADR-0004). +/// client is built from this document — it is the frontend contract (ADR-0002). public static class OpenApi { public static void Configure(WebApplicationBuilder builder) From 676dacfd105b7c82749aff510ecff21e3d72b736 Mon Sep 17 00:00:00 2001 From: KaliCZ Date: Thu, 16 Jul 2026 22:54:34 +0200 Subject: [PATCH 3/4] Renumber ADRs to close the gaps: 0005-0008 become 0003-0006 Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 4 ++-- README.md | 2 +- ...pkce.md => 0003-auth-zitadel-oidc-pkce.md} | 4 ++-- ...0004-seeding-at-startup-not-migrations.md} | 2 +- ...md => 0005-tests-use-real-dependencies.md} | 2 +- ...nd-vue-spa.md => 0006-frontend-vue-spa.md} | 4 ++-- docs/adr/README.md | 11 +++++----- docs/assumptions.md | 4 ++-- docs/technical-requirements.md | 20 +++++++++---------- frontend/playwright.config.ts | 2 +- frontend/vite.config.ts | 2 +- .../Features/Reviews/ReviewAuthor.cs | 2 +- .../Infrastructure/CurrentUser.cs | 2 +- .../Infrastructure/Persistence.cs | 2 +- .../Persistence/Seeding/DemoDataSeeder.cs | 2 +- src/ProductReviews.AppHost/AppHost.cs | 4 ++-- .../Zitadel/ZitadelHosting.cs | 2 +- .../SharedApiContext.cs | 4 ++-- .../TestTokens.cs | 2 +- 19 files changed, 39 insertions(+), 38 deletions(-) rename docs/adr/{0005-auth-zitadel-oidc-pkce.md => 0003-auth-zitadel-oidc-pkce.md} (94%) rename docs/adr/{0006-seeding-at-startup-not-migrations.md => 0004-seeding-at-startup-not-migrations.md} (96%) rename docs/adr/{0007-tests-use-real-dependencies.md => 0005-tests-use-real-dependencies.md} (96%) rename docs/adr/{0008-frontend-vue-spa.md => 0006-frontend-vue-spa.md} (92%) diff --git a/CLAUDE.md b/CLAUDE.md index 1d2466f..a085609 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -15,8 +15,8 @@ governs it: | A decision that feels reversible | The matching ADR in [docs/adr/](docs/adr/README.md) | | Validation, DTOs, domain types | [ADR-0002](docs/adr/0002-validation-lives-in-the-type-system.md), [ADR-0001](docs/adr/0001-vertical-slices-and-project-layout.md) | | API surface / frontend types | [ADR-0002](docs/adr/0002-validation-lives-in-the-type-system.md) | -| Auth | [ADR-0005](docs/adr/0005-auth-zitadel-oidc-pkce.md) | -| Tests | [ADR-0007](docs/adr/0007-tests-use-real-dependencies.md) | +| Auth | [ADR-0003](docs/adr/0003-auth-zitadel-oidc-pkce.md) | +| Tests | [ADR-0005](docs/adr/0005-tests-use-real-dependencies.md) | If a change contradicts a doc, change the doc in the same PR — or don't make the change. diff --git a/README.md b/README.md index 2be6dfb..cafc415 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,7 @@ npm --prefix frontend run test:e2e ``` Nothing is mocked anywhere — see -[ADR-0007](docs/adr/0007-tests-use-real-dependencies.md). +[ADR-0005](docs/adr/0005-tests-use-real-dependencies.md). ## The spec diff --git a/docs/adr/0005-auth-zitadel-oidc-pkce.md b/docs/adr/0003-auth-zitadel-oidc-pkce.md similarity index 94% rename from docs/adr/0005-auth-zitadel-oidc-pkce.md rename to docs/adr/0003-auth-zitadel-oidc-pkce.md index f64028b..c7dfe82 100644 --- a/docs/adr/0005-auth-zitadel-oidc-pkce.md +++ b/docs/adr/0003-auth-zitadel-oidc-pkce.md @@ -1,4 +1,4 @@ -# ADR-0005 — Auth is Zitadel OIDC + PKCE; author identity is a hash of `sub` +# ADR-0003 — Auth is Zitadel OIDC + PKCE; author identity is a hash of `sub` **Status:** Accepted (2026-07-15) @@ -6,7 +6,7 @@ Writing reviews and voting require a signed-in user (business requirement "Access"), so the demo needs a real identity provider that runs locally with -zero manual setup. The frontend is a browser SPA (ADR-0008) with no backend of +zero manual setup. The frontend is a browser SPA (ADR-0006) with no backend of its own, so it cannot keep a client secret or host a BFF token exchange. ## Decision diff --git a/docs/adr/0006-seeding-at-startup-not-migrations.md b/docs/adr/0004-seeding-at-startup-not-migrations.md similarity index 96% rename from docs/adr/0006-seeding-at-startup-not-migrations.md rename to docs/adr/0004-seeding-at-startup-not-migrations.md index 0aa5aba..4dee177 100644 --- a/docs/adr/0006-seeding-at-startup-not-migrations.md +++ b/docs/adr/0004-seeding-at-startup-not-migrations.md @@ -1,4 +1,4 @@ -# ADR-0006 — Seed data runs at startup, never inside migrations +# ADR-0004 — Seed data runs at startup, never inside migrations **Status:** Accepted (2026-07-15) diff --git a/docs/adr/0007-tests-use-real-dependencies.md b/docs/adr/0005-tests-use-real-dependencies.md similarity index 96% rename from docs/adr/0007-tests-use-real-dependencies.md rename to docs/adr/0005-tests-use-real-dependencies.md index 0ec4f7b..868861e 100644 --- a/docs/adr/0007-tests-use-real-dependencies.md +++ b/docs/adr/0005-tests-use-real-dependencies.md @@ -1,4 +1,4 @@ -# ADR-0007 — Tests exercise real dependencies; nothing is mocked +# ADR-0005 — Tests exercise real dependencies; nothing is mocked **Status:** Accepted (2026-07-15) diff --git a/docs/adr/0008-frontend-vue-spa.md b/docs/adr/0006-frontend-vue-spa.md similarity index 92% rename from docs/adr/0008-frontend-vue-spa.md rename to docs/adr/0006-frontend-vue-spa.md index e01ea99..ba15eda 100644 --- a/docs/adr/0008-frontend-vue-spa.md +++ b/docs/adr/0006-frontend-vue-spa.md @@ -1,4 +1,4 @@ -# ADR-0008 — The frontend is a Vue 3 + Vite SPA, not a server-rendered app +# ADR-0006 — The frontend is a Vue 3 + Vite SPA, not a server-rendered app **Status:** Accepted (2026-07-15) @@ -32,6 +32,6 @@ house preference (Vue reads closer to an MVC view layer). for logic" rule. - No SEO/SSR for product pages; irrelevant for a local demo, and a future Nuxt migration would keep the generated-client contract intact. -- Auth must be a browser-side OIDC flow (see ADR-0005) since there is no +- Auth must be a browser-side OIDC flow (see ADR-0003) since there is no server to hold a session; the access token stays in memory, not in localStorage. diff --git a/docs/adr/README.md b/docs/adr/README.md index 9580fc5..56b5cfd 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -19,13 +19,14 @@ Two hard rules: Format: **Status / Context / Decision / Consequences** (plus **Alternatives considered** where the rejected option is instructive). -Numbers are identity, not sequence — a removed aspect leaves a gap. +Numbering stays compact: when an aspect is removed or merged away, the +remaining ADRs are renumbered (references updated in the same PR). | # | Decision | Status | | ---- | ------------------------------------------------------------------------------------------ | -------- | | 0001 | [Vertical feature slices: one API project, controllers, `Result` error enums](0001-vertical-slices-and-project-layout.md) | Accepted | | 0002 | [Validation lives in the type system and flows end to end via OpenAPI](0002-validation-lives-in-the-type-system.md) | Accepted | -| 0005 | [Auth is Zitadel OIDC + PKCE; author identity is a hash of `sub`](0005-auth-zitadel-oidc-pkce.md) | Accepted | -| 0006 | [Seed data runs at startup, never inside migrations](0006-seeding-at-startup-not-migrations.md) | Accepted | -| 0007 | [Tests exercise real dependencies; nothing is mocked](0007-tests-use-real-dependencies.md) | Accepted | -| 0008 | [The frontend is a Vue 3 + Vite SPA, not a server-rendered app](0008-frontend-vue-spa.md) | Accepted | +| 0003 | [Auth is Zitadel OIDC + PKCE; author identity is a hash of `sub`](0003-auth-zitadel-oidc-pkce.md) | Accepted | +| 0004 | [Seed data runs at startup, never inside migrations](0004-seeding-at-startup-not-migrations.md) | Accepted | +| 0005 | [Tests exercise real dependencies; nothing is mocked](0005-tests-use-real-dependencies.md) | Accepted | +| 0006 | [The frontend is a Vue 3 + Vite SPA, not a server-rendered app](0006-frontend-vue-spa.md) | Accepted | diff --git a/docs/assumptions.md b/docs/assumptions.md index 1ff3c07..ebf28a3 100644 --- a/docs/assumptions.md +++ b/docs/assumptions.md @@ -6,11 +6,11 @@ Each is easy to reverse — flag any that should go the other way. 1. **Vue 3 + Vite SPA, not Nuxt.** The brief mentioned both a Nuxt preference and an explicit "Vue 3 + Vite" stack line; the explicit stack line won - (ADR-0008). A Nuxt rewrite would keep the generated-client contract + (ADR-0006). A Nuxt rewrite would keep the generated-client contract intact. 2. **Zitadel is in.** The stack list dropped only Temporal and Redis, so the earlier "Zitadel for auth" instruction stands: writes require a real OIDC - sign-in (ADR-0005). If the demo should run auth-free, the AppHost and the + sign-in (ADR-0003). If the demo should run auth-free, the AppHost and the `[Authorize]` attributes are the only touchpoints. 3. **No moderation at all.** Dropping Temporal removed the durable moderation workflows; rather than fake the rules synchronously (1/2/5-star reviews diff --git a/docs/technical-requirements.md b/docs/technical-requirements.md index f39e312..845b792 100644 --- a/docs/technical-requirements.md +++ b/docs/technical-requirements.md @@ -19,12 +19,12 @@ into the database and back out into the generated TypeScript client. | Validation | `Kalicz.StrongTypes` types only — no data annotations (ADR-0002) | | Error handling | `Result` + per-operation error enums (ADR-0001) | | Persistence | PostgreSQL via EF Core (Npgsql), `.UseStrongTypes()` | -| Auth | Zitadel (OIDC, PKCE SPA client) + `JwtBearer` (ADR-0005) | +| Auth | Zitadel (OIDC, PKCE SPA client) + `JwtBearer` (ADR-0003) | | API docs | Swashbuckle + `Kalicz.StrongTypes.OpenApi.Swashbuckle` (ADR-0002) | -| Frontend | Vue 3 + Vite + TypeScript SPA (ADR-0008) | +| Frontend | Vue 3 + Vite + TypeScript SPA (ADR-0006) | | Frontend client | Generated: `openapi-typescript` types + `openapi-fetch` runtime | | Orchestration | .NET Aspire AppHost: Postgres, Zitadel, API, frontend | -| Backend tests | xUnit + FsCheck (property) + Testcontainers Postgres (ADR-0007) | +| Backend tests | xUnit + FsCheck (property) + Testcontainers Postgres (ADR-0005) | | Frontend tests | Vitest (unit/component) + Playwright (E2E against the real stack) | ```mermaid @@ -81,10 +81,10 @@ Each of these is an ADR; the numbered file is the authority: proof-of-loading read models — [ADR-0001](adr/0001-vertical-slices-and-project-layout.md) - Strong types are the only validation, and the constraints flow through OpenAPI into the generated, drift-checked frontend client — [ADR-0002](adr/0002-validation-lives-in-the-type-system.md) -- Zitadel OIDC + PKCE; `AuthorId` = SHA-256 of `sub` — [ADR-0005](adr/0005-auth-zitadel-oidc-pkce.md) -- Seeding at startup, never in migrations — [ADR-0006](adr/0006-seeding-at-startup-not-migrations.md) -- Real dependencies in tests, no mocks — [ADR-0007](adr/0007-tests-use-real-dependencies.md) -- Vue 3 + Vite SPA, same-origin proxy to the API — [ADR-0008](adr/0008-frontend-vue-spa.md) +- Zitadel OIDC + PKCE; `AuthorId` = SHA-256 of `sub` — [ADR-0003](adr/0003-auth-zitadel-oidc-pkce.md) +- Seeding at startup, never in migrations — [ADR-0004](adr/0004-seeding-at-startup-not-migrations.md) +- Real dependencies in tests, no mocks — [ADR-0005](adr/0005-tests-use-real-dependencies.md) +- Vue 3 + Vite SPA, same-origin proxy to the API — [ADR-0006](adr/0006-frontend-vue-spa.md) ## 4. Domain model @@ -215,7 +215,7 @@ DTO rules: `ValidationProblem` with a field key; missing resources become 404; ownership violations become 403; duplicates become 409. -## 7. Auth (summary — ADR-0005 is the authority) +## 7. Auth (summary — ADR-0003 is the authority) Zitadel runs as an Aspire-managed container; the AppHost idempotently provisions the org/project/PKCE SPA client and two demo users, and injects @@ -269,7 +269,7 @@ deploy gate can grep the running build. databases: `productreviews`, `zitadel`), Zitadel (+ its login UI container), the API, and the frontend dev server; installs frontend dependencies on first run; provisions Zitadel; and opens the dashboard. -- The API migrates (`MigrateAsync`) and seeds (ADR-0006) at startup in +- The API migrates (`MigrateAsync`) and seeds (ADR-0004) at startup in Development. Ten products with reviews and votes are browsable immediately. - Fixed ports where the browser needs stability (Zitadel issuer `:8090`, frontend `:5173` dev / `:4173` E2E); everything else is dynamic and flows @@ -289,7 +289,7 @@ deploy gate can grep the running build. Test conventions: `Method_Condition_ExpectedResult` naming; integration tests send anonymous objects and assert on raw `JsonElement`s (contract renames must fail tests); one shared Postgres container per run, isolation via unique data -per test; no mocking anywhere (ADR-0007). The OpenAPI document itself is +per test; no mocking anywhere (ADR-0005). The OpenAPI document itself is asserted in integration tests (email format, `minLength`, `exclusiveMinimum`, rating bounds) — it is the demo's headline claim. diff --git a/frontend/playwright.config.ts b/frontend/playwright.config.ts index 3dbac2e..8f45c1c 100644 --- a/frontend/playwright.config.ts +++ b/frontend/playwright.config.ts @@ -2,7 +2,7 @@ import { defineConfig } from "@playwright/test"; // The E2E stack is the real thing: the Aspire AppHost boots Postgres, Zitadel (+ its // hosted login), the API, and this frontend as a production preview build on :4173. -// Sign-in happens through the real Zitadel form — no bypass (ADR-0007). +// Sign-in happens through the real Zitadel form — no bypass (ADR-0005). const webBaseUrl = "http://localhost:4173"; export default defineConfig({ diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index 1db9de0..920f556 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -3,7 +3,7 @@ import vue from "@vitejs/plugin-vue"; import { defineConfig } from "vite"; // PORT and API_PROXY_TARGET are injected by the Aspire AppHost. The proxy keeps the -// browser same-origin with the API — no CORS, no API URL in browser code (ADR-0008). +// browser same-origin with the API — no CORS, no API URL in browser code (ADR-0006). const port = Number(process.env.PORT ?? 5173); const apiProxyTarget = process.env.API_PROXY_TARGET ?? "http://localhost:5000"; // /swagger rides along so the OpenAPI document (and its UI) is reachable from the diff --git a/src/ProductReviews.Api/Features/Reviews/ReviewAuthor.cs b/src/ProductReviews.Api/Features/Reviews/ReviewAuthor.cs index 1324499..247c697 100644 --- a/src/ProductReviews.Api/Features/Reviews/ReviewAuthor.cs +++ b/src/ProductReviews.Api/Features/Reviews/ReviewAuthor.cs @@ -3,6 +3,6 @@ namespace ProductReviews.Api.Features.Reviews; /// The acting reviewer, as the domain sees them: an opaque stable id -/// (derived from the identity provider's subject, see ADR-0005) and the display +/// (derived from the identity provider's subject, see ADR-0003) and the display /// name snapshotted onto anything they write. public sealed record ReviewAuthor(Guid AuthorId, NonEmptyString DisplayName); diff --git a/src/ProductReviews.Api/Infrastructure/CurrentUser.cs b/src/ProductReviews.Api/Infrastructure/CurrentUser.cs index 13e003a..5944271 100644 --- a/src/ProductReviews.Api/Infrastructure/CurrentUser.cs +++ b/src/ProductReviews.Api/Infrastructure/CurrentUser.cs @@ -8,7 +8,7 @@ namespace ProductReviews.Api.Infrastructure; /// Maps the authenticated principal to the domain's . /// AuthorId is the SHA-256 of the OIDC sub claim (first 16 bytes as a Guid), so the -/// database stays Guid-keyed whatever shape the identity provider's ids have (ADR-0005). +/// database stays Guid-keyed whatever shape the identity provider's ids have (ADR-0003). public static class CurrentUser { public static ReviewAuthor Author(this ClaimsPrincipal user) diff --git a/src/ProductReviews.Api/Infrastructure/Persistence.cs b/src/ProductReviews.Api/Infrastructure/Persistence.cs index 4920f60..0899de3 100644 --- a/src/ProductReviews.Api/Infrastructure/Persistence.cs +++ b/src/ProductReviews.Api/Infrastructure/Persistence.cs @@ -17,7 +17,7 @@ public static void Configure(WebApplicationBuilder builder) configureSettings: settings => settings.DisableHealthChecks = true, configureDbContextOptions: options => options.UseStrongTypes()); - /// Development-only convenience (ADR-0006): production applies migrations + /// Development-only convenience (ADR-0004): production applies migrations /// from the deploy pipeline and never seeds. public static async Task MigrateAndSeedAsync(WebApplication app) { diff --git a/src/ProductReviews.Api/Persistence/Seeding/DemoDataSeeder.cs b/src/ProductReviews.Api/Persistence/Seeding/DemoDataSeeder.cs index c41f1d5..677e83c 100644 --- a/src/ProductReviews.Api/Persistence/Seeding/DemoDataSeeder.cs +++ b/src/ProductReviews.Api/Persistence/Seeding/DemoDataSeeder.cs @@ -6,7 +6,7 @@ namespace ProductReviews.Api.Persistence.Seeding; -/// Idempotent demo seeding (ADR-0006): runs after migrations, does nothing +/// Idempotent demo seeding (ADR-0004): runs after migrations, does nothing /// once products exist, and builds everything through the domain entities so seed /// data satisfies the same invariants as user input. public static class DemoDataSeeder diff --git a/src/ProductReviews.AppHost/AppHost.cs b/src/ProductReviews.AppHost/AppHost.cs index 21f22da..bb8e07b 100644 --- a/src/ProductReviews.AppHost/AppHost.cs +++ b/src/ProductReviews.AppHost/AppHost.cs @@ -20,7 +20,7 @@ // "zitadeldb", not "zitadel" — the container resource already claims that name. var zitadelDatabase = postgres.AddDatabase("zitadeldb"); -// --- Identity provider: self-hosted Zitadel (ADR-0005) ----------------------- +// --- Identity provider: self-hosted Zitadel (ADR-0003) ----------------------- // Zitadel writes its first-instance machine-user PATs into this bind-mounted dir on first // init; the AppHost reads them to drive provisioning. Gitignored. E2E gets a unique temp // dir: its Postgres is throwaway, so a PAT left over from a previous run would belong to a @@ -86,7 +86,7 @@ } }); -// --- Frontend: Vue 3 + Vite SPA (ADR-0008) ------------------------------------- +// --- Frontend: Vue 3 + Vite SPA (ADR-0006) ------------------------------------- // AddNpmApp only runs the script — install once when node_modules is absent and gate on it. var frontendDirectory = Path.Combine(repositoryRoot, "frontend"); IResourceBuilder? frontendInstall = null; diff --git a/src/ProductReviews.AppHost/Zitadel/ZitadelHosting.cs b/src/ProductReviews.AppHost/Zitadel/ZitadelHosting.cs index bdacff3..7142a91 100644 --- a/src/ProductReviews.AppHost/Zitadel/ZitadelHosting.cs +++ b/src/ProductReviews.AppHost/Zitadel/ZitadelHosting.cs @@ -1,6 +1,6 @@ namespace ProductReviews.AppHost.Zitadel; -/// The self-hosted Zitadel containers (ADR-0005): the core (API + Console) and the +/// The self-hosted Zitadel containers (ADR-0003): the core (API + Console) and the /// separate Login V2 app the core redirects browsers to for the hosted sign-in page. internal static class ZitadelHosting { diff --git a/tests/ProductReviews.Api.IntegrationTests/SharedApiContext.cs b/tests/ProductReviews.Api.IntegrationTests/SharedApiContext.cs index e0724ae..4e9c89d 100644 --- a/tests/ProductReviews.Api.IntegrationTests/SharedApiContext.cs +++ b/tests/ProductReviews.Api.IntegrationTests/SharedApiContext.cs @@ -13,7 +13,7 @@ public sealed class ReviewsApiFactory(string connectionString) : WebApplicationF { protected override void ConfigureWebHost(IWebHostBuilder builder) { - // Development runs the real migrate + seed path at startup (ADR-0006) — + // Development runs the real migrate + seed path at startup (ADR-0004) — // the tests exercise it instead of preparing the schema themselves. builder.UseEnvironment("Development"); builder.UseSetting("ConnectionStrings:productreviews", connectionString); @@ -34,7 +34,7 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) } } -/// One Postgres container + one API host for the whole run (ADR-0007); +/// One Postgres container + one API host for the whole run (ADR-0005); /// tests isolate through unique authors/products, never by resetting the database. public sealed class SharedApiContext : IAsyncLifetime { diff --git a/tests/ProductReviews.Api.IntegrationTests/TestTokens.cs b/tests/ProductReviews.Api.IntegrationTests/TestTokens.cs index 3e179e0..402660c 100644 --- a/tests/ProductReviews.Api.IntegrationTests/TestTokens.cs +++ b/tests/ProductReviews.Api.IntegrationTests/TestTokens.cs @@ -5,7 +5,7 @@ namespace ProductReviews.Api.IntegrationTests; /// Mints JWTs against a test-owned symmetric key. The API's real JwtBearer -/// pipeline validates them (ADR-0007) — only the trust anchor is swapped, not the code path. +/// pipeline validates them (ADR-0005) — only the trust anchor is swapped, not the code path. public static class TestTokens { public const string Issuer = "https://tests.productreviews.local"; From 396b853d58699e68d286c4bfb115e8ac5bd9b58b Mon Sep 17 00:00:00 2001 From: Pavel Kalandra Date: Thu, 16 Jul 2026 23:09:05 +0200 Subject: [PATCH 4/4] Update CLAUDE.md --- CLAUDE.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index a085609..dd21f6a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,7 +26,8 @@ how the application works *now*. Changing or rolling back a decision means rewriting the matching ADR to the new thinking, in the same PR — git holds the history. Never mark an ADR superseded, and never add a new ADR for an aspect an existing one covers; new numbers are only for new aspects. A -rolled-back approach usually earns a line under "Alternatives considered". +rolled-back approach usually earns a line under "Alternatives considered" +or "Previous implementation". ## Commands