diff --git a/src/GameCult.Caching/CultElementIds.cs b/src/GameCult.Caching/CultElementIds.cs index c27fef5f..77ba8501 100644 --- a/src/GameCult.Caching/CultElementIds.cs +++ b/src/GameCult.Caching/CultElementIds.cs @@ -148,6 +148,10 @@ namespace GameCult.Caching private static void VisitMemberType(Type documentType, Type owner, MemberInfo member, Type memberType, Type[] through, bool hidden, HashSet seen, Survey survey, Graph graph) { memberType = Nullable.GetUnderlyingType(memberType) ?? memberType; + // A sequence or map whose element is itself (an unkeyed Node : IEnumerable) ends the type walk here; without + // this it would recurse until the stack overflows. + if (Array.IndexOf(through, memberType) >= 0) + return; var (kind, inner) = Classify(memberType); switch (kind) { @@ -565,8 +569,10 @@ namespace GameCult.Caching private static readonly ConcurrentDictionary Classes = new(); - // The one recognition of what MessagePack writes for a declared type: registration, Plan, Holds and the codec's id path - // all go through it. Inner is a map's value type, a sequence's element type, or the type itself for an owner. + // The walk's model of a declared type: registration, Plan, Holds and the codec's id path all go through it. A + // [MessagePackObject] or [Union] type is read by key even when it is also IEnumerable; the codec checks, for every type + // on the id path, that MessagePack writes it the way this model reads it, and refuses the document when it does not. + // Inner is a map's value type, a sequence's element type, or the type itself for an owner. // Leaf a value written whole that holds no element list: strings (an IEnumerable MessagePack writes whole), // a struct, class or interface with no [MessagePackObject] or [Union] (primitives, enums, float2, DateTime, // and object, which is untyped on disk). @@ -579,13 +585,15 @@ namespace GameCult.Caching type = Nullable.GetUnderlyingType(type) ?? type; if (type == typeof(string)) return (Kind.Leaf, type); + if (IsKeyed(type)) + return (Kind.Owner, type); if (type.IsValueType) - return IsKeyed(type) ? (Kind.Owner, type) : (Kind.Leaf, type); + return (Kind.Leaf, type); if (MapTypes(type) is { } map) return (Kind.Map, map.Value); if (ElementTypeOf(type) is { } element) return (Kind.Sequence, element); - return IsKeyed(type) ? (Kind.Owner, type) : (Kind.Leaf, type); + return (Kind.Leaf, type); }); // A type MessagePack serializes by key: [MessagePackObject] or [Union]. diff --git a/tests/GameCult.Caching.Tests/ElementIdEngineTests.cs b/tests/GameCult.Caching.Tests/ElementIdEngineTests.cs index 45ab621f..1a4417b8 100644 --- a/tests/GameCult.Caching.Tests/ElementIdEngineTests.cs +++ b/tests/GameCult.Caching.Tests/ElementIdEngineTests.cs @@ -485,7 +485,64 @@ namespace GameCult.Caching.Tests Assert.That(CultElementIds.Inspect(typeof(HHiddenList)).IdPath, Is.Empty); } + // MessagePack writes a [MessagePackObject] class that is only IEnumerable by its keys, so the walk reads it by key. A + // keyed tree node that enumerates its own children is the shape that overflowed the stack when enumeration came first. + [Test] + public void ATypeThatEnumeratesItselfIsWalkedByKeyAndTerminates() + { + var survey = CultElementIds.Inspect(typeof(HKNode)); + Assert.That(survey.Problems, Is.Empty); + Assert.That(survey.IdPath, Is.EquivalentTo(new[] { typeof(HKNode), typeof(List), typeof(KNode) })); + + var root = new HKNode { Nodes = { new KNode { Children = { new KNode { Children = { new KNode() } } } } } }; + Assert.That(Assign(root, "r", deterministic: true), Is.EqualTo(3)); + Assert.That(root.Nodes[0].Id, Has.Length.EqualTo(12)); + Assert.That(root.Nodes[0].Children[0].Children[0].Id, Has.Length.EqualTo(12)); + + // An unkeyed type that enumerates itself holds no element list and ends the type walk. + var unkeyed = CultElementIds.Inspect(typeof(HUNode)); + Assert.That(unkeyed.Problems, Is.Empty); + Assert.That(unkeyed.IdPath, Is.Empty); + + // A keyed class that only enumerates is read by its keyed lists, so both are judged and minted. + Assert.That(Problems(typeof(KBag)).Single(), Does.Contain("EBare")); + var bag = new KBagIded { Items = { new EIded(), new EIded() } }; + Assert.That(Assign(bag, "r", deterministic: false), Is.EqualTo(2)); + Assert.That(CultElementIds.Holds(typeof(KBagIded), bag), Is.True); + } + // ---- shapes ---- + [MessagePackObject] + public class KNode : IEnumerable + { + [Key(0)] [CultElementId] public string? Id { get; set; } + [Key(1)] public List Children { get; set; } = new(); + public IEnumerator GetEnumerator() => Children.GetEnumerator(); + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); + } + [MessagePackObject] public class HKNode { [Key(0)] public List Nodes { get; set; } = new(); } + public class UNode : IEnumerable + { + public IEnumerator GetEnumerator() { yield break; } + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); + } + [MessagePackObject] public class HUNode { [Key(0)] public UNode? N { get; set; } } + [MessagePackObject] + public class KBag : IEnumerable + { + [Key(0)] public List Items { get; set; } = new(); + [Key(1)] public List Other { get; set; } = new(); + public IEnumerator GetEnumerator() { yield break; } + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); + } + [MessagePackObject] + public class KBagIded : IEnumerable + { + [Key(0)] public List Items { get; set; } = new(); + public IEnumerator GetEnumerator() { yield break; } + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); + } + [MessagePackObject] public class EBare { [Key(0)] public string X { get; set; } = ""; } [MessagePackObject] public class EIded { [Key(0)] public string X { get; set; } = ""; [Key(1)] [CultElementId] public string Id { get; set; } = ""; } [MessagePackObject] public class ENest { [Key(0)] [CultElementId] public string Id { get; set; } = ""; [Key(1)] public List Sub { get; set; } = new(); }