diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fa72bc..e5b2569 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ Breaking changes are marked **BREAKING**. ### Added -- **BREAKING** A maximum depth of 128 levels. An object at a path of more than 128 segments is not validated, and the validation fails with one error at that path: `The object is nested more than 128 levels deep and was not validated.` Each property step and each collection index counts as one level, so a tree that holds its children in a `List` can be 64 levels deep. Before, the validator had no limit, and a graph of about 1,500 objects, or a property that builds a new object on each read, overflowed the stack, which ends the process. A graph that is deeper than 128 levels passed before and fails now. Change any model that is that deep, or mark the property that leads into it with `[SkipRecursiveValidation]`. The limit is not a setting. +- **BREAKING** A maximum depth of 128 levels. An object at a path of more than 128 segments is not validated, and the validation fails with one error at that path: `The object is nested more than 128 levels deep and was not validated.` Each property step and each collection index counts as one level, so a tree that holds its children in a `List` can be 64 levels deep. The depth of an object is the length of the shortest path to it, so a link back to a parent does not make it deeper. Before, the validator had no limit, and a graph of about 1,500 objects, or a property that builds a new object on each read, overflowed the stack, which ends the process. The validator now keeps its work in a queue, so the depth of a graph no longer uses the stack. A graph that is deeper than 128 levels passed before and fails now. Change any model that is that deep, or mark the property that leads into it with `[SkipRecursiveValidation]`. The limit is not a setting. ### Fixed @@ -28,7 +28,12 @@ Breaking changes are marked **BREAKING**. - `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`. - **BREAKING** An item that is a collection is now enumerated, so a lazy sequence in an item runs, as it does when a property holds it. An item that throws when enumerated now throws from validation, and an item that never ends makes validation hang. The same holds for a struct collection that a property holds, such as `ImmutableArray`: one that throws when enumerated now throws from validation, and one that builds new objects on each read stops at the maximum depth and fails the validation. To avoid it, mark the property that holds the collection with `[SkipRecursiveValidation]`. -- A deeply nested collection uses more of the stack. On a 1 MB stack, a chain of `List` that holds the next list passes at 1,100 levels and overflows at 1,200, which ends the process (net8.0, macOS, Release). Before, such a list passed without being enumerated. The maximum depth of 128 now stops the walk long before the stack is full. +- **BREAKING** The validator walks the graph breadth first, not depth first. Change any code that depends on the order of the results or on the member name of an object that two paths reach: + - The results come shallowest first. The errors of one object stay together and in the same order. An item of a collection that a property holds is two levels below the object, so for a model with `First`, `Items` and `Last`, the order was `First`, `Items[0]`, `Items[1]`, `Last` and is now `First`, `Last`, `Items[0]`, `Items[1]`. Sort the list if you need a fixed order. + - An object that two paths reach is reported with the shortest path. Before, it was the first path in property order. If two paths are equally short, the first property still wins. In a graph with links back to parents, such as the entities of an ORM, the member names are now much shorter. For example, in a store where each order shares a product with the next order, the second order in `Store.Orders` was reported as `Orders[0].Lines[1].Product.Lines[1].Order.Code` when a longer path reached it first, and is now `Orders[1].Code`. + - The maximum depth checks the shortest path to an object. An object that a path of more than 128 levels reaches, and a path of 128 levels or fewer also reaches, is validated, and gets no depth error. An object that every path reaches deeper than 128 levels gets one error, and not one for each path. + - An object that `Equals` an ancestor on its shortest path is still walked when a longer path reaches it without an equal ancestor. Before, the first path in property order decided, so a model passed or failed by the order of its properties. Known limit: an object that every path reaches through an equal ancestor is not walked. This happens when a shared object, such as a customer, is reached first through the equal ancestor. + - A collection that a property holds is enumerated later than before: after the objects at its own level and above are validated, not when the property is read. A lazy sequence runs later. A change that a `Validate` method makes to the collection is now seen. An item whose `Validate` adds to its own list no longer throws `Collection was modified`. When a collection throws, the exception is the same, but the result list holds different partial results. - The NuGet package title now reads "Recursive DataAnnotations Validation". It was misspelled "Recurisive". - Known limit: an invalid struct that an item collection returns by enumeration and also through one of its properties is reported twice, with two member names, for example `Value[0][0].Text` and `Value[0].Array[0].Text` for an `ArraySegment`. The validator cannot match two copies of a struct by reference. A struct that two properties hold was already reported twice. Objects of a class are reported once. diff --git a/README.md b/README.md index 34ac433..f40684c 100644 --- a/README.md +++ b/README.md @@ -28,22 +28,28 @@ The [`[SkipRecursiveValidation]`](https://github.com/tgharold/RecursiveDataAnnot ### Shared objects, cycles and computed properties -- Each object is validated once, even when several properties point to it. This also stops cycles, such as a child that points back to its parent. +- Each object is validated once, even when several properties point to it. This also stops cycles, such as a child that points back to its parent. The error names the shortest path to the object (see "Order of results" below). - A struct is the exception to the first rule. The validator cannot tell that two copies of a struct are the same value, so it reports an invalid struct once for each route that reaches it. For example, two properties that hold the same array of structs report each struct twice, and so does a `List` that holds an `ArraySegment` of structs, because the segment returns its items by enumeration and through its `Array` property. The error is not lost, but the result list has a duplicate with a different member name. For a collection type of your own, mark the property that repeats the items with `[SkipRecursiveValidation]`. - Objects are compared by reference. Two separate objects that are `Equals` to each other, such as records with the same values or entities with the same `Id`, are each validated. - Public static properties are walked, as well as instance properties. -- A property that builds a new object on each read, such as `public Money Zero => new Money(0)`, could make the walk go on forever. So when an object's type overrides `Equals`, and the object equals an object of the same type, a base type or a derived type on its own path from the root, the validator checks that object's own attributes but does not walk into its properties. An invalid object below it is not reached. +- A property that builds a new object on each read, such as `public Money Zero => new Money(0)`, could make the walk go on forever. So when an object's type overrides `Equals`, and the object equals an object of the same type, a base type or a derived type on its own path from the root, the validator checks that object's own attributes but does not walk into its properties from there. The check uses the shortest path to the object. A longer path that does not pass an equal object walks into it. An invalid object that every path reaches through an equal ancestor is not reached. This can happen when a shared object, such as a customer, is reached first through an equal ancestor. - For a type that does not override `Equals`, mark such a property with `[SkipRecursiveValidation]`. Otherwise the walk goes on until it reaches the maximum depth (see below), and the validation fails. - Two records that reference each other can also overflow the stack. A record's generated `Equals` compares properties in declaration order, so it follows the reference forever if it reaches it before a property that differs. This happens when the records have equal values, or when the reference is declared first. On .NET Framework it always happens, because the framework's `Validator` calls the record's generated `GetHashCode`, which follows the reference too. Marking the reference with `[SkipRecursiveValidation]` avoids the walk, but not the `GetHashCode` call on .NET Framework. There, override `GetHashCode` so it does not include the reference. ### Maximum depth -The validator walks at most 128 levels. The depth of an object is the number of segments in its path: each property and each collection index is one level, and the root object is level 0. In `Items[1].Name`, `Items` is level 1, `[1]` is level 2 and `Name` is level 3. System.Text.Json counts nearly the same way, with each object and each array as one level, and its default limit is 64. A JSON document within that limit stays within about 63 levels here. +The validator walks at most 128 levels. The depth of an object is the number of segments in the shortest path to it: each property and each collection index is one level, and the root object is level 0. A link back to a parent, or a second path to a shared object, does not make an object deeper. In `Items[1].Name`, `Items` is level 1, `[1]` is level 2 and `Name` is level 3. System.Text.Json counts nearly the same way, with each object and each array as one level, and its default limit is 64. A JSON document within that limit stays within about 63 levels here. -An object at level 128 is validated. An object at level 129 is not validated, and the validation fails with one error at its path: `The object is nested more than 128 levels deep and was not validated.` The walk does not go deeper below that object. A graph that is too deep means that something has gone wrong, such as a property that builds a new object on each read, so the validator fails the validation instead of letting the graph pass. It does not throw. +An object at level 128 is validated. An object at level 129 is not validated, and the validation fails with one error at its path: `The object is nested more than 128 levels deep and was not validated.` The walk does not go deeper below that object. A graph that is too deep means that something has gone wrong, such as a property that builds a new object on each read, so the validator fails the validation instead of letting the graph pass. It does not throw. The limit does not help a property that builds two or more new objects on each read. The number of objects doubles at each level, so the walk runs until memory runs out, long before level 128. Mark such a property with `[SkipRecursiveValidation]`. A tree that holds its children in a `List` uses two levels for each tree level, so it can be 64 levels deep. The limit is fixed. +### Order of results + +The validator walks the graph one level at a time, from the root outward. The results come in that order: the root object's own errors first, then the errors of the objects one level below it, then two levels below it, and so on. The errors of one object stay together, in the order that `Validator.TryValidateObject` returns them. Objects on the same level come in property order. When an item of a collection is itself a collection, its own items come before the objects that its properties hold. An item of a collection that a property holds is two levels below that object, one for the property and one for the index. So with the properties `First`, `Items` and `Last`, the error of `Last` comes before the error of `Items[0]`. Do not rely on the order of the list. Sort it if you need a fixed order. + +An object that several paths reach is validated once, and its errors name the shortest path. If two paths are equally short, the one through the property that comes first wins. This matters most for a model with links back to its parents, such as the entities of an ORM. An order that is in `Store.Orders` is reported as `Orders[0].Code`, not as a long chain of links through its lines and products. + ### 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`. diff --git a/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs b/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs index eda7f25..7ba32d6 100644 --- a/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs +++ b/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Linq; +using System.Text; using System.Threading.Tasks; using RecursiveDataAnnotationsValidation.Extensions; @@ -12,9 +13,10 @@ namespace RecursiveDataAnnotationsValidation public class RecursiveDataAnnotationValidator : IRecursiveDataAnnotationValidator, IAsyncRecursiveDataAnnotationValidator { //The deepest level that is validated. The depth of an object is the number of segments in its - //path: each property step and each collection index is one level, and the root is level 0. - //An object at a deeper level is not validated and fails the validation (see DepthExceededResult). - //The walk calls itself once for each level, and a StackOverflowException cannot be caught. + //shortest path: each property step and each collection index is one level, and the root is level 0. + //An object at a deeper level is not validated and fails the validation (see GraphWalk.Walk). + //The limit stops a computed property that builds one new object on each read, which never ends + //otherwise. It does not stop one that builds two or more, because the walk runs out of memory first. //128 is twice the depth that System.Text.Json allows by default, which counts nearly the same way. internal const int MaxDepth = 128; @@ -111,7 +113,7 @@ public async Task TryValidateObjectRecursiveAsync( //serviceProvider is the caller's ValidationContext, or null when the caller passed only items. //Validator passes the outer context as the service provider of each context it builds, so //GetService on a context built here reaches the caller's provider the same way. - private bool TryValidateGraph( + private static bool TryValidateGraph( object obj, List validationResults, IServiceProvider serviceProvider, @@ -121,311 +123,400 @@ IDictionary validationContextItems //like Validator.TryValidateObject, a null list means the caller wants only the return value validationResults = validationResults ?? new List(); - return TryValidateObjectRecursive( - obj, - validationResults, - new HashSet(ObjectReferenceComparer.Instance), - new List(), - serviceProvider, - validationContextItems, - 0 - ); + return new GraphWalk(validationResults, serviceProvider, validationContextItems).Run(obj); } - /// - /// Validates the specified object and adds any validation results to the provided collection. - /// - /// The object to validate. - /// A collection to receive any validation errors. - /// Service provider for the validation context, or null. - /// Optional context items for the validation context. - /// True if the object is valid; otherwise, false. - private bool TryValidateObject( - object obj, - ICollection validationResults, - IServiceProvider serviceProvider, - IDictionary validationContextItems - ) + //One walk over an object graph, breadth first. The walk keeps its work in a queue and calls + //nothing recursively, so the depth of a graph does not use the stack. + // + //A level is one segment of a path: a property or an index. Each step from an object to the + //next one is one level, so the queue holds the levels in order, and the first path that + //reaches an object is the shortest one. The walk marks an object when it queues it, and + //drops every later path to it. The depth of an object is therefore true. + // + //A property that holds a collection is a step of its own: the collection waits in the queue + //one level below its object, and its items are queued one level below that. Without it, an + //item would be two levels away from its object and the queue would not be in order. + private sealed class GraphWalk { - return Validator.TryValidateObject( - obj, - new ValidationContext( - obj, - serviceProvider, - validationContextItems - ), - validationResults, - true - ); - } + private readonly Queue queue = new Queue(); - //True when obj Equals an object on the path whose type is obj's type, a base of it, or derived - //from it. Related types cover a computed property that alternates between a type and its - //subclass. Unrelated types are not compared, so an Equals that casts without a type check - //does not throw. - private static bool EqualsAnAncestor(object obj, Type type, List equalityPath) - { - foreach (var ancestor in equalityPath) + //every object that was queued, compared by reference + private readonly HashSet queuedObjects = new HashSet(ObjectReferenceComparer.Instance); + + //every object that was validated but not walked, because it Equals an ancestor on its path + private readonly HashSet stoppedObjects = new HashSet(ObjectReferenceComparer.Instance); + + private readonly ICollection validationResults; + private readonly IServiceProvider serviceProvider; + private readonly IDictionary validationContextItems; + + public GraphWalk( + ICollection validationResults, + IServiceProvider serviceProvider, + IDictionary validationContextItems + ) { - var ancestorType = ancestor.GetType(); - if ((ancestorType.IsAssignableFrom(type) || type.IsAssignableFrom(ancestorType)) - && obj.Equals(ancestor)) + this.validationResults = validationResults; + this.serviceProvider = serviceProvider; + this.validationContextItems = validationContextItems; + } + + public bool Run(object root) + { + //an object of a leaf type can never produce a result (see IsLeafType) + if (root.GetType().IsLeafType()) { return true; } - } - return false; - } + queuedObjects.Add(root); + queue.Enqueue(new WorkItem(root, null, 0, null, false, false, false)); - //validatedObjects holds every object visited so far, compared by reference. - //equalityPath holds the objects on the path from the root to this object whose type overrides Equals. - //depth is the number of segments in the path of obj, which is 0 for the root object. - //enumerateItems is true for an item of a collection. If the item is itself a collection, its - //items are validated too. A collection that a property holds is enumerated by the caller, and - //the root object is never enumerated. - private bool TryValidateObjectRecursive( - object obj, - ICollection validationResults, - ISet validatedObjects, - List equalityPath, - IServiceProvider serviceProvider, - IDictionary validationContextItems, - int depth, - bool enumerateItems = false - ) - { - var type = obj.GetType(); + var valid = true; + while (queue.Count > 0) + { + var item = queue.Dequeue(); + if (item.IsCollectionOfProperty) + { + EnqueueItems((IEnumerable)item.Value, item.Path, item.Depth + 1, item.Ancestors); + } + else if (!Walk(item)) + { + valid = false; + } + } - //an object of a leaf type can never produce a result, such as a boxed int in an object[] (see IsLeafType) - if (type.IsLeafType()) - { - return true; + return valid; } - //short-circuit to avoid infinite loops on cyclical object graphs - if (validatedObjects.Contains(obj)) + //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. + private void Enqueue( + object value, + PathStep parent, + string propertyName, + int index, + int depth, + EqualsAncestor ancestors, + bool enumerateItems + ) { - return true; - } + //an object of a leaf type can never produce a result, such as a boxed int in an object[] (see IsLeafType) + //An object that was queued before is skipped, which also ends a cycle in the graph. + var type = value.GetType(); + if (type.IsLeafType() || queuedObjects.Contains(value)) + { + return; + } - //a computed property can return a new, equal object on each read, such as - //`Money Zero => new Money(0)`, so references never repeat and the walk would not end. - //Stop at an object that Equals an object on its own path: validate its own attributes, - //so a child that Equals its parent by Id is still checked, but don't walk into it. - var overridesEquals = type.OverridesEquals(); - if (overridesEquals && EqualsAnAncestor(obj, type, equalityPath)) - { - validatedObjects.Add(obj); - return TryValidateObject(obj, validationResults, serviceProvider, validationContextItems); + //An object that Equals an ancestor on this path is validated, but not walked from here. + //It is not marked, so a later path that does not pass an equal ancestor can still walk it. + var stopHere = type.OverridesEquals() && EqualsAnAncestor(value, type, ancestors); + if (!stopHere) + { + queuedObjects.Add(value); + } + + queue.Enqueue(new WorkItem( + value, + new PathStep(parent, propertyName, index), + depth, + ancestors, + enumerateItems, + false, + stopHere + )); } - //an object this deep is not validated and not walked, and the validation fails: nobody - //has checked it, so it must not pass. A leaf, an object seen before and an object that - //Equals an ancestor all returned above, because none of them walks any further. - if (depth > MaxDepth) + //Queues each item of a collection. The path of an item is the path of the collection + //plus its index, and its depth is itemDepth. + private void EnqueueItems( + IEnumerable items, + PathStep collectionPath, + int itemDepth, + EqualsAncestor ancestors + ) { - validationResults.Add(new DepthExceededResult()); - return false; + var arrayIndex = -1; + foreach (var item in items) + { + arrayIndex++; + + //NOTE: Possibly should have a separate case for Dictionary which reports on the key + + if (item == null) continue; + Enqueue(item, collectionPath, null, arrayIndex, itemDepth, ancestors, true); + } } - validatedObjects.Add(obj); - if (overridesEquals) equalityPath.Add(obj); - - var result = TryValidateObject(obj, validationResults, serviceProvider, validationContextItems); - - //An item that is a collection is validated as an object above, so its own attributes run. - //Then its items are validated, before its properties are walked. An object that the - //collection also returns from a property, such as Array.SyncRoot, is then reported at its - //index (Value[0][0]) and not through the property (Value[0].SyncRoot[0]). - //A collection of leaf types is skipped, like a collection that a property holds. A default - //struct, such as an ImmutableArray nobody set, is skipped: it holds nothing and enumerating it throws. - var enumeratedItems = false; - if (enumerateItems - && obj is IEnumerable items - && !type.IsCollectionOfLeafType() - && !obj.IsDefaultStruct()) + //Walks an object: validates it, then queues what it holds. Returns false if the object has a result. + private bool Walk(WorkItem item) { - enumeratedItems = true; - if (!TryValidateItems( - items, - "", - validationResults, - validatedObjects, - equalityPath, - serviceProvider, - validationContextItems, - depth - )) + var obj = item.Value; + var type = obj.GetType(); + + //a computed property can return a new, equal object on each read, such as + //`Money Zero => new Money(0)`, so references never repeat and the walk would not end. + //Stop at an object that Equals an object on its own path: validate its own attributes, + //so a child that Equals its parent by Id is still checked, but don't walk into it. + //(the check runs when the object is queued, see Enqueue) + if (item.EqualsAnAncestor) { - result = false; + //Validated once, however many paths stop at it. A path that walks it was queued + //after this one (Enqueue drops a stop for an object that is queued), so it comes out + //of the queue later, and it skips the validation (see below). + return !stoppedObjects.Add(obj) || Validate(item); } - } - //IsWalked leaves out properties declared by framework types that throw or never end when - //read, such as Uri.Segments on a relative Uri, DirectoryInfo.Root, or the properties of - //a Thread or Process read from the wrong thread or process (see IsUnsafeToWalk) - var properties = type.GetProperties().Where(prop => prop.IsWalked()).ToList(); + var overridesEquals = type.OverridesEquals(); - foreach (var property in properties) - { - var value = property.GetValue(obj, null); + //An object this deep is not validated and not walked, and the validation fails: + //nobody has checked it, so it must not pass. The path to it is the shortest one, + //so the depth is true, and the object gets one error, and not one for each path. + //A leaf and an object queued before never get here (see Enqueue), and an object that + //Equals an ancestor on its path returned above, so none of them gets a depth error. + if (item.Depth > MaxDepth) + { + validationResults.Add(new ValidationResult( + "The object is nested more than " + MaxDepth + " levels deep and was not validated.", + new[] { item.Path.ToString() } + )); + return false; + } - List nestedResults; - switch (value) + var ancestors = overridesEquals ? new EqualsAncestor(obj, item.Ancestors) : item.Ancestors; + + //an object that a shorter path stopped at was validated there, so only walk it now + var result = (stoppedObjects.Count > 0 && stoppedObjects.Contains(obj)) || Validate(item); + + //An item that is a collection is validated as an object above, so its own attributes run. + //Then its items are queued, before its properties. An object that the collection also + //returns from a property, such as Array.SyncRoot, is then reported at its index + //(Value[0][0]) and not through the property (Value[0].SyncRoot[0]). + //A collection of leaf types is skipped, like a collection that a property holds. A default + //struct, such as an ImmutableArray nobody set, is skipped: it holds nothing and enumerating it throws. + var enumeratedItems = false; + if (item.EnumerateItems + && obj is IEnumerable items + && !type.IsCollectionOfLeafType() + && !obj.IsDefaultStruct()) { - case null: - continue; + enumeratedItems = true; + EnqueueItems(items, item.Path, item.Depth + 1, ancestors); + } - //items of a leaf type can never produce a result, so don't enumerate them (see IsLeafType) - case IEnumerable _ when value.GetType().IsCollectionOfLeafType(): - continue; + //IsWalked leaves out properties declared by framework types that throw or never end when + //read, such as Uri.Segments on a relative Uri, DirectoryInfo.Root, or the properties of + //a Thread or Process read from the wrong thread or process (see IsUnsafeToWalk) + var properties = type.GetProperties().Where(prop => prop.IsWalked()).ToList(); - //a struct collection nobody set, such as a default ImmutableArray, holds nothing - //and enumerating it throws, so skip it as an item is skipped. Only for a property - //declared as a struct: a property declared as an interface or object was always - //enumerated, and a boxed struct in one must not start to be skipped. - case IEnumerable _ when property.PropertyType.IsValueType && value.IsDefaultStruct(): - continue; + foreach (var property in properties) + { + var value = property.GetValue(obj, null); - case IEnumerable asEnumerable: - //an item that was enumerated above can return itself from a property, such as - //Array.SyncRoot, and enumerating it a second time would find nothing new - if (enumeratedItems && ReferenceEquals(value, obj)) continue; - - //the property is one level and the index of each item is another - if (!TryValidateItems( - asEnumerable, - property.Name, - validationResults, - validatedObjects, - equalityPath, - serviceProvider, - validationContextItems, - depth + 1 - )) - { - result = false; - } - break; - - default: - nestedResults = new List(); - if (!TryValidateObjectRecursive( - value, - nestedResults, - validatedObjects, - equalityPath, - serviceProvider, - validationContextItems, - depth + 1 - )) - { - result = false; - foreach (var validationResult in nestedResults) - { - var property1 = property; - - //an object that is too deep has no member names, and its path is the property - var memberNames = validationResult is DepthExceededResult - ? new[] { property1.Name } - : validationResult.MemberNames.Select(x => property1.Name + '.' + x); - validationResults.Add(new ValidationResult(validationResult.ErrorMessage, memberNames)); - } - } - break; + switch (value) + { + case null: + continue; + + //items of a leaf type can never produce a result, so don't enumerate them (see IsLeafType) + case IEnumerable _ when value.GetType().IsCollectionOfLeafType(): + continue; + + //a struct collection nobody set, such as a default ImmutableArray, holds nothing + //and enumerating it throws, so skip it as an item is skipped. Only for a property + //declared as a struct: a property declared as an interface or object was always + //enumerated, and a boxed struct in one must not start to be skipped. + case IEnumerable _ when property.PropertyType.IsValueType && value.IsDefaultStruct(): + continue; + + case IEnumerable asEnumerable: + //an item that was enumerated above can return itself from a property, such as + //Array.SyncRoot, and enumerating it a second time would find nothing new + if (enumeratedItems && ReferenceEquals(value, obj)) continue; + + //the property is one level and the index of each item is another + queue.Enqueue(new WorkItem( + asEnumerable, + new PathStep(item.Path, property.Name, -1), + item.Depth + 1, + ancestors, + false, + true, + false + )); + break; + + default: + Enqueue(value, item.Path, property.Name, -1, item.Depth + 1, ancestors, false); + break; + } } + + return result; } - if (overridesEquals) equalityPath.RemoveAt(equalityPath.Count - 1); + //Validates the attributes and IValidatableObject of one object. A result of the root + //object goes to the caller's list as it is. A result of any other object gets its member + //names prefixed by the path of the object, so the names are the full path from the root. + //The root object has no path, so its results keep their member names. A result that has no + //member names, such as an error of the whole object, stays without names. + private bool Validate(WorkItem item) + { + if (item.Path == null) + { + return TryValidateObject(item.Value, validationResults); + } - return result; - } - - //Validates each item of a collection. The result of an item is added to validationResults with - //its member names prefixed by the property name, if there is one, and the index of the item. - //collectionDepth is the depth of the collection, or of the property that holds it, so each - //item is one level deeper. - private bool TryValidateItems( - IEnumerable items, - string propertyName, - ICollection validationResults, - ISet validatedObjects, - List equalityPath, - IServiceProvider serviceProvider, - IDictionary validationContextItems, - int collectionDepth - ) - { - var result = true; - var arrayIndex = -1; - foreach (var item in items) + var results = new List(); + if (TryValidateObject(item.Value, results)) + { + return true; + } + + var path = item.Path.ToString(); + foreach (var validationResult in results) + { + var memberNames = validationResult.MemberNames.Select(x => path + "." + x).ToList(); + validationResults.Add(new ValidationResult(validationResult.ErrorMessage, memberNames)); + } + + return false; + } + + private bool TryValidateObject(object obj, ICollection results) + { + return Validator.TryValidateObject( + obj, + new ValidationContext( + obj, + serviceProvider, + validationContextItems + ), + results, + true + ); + } + + //True when obj Equals an object on the path whose type is obj's type, a base of it, or derived + //from it. Related types cover a computed property that alternates between a type and its + //subclass. Unrelated types are not compared, so an Equals that casts without a type check + //does not throw. + private static bool EqualsAnAncestor(object obj, Type type, EqualsAncestor ancestors) { - arrayIndex++; - - //NOTE: Possibly should have a separate case for Dictionary which reports on the key - - if (item == null) continue; - var nestedResults = new List(); - if (!TryValidateObjectRecursive( - item, - nestedResults, - validatedObjects, - equalityPath, - serviceProvider, - validationContextItems, - collectionDepth + 1, - enumerateItems: true - )) + for (var ancestor = ancestors; ancestor != null; ancestor = ancestor.Parent) { - result = false; - var index = arrayIndex; - foreach (var validationResult in nestedResults) + var ancestorType = ancestor.Value.GetType(); + if ((ancestorType.IsAssignableFrom(type) || type.IsAssignableFrom(ancestorType)) + && obj.Equals(ancestor.Value)) { - //the member names of an item that is a collection start with the index of its own items - var startsWithIndex = validationResult is ItemOfCollectionResult; - //an item that is too deep has no member names, and its path is the index - var memberNames = validationResult is DepthExceededResult - ? new List { propertyName + "[" + index + "]" } - : validationResult.MemberNames - .Select(x => propertyName + "[" + index + "]" + (startsWithIndex ? "" : ".") + x) - .ToList(); - - //With no property name, the result is for an item that is a collection: the - //caller puts its own index in front of these names, with no dot. - validationResults.Add(propertyName.Length == 0 - ? new ItemOfCollectionResult(validationResult.ErrorMessage, memberNames) - : new ValidationResult(validationResult.ErrorMessage, memberNames)); + return true; } } + + return false; + } + } + + //An object that waits in the queue, and how the walk got to it. + //Path is null for the object that the caller passed in. + //IsCollectionOfProperty is true for the collection that a property holds. It is not validated + //as an object. Its items are queued when it comes out of the queue. + //Ancestors are the objects on the path whose type overrides Equals, the nearest one first. + //EnumerateItems is true for an item of a collection. If the item is itself a collection, its + //items are walked too. A collection that a property holds is enumerated by the object that + //holds it, and the root object is never enumerated. + private sealed class WorkItem + { + public WorkItem( + object value, + PathStep path, + int depth, + EqualsAncestor ancestors, + bool enumerateItems, + bool isCollectionOfProperty, + bool equalsAnAncestor + ) + { + EqualsAnAncestor = equalsAnAncestor; + Value = value; + Path = path; + Depth = depth; + Ancestors = ancestors; + EnumerateItems = enumerateItems; + IsCollectionOfProperty = isCollectionOfProperty; } - return result; + public object Value { get; } + public PathStep Path { get; } + public int Depth { get; } + public EqualsAncestor Ancestors { get; } + public bool EnumerateItems { get; } + public bool IsCollectionOfProperty { get; } + public bool EqualsAnAncestor { get; } } - //The result for an object that is deeper than MaxDepth. It has no member names at first. The - //caller that knows how it reached the object gives it that path as its only member name, and - //every caller above it prefixes the name like any other, so the final name is the full path. - private sealed class DepthExceededResult : ValidationResult + //The last segment of a path and a link to the rest of it: a property name, or an index if + //the name is null. Many objects share the first part of their paths, so the string of a + //path is built only when an object has a result. + private sealed class PathStep { - public DepthExceededResult() - : base("The object is nested more than " + MaxDepth + " levels deep and was not validated.") + private readonly PathStep parent; + private readonly string propertyName; + private readonly int index; + + public PathStep(PathStep parent, string propertyName, int index) + { + this.parent = parent; + this.propertyName = propertyName; + this.index = index; + } + + //"Orders[0].Lines[1].Product": a name after the first one is joined with a dot, and an index with none + public override string ToString() { + var steps = new List(); + for (var step = this; step != null; step = step.parent) + { + steps.Add(step); + } + + var path = new StringBuilder(); + for (var i = steps.Count - 1; i >= 0; i--) + { + var step = steps[i]; + if (step.propertyName == null) + { + path.Append('[').Append(step.index).Append(']'); + continue; + } + + if (path.Length > 0) + { + path.Append('.'); + } + + path.Append(step.propertyName); + } + + return path.ToString(); } } - //A result whose member names start with the index of an item, such as "[0].Name", and not - //with a property name. It marks the names that the caller must join without a dot. - //The names the items choose are not looked at, so a name that starts with "[", or a null - //name, is joined like any other name. A result of this type never reaches the caller, - //because the object passed to the validator is not enumerated, and every other result - //is rebuilt with a property name in front. - private sealed class ItemOfCollectionResult : ValidationResult + //A link in the chain of the objects on a path whose type overrides Equals. The chain + //starts at the nearest one, so an object can be compared with its ancestors only. + private sealed class EqualsAncestor { - public ItemOfCollectionResult(string errorMessage, IEnumerable memberNames) - : base(errorMessage, memberNames) + public EqualsAncestor(object value, EqualsAncestor parent) { + Value = value; + Parent = parent; } + + public object Value { get; } + public EqualsAncestor Parent { get; } } /* Note 1: diff --git a/test/RecursiveDataAnnotationsValidation.Tests/BreadthFirstWalkTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/BreadthFirstWalkTests.cs new file mode 100644 index 0000000..53ce810 --- /dev/null +++ b/test/RecursiveDataAnnotationsValidation.Tests/BreadthFirstWalkTests.cs @@ -0,0 +1,991 @@ +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.Linq; +using System.Threading.Tasks; +using Xunit; + +namespace RecursiveDataAnnotationsValidation.Tests +{ + /// + /// The walk visits the shallowest objects first (a breadth-first walk) and not the first route + /// it finds (a depth-first walk). + /// + /// The maximum depth is 128 (see MaxDepthTests). A depth-first walk measures the depth of the + /// route it took, and a graph with links back to its parents has a long route even when it is + /// shallow. An object that was validated by another, shorter route then still failed with + /// "nested more than 128 levels deep", which was false. A breadth-first walk reaches each + /// object by its shortest route, so the depth in that message is always true. + /// + /// Three rules follow: + /// 1. The depth of an object is the length of its shortest path from the object that was passed in. + /// A property is one level and a collection index is one level. + /// 2. An object that can be reached by two routes is reported with its shortest path. If the + /// routes are equally long, the first one wins: the property that comes first, and an item of + /// a collection before a property of the same object. + /// 3. The results are in the order of the walk: shallowest objects first. The results of one + /// object stay together, in the order that the framework's Validator gives them. + /// + /// 4. An object that Equals an ancestor on its shortest path is validated, and not walked, from + /// that path. A longer path that does not pass an equal ancestor still walks it. + /// + /// The tests build graphs that stay within the limit of the walk. A depth-first walk that has + /// the limit of 128, as the walk had before 3.0 added this one, fails them with an error. A + /// depth-first walk with no limit, such as v2.3.3, overflows the stack on + /// Valid_store_of_200_orders_passes, which ends the test run. + /// + public class BreadthFirstWalkTests + { + private const string TooDeep = "The object is nested more than 128 levels deep and was not validated."; + + private const string NameRequired = "Name 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)); + } + + // The results in the order that the validator returned them. ResultText.Describe sorts them. + private static List RunInOrder(object model) + { + var results = new List(); + new RecursiveDataAnnotationValidator().TryValidateObjectRecursive(model, results); + return results.Select(r => $"{string.Join(",", r.MemberNames)} | {r.ErrorMessage}").ToList(); + } + + public class Node + { + public Node Next { get; set; } + + [Required(ErrorMessage = NameRequired)] + public string Name { get; set; } = "ok"; + } + + /// + /// A model of what an ORM such as Entity Framework loads: every order links to its lines, + /// each line links back to its order and to a product, and each product links to the lines + /// that use it. Here each order shares one product with the next order, so the whole store + /// is one connected graph. The graph is shallow, because the store holds the orders directly, + /// and the links back and sideways make a depth-first route as long as the number of orders. + /// + public class EfStore + { + public class Store + { + public List Orders { get; set; } = new List(); + } + + public class Order + { + [Required(ErrorMessage = "Code is required")] + public string Code { get; set; } = "o"; + + public Customer Customer { get; set; } + + public List Lines { get; set; } = new List(); + } + + public class Line + { + [Required(ErrorMessage = "Sku is required")] + public string Sku { get; set; } = "s"; + + public Order Order { get; set; } + + public Product Product { get; set; } + } + + public class Product + { + [Required(ErrorMessage = NameRequired)] + public string Name { get; set; } = "p"; + + public List Lines { get; set; } = new List(); + } + + public class Customer + { + [Required(ErrorMessage = NameRequired)] + public string Name { get; set; } = "c"; + + public List Orders { get; set; } = new List(); + } + + // Order i has two lines, for product i and product i + 1. + private static Store Build(int orders) + { + var store = new Store(); + var products = Enumerable.Range(0, orders + 1).Select(_ => new Product()).ToList(); + for (var i = 0; i < orders; i++) + { + var customer = new Customer(); + var order = new Order { Customer = customer }; + customer.Orders.Add(order); + foreach (var product in new[] { products[i], products[i + 1] }) + { + var line = new Line { Order = order, Product = product }; + order.Lines.Add(line); + product.Lines.Add(line); + } + + store.Orders.Add(order); + } + + return store; + } + + /// + /// With 25 orders, the depth-first route is about 150 levels long, and two objects + /// failed with "nested more than 128 levels deep". Each of them was in fact validated + /// by another route, so the message was false and the valid store failed. + /// + [Fact] + public void Valid_store_of_25_orders_passes() + { + var (valid, errors) = Run(Build(25)); + + Assert.True(valid); + Assert.Empty(errors); + } + + [Fact] + public async Task Valid_store_of_25_orders_passes_in_the_async_method() + { + var results = new List(); + + var valid = await new RecursiveDataAnnotationValidator() + .TryValidateObjectRecursiveAsync(Build(25), results); + + Assert.True(valid); + Assert.Empty(results); + } + + [Fact] + public void Valid_store_of_200_orders_passes() + { + var (valid, errors) = Run(Build(200)); + + Assert.True(valid); + Assert.Empty(errors); + } + + /// + /// Every order is found by the store's own list, one level below the list, so its path is + /// "Orders[i].Code". A depth-first walk reaches most orders through the links between + /// products and lines, and the path of each one is a long chain of those links. + /// + [Fact] + public void Each_order_is_reported_once_with_its_shortest_path() + { + var store = Build(25); + foreach (var order in store.Orders) order.Code = null; + + var (valid, errors) = Run(store); + + Assert.False(valid); + Assert.Equal( + ResultText.Expect(Enumerable.Range(0, 25).Select(i => $"Orders[{i}].Code | Code is required").ToArray()), + errors); + } + + /// + /// The products are reached through the lines of an order: "Orders[0].Lines[0].Product" is + /// the shortest path to the first one, and the second line of the first order is the shortest + /// path to the second one. Each product is at most 5 levels deep. + /// + [Fact] + public void Each_product_is_reported_once_with_its_shortest_path() + { + var store = Build(3); + foreach (var line in store.Orders.SelectMany(o => o.Lines)) line.Product.Name = null; + + var (valid, errors) = Run(store); + + Assert.False(valid); + Assert.Equal( + ResultText.Expect( + $"Orders[0].Lines[0].Product.Name | {NameRequired}", + $"Orders[0].Lines[1].Product.Name | {NameRequired}", + $"Orders[1].Lines[1].Product.Name | {NameRequired}", + $"Orders[2].Lines[1].Product.Name | {NameRequired}"), + errors); + } + } + + /// + /// A ring of objects where each one links to the next one and to the one before it. Previous + /// is declared first, so a depth-first walk starts from the head and goes backwards around + /// the ring: its route to the object just after the head is the whole ring, 200 levels. + /// Every object is at most about 100 levels from the head, which is the shortest path. + /// + public class RingOfObjects + { + public class RingNode + { + public RingNode Previous { get; set; } + + public RingNode Next { get; set; } + + [Required(ErrorMessage = NameRequired)] + public string Name { get; set; } = "ok"; + } + + public class Ring + { + public RingNode Head { get; set; } + } + + private static Ring Build(int count) + { + var nodes = Enumerable.Range(0, count).Select(_ => new RingNode()).ToList(); + for (var i = 0; i < count; i++) + { + nodes[i].Next = nodes[(i + 1) % count]; + nodes[i].Previous = nodes[(i + count - 1) % count]; + } + + return new Ring { Head = nodes[0] }; + } + + [Fact] + public void Valid_ring_of_200_objects_passes() + { + var (valid, errors) = Run(Build(200)); + + Assert.True(valid); + Assert.Empty(errors); + } + + /// + /// The object opposite the head is 100 steps away in both directions. The walk goes + /// backwards first, so the tie goes to "Previous". + /// + [Fact] + public void Object_opposite_the_head_is_reported_with_the_shortest_path() + { + var ring = Build(200); + var opposite = ring.Head; + for (var i = 0; i < 100; i++) opposite = opposite.Next; + opposite.Name = null; + + var (valid, errors) = Run(ring); + + Assert.False(valid); + Assert.Equal( + ResultText.Expect($"Head.{string.Join(".", Enumerable.Repeat("Previous", 100))}.Name | {NameRequired}"), + errors); + } + } + + /// + /// A chain of 200 objects is 200 levels deep, and no other route is shorter. The ring above + /// is different, because it has a shortcut. These tests keep the limit for a real chain. + /// + public class AChainThatIsReallyTooDeep + { + private static Node Chain(int levels) + { + var root = new Node(); + var current = root; + for (var i = 0; i < levels; i++) + { + current.Next = new Node(); + current = current.Next; + } + + return root; + } + + [Fact] + public void Object_at_level_129_gets_one_depth_error() + { + var (valid, errors) = Run(Chain(129)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{string.Join(".", Enumerable.Repeat("Next", 129))} | {TooDeep}"), errors); + } + + [Fact] + public void Chain_of_200_gets_one_depth_error() + { + var (valid, errors) = Run(Chain(200)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{string.Join(".", Enumerable.Repeat("Next", 129))} | {TooDeep}"), errors); + } + } + + /// + /// Rule 2: an object that two routes reach is reported with the shorter one. + /// + public class ObjectsThatTwoRoutesReach + { + public class Wrapper + { + public Wrapper Inner { get; set; } + + public Node Shared { get; set; } + } + + public class ShortAndLong + { + // The long route is declared first. A depth-first walk takes it first. + public Wrapper Long { get; set; } + + public Node Short { get; set; } + } + + public class TwoEqualRoutes + { + public Wrapper First { get; set; } + + public Wrapper Second { get; set; } + } + + public class ListOrProperty + { + public List Items { get; set; } + + public Node Direct { get; set; } + } + + [Fact] + public void Shortest_path_wins_over_the_first_path() + { + var shared = new Node { Name = null }; + var model = new ShortAndLong + { + Long = new Wrapper { Inner = new Wrapper { Shared = shared } }, + Short = shared, + }; + + var (valid, errors) = Run(model); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"Short.Name | {NameRequired}"), errors); + } + + /// + /// Both routes are two levels long. The property that is declared first wins, as it did + /// in a depth-first walk. + /// + [Fact] + public void Equally_long_routes_report_the_first_property() + { + var shared = new Node { Name = null }; + var model = new TwoEqualRoutes + { + First = new Wrapper { Shared = shared }, + Second = new Wrapper { Shared = shared }, + }; + + var (valid, errors) = Run(model); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"First.Shared.Name | {NameRequired}"), errors); + } + + /// + /// "Items[0]" is two levels, one for the property and one for the index, and "Direct" is one. + /// + [Fact] + public void A_property_is_shorter_than_an_item_of_a_collection() + { + var shared = new Node { Name = null }; + var model = new ListOrProperty { Items = new List { shared }, Direct = shared }; + + var (valid, errors) = Run(model); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"Direct.Name | {NameRequired}"), errors); + } + } + + /// + /// Rule 3: the results are in the order of the walk. + /// + public class ResultOrder + { + public class Parent + { + public Child A { get; set; } + + public Child B { get; set; } + + [Required(ErrorMessage = NameRequired)] + public string Name { get; set; } + } + + public class Child + { + public Child Deeper { get; set; } + + [Required(ErrorMessage = NameRequired)] + public string Name { get; set; } + } + + /// + /// A depth-first walk reports A.Deeper.Name before B.Name, because it goes down A first. + /// + [Fact] + public void Shallower_objects_come_before_deeper_objects() + { + var model = new Parent + { + A = new Child { Name = null, Deeper = new Child { Name = null } }, + B = new Child { Name = null }, + Name = null, + }; + + var errors = RunInOrder(model); + + Assert.Equal( + new[] + { + $"Name | {NameRequired}", + $"A.Name | {NameRequired}", + $"B.Name | {NameRequired}", + $"A.Deeper.Name | {NameRequired}", + }, + errors); + } + + /// + /// The walk finishes a level for all the parents before it starts the next one. A depth-first + /// walk reports A.X.Name before B.Name. + /// + [Fact] + public void A_level_is_finished_for_every_parent_before_the_next_level_starts() + { + var model = new Parent + { + A = new Child { Name = null, Deeper = new Child { Name = null } }, + B = new Child { Name = null, Deeper = new Child { Name = null } }, + Name = null, + }; + + var errors = RunInOrder(model); + + Assert.Equal( + new[] + { + $"Name | {NameRequired}", + $"A.Name | {NameRequired}", + $"B.Name | {NameRequired}", + $"A.Deeper.Name | {NameRequired}", + $"B.Deeper.Name | {NameRequired}", + }, + errors); + } + + public class Ordered + { + [Required(ErrorMessage = "A is required")] + public string A { get; set; } + + public Child First { get; set; } + + public List Items { get; set; } + + public Child Last { get; set; } + + [Required(ErrorMessage = "Z is required")] + public string Z { get; set; } + } + + /// + /// Callers who show errors in a list see them in this order: the root object's own + /// results first, as the framework's Validator returns them, then the results of the + /// objects one level below it, in the order reflection lists the properties, then the + /// objects two levels below it. The walk is breadth first since 3.0, so the shallowest + /// objects come first. Up to 2.3 it was depth first, and the items of Items came + /// between First and Last. An item of a collection is two levels below its object, one + /// for the property and one for the index, so it comes after Last, which is one level + /// below. Reflection does not promise declaration order, but every release makes the + /// same GetProperties call. The test runs on every target framework. + /// See: https://learn.microsoft.com/dotnet/api/system.type.getproperties + /// + [Fact] + public void Root_results_come_first_then_each_level_in_property_order() + { + var model = new Ordered + { + First = new Child(), + Items = new List { new Child(), new Child() }, + Last = new Child(), + }; + + var errors = RunInOrder(model); + + Assert.Equal( + new[] + { + "A | A is required", + "Z | Z is required", + $"First.Name | {NameRequired}", + $"Last.Name | {NameRequired}", + $"Items[0].Name | {NameRequired}", + $"Items[1].Name | {NameRequired}", + }, + errors); + } + + /// + /// An item of a collection that a property holds is two levels below the object: one for + /// the property and one for the index. It comes after an object that another property + /// holds directly, which is one level below, even if the collection is declared first. + /// + public class WithItems + { + public List Items { get; set; } + + public Child Direct { get; set; } + } + + [Fact] + public void Items_of_a_property_collection_come_after_a_direct_property_of_the_same_object() + { + var model = new WithItems + { + Items = new List { new Child { Name = null } }, + Direct = new Child { Name = null }, + }; + + var errors = RunInOrder(model); + + Assert.Equal( + new[] + { + $"Direct.Name | {NameRequired}", + $"Items[0].Name | {NameRequired}", + }, + errors); + } + } + + /// + /// The depth in a depth error is the length of the shortest path, so the error is never false. + /// + public class DepthErrors + { + public class Link + { + public Link Next { get; set; } + + public Fork Fork { get; set; } + } + + public class Fork + { + public Branch A { get; set; } + + public Branch B { get; set; } + } + + public class Branch + { + public List Items { get; set; } + + public Node Direct { get; set; } + } + + // `links` Link objects in a row, the last one holds the fork, which is one level below it. + private static Link ChainToFork(int links, Fork fork) + { + var root = new Link(); + var current = root; + for (var i = 1; i < links; i++) + { + current.Next = new Link(); + current = current.Next; + } + + current.Fork = fork; + return root; + } + + private static string NextPath(int links) => + links == 1 ? "" : string.Join(".", Enumerable.Repeat("Next", links - 1)) + "."; + + /// + /// The fork is at level 126, and its branches A and B are at level 127. The shared object + /// is at level 129 from A, which holds it in a list (a property and an index are two + /// levels), and at level 128 from B, which holds it in a property. Level 128 is within the + /// limit, so it is validated. A walk that takes the first route it finds, A, would see + /// level 129 and fail it with "nested more than 128 levels deep", which is false. + /// + [Fact] + public void An_object_that_a_shorter_route_reaches_within_the_limit_is_validated() + { + var shared = new Node { Name = null }; + var fork = new Fork + { + A = new Branch { Items = new List { shared } }, + B = new Branch { Direct = shared }, + }; + + var (valid, errors) = Run(ChainToFork(126, fork)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{NextPath(126)}Fork.B.Direct.Name | {NameRequired}"), errors); + } + + /// + /// Two routes reach the same object at level 129, and no route is shorter. Nobody validated + /// it, so the validator reports it once, and not once for each route. + /// + [Fact] + public void An_object_that_is_too_deep_by_every_route_gets_one_depth_error() + { + var shared = new Node(); + var fork = new Fork + { + A = new Branch { Direct = new Node { Next = shared } }, + B = new Branch { Direct = new Node { Next = shared } }, + }; + + var (valid, errors) = Run(ChainToFork(126, fork)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{NextPath(126)}Fork.A.Direct.Next | {TooDeep}"), errors); + } + } + + /// + /// Rule 4. A computed property can return a new, equal object on each read, so an object that + /// Equals an object on its own path is validated for its own attributes and not walked (see + /// the README, "Shared objects, cycles and computed properties"). The path is the shortest + /// one. Another path to the same object that passes no equal ancestor still walks it, and + /// the walk does not skip it because the shortest path stopped there. + /// + public class ObjectsThatEqualAnAncestor + { + public class Leaf + { + [Required(ErrorMessage = NameRequired)] + public string Name { get; set; } + } + + /// An entity that Equals another one with the same Id, as an ORM entity does. + public class Entity + { + public int Id { get; set; } + + public Entity Copy { get; set; } + + public Leaf Child { get; set; } + + public override bool Equals(object obj) => obj is Entity other && other.Id == Id; + + public override int GetHashCode() => Id; + } + + public class Wrapper2 + { + public Wrapper3 Inner { get; set; } + } + + public class Wrapper3 + { + public Entity X { get; set; } + } + + public class LongFirst + { + public Wrapper2 Long { get; set; } + + public Entity Short { get; set; } + } + + public class ShortFirst + { + public Entity Short { get; set; } + + public Wrapper2 Long { get; set; } + } + + // `x` has the same Id as `a`, and `a` holds `x` as its Copy, so the shortest path to `x` + // (Short.Copy, 2 levels) passes an equal ancestor. A second path to `x` (Long.Inner.X, + // 3 levels) passes none, and `x` holds an invalid object. + private static (Entity A, Entity X, Wrapper2 Long) Build() + { + var x = new Entity { Id = 1, Child = new Leaf { Name = null } }; + var a = new Entity { Id = 1, Copy = x }; + var path = new Wrapper2 { Inner = new Wrapper3 { X = x } }; + return (a, x, path); + } + + /// + /// v2.3.3 walked the first path in property order, which is Long, and found the invalid + /// Leaf. It must still be found, although the walk takes Short first. + /// + [Fact] + public void A_longer_path_that_passes_no_equal_ancestor_walks_the_object() + { + var (a, _, path) = Build(); + + var (valid, errors) = Run(new LongFirst { Long = path, Short = a }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"Long.Inner.X.Child.Name | {NameRequired}"), errors); + } + + /// + /// The same graph with the properties in the other order. v2.3.3 took Short first, stopped + /// at `x` there, and then dropped the second path, so it passed. That was a known gap. + /// + [Fact] + public void The_property_order_does_not_decide() + { + var (a, _, path) = Build(); + + var (valid, errors) = Run(new ShortFirst { Short = a, Long = path }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"Long.Inner.X.Child.Name | {NameRequired}"), errors); + } + + public class NamedEntity + { + public int Id { get; set; } + + [Required(ErrorMessage = NameRequired)] + public string Name { get; set; } = "ok"; + + public NamedEntity First { get; set; } + + public NamedEntity Second { get; set; } + + public override bool Equals(object obj) => obj is NamedEntity other && other.Id == Id; + + public override int GetHashCode() => Id; + } + + /// + /// Two paths stop at the same object. Its own attributes are validated once, and not once + /// for each path. + /// + [Fact] + public void An_object_that_two_paths_stop_at_is_validated_once() + { + var copy = new NamedEntity { Id = 1, Name = null }; + var root = new NamedEntity { Id = 1, First = copy, Second = copy }; + + var (valid, errors) = Run(root); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"First.Name | {NameRequired}"), errors); + } + + public class EqChain + { + public int Id { get; set; } + + public EqChain Next { get; set; } + + [Required(ErrorMessage = NameRequired)] + public string Name { get; set; } = "ok"; + + public override bool Equals(object obj) => obj is EqChain other && other.Id == Id; + + public override int GetHashCode() => Id; + } + + /// + /// The check for an equal ancestor comes before the check for the depth. The object at + /// level 129 Equals the object at level 5, so it is validated and not walked, and it gets + /// its own error and no depth error. + /// + [Fact] + public void An_object_that_equals_an_ancestor_at_level_129_is_validated_and_has_no_depth_error() + { + var root = new EqChain { Id = 0 }; + var current = root; + for (var id = 1; id <= 128; id++) + { + current.Next = new EqChain { Id = id }; + current = current.Next; + } + + current.Next = new EqChain { Id = 5, Name = null }; + + var (valid, errors) = Run(root); + + Assert.False(valid); + Assert.Equal( + ResultText.Expect($"{string.Join(".", Enumerable.Repeat("Next", 129))}.Name | {NameRequired}"), + errors); + } + + public class RegionStore + { + public List Regions { get; set; } + + public List Orders { get; set; } + } + + public class Region + { + public List Customers { get; set; } + } + + public class RegionCustomer + { + public List Orders { get; set; } + } + + public class RegionOrder + { + public int Id { get; set; } + + public RegionCustomer Customer { get; set; } + + public List Lines { get; set; } + + public override bool Equals(object obj) => obj is RegionOrder other && other.Id == Id; + + public override int GetHashCode() => Id; + } + + public class RegionLine + { + [Required(ErrorMessage = "Sku is required")] + public string Sku { get; set; } + } + + /// + /// Known limit. An ORM that loads without tracking can return two instances of order 7. + /// Here `copy` is only reachable through customer `c`, and the shortest path to `c` is + /// Orders[0].Customer, below the tracked order 7. The copy is the order that `c` holds, + /// so it Equals an ancestor on that path, and the walk stops there. The customer is + /// reached by another path, Regions[0].Customers[0], which has no equal ancestor, but + /// the customer was queued on its shortest path and is not walked a second time. To find + /// the line, the walk would have to walk a shared object once for each different chain of + /// equal ancestors, and that number has no bound. v2.3.3 found it, by the first path in + /// property order. + /// + [Fact(Skip = "Known limit. A shared object is walked once, by its shortest path, and an equal ancestor on that path stops the walk.")] + public void An_object_below_a_shared_customer_is_validated_when_the_shortest_path_has_an_equal_ancestor() + { + var customer = new RegionCustomer(); + var tracked = new RegionOrder { Id = 7, Customer = customer }; + var copy = new RegionOrder + { + Id = 7, + Customer = customer, + Lines = new List { new RegionLine { Sku = null } }, + }; + customer.Orders = new List { copy }; + var store = new RegionStore + { + Regions = new List { new Region { Customers = new List { customer } } }, + Orders = new List { tracked }, + }; + + var (valid, errors) = Run(store); + + Assert.False(valid); + Assert.Equal( + ResultText.Expect("Regions[0].Customers[0].Orders[0].Lines[0].Sku | Sku is required"), + errors); + } + } + + /// + /// A collection that a property holds is enumerated when the walk reaches it, which is after + /// the objects at its own level and above are validated. It is not enumerated when the + /// property is read. Up to 2.3, the walk enumerated it at once, while it validated the + /// object that holds it. So a Validate method that changes a collection is now seen by the + /// walk, and a Validate method that changes the list that holds it no longer makes + /// enumeration throw. Changing a collection during validation is unusual, and the tests pin + /// the order so that a change to it is on purpose. + /// See: https://learn.microsoft.com/dotnet/api/system.invalidoperationexception + /// (a List<T> that changes during a foreach throws it). + /// + public class WhenACollectionIsEnumerated + { + public class HasItems + { + public List Items { get; set; } = new List { new Node() }; + } + + /// Adds an invalid item to the collection of another object, when it is validated. + public class Mutator : IValidatableObject + { + public HasItems Target { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + Target.Items.Add(new Node { Name = null }); + return Enumerable.Empty(); + } + } + + public class MutateRoot + { + public HasItems A { get; set; } + + public Mutator B { get; set; } + } + + /// + /// B is validated before the items of A are enumerated, so the item that B adds is validated. + /// Up to 2.3, A.Items was enumerated first, and the item that B added was never seen. + /// + [Fact] + public void A_change_that_a_sibling_makes_during_validation_is_seen() + { + var a = new HasItems(); + + var (valid, errors) = Run(new MutateRoot { A = a, B = new Mutator { Target = a } }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"A.Items[1].Name | {NameRequired}"), errors); + } + + /// An item that adds a second item to the list that holds it, when it is validated. + public class SelfAdder : IValidatableObject + { + [Required(ErrorMessage = NameRequired)] + public string Name { get; set; } = "ok"; + + public List Owner { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + if (Owner != null && Owner.Count == 1) + { + Owner.Add(new SelfAdder { Name = null }); + } + + return Enumerable.Empty(); + } + } + + public class SelfAdderRoot + { + public List Items { get; set; } + } + + /// + /// Up to 2.3, the walk was inside the foreach over Items when the item added to the + /// list, so the next step of the foreach threw "Collection was modified". The list of + /// Items is now enumerated completely before the item is validated, and the item's own + /// Owner property, which is the same list, is enumerated after it. + /// + [Fact] + public void An_item_that_adds_to_the_list_that_holds_it_does_not_make_enumeration_throw() + { + var item = new SelfAdder(); + var list = new List { item }; + item.Owner = list; + + var (valid, errors) = Run(new SelfAdderRoot { Items = list }); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"Items[0].Owner[1].Name | {NameRequired}"), errors); + } + } + } +} diff --git a/test/RecursiveDataAnnotationsValidation.Tests/GraphSizeTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/GraphSizeTests.cs index 8e44291..ada2832 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/GraphSizeTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/GraphSizeTests.cs @@ -14,10 +14,11 @@ namespace RecursiveDataAnnotationsValidation.Tests /// comment thread or a category tree can be dozens of levels deep, and an import can hold /// thousands of items. /// - /// The walk calls itself once for each level, so the depth of the graph is the depth of the - /// call stack. These tests run the walk on a thread with a 1 MB stack, which is the default on - /// Windows. macOS and Linux give the main thread 8 MB, so a test that runs on the default - /// stack would pass on those systems and still overflow on Windows. + /// Up to 2.3 the walk called itself once for each level, so the depth of the graph was the depth + /// of the call stack. Since 3.0 the walk keeps its work in a queue and uses no stack for the + /// depth of the graph. These tests still run the walk on a thread with a 1 MB stack, which is + /// the default on Windows, so a walk that calls itself again fails here. macOS and Linux give + /// the main thread 8 MB, so a test that runs on the default stack would not show that. /// See: https://learn.microsoft.com/dotnet/api/system.threading.thread.-ctor /// /// The validator has a maximum depth of 128 (see MaxDepthTests), which these tests stay below. @@ -147,7 +148,7 @@ public void Error_at_the_bottom_has_the_full_path() public async Task Error_at_the_bottom_has_the_full_path_in_the_async_method() { // Task.Run uses a thread-pool thread, whose stack size the caller cannot choose. - // A depth of 120 is far below the limit measured for a 1 MB stack. + // A depth of 120 is below the maximum depth. var results = new List(); var valid = await new RecursiveDataAnnotationValidator() diff --git a/test/RecursiveDataAnnotationsValidation.Tests/MaxDepthTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/MaxDepthTests.cs index ef9d007..f32be3b 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/MaxDepthTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/MaxDepthTests.cs @@ -12,14 +12,20 @@ namespace RecursiveDataAnnotationsValidation.Tests /// /// The maximum depth of the object graph is 128. /// - /// The walk calls itself once for each level, so a deep graph is a deep call stack. .NET cannot - /// catch a StackOverflowException (https://learn.microsoft.com/dotnet/api/system.stackoverflowexception), - /// so the process ends. A graph this deep is not a real model. It points to a cycle that the - /// validator cannot see, such as a computed property that returns a new object on each read, - /// or to a graph that was built from untrusted input. The validator therefore fails the - /// validation and does not walk any further. + /// A graph this deep is not a real model. It points to a cycle that the validator cannot see, + /// such as a computed property that returns a new object on each read, or to a graph that was + /// built from untrusted input. Without a limit, such a walk never ends. The validator therefore + /// fails the validation and does not walk any further. /// - /// The depth of an object is the number of segments in its path. In "Value[0][0].Name", + /// Up to 2.3 the walk called itself once for each level, so a deep graph was a deep call stack. + /// .NET cannot catch a StackOverflowException + /// (https://learn.microsoft.com/dotnet/api/system.stackoverflowexception), so the process ended. + /// Since 3.0 the walk is breadth first and keeps its work in a queue (see BreadthFirstWalkTests), + /// so the stack no longer limits the depth. The tests below still use short graphs, so they + /// also run against the older walk without ending the test run. + /// + /// The depth of an object is the number of segments in the shortest path to it. An object that + /// other, longer paths reach is not too deep. In "Value[0][0].Name", /// Value is level 1, the first [0] is level 2, the second [0] is level 3 and Name is level 4. /// Each property step and each collection index counts as one level. System.Text.Json counts /// nearly the same way, where each object and each array is one level, and its limit is 64 @@ -225,8 +231,8 @@ public async Task The_async_method_fails_the_same_way() /// /// Before the limit, a chain of about 1,300 objects was the most that a 1 MB stack, the /// default on Windows, could walk. A chain of 600 is well below that, so this test cannot - /// end the test run when the limit is missing, and it still shows that the validator now - /// fails the validation at level 129. A chain of 100,000 would end the process. + /// end the test run when the limit is missing, and it still shows that the validator + /// fails the validation at level 129. /// [Fact] public void A_chain_of_600_objects_fails_on_a_small_stack() diff --git a/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs index 152ae5b..4319629 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs @@ -15,7 +15,8 @@ namespace RecursiveDataAnnotationsValidation.Tests /// the same true/false answer and the same results as 2.2, with the same member names and /// messages, in the same order. /// - /// Every test here passes against the validator source of both v2.2.0 and v2.2.4. To check + /// Every test here passes against the validator source of both v2.2.0 and v2.2.4, in the same + /// order, except where the list below says that 3.0 differs. To check /// that again, copy the old source over the current one, run only this class, then restore it: /// git show v2.2.0:src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs \ /// > src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs @@ -29,6 +30,8 @@ namespace RecursiveDataAnnotationsValidation.Tests /// - Framework types such as Type, delegates and relative Uris (ValidatorHardeningTests). /// - Lazy sequences of primitives, which 2.3 no longer runs (ValidatorHardeningTests). /// - A null object or context, and a property hidden with `new` (fixed in 2.2.4). + /// - 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). /// /// The messages are the framework's default English messages, or a custom ErrorMessage where /// .NET Framework and .NET might word a default differently. @@ -131,47 +134,6 @@ public void A_valid_object_returns_true_even_when_the_list_already_has_results() Assert.Single(list); } - /// - /// Callers who show errors in a list see them in this order: the root object's own - /// results first, as the framework's Validator returns them, then the results of each - /// walked property in the order reflection lists the properties. Reflection does not - /// promise declaration order, but 2.2 and 2.3 make the same GetProperties call, so the - /// order is the same in both. The test runs on every target framework. - /// See: https://learn.microsoft.com/dotnet/api/system.type.getproperties - /// - [Fact] - public void Root_results_come_first_then_nested_results_in_property_order() - { - var root = new Ordered - { - First = new Leaf(), - Items = new List { new Leaf(), new Leaf() }, - Last = new Leaf(), - }; - - var list = new List(); - new RecursiveDataAnnotationValidator().TryValidateObjectRecursive(root, list); - - Assert.Equal( - new[] { "A", "Z", "First.Name", "Items[0].Name", "Items[1].Name", "Last.Name" }, - list.Select(r => string.Join(",", r.MemberNames))); - } - - public class Ordered - { - [Required] - public string A { get; set; } - - public Leaf First { get; set; } - - public List Items { get; set; } - - public Leaf Last { get; set; } - - [Required] - public string Z { get; set; } - } - /// /// A nested object is validated with its own ValidationContext, so its message names /// only its own property, never the path. Only the member names get the path. @@ -537,9 +499,9 @@ public void Deep_chain_reports_the_full_dotted_path() /// /// A chain a hundred objects deep is validated without running out of stack. - /// Both versions recurse once per level, so very deep graphs (thousands of levels) - /// can still overflow the stack in the old version. Since 3.0 the validator stops at - /// 128 levels and fails the validation (see MaxDepthTests), so a chain of 200, which + /// The old version recurses once per level, so very deep graphs (thousands of levels) + /// can overflow the stack in it. Since 3.0 the walk uses a queue and stops at + /// 128 levels, failing the validation (see MaxDepthTests), so a chain of 200, which /// this test used before, is no longer the same in both versions. /// [Fact] diff --git a/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs index ea5b795..6e0ea57 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs @@ -120,8 +120,9 @@ public async Task Null_validation_context_throws_ArgumentNullException_async() /// Computed properties need one more check. A property can return a new, equal object on /// each read, such as `Point Origin => new Point(0, 0)` on a record or /// `Money Zero => new Money(0)` on a value object. Each read is a new reference, so a - /// reference check alone never stops, and the walk overflows the stack. v2.2.0 stopped - /// these only because of value equality. + /// reference check alone never stops. Before 3.0 the walk overflowed the stack. Now it goes + /// on until the maximum depth and fails the validation (see MaxDepthTests.ComputedProperties). + /// v2.2.0 stopped these only because of value equality. /// So the validator keeps a second list: the objects on the path from the root to the /// current object whose type overrides Equals. When an object's type overrides Equals, /// and the object Equals an object of a related type on that list, the validator: @@ -460,7 +461,8 @@ public class PriceModel } // Guard. On v2.2.0, value equality stopped each chain at the second Money(0). If this - // breaks, the stack overflows and kills the test host (see the Origin guards). + // breaks, the walk goes on to the maximum depth and the validation fails with a depth + // error. Before 3.0 it overflowed the stack and killed the test host (see the Origin guards). [Fact] public void Value_object_property_that_returns_a_new_equal_object_terminates() { @@ -501,7 +503,8 @@ public class LazyPriceModel // Guard. On v2.2.0, value equality stopped the chain at the second LazyMoney(0). A // rule that asks whether the property is computed misses this one, because the - // property is stored, and the walk overflows the stack. + // property is stored, and the walk goes on to the maximum depth, where the validation + // fails. Before 3.0 it overflowed the stack. [Fact] public void Stored_sequence_that_yields_new_equal_objects_terminates() { @@ -565,9 +568,10 @@ public class ShapeModel // Guard. On v2.2.0, value equality stops the walk at the second Origin, because it // Equals the first. A reference-equality set alone would never stop: each read is a - // new instance, so the walk recurses until the stack overflows. A stack overflow - // cannot be caught and kills the test host, so if this guard breaks, the whole test - // run crashes instead of reporting one failure. + // new instance, so the walk goes on until the maximum depth, and the validation fails + // with a depth error. Before 3.0 the walk recursed until the stack overflowed. A stack + // overflow cannot be caught and kills the test host, so a broken guard crashed the whole + // test run instead of reporting one failure. // See: https://learn.microsoft.com/dotnet/api/system.stackoverflowexception [Fact] public void Record_property_that_returns_a_new_equal_record_terminates()