feat: add request validation handler with FluentValidation provider (#4175)

[xbizzybone]

What

Adds request validation to Brighter as an optional, additive concern, with FluentValidation as the first provider — the starting point suggested in the issue.

Contributes to #4175

Design

Two assemblies so a single [ValidateQuery] attribute serves every framework, while users only take the dependency they need:

Paramore.Brighter.Validation (provider-agnostic, depends only on the core)

• [ValidateQuery] / [ValidateQueryAsync] — extend RequestHandlerAttribute, mirroring RequestLoggingAttribute; they target the abstract base handlers.
• ValidateRequestHandler<TRequest> / ValidateRequestHandlerAsync<TRequest> — abstract base handlers that own the shared behaviour: null-guard the request, ask the provider for failures, throw before calling next, otherwise call base.Handle/base.HandleAsync.
• RequestValidationException carrying a framework-agnostic IReadOnlyCollection<ValidationError> (property, message, attempted value, error code), so a caller catches one exception type regardless of provider.

Paramore.Brighter.Validation.FluentValidation (this PR's provider)

• FluentValidationRequestHandler<TRequest> / ...Async derive from the base handlers and resolve a FluentValidation IValidator<TRequest> from the container, mapping its failures onto ValidationError.
• UseFluentValidation() maps the abstract handlers to these implementations. The core is not modified.

Because [ValidateQuery] targets the abstract handler and the provider package maps it to a concrete one, adding Paramore.Brighter.Validation.DataAnnotations or ...Specification later is purely additive — a new concrete handler plus a UseX() registration, with no change to the abstractions or to this package.

A request marked [ValidateQuery] with no registered validator throws ConfigurationException (Brighter's existing misconfiguration type), fail-fast, rather than silently skipping validation. An ADR (0063) is included as the first commit, per CONTRIBUTING.

Scope / additive guarantees

• Two new assemblies + a new test project. The two edits to existing files are unavoidable wiring with no logic change:
  • Brighter.slnx — register the new projects.
  • Directory.Packages.props — central version for FluentValidation (11.12.0, the last 11.x line that still supports netstandard2.0, which Brighter targets).
• Targets netstandard2.0;net8.0;net9.0;net10.0; strong-named with Brighter.snk.

Tests

xUnit, one behavior per file. Adversarial coverage through the FluentValidation provider: valid request (sync + async), single and multiple validation errors, missing validator (ConfigurationException), a validator that throws (propagated, not swallowed), null request (ArgumentNullException), cancellation-token propagation in the async handler, the exact shape of RequestValidationException, and concurrent reuse of a single handler instance. All green on net10.0 locally; CI covers every TFM.

Open questions for the maintainer

1. Naming. The issue names the attribute [ValidateQuery], but the concern validates commands as well as queries. Keep it, or rename to [ValidateRequest] (optionally keeping [ValidateQuery] as an alias)? Implemented as specified.
2. Placement. The abstractions live in a new Paramore.Brighter.Validation package; they depend only on the core, so they could equally live in the core assembly. Which do you prefer?
3. Issue. This delivers FluentValidation only; I used "Contributes to" rather than "Closes" so you can keep Add a Validation Handler to Brighter #4175 open to track DataAnnotations / Specification.
4. Darker. The same requirement exists for Darker V5; out of scope here, happy to follow up.

Contributor License Agreement

Signed via CLA assistant — the license/cla check is green.

Minor docs note: CONTRIBUTING.md still links to clahub.com for the CLA, but ClaHub has shut down; the CLA assistant GitHub App is what actually runs on PRs. Might be worth updating that link in a follow-up.

Type of Change

• New feature (non-breaking change which adds functionality)

Checklist

• I have read the Contributing Guide
• I have checked the documentation for relevant guidance
• I have added/updated XML documentation for any public API changes
• I have added/updated tests as appropriate
• My changes follow the existing code style and conventions

Additional Notes

User-facing documentation for the BrighterCommand docs site can follow as a separate change once the API here is confirmed.

[CLAassistant]

All committers have signed the CLA.

[iancooper]

Thanks for this contribution! It helps a lot. It looks good and we should try to get it out asap.

1. Let's fold the abstractions into Paramore. Brighter, within a sub-folder. They depend only on the core and carry no third-party reference. Since RequestLoggingAttribute and friends already live in core, this follows the same pattern for built-ins. We should still ship providers separately.
2. Let's name these [ValidateRequest] and not [ValidateQuery]; they query name is a holdover from writing up the issue for Darker. Brighter has requests, not queries.
3. Miguel Ramirez in both src csproj files overrides the repo-wide Authors=Ian Cooper / Company=Brighter Command from src/Directory.Build.props. I'm not trying to steal your work, it just helps us attribute official packages. Your attribution belongs in the file header's copyright notice, not in the package author field. You can also add yourself to the contributors file at the root (saves me the job)
4. There is no friendly error from ValidatePipelines() when UseFluentValidation() is omitted, but we have attributed handlers. We can put that in a separate issue/PR, though
5. UseFluentValidation() itself is never exercised.

[iancooper]

Let's leave Darker for now, as it has a lot of structural changes for V5, but I can tag you there when it is ready to go. You are welcome to push implementations of DataAnnotations and Specifications here, too.

[xbizzybone]

Thanks for the thorough review, @iancooper — really appreciate it. I've pushed the changes addressing all your points:

1. Folded into the core. The abstractions now live in Paramore.Brighter under RequestValidation/ (with Attributes/ + Handlers/ sub-namespaces, mirroring Logging/Inbox); the standalone Paramore.Brighter.Validation package is gone and the providers stay separate. One thing to flag: I renamed the error type to RequestValidationError to avoid clashing with the existing Paramore.Brighter.ValidationError from the startup pipeline-validation (ADR 0053). I also used file-scoped namespaces to match the current core style.
2. [ValidateRequest] — renamed (and [ValidateRequestAsync]); the "query" name is gone.
3. Authors — dropped the <Authors> override so the repo-wide Authors/Company from src/Directory.Build.props apply, and added myself to CONTRIBUTORS.md.
4. Friendly ValidatePipelines() error — left as you suggested; happy to follow up in a separate issue/PR.
5. UseFluentValidation() coverage — added a test that drives the real AddBrighter().UseFluentValidation() through the pipeline (previously the handlers were only wired by hand in the test harness).

I also took you up on the offer to add the other two providers in this PR:

• Paramore.Brighter.Validation.DataAnnotations — validates the request's own System.ComponentModel.DataAnnotations attributes (no per-request validator, no extra dependency — the types ship with the framework).
• Paramore.Brighter.Validation.Specification — built on your Specification pattern (ADR 0040): resolves an ISpecification<TRequest> and collects failures via the public ValidationResultCollector. I documented that the specification should be registered with a per-request lifetime, since Specification<T> carries per-evaluation state.

The registration methods are UseFluentValidation() / UseDataAnnotations() / UseSpecification(). Each provider has the same adversarial test coverage plus sync and async UseX() wiring tests. Darker — left out for now, tag me when V5 is ready.

[iancooper]

This looks really good, and the comments here are mostly about creating clarity rather than providing functional fixes. As this is the first PR for this, it would be nice to get the baseline right.

It would be useful to have a simple sample for these. It can be something as simple as /Users/ian_hammond_cooper/CSharpProjects/github/xbizzybone/Brighter/samples/CommandProcessor but in a separate folder Validation and include some samples that support the different validation types that we have.

The samples will also help us create the docs

[iancooper]

Can we rename this? I found it confusing on first review, as it's a CommandProcessorBuilder, not the pipeline doing validation, which Brighter itself provides.

[xbizzybone]

Done — renamed ValidationPipeline → CommandProcessorHarness. That drops the misleading "pipeline" (Brighter owns the validation pipeline) and names it for what it actually is: a builder whose With(...)/WithAsync(...) factories assemble a real CommandProcessor. Updated the XML doc and all call sites (the local pipeline is now harness).

[iancooper]

Really, what you are testing here is that we don't throw an exception. The fact that the test returns the same request is just an artifact of the path having completed; it's not what you are trying to confirm. Assert.True() would work here as well, because if an exception were thrown, you would throw an exception from the test.

[xbizzybone]

Agreed, the request-equality was incidental. Switched to Assert.Null(Record.Exception(() => handler.Handle(validRequest))) — the repo's existing "does not throw" idiom — so the test confirms exactly that. I applied the same fix to the four sibling happy-path tests (DataAnnotations + Specification, sync/async) that shared the Assert.Same pattern, and renamed them ..._should_return_the_request → ..._should_not_throw to match the intent.

[xbizzybone]

Thanks @iancooper — addressed the review:

• Renamed the ValidationPipeline test double → CommandProcessorHarness. It's a builder for the CommandProcessor (Brighter owns the actual validation pipeline), and the name now reflects that it bundles the processor with the HandlerReceipt. Doc comment and call sites updated.
• Valid-request tests now assert "does not throw" via Assert.Null(Record.Exception(...)) instead of returning the request; renamed the four affected happy-path tests to ..._should_not_throw.
• Added a sample as suggested: samples/Validation/ (modeled on samples/CommandProcessor) with three console apps — FluentValidationSample, DataAnnotationsSample, SpecificationSample — each sending one valid request (reaches the handler) and one invalid request (rejected with the collected RequestValidationException.Errors), plus a README on when to use each provider and the async variant. Verified each runs end-to-end.

[codescene-delta-analysis]

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

[iancooper]

Thanks for this contribution @xbizzybone. It is much appreciated. Feel free to ping us about other issues that you might want to pick up, and we will let you know. Typically, look for ones that are unassigned. Avoid the gateway for a couple of weeks, as we want to get the generic test framework in