feat(binder): configure a host-agnostic default for Bind.PropertiesOf - #183
Conversation
Bind.PropertiesOf binds with RequestBinderOptions.Default, which was a fixed built-in. Make it a settable application default, configured once at startup (no DI container needed, so a CLI or worker can set it) and frozen on first use: the first bind reads it, and a later assignment throws, so it cannot drift once requests are flowing (the JsonSerializerOptions discipline). Bind gains no configuration surface; a per-call Bind.WithOptions still overrides it. A scoped, test-only override (AsyncLocal) keeps the suite parallel-safe. Refs: #181
Record the decision behind #181: RequestBinderOptions.Default is a settable application default, configured once at startup and frozen on first use. It revisits the process-wide ambient default that ADR-0012 had weighed and rejected as an alternative, adopting it with mitigations; ADR-0012's own entry-point-fix decision is unchanged, so ADR-0012 is not superseded. Drafted as Proposed and indexed (EN + FR); the maintainer decides its status. Refs: #181
The maintainer accepts the decision recorded in ADR-0017: the binder's options default is a settable application default, configured once at startup and frozen on first use. Flip its status from Proposed to Accepted (EN + FR) and update the index. Refs: #181
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a74f384183
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A late Default assignment could race the first bind's read: the setter observed _frozen == false, the getter then set _frozen = true and returned the old default, and the setter still assigned -- leaving the first binding on the old options and later bindings on the new, violating the frozen-on-first-use guarantee. Guard the freeze/check/assign with a lock so a concurrent set either wins before any read froze the default, or observes the freeze and throws. Refs: #181
|
@codex no more issues ? |
|
No blocking issue found in my follow-up review. The previously reported concurrency issue appears resolved: I also checked the related tests around the scoped test override, default fallback, per-call override, null rejection, and frozen-after-first-use behavior; I did not find another actionable issue in that area. FirstClassErrors.RequestBinder.UnitTests/DefaultOptionsTests.csL30-L95 Testing / checks
|
2 participants
Summary
Bind.PropertiesOf(request)binds withRequestBinderOptions.Default, which was a fixed built-in — so the only way to bind with custom options (a naming policy, the overridable structural codes from #147) wasBind.WithOptions(...), threading the configured entry point through every call or resolving it from DI. This makesRequestBinderOptions.Defaulta settable application default, configured once at startup with no DI container needed (a CLI or worker can set it), and frozen on first use so it cannot drift once requests are flowing.Bindgains no configuration surface; a per-callBind.WithOptions(...)still overrides it.Type of change
Changes
RequestBinderOptions.Defaulta settable property (was get-only). Setting it rejectsnulland throws once the first bind has read it (frozen on first use — theJsonSerializerOptionsdiscipline), so it is configured at composition time and cannot change at runtime. The value is an immutableRequestBinderOptions, so there is no shared-mutable-settings hazard.Bind.PropertiesOfalready readsRequestBinderOptions.Default, soBindis unchanged — no configuration method pollutes theBind.surface.Bind.WithOptions(o)still overrides per call.OverrideDefaultForTests, backed by anAsyncLocal) so the library's own suite stays parallel-safe without mutating the process default — the same seam pattern as the ambient clock.DefaultOptionsTests(7 tests):Bind.PropertiesOfbinds with the configured default (naming + codes) withoutWithOptions; falls back to the built-in outside the scope; a per-callWithOptionswins; the setter rejectsnulland throws after first use.Testing
dotnet build FirstClassErrors.sln— succeeded, 0 warnings / 0 errors.dotnet test FirstClassErrors.sln— all green (998 total, 0 failed); the freeze andAsyncLocalseam do not leak into the other binder tests.FirstClassErrors.Analyzers.UnitTests) — 85 passed.Documentation
RequestBinderOptions.Defaultand the test seam.doc/updated — RequestBinder guide (EN), new "Configuring the default for the whole application" section.RequestBinder.fr.md) and ADR-0017 (.fr.md), in lockstep.Architecture decisions
Proposed: ADR-0017ADR-0017 records the decision and is
Accepted(per the maintainer). It revisits the process-wide ambient default that ADR-0012 had weighed and rejected as an alternative, adopting it with mitigations (frozen-on-first-use, immutable value,AsyncLocaltest seam). ADR-0012 is not superseded — its own decision (a binder's options are fixed at its entry point) is unchanged and staysAccepted.Related issues
Closes #181
🤖 Generated with Claude Code
Generated by Claude Code