diff --git a/CLAUDE.md b/CLAUDE.md index 37cb8a4..dd21f6a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,15 +12,23 @@ 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 | -| 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) | -| Tests | [ADR-0007](docs/adr/0007-tests-use-real-dependencies.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-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-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. +**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" +or "Previous implementation". + ## Commands ```shell @@ -31,7 +39,7 @@ npm --prefix frontend run test:unit # Vitest npm --prefix frontend run test:e2e # Playwright against the full stack npm --prefix frontend run typecheck # vue-tsc npm --prefix frontend run refresh:api # refresh openapi.json + regenerate schema.d.ts (stack must be running) -dotnet ef migrations add --project src/ProductReviews.Domain --output-dir Persistence/Migrations +dotnet ef migrations add --project src/ProductReviews.Api --output-dir Persistence/Migrations ``` Demo sign-in: `demo@productreviews.local` / `ProductReviews123!` (second user: @@ -42,11 +50,13 @@ restart the AppHost. ## Architecture rules (the non-negotiables) -1. **Vertical slices.** A feature lives in `Features//` (API) and - `/` (Domain) — one folder each, everything it owns inside. This - applies to infrastructure too: one concern per file - (`Infrastructure/RateLimits.cs`, `Observability.cs`, …) with +1. **Vertical slices.** A feature lives in `Features//` — one folder, + everything it owns inside: entities, handlers + error enums, controller, + 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 + results, and DTOs stay in the controller. 2. **DDD for data and logic.** Entities own their behavior (`Review.ApplyEdit`, `Product.RefreshRatingSummary`); handlers orchestrate, never property-spray. One handler file per operation, containing its error @@ -98,8 +108,9 @@ restart the AppHost. commit `openapi.json` + `schema.d.ts` together with the API change — the E2E contract test fails otherwise. 14. **Nothing blanket-global.** `[Authorize]` per action (no - `RequireAuthorization()` on the route table), ServiceDefaults in its own - namespace, no global route middleware in the frontend. + `RequireAuthorization()` on the route table), resilience per HTTP client + (no `ConfigureHttpClientDefaults`), no global route middleware in the + frontend. ## Naming and style @@ -122,11 +133,11 @@ restart the AppHost. | New… | Goes to | | -------------------------- | ------- | | Endpoint + DTOs | `src/ProductReviews.Api/Features//` | -| Business operation | `src/ProductReviews.Domain//.cs` (handler + error enum) | -| Entity / value type | `src/ProductReviews.Domain//` | -| EF configuration | `src/ProductReviews.Domain/Persistence/Configurations/` | +| Business operation | `src/ProductReviews.Api/Features//.cs` (handler + error enum) | +| Entity / value type | `src/ProductReviews.Api/Features//` | +| EF configuration | `src/ProductReviews.Api/Persistence/Configurations/` | | Cross-cutting API concern | `src/ProductReviews.Api/Infrastructure/.cs` | | Orchestration resource | `src/ProductReviews.AppHost/` | | Page / component | `frontend/src/pages/` / `frontend/src/components/` | -| Domain test | `tests/ProductReviews.Domain.Tests/` (prefer a property test) | +| Domain test | `tests/ProductReviews.Api.UnitTests/` (prefer a property test) | | API behavior test | `tests/ProductReviews.Api.IntegrationTests/` (wire-level) | diff --git a/README.md b/README.md index 67a44d8..cafc415 100644 --- a/README.md +++ b/README.md @@ -54,15 +54,15 @@ Where to click: | The point | Where | | --- | --- | | DTOs with strong types, zero annotations | [ReviewContracts.cs](src/ProductReviews.Api/Features/Reviews/ReviewContracts.cs) | -| `Maybe` three-state PATCH (omit = keep, `{}` = clear, `{"Value": …}` = set) | [EditReviewRequest](src/ProductReviews.Api/Features/Reviews/ReviewContracts.cs), [Review.ApplyEdit](src/ProductReviews.Domain/Reviews/Review.cs), [EditReview.cs](src/ProductReviews.Domain/Reviews/EditReview.cs) | -| `Result` + error enums instead of exceptions | [SubmitReview.cs](src/ProductReviews.Domain/Reviews/SubmitReview.cs) → mapped exhaustively in [ReviewsController.cs](src/ProductReviews.Api/Features/Reviews/ReviewsController.cs) | -| Declaring your **own** strong type (`[NumericWrapper]`) | [Rating.cs](src/ProductReviews.Domain/Reviews/Rating.cs) | -| EF Core storing strong types directly (`UseStrongTypes()`) | [Persistence.cs](src/ProductReviews.Api/Infrastructure/Persistence.cs), [ReviewsDbContext.cs](src/ProductReviews.Domain/Persistence/ReviewsDbContext.cs), the [migration](src/ProductReviews.Domain/Persistence/Migrations/) (plain `varchar`/`int` columns) | +| `Maybe` three-state PATCH (omit = keep, `{}` = clear, `{"Value": …}` = set) | [EditReviewRequest](src/ProductReviews.Api/Features/Reviews/ReviewContracts.cs), [Review.ApplyEdit](src/ProductReviews.Api/Features/Reviews/Review.cs), [EditReview.cs](src/ProductReviews.Api/Features/Reviews/EditReview.cs) | +| `Result` + error enums instead of exceptions | [SubmitReview.cs](src/ProductReviews.Api/Features/Reviews/SubmitReview.cs) → mapped exhaustively in [ReviewsController.cs](src/ProductReviews.Api/Features/Reviews/ReviewsController.cs) | +| Declaring your **own** strong type (`[NumericWrapper]`) | [Rating.cs](src/ProductReviews.Api/Features/Reviews/Rating.cs) | +| EF Core storing strong types directly (`UseStrongTypes()`) | [Persistence.cs](src/ProductReviews.Api/Infrastructure/Persistence.cs), [ReviewsDbContext.cs](src/ProductReviews.Api/Persistence/ReviewsDbContext.cs), the [migration](src/ProductReviews.Api/Persistence/Migrations/) (plain `varchar`/`int` columns) | | OpenAPI carrying the real constraints (`AddStrongTypes()`) | [OpenApi.cs](src/ProductReviews.Api/Infrastructure/OpenApi.cs), snapshot in [frontend/openapi.json](frontend/openapi.json) | | Constraints flowing into TypeScript (generated client) | [schema.d.ts](frontend/src/api/schema.d.ts) (generated), [client.ts](frontend/src/api/client.ts), [editReviewBody.ts](frontend/src/api/editReviewBody.ts) | -| Property-based tests with generated strong-typed values | [RatingTests.cs](tests/ProductReviews.Domain.Tests/RatingTests.cs), [ReviewEditTests.cs](tests/ProductReviews.Domain.Tests/ReviewEditTests.cs) | +| Property-based tests with generated strong-typed values | [RatingTests.cs](tests/ProductReviews.Api.UnitTests/RatingTests.cs), [ReviewEditTests.cs](tests/ProductReviews.Api.UnitTests/ReviewEditTests.cs) | | The OpenAPI claims verified against the running API | [OpenApiDocumentTests.cs](tests/ProductReviews.Api.IntegrationTests/OpenApiDocumentTests.cs) | -| Parse-don't-validate at the query boundary (`Positive` paging, `Rating[]` filters) | [ReviewsController.cs](src/ProductReviews.Api/Features/Reviews/ReviewsController.cs), [GetReviewsPage.cs](src/ProductReviews.Domain/Reviews/GetReviewsPage.cs) | +| Parse-don't-validate at the query boundary (`Positive` paging, `Rating[]` filters) | [ReviewsController.cs](src/ProductReviews.Api/Features/Reviews/ReviewsController.cs), [GetReviewsPage.cs](src/ProductReviews.Api/Features/Reviews/GetReviewsPage.cs) | ## Tests @@ -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/0001-vertical-slices-and-project-layout.md b/docs/adr/0001-vertical-slices-and-project-layout.md index 906f856..7d758fc 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: one API project, controllers, `Result` error enums -**Status:** Accepted (2026-07-15) +**Status:** Accepted (2026-07-15, revised 2026-07-16) ## Context @@ -8,62 +8,93 @@ 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. + +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 -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. -- **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. +- **`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. +- **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 — both directions live in the API - slice. + 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. `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. +- **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 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. +- 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 the template stays approachable for readers new to the stack. - 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 - **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. +- **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/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/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/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 1935a2b..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) @@ -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 @@ -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 7f9ec3b..56b5cfd 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -2,21 +2,31 @@ 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. + +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 in two projects; 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 | -| 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 | +| 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 | +| 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 307b987..845b792 100644 --- a/docs/technical-requirements.md +++ b/docs/technical-requirements.md @@ -17,14 +17,14 @@ 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) | -| Frontend | Vue 3 + Vite + TypeScript SPA (ADR-0008) | +| 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-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 @@ -34,12 +34,11 @@ flowchart LR end subgraph Aspire AppHost FE[Vite dev server
proxies /api] - API[ProductReviews.Api
controllers + DTOs] - DOM[ProductReviews.Domain
entities + handlers + EF Core] + API[ProductReviews.Api
feature slices: controllers + DTOs
+ entities + handlers + EF Core] PG[(PostgreSQL)] ZIT[Zitadel
OIDC provider] end - SPA -->|/api/*| FE -->|proxy| API --> DOM --> PG + SPA -->|/api/*| FE -->|proxy| API --> PG SPA -->|OIDC redirect + token| ZIT API -->|JWKS / discovery| ZIT ZIT --> PG @@ -52,22 +51,17 @@ flowchart LR ├── docs/ # this spec: requirements + ADRs ├── src/ │ ├── ProductReviews.AppHost/ # Aspire orchestration + Zitadel provisioning -│ ├── ProductReviews.ServiceDefaults/ # OTel, health checks, resilience (own namespace) -│ ├── ProductReviews.Api/ -│ │ ├── Features/ -│ │ │ ├── Catalog/ # controller + response DTOs + mapping -│ │ │ ├── Reviews/ # controller + request/response DTOs + mapping -│ │ │ ├── Votes/ -│ │ │ └── Profile/ # GET /api/me -│ │ ├── Infrastructure/ # one concern per file (§9) -│ │ └── Program.cs # thin orchestrator, calls the slices -│ └── ProductReviews.Domain/ -│ ├── Catalog/ # Product entity + catalog queries -│ ├── Reviews/ # Review, Rating, submit/edit/delete handlers -│ ├── Votes/ # ReviewVote + cast/remove handlers -│ └── Persistence/ # DbContext, configurations, migrations, seeding +│ └── 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 +│ │ ├── Votes/ # ReviewVote + cast/remove + controller + DTOs +│ │ └── Profile/ # GET /api/me +│ ├── Persistence/ # DbContext, configurations, migrations, seeding +│ ├── Infrastructure/ # one concern per file (§9) +│ └── Program.cs # thin orchestrator, calls the slices ├── tests/ -│ ├── ProductReviews.Domain.Tests/ # unit + FsCheck property tests +│ ├── ProductReviews.Api.UnitTests/ # unit + FsCheck property tests │ └── ProductReviews.Api.IntegrationTests/ # Testcontainers Postgres, wire-level ├── frontend/ # Vue 3 + Vite SPA │ ├── src/api/ # generated schema types + typed client @@ -82,15 +76,15 @@ flowchart LR Each of these is an ADR; the numbered file is the authority: -- Feature slices in two projects, 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) -- 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) +- 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-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 @@ -155,9 +149,9 @@ wrappers with the same three-line recipe the library uses internally. ## 5. Domain operations -One file per operation in `ProductReviews.Domain`, named `.cs`, +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 | | ---------------- | ----------------------------------------------------------------------- | --------------------------------- | @@ -212,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. @@ -221,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 @@ -256,11 +250,17 @@ class with `Configure(builder)` and, where middleware exists, `Use(app)`: | `OpenApi.cs` | Swagger UI + `AddStrongTypes()` + `Rating` schema mapping | | `RateLimits.cs` | Fixed-window limiter on write endpoints | | `ErrorHandling.cs` | Consistent RFC 7807 output for unhandled failures | -| `Observability.cs` | Health checks (`/health`, `/alive`, DbContext check) | - -OpenTelemetry, service discovery, and HTTP resilience come from -`ProductReviews.ServiceDefaults` — which lives in its **own** namespace -(`ProductReviews.ServiceDefaults`), not a hijacked framework namespace. +| `Observability.cs` | OpenTelemetry logging/metrics/tracing, OTLP export | +| `Health.cs` | Health checks and the `/health` + `/alive` endpoints | + +Health follows the house pattern: `/alive` is liveness only — the +commit-hash check (`version`, from the assembly's `SourceRevisionId`) and no +dependencies, so a shared Postgres or Zitadel outage never reads as a dead +process. `/health` is full readiness: the EF Core `DbContext` check +(`database`) plus every transitive dependency (`zitadel`, probing the OIDC +discovery document — Degraded, not Unhealthy, because reads survive an +identity-provider outage). Both endpoints answer `" "`, so a +deploy gate can grep the running build. ## 10. Local development & orchestration @@ -269,7 +269,7 @@ OpenTelemetry, service discovery, and HTTP resilience come from 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 @@ -280,7 +280,7 @@ OpenTelemetry, service discovery, and HTTP resilience come from | Suite | Project / tool | What it proves | | --- | --- | --- | -| Domain unit | `ProductReviews.Domain.Tests` (xUnit) | Entity behavior and invariants — pure, no I/O | +| Domain unit | `ProductReviews.Api.UnitTests` (xUnit) | Entity behavior and invariants — pure, no I/O | | Property | same project, FsCheck + `Kalicz.StrongTypes.FsCheck` | Invariants hold across generated strong-typed inputs (score arithmetic, edit semantics, rating bounds) | | API integration | `ProductReviews.Api.IntegrationTests` (xUnit, Testcontainers) | Wire-level behavior against real Postgres: contracts, constraints, error mapping, OpenAPI content | | Frontend unit | Vitest + Vue Test Utils | Component logic; PATCH body construction (three-state semantics) | @@ -289,7 +289,7 @@ OpenTelemetry, service discovery, and HTTP resilience come from 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/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/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