From 49b307c22bd79fc410ddb90bdaf53fbbe1514cf9 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 14:28:02 +0300 Subject: [PATCH 01/18] test(04-01): add failing tests for List-family conversion - Add fixture interfaces for List/IList/ICollection/IReadOnlyList/IReadOnlyCollection - Assert each materializes a List from a delimited scalar - Add per-element DayOfWeek list test (element converter chain) --- .../Conversion/CollectionConversionTests.cs | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/CollectionConversionTests.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/CollectionConversionTests.cs index 163bfe4..ef84b43 100644 --- a/src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/CollectionConversionTests.cs +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/CollectionConversionTests.cs @@ -86,6 +86,59 @@ public async Task Convert_UnboundEnumerable_NoDefault_YieldsEmptyArray() await Assert.That(result.Values is int[]).IsTrue(); } + [Test] + public async Task Convert_DelimitedString_ToIntList_MaterializesList() + { + var result = Build("IntList", nameof(IIntList.Values), "1,2,3"); + + await Assert.That(result.Values is List).IsTrue(); + await Assert.That(result.Values.SequenceEqual([1, 2, 3])).IsTrue(); + } + + [Test] + public async Task Convert_DelimitedString_ToIntIList_MaterializesList() + { + var result = Build("IntIList", nameof(IIntIList.Values), "1,2,3"); + + await Assert.That(result.Values is List).IsTrue(); + await Assert.That(result.Values.SequenceEqual([1, 2, 3])).IsTrue(); + } + + [Test] + public async Task Convert_DelimitedString_ToIntICollection_MaterializesList() + { + var result = Build("IntICollection", nameof(IIntICollection.Values), "4,5,6"); + + await Assert.That(result.Values is List).IsTrue(); + await Assert.That(result.Values.SequenceEqual([4, 5, 6])).IsTrue(); + } + + [Test] + public async Task Convert_DelimitedString_ToIntReadOnlyList_MaterializesList() + { + var result = Build("IntReadOnlyList", nameof(IIntReadOnlyList.Values), "7,8,9"); + + await Assert.That(result.Values is List).IsTrue(); + await Assert.That(result.Values.SequenceEqual([7, 8, 9])).IsTrue(); + } + + [Test] + public async Task Convert_DelimitedString_ToIntReadOnlyCollection_MaterializesList() + { + var result = Build("IntReadOnlyCollection", nameof(IIntReadOnlyCollection.Values), "10,20,30"); + + await Assert.That(result.Values is List).IsTrue(); + await Assert.That(result.Values.SequenceEqual([10, 20, 30])).IsTrue(); + } + + [Test] + public async Task Convert_DelimitedString_ToDayOfWeekList_ParsesEachElement() + { + var result = Build("DayOfWeekList", nameof(IDayOfWeekList.Values), "Monday,Friday"); + + await Assert.That(result.Values.SequenceEqual(new[] { DayOfWeek.Monday, DayOfWeek.Friday })).IsTrue(); + } + [Test] public async Task Convert_DelimitedString_ToEnumArray_ParsesEachElement() { @@ -176,5 +229,35 @@ public interface IUriArray { Uri[] Values { get; set; } } + + public interface IIntList + { + List Values { get; set; } + } + + public interface IIntIList + { + IList Values { get; set; } + } + + public interface IIntICollection + { + ICollection Values { get; set; } + } + + public interface IIntReadOnlyList + { + IReadOnlyList Values { get; set; } + } + + public interface IIntReadOnlyCollection + { + IReadOnlyCollection Values { get; set; } + } + + public interface IDayOfWeekList + { + List Values { get; set; } + } } } From 342f827077718a81e1fc320953efb7121565a9d9 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 14:28:50 +0300 Subject: [PATCH 02/18] feat(04-01): add List-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) --- .../Conversion/CollectionTypeConverter.cs | 2 +- .../Conversion/ListTypeConverter.cs | 56 +++++++++++++++++++ .../Conversion/TypeConvertersCollections.cs | 1 + .../Core/Reflection/TypeExtensions.cs | 22 ++++++++ 4 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 src/Core/ExistForAll.SimpleSettings/Conversion/ListTypeConverter.cs diff --git a/src/Core/ExistForAll.SimpleSettings/Conversion/CollectionTypeConverter.cs b/src/Core/ExistForAll.SimpleSettings/Conversion/CollectionTypeConverter.cs index 302a068..d03276a 100644 --- a/src/Core/ExistForAll.SimpleSettings/Conversion/CollectionTypeConverter.cs +++ b/src/Core/ExistForAll.SimpleSettings/Conversion/CollectionTypeConverter.cs @@ -22,7 +22,7 @@ protected CollectionTypeConverter(SettingsOptions settingsOptions, TypeConverter // The element type to convert each item to: the array's element type, or the IEnumerable argument. protected abstract Type GetElementType(Type settingsType); - public object Convert(object value, Type settingsType) + public virtual object Convert(object value, Type settingsType) { var elementType = GetElementType(settingsType); var source = AsArray(value); diff --git a/src/Core/ExistForAll.SimpleSettings/Conversion/ListTypeConverter.cs b/src/Core/ExistForAll.SimpleSettings/Conversion/ListTypeConverter.cs new file mode 100644 index 0000000..4a4b0cb --- /dev/null +++ b/src/Core/ExistForAll.SimpleSettings/Conversion/ListTypeConverter.cs @@ -0,0 +1,56 @@ +using System.Collections.Concurrent; +using System.Reflection; +using ExistForAll.SimpleSettings.Core.Reflection; + +namespace ExistForAll.SimpleSettings.Conversion +{ + // Handles the List family (List/IList/ICollection/IReadOnlyList/IReadOnlyCollection) by reusing the + // base array-build path and copying the result into a List. Materialization uses a cached per-element + // delegate (built once, invoked on every populate) so the warm path carries no MakeGenericType/Activator + // reflection (review S-4/A1). + internal class ListTypeConverter : CollectionTypeConverter + { + private static readonly MethodInfo CreateListMethod = + typeof(ListTypeConverter).GetMethod(nameof(CreateList), BindingFlags.NonPublic | BindingFlags.Static)!; + + private static readonly ConcurrentDictionary> ListFactories = new(); + + public ListTypeConverter(SettingsOptions settingsOptions, TypeConvertersCollections converters) + : base(settingsOptions, converters) + { + } + + public override bool CanConvert(Type settingsType) + { + return settingsType.IsListLike(); + } + + protected override Type GetElementType(Type settingsType) + { + return settingsType.GetTypeInfo().GetGenericArguments()[0]; + } + + public override object Convert(object value, Type settingsType) + { + var elementType = GetElementType(settingsType); + var builtArray = (Array)base.Convert(value, settingsType); + var factory = ListFactories.GetOrAdd(elementType, BuildFactory); + + return factory(builtArray); + } + + private static Func BuildFactory(Type elementType) + { + var closed = CreateListMethod.MakeGenericMethod(elementType); + + return (Func)closed.CreateDelegate(typeof(Func)); + } + + // The source is the freshly built T[] (ICollection), so new List(source) copies into a + // right-sized buffer rather than aliasing the array. + private static object CreateList(Array source) + { + return new List((TElement[])source); + } + } +} diff --git a/src/Core/ExistForAll.SimpleSettings/Conversion/TypeConvertersCollections.cs b/src/Core/ExistForAll.SimpleSettings/Conversion/TypeConvertersCollections.cs index d64747f..bb74968 100644 --- a/src/Core/ExistForAll.SimpleSettings/Conversion/TypeConvertersCollections.cs +++ b/src/Core/ExistForAll.SimpleSettings/Conversion/TypeConvertersCollections.cs @@ -8,6 +8,7 @@ public TypeConvertersCollections(SettingsOptions settingsOptions) AddLast(new UriTypeConvertor()); AddLast(new ArrayTypeConverter(settingsOptions, this)); AddLast(new EnumerableTypeConverter(settingsOptions, this)); + AddLast(new ListTypeConverter(settingsOptions, this)); AddLast(new EnumTypeConverter()); AddLast(new DefaultTypeConverter()); } diff --git a/src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeExtensions.cs b/src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeExtensions.cs index 1d6db31..3fd4f67 100644 --- a/src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeExtensions.cs +++ b/src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeExtensions.cs @@ -24,5 +24,27 @@ public static bool IsEnumerable(this Type type) return info.IsGenericType && info.GetGenericTypeDefinition() == typeof(IEnumerable<>); } + + // Disjoint from IsEnumerable/IsArray: matches only the List family the ListTypeConverter claims. + public static bool IsListLike(this Type type) + { + var info = type.GetTypeInfo(); + + if (!info.IsGenericType) + return false; + + var definition = info.GetGenericTypeDefinition(); + + return definition == typeof(List<>) + || definition == typeof(IList<>) + || definition == typeof(ICollection<>) + || definition == typeof(IReadOnlyList<>) + || definition == typeof(IReadOnlyCollection<>); + } + + public static bool IsCollectionShape(this Type type) + { + return type.IsArray || type.IsEnumerable() || type.IsListLike(); + } } } \ No newline at end of file From 64a1e0bb9a7f8c489f11c93b51d43829d2a6f650 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 14:29:29 +0300 Subject: [PATCH 03/18] test(04-01): add failing tests for empty-not-null collection defaults - Unbound int[] -> empty int[], unbound List -> empty List - Regression: unbound IEnumerable -> empty int[] - B-3: two null-binds of List return reference-distinct, mutation-isolated instances --- .../Core/TypeConverterTests.cs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs index 093cfe2..6165439 100644 --- a/src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs @@ -51,6 +51,54 @@ public async Task Convert_WhenConverterTypeSetOnCollectionProperty_BypassesColle await Assert.That(((int[])result!).SequenceEqual(new[] { -1 })).IsTrue(); } + [Test] + public async Task Convert_NullForArrayProperty_ReturnsEmptyArray() + { + var conversion = ResolveConversion(nameof(IIntArrayProp.Values)); + + var result = conversion.Convert(null); + + await Assert.That(result is int[]).IsTrue(); + await Assert.That(((int[])result!).Length).IsEqualTo(0); + } + + [Test] + public async Task Convert_NullForListProperty_ReturnsEmptyList() + { + var conversion = ResolveConversion(nameof(IIntListProp.Values)); + + var result = conversion.Convert(null); + + await Assert.That(result is List).IsTrue(); + await Assert.That(((List)result!).Count).IsEqualTo(0); + } + + [Test] + public async Task Convert_NullForEnumerableProperty_ReturnsEmptyArray() + { + var conversion = ResolveConversion(nameof(IIntEnumerableProp.Values)); + + var result = conversion.Convert(null); + + await Assert.That(result is int[]).IsTrue(); + await Assert.That(((int[])result!).Length).IsEqualTo(0); + } + + [Test] + public async Task Convert_NullForListProperty_YieldsFreshInstancePerBind() + { + var conversion = ResolveConversion(nameof(IIntListProp.Values)); + + var first = (List)conversion.Convert(null)!; + var second = (List)conversion.Convert(null)!; + + await Assert.That(ReferenceEquals(first, second)).IsFalse(); + + first.Add(99); + + await Assert.That(second.Count).IsEqualTo(0); + } + private static PropertyConversion ResolveConversion(string propertyName) { var options = new SettingsOptions(); // Converters auto-seeded: DateTime,Uri,Array,Enumerable,Enum,Default @@ -72,6 +120,21 @@ public interface IWithConverterOverride IEnumerable Values { get; set; } } + public interface IIntArrayProp + { + int[] Values { get; set; } + } + + public interface IIntListProp + { + List Values { get; set; } + } + + public interface IIntEnumerableProp + { + IEnumerable Values { get; set; } + } + // Returns a distinctive sentinel so a passing test proves the attribute ConverterType was chosen // instead of CollectionTypeConverter. private class SentinelConverter : ISettingsTypeConverter From 648f4f5c9ba02d54712e13bf72d0f07b2096264c Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 14:30:06 +0300 Subject: [PATCH 04/18] 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 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 --- .../Conversion/PropertyConversion.cs | 5 +++ .../Core/Reflection/TypeConverter.cs | 33 ++++++++++++++----- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs b/src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs index 30232e6..65b94d1 100644 --- a/src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs +++ b/src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs @@ -33,6 +33,11 @@ public PropertyConversion(ISettingsTypeConverter converter, if (_throwOnNull) throw new SettingsPropertyNullException(_propertyName); + // A list null-result is a factory so each bind gets a fresh mutable List (review B-3); + // arrays / IEnumerable / value-type defaults stay on the shared cached instance. + if (_nullResult is Func listFactory) + return listFactory(); + return _nullResult; } diff --git a/src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeConverter.cs b/src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeConverter.cs index 663338f..2fcc5e4 100644 --- a/src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeConverter.cs +++ b/src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeConverter.cs @@ -22,17 +22,34 @@ public PropertyConversion CreateConversion(PropertyInfo propertyInfo, SettingsPr return new PropertyConversion(converter, strippedType, throwOnNull, nullResult, propertyInfo.Name); } - // The value a null bound value converts to: an empty sequence for IEnumerable, a default instance - // for a value type, otherwise null. Constant per property, so it is materialized once at plan build. + private static readonly MethodInfo EmptyListFactoryMethod = + typeof(TypeConverter).GetMethod(nameof(CreateEmptyList), BindingFlags.NonPublic | BindingFlags.Static)!; + + // The value a null bound value converts to per shape: a shared empty array for arrays and IEnumerable, + // a fresh-per-bind empty List for the List family (review B-3: a mutable list must not be shared), a + // default instance for a value type, otherwise null. Resolved once at plan build; the list branch bakes a + // factory delegate (cold reflection here, plain new List() per call) rather than a shared instance. private static object? CreateNullResult(Type propertyType) { - if (!propertyType.IsEnumerable()) - return propertyType.GetTypeInfo().IsValueType ? Activator.CreateInstance(propertyType) : null; + if (propertyType.IsArray) + return Array.CreateInstance(propertyType.GetElementType()!, 0); + + if (propertyType.IsEnumerable()) + return Array.CreateInstance(propertyType.GetTypeInfo().GetGenericArguments()[0], 0); + + if (propertyType.IsListLike()) + { + var elementType = propertyType.GetTypeInfo().GetGenericArguments()[0]; + return (Func)EmptyListFactoryMethod.MakeGenericMethod(elementType) + .CreateDelegate(typeof(Func)); + } + + return propertyType.GetTypeInfo().IsValueType ? Activator.CreateInstance(propertyType) : null; + } - // An empty IEnumerable is just an empty T[] (arrays implement IEnumerable). Array.CreateInstance - // replaces the old Enumerable.Empty() built via GetMethod("Empty").MakeGenericMethod().Invoke(). - var elementType = propertyType.GetTypeInfo().GetGenericArguments()[0]; - return Array.CreateInstance(elementType, 0); + private static object CreateEmptyList() + { + return new List(); } private static ISettingsTypeConverter GetConverter(Type strippedType, From d719213e00ef16b64771672795f13e4b9cabb4e5 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 16:27:23 +0300 Subject: [PATCH 05/18] test(04-02): add failing child-sequence bind tests for ConfigurationBinder - child-section sequence binds string[]/int[]/List 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 --- ...lderConfigurationBinderIntegrationTests.cs | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/ConfigBuilderConfigurationBinderIntegrationTests.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests/ConfigBuilderConfigurationBinderIntegrationTests.cs index 94bb1ff..2d5e4da 100644 --- a/src/Tests/ExistForAll.SimpleSettings.UnitTests/ConfigBuilderConfigurationBinderIntegrationTests.cs +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/ConfigBuilderConfigurationBinderIntegrationTests.cs @@ -78,6 +78,108 @@ public async Task Build_WhenMemoryBinderSetValues_ShouldSetProperData() await Assert.That(settings.Value).IsEqualTo(value); } + [Test] + public async Task Bind_ChildSequence_ToStringArray_BindsEachElement() + { + var settings = BindSequence(new Dictionary + { + ["SequenceStringSetting:Values:0"] = "a", + ["SequenceStringSetting:Values:1"] = "b", + ["SequenceStringSetting:Values:2"] = "c", + }); + + await Assert.That(settings.Values).IsEquivalentTo(new[] { "a", "b", "c" }); + } + + [Test] + public async Task Bind_ChildSequence_ToIntArray_BindsEachElement() + { + var settings = BindSequence(new Dictionary + { + ["SequenceIntSetting:Numbers:0"] = "1", + ["SequenceIntSetting:Numbers:1"] = "2", + ["SequenceIntSetting:Numbers:2"] = "3", + }); + + await Assert.That(settings.Numbers).IsEquivalentTo(new[] { 1, 2, 3 }); + } + + [Test] + public async Task Bind_ChildSequence_ToStringList_BindsEachElement() + { + var settings = BindSequence(new Dictionary + { + ["SequenceStringListSetting:Items:0"] = "x", + ["SequenceStringListSetting:Items:1"] = "y", + }); + + await Assert.That(settings.Items).IsEquivalentTo(new List { "x", "y" }); + } + + [Test] + public async Task Bind_ScalarAndChildren_ChildrenWin() + { + var settings = BindSequence(new Dictionary + { + ["SequenceStringSetting:Values"] = "scalar-should-lose", + ["SequenceStringSetting:Values:0"] = "a", + ["SequenceStringSetting:Values:1"] = "b", + }); + + await Assert.That(settings.Values).IsEquivalentTo(new[] { "a", "b" }); + } + + [Test] + public async Task Bind_CommaScalar_NoChildren_StillBinds() + { + var settings = BindSequence(new Dictionary + { + ["SequenceStringSetting:Values"] = "a,b,c", + }); + + await Assert.That(settings.Values).IsEquivalentTo(new[] { "a", "b", "c" }); + } + + [Test] + public async Task Bind_WhitespaceScalar_ToStringArray_BindsEmpty() + { + var settings = BindSequence(new Dictionary + { + ["SequenceStringSetting:Values"] = " ", + }); + + await Assert.That(settings.Values.Length).IsEqualTo(0); + } + + [Test] + public async Task Bind_EmptySequence_BindsEmpty() + { + var settings = BindSequence(new Dictionary()); + + await Assert.That(settings.Values.Length).IsEqualTo(0); + } + + [Test] + public async Task Bind_ChildSequence_WithRootSection_ResolvesUnderPrefix() + { + var settings = BindSequence(new Dictionary + { + ["MyRoot:SequenceStringSetting:Values:0"] = "a", + ["MyRoot:SequenceStringSetting:Values:1"] = "b", + }, "MyRoot"); + + await Assert.That(settings.Values).IsEquivalentTo(new[] { "a", "b" }); + } + + private static T BindSequence(Dictionary data, string? rootSection = null) where T : class + { + var configuration = new ConfigurationBuilder().AddInMemoryCollection(data).Build(); + + return SettingsBuilder + .CreateBuilder(x => x.AddSectionBinder(new ConfigurationBinder(configuration, rootSection))) + .GetSettings(); + } + private IConfiguration GetConfiguration() { return new ConfigurationBuilder() @@ -97,5 +199,20 @@ private static SettingsBuilder BuildSutWithBinder(params ISectionBinder[] binder return sut; } + + public interface ISequenceStringSetting + { + string[] Values { get; set; } + } + + public interface ISequenceIntSetting + { + int[] Numbers { get; set; } + } + + public interface ISequenceStringListSetting + { + List Items { get; set; } + } } } From 7433bc4da14df9045e1d56c04170ccbb0d29d316 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 16:28:12 +0300 Subject: [PATCH 06/18] 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 --- .../ConfigurationBinder.cs | 45 +++++++++++++++++++ src/Core/ExistForAll.SimpleSettings/Info.cs | 1 + 2 files changed, 46 insertions(+) diff --git a/src/Core/ExistForAll.SimpleSettings.Extensions.Binders/ConfigurationBinder.cs b/src/Core/ExistForAll.SimpleSettings.Extensions.Binders/ConfigurationBinder.cs index 119747b..3adb1eb 100644 --- a/src/Core/ExistForAll.SimpleSettings.Extensions.Binders/ConfigurationBinder.cs +++ b/src/Core/ExistForAll.SimpleSettings.Extensions.Binders/ConfigurationBinder.cs @@ -1,4 +1,5 @@ using System.Collections.Concurrent; +using ExistForAll.SimpleSettings.Core.Reflection; using Microsoft.Extensions.Configuration; namespace ExistForAll.SimpleSettings.Binders @@ -29,12 +30,56 @@ public void BindPropertySettings(BindingContext context) // capturing lambda would build a fresh delegate on every call and eat most of the win. var section = _sections.GetOrAdd(context.Section, static (name, self) => self.ResolveSection(name), this); + var isCollection = context.PropertyType.IsCollectionShape(); + + if (isCollection && TrySetChildSequence(section, context)) + return; + var value = section[context.Key]; + if (isCollection) + { + if (!string.IsNullOrWhiteSpace(value)) + context.SetNewValue(value); + + return; + } + if (value != null) context.SetNewValue(value); } + // Child-section sequence path (COLL-03): a collection property may be expressed as an indexed + // sub-section (Key:0, Key:1, ...) rather than a delimited scalar. Materialize the element values into a + // right-sized string[] with a manual two-pass walk (no LINQ projection on the bind path) so the element + // converter chain runs per item. Children win over the scalar; an empty sequence falls through to the + // scalar path. No element value is placed in any exception (S1/D-06). + private static bool TrySetChildSequence(IConfigurationSection section, BindingContext context) + { + var childSection = section.GetSection(context.Key); + + var count = 0; + foreach (var child in childSection.GetChildren()) + { + if (child.Value != null) + count++; + } + + if (count == 0) + return false; + + var values = new string[count]; + var index = 0; + foreach (var child in childSection.GetChildren()) + { + if (child.Value != null) + values[index++] = child.Value; + } + + context.SetNewValue(values); + return true; + } + private IConfigurationSection ResolveSection(string section) { return _configuration.GetSection(string.IsNullOrWhiteSpace(RootSection) ? section : $"{RootSection}:{section}"); diff --git a/src/Core/ExistForAll.SimpleSettings/Info.cs b/src/Core/ExistForAll.SimpleSettings/Info.cs index 4f8341b..75c27cd 100644 --- a/src/Core/ExistForAll.SimpleSettings/Info.cs +++ b/src/Core/ExistForAll.SimpleSettings/Info.cs @@ -2,3 +2,4 @@ [assembly:InternalsVisibleTo("ExistForAll.SimpleSettings.UnitTests")] [assembly:InternalsVisibleTo("ExistForAll.SimpleSettings.Benchmark")] +[assembly:InternalsVisibleTo("ExistForAll.SimpleSettings.Binders")] From 0497ab9277127e1e0f5c4574c4bd0a9050880158 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 16:29:58 +0300 Subject: [PATCH 07/18] 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 case (S-6): redaction holds across the array and list converter shapes - drives the new ConfigurationBinder GetChildren() sequence path, not the scalar InMemoryBinder path --- .../Conversion/ExceptionRedactionTests.cs | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ExceptionRedactionTests.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ExceptionRedactionTests.cs index 3d1a1b3..fdab3cb 100644 --- a/src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ExceptionRedactionTests.cs +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ExceptionRedactionTests.cs @@ -1,5 +1,7 @@ using ExistForAll.SimpleSettings.Binder; +using ExistForAll.SimpleSettings.Binders; using ExistForAll.SimpleSettings.Conversion; +using Microsoft.Extensions.Configuration; namespace ExistForAll.SimpleSettings.UnitTests.Conversion { @@ -54,6 +56,58 @@ public async Task Convert_CustomConverterLeakingValue_IsStillRedacted() await AssertRedacted(ex, nameof(ILeakyConverterSetting.Value), nameof(String)); } + [Test] + public async Task Convert_SecretInSequenceElement_DoesNotLeakValue() + { + // D-06/S1 on the ConfigurationBinder GetChildren() sequence path: an int[] element that fails to + // convert is wrapped value-free — the secret in a LATER element never reaches the ToString() chain. + var ex = CaptureSequenceConversionFailure(nameof(IIntArraySetting.Values), "1", Secret); + await AssertRedacted(ex, nameof(IIntArraySetting.Values), nameof(Int32)); + } + + [Test] + public async Task Convert_SecretInFirstSequenceElement_DoesNotLeakValue() + { + // S-6: element position must not matter — the throw site is the shared per-element convert, so a + // secret in the FIRST element is redacted identically. + var ex = CaptureSequenceConversionFailure(nameof(IIntArraySetting.Values), Secret, "2"); + await AssertRedacted(ex, nameof(IIntArraySetting.Values), nameof(Int32)); + } + + [Test] + public async Task Convert_SecretInListSequenceElement_DoesNotLeakValue() + { + // S-6: the List converter wraps into a List only AFTER the element-convert throw site, so the + // redaction contract holds identically for the List shape as for the array shape. + var ex = CaptureSequenceConversionFailure(nameof(IIntListSetting.Values), "1", Secret); + await AssertRedacted(ex, nameof(IIntListSetting.Values), "List"); + } + + private static SettingsPropertyValueException CaptureSequenceConversionFailure(string key, params string[] elements) + where T : class + { + var data = new Dictionary(); + var section = SectionOf(); + for (var i = 0; i < elements.Length; i++) + { + data[$"{section}:{key}:{i}"] = elements[i]; + } + + var configuration = new ConfigurationBuilder().AddInMemoryCollection(data).Build(); + var builder = SettingsBuilder.CreateBuilder(x => x.AddSectionBinder(new ConfigurationBinder(configuration))); + + try + { + builder.GetSettings(); + } + catch (SettingsPropertyValueException e) + { + return e; + } + + throw new Exception($"Expected a SettingsPropertyValueException for [{typeof(T).Name}], but none was thrown."); + } + private static SettingsPropertyValueException CaptureConversionFailure(string key, string value) where T : class { @@ -104,6 +158,16 @@ public interface IUriSetting Uri Endpoint { get; set; } } + public interface IIntArraySetting + { + int[] Values { get; set; } + } + + public interface IIntListSetting + { + List Values { get; set; } + } + public interface ILeakyConverterSetting { [SettingsProperty(ConverterType = typeof(LeakyConverter))] From 944f0497daaea025a70da8c5c2759f9affaa5705 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 16:45:55 +0300 Subject: [PATCH 08/18] feat(04-03): sync validation contracts, context ctors, validator attribute, aggregate exception - ISettingsValidator/ISettingValidation.Validate return ValidationResult (drop Task<>) (D-07) - ValidationContext(object)/ValidationContext(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) --- .../ExistForAll.SimpleSettings/Resources.cs | 19 +++++++++++ .../SettingsValidationException.cs | 34 +++++++++++++++++++ .../SettingsValidatorAttribute.cs | 13 +++++++ .../Validations/ISettingValidation.cs | 14 ++++---- .../Validations/ISettingsValidator.cs | 14 ++++---- .../Validations/ValidationContext.cs | 19 +++++++---- .../Validations/ValidationContextOfT.cs | 20 +++++++---- 7 files changed, 105 insertions(+), 28 deletions(-) create mode 100644 src/Core/ExistForAll.SimpleSettings/SettingsValidationException.cs create mode 100644 src/Core/ExistForAll.SimpleSettings/SettingsValidatorAttribute.cs diff --git a/src/Core/ExistForAll.SimpleSettings/Resources.cs b/src/Core/ExistForAll.SimpleSettings/Resources.cs index 69f9dd6..7eaa3ec 100644 --- a/src/Core/ExistForAll.SimpleSettings/Resources.cs +++ b/src/Core/ExistForAll.SimpleSettings/Resources.cs @@ -1,4 +1,6 @@ using System.Reflection; +using System.Text; +using ExistForAll.SimpleSettings.Validations; namespace ExistForAll.SimpleSettings { @@ -48,5 +50,22 @@ public static string TypeIsNotInterface(string typeName) => public static string SettingsOptionAttributeTypeMessage(Type type) => $"SimpleSettings support Attribute indication of interfaces, the type provided [{type.FullName}] is not an attribute."; + // Composed ONLY from author-supplied ValidationError text (SettingsName + ErrorMessage). The inspected + // settings values are deliberately never embedded — a validator may run against a secret, and this + // message reaches logs via Exception.ToString(). See D-12/S1 and SettingsValidationException. + public static string SettingsValidationExceptionMessage(IReadOnlyList errors) + { + var builder = new StringBuilder(); + builder.Append("Settings validation failed with ").Append(errors.Count).Append(" error(s):"); + + for (var i = 0; i < errors.Count; i++) + { + var error = errors[i]; + builder.Append(Environment.NewLine).Append(" - [").Append(error.SettingsName).Append("] ").Append(error.ErrorMessage); + } + + return builder.ToString(); + } + } } \ No newline at end of file diff --git a/src/Core/ExistForAll.SimpleSettings/SettingsValidationException.cs b/src/Core/ExistForAll.SimpleSettings/SettingsValidationException.cs new file mode 100644 index 0000000..eded80b --- /dev/null +++ b/src/Core/ExistForAll.SimpleSettings/SettingsValidationException.cs @@ -0,0 +1,34 @@ +using ExistForAll.SimpleSettings.Validations; + +namespace ExistForAll.SimpleSettings +{ + // Aggregates every ValidationError produced by the declared validators into one exception. Like the rest of + // the family it embeds NO bound value: the message is composed only from author-supplied ValidationError text + // (SettingsName + ErrorMessage), never from the settings values the validators inspected (which may be + // secrets). See D-12/S1 and SettingsPropertyValueException. The aggregate-and-throw is centralized in + // ThrowIfAny so the core populate path and the DI-resolved path share one thrown contract. + public class SettingsValidationException : SimpleSettingsException + { + public SettingsValidationException(IReadOnlyList errors) + : base(Resources.SettingsValidationExceptionMessage(errors)) + { + Errors = errors; + } + + public IReadOnlyList Errors { get; } + + // The single aggregate-and-throw entry point shared by the core populate path (Plan 03) and the + // DI-resolved runner (Plan 04): throws one aggregated exception when any error is present, otherwise + // returns. Centralizing it keeps the thrown contract (type + Errors + value-free message) identical + // across both paths. + public static void ThrowIfAny(IReadOnlyList errors) + { + if (errors == null) throw new ArgumentNullException(nameof(errors)); + + if (errors.Count == 0) + return; + + throw new SettingsValidationException(errors); + } + } +} diff --git a/src/Core/ExistForAll.SimpleSettings/SettingsValidatorAttribute.cs b/src/Core/ExistForAll.SimpleSettings/SettingsValidatorAttribute.cs new file mode 100644 index 0000000..626f34c --- /dev/null +++ b/src/Core/ExistForAll.SimpleSettings/SettingsValidatorAttribute.cs @@ -0,0 +1,13 @@ +namespace ExistForAll.SimpleSettings +{ + [AttributeUsage(AttributeTargets.Interface)] + public class SettingsValidatorAttribute : Attribute + { + public Type ValidatorType { get; } + + public SettingsValidatorAttribute(Type validatorType) + { + ValidatorType = validatorType; + } + } +} diff --git a/src/Core/ExistForAll.SimpleSettings/Validations/ISettingValidation.cs b/src/Core/ExistForAll.SimpleSettings/Validations/ISettingValidation.cs index 293f9e1..0676398 100644 --- a/src/Core/ExistForAll.SimpleSettings/Validations/ISettingValidation.cs +++ b/src/Core/ExistForAll.SimpleSettings/Validations/ISettingValidation.cs @@ -1,7 +1,7 @@ -namespace ExistForAll.SimpleSettings.Validations -{ - public interface ISettingValidation : ISettingsValidator - { - Task Validate(ValidationContext context); - } -} \ No newline at end of file +namespace ExistForAll.SimpleSettings.Validations +{ + public interface ISettingValidation : ISettingsValidator + { + ValidationResult Validate(ValidationContext context); + } +} diff --git a/src/Core/ExistForAll.SimpleSettings/Validations/ISettingsValidator.cs b/src/Core/ExistForAll.SimpleSettings/Validations/ISettingsValidator.cs index c4f6953..5dd9859 100644 --- a/src/Core/ExistForAll.SimpleSettings/Validations/ISettingsValidator.cs +++ b/src/Core/ExistForAll.SimpleSettings/Validations/ISettingsValidator.cs @@ -1,7 +1,7 @@ -namespace ExistForAll.SimpleSettings.Validations -{ - public interface ISettingsValidator - { - Task Validate(ValidationContext context); - } -} \ No newline at end of file +namespace ExistForAll.SimpleSettings.Validations +{ + public interface ISettingsValidator + { + ValidationResult Validate(ValidationContext context); + } +} diff --git a/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContext.cs b/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContext.cs index 97c4e2d..d9fc18f 100644 --- a/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContext.cs +++ b/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContext.cs @@ -1,7 +1,12 @@ -namespace ExistForAll.SimpleSettings.Validations -{ - public class ValidationContext - { - public object? Settings { get; } - } -} \ No newline at end of file +namespace ExistForAll.SimpleSettings.Validations +{ + public class ValidationContext + { + public ValidationContext(object settings) + { + Settings = settings; + } + + public object? Settings { get; } + } +} diff --git a/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContextOfT.cs b/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContextOfT.cs index ffe8878..3eaf109 100644 --- a/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContextOfT.cs +++ b/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContextOfT.cs @@ -1,7 +1,13 @@ -namespace ExistForAll.SimpleSettings.Validations -{ - public class ValidationContext : ValidationContext - { - public new T? Settings { get; } - } -} \ No newline at end of file +namespace ExistForAll.SimpleSettings.Validations +{ + public class ValidationContext : ValidationContext + { + public ValidationContext(T settings) + : base(settings!) + { + Settings = settings; + } + + public new T? Settings { get; } + } +} From 180317df2392f7f1c37c375023ec8c9a88363b71 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 16:47:59 +0300 Subject: [PATCH 09/18] 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 --- .../SimpleSettings/SettingsValidationTests.cs | 215 ++++++++++++++++++ 1 file changed, 215 insertions(+) create mode 100644 src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsValidationTests.cs diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsValidationTests.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsValidationTests.cs new file mode 100644 index 0000000..f813857 --- /dev/null +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsValidationTests.cs @@ -0,0 +1,215 @@ +using ExistForAll.SimpleSettings.Binder; +using ExistForAll.SimpleSettings.Validations; + +namespace ExistForAll.SimpleSettings.UnitTests.SimpleSettings +{ + // Fixtures are intentionally NON-settings-indicated (no "Settings" suffix, no [SettingsSection]/ + // ISettingsSection) and resolved via GetSettings() so a deliberately-failing validator never trips + // ScanAssemblies-based tests elsewhere in the suite. + public class SettingsValidationTests + { + [Test] + public async Task ObjectValidator_WhenItAddsError_ThrowsSettingsValidationException() + { + await Assert.That(() => Build()).Throws(); + } + + [Test] + public async Task ObjectValidator_WhenValid_DoesNotThrow() + { + var result = Build(); + + await Assert.That(result).IsNotNull(); + } + + [Test] + public async Task PropertyValidator_RunsAgainstThatPropertyValue() + { + await Assert.That(() => Build()).Throws(); + } + + [Test] + public async Task MultipleErrors_AggregateIntoOneExceptionWithAllErrors() + { + var exception = await Assert.That(() => Build()) + .Throws(); + + await Assert.That(exception!.Errors.Count).IsEqualTo(2); + } + + [Test] + public async Task CrossPropertyValidator_ObservesEveryPropertySet() + { + var collection = new InMemoryCollection(); + collection.Add("CrossProperty", nameof(ICrossProperty.Min), "5"); + collection.Add("CrossProperty", nameof(ICrossProperty.Max), "10"); + + var result = Build(collection); + + await Assert.That(result.Min).IsEqualTo(5); + await Assert.That(result.Max).IsEqualTo(10); + } + + [Test] + public async Task CrossPropertyValidator_WhenRuleViolated_Throws() + { + var collection = new InMemoryCollection(); + collection.Add("CrossProperty", nameof(ICrossProperty.Min), "10"); + collection.Add("CrossProperty", nameof(ICrossProperty.Max), "5"); + + await Assert.That(() => Build(collection)).Throws(); + } + + [Test] + public async Task Exception_CarriesAuthorMessage_AndNoBoundValue() + { + var collection = new InMemoryCollection(); + collection.Add("Redaction", nameof(IRedaction.ApiKey), Sentinel); + + var exception = await Assert.That(() => Build(collection)) + .Throws(); + + var text = exception!.ToString(); + await Assert.That(text.Contains("ApiKey failed policy")).IsTrue(); + await Assert.That(text.Contains(Sentinel)).IsFalse(); + } + + [Test] + public async Task NoValidators_PopulatesAndReturnsWithoutThrowing() + { + var result = Build(); + + await Assert.That(result).IsNotNull(); + } + + private const string Sentinel = "SUPER_SECRET_API_KEY_VALUE"; + + private static T Build(InMemoryCollection? collection = null) where T : class + { + var builder = collection == null + ? SettingsBuilder.CreateBuilder() + : SettingsBuilder.CreateBuilder(x => x.AddSectionBinder(new InMemoryBinder(collection))); + + return builder.GetSettings(); + } + + [SettingsValidator(typeof(FailingObjectValidator))] + public interface IObjectValidated + { + string Name { get; set; } + } + + [SettingsValidator(typeof(PassingObjectValidator))] + public interface IObjectValid + { + string Name { get; set; } + } + + public interface IPropertyValidated + { + [SettingsProperty(ValidatorType = typeof(PositivePortValidator))] + int Port { get; set; } + } + + [SettingsValidator(typeof(TwoErrorValidator))] + public interface ITwoErrors + { + string Name { get; set; } + } + + [SettingsValidator(typeof(CrossPropertyValidator))] + public interface ICrossProperty + { + int Min { get; set; } + int Max { get; set; } + } + + [SettingsValidator(typeof(RedactionValidator))] + public interface IRedaction + { + string ApiKey { get; set; } + } + + public interface INoValidators + { + string Name { get; set; } + } + + private class FailingObjectValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + => Validate((ValidationContext)context); + + public ValidationResult Validate(ValidationContext context) + { + var result = new ValidationResult(); + result.AddError(new ValidationError(nameof(IObjectValidated.Name), "name is invalid")); + return result; + } + } + + private class PassingObjectValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + => Validate((ValidationContext)context); + + public ValidationResult Validate(ValidationContext context) => new(); + } + + private class PositivePortValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + => Validate((ValidationContext)context); + + public ValidationResult Validate(ValidationContext context) + { + var result = new ValidationResult(); + if (context.Settings <= 0) + result.AddError(new ValidationError("Port", "port must be positive")); + return result; + } + } + + private class TwoErrorValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + => Validate((ValidationContext)context); + + public ValidationResult Validate(ValidationContext context) + { + var result = new ValidationResult(); + result.AddError(new ValidationError("Name", "first failure")); + result.AddError(new ValidationError("Name", "second failure")); + return result; + } + } + + private class CrossPropertyValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + => Validate((ValidationContext)context); + + public ValidationResult Validate(ValidationContext context) + { + var result = new ValidationResult(); + var settings = context.Settings!; + if (settings.Min > settings.Max) + result.AddError(new ValidationError("Min", "Min must not exceed Max")); + return result; + } + } + + private class RedactionValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + => Validate((ValidationContext)context); + + public ValidationResult Validate(ValidationContext context) + { + var result = new ValidationResult(); + result.AddError(new ValidationError("ApiKey", "ApiKey failed policy")); + return result; + } + } + } +} From f99d6047adbe27c632b18842a5421989ed2b8f48 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 16:49:46 +0300 Subject: [PATCH 10/18] 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) --- .../SettingsPlan.cs | 29 ++++++- .../ValuesPopulator.cs | 81 ++++++++++++++++++- 2 files changed, 106 insertions(+), 4 deletions(-) diff --git a/src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs b/src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs index 0dad595..4f41ff0 100644 --- a/src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs +++ b/src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs @@ -13,11 +13,24 @@ internal sealed class SettingsPlan private readonly SettingsOptions _options; private string? _sectionName; - public SettingsPlan(Type settingsType, SettingsOptions options, PropertyPlan[] properties) + public SettingsPlan(Type settingsType, SettingsOptions options, PropertyPlan[] properties, Type? objectValidatorType) { _settingsType = settingsType; _options = options; Properties = properties; + ObjectValidatorType = objectValidatorType; + HasValidators = objectValidatorType is not null || AnyPropertyHasValidator(properties); + } + + private static bool AnyPropertyHasValidator(PropertyPlan[] properties) + { + for (var i = 0; i < properties.Length; i++) + { + if (properties[i].ValidatorType is not null) + return true; + } + + return false; } // Resolved lazily and cached: a builder with no binders (e.g. the cold assembly scan) never reads it, @@ -26,18 +39,27 @@ public SettingsPlan(Type settingsType, SettingsOptions options, PropertyPlan[] p public string SectionName => _sectionName ??= _settingsType.GetSectionName(_options); public PropertyPlan[] Properties { get; } + + // The [SettingsValidator] object-level validator declared on the settings interface, resolved once at + // plan build (null when none is declared). + public Type? ObjectValidatorType { get; } + + // Computed once at plan build so the post-populate hook can short-circuit with a single field read + // before allocating anything on the validator-free warm path (review B-2). + public bool HasValidators { get; } } // A readonly struct held inline in SettingsPlan.Properties: the whole per-type plan is then one array // allocation plus the section string, rather than an object per property (matters at scan scale). internal readonly struct PropertyPlan { - public PropertyPlan(PropertyInfo property, string key, object? defaultValue, PropertyConversion conversion) + public PropertyPlan(PropertyInfo property, string key, object? defaultValue, PropertyConversion conversion, Type? validatorType) { Property = property; Key = key; DefaultValue = defaultValue; Conversion = conversion; + ValidatorType = validatorType; } public PropertyInfo Property { get; } @@ -47,5 +69,8 @@ public PropertyPlan(PropertyInfo property, string key, object? defaultValue, Pro public object? DefaultValue { get; } public PropertyConversion Conversion { get; } + + // The [SettingsProperty(ValidatorType)] validator for this property, resolved once at plan build. + public Type? ValidatorType { get; } } } diff --git a/src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs b/src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs index 9d366bd..7753551 100644 --- a/src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs +++ b/src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs @@ -1,6 +1,7 @@ using System.Collections.Concurrent; using System.Reflection; using ExistForAll.SimpleSettings.Core.Reflection; +using ExistForAll.SimpleSettings.Validations; namespace ExistForAll.SimpleSettings { @@ -62,6 +63,76 @@ public void PopulateInstanceWithValues(object instance, var propertyValue = ConvertPropertyValue(settings, tempValue, propertyPlan); propertyPlan.Property.SetValue(instance, propertyValue); } + + // Runs AFTER the property-set loop so cross-property rules see the fully-populated instance (D-09). + RunValidators(instance, plan); + } + + // Zero-allocation short-circuit for validator-free types (review B-2): a single field read returns + // before anything is allocated, keeping the warm populate path byte-identical to the pre-validation + // build. The error list is allocated lazily only after a validator actually produces an error. + private static void RunValidators(object instance, SettingsPlan plan) + { + if (!plan.HasValidators) + return; + + List? errors = null; + + if (plan.ObjectValidatorType is not null) + errors = InvokeValidator(plan.ObjectValidatorType, instance, errors); + + foreach (var propertyPlan in plan.Properties) + { + if (propertyPlan.ValidatorType is null) + continue; + + var propertyValue = propertyPlan.Property.GetValue(instance); + errors = InvokeValidator(propertyPlan.ValidatorType, propertyValue, errors); + } + + // The single aggregate-and-throw entry point shared with Plan 04's DI runner (review S-3): the + // thrown contract cannot drift between the two paths. + if (errors is not null) + SettingsValidationException.ThrowIfAny(errors); + } + + private static List? InvokeValidator(Type validatorType, object? target, List? errors) + { + var validator = Activator.CreateInstance(validatorType)!; + + var closedContextType = ResolveContextType(validatorType); + + var context = Activator.CreateInstance(closedContextType, target)!; + + // ISettingValidation : ISettingsValidator declares TWO Validate overloads, so the parameterless + // GetMethod("Validate") throws AmbiguousMatchException — select the overload by its parameter type + // explicitly (review S-2). + var validateMethod = validatorType.GetMethod(nameof(ISettingsValidator.Validate), new[] { closedContextType })!; + + var result = (ValidationResult)validateMethod.Invoke(validator, new[] { context })!; + + foreach (var error in result.Errors) + { + errors ??= new List(); + errors.Add(error); + } + + return errors; + } + + private static Type ResolveContextType(Type validatorType) + { + foreach (var contract in validatorType.GetInterfaces()) + { + var info = contract.GetTypeInfo(); + if (info.IsGenericType && info.GetGenericTypeDefinition() == typeof(ISettingValidation<>)) + { + var validatedType = info.GetGenericArguments()[0]; + return typeof(ValidationContext<>).MakeGenericType(validatedType); + } + } + + return typeof(ValidationContext); } private SettingsPlan GetOrBuildPlan(Type settings, SettingsOptions options) @@ -90,7 +161,8 @@ private SettingsPlan GetOrBuildPlan(Type settings, SettingsOptions options) propertyPlans[i] = new PropertyPlan(property, key, attribute?.DefaultValue, - _typeConverter.CreateConversion(property, attribute, options)); + _typeConverter.CreateConversion(property, attribute, options), + attribute?.ValidatorType); } catch (Exception e) { @@ -102,7 +174,12 @@ private SettingsPlan GetOrBuildPlan(Type settings, SettingsOptions options) } } - var plan = new SettingsPlan(settings, options, propertyPlans); + // Read the object-level [SettingsValidator] once per type at plan build, mirroring the per-property + // attribute read above — never on the hot populate path. + var objectValidatorType = settings.GetTypeInfo() + .GetCustomAttribute(inherit: true)?.ValidatorType; + + var plan = new SettingsPlan(settings, options, propertyPlans, objectValidatorType); // Concurrent builds would produce equivalent plans, so last-writer-wins is harmless. A build that // throws is never cached (we don't reach here), so the next call retries — matching the old From b1156bd4a9f100db951a315f894950ed4007d9ea Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 16:55:21 +0300 Subject: [PATCH 11/18] 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 --- .../SimpleSettings/SettingsPropertyTests.cs | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsPropertyTests.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsPropertyTests.cs index b84f878..c713cb7 100644 --- a/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsPropertyTests.cs +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsPropertyTests.cs @@ -1,3 +1,5 @@ +using ExistForAll.SimpleSettings.Binder; + namespace ExistForAll.SimpleSettings.UnitTests.SimpleSettings { public class SettingsPropertyTests @@ -15,10 +17,80 @@ public async Task Build_WhenAllowEmptyIsFalse_ShouldThrowException() await Assert.That(exception!.Message.Contains(nameof(IWithNonNullInterface.Value))).IsTrue(); } + [Test] + public async Task Build_WhenAllowEmptyIsFalse_EmptyString_ShouldThrowNullException() + { + var sut = BuildWith(nameof(IRejectsEmpty.Value), ""); + + await Assert.That(() => sut.GetSettings()) + .Throws(); + } + + [Test] + public async Task Build_WhenAllowEmptyIsFalse_WhitespaceString_ShouldThrowNullException() + { + var sut = BuildWith(nameof(IRejectsEmpty.Value), " "); + + await Assert.That(() => sut.GetSettings()) + .Throws(); + } + + [Test] + public async Task Build_WhenAllowEmptyIsFalse_EmptyString_MessageIsValueFreeWithPropertyName() + { + var sut = BuildWith(nameof(IRejectsEmpty.Value), " "); + + var exception = await Assert.That(() => sut.GetSettings()) + .Throws(); + + await Assert.That(exception!.Message.Contains(nameof(IRejectsEmpty.Value))).IsTrue(); + await Assert.That(exception!.Message.Contains(" ")).IsFalse(); + } + + [Test] + public async Task Build_WhenAllowEmptyIsTrue_EmptyString_BindsValue() + { + var sut = BuildWith(nameof(IAllowsEmpty.Value), ""); + + var settings = sut.GetSettings(); + + await Assert.That(settings.Value).IsEqualTo(""); + } + + [Test] + public async Task Build_WhenAllowEmptyIsTrue_WhitespaceString_BindsValue() + { + var sut = BuildWith(nameof(IAllowsEmpty.Value), " "); + + var settings = sut.GetSettings(); + + await Assert.That(settings.Value).IsEqualTo(" "); + } + + private static SettingsBuilder BuildWith(string key, string value) + { + var collection = new InMemoryCollection(); + collection.Add("RejectsEmpty", key, value); + collection.Add("AllowsEmpty", key, value); + + return SettingsBuilder.CreateBuilder(x => x.AddSectionBinder(new InMemoryBinder(collection))); + } + public interface IWithNonNullInterface { [SettingsProperty(AllowEmpty = false)] int Value { get; set; } } + + public interface IRejectsEmpty + { + [SettingsProperty(AllowEmpty = false)] + string Value { get; set; } + } + + public interface IAllowsEmpty + { + string Value { get; set; } + } } } From 3f24f915c66719947319e41145a8152d3b57a77c Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 16:56:05 +0300 Subject: [PATCH 12/18] 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 --- .../Conversion/PropertyConversion.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs b/src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs index 65b94d1..ae43013 100644 --- a/src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs +++ b/src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs @@ -28,11 +28,12 @@ public PropertyConversion(ISettingsTypeConverter converter, public object? Convert(object? value) { + // AllowEmpty=false (VAL-02/D-13) rejects null AND null-or-whitespace strings, value-free (D-14). + if (_throwOnNull && (value == null || (value is string s && string.IsNullOrWhiteSpace(s)))) + throw new SettingsPropertyNullException(_propertyName); + if (value == null) { - if (_throwOnNull) - throw new SettingsPropertyNullException(_propertyName); - // A list null-result is a factory so each bind gets a fresh mutable List (review B-3); // arrays / IEnumerable / value-type defaults stay on the shared cached instance. if (_nullResult is Func listFactory) From cc0d3f2bbdd70f61f0c653c703033ab70b63629a Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 18:54:52 +0300 Subject: [PATCH 13/18] =?UTF-8?q?fix(04-02):=20single-pass=20child-sequenc?= =?UTF-8?q?e=20bind=20=E2=80=94=20reload-safe,=20drops=20empty=20elements?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 — review MED-2. - PR #33 review comment 1. --- .../ConfigurationBinder.cs | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/Core/ExistForAll.SimpleSettings.Extensions.Binders/ConfigurationBinder.cs b/src/Core/ExistForAll.SimpleSettings.Extensions.Binders/ConfigurationBinder.cs index 3adb1eb..88d9a6a 100644 --- a/src/Core/ExistForAll.SimpleSettings.Extensions.Binders/ConfigurationBinder.cs +++ b/src/Core/ExistForAll.SimpleSettings.Extensions.Binders/ConfigurationBinder.cs @@ -1,4 +1,4 @@ -using System.Collections.Concurrent; +using System.Collections.Concurrent; using ExistForAll.SimpleSettings.Core.Reflection; using Microsoft.Extensions.Configuration; @@ -49,34 +49,28 @@ public void BindPropertySettings(BindingContext context) context.SetNewValue(value); } - // Child-section sequence path (COLL-03): a collection property may be expressed as an indexed - // sub-section (Key:0, Key:1, ...) rather than a delimited scalar. Materialize the element values into a - // right-sized string[] with a manual two-pass walk (no LINQ projection on the bind path) so the element - // converter chain runs per item. Children win over the scalar; an empty sequence falls through to the - // scalar path. No element value is placed in any exception (S1/D-06). + // Child-section sequence path (COLL-03): a collection property may be an indexed sub-section + // (Key:0, Key:1, ...) rather than a delimited scalar. Enumerate the live config view ONCE (a second + // pass could race a reload), collecting non-empty element values into a string[] so the element + // converter chain runs per item. Empty entries are dropped for parity with the comma-scalar split + // (RemoveEmptyEntries). Children win over the scalar; an all-empty sequence falls through to the scalar + // path. No element value is placed in any exception (S1). private static bool TrySetChildSequence(IConfigurationSection section, BindingContext context) { var childSection = section.GetSection(context.Key); - var count = 0; + List? values = null; foreach (var child in childSection.GetChildren()) { - if (child.Value != null) - count++; + var childValue = child.Value; + if (!string.IsNullOrEmpty(childValue)) + (values ??= new List()).Add(childValue); } - if (count == 0) + if (values is null) return false; - var values = new string[count]; - var index = 0; - foreach (var child in childSection.GetChildren()) - { - if (child.Value != null) - values[index++] = child.Value; - } - - context.SetNewValue(values); + context.SetNewValue(values.ToArray()); return true; } From 779f0fcd3e832f58c5043b19a4afb8d85ddc85ec Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 18:54:52 +0300 Subject: [PATCH 14/18] 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. --- .../Conversion/PropertyConversion.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs b/src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs index ae43013..3d544dc 100644 --- a/src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs +++ b/src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs @@ -28,20 +28,21 @@ public PropertyConversion(ISettingsTypeConverter converter, public object? Convert(object? value) { - // AllowEmpty=false (VAL-02/D-13) rejects null AND null-or-whitespace strings, value-free (D-14). - if (_throwOnNull && (value == null || (value is string s && string.IsNullOrWhiteSpace(s)))) - throw new SettingsPropertyNullException(_propertyName); - if (value == null) { - // A list null-result is a factory so each bind gets a fresh mutable List (review B-3); - // arrays / IEnumerable / value-type defaults stay on the shared cached instance. - if (_nullResult is Func listFactory) - return listFactory(); + // AllowEmpty=false rejects a missing required value, value-free. + if (_throwOnNull) + throw new SettingsPropertyNullException(_propertyName); - return _nullResult; + // A list null-result is a factory so each bind gets a fresh mutable List; arrays / + // IEnumerable / value-type defaults stay on the shared cached instance. + return _nullResult is Func listFactory ? listFactory() : _nullResult; } + // AllowEmpty=false also rejects empty/whitespace strings, not just null. + if (_throwOnNull && value is string s && string.IsNullOrWhiteSpace(s)) + throw new SettingsPropertyNullException(_propertyName); + return _converter.Convert(value, _strippedType); } } From c393202c8196295e1c78b3aba5a8f714555c9171 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 19:04:30 +0300 Subject: [PATCH 15/18] refactor(04-03): dispatch validators via ISettingValidation DIM bridge; guard invocation value-free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ISettingValidation 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. --- .../ExistForAll.SimpleSettings/Resources.cs | 7 +- .../SettingsValidatorInvocationException.cs | 19 ++++ .../Validations/ISettingValidation.cs | 5 + .../Validations/ValidationContext.cs | 2 +- .../Validations/ValidationContextOfT.cs | 2 +- .../ValuesPopulator.cs | 52 ++++------ .../SimpleSettings/SettingsValidationTests.cs | 98 +++++++++++++++---- 7 files changed, 130 insertions(+), 55 deletions(-) create mode 100644 src/Core/ExistForAll.SimpleSettings/SettingsValidatorInvocationException.cs diff --git a/src/Core/ExistForAll.SimpleSettings/Resources.cs b/src/Core/ExistForAll.SimpleSettings/Resources.cs index 7eaa3ec..caf1366 100644 --- a/src/Core/ExistForAll.SimpleSettings/Resources.cs +++ b/src/Core/ExistForAll.SimpleSettings/Resources.cs @@ -42,7 +42,7 @@ public static string SettingsPropertiesExtractionMessage(Type type) => all properties from type [{type.FullName}]"; public static string PropertyNotAllowNullMessage(string propertyName) => - $@"[{propertyName}] is marked as Null not allowed, yet the value is null. please provide value via binder or attribute"; + $@"[{propertyName}] is marked as Null not allowed, yet the value is missing (null, empty, or whitespace). please provide value via binder or attribute"; public static string TypeIsNotInterface(string typeName) => $@"[{typeName}] is not an interface, SimpleSettings supports only interfaces"; @@ -67,5 +67,10 @@ public static string SettingsValidationExceptionMessage(IReadOnlyList + $"Validator [{validatorType.Name}] threw [{failureType.Name}] while validating. The settings value is omitted to avoid leaking secrets into logs."; + } } \ No newline at end of file diff --git a/src/Core/ExistForAll.SimpleSettings/SettingsValidatorInvocationException.cs b/src/Core/ExistForAll.SimpleSettings/SettingsValidatorInvocationException.cs new file mode 100644 index 0000000..28049e3 --- /dev/null +++ b/src/Core/ExistForAll.SimpleSettings/SettingsValidatorInvocationException.cs @@ -0,0 +1,19 @@ +namespace ExistForAll.SimpleSettings +{ + // A declared validator threw instead of returning a ValidationResult. Value-free like the rest of the + // family: carries only the validator and failure types — never the inspected settings value (which may be a + // secret) and never the chained inner. See S1 and SettingsValidationException. + public class SettingsValidatorInvocationException : SimpleSettingsException + { + public SettingsValidatorInvocationException(Type validatorType, Type failureType) + : base(Resources.SettingsValidatorInvocationExceptionMessage(validatorType, failureType)) + { + ValidatorType = validatorType; + FailureType = failureType; + } + + public Type ValidatorType { get; } + + public Type FailureType { get; } + } +} diff --git a/src/Core/ExistForAll.SimpleSettings/Validations/ISettingValidation.cs b/src/Core/ExistForAll.SimpleSettings/Validations/ISettingValidation.cs index 0676398..362d817 100644 --- a/src/Core/ExistForAll.SimpleSettings/Validations/ISettingValidation.cs +++ b/src/Core/ExistForAll.SimpleSettings/Validations/ISettingValidation.cs @@ -3,5 +3,10 @@ namespace ExistForAll.SimpleSettings.Validations public interface ISettingValidation : ISettingsValidator { ValidationResult Validate(ValidationContext context); + + // Bridges the non-generic base call to the generic overload so callers dispatch via a plain + // ISettingsValidator.Validate cast — no reflection. Authors implement only the generic Validate. + ValidationResult ISettingsValidator.Validate(ValidationContext context) + => Validate(new ValidationContext((T)context.Settings!)); } } diff --git a/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContext.cs b/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContext.cs index d9fc18f..b60f217 100644 --- a/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContext.cs +++ b/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContext.cs @@ -2,7 +2,7 @@ namespace ExistForAll.SimpleSettings.Validations { public class ValidationContext { - public ValidationContext(object settings) + public ValidationContext(object? settings) { Settings = settings; } diff --git a/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContextOfT.cs b/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContextOfT.cs index 3eaf109..3e61f6f 100644 --- a/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContextOfT.cs +++ b/src/Core/ExistForAll.SimpleSettings/Validations/ValidationContextOfT.cs @@ -3,7 +3,7 @@ namespace ExistForAll.SimpleSettings.Validations public class ValidationContext : ValidationContext { public ValidationContext(T settings) - : base(settings!) + : base(settings) { Settings = settings; } diff --git a/src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs b/src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs index 7753551..880bb34 100644 --- a/src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs +++ b/src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs @@ -64,13 +64,13 @@ public void PopulateInstanceWithValues(object instance, propertyPlan.Property.SetValue(instance, propertyValue); } - // Runs AFTER the property-set loop so cross-property rules see the fully-populated instance (D-09). + // Runs AFTER the property-set loop so cross-property rules see the fully-populated instance. RunValidators(instance, plan); } - // Zero-allocation short-circuit for validator-free types (review B-2): a single field read returns - // before anything is allocated, keeping the warm populate path byte-identical to the pre-validation - // build. The error list is allocated lazily only after a validator actually produces an error. + // Zero-allocation short-circuit for validator-free types: a single field read returns before anything is + // allocated, keeping the warm populate path byte-identical to the pre-validation build. The error list + // is allocated lazily only after a validator actually produces an error. private static void RunValidators(object instance, SettingsPlan plan) { if (!plan.HasValidators) @@ -90,7 +90,7 @@ private static void RunValidators(object instance, SettingsPlan plan) errors = InvokeValidator(propertyPlan.ValidatorType, propertyValue, errors); } - // The single aggregate-and-throw entry point shared with Plan 04's DI runner (review S-3): the + // The single aggregate-and-throw entry point shared with the DI-resolved runner: the // thrown contract cannot drift between the two paths. if (errors is not null) SettingsValidationException.ThrowIfAny(errors); @@ -98,18 +98,21 @@ private static void RunValidators(object instance, SettingsPlan plan) private static List? InvokeValidator(Type validatorType, object? target, List? errors) { - var validator = Activator.CreateInstance(validatorType)!; - - var closedContextType = ResolveContextType(validatorType); - - var context = Activator.CreateInstance(closedContextType, target)!; - - // ISettingValidation : ISettingsValidator declares TWO Validate overloads, so the parameterless - // GetMethod("Validate") throws AmbiguousMatchException — select the overload by its parameter type - // explicitly (review S-2). - var validateMethod = validatorType.GetMethod(nameof(ISettingsValidator.Validate), new[] { closedContextType })!; - - var result = (ValidationResult)validateMethod.Invoke(validator, new[] { context })!; + ValidationResult result; + try + { + // The validator type comes from a compile-time attribute; instantiate it and dispatch through the + // non-generic ISettingsValidator.Validate. ISettingValidation's default implementation forwards + // to the author's generic overload, so no reflection over the Validate methods is needed. + var validator = (ISettingsValidator)Activator.CreateInstance(validatorType)!; + result = validator.Validate(new ValidationContext(target)); + } + catch (Exception e) + { + // A validator threw instead of returning a result. Surface value-free: the inner may embed a + // secret the validator read, so it is never chained and the value never enters the message. See S1. + throw new SettingsValidatorInvocationException(validatorType, e.GetType()); + } foreach (var error in result.Errors) { @@ -120,21 +123,6 @@ private static void RunValidators(object instance, SettingsPlan plan) return errors; } - private static Type ResolveContextType(Type validatorType) - { - foreach (var contract in validatorType.GetInterfaces()) - { - var info = contract.GetTypeInfo(); - if (info.IsGenericType && info.GetGenericTypeDefinition() == typeof(ISettingValidation<>)) - { - var validatedType = info.GetGenericArguments()[0]; - return typeof(ValidationContext<>).MakeGenericType(validatedType); - } - } - - return typeof(ValidationContext); - } - private SettingsPlan GetOrBuildPlan(Type settings, SettingsOptions options) { if (_plans.TryGetValue(settings, out var existing)) diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsValidationTests.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsValidationTests.cs index f813857..c7cc874 100644 --- a/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsValidationTests.cs +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsValidationTests.cs @@ -23,9 +23,25 @@ public async Task ObjectValidator_WhenValid_DoesNotThrow() } [Test] - public async Task PropertyValidator_RunsAgainstThatPropertyValue() + public async Task PropertyValidator_WhenValueInvalid_Throws() { - await Assert.That(() => Build()).Throws(); + // Bind an explicitly invalid value (-5) so the failure is distinct from the unbound default (0) — + // this proves the validator observed the BOUND value, not just the default. + var collection = new InMemoryCollection(); + collection.Add("PropertyValidated", nameof(IPropertyValidated.Port), "-5"); + + await Assert.That(() => Build(collection)).Throws(); + } + + [Test] + public async Task PropertyValidator_WhenValueValid_DoesNotThrow() + { + var collection = new InMemoryCollection(); + collection.Add("PropertyValidated", nameof(IPropertyValidated.Port), "8080"); + + var result = Build(collection); + + await Assert.That(result.Port).IsEqualTo(8080); } [Test] @@ -37,6 +53,22 @@ public async Task MultipleErrors_AggregateIntoOneExceptionWithAllErrors() await Assert.That(exception!.Errors.Count).IsEqualTo(2); } + [Test] + public async Task ObjectAndPropertyValidators_BothFail_AggregateWithObjectErrorFirst() + { + // A type carrying BOTH an object-level and a property-level validator, each failing, aggregates + // into ONE exception; the object error is accumulated before the property error (runs object-first). + var collection = new InMemoryCollection(); + collection.Add("BothValidated", nameof(IBothValidated.Port), "-1"); + + var exception = await Assert.That(() => Build(collection)) + .Throws(); + + await Assert.That(exception!.Errors.Count).IsEqualTo(2); + await Assert.That(exception.Errors[0].SettingsName).IsEqualTo("Object"); + await Assert.That(exception.Errors[1].SettingsName).IsEqualTo("Port"); + } + [Test] public async Task CrossPropertyValidator_ObservesEveryPropertySet() { @@ -74,6 +106,21 @@ public async Task Exception_CarriesAuthorMessage_AndNoBoundValue() await Assert.That(text.Contains(Sentinel)).IsFalse(); } + [Test] + public async Task ThrowingValidator_IsWrappedValueFree_NotLeaked() + { + // A validator that throws (rather than returning a result) with the secret in its message must be + // surfaced value-free: the invocation is wrapped in SettingsValidatorInvocationException, whose + // ToString() carries no bound value and does not chain the leaking inner. + var collection = new InMemoryCollection(); + collection.Add("Throwing", nameof(IThrowing.ApiKey), Sentinel); + + var exception = await Assert.That(() => Build(collection)) + .Throws(); + + await Assert.That(exception!.ToString().Contains(Sentinel)).IsFalse(); + } + [Test] public async Task NoValidators_PopulatesAndReturnsWithoutThrowing() { @@ -117,6 +164,13 @@ public interface ITwoErrors string Name { get; set; } } + [SettingsValidator(typeof(BothObjectValidator))] + public interface IBothValidated + { + [SettingsProperty(ValidatorType = typeof(PositivePortValidator))] + int Port { get; set; } + } + [SettingsValidator(typeof(CrossPropertyValidator))] public interface ICrossProperty { @@ -130,6 +184,12 @@ public interface IRedaction string ApiKey { get; set; } } + [SettingsValidator(typeof(ThrowingValidator))] + public interface IThrowing + { + string ApiKey { get; set; } + } + public interface INoValidators { string Name { get; set; } @@ -137,9 +197,6 @@ public interface INoValidators private class FailingObjectValidator : ISettingValidation { - public ValidationResult Validate(ValidationContext context) - => Validate((ValidationContext)context); - public ValidationResult Validate(ValidationContext context) { var result = new ValidationResult(); @@ -150,17 +207,11 @@ public ValidationResult Validate(ValidationContext context) private class PassingObjectValidator : ISettingValidation { - public ValidationResult Validate(ValidationContext context) - => Validate((ValidationContext)context); - public ValidationResult Validate(ValidationContext context) => new(); } private class PositivePortValidator : ISettingValidation { - public ValidationResult Validate(ValidationContext context) - => Validate((ValidationContext)context); - public ValidationResult Validate(ValidationContext context) { var result = new ValidationResult(); @@ -172,9 +223,6 @@ public ValidationResult Validate(ValidationContext context) private class TwoErrorValidator : ISettingValidation { - public ValidationResult Validate(ValidationContext context) - => Validate((ValidationContext)context); - public ValidationResult Validate(ValidationContext context) { var result = new ValidationResult(); @@ -184,11 +232,18 @@ public ValidationResult Validate(ValidationContext context) } } - private class CrossPropertyValidator : ISettingValidation + private class BothObjectValidator : ISettingValidation { - public ValidationResult Validate(ValidationContext context) - => Validate((ValidationContext)context); + public ValidationResult Validate(ValidationContext context) + { + var result = new ValidationResult(); + result.AddError(new ValidationError("Object", "object-level failure")); + return result; + } + } + private class CrossPropertyValidator : ISettingValidation + { public ValidationResult Validate(ValidationContext context) { var result = new ValidationResult(); @@ -201,9 +256,6 @@ public ValidationResult Validate(ValidationContext context) private class RedactionValidator : ISettingValidation { - public ValidationResult Validate(ValidationContext context) - => Validate((ValidationContext)context); - public ValidationResult Validate(ValidationContext context) { var result = new ValidationResult(); @@ -211,5 +263,11 @@ public ValidationResult Validate(ValidationContext context) return result; } } + + private class ThrowingValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + => throw new InvalidOperationException($"boom with {context.Settings!.ApiKey}"); + } } } From 53ee4a8ef88076979c210ee5e45e8ac4bd3e8ed7 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 19:04:30 +0300 Subject: [PATCH 16/18] test(04-02): lock child-sequence element order + cover empty-element drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- ...lderConfigurationBinderIntegrationTests.cs | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/ConfigBuilderConfigurationBinderIntegrationTests.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests/ConfigBuilderConfigurationBinderIntegrationTests.cs index 2d5e4da..a1fde01 100644 --- a/src/Tests/ExistForAll.SimpleSettings.UnitTests/ConfigBuilderConfigurationBinderIntegrationTests.cs +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/ConfigBuilderConfigurationBinderIntegrationTests.cs @@ -1,3 +1,4 @@ +using System.Linq; using ExistForAll.SimpleSettings.Binder; using ExistForAll.SimpleSettings.Binders; using Microsoft.Extensions.Configuration; @@ -79,7 +80,7 @@ public async Task Build_WhenMemoryBinderSetValues_ShouldSetProperData() } [Test] - public async Task Bind_ChildSequence_ToStringArray_BindsEachElement() + public async Task Bind_ChildSequence_ToStringArray_BindsEachElementInOrder() { var settings = BindSequence(new Dictionary { @@ -88,11 +89,11 @@ public async Task Bind_ChildSequence_ToStringArray_BindsEachElement() ["SequenceStringSetting:Values:2"] = "c", }); - await Assert.That(settings.Values).IsEquivalentTo(new[] { "a", "b", "c" }); + await Assert.That(settings.Values.SequenceEqual(new[] { "a", "b", "c" })).IsTrue(); } [Test] - public async Task Bind_ChildSequence_ToIntArray_BindsEachElement() + public async Task Bind_ChildSequence_ToIntArray_BindsEachElementInOrder() { var settings = BindSequence(new Dictionary { @@ -101,11 +102,26 @@ public async Task Bind_ChildSequence_ToIntArray_BindsEachElement() ["SequenceIntSetting:Numbers:2"] = "3", }); - await Assert.That(settings.Numbers).IsEquivalentTo(new[] { 1, 2, 3 }); + await Assert.That(settings.Numbers.SequenceEqual(new[] { 1, 2, 3 })).IsTrue(); } [Test] - public async Task Bind_ChildSequence_ToStringList_BindsEachElement() + public async Task Bind_ChildSequence_WithEmptyElements_DropsThemLikeCommaScalar() + { + // Empty entries are dropped for parity with the comma-scalar RemoveEmptyEntries, so an interspersed + // empty element does not crash the int[] bind (review MED-2). + var settings = BindSequence(new Dictionary + { + ["SequenceIntSetting:Numbers:0"] = "1", + ["SequenceIntSetting:Numbers:1"] = "", + ["SequenceIntSetting:Numbers:2"] = "3", + }); + + await Assert.That(settings.Numbers.SequenceEqual(new[] { 1, 3 })).IsTrue(); + } + + [Test] + public async Task Bind_ChildSequence_ToStringList_BindsEachElementInOrder() { var settings = BindSequence(new Dictionary { @@ -113,7 +129,7 @@ public async Task Bind_ChildSequence_ToStringList_BindsEachElement() ["SequenceStringListSetting:Items:1"] = "y", }); - await Assert.That(settings.Items).IsEquivalentTo(new List { "x", "y" }); + await Assert.That(settings.Items.SequenceEqual(new List { "x", "y" })).IsTrue(); } [Test] @@ -126,7 +142,7 @@ public async Task Bind_ScalarAndChildren_ChildrenWin() ["SequenceStringSetting:Values:1"] = "b", }); - await Assert.That(settings.Values).IsEquivalentTo(new[] { "a", "b" }); + await Assert.That(settings.Values.SequenceEqual(new[] { "a", "b" })).IsTrue(); } [Test] @@ -137,7 +153,7 @@ public async Task Bind_CommaScalar_NoChildren_StillBinds() ["SequenceStringSetting:Values"] = "a,b,c", }); - await Assert.That(settings.Values).IsEquivalentTo(new[] { "a", "b", "c" }); + await Assert.That(settings.Values.SequenceEqual(new[] { "a", "b", "c" })).IsTrue(); } [Test] @@ -168,7 +184,7 @@ public async Task Bind_ChildSequence_WithRootSection_ResolvesUnderPrefix() ["MyRoot:SequenceStringSetting:Values:1"] = "b", }, "MyRoot"); - await Assert.That(settings.Values).IsEquivalentTo(new[] { "a", "b" }); + await Assert.That(settings.Values.SequenceEqual(new[] { "a", "b" })).IsTrue(); } private static T BindSequence(Dictionary data, string? rootSection = null) where T : class From d4ebe41efd80c260cc1fac7f89ecd39fb4cfad16 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 19:04:30 +0300 Subject: [PATCH 17/18] chore(04): strip internal review-ID tokens from code comments --- .../ExistForAll.SimpleSettings/Conversion/ListTypeConverter.cs | 2 +- .../ExistForAll.SimpleSettings/Core/Reflection/TypeConverter.cs | 2 +- src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Core/ExistForAll.SimpleSettings/Conversion/ListTypeConverter.cs b/src/Core/ExistForAll.SimpleSettings/Conversion/ListTypeConverter.cs index 4a4b0cb..b6b8a93 100644 --- a/src/Core/ExistForAll.SimpleSettings/Conversion/ListTypeConverter.cs +++ b/src/Core/ExistForAll.SimpleSettings/Conversion/ListTypeConverter.cs @@ -7,7 +7,7 @@ namespace ExistForAll.SimpleSettings.Conversion // Handles the List family (List/IList/ICollection/IReadOnlyList/IReadOnlyCollection) by reusing the // base array-build path and copying the result into a List. Materialization uses a cached per-element // delegate (built once, invoked on every populate) so the warm path carries no MakeGenericType/Activator - // reflection (review S-4/A1). + // reflection. internal class ListTypeConverter : CollectionTypeConverter { private static readonly MethodInfo CreateListMethod = diff --git a/src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeConverter.cs b/src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeConverter.cs index 2fcc5e4..f9778a1 100644 --- a/src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeConverter.cs +++ b/src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeConverter.cs @@ -26,7 +26,7 @@ public PropertyConversion CreateConversion(PropertyInfo propertyInfo, SettingsPr typeof(TypeConverter).GetMethod(nameof(CreateEmptyList), BindingFlags.NonPublic | BindingFlags.Static)!; // The value a null bound value converts to per shape: a shared empty array for arrays and IEnumerable, - // a fresh-per-bind empty List for the List family (review B-3: a mutable list must not be shared), a + // a fresh-per-bind empty List for the List family (a mutable list must not be shared), a // default instance for a value type, otherwise null. Resolved once at plan build; the list branch bakes a // factory delegate (cold reflection here, plain new List() per call) rather than a shared instance. private static object? CreateNullResult(Type propertyType) diff --git a/src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs b/src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs index 4f41ff0..aa7512e 100644 --- a/src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs +++ b/src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs @@ -45,7 +45,7 @@ private static bool AnyPropertyHasValidator(PropertyPlan[] properties) public Type? ObjectValidatorType { get; } // Computed once at plan build so the post-populate hook can short-circuit with a single field read - // before allocating anything on the validator-free warm path (review B-2). + // before allocating anything on the validator-free warm path. public bool HasValidators { get; } } From b049f8db2caeb66c438b612e6517231e3ae189b6 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Wed, 15 Jul 2026 20:11:48 +0300 Subject: [PATCH 18/18] 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. --- .../SettingsPlan.cs | 4 +- .../SettingsSectionAttribute.cs | 5 +- .../SettingsValidatorAttribute.cs | 13 -- .../ValuesPopulator.cs | 6 +- src/SimpleSettings.slnx | 1 + ...l.SimpleSettings.UnitTests.Fixtures.csproj | 9 ++ .../ValidationFixtures.cs | 136 ++++++++++++++++ ...xistForAll.SimpleSettings.UnitTests.csproj | 1 + .../SimpleSettings/SettingsValidationTests.cs | 147 ++---------------- 9 files changed, 168 insertions(+), 154 deletions(-) delete mode 100644 src/Core/ExistForAll.SimpleSettings/SettingsValidatorAttribute.cs create mode 100644 src/Tests/ExistForAll.SimpleSettings.UnitTests.Fixtures/ExistForAll.SimpleSettings.UnitTests.Fixtures.csproj create mode 100644 src/Tests/ExistForAll.SimpleSettings.UnitTests.Fixtures/ValidationFixtures.cs diff --git a/src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs b/src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs index aa7512e..0509e85 100644 --- a/src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs +++ b/src/Core/ExistForAll.SimpleSettings/SettingsPlan.cs @@ -40,8 +40,8 @@ private static bool AnyPropertyHasValidator(PropertyPlan[] properties) public PropertyPlan[] Properties { get; } - // The [SettingsValidator] object-level validator declared on the settings interface, resolved once at - // plan build (null when none is declared). + // The object-level validator ([SettingsSection].ValidatorType) declared on the settings interface, + // resolved once at plan build (null when none is declared). public Type? ObjectValidatorType { get; } // Computed once at plan build so the post-populate hook can short-circuit with a single field read diff --git a/src/Core/ExistForAll.SimpleSettings/SettingsSectionAttribute.cs b/src/Core/ExistForAll.SimpleSettings/SettingsSectionAttribute.cs index 50de319..472ce22 100644 --- a/src/Core/ExistForAll.SimpleSettings/SettingsSectionAttribute.cs +++ b/src/Core/ExistForAll.SimpleSettings/SettingsSectionAttribute.cs @@ -5,11 +5,12 @@ public class SettingsSectionAttribute : Attribute { public string? Name { get; set; } + public Type? ValidatorType { get; set; } + public SettingsSectionAttribute() { - } - + public SettingsSectionAttribute(string name) { Name = name; diff --git a/src/Core/ExistForAll.SimpleSettings/SettingsValidatorAttribute.cs b/src/Core/ExistForAll.SimpleSettings/SettingsValidatorAttribute.cs deleted file mode 100644 index 626f34c..0000000 --- a/src/Core/ExistForAll.SimpleSettings/SettingsValidatorAttribute.cs +++ /dev/null @@ -1,13 +0,0 @@ -namespace ExistForAll.SimpleSettings -{ - [AttributeUsage(AttributeTargets.Interface)] - public class SettingsValidatorAttribute : Attribute - { - public Type ValidatorType { get; } - - public SettingsValidatorAttribute(Type validatorType) - { - ValidatorType = validatorType; - } - } -} diff --git a/src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs b/src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs index 880bb34..a39dff4 100644 --- a/src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs +++ b/src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs @@ -162,10 +162,10 @@ private SettingsPlan GetOrBuildPlan(Type settings, SettingsOptions options) } } - // Read the object-level [SettingsValidator] once per type at plan build, mirroring the per-property - // attribute read above — never on the hot populate path. + // Read the object-level validator ([SettingsSection].ValidatorType) once per type at plan build, + // mirroring the per-property attribute read above — never on the hot populate path. var objectValidatorType = settings.GetTypeInfo() - .GetCustomAttribute(inherit: true)?.ValidatorType; + .GetCustomAttribute(inherit: true)?.ValidatorType; var plan = new SettingsPlan(settings, options, propertyPlans, objectValidatorType); diff --git a/src/SimpleSettings.slnx b/src/SimpleSettings.slnx index e96c1d8..8c36805 100644 --- a/src/SimpleSettings.slnx +++ b/src/SimpleSettings.slnx @@ -14,6 +14,7 @@ + diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests.Fixtures/ExistForAll.SimpleSettings.UnitTests.Fixtures.csproj b/src/Tests/ExistForAll.SimpleSettings.UnitTests.Fixtures/ExistForAll.SimpleSettings.UnitTests.Fixtures.csproj new file mode 100644 index 0000000..84edc35 --- /dev/null +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests.Fixtures/ExistForAll.SimpleSettings.UnitTests.Fixtures.csproj @@ -0,0 +1,9 @@ + + + net8.0;net10.0 + false + + + + + diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests.Fixtures/ValidationFixtures.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests.Fixtures/ValidationFixtures.cs new file mode 100644 index 0000000..300e44d --- /dev/null +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests.Fixtures/ValidationFixtures.cs @@ -0,0 +1,136 @@ +using ExistForAll.SimpleSettings.Validations; + +namespace ExistForAll.SimpleSettings.UnitTests.Fixtures +{ + // Separate, never-scanned assembly so these deliberately-failing validators dodge the whole-assembly ScanAssemblies tests. + + [SettingsSection(ValidatorType = typeof(FailingObjectValidator))] + public interface IObjectValidated + { + string Name { get; set; } + } + + [SettingsSection(ValidatorType = typeof(PassingObjectValidator))] + public interface IObjectValid + { + string Name { get; set; } + } + + public interface IPropertyValidated + { + [SettingsProperty(ValidatorType = typeof(PositivePortValidator))] + int Port { get; set; } + } + + [SettingsSection(ValidatorType = typeof(TwoErrorValidator))] + public interface ITwoErrors + { + string Name { get; set; } + } + + [SettingsSection(ValidatorType = typeof(BothObjectValidator))] + public interface IBothValidated + { + [SettingsProperty(ValidatorType = typeof(PositivePortValidator))] + int Port { get; set; } + } + + [SettingsSection(ValidatorType = typeof(CrossPropertyValidator))] + public interface ICrossProperty + { + int Min { get; set; } + int Max { get; set; } + } + + [SettingsSection(ValidatorType = typeof(RedactionValidator))] + public interface IRedaction + { + string ApiKey { get; set; } + } + + [SettingsSection(ValidatorType = typeof(ThrowingValidator))] + public interface IThrowing + { + string ApiKey { get; set; } + } + + public interface INoValidators + { + string Name { get; set; } + } + + public class FailingObjectValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + { + var result = new ValidationResult(); + result.AddError(new ValidationError(nameof(IObjectValidated.Name), "name is invalid")); + return result; + } + } + + public class PassingObjectValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) => new(); + } + + public class PositivePortValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + { + var result = new ValidationResult(); + if (context.Settings <= 0) + result.AddError(new ValidationError("Port", "port must be positive")); + return result; + } + } + + public class TwoErrorValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + { + var result = new ValidationResult(); + result.AddError(new ValidationError("Name", "first failure")); + result.AddError(new ValidationError("Name", "second failure")); + return result; + } + } + + public class BothObjectValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + { + var result = new ValidationResult(); + result.AddError(new ValidationError("Object", "object-level failure")); + return result; + } + } + + public class CrossPropertyValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + { + var result = new ValidationResult(); + var settings = context.Settings!; + if (settings.Min > settings.Max) + result.AddError(new ValidationError("Min", "Min must not exceed Max")); + return result; + } + } + + public class RedactionValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + { + var result = new ValidationResult(); + result.AddError(new ValidationError("ApiKey", "ApiKey failed policy")); + return result; + } + } + + public class ThrowingValidator : ISettingValidation + { + public ValidationResult Validate(ValidationContext context) + => throw new InvalidOperationException($"boom with {context.Settings!.ApiKey}"); + } +} diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/ExistForAll.SimpleSettings.UnitTests.csproj b/src/Tests/ExistForAll.SimpleSettings.UnitTests/ExistForAll.SimpleSettings.UnitTests.csproj index 0804179..9bf98dc 100644 --- a/src/Tests/ExistForAll.SimpleSettings.UnitTests/ExistForAll.SimpleSettings.UnitTests.csproj +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/ExistForAll.SimpleSettings.UnitTests.csproj @@ -18,5 +18,6 @@ + diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsValidationTests.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsValidationTests.cs index c7cc874..ddab35a 100644 --- a/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsValidationTests.cs +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsValidationTests.cs @@ -1,11 +1,9 @@ using ExistForAll.SimpleSettings.Binder; -using ExistForAll.SimpleSettings.Validations; +using ExistForAll.SimpleSettings.UnitTests.Fixtures; namespace ExistForAll.SimpleSettings.UnitTests.SimpleSettings { - // Fixtures are intentionally NON-settings-indicated (no "Settings" suffix, no [SettingsSection]/ - // ISettingsSection) and resolved via GetSettings() so a deliberately-failing validator never trips - // ScanAssemblies-based tests elsewhere in the suite. + // Fixtures live in the never-scanned ExistForAll.SimpleSettings.UnitTests.Fixtures project; resolved via GetSettings(). public class SettingsValidationTests { [Test] @@ -129,6 +127,17 @@ public async Task NoValidators_PopulatesAndReturnsWithoutThrowing() await Assert.That(result).IsNotNull(); } + [Test] + public async Task ScanAssemblies_WhenSectionCarriesValidator_RunsItAndSurfacesFailure() + { + // Guards the scan path for the merged [SettingsSection(ValidatorType=...)]: scanning the Fixtures + // assembly (whose discovered sections carry failing validators) must surface a validation failure. + var builder = SettingsBuilder.CreateBuilder(); + + await Assert.That(() => builder.ScanAssemblies(typeof(IObjectValidated).Assembly)) + .Throws(); + } + private const string Sentinel = "SUPER_SECRET_API_KEY_VALUE"; private static T Build(InMemoryCollection? collection = null) where T : class @@ -139,135 +148,5 @@ private static T Build(InMemoryCollection? collection = null) where T : class return builder.GetSettings(); } - - [SettingsValidator(typeof(FailingObjectValidator))] - public interface IObjectValidated - { - string Name { get; set; } - } - - [SettingsValidator(typeof(PassingObjectValidator))] - public interface IObjectValid - { - string Name { get; set; } - } - - public interface IPropertyValidated - { - [SettingsProperty(ValidatorType = typeof(PositivePortValidator))] - int Port { get; set; } - } - - [SettingsValidator(typeof(TwoErrorValidator))] - public interface ITwoErrors - { - string Name { get; set; } - } - - [SettingsValidator(typeof(BothObjectValidator))] - public interface IBothValidated - { - [SettingsProperty(ValidatorType = typeof(PositivePortValidator))] - int Port { get; set; } - } - - [SettingsValidator(typeof(CrossPropertyValidator))] - public interface ICrossProperty - { - int Min { get; set; } - int Max { get; set; } - } - - [SettingsValidator(typeof(RedactionValidator))] - public interface IRedaction - { - string ApiKey { get; set; } - } - - [SettingsValidator(typeof(ThrowingValidator))] - public interface IThrowing - { - string ApiKey { get; set; } - } - - public interface INoValidators - { - string Name { get; set; } - } - - private class FailingObjectValidator : ISettingValidation - { - public ValidationResult Validate(ValidationContext context) - { - var result = new ValidationResult(); - result.AddError(new ValidationError(nameof(IObjectValidated.Name), "name is invalid")); - return result; - } - } - - private class PassingObjectValidator : ISettingValidation - { - public ValidationResult Validate(ValidationContext context) => new(); - } - - private class PositivePortValidator : ISettingValidation - { - public ValidationResult Validate(ValidationContext context) - { - var result = new ValidationResult(); - if (context.Settings <= 0) - result.AddError(new ValidationError("Port", "port must be positive")); - return result; - } - } - - private class TwoErrorValidator : ISettingValidation - { - public ValidationResult Validate(ValidationContext context) - { - var result = new ValidationResult(); - result.AddError(new ValidationError("Name", "first failure")); - result.AddError(new ValidationError("Name", "second failure")); - return result; - } - } - - private class BothObjectValidator : ISettingValidation - { - public ValidationResult Validate(ValidationContext context) - { - var result = new ValidationResult(); - result.AddError(new ValidationError("Object", "object-level failure")); - return result; - } - } - - private class CrossPropertyValidator : ISettingValidation - { - public ValidationResult Validate(ValidationContext context) - { - var result = new ValidationResult(); - var settings = context.Settings!; - if (settings.Min > settings.Max) - result.AddError(new ValidationError("Min", "Min must not exceed Max")); - return result; - } - } - - private class RedactionValidator : ISettingValidation - { - public ValidationResult Validate(ValidationContext context) - { - var result = new ValidationResult(); - result.AddError(new ValidationError("ApiKey", "ApiKey failed policy")); - return result; - } - } - - private class ThrowingValidator : ISettingValidation - { - public ValidationResult Validate(ValidationContext context) - => throw new InvalidOperationException($"boom with {context.Settings!.ApiKey}"); - } } }