feat: Reviews aggregate + FluentValidation + Trellis.Testing + ServiceDefaults#31
Merged
xavierjohn merged 11 commits intoJun 5, 2026
Merged
Conversation
…+ Trellis.ServiceDefaults Three new Trellis V3 packages plus their package-shipped reference docs into .github. Trellis.Testing transitively requires FluentAssertions >= 7.2.2 (the latest major), so the central pin is bumped from 6.12.0. FA 7 has different licensing terms than 6.x — fine for this showcase, downstream consumers of similar repos should evaluate before adopting.
….Testing matchers MenuReview is a guest's rating + comment on a menu they experienced at a dinner. No state machine (no transitions); two operations: TryCreate (initial submit) and UpdateContent (edit). Domain invariants enforced by FluentValidation: rating in [1,5], non-empty comment <= 1000 chars. UpdateContent uses snapshot+rollback on validation failure so an invalid update doesn't leak into subsequent reads (same lesson as Menu.Update from PR 1 review). Drive-by stub cleanup: the pre-existing Domain/src/MenuReview/ValueObjects/MenuId.cs defined MenuReviewId in the WRONG namespace (Menu.ValueObject instead of MenuReview.ValueObject). Menu.cs consumed it through that wrong namespace, papering over the bug. Removes the stub, adds the canonical MenuReviewId.cs under the right namespace, updates Menu.cs to use the canonical path. Eight domain tests in MenuReviewTests.cs use Trellis.Testing matchers throughout — Should().BeSuccess()/.BeFailureOfType<>()/.Unwrap() — to give future authors a side-by-side reference vs the older Dinner/Reservation tests that still use the imperative .Match(...).Should().BeOfType<>() style.
…2 FluentValidation rules + 2 log handlers Application: * SubmitMenuReviewCommand + handler (ICommand<Result<MenuReview>>, Recipe 22 fail-loud on missing Menu) * UpdateMenuReviewCommand + handler (NotFound-leak-shield ownership check via shared LoadReviewOwnedByAsync ROP helper) * GetMenuReviewQuery (single load, expression-bodied via T?.ToResult) * ListReviewsForMenuQuery (paginated, same shape as PR 3's ListDinners/ListMenus) * Two AbstractValidator<TCommand> classes for SubmitMenuReviewCommand and UpdateMenuReviewCommand — these are the FluentValidation rules the Mediator pipeline runs BEFORE the handler. Wired via AddTrellis(t => t.UseFluentValidation(...)) in the API DI. * Two log-only IDomainEventHandler<> impls for MenuReviewSubmitted/Updated, registered per-handler in API DI (AOT-safe pattern). Infrastructure: MenuReviewInMemoryRepository mirrors the Reservation/Dinner/Menu repos — static aggregate list + ETag tracking + per-menu paginated read. DI binds IMenuReviewRepository alongside IRepository<MenuReview> to the same scoped concrete instance in both InMemory and Cosmos branches (same same-instance invariant as PR 3's Menu fix).
Four endpoints under /menu-reviews:
* POST /menu-reviews — submit (FluentValidation kicks in at the Mediator pipeline)
* GET /menu-reviews/{id:MenuReviewId} — single, with strong ETag + conditional GET (304 on If-None-Match)
* PUT /menu-reviews/{id:MenuReviewId} — update (FluentValidation also applies)
* GET /menu-reviews/for-menu/{menuId:MenuId} — paginated list of one menu's reviews
DTO converts wire body to command via the same Result/Combine + ToHttpResponseAsync pipeline as ScheduleDinnerRequest/UpdateMenuRequest — no hand-rolled error bodies.
…...)) builder Trellis.ServiceDefaults ships TrellisServiceBuilder, a fluent composition root that fixes the mutually-required call order across UseAsp / UseScalarValueValidation / UseProblemDetails / UseMediator / UseClaimsActorProvider / UseResourceAuthorization / UseFluentValidation / UseDomainEvents / UseIdempotency so misconfigurations fail at startup instead of at request time. Refactored Api/src/DependencyInjection.cs to use the builder — drops ~30 lines of Add*/explanatory comments. Per-type registrations stay separate (AddTrellisRouteConstraint<T>, AddDomainEventHandler<TEvent, THandler>, AddInMemoryIdempotencyStore, AddSingleton(TimeProvider.System)) — those are 'application-owned' per the docs and not covered by the builder. Program.cs adds app.UseTrellisIdempotency() AFTER UseAuthentication/UseAuthorization (PR 4 lesson preserved).
…+ ownership + pagination MenuReviewTests covers the wire-observable contracts: * Submit returns 201 with ETag + Location * Submit rating=99 -> 422 with errors.Rating (FluentValidation at Mediator) * Submit comment='' -> 422 with errors.Comment * Submit against missing menu -> 404 (Recipe 22 fail-loud) * Update with rating=0 -> 422 (validator runs on Update too) * Cross-guest update -> 404 (NotFound-leak-shield) * Paginated list across 7 items at limit=3 -> 3+3+1, last page next=null
Docs/PR5_REVIEWS_FLUENTVALIDATION_TESTING_DEFAULTS.md is the capstone showcase write-up — full wire dumps for FluentValidation rejecting both single and multiple field violations, the before/after collapse of the composition root into one AddTrellis(t => t.Use*(...)) expression, the side-by-side test-assertion style with Trellis.Testing matchers vs the older imperative style, and the orphan-stub cleanup story. Replaces the stub Docs/DomainModels/Aggregates.MenuReview.md with the actual implementation shape: state diagram (no state machine), event table, three-layer validation explanation (domain invariants + FluentValidation at command boundary + DTO parsing), wire JSON. Four new .http files: - Requests/MenuReviews/SubmitReview.http — happy path 201 - Requests/MenuReviews/SubmitReview-ValidationFailure.http — three scenarios for FluentValidation 422 (single rule, single rule, both rules aggregated) - Requests/MenuReviews/UpdateReview.http — PUT happy path + ownership note - Requests/MenuReviews/ListReviewsForMenu.http — paginated
…ared ValidateContentInputs
Drops the aggregate-wide InlineValidator<MenuReview> and the snapshot-rollback dance in favour of a static ValidateContentInputs(rating, comment) helper that runs BEFORE mutation. Both TryCreate and UpdateContent become single-expression ROP chains:
ValidateContentInputs(rating, comment).Map(inputs => { /* construct or mutate */ return this; });
Validating inputs UPFRONT means no rollback is ever needed — invalid inputs short-circuit before the aggregate's state is touched. Removes any possibility of half-applied mutation leaking through live in-memory repo references (the original reason the snapshot-rollback was introduced in PR 1's Menu.Update review fix).
Net -27 / +20 across the file. Same rules in one place, no duplication between the aggregate validator and any input validator. Other fields (Id, MenuId, DinnerId, GuestUserId) are already proven non-empty by their typed-VO constructors so the aggregate-wide validator was carrying no weight.
Same lesson applies to Menu.Update (PR 1) and Dinner.TryCreate/Cancel (PR 2). Worth a small follow-up cleanup but out of scope here.
…e for the ROP chain Replaces the hand-rolled .Ensure(...).Ensure(...).Ensure(...) chain with an InlineValidator<ContentInputs> applied via IValidator<T>.ValidateToResult — same Trellis.FluentValidation bridge the other aggregates in this codebase already use, just applied to an input record instead of 'this' so validation runs BEFORE mutation. Benefits over the .Ensure chain: - Declarative rules with FluentValidation's standard WithMessage overrides - Multiple violations aggregate into one Error.InvalidInput automatically (no manual EnsureAll wiring) - Consistent shape with the FluentValidation rule classes in Application/MenuReviews/Validators/ - Same s_validator pattern other aggregates use, just promoted to validate INPUTS not 'this'
a032c13 to
32136f9
Compare
There was a problem hiding this comment.
Pull request overview
This PR completes the showcase arc by adding the MenuReview aggregate and wiring additional Trellis V3 capabilities (Mediator FluentValidation integration, Trellis.Testing matchers, and the Trellis.ServiceDefaults composition builder). It extends the API surface with menu-review endpoints, adds in-memory persistence + pagination support for reviews, and updates docs and request samples to demonstrate the new behavior end-to-end.
Changes:
- Add
MenuReviewdomain aggregate + domain events, plus supporting in-memory repository and persistence DTO adjustments. - Introduce menu-review application commands/queries/handlers and FluentValidation validators wired into the Mediator pipeline.
- Add menu-review API endpoints + integration tests, and refactor the API composition root to use
services.AddTrellis(t => t.Use*(...))(ServiceDefaults).
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Requests/MenuReviews/UpdateReview.http | Adds an update-review HTTP demo with expected responses. |
| Requests/MenuReviews/SubmitReview.http | Adds a submit-review HTTP demo request/response shape. |
| Requests/MenuReviews/SubmitReview-ValidationFailure.http | Adds validation-failure demo scenarios for submit review. |
| Requests/MenuReviews/ListReviewsForMenu.http | Adds paginated list-for-menu HTTP demo. |
| Infrastructure/tests/MenuRepositoryTests.cs | Updates test imports to reference moved MenuReviewId namespace. |
| Infrastructure/src/Persistence/Memory/MenuReviewInMemoryRepository.cs | Adds in-memory repository for MenuReview with ETag tracking and paging. |
| Infrastructure/src/Persistence/Dto/MenuDto.cs | Updates DTO mapping to use canonical MenuReviewId namespace. |
| Infrastructure/src/DependencyInjection.cs | Registers MenuReviewInMemoryRepository for both in-memory and Cosmos fallback wiring. |
| Domain/tests/MenuReviewTests.cs | Adds domain tests using Trellis.Testing matchers for MenuReview. |
| Domain/tests/BuberDinner.Domain.Tests.csproj | Adds Trellis.Testing package reference for domain tests. |
| Domain/src/MenuReview/ValueObjects/MenuReviewId.cs | Fixes MenuReviewId into the correct MenuReview namespace. |
| Domain/src/MenuReview/Events/MenuReviewEvents.cs | Adds MenuReviewSubmitted and MenuReviewUpdated domain events. |
| Domain/src/MenuReview/Entities/MenuReview.cs | Implements MenuReview aggregate with validation + update behavior. |
| Domain/src/Menu/Menu.cs | Updates imports for moved MenuReviewId namespace. |
| Docs/PR5_REVIEWS_FLUENTVALIDATION_TESTING_DEFAULTS.md | Adds PR5 walkthrough documentation (reviews + validation + testing + defaults). |
| Docs/DomainModels/Aggregates.MenuReview.md | Replaces stub with real aggregate documentation and wire shape. |
| Directory.Packages.props | Adds/bumps central package versions (ServiceDefaults, Testing, FV adapter, FluentAssertions). |
| Application/src/MenuReviews/Validators/MenuReviewValidators.cs | Adds FluentValidation validators for submit/update commands. |
| Application/src/MenuReviews/Queries/ListReviewsForMenuQuery.cs | Adds paginated list query/handler for reviews by menu. |
| Application/src/MenuReviews/Queries/GetMenuReviewQuery.cs | Adds get-by-id query/handler for reviews. |
| Application/src/MenuReviews/Events/MenuReviewEventHandlers.cs | Adds log-only domain event handlers for review submit/update. |
| Application/src/MenuReviews/Commands/UpdateMenuReviewCommand.cs | Adds update command/handler with ownership check + persistence update. |
| Application/src/MenuReviews/Commands/SubmitMenuReviewCommandHandler.cs | Adds submit handler (fail-loud on missing menu) + persistence add. |
| Application/src/MenuReviews/Commands/SubmitMenuReviewCommand.cs | Adds submit command type. |
| Application/src/BuberDinner.Application.csproj | Adds Trellis.Mediator.FluentValidation package reference. |
| Application/src/Abstractions/Persistence/IMenuReviewRepository.cs | Introduces IMenuReviewRepository contract with paging method. |
| Api/tests/BuberDinner.Api.Tests.csproj | Adds Trellis.Testing package reference for API tests. |
| Api/tests/2022-12-21/MenuReviewTests.cs | Adds end-to-end integration tests for submit/update/get/list review scenarios. |
| Api/src/Program.cs | Removes explanatory comment around middleware ordering (keeps behavior). |
| Api/src/DependencyInjection.cs | Refactors composition root to AddTrellis(...Use*...) + wires review route constraint + event handlers. |
| Api/src/BuberDinner.Api.csproj | Adds Trellis.ServiceDefaults package reference. |
| Api/src/2022-12-21/Models/MenuReviews/MenuReviewDtos.cs | Adds menu-review request/response DTOs + Mapster mapping config. |
| Api/src/2022-12-21/Controllers/MenuReviewsController.cs | Adds menu-review endpoints (submit/get/update/list) with ETag + pagination. |
| .github/trellis-api-testing.md | Adds Trellis.Testing API reference drop for repo-local guidance. |
| .github/trellis-api-servicedefaults.md | Adds Trellis.ServiceDefaults API reference drop for repo-local guidance. |
| .github/trellis-api-mediator-fluentvalidation.md | Adds Trellis.Mediator.FluentValidation API reference drop for repo-local guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| `services.AddTrellis(t => t.UseFluentValidation(typeof(SubmitMenuReviewCommandValidator).Assembly))` | ||
| scans the assembly, registers every `IValidator<T>` as scoped, and plugs them into | ||
| the existing `ValidationBehavior<,>` mediator slot. Failures aggregate into a single | ||
| `Error.InvalidInput`, which the ASP boundary serialises as RFC 4918 Problem Details. |
Comment on lines
+58
to
+63
| MenuReview? review; | ||
| lock (s_lock) | ||
| review = s_reviews.SingleOrDefault(r => r.Id.Value.ToString() == id); | ||
| if (review is not null && s_etags.TryGetValue(id, out var etag)) | ||
| AggregateETagWriter.SetETag(review, etag); | ||
| return ValueTask.FromResult(review); |
Comment on lines
+50
to
+55
| .Ensure( | ||
| r => r.GuestUserId == callerGuestUserId, | ||
| new Error.NotFound(ResourceRef.For<MenuReview>(reviewId)) | ||
| { | ||
| Detail = "Review does not belong to the calling guest.", | ||
| }); |
…dator
Same lesson as MenuReview.UpdateContent earlier in this PR — validate inputs BEFORE mutating so a rejected update never touches aggregate state. Drops the snapshot-rollback dance introduced in PR 1's review fix:
Before: snapshot + assign + s_validator.ValidateToResult(this) + rollback-on-failure (15 lines)
After: s_updateInputValidator.ValidateToResult(new UpdateInputs(...)).Map(inputs => { /* mutate */; return this; }) (8 lines)
Sweep summary across the rest of the codebase — only Menu.Update matched this anti-pattern. Other s_validator.ValidateToResult call sites (Menu.TryCreate, Dinner.TryCreate, Reservation.TryCreate, Host.TryCreate, User.TryCreate, MenuItem.TryCreate, MenuSection.TryCreate) are construct-then-validate factories where the local is discarded on failure (no state leak), and Dinner.Cancel/Reservation.Cancel already use FireResult().Map(...) so mutation only happens on success. No further sweeps needed.
8cff609 to
b8ef2ea
Compare
…view) Three fixes from the final review pass. Medium 1 - SubmitMenuReviewCommandHandler now verifies the full attendance chain. Previously accepted any DinnerId without checking the dinner existed, belonged to the menu, or that the guest reserved it (orphan-FK / IDOR-lite). Added 4 ROP gates after the existing menu-exists check, ordered so 404 leak-shields fire BEFORE status-revealing 422s: 1. Menu exists (existing) -> 404 2. Dinner exists -> 404 3. Dinner.MenuId == request.MenuId -> 404 (leak-shielded) 4. Caller has non-cancelled reservation -> 404 (leak-shielded) 5. Dinner.Status == Ended -> 422 review.dinner-not-ended Adds IReservationRepository.FindByDinnerAndGuest(DinnerId, UserId) + in-memory impl using FirstOrDefault (dupes possible since uniqueness is not enforced - out of scope here). Filters for ReservationStatus.Reserved so cancelled reservations do not grant review rights. 5 new negative-path API tests cover: missing dinner (404), dinner-not-matching- menu (404), no reservation (404), upcoming dinner (422 with rule code), cancelled reservation (404). Existing happy-path tests updated to use a new SeedReadyToReviewAsync helper that reserves + starts + ends the dinner. Medium 2 - Aggregates.MenuReview.md: replaced the inaccurate claim that s_validator enforces non-empty IDs with an accurate four-section breakdown of where validation actually lives (content invariants on the ContentInputs record, FK integrity at the wire DTO, cross-aggregate gates in the handler, command-boundary FluentValidation). Low 3 - PR5 walkthrough Problem Details example: input rating=99 comment="x" cannot trigger MaxLength(1000). Changed body to rating=99 comment="" and the second error message to "Comment is required." so the example matches what the API actually emits. Also gitignored .github/trellis-api-efcore.md - same transitive-doc problem as trellis-api-testing-reference.md (BuberDinner does not reference Trellis.EntityFrameworkCore).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR 5 — Reviews + FluentValidation + Trellis.Testing + Trellis.ServiceDefaults
TL;DR
MenuReviewaggregate (Submit,UpdateContent)Domain/src/MenuReview/Entities/MenuReview.csMenuReviewSubmitted,MenuReviewUpdated)Domain/src/MenuReview/Events/POST /menu-reviews·PUT /menu-reviews/{id}·GET /menu-reviews/{id}·GET /menu-reviews/for-menu/{menuId}MenuReviewsControllerAbstractValidator<TCommand>+ auto-wired Mediator behaviorSubmitMenuReviewCommandValidator,UpdateMenuReviewCommandValidatorTrellis.Testingmatchers (.Should().BeSuccess() / .BeFailureOfType<>() / .Unwrap())Domain/tests/MenuReviewTests.csTrellis.ServiceDefaultscomposition builder —AddTrellis(t => t.Use*(...))replaces 10+ individualAdd*callsApi/src/DependencyInjection.csMenuReviewIdin the wrong namespaceMenu.cs,MenuDto.cs,MenuRepositoryTests.csFluentValidation kicking in at the wire
Both
SubmitMenuReviewCommandandUpdateMenuReviewCommandhave a matchingAbstractValidator<TCommand>inApplication/MenuReviews/Validators/. OnceUseFluentValidation(typeof(SubmitMenuReviewCommandValidator).Assembly)is wired,the framework scans the assembly, registers every
IValidator<T>as scoped, andplugs them into the existing
ValidationBehavior<,>mediator slot. Failuresaggregate into a single
Error.InvalidInputwhich the ASP boundary serialises asRFC 4918 Problem Details. Per-handler
AddScoped<IValidator<X>, XValidator>()isalso supported (AOT-safe variant).
The composition root before vs after
Before (PR 4 tip, ~70 lines of
services.Add*(...)inApi/DependencyInjection.cs):After (PR 5, one fluent
AddTrellis(...)expression):The builder fixes the mutually-required call order so misconfigurations fail at
startup, not at request time. Per-type registrations (
AddTrellisRouteConstraint<HostId>,AddDomainEventHandler<DinnerScheduled, LogDinnerScheduledHandler>,AddInMemoryIdempotencyStore,AddSingleton(TimeProvider.System)) stay separate —those are "application-owned" per the docs and not covered by the builder.
Test-side cleanup with
Trellis.TestingmatchersOlder Dinner/Reservation tests kept in their existing style to avoid churn — the 8
new
MenuReviewTestsuse the new matchers exclusively so future authors have aside-by-side reference.
Domain-aggregate cleanup (drive-by)
A pre-existing stub
Domain/src/MenuReview/ValueObjects/MenuId.csdefinedMenuReviewIdin the wrong namespace (BuberDinner.Domain.Menu.ValueObjectinstead of
BuberDinner.Domain.MenuReview.ValueObject).Menu.csconsumed itthrough that wrong namespace, papering over the bug. PR 5 deletes the stub, adds
the canonical
MenuReviewId.csunder the right namespace, and updates the 3consumers.
Build, test
MenuReviewTestsusingTrellis.Testingmatchers)MenuReviewTestscovering all wire scenarios)Wire scenarios covered by
MenuReviewTests:errors.Rating(FluentValidation)errors.Comment(FluentValidation)errors.Rating(validator runs on Update too)next=nullCommit walkthrough (7 commits)
360a8a2chore(deps): 3 new packages + FluentAssertions 6.12 → 7.2.2 (Trellis.Testing requirement)073c298feat(domain): MenuReview aggregate + 2 events + 8 tests + orphan stub cleanup1dc771efeat(infra+app): repo + 4 commands/queries + 2 FluentValidation rules + 2 log handlers995e45afeat(api): MenuReviewsController + DTOs26828bfrefactor(api): collapseAdd*composition intoAddTrellis(t => t.Use*(...))builder6f2a336test(api): 7 integration testsf157b4edocs: walkthrough + 4.httpdemos + Aggregates.MenuReview.mdRoadmap complete
This is the final PR in the showcase arc. After merge, BuberDinner exercises every
Trellis V3 package shipped today except
Trellis.EntityFrameworkCore:Trellis.Core·Trellis.Primitives(PR Migrate to Trellis 3.0.0-alpha.342 on .NET 10 (closed Error union, AddTrellisBehaviors, 3-of-3 framework regressions adopted) #26)Trellis.Asp·Trellis.Http.Abstractions·Trellis.Authorization(PR feat: ETag conditional requests + resource-based authorization + scalar VO route constraints #27)Trellis.Mediator·Trellis.StateMachine(PR feat: Dinner aggregate with Stateless state machine + per-transition domain events #28)Trellis.Mediator.FluentValidation·Trellis.Testing·Trellis.ServiceDefaults(PR feat: Reservations aggregate + IETF Idempotency-Key middleware + multi-aggregate fail-loud #30 + this)In-memory persistence is intentional for the showcase. An EF Core swap is the obvious
clean follow-up —
IRepository<T>is the only contact point the application layersees, so swapping the implementation requires no Application/Api changes.