Remove default value attribute and use a single property - #3
Merged
Conversation
guy-lud
added a commit
that referenced
this pull request
Jul 15, 2026
…ype (PR #33 comment #3) Reuse the existing section attribute for the object-level validator instead of a separate SettingsValidatorAttribute (now deleted; it was unreleased, so not a breaking change). ValuesPopulator reads the validator Type from SettingsSectionAttribute.ValidatorType; the dispatch and value-free exception wrapping are unchanged. Because [SettingsSection] is the scan-discovery marker, the deliberately-failing validation fixtures would now be eagerly built by the whole-assembly ScanAssemblies tests. Making them internal is not viable: the Reflection.Emit proxy generator emits into a random-GUID dynamic assembly that cannot implement an internal interface (verified: TypeLoadException). So the fixtures move to a new, never-scanned ExistForAll.SimpleSettings.UnitTests.Fixtures library, referenced by UnitTests but never passed to ScanAssemblies. Adds a regression test that the merged validator also runs under the assembly-scan path. Full suite 143/143 on net10; build clean on net8 + net10.
guy-lud
added a commit
that referenced
this pull request
Jul 15, 2026
* test(04-01): add failing tests for List<T>-family conversion
- Add fixture interfaces for List/IList/ICollection/IReadOnlyList/IReadOnlyCollection<int>
- Assert each materializes a List<int> from a delimited scalar
- Add per-element DayOfWeek list test (element converter chain)
* feat(04-01): add List<T>-family converter with disjoint shape predicates
- Add IsListLike/IsCollectionShape predicates disjoint from IsEnumerable/IsArray
- Add ListTypeConverter subclassing CollectionTypeConverter; materialize via cached per-element factory delegate (no warm-path reflection, S-4/A1)
- Promote CollectionTypeConverter.Convert to virtual
- Register ListTypeConverter after EnumerableTypeConverter, before EnumTypeConverter (disjoint, order preserved)
* test(04-01): add failing tests for empty-not-null collection defaults
- Unbound int[] -> empty int[], unbound List<int> -> empty List<int>
- Regression: unbound IEnumerable<int> -> empty int[]
- B-3: two null-binds of List<int> return reference-distinct, mutation-isolated instances
* feat(04-01): empty-not-null default for every collection shape
- CreateNullResult branches IsArray -> IsEnumerable -> IsListLike -> value-type/null
- Array/IEnumerable keep the shared cached empty array
- List family binds a fresh empty List<T> per bind via a baked factory delegate (B-3), reusing the existing _nullResult slot so PropertyPlan[] layout is unchanged
- PropertyConversion.Convert invokes the factory when _nullResult is Func<object>
* test(04-02): add failing child-sequence bind tests for ConfigurationBinder
- child-section sequence binds string[]/int[]/List<string> elements
- children win over coexisting scalar
- comma-scalar regression guard (prod override)
- whitespace scalar and empty sequence bind empty (no [" "] token)
- RootSection prefix honored on the sequence path
* feat(04-02): bind collections from IConfiguration child-section sequences
- grant InternalsVisibleTo to the Binders assembly so ConfigurationBinder
reuses Core's internal IsCollectionShape predicate (single source of truth)
- add GetChildren() branch: indexed sub-sections materialize a right-sized
string[] via a manual two-pass walk (no LINQ on the bind path)
- children win over a coexisting scalar; empty sequence falls through
- whitespace/empty scalar on a collection target skips SetNewValue so COLL-02's
empty-not-null default applies (avoids the [" "] token pitfall)
- element chain and RootSection prefix inherited unchanged
* test(04-02): D-06 secret redaction on the child-sequence bind path
- Convert_SecretInSequenceElement_DoesNotLeakValue: int[] secret in a later
element is absent from the whole SettingsPropertyValueException.ToString()
- first-element case (S-6): element position must not matter
- List<int> case (S-6): redaction holds across the array and list converter shapes
- drives the new ConfigurationBinder GetChildren() sequence path, not the scalar
InMemoryBinder path
* feat(04-03): sync validation contracts, context ctors, validator attribute, aggregate exception
- ISettingsValidator/ISettingValidation<T>.Validate return ValidationResult (drop Task<>) (D-07)
- ValidationContext(object)/ValidationContext<T>(T) constructors carry the settings instance (D-08)
- SettingsValidatorAttribute (AttributeTargets.Interface, Type ValidatorType) (D-11)
- SettingsValidationException : SimpleSettingsException + static ThrowIfAny shared aggregate-and-throw (D-10/S-3)
- Resources.SettingsValidationExceptionMessage composed only from author ValidationError text (D-12)
* test(04-03): add failing tests for core-path settings validation
- object-level [SettingsValidator] and property-level [SettingsProperty(ValidatorType)] invocation
- multi-error aggregation into one SettingsValidationException
- cross-property rule observing the fully-populated instance
- D-12 value-free exception message
- B-2 validator-free short-circuit fast path
* feat(04-03): run declared validators in the core populate path
- thread object-level [SettingsValidator] and property-level [SettingsProperty(ValidatorType)] into the cached plan at build (D-11)
- SettingsPlan.HasValidators computed once at build; post-populate hook short-circuits before any allocation, error list allocated lazily (review B-2)
- validators run after the property-set loop so cross-property rules see the full instance (D-09)
- reflective dispatch selects the Validate overload via GetMethod(name, new[]{closedContextType}) + MakeGenericType context, avoiding AmbiguousMatchException (review S-2)
- aggregate-and-throw routed through shared SettingsValidationException.ThrowIfAny (review S-3/D-10)
* test(04-05): add failing AllowEmpty=false empty/whitespace rejection tests
- AllowEmpty=false rejects "" and whitespace via SettingsPropertyNullException (value-free)
- AllowEmpty=true still binds "" and whitespace as-is
* feat(04-05): reject empty/whitespace strings for AllowEmpty=false (VAL-02)
- Convert now throws value-free SettingsPropertyNullException for null or null-or-whitespace strings when AllowEmpty=false
- AllowEmpty=true falls through to normal conversion (empty/whitespace bound as-is)
- 04-01 list null-result factory dispatch preserved
* fix(04-02): single-pass child-sequence bind — reload-safe, drops empty elements
- Enumerate the live config view once (a second GetChildren() pass could race a
reload and overrun the count-sized array or inject null holes) — review MED-1.
- Drop empty entries for parity with the comma-scalar split (RemoveEmptyEntries),
so ["1","","3"] no longer diverges from "1,,3" and crashes int[]/List<int> — review MED-2.
- PR #33 review comment 1.
* refactor(04-05): check value==null once in PropertyConversion.Convert
- Consolidate the two null branches into one block (throw if required, else the
null-result/factory), keeping the whitespace-string reject and the AllowEmpty=true
fall-through intact. PR #33 review comment 2.
* refactor(04-03): dispatch validators via ISettingValidation<T> DIM bridge; guard invocation value-free
- ISettingValidation<T> default-implements the base Validate, forwarding to the generic overload, so
ValuesPopulator dispatches through a plain ISettingsValidator cast — no MakeGenericType/GetMethod/Invoke
reflection and no AmbiguousMatchException workaround (PR #33 comment 4).
- Wrap validator invocation: a validator that throws is surfaced as the value-free
SettingsValidatorInvocationException (never chains the inner, which may hold a secret) — review MED-3.
- ValidationContext ctor accepts object? (drops the !); fixtures implement only the generic Validate; add
property-validator positive/discriminating, object+property aggregation, and throwing-validator coverage.
* test(04-02): lock child-sequence element order + cover empty-element drop
- Assert order-sensitive SequenceEqual on indexed-sequence cases (IsEquivalentTo was order-insensitive
and would not catch an order regression) — review test-gap T1.
- Add an interspersed-empty-element case proving parity with the comma-scalar — review MED-2.
* chore(04): strip internal review-ID tokens from code comments
* refactor(04): fold object validator onto [SettingsSection].ValidatorType (PR #33 comment #3)
Reuse the existing section attribute for the object-level validator instead of a separate
SettingsValidatorAttribute (now deleted; it was unreleased, so not a breaking change).
ValuesPopulator reads the validator Type from SettingsSectionAttribute.ValidatorType; the
dispatch and value-free exception wrapping are unchanged.
Because [SettingsSection] is the scan-discovery marker, the deliberately-failing validation
fixtures would now be eagerly built by the whole-assembly ScanAssemblies tests. Making them
internal is not viable: the Reflection.Emit proxy generator emits into a random-GUID dynamic
assembly that cannot implement an internal interface (verified: TypeLoadException). So the
fixtures move to a new, never-scanned ExistForAll.SimpleSettings.UnitTests.Fixtures library,
referenced by UnitTests but never passed to ScanAssemblies.
Adds a regression test that the merged validator also runs under the assembly-scan path.
Full suite 143/143 on net10; build clean on net8 + net10.
guy-lud
added a commit
that referenced
this pull request
Jul 19, 2026
…sure (VAL-01 DI path, API-02) (#35) * docs: refresh session handoff (Phase 4 Waves 1-2 + comment #3 merged; Wave 3 next) * feat: expose ISettingsCollection from AddSimpleSettings (API-02/D-15) Register the built ISettingsCollection as a DI singleton and add an AddSimpleSettings(out ISettingsCollection, Action?) overload that surfaces the same instance while preserving the IServiceCollection fluent chain. * feat: DI-resolved settings validator path (VAL-01 DI path) Add a deferred, opt-in ISettingsValidationRunner resolved via IServiceProvider.ValidateSimpleSettings(). It resolves DI-registered ISettingValidation<T> from a fresh scope (so scoped-dependency validators work), dispatches through the ISettingsValidator cast (the default-interface bridge, no reflection), and aggregates via the shared SettingsValidationException.ThrowIfAny so the thrown contract is identical to the core populate path. A throwing validator surfaces value-free as SettingsValidatorInvocationException (type-only, no bound value, no inner). * refactor: address Wave 3 review findings - ValidateSimpleSettings surfaces a caller-facing error when AddSimpleSettings was not called (internal runner type no longer leaks into the message). - Trim runner comments to one line each; drop internal planning-ID reference. - Harden tests: deferral test is now an invocation-counter timing probe (0 runs at build, 1 on the explicit call); scope test asserts the scoped validator actually executed; redaction test pins that the secret is bound; add no-op (no DI validators) and misuse coverage. * docs(planning): Phase 4 Wave 3 (04-04) summary, review, plan patch, state Record the VAL-01 DI path + API-02 execution: 04-04-SUMMARY.md, the gsd-code-reviewer 04-04-REVIEW.md, STATE.md (all 5 Phase 4 plans landed; verify+secure pending), and patch the plan's superseded reflective-dispatch text to the shipped default-interface-bridge dispatch.
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
1 participant
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.
No description provided.