fix(binder): lock binder options before any property is bound - #174
Merged
Conversation
RequestBinder.WithOptions could be called mid-binding, swapping the IArgumentNameProvider after some properties were already bound, so a single failure envelope could mix argument paths under two naming policies (GuestEmail beside guest_email) — silently inconsistent for a client mapping paths back to the keys it sent. Move option configuration to the entry point: Bind.WithOptions(options) .PropertiesOf(request) fixes the options once, before the binder exists, and the instance WithOptions is removed. The mid-binding swap is now impossible to write rather than merely discouraged, and Options is immutable. The configured entry point (ConfiguredBind) carries no per-request state, so it can be built once and reused per request; options no longer depend on the request type. Refs: #145
Record the public-API-contract decision behind #145: the binder's options are fixed once at the entry point (Bind.WithOptions(options).PropertiesOf) before binding begins, keeping the library's no-global-mutable-state stance and rejecting a static Configure. Drafted as Proposed and indexed (EN + FR); the maintainer decides its status. Refs: #145
The maintainer accepts the decision recorded in ADR-0012: the binder options are fixed once at the entry point before binding begins. Flip its status from Proposed to Accepted (EN + FR) and update the index. Refs: #145
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
2 participants
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.
Summary
RequestBinder.WithOptionscould be called mid-binding, swapping theIArgumentNameProviderafter some properties were already bound — so one failure envelope could report argument paths under two naming policies (GuestEmailbesideguest_email), silently inconsistent for a client mapping paths back to the keys it sent. Option configuration moves to the entry point (Bind.WithOptions(options).PropertiesOf(request)), fixed once before the binder exists; the instanceWithOptionsis removed, so the mid-binding swap is now impossible to write rather than merely discouraged.Type of change
Changes
Bind.WithOptions(options)returning a newConfiguredBindentry point whosePropertiesOf(request)starts binding with those options.Bind.PropertiesOf(request)keeps the default options.RequestBinder.WithOptions;Optionsis now immutable. Because the mid-binding setter is gone, an inconsistent envelope cannot be written.RequestBinderEnvelopeStagesoFailWithbuilds the binder with the configured options; nested binders keep inheriting them.ConfiguredBindcarries no per-request state, so it can be built once (e.g. at application startup) and reused for every request. Options no longer depend onTRequest.Bind.WithOptions(null!)), and add a test pinning the reuse-across-requests property (binder suite: 76 tests).Bind.WithOptions(...).PropertiesOf(...)example and the "configure once, reuse per request" note.Testing
dotnet build FirstClassErrors.sln— succeeded, 0 warnings / 0 errors.dotnet test FirstClassErrors.sln— all green (Core 423, GenDoc 155, Dummies 128, Analyzers 85, RequestBinder 76, Cli 64, Property 21), 952 total, 0 failed.FirstClassErrors.Analyzers.UnitTests) — 85 passed (run as part of the full-solution suite above).Documentation
Bind.WithOptions,ConfiguredBind, and the immutableOptions.doc/updated — RequestBinder guide (EN).doc/handwritten/for-users/RequestBinder.fr.md) and ADR-0012 (.fr.md) updated in lockstep.Architecture decisions
Proposed: ADR-0012ADR-0012 records the public-API-contract decision (options fixed at the entry point) and, per the maintainer's direction, is set to
Acceptedin this PR. It keeps the library's no-global-mutable-state stance (ADR-0006) and explicitly rejects a staticConfigure, leaving the application-level configuration ergonomic to the future ASP.NET Core / DI integration.Related issues
Closes #145
🤖 Generated with Claude Code
Generated by Claude Code