From 098bb14fdd39523aea1fdd9d5b59b678a80205d3 Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 18:58:56 -0400 Subject: [PATCH 1/3] test: specs for reporting a dictionary value by its key A dictionary value is to be reported by its key, in the form MVC model binding uses: Map[Primary].Name, not Map[0].Value.Name. Specs: string, number, enum, Guid and decimal keys (invariant culture), a key with path characters (not escaped), each entry choosing its form, struct values, every kind of dictionary, the value one level below the dictionary, a shared dictionary of structs, and an entry that a Validate method adds, found by a second route. Guards: an object key and a key with something to validate keep the index form, a null value is skipped, and a list of KeyValuePair items is not a dictionary. Existing tests move to the key form. The 2.x dictionary test leaves UpgradeCompatibilityTests, because 3.0 differs there. Co-Authored-By: Claude Opus 5.5 --- .../DictionaryKeyPathTests.cs | 380 ++++++++++++++++++ .../MemberNameFormatTests.cs | 6 +- .../OddShapeTests.cs | 20 +- .../StructCollectionPropertyTests.cs | 6 +- .../StructsReachedByTwoRoutesTests.cs | 5 +- .../UpgradeCompatibilityTests.cs | 17 +- .../ValidatorHardeningTests.cs | 8 +- 7 files changed, 404 insertions(+), 38 deletions(-) create mode 100644 test/RecursiveDataAnnotationsValidation.Tests/DictionaryKeyPathTests.cs diff --git a/test/RecursiveDataAnnotationsValidation.Tests/DictionaryKeyPathTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/DictionaryKeyPathTests.cs new file mode 100644 index 0000000..2e618fc --- /dev/null +++ b/test/RecursiveDataAnnotationsValidation.Tests/DictionaryKeyPathTests.cs @@ -0,0 +1,380 @@ +using System; +using System.Collections; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.ComponentModel.DataAnnotations; +using System.Globalization; +using System.Linq; +using Xunit; + +namespace RecursiveDataAnnotationsValidation.Tests +{ + /// + /// A value of a dictionary is reported by its key: Map[Primary].Name. Before 3.0 it was + /// reported by its position in enumeration order, through the KeyValuePair that the dictionary + /// yields: Map[0].Value.Name. + /// + /// The form is the one that MVC model binding uses for a dictionary, such as + /// selectedCourses[1050]=Chemistry. The key is not quoted and not escaped, so a key that + /// contains "]" or "." makes the path ambiguous. + /// See: https://learn.microsoft.com/aspnet/core/mvc/models/model-binding + /// + /// The rules: + /// - A dictionary implements IDictionary, IDictionary<TKey, TValue> or + /// IReadOnlyDictionary<TKey, TValue>. A list of KeyValuePair items is not one. + /// - A key names its value when it is a string, a primitive, an enum, or a struct that formats + /// itself, such as a decimal or a Guid, and it has nothing to validate. The text uses the + /// invariant culture, so a decimal key is "1.5" on a German server too. + /// See: https://learn.microsoft.com/dotnet/api/system.globalization.cultureinfo.invariantculture + /// - Any other key, such as an object of your own, keeps the old form for that entry, so the + /// key is still validated: Map[0].Key.Name and Map[0].Value.Name. + /// - The value is one level below the dictionary, as an item of a list is. Before, the + /// KeyValuePair was one more level, so a value now comes earlier in the results. + /// + public class DictionaryKeyPathTests + { + public class Leaf + { + [Required] + public string Name { get; set; } + } + + public struct Line + { + [Required] + public string Text { get; set; } + } + + public class Holder + { + public T Map { get; set; } + } + + // An attribute that never fails. On an enum it makes the enum a type with something to + // validate, so it is not a leaf type and cannot name a value. + [AttributeUsage(AttributeTargets.Enum)] + public class AlwaysValidAttribute : ValidationAttribute + { + public override bool IsValid(object value) => true; + } + + [AlwaysValid] + public enum CheckedColor { Red } + + // A dictionary that implements only IReadOnlyDictionary. + public class ReadOnlyLeafMap : IReadOnlyDictionary + { + private readonly Dictionary _inner; + + public ReadOnlyLeafMap(Dictionary inner) + { + _inner = inner; + } + + public Leaf this[string key] => _inner[key]; + public IEnumerable Keys => _inner.Keys; + public IEnumerable Values => _inner.Values; + public int Count => _inner.Count; + public bool ContainsKey(string key) => _inner.ContainsKey(key); + public bool TryGetValue(string key, out Leaf value) => _inner.TryGetValue(key, out value); + public IEnumerator> GetEnumerator() => _inner.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class Ordered + { + public Leaf First { get; set; } + public Dictionary Map { get; set; } + public List Items { get; set; } + } + + public class TwoMaps + { + public Dictionary First { get; set; } + public Dictionary Second { get; set; } + } + + // Adds an invalid entry to the dictionary that holds it, when it is validated. + public class Adder : IValidatableObject + { + public string Name { get; set; } = "ok"; + + public Dictionary Owner { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + if (Owner != null && !Owner.ContainsKey("added")) + { + Owner["added"] = new Leaf(); + } + + return Enumerable.Empty(); + } + } + + public class AdderRoot + { + public Dictionary Map { get; set; } + public Adder Adder { get; set; } + } + + private const string NameRequired = " | The Name field is required."; + private const string TextRequired = " | The Text field is required."; + + private static (bool Valid, List Errors) Run(object model) + { + var results = new List(); + var valid = new RecursiveDataAnnotationValidator().TryValidateObjectRecursive(model, results); + return (valid, ResultText.Describe(results)); + } + + // Spec. + [Fact] + public void String_key_names_the_value() + { + var (valid, errors) = Run(new Holder> + { + Map = new Dictionary { ["Primary"] = new Leaf() }, + }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Map[Primary].Name" + NameRequired), errors); + } + + // Spec. The key, not the position: a SortedDictionary enumerates in key order, so "b" + // was Map[1] before. + // See: https://learn.microsoft.com/dotnet/api/system.collections.generic.sorteddictionary-2 + [Fact] + public void Key_is_used_and_not_the_position() + { + var (valid, errors) = Run(new Holder> + { + Map = new SortedDictionary + { + ["b"] = new Leaf(), + ["a"] = new Leaf { Name = "ok" }, + ["c"] = new Leaf { Name = "ok" }, + }, + }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Map[b].Name" + NameRequired), errors); + } + + // Spec. + [Fact] + public void Number_and_enum_keys_name_the_value() + { + Assert.Equal( + ResultText.Expect("Map[42].Name" + NameRequired), + Run(new Holder> { Map = new Dictionary { [42] = new Leaf() } }).Errors); + + Assert.Equal( + ResultText.Expect("Map[Monday].Name" + NameRequired), + Run(new Holder> { Map = new Dictionary { [DayOfWeek.Monday] = new Leaf() } }).Errors); + } + + // Spec. The key is formatted with the invariant culture, so the path is the same on every + // server. German formats 1.5 as "1,5". + [Fact] + public void Key_is_formatted_with_the_invariant_culture() + { + var original = CultureInfo.CurrentCulture; + try + { + CultureInfo.CurrentCulture = new CultureInfo("de-DE"); + + var (valid, errors) = Run(new Holder> + { + Map = new Dictionary { [1.5m] = new Leaf() }, + }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Map[1.5].Name" + NameRequired), errors); + } + finally + { + CultureInfo.CurrentCulture = original; + } + } + + // Spec. + [Fact] + public void Guid_key_names_the_value() + { + var key = new Guid("0f8fad5b-d9cb-469f-a165-70867728950e"); + + var (valid, errors) = Run(new Holder> { Map = new Dictionary { [key] = new Leaf() } }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Map[0f8fad5b-d9cb-469f-a165-70867728950e].Name" + NameRequired), errors); + } + + // Spec. The key is not escaped, as in MVC, so this path cannot be told apart from a key + // "b" that holds a list. The README says so. + [Fact] + public void Key_with_path_characters_is_not_escaped() + { + var (valid, errors) = Run(new Holder> + { + Map = new Dictionary { ["b.c[0]"] = new Leaf() }, + }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Map[b.c[0]].Name" + NameRequired), errors); + } + + // Guard. A key that is an object keeps the old form, so the key is validated too. + [Fact] + public void Object_key_keeps_the_index_form() + { + var (valid, errors) = Run(new Holder> + { + Map = new Dictionary { [new Leaf()] = new Leaf() }, + }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Map[0].Key.Name" + NameRequired, "Map[0].Value.Name" + NameRequired), errors); + } + + // Spec. The form is chosen for each entry. The second entry has an object key, so it uses + // its position. A Dictionary with no removals enumerates in the order the entries were added. + // See: https://learn.microsoft.com/dotnet/api/system.collections.generic.dictionary-2#remarks + [Fact] + public void Each_entry_chooses_its_own_form() + { + var (valid, errors) = Run(new Holder> + { + Map = new Dictionary + { + ["a"] = new Leaf(), + [new Leaf { Name = "key" }] = new Leaf(), + }, + }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Map[a].Name" + NameRequired, "Map[1].Value.Name" + NameRequired), errors); + } + + // Guard. An enum with a validation attribute has something to validate, so it does not + // name the value, and the entry keeps the index form. + [Fact] + public void Key_with_something_to_validate_keeps_the_index_form() + { + var (valid, errors) = Run(new Holder> + { + Map = new Dictionary { [CheckedColor.Red] = new Leaf() }, + }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Map[0].Value.Name" + NameRequired), errors); + } + + // Spec. + [Fact] + public void Struct_value_is_named_by_its_key() + { + var (valid, errors) = Run(new Holder> { Map = new Dictionary { ["a"] = new Line() } }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Map[a].Text" + TextRequired), errors); + } + + // Guard. + [Fact] + public void Null_value_is_skipped() + { + var (valid, errors) = Run(new Holder> { Map = new Dictionary { ["a"] = null } }); + + Assert.True(valid); + Assert.Empty(errors); + } + + // Guard. A list of KeyValuePair items is not a dictionary, so it keeps the index form. + [Fact] + public void List_of_pairs_keeps_the_index_form() + { + var (valid, errors) = Run(new Holder>> + { + Map = new List> { new KeyValuePair("a", new Leaf()) }, + }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Map[0].Value.Name" + NameRequired), errors); + } + + public static IEnumerable Dictionaries() + { + Func> one = () => new Dictionary { ["a"] = new Leaf() }; + + yield return new object[] { "Hashtable", (Func)(() => new Hashtable { ["a"] = new Leaf() }) }; + yield return new object[] { "SortedList", (Func)(() => new SortedList(one())) }; + yield return new object[] { "ConcurrentDictionary", (Func)(() => new ConcurrentDictionary(one())) }; + yield return new object[] { "ReadOnlyDictionary", (Func)(() => new ReadOnlyDictionary(one())) }; + yield return new object[] { "IReadOnlyDictionary only", (Func)(() => new ReadOnlyLeafMap(one())) }; +#if NET8_0_OR_GREATER + yield return new object[] { "ImmutableDictionary", (Func)(() => System.Collections.Immutable.ImmutableDictionary.CreateRange(one())) }; +#endif + } + + // Spec. Every kind of dictionary uses the key. + [Theory] + [MemberData(nameof(Dictionaries))] + public void Every_kind_of_dictionary_uses_the_key(string name, Func create) + { + Assert.NotNull(name); + + var (valid, errors) = Run(new Holder { Map = create() }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Map[a].Name" + NameRequired), errors); + } + + // Spec. A value is one level below the dictionary, like an item of a list, so it comes in + // property order with the items of Items. Before, it came after them: Map[0] was one more + // level than Items[0]. + [Fact] + public void Value_is_one_level_below_the_dictionary() + { + var (valid, errors) = Run(new Ordered + { + First = new Leaf(), + Map = new Dictionary { ["a"] = new Leaf() }, + Items = new List { new Leaf() }, + }); + + Assert.False(valid); + Assert.Equal( + ResultText.Expect("First.Name" + NameRequired, "Map[a].Name" + NameRequired, "Items[0].Name" + NameRequired), + errors); + } + + // Spec. A dictionary of structs that two properties hold reports each struct once, at the + // shortest path, as an array of structs does. + [Fact] + public void Struct_value_in_a_dictionary_that_two_properties_hold_is_reported_once() + { + var shared = new Dictionary { ["a"] = new Line() }; + + var (valid, errors) = Run(new TwoMaps { First = shared, Second = shared }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("First[a].Text" + TextRequired), errors); + } + + // Spec. A second route enumerates the dictionary again and finds the entry that the + // Validate method of Adder added in the meantime. Its value is a class object, so it is + // not skipped as a struct would be. + [Fact] + public void Value_added_by_validation_is_found_by_a_second_route() + { + var map = new Dictionary { ["a"] = new Leaf { Name = "ok" } }; + + var (valid, errors) = Run(new AdderRoot { Map = map, Adder = new Adder { Owner = map } }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect("Adder.Owner[added].Name" + NameRequired), errors); + } + } +} diff --git a/test/RecursiveDataAnnotationsValidation.Tests/MemberNameFormatTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/MemberNameFormatTests.cs index c2f73de..0f30e1e 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/MemberNameFormatTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/MemberNameFormatTests.cs @@ -135,8 +135,8 @@ public class Container // Each case below shows one rule for building the member name: // - An array item gets an index like a list item. // - A null item is skipped but still uses up its index: NoteList[1], not NoteList[0]. - // - A dictionary item is a KeyValuePair, so its value is reached through ".Value", and the - // index is the position in enumeration order, not the key. + // - A dictionary value is reported by its key: NoteMap[first]. Before 3.0 it was reached + // through ".Value", and the index was the position in enumeration order. // - A result with two member names gets the prefix on each of them. // - A result with no member names gets the path of the item as its member name. // Before 3.0 it had no path at all. @@ -160,7 +160,7 @@ public void Collection_items_keep_their_member_names() { "NoteArray[0].Text | The Text field is required.", "NoteList[1].Text | The Text field is required.", - "NoteMap[0].Value.Text | The Text field is required.", + "NoteMap[first].Text | The Text field is required.", "Ranges[0].Low,Ranges[0].High | Low must not exceed High.", "Ranges[0] | The range is negative.", }; diff --git a/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs index b2401b3..7297096 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/OddShapeTests.cs @@ -78,8 +78,8 @@ private static (bool Valid, List Errors) Run(object model) /// must not remove an error that 2.3.3 reports (see CollectionsWithMembersOfTheirOwn). /// The items are enumerated before the properties are walked. An object is validated once, /// so a Leaf that is reachable both ways is reported at the enumeration path. - /// A dictionary item is enumerated as KeyValuePair items, so a List of Dictionary reports - /// "Value[0][0].Value.Name", the same member names as a dictionary held in a property. + /// A dictionary value is reported by its key, so a List of Dictionary reports + /// "Value[0][k].Name", the same member names as a dictionary held in a property. /// A collection of a struct type, such as ImmutableArray or a readonly struct that implements /// IEnumerable, is enumerated like any other, so an item that hides objects does not pass. /// See: https://learn.microsoft.com/dotnet/api/system.array.syncroot @@ -453,11 +453,11 @@ public void List_of_dictionaries_is_validated() }); Assert.False(valid); - Assert.Equal(ResultText.Expect("Value[0][0].Value.Name" + NameRequired), errors); + Assert.Equal(ResultText.Expect("Value[0][k].Name" + NameRequired), errors); } [Fact] - public void Dictionary_of_lists_is_validated_through_the_pair_value() + public void Dictionary_of_lists_is_validated_through_the_key() { var (valid, errors) = Run(new Holder>> { @@ -465,7 +465,7 @@ public void Dictionary_of_lists_is_validated_through_the_pair_value() }); Assert.False(valid); - Assert.Equal(ResultText.Expect("Value[0].Value[0].Name" + NameRequired), errors); + Assert.Equal(ResultText.Expect("Value[k][0].Name" + NameRequired), errors); } // Up to release 2.3.3 the path was "Value[0].SyncRoot[0].Name". @@ -623,14 +623,14 @@ public void Items_of_a_root_array_are_validated_at_their_index() Assert.Equal(ResultText.Expect("[0].Name" + NameRequired), errors); } - // A dictionary is enumerated as KeyValuePair items, as when a property holds it. + // A dictionary value is reported by its key, as when a property holds the dictionary. [Fact] public void Values_of_a_root_dictionary_are_validated() { var (valid, errors) = Run(new Dictionary { ["a"] = new Leaf() }); Assert.False(valid); - Assert.Equal(ResultText.Expect("[0].Value.Name" + NameRequired), errors); + Assert.Equal(ResultText.Expect("[a].Name" + NameRequired), errors); } [Fact] @@ -1112,7 +1112,7 @@ public void Hashtable_values_are_validated() var (valid, errors) = Run(new Holder { Value = new Hashtable { ["k"] = new Leaf() } }); Assert.False(valid); - Assert.Equal(ResultText.Expect("Value[0].Value.Name" + NameRequired), errors); + Assert.Equal(ResultText.Expect("Value[k].Name" + NameRequired), errors); } [Fact] @@ -1133,7 +1133,7 @@ public void ExpandoObject_values_are_validated() var (valid, errors) = Run(new Holder { Value = expando }); Assert.False(valid); - Assert.Equal(ResultText.Expect("Value[0].Value.Name" + NameRequired), errors); + Assert.Equal(ResultText.Expect("Value[item].Name" + NameRequired), errors); } [Fact] @@ -1148,7 +1148,7 @@ public void Dictionary_of_object_holding_a_dictionary_is_validated() }); Assert.False(valid); - Assert.Equal(ResultText.Expect("Value[0].Value[0].Value.Name" + NameRequired), errors); + Assert.Equal(ResultText.Expect("Value[a][b].Name" + NameRequired), errors); } [Fact] diff --git a/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs index 2c32405..9dabf5e 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/StructCollectionPropertyTests.cs @@ -481,8 +481,8 @@ public void Default_struct_with_an_Equals_that_throws_is_skipped() } #if NET8_0_OR_GREATER - // A dictionary value that is a struct collection. The KeyValuePair item of the dictionary is - // walked, and so is the struct collection that its Value property holds. + // A dictionary value that is a struct collection. The value is reported by its key, and the + // struct collection is enumerated. [Fact] public void Dictionary_value_that_is_a_struct_collection_is_validated() { @@ -492,7 +492,7 @@ public void Dictionary_value_that_is_a_struct_collection_is_validated() }); Assert.False(valid); - Assert.Equal(ResultText.Expect("Value[0].Value[0].Name" + NameRequired), errors); + Assert.Equal(ResultText.Expect("Value[a][0].Name" + NameRequired), errors); } [Fact] diff --git a/test/RecursiveDataAnnotationsValidation.Tests/StructsReachedByTwoRoutesTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/StructsReachedByTwoRoutesTests.cs index 35bf09a..157195e 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/StructsReachedByTwoRoutesTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/StructsReachedByTwoRoutesTests.cs @@ -223,7 +223,8 @@ public void Struct_in_a_root_linked_list_is_reported_once() Assert.Equal(ResultText.Expect("[0].Text" + TextRequired), errors); } - // Spec. Before, the Values property gave a second route: Value[0].Values[0].Text. + // Spec. Before, the Values property gave a second route: Value[0].Values[0].Text. The + // value is reported by its key. [Fact] public void Struct_value_in_a_dictionary_item_is_reported_once() { @@ -232,7 +233,7 @@ public void Struct_value_in_a_dictionary_item_is_reported_once() var (valid, errors) = Run(new Holder>> { Value = new List> { map } }); Assert.False(valid); - Assert.Equal(ResultText.Expect("Value[0][0].Value.Text" + TextRequired), errors); + Assert.Equal(ResultText.Expect("Value[0][a].Text" + TextRequired), errors); } } } diff --git a/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs index 3a59388..69942b4 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs @@ -33,6 +33,7 @@ namespace RecursiveDataAnnotationsValidation.Tests /// - The order of the results, and the path of an object that two routes reach, which 3.0 /// changed with its breadth-first walk (BreadthFirstWalkTests). /// - A struct held in a property, which 3.0 walks (OddShapeTests.StructProperties). + /// - A dictionary value, which 3.0 reports by its key (DictionaryKeyPathTests). /// /// The messages are the framework's default English messages, or a custom ErrorMessage where /// .NET Framework and .NET might word a default differently. @@ -890,22 +891,6 @@ public void Stack_index_counts_from_the_top() Assert.Equal(ResultText.Expect("Items[2].Name | The Name field is required."), results); } - /// - /// A dictionary is a collection of KeyValuePair items, so a value is reached through - /// ".Value", and the index is the position in enumeration order, not the key. A - /// SortedDictionary enumerates in key order. - /// See: https://learn.microsoft.com/dotnet/api/system.collections.generic.keyvaluepair-2 - /// See: https://learn.microsoft.com/dotnet/api/system.collections.generic.sorteddictionary-2 - /// - [Fact] - public void Dictionary_value_is_reached_through_Value() - { - var map = new SortedDictionary { ["b"] = Bad(), ["a"] = Good(), ["c"] = Good() }; - - Assert.False(Validate(new Holder { Items = map }, out var results)); - Assert.Equal(ResultText.Expect("Items[1].Value.Name | The Name field is required."), results); - } - [Fact] public void Tuple_items_are_reached_through_their_properties() { diff --git a/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs index 36ea2a4..1fbfafe 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs @@ -691,8 +691,8 @@ public void Property_hidden_with_new_does_not_throw() /// Rejected fix: skipping every type in a System.* or Microsoft.* namespace. User objects /// inside framework wrappers are validated, and that rule would silently stop it. /// A Tuple<Child, int> property reports "Pair.Item1.Name". A Dictionary<string, Child> - /// reports "Map[0].Value.Name", because each item is a boxed KeyValuePair, which lives in - /// System.Collections.Generic. + /// reports "Map[a].Name": each item is a boxed KeyValuePair, which lives in + /// System.Collections.Generic, and its value is reported by its key. /// Fix: a narrow deny list (IsUnsafeToWalk). The walk skips each property declared by a /// denied type, or by a framework type derived from one, such as Type.DeclaringMethod or /// Uri.Segments. The object itself is still validated, and a user's subclass still has the @@ -1042,7 +1042,7 @@ public class DictionaryHolder public Dictionary Map { get; set; } } - // Guard. Each dictionary item is a boxed KeyValuePair, and its Value is still walked. + // Guard. Each dictionary item is a boxed KeyValuePair, and its value is still validated. [Fact] public void Dictionary_of_objects_is_still_validated() { @@ -1052,7 +1052,7 @@ public void Dictionary_of_objects_is_still_validated() var valid = new RecursiveDataAnnotationValidator().TryValidateObjectRecursive(model, results); Assert.False(valid); - Assert.Contains(results, r => r.MemberNames.Contains("Map[0].Value.Name")); + Assert.Contains(results, r => r.MemberNames.Contains("Map[a].Name")); } // Guard. Items of a reference type can carry attributes, so they are still validated. From 6dd47e5dc0157a3516ab4d73032f7f6d16a845d7 Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 18:59:12 -0400 Subject: [PATCH 2/3] fix: report a dictionary value by its key A value of a dictionary (IDictionary, IDictionary<,> or IReadOnlyDictionary<,>) is now reported by its key, as MVC model binding names it: Map[Primary].Name instead of Map[0].Value.Name. The KeyValuePair is no longer a path step, so the value is one level below the dictionary. A key names its value when it is a string, primitive, enum or a self-formatting struct with nothing to validate. It is formatted with the invariant culture and not escaped. Any other key keeps the index form for that entry, so the key object is still validated. On a second enumeration, the struct check looks at what is queued: the value for a key-form entry, and the key and value for an index-form entry. A dictionary of class values no longer skips an entry that was added in the meantime. PathStep gains a key step, and Enqueue takes a ready-made step. Removes the NOTE in EnqueueItems. Co-Authored-By: Claude Opus 5.5 --- .../Extensions/TypeExtensions.cs | 72 ++++++++++++++++++ .../RecursiveDataAnnotationValidator.cs | 74 +++++++++++++++---- 2 files changed, 132 insertions(+), 14 deletions(-) diff --git a/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs b/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs index a58531e..1104d0b 100644 --- a/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs +++ b/src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs @@ -5,6 +5,7 @@ using System.ComponentModel; using System.ComponentModel.DataAnnotations; using System.Diagnostics; +using System.Globalization; using System.IO; using System.Linq; using System.Reflection; @@ -38,6 +39,12 @@ internal static class TypeExtensions private static readonly ConcurrentDictionary UnsafeToWalkCache = new ConcurrentDictionary(); + private static readonly ConcurrentDictionary DictionaryTypeCache = + new ConcurrentDictionary(); + + private static readonly ConcurrentDictionary KeyValuePairProperties = + new ConcurrentDictionary(); + // Framework types whose properties throw, wait, or never end, when the validator walks them. // They carry no validation attributes. Only the properties these types and their framework // subclasses declare are skipped, so a user subclass still has its own properties walked. @@ -169,6 +176,71 @@ public static List GetWalkedProperties(this Type type) .ToList(); } + /// + /// True for a dictionary: a type that implements IDictionary, IDictionary<TKey, TValue> or + /// IReadOnlyDictionary<TKey, TValue>. The validator reports a value of a dictionary by its key. + /// A list of KeyValuePair items is not a dictionary. + /// + public static bool IsDictionaryType(this Type type) + { + return DictionaryTypeCache.GetOrAdd(type, t => + typeof(IDictionary).IsAssignableFrom(t) + || t.GetInterfaces().Any(i => i.IsGenericType + && (i.GetGenericTypeDefinition() == typeof(IDictionary<,>) + || i.GetGenericTypeDefinition() == typeof(IReadOnlyDictionary<,>)))); + } + + /// + /// Splits an entry that a dictionary yields, a KeyValuePair<TKey, TValue> or a + /// DictionaryEntry, into its key and value. False for any other item. + /// + public static bool TryGetDictionaryEntry(this object item, out object key, out object value) + { + if (item is DictionaryEntry entry) + { + key = entry.Key; + value = entry.Value; + return true; + } + + var type = item.GetType(); + if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(KeyValuePair<,>)) + { + var properties = KeyValuePairProperties.GetOrAdd(type, t => (t.GetProperty("Key"), t.GetProperty("Value"))); + key = properties.Key.GetValue(item, null); + value = properties.Value.GetValue(item, null); + return true; + } + + key = null; + value = null; + return false; + } + + /// + /// The text that names a dictionary value in a path, such as "Primary" in Map[Primary].Name, + /// or null for a key that cannot name it. A key can when it has nothing to validate (see + /// ) and is a string, a primitive, an enum, or a struct that formats + /// itself, such as a decimal, a Guid or a DateTime. The text uses the invariant culture, so a + /// path does not change with the culture of the thread. It is not escaped, as in the names + /// that MVC model binding uses for a dictionary, so a key that contains "]" or "." can make a + /// path ambiguous. + /// + public static string GetKeyText(this object key) + { + var type = key.GetType(); + var isSimple = type == typeof(string) + || type.IsPrimitive + || type.IsEnum + || (type.IsValueType && key is IFormattable); + + if (!isSimple || !type.IsLeafType()) return null; + + return key is IFormattable formattable + ? formattable.ToString(null, CultureInfo.InvariantCulture) + : key.ToString(); + } + /// /// True when the type's Equals(object) is not object.Equals, so two different objects /// of the type can be equal. Records, structs (through ValueType.Equals) and classes diff --git a/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs b/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs index d2f7c82..b5708ac 100644 --- a/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs +++ b/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs @@ -197,14 +197,11 @@ public bool Run(object root) return valid; } - //Queues an object that was found at the end of a path. The path is the parent path plus a - //property name, or plus an index if the name is null. A path is a chain of steps, and it is - //turned into a string only for an object that has a result. + //Queues an object that was found at the end of a path. A path is a chain of steps, and it + //is turned into a string only for an object that has a result. private void Enqueue( object value, - PathStep parent, - string propertyName, - int index, + PathStep path, int depth, EqualsAncestor ancestors ) @@ -227,7 +224,7 @@ EqualsAncestor ancestors queue.Enqueue(new WorkItem( value, - new PathStep(parent, propertyName, index), + path, depth, ancestors, false, @@ -246,20 +243,47 @@ EqualsAncestor ancestors ) { var enumeratedBefore = !enumeratedCollections.Add(items); + var isDictionary = items.GetType().IsDictionaryType(); var arrayIndex = -1; foreach (var item in items) { arrayIndex++; + if (item == null) continue; - //NOTE: Possibly should have a separate case for Dictionary which reports on the key + //A value of a dictionary is reported by its key, as in Map[Primary].Name, when the + //key can name it (see GetKeyText). Otherwise the whole entry is an item, as in any + //collection, so a key that is an object is validated too: Map[0].Key.Name. + var queued = item; + PathStep path = null; + if (isDictionary && item.TryGetDictionaryEntry(out var key, out var value) && key != null) + { + var keyText = key.GetKeyText(); + if (keyText != null) + { + if (value == null) continue; + queued = value; + path = PathStep.ForKey(collectionPath, keyText); + } + } - if (item == null) continue; - if (enumeratedBefore && item.GetType().IsValueType) continue; - Enqueue(item, collectionPath, null, arrayIndex, itemDepth, ancestors); + if (enumeratedBefore && HasNoIdentity(queued)) continue; + Enqueue(queued, path ?? new PathStep(collectionPath, null, arrayIndex), itemDepth, ancestors); } } + //True for a struct, which queuedObjects cannot match by reference (see enumeratedCollections). + //A dictionary entry is a struct too, but a key or value of a class in it has an identity. + private static bool HasNoIdentity(object item) + { + if (!item.GetType().IsValueType) return false; + + return !item.TryGetDictionaryEntry(out var key, out var value) + || !(IsOfAClass(key) || IsOfAClass(value)); + } + + private static bool IsOfAClass(object value) => value != null && !value.GetType().IsValueType; + //Walks an object: validates it, then queues what it holds. Returns false if the object has a result. private bool Walk(WorkItem item) { @@ -353,7 +377,7 @@ private bool Walk(WorkItem item) break; default: - Enqueue(value, item.Path, property.Name, -1, item.Depth + 1, ancestors); + Enqueue(value, new PathStep(item.Path, property.Name, -1), item.Depth + 1, ancestors); break; } } @@ -481,6 +505,9 @@ private sealed class PathStep private readonly string propertyName; private readonly int index; + //the text of a dictionary key, which takes the place of the index + private readonly string key; + public PathStep(PathStep parent, string propertyName, int index) { this.parent = parent; @@ -488,7 +515,16 @@ public PathStep(PathStep parent, string propertyName, int index) this.index = index; } - //"Orders[0].Lines[1].Product": a name after the first one is joined with a dot, and an index with none + private PathStep(PathStep parent, string key) + { + this.parent = parent; + this.key = key; + } + + public static PathStep ForKey(PathStep parent, string key) => new PathStep(parent, key); + + //"Orders[0].Lines[1].Product", "Endpoints[Primary].Url": a name after the first one is + //joined with a dot, and an index or a key with none public override string ToString() { var steps = new List(); @@ -503,7 +539,17 @@ public override string ToString() var step = steps[i]; if (step.propertyName == null) { - path.Append('[').Append(step.index).Append(']'); + path.Append('['); + if (step.key != null) + { + path.Append(step.key); + } + else + { + path.Append(step.index); + } + + path.Append(']'); continue; } From 692c7fc9634af279be319eb2fff49f89eed508c4 Mon Sep 17 00:00:00 2001 From: Thomas Harold Date: Fri, 2 Oct 2026 19:31:15 -0400 Subject: [PATCH 3/3] docs: describe dictionary values reported by key README: a Dictionaries section with the key form, which keys name a value, the invariant culture, the unescaped key, the index form for other keys, and the level of a value. Changelog: a BREAKING entry for the key form, and the dictionary item path in the earlier collection entry now shows the 3.0 form. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 ++- README.md | 10 +++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62e7d05..1bc89da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ Breaking changes are marked **BREAKING**. - **BREAKING** The member names of an object in an item that is a collection. The validator now enumerates the items first, so the path no longer goes through a property of the item. Change any code that matches these names: - Array, multi-dimensional array, `ArrayList`: `Value[0].SyncRoot[0].Name` is now `Value[0][0].Name`. - - Dictionary, `Hashtable`: `Value[0].Values[0].Name` is now `Value[0][0].Value.Name`, and `Value[0].Keys[0].Name` is now `Value[0][0].Key.Name`. + - Dictionary, `Hashtable`: `Value[0].Values[0].Name` is now `Value[0][a].Name` for the key `a` (see the dictionary entry below), and `Value[0].Keys[0].Name` is now `Value[0][0].Key.Name` for a key that is an object. - `LinkedList`: `Value[0].First.List[1].Name` is now `Value[0][1].Name`. - `SortedSet`: `Value[0].Min.Name` is now `Value[0][0].Name`. - A collection of your own with a property that returns its items, such as `View`: `Value[0].View[0].Name` is now `Value[0][0].Name`. @@ -37,6 +37,7 @@ Breaking changes are marked **BREAKING**. - The NuGet package title now reads "Recursive DataAnnotations Validation". It was misspelled "Recurisive". - The NuGet package now has a description. It showed "Package Description" before. - **BREAKING** A struct that the validator reaches by two routes is reported once where the validator can tell. For a collection that is an item or the root object, the properties that framework types declare, such as `ArraySegment.Array`, `LinkedList.First` and `Dictionary.Values`, are no longer walked, as for a collection that a property holds. A collection that a second route reaches is enumerated again, so an item added in the meantime is found, but its struct items are not reported again. Change any code that counts the results: `Value[0].Array[0].Text`, `Value[0].First.Value.Text` and `Value[0].Values[0].Text` are gone, and so is `Second[0].Text` for an array that `First` also holds. An object that only such a property reached is no longer validated, such as an item of the array outside an `ArraySegment`, or the comparer of a dictionary. A jagged array of simple values, such as `int[][]`, is no longer enumerated, because it cannot hold an invalid object. +- **BREAKING** A value of a dictionary is reported by its key, in the form that MVC model binding uses: `Map[0].Value.Name` is now `Map[Primary].Name`, and `[0].Value.Name` for a root dictionary is now `[Primary].Name`. This holds for any type that implements `IDictionary`, `IDictionary` or `IReadOnlyDictionary`, when the key is a string, a primitive, an enum or a struct that formats itself, such as a `decimal` or a `Guid`, and has nothing to validate. The key is formatted with the invariant culture and is not escaped. Any other key, such as an object, keeps the old form for that entry: `Map[0].Key.Name` and `Map[0].Value.Name`. A value is now one level below the dictionary, as an item of a list is, so it can come earlier in the results and counts one level less for the maximum depth. Change any code that matches these names. - Known limit: two properties that each return the same struct, such as a property and a computed copy of it, report it twice, with two member names. The validator cannot match two copies of a struct by reference. Objects of a class are reported once. - README: say what `Validator.TryValidateObject` leaves out, show the usage as C#, and tell the `Tuple` classes, which are walked, from value tuples, which are not. diff --git a/README.md b/README.md index 1fce1c6..b78dd6a 100644 --- a/README.md +++ b/README.md @@ -59,13 +59,21 @@ An object that several paths reach is validated once, and its errors name the sh ### Collections - The items of a collection that a property holds are validated. The error names the property and the index of the item: `Items[1].Name`. -- An item that is itself a collection is validated too, at each level: `Matrix[0][2].Name`. This holds for lists, arrays, sets, dictionaries and your own collection types. A dictionary is enumerated as `KeyValuePair` items, so a value that is an object is reported as `Map[0].Value.Name`. A value that is a struct is walked when it has something to validate, as it is when a property holds it (see "Structs" below). +- An item that is itself a collection is validated too, at each level: `Matrix[0][2].Name`. This holds for lists, arrays, sets, dictionaries and your own collection types. A value of a dictionary is reported by its key (see "Dictionaries" below). A value that is a struct is walked when it has something to validate, as it is when a property holds it (see "Structs" below). - An item that is a collection is validated as an object first, so its own attributes and `IValidatableObject.Validate` run, then its items. The properties that a framework type declares on it, such as `ArraySegment.Array`, `LinkedList.First` or `Dictionary.Values`, are not walked, because they repeat the items. A collection that a property holds is treated the same way. The properties that your own collection type adds are walked. - A collection of simple values, such as `List` or `string[]`, is not enumerated, because it cannot hold an invalid object. - A collection that you pass to the validator as the root object is validated as an object, then its items are validated. The error starts with the index of the item: `[1].Name`. - A collection that is a struct, such as `ImmutableArray`, is validated when a property holds it and when it is an item of another collection: `Lines[0].Sku`. - The validator runs each collection it enumerates, so a lazy sequence, such as a LINQ query or an iterator, runs during validation. A sequence that never ends makes validation hang. The validator does not catch exceptions, so an exception that a collection throws when it is enumerated reaches your code. A struct collection that is its default value, such as an `ImmutableArray` or an `ArraySegment` that nobody set, is skipped when it is an item or when the property is declared as that struct, because it holds nothing and enumerating it throws. The validator compares the memory of the struct with its default. It does not call the `Equals` of your struct. A property declared as an interface or as `object` is always enumerated. +### Dictionaries + +- A value of a dictionary is reported by its key: `Endpoints[Primary].Url`, `Ports[8080].Name`, `ByDay[Monday].Name`. This is the form that MVC model binding uses for a dictionary. A dictionary is a type that implements `IDictionary`, `IDictionary` or `IReadOnlyDictionary`. A list of `KeyValuePair` items is not one, so it keeps `Pairs[0].Value.Name`. +- A key names its value when it is a string, a number or another primitive, an enum, or a struct that formats itself, such as a `decimal`, a `Guid` or a `DateTime`, and it has nothing to validate. The key is formatted with the invariant culture, so a `decimal` key of 1.5 is `Map[1.5]` on every server. +- The key is not quoted or escaped. A key that contains `]` or `.`, such as `b.c[0]`, gives a path that you cannot tell apart from a deeper one: `Map[b.c[0]].Name`. +- Any other key, such as an object of your own, keeps the position of the entry, so the key is validated too: `Map[0].Key.Name` and `Map[0].Value.Name`. The position is the order in which the dictionary enumerates its entries. Each entry chooses its own form. +- A value is one level below the dictionary, as an item of a list is, for the maximum depth and the order of results. + ### Structs - A struct that a property holds is walked when it has something to validate: a validation attribute on the struct or on one of its properties, `IValidatableObject`, or a property that holds an object, such as the `Value` of a `KeyValuePair`. The error names the path, as for a class: `Price.Amount`, `Pair.Value.Name`.