diff --git a/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs b/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs index cd120a33..ee5691f5 100644 --- a/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs +++ b/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs @@ -84,8 +84,8 @@ public static class CultDocumentMessagePackSerialization var generic = GetFormatterMethod.MakeGenericMethod(type); // A resolver that hands back MessagePack's own formatter for the type writes its [Key] layout: that is no claim, and the // composite stops there, so no later resolver is asked. StandardResolverAllowPrivate's formatter writes the same layout - // plus non-public [Key] members, and registration refuses a non-public member on the way to an object list, so it - // claims nothing either. + // plus non-public [Key] members; those extra members are whole values the walk does not own, so it claims nothing + // either. object? standard; try { @@ -93,15 +93,14 @@ public static class CultDocumentMessagePackSerialization } catch (Exception exception) { - return $"Cult document {documentType.Name}: MessagePack cannot build a formatter for {type.Name}, which is on the path to a list whose elements carry ids ({(exception.InnerException ?? exception).Message})."; + return CannotBuild(documentType, type, (exception.InnerException ?? exception).Message); } - if (WrittenAgainstWalk(type, generic, standard)) - { - return CultElementIds.WalksAsOwner(type) - ? $"Cult document {documentType.Name}: MessagePack writes {type.Name} as a collection, but the element-id walk reads it by its [Key] layout, so element ids would be lost or invented. A [MessagePackObject] type that is also a collection (ICollection, IList, IDictionary) is written as a collection; make it one or the other." - : $"Cult document {documentType.Name}: MessagePack writes {type.Name} as an object, but the element-id walk reads it as a collection, so element ids would be lost or invented. A [DataContract] type that is also enumerable is written by its members; mark it [MessagePackObject] with integer [Key]s, or drop the contract."; - } + if (standard == null) + return CannotBuild(documentType, type, "StandardResolver has no formatter for it"); + + if (WrittenAsCollection(type, generic, standard)) + return $"Cult document {documentType.Name}: MessagePack writes {type.Name} as a collection, but the element-id walk reads it by its [Key] layout, so element ids would be lost or invented. A [MessagePackObject] type that is also a collection (ICollection, IList, IDictionary) is written as a collection; make it one or the other. Or declare [MessagePackFormatter] on the type or member, which makes it one whole value with no element ids."; var allowPrivateAsked = false; object? allowPrivate = null; @@ -136,23 +135,22 @@ public static class CultDocumentMessagePackSerialization if (allowPrivate != null && ReferenceEquals(formatter, allowPrivate)) break; - return $"Cult document {documentType.Name}: formatter resolver {resolver.GetType().FullName} formats {type.Name}, which is on the path to a list whose elements carry ids, so its element ids would never reach disk. A resolver may not claim such a type; write it by its [Key] layout or keep it out of the document's lists."; + return $"Cult document {documentType.Name}: formatter resolver {resolver.GetType().FullName} formats {type.Name}, which is on the path to a list whose elements carry ids, so its element ids would never reach disk. A resolver may not claim such a type; write it by its [Key] layout, keep it out of the document's lists, or declare [MessagePackFormatter] on the type or member, which makes it one whole value with no element ids."; } } return null; } - // The door refuses only a proven mismatch: StandardResolver's formatter being the very instance the contrary resolver's - // public singleton hands out (one is cached per type). An owner is contradicted by the generic resolver (a keyed - // ICollection or dictionary), a collection by the object resolver (a [DataContract] enumerable). Union types walk as - // owners and the immutable and builtin resolvers claim only framework types, so neither can contradict. Any other - // formatter, a source-generated one among them, passes: the walk owns which types carry ids and this door is its safety net. - private static bool WrittenAgainstWalk(Type type, MethodInfo generic, object? standard) - { - var contrary = CultElementIds.WalksAsOwner(type) ? (IFormatterResolver)DynamicGenericResolver.Instance : DynamicObjectResolver.Instance; - return standard != null && ReferenceEquals(Formatter(contrary, generic), standard); - } + private static string CannotBuild(Type documentType, Type type, string reason) => + $"Cult document {documentType.Name}: MessagePack cannot build a formatter for {type.Name}, which is on the path to a list whose elements carry ids ({reason})."; + + // The door verifies the walk and never decides ownership. It refuses a proven mismatch: a type the walk reads by key + // (an owner) whose StandardResolver formatter is the very instance the generic resolver's singleton hands out (a keyed + // ICollection or dictionary). Union types walk as owners and the immutable and builtin resolvers claim only framework + // types, so neither can contradict. Any other formatter, a source-generated one among them, passes. + private static bool WrittenAsCollection(Type type, MethodInfo generic, object? standard) => + CultElementIds.WalksAsOwner(type) && ReferenceEquals(Formatter(DynamicGenericResolver.Instance, generic), standard); private static object? Formatter(IFormatterResolver resolver, MethodInfo generic) { diff --git a/src/GameCult.Caching/CultElementIds.cs b/src/GameCult.Caching/CultElementIds.cs index 4746b59c..99eb094a 100644 --- a/src/GameCult.Caching/CultElementIds.cs +++ b/src/GameCult.Caching/CultElementIds.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; using System.Reflection; +using System.Runtime.Serialization; using System.Security.Cryptography; using System.Text; using MessagePack; @@ -109,10 +110,8 @@ namespace GameCult.Caching // can sit in a cycle, so the path is a fixed point over these edges and not a stack. private sealed class Graph { - // Hidden is true when the member that starts the entry is non-public: MessagePack writes it only under AllowPrivate, - // and the walk reads public members only. - public List<(Type Owner, MemberInfo? Member, Type Child, Type[] Through, bool Hidden)> Edges { get; } = new(); - public List<(Type Owner, MemberInfo Member, Type Element, List Concrete, Type[] Through, bool Hidden)> Lists { get; } = new(); + public List<(Type Owner, MemberInfo? Member, Type Child, Type[] Through)> Edges { get; } = new(); + public List<(Type Owner, MemberInfo Member, Type Element, List Concrete, Type[] Through)> Lists { get; } = new(); // Keyed types that also enumerate objects: the walk reads them by key, so the codec must confirm MessagePack does too. public List Enumerating { get; } = new(); @@ -140,17 +139,15 @@ namespace GameCult.Caching if (EnumeratesObjects(type)) graph.Enumerating.Add(type); foreach (var (member, memberType) in KeyedMembers(type)) - VisitMemberType(documentType, type, member, memberType, Array.Empty(), false, seen, survey, graph); - foreach (var (member, memberType) in UnreadKeyedMembers(type)) - VisitMemberType(documentType, type, member, memberType, Array.Empty(), true, seen, survey, graph); + VisitMemberType(documentType, type, member, memberType, Array.Empty(), seen, survey, graph); foreach (var union in type.GetCustomAttributes(true)) { - graph.Edges.Add((type, null, union.SubType, Array.Empty(), false)); + graph.Edges.Add((type, null, union.SubType, Array.Empty())); VisitObject(documentType, union.SubType, seen, survey, graph); } } - private static void VisitMemberType(Type documentType, Type owner, MemberInfo member, Type memberType, Type[] through, bool hidden, HashSet seen, Survey survey, Graph graph) + private static void VisitMemberType(Type documentType, Type owner, MemberInfo member, Type memberType, Type[] through, 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 @@ -161,17 +158,17 @@ namespace GameCult.Caching switch (kind) { case Kind.Leaf: + // A shape the walk does not own is one whole value, never refused (q3 amended). return; case Kind.Map: - VisitMemberType(documentType, owner, member, inner, through.Append(memberType).ToArray(), hidden, seen, survey, graph); + VisitMemberType(documentType, owner, member, inner, through.Append(memberType).ToArray(), seen, survey, graph); return; case Kind.Sequence when IsElement(inner): var concretes = ConcreteTypes(inner).ToList(); - graph.Lists.Add((owner, member, inner, concretes, through.Append(memberType).ToArray(), hidden)); + graph.Lists.Add((owner, member, inner, concretes, through.Append(memberType).ToArray())); foreach (var concrete in concretes) { - // A hidden list is refused as a member (Settle), not once per element type. - if (hidden || !survey.Judged.Add(concrete)) + if (!survey.Judged.Add(concrete)) continue; var problem = IsElement(concrete) ? IdProblem(concrete) : UnkeyedElementProblem; if (problem != null) @@ -181,10 +178,10 @@ namespace GameCult.Caching VisitObject(documentType, inner, seen, survey, graph); return; case Kind.Sequence: - VisitMemberType(documentType, owner, member, inner, through.Append(memberType).ToArray(), hidden, seen, survey, graph); + VisitMemberType(documentType, owner, member, inner, through.Append(memberType).ToArray(), seen, survey, graph); return; default: - graph.Edges.Add((owner, member, memberType, through, hidden)); + graph.Edges.Add((owner, member, memberType, through)); VisitObject(documentType, memberType, seen, survey, graph); return; } @@ -193,24 +190,21 @@ namespace GameCult.Caching private const string UnkeyedElementProblem = "is written by a formatter the cache does not own, so its element id would never reach disk; mark it [MessagePackObject] with [Key] members, or keep it out of lists."; - // The id path: every type that reaches an object list, the list's element types and the concrete types under them. - // [MessagePackFormatter] on any of them, or on a member the walk descends through, would hand the bytes to a formatter - // that does not write the [Key] layout the id lives in. + // The id path: every type that reaches an object list through members MessagePack writes by key, the lists' element and + // concrete types, and the keyed types that enumerate objects. The codec verifies that MessagePack writes each of them the + // way Classify reads it; it never decides ownership. private static void Settle(Type documentType, Survey survey, Graph graph) { - var lists = graph.Lists.Where(list => !list.Hidden).ToArray(); + var lists = graph.Lists; var reaching = new HashSet(lists.Select(list => list.Owner)); - var members = new List(lists.Select(list => list.Member)); var through = new List(lists.SelectMany(list => list.Through)); for (var grew = true; grew;) { grew = false; - foreach (var (owner, member, child, via, hidden) in graph.Edges) + foreach (var (owner, _, child, via) in graph.Edges) { - if (hidden || !reaching.Contains(child)) + if (!reaching.Contains(child)) continue; - if (member != null && !members.Contains(member)) - members.Add(member); through.AddRange(via); grew |= reaching.Add(owner); } @@ -239,30 +233,9 @@ namespace GameCult.Caching foreach (var type in graph.Enumerating) Add(type); - foreach (var type in path) - { - survey.IdPath.Add(type); - if (type.IsDefined(typeof(MessagePackFormatterAttribute), true)) - survey.Problems.Add($"Cult document {documentType.Name}: {type.Name} is on the path to a list whose elements carry ids and declares [MessagePackFormatter], so its element ids would never reach disk; remove the attribute or keep this type out of the document's lists."); - } - - foreach (var member in members) - { - if (Declares(member, typeof(MessagePackFormatterAttribute))) - survey.Problems.Add($"Cult document {documentType.Name}: {member.DeclaringType!.Name}.{member.Name} leads to a list whose elements carry ids and declares [MessagePackFormatter], so its element ids would never reach disk; remove the attribute."); - } - - // The fixed point has settled which types reach a list through public members. A non-public [Key] member that starts - // a list, or leads to a type that reaches one, is refused: nothing reads or checks the ids it holds. - foreach (var list in graph.Lists.Where(list => list.Hidden)) - survey.Problems.Add(HiddenProblem(documentType, list.Owner, list.Member)); - foreach (var edge in graph.Edges.Where(edge => edge.Hidden && reaching.Contains(edge.Child))) - survey.Problems.Add(HiddenProblem(documentType, edge.Owner, edge.Member!)); + survey.IdPath.AddRange(path); } - private static string HiddenProblem(Type documentType, Type owner, MemberInfo member) => - $"Cult document {documentType.Name}: {owner.Name}.{member.Name} is a non-public [Key] member on the way to a list whose elements carry ids; MessagePack writes it only under AllowPrivate and the element-id walk reads public members, so its ids could never be checked. Make it public, or keep lists out of it."; - // The types an element can actually be: the element type itself when it can be instantiated, and every union subtype // under it. Shapes are built from these, so registration judges exactly what minting will meet. private static IEnumerable ConcreteTypes(Type element) @@ -302,14 +275,14 @@ namespace GameCult.Caching if (KeyOf(id) is not { } idKey) return $"declares [CultElementId] on {id.Name}, which has no integer [Key], so it is never persisted."; if (MembersOf(element).FirstOrDefault(member => member.Name == id.Name && member.MemberType == id.MemberType) is not { } written) - return $"declares [CultElementId] on {id.Name}, which MessagePack does not write ([IgnoreMember], non-public, or a property whose getter is not public); the id would never reach disk."; + return $"declares [CultElementId] on {id.Name}, which MessagePack does not write ([IgnoreMember], or non-public on a type without [MessagePackObject(AllowPrivate = true)]); the id would never reach disk."; id = written; var sharing = KeyedMembers(element).FirstOrDefault(entry => entry.Member != id && KeyOf(entry.Member) == idKey); if (sharing.Member != null) return $"declares [CultElementId] on {id.Name} with [Key({idKey})], which {sharing.Member.Name} also uses; an id needs a key of its own."; if (id is PropertyInfo { SetMethod: null } or FieldInfo { IsInitOnly: true }) return $"declares [CultElementId] on {id.Name}, which cannot be assigned; the cache mints into it."; - if (id is PropertyInfo { SetMethod.IsPublic: false }) + if (id is PropertyInfo { SetMethod.IsPublic: false } && !AllowsPrivate(element)) return $"declares [CultElementId] on {id.Name}, whose setter is not public: MessagePack writes the id but does not read it back, so every load would mint a different one. Make the setter public (init is fine)."; var source = id.GetCustomAttribute(true)!.DerivedFrom; if (source != null) @@ -551,10 +524,20 @@ namespace GameCult.Caching return shape; } - // The members MessagePack's StandardResolver writes for an integer-keyed type: public instance fields, and public - // instance properties with a public getter, none of them [IgnoreMember]. TheWalkReadsTheMembersMessagePackWrites checks - // this set against MessagePack's own member discovery. + // The members MessagePack's StandardResolver writes for an integer-keyed type, none of them [IgnoreMember]. Without + // AllowPrivate: public instance fields, and public instance properties with a public getter. With it, read from the most + // derived [MessagePackObject] declaration (a derived type without it is written without it, probe NoApDerived): every + // instance field and indexer-free property up the base chain at any visibility, the most derived of a name. + // TheWalkReadsTheMembersMessagePackWrites checks this set against MessagePack's own member discovery. private static IEnumerable MembersOf(Type type) => + AllowsPrivate(type) + ? AllInstanceMembers(type).Where(member => !Declares(member, typeof(IgnoreMemberAttribute))) + .GroupBy(member => member.Name).Select(group => group.First()) + : PublicMembersOf(type); + + private static bool AllowsPrivate(Type type) => type.GetCustomAttribute(true)?.AllowPrivate == true; + + private static IEnumerable PublicMembersOf(Type type) => type.GetFields(BindingFlags.Instance | BindingFlags.Public).Cast() .Concat(type.GetProperties(BindingFlags.Instance | BindingFlags.Public) .Where(property => property.GetIndexParameters().Length == 0 && property.GetMethod is { IsPublic: true })) @@ -573,36 +556,17 @@ namespace GameCult.Caching } } - // The members the walk reads, as MessagePack writes them, for the oracle test that checks both against MessagePack. - internal static IEnumerable WrittenKeyedMembers(Type type) => KeyedMembers(type).Select(entry => entry.Member); - - // The [Key] members MessagePack writes only under AllowPrivate, for the same test. - internal static IEnumerable UnreadMembers(Type type) => UnreadKeyedMembers(type).Select(entry => entry.Member); + // The [Key] members MessagePack writes, for the oracle test that checks them against MessagePack. + internal static IEnumerable WrittenKeyedMembers(Type type) => MembersOf(type).Where(member => KeyOf(member) != null); private static IEnumerable<(MemberInfo Member, Type Type)> KeyedMembers(Type type) => - MembersOf(type).Where(member => KeyOf(member) != null).Select(member => (member, TypeOf(member))); + WrittenKeyedMembers(type).Where(member => !Declares(member, typeof(MessagePackFormatterAttribute))) + .Select(member => (member, TypeOf(member))); // An attribute a member declares, or inherits through an override: MemberInfo.IsDefined ignores inherit for properties, so an // override of a base virtual member would not see its base's attribute. private static bool Declares(MemberInfo member, Type attribute) => Attribute.IsDefined(member, attribute, inherit: true); - // [Key] members MessagePack writes only under AllowPrivate: non-public fields and properties, and public properties whose - // getter is not public, up the base chain. An [IgnoreMember] member is never written, so it is not one of them. - private static IEnumerable<(MemberInfo Member, Type Type)> UnreadKeyedMembers(Type type) - { - for (var level = type; level != null && level != typeof(object); level = level.BaseType) - { - const BindingFlags flags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly; - foreach (var member in level.GetFields(flags).Where(field => !field.IsPublic).Cast() - .Concat(level.GetProperties(flags).Where(property => - property.GetIndexParameters().Length == 0 && property.GetMethod is { IsPublic: false }))) - { - if (KeyOf(member) != null && !Declares(member, typeof(IgnoreMemberAttribute))) - yield return (member, TypeOf(member)); - } - } - } - // True when the walk reads the type by its [Key] members: the codec compares this with how MessagePack writes it. internal static bool WalksAsOwner(Type type) => Classify(Nullable.GetUnderlyingType(type) ?? type).Kind == Kind.Owner; @@ -623,18 +587,23 @@ namespace GameCult.Caching // [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). + // The owned shapes are a closed list (Owner, Sequence, Map); every other shape is Leaf: one whole value with no element + // ids, replaced whole by a variant (q3 amended 2026-10-16). When in doubt the answer is Leaf. + // Leaf strings (an IEnumerable MessagePack writes whole), a type written whole (IsWrittenWhole: attribute- + // formatted, [DataContract], string-keyed), a struct or a class or interface with no [MessagePackObject] or + // [Union] and no collection shape (primitives, enums, float2, DateTime, ImmutableArray, ArraySegment, Memory, + // KeyValuePair, tuples, Lazy, unkeyed classes), and object, which is untyped on disk. // Map a class or interface implementing IDictionary<,> or IReadOnlyDictionary<,>. - // Sequence an array or an IEnumerable. - // Owner a class, interface or struct MessagePack serializes by key. A keyed struct is walked as an owner; it is - // never an element (IsElement), since a boxed copy cannot carry a minted id. + // Sequence an array or an IEnumerable class or interface. + // Owner a class, interface or struct MessagePack serializes by integer key. A keyed struct is walked as an owner; + // it is never an element (IsElement), since a boxed copy cannot carry a minted id. private static (Kind Kind, Type Inner) Classify(Type declared) => Classes.GetOrAdd(declared, static type => { type = Nullable.GetUnderlyingType(type) ?? type; if (type == typeof(string)) return (Kind.Leaf, type); + if (IsWrittenWhole(type)) + return (Kind.Leaf, type); if (IsKeyed(type)) return (Kind.Owner, type); if (type.IsValueType) @@ -646,6 +615,15 @@ namespace GameCult.Caching return (Kind.Leaf, type); }); + // A type MessagePack writes as one value the walk cannot look into: its own formatter ([MessagePackFormatter] on the type, + // inherited), [DataContract] on a type that is not [MessagePackObject] (the object resolver writes its members), or string + // keys (KeyAsPropertyName, or any [Key] with a string key). Such a type is Leaf, even when it is also enumerable. + private static bool IsWrittenWhole(Type type) => + type.IsDefined(typeof(MessagePackFormatterAttribute), true) || + (type.IsDefined(typeof(DataContractAttribute), true) && !type.IsDefined(typeof(MessagePackObjectAttribute), true)) || + type.GetCustomAttribute(true)?.KeyAsPropertyName == true || + AllInstanceMembers(type).Any(member => member.GetCustomAttribute(true)?.StringKey != null); + // A type MessagePack serializes by key: [MessagePackObject] or [Union]. private static bool IsKeyed(Type type) => type.IsDefined(typeof(MessagePackObjectAttribute), true) || type.IsDefined(typeof(UnionAttribute), true); diff --git a/tests/GameCult.Caching.Tests/ElementIdEngineTests.cs b/tests/GameCult.Caching.Tests/ElementIdEngineTests.cs index 29fc497b..edb1c1c7 100644 --- a/tests/GameCult.Caching.Tests/ElementIdEngineTests.cs +++ b/tests/GameCult.Caching.Tests/ElementIdEngineTests.cs @@ -1,7 +1,9 @@ #nullable enable using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; +using System.Reflection; using System.Text.RegularExpressions; using GameCult.Caching; using MessagePack; @@ -302,16 +304,27 @@ namespace GameCult.Caching.Tests Assert.That(Problems(typeof(HValueList)), Is.Empty, "a list of a non-keyed type is a value list"); } + // [MessagePackFormatter] on an element type, an owner, a collection class or a member makes that value one whole value: it + // has no element ids and is off the id path. The attribute on the id member itself is the one refusal that stays. [Test] - public void AFormatterAttributeOnTheIdPathIsRefused() + public void AFormatterAttributeMakesAWholeValueExceptOnTheId() { - Assert.That(Problems(typeof(HFormattedElement)).Single(), Does.Contain("FormattedElement").And.Contain("[MessagePackFormatter]")); - Assert.That(Problems(typeof(HFormattedList)).Single(), Does.Contain("Items").And.Contain("[MessagePackFormatter]")); Assert.That(Problems(typeof(HFormattedId)).Single(), Does.Contain("Id").And.Contain("[MessagePackFormatter]")); - Assert.That(Problems(typeof(HFormattedOwner)).Single(), Does.Contain("FormattedOwner").And.Contain("[MessagePackFormatter]")); - Assert.That(Problems(typeof(HFormattedMember)).Single(), Does.Contain("Inner").And.Contain("[MessagePackFormatter]")); + foreach (var (type, holder) in new (Type, object)[] + { + (typeof(HFormattedElement), new HFormattedElement { Items = { new FormattedElement() } }), + (typeof(HFormattedList), new HFormattedList { Items = { new EIded() } }), + (typeof(HFormattedOwner), new HFormattedOwner { Owner = { Items = { new EIded() } } }), + (typeof(HFormattedMember), new HFormattedMember { Inner = { Items = { new EIded() } } }), + (typeof(HFormattedDerived), new HFormattedDerived { Items = { new FormattedDerived() } }), + }) + { + Assert.That(Problems(type), Is.Empty, type.Name); + Assert.That(CultElementIds.Inspect(type).IdPath, Is.Empty, type.Name); + Assert.That(Assign(holder, "r", deterministic: false), Is.EqualTo(0), type.Name); + } + Assert.That(Problems(typeof(HFormattedField)), Is.Empty, "a formatted type with no list below it is a field"); - Assert.That(Problems(typeof(HFormattedDerived)).Single(), Does.Contain("FormattedDerived").And.Contain("[MessagePackFormatter]"), "the attribute is inherited"); } // The collection types between a member and its object list are written by some formatter too, so they sit on the id path. @@ -331,18 +344,19 @@ namespace GameCult.Caching.Tests } [Test] - public void AFormatterOnACollectionClassOfAnObjectListIsRefused() + public void AFormatterOnACollectionClassOfAnObjectListIsAWholeValue() { - Assert.That(Problems(typeof(HRoles)).Single(), Does.Contain("Roles").And.Contain("[MessagePackFormatter]")); + Assert.That(Problems(typeof(HRoles)), Is.Empty); + Assert.That(Assign(new HRoles { Items = { new EIded() } }, "r", deterministic: false), Is.EqualTo(0), "its elements carry no ids and are never minted"); Assert.That(Problems(typeof(HFormattedValueCollection)), Is.Empty, "a collection class of a non-keyed element is a value list"); } // MemberInfo.IsDefined ignores inherit for properties: an override sees its base's attribute only through Declares. [Test] - public void AnInheritedFormatterOnAnOverriddenMemberIsRefused() + public void AnInheritedFormatterOnAnOverriddenMemberIsHonoured() { Assert.That(Problems(typeof(HOverFormattedId)).Single(), Does.Contain("Id").And.Contain("[MessagePackFormatter]")); - Assert.That(Problems(typeof(OwnerOverFormatted)).Single(), Does.Contain("Items").And.Contain("[MessagePackFormatter]")); + Assert.That(Problems(typeof(OwnerOverFormatted)), Is.Empty, "the override inherits its base's formatter, so the member is a whole value"); Assert.That(Problems(typeof(HIdOverride)), Is.Empty, "a base [CultElementId] is the override's id"); var root = new HIdOverride { Items = { new IdOverride() } }; @@ -467,22 +481,28 @@ namespace GameCult.Caching.Tests Assert.That(Assign(arm, "r", deterministic: false), Is.EqualTo(1)); } - // MessagePack writes a non-public [Key] member only under AllowPrivate, and the element-id walk reads public members, so - // an object list a non-public member leads to could never be checked. It is refused; a non-public member that reaches no - // object list is nobody's business. + // MessagePack writes a non-public [Key] member only under AllowPrivate, so a non-public member of a type without it is nobody's + // id business; under AllowPrivate it is written, read and minted like any other member. [Test] - public void ANonPublicKeyedMemberOnTheWayToAnObjectListIsRefused() + public void ANonPublicMemberIsWalkedOnlyUnderAllowPrivate() { - foreach (var (type, member) in new[] + foreach (var type in new[] { typeof(HHiddenList), typeof(HHiddenOwner), typeof(HHiddenFromBase) }) { - (typeof(HHiddenList), "Stash"), (typeof(HHiddenOwner), "stash"), (typeof(HOptHidden), "stash"), (typeof(HHiddenFromBase), "stash"), - }) - Assert.That(Problems(type).Single(), Does.Contain(member).And.Contain("non-public"), type.Name); + Assert.That(Problems(type), Is.Empty, type.Name); + Assert.That(CultElementIds.Inspect(type).IdPath, Is.Empty, type.Name); + } + + Assert.That(Problems(typeof(HOptHidden)), Is.Empty); + Assert.That(CultElementIds.Inspect(typeof(HOptHidden)).IdPath, Is.EquivalentTo(new[] { typeof(HOptHidden), typeof(List), typeof(EIded) })); + var opt = new HOptHidden(); + opt.Peek.Add(new EIded()); + opt.Peek.Add(new EIded()); + Assert.That(Assign(opt, "r", deterministic: false), Is.EqualTo(2)); + Assert.That(opt.Peek.Select(item => item.Id), Has.All.Match("^[0-9a-f]{12}$")); Assert.That(Problems(typeof(HHiddenLeaf)), Is.Empty); Assert.That(CultElementIds.Inspect(typeof(HHiddenLeaf)).IdPath, Is.EquivalentTo(new[] { typeof(HHiddenLeaf), typeof(List), typeof(EIded) }), - "a hidden member adds nothing to the id path"); - Assert.That(CultElementIds.Inspect(typeof(HHiddenList)).IdPath, Is.Empty); + "a non-public member adds nothing to the id path"); } // MessagePack writes a [MessagePackObject] class that is only IEnumerable by its keys, so the walk reads it by key. A @@ -515,26 +535,24 @@ namespace GameCult.Caching.Tests Assert.That(CultElementIds.Holds(typeof(KBagIded), bag), Is.True); } - // The walk reads the members MessagePack writes: a public field or a property with a public getter, no [IgnoreMember]. - // Anything else that carries a [Key] is MessagePack's AllowPrivate business and, on the way to an object list, is refused. + // The walk reads the members MessagePack writes: without AllowPrivate a public field or a property with a public getter, with + // it every member up the base chain, the most derived [MessagePackObject] declaration deciding; no [IgnoreMember]. [Test] public void TheWalkReadsTheMembersMessagePackWrites() { Assert.That(Problems(typeof(PubIgn)), Is.Empty, "an ignored member is never written, so its list is nobody's business"); - Assert.That(Problems(typeof(ApIgn)), Is.Empty, "an ignored private member is not a hidden list"); - var held = Problems(typeof(PgHold)).Single(); - Assert.That(held, Does.Contain("Items").And.Contain("non-public")); + Assert.That(Problems(typeof(ApIgn)), Is.Empty, "an ignored private member is not written"); + Assert.That(Problems(typeof(PgHold)), Is.Empty, "a list behind a private getter is not written without AllowPrivate"); - foreach (var type in new[] { typeof(EIded), typeof(HHiddenLeaf), typeof(PubIgn), typeof(ApIgn), typeof(PgHold), typeof(MemberMix) }) + foreach (var type in new[] { typeof(EIded), typeof(HHiddenLeaf), typeof(PubIgn), typeof(ApIgn), typeof(PgHold), typeof(MemberMix), typeof(HOptHidden), typeof(ApDerived), typeof(NoApDerived) }) { - var written = MessagePackMembers(type, allowPrivate: false); - Assert.That(CultElementIds.WrittenKeyedMembers(type).Select(member => member.Name), Is.EquivalentTo(written), type.Name); - var readable = MessagePackMembers(type, allowPrivate: true); - Assert.That(CultElementIds.UnreadMembers(type).Select(member => member.Name), Is.EquivalentTo(readable.Except(written)), type.Name); + var allowPrivate = type.GetCustomAttribute(true)!.AllowPrivate; + Assert.That(CultElementIds.WrittenKeyedMembers(type).Select(member => member.Name), + Is.EquivalentTo(MessagePackMembers(type, allowPrivate)), type.Name); } - Assert.That(CultElementIds.UnreadMembers(typeof(MemberMix)).Select(member => member.Name), Is.EquivalentTo(new[] { "priv" })); - Assert.That(CultElementIds.UnreadMembers(typeof(PgHold)).Select(member => member.Name), Is.EquivalentTo(new[] { "Items" })); + Assert.That(CultElementIds.WrittenKeyedMembers(typeof(NoApDerived)).Select(member => member.Name), Is.EquivalentTo(new[] { "N" }), + "the derived declaration decides, so an AllowPrivate base adds nothing"); } // The names of the members MessagePack's own discovery reads for a type, by reflection into its internal type: the @@ -599,6 +617,69 @@ namespace GameCult.Caching.Tests } } + // Every shape the walk does not own is one whole value: no problem, off the id path, never minted into and never counted. + [Test] + public void AShapeTheWalkDoesNotOwnIsOneWholeValue() + { + const string Set = "aaaaaaaaaaaa"; + EIded E(string id) => new() { Id = id }; + var holders = new (Type Type, Func Make)[] + { + (typeof(Whole>), id => new Whole> { Value = ImmutableArray.Create(E(id)) }), + (typeof(Whole?>), id => new Whole?> { Value = ImmutableArray.Create(E(id)) }), + (typeof(Whole>), id => new Whole> { Value = new ArraySegment(new[] { E(id) }) }), + (typeof(Whole>), id => new Whole> { Value = new[] { E(id) } }), + (typeof(Whole>), id => new Whole> { Value = new[] { E(id) } }), + (typeof(Whole>>), id => new Whole>> { Value = new("k", new List { E(id) }) }), + (typeof(Whole<(List, int)>), id => new Whole<(List, int)> { Value = (new List { E(id) }, 1) }), + (typeof(Whole, int>>), id => new Whole, int>> { Value = Tuple.Create(new List { E(id) }, 1) }), + (typeof(Whole>>), id => new Whole>> { Value = new Lazy>(() => new List { E(id) }) }), + (typeof(Whole), id => new Whole { Value = new NkHold { Items = { E(id) } } }), + (typeof(Whole), id => new Whole { Value = new SkHold { Items = { E(id) } } }), + (typeof(Whole), id => new Whole { Value = new DcHold { Items = { E(id) } } }), + (typeof(HRoles), id => new HRoles { Items = { E(id) } }), + (typeof(HFormattedList), id => new HFormattedList { Items = { E(id) } }), + (typeof(HHiddenList), id => new HHiddenList()), + }; + foreach (var (type, make) in holders) + { + Assert.That(Problems(type), Is.Empty, type.ToString()); + Assert.That(CultElementIds.Inspect(type).IdPath, Is.Empty, type.ToString()); + Assert.That(CultElementIds.CanHoldIds(type), Is.False, type.ToString()); + Assert.That(Assign(make(string.Empty), "r", deterministic: false), Is.EqualTo(0), type.ToString()); + Assert.That(Assign(make(string.Empty), "r", deterministic: true), Is.EqualTo(0), type.ToString()); + Assert.That(CultElementIds.Holds(type, make(Set)), Is.False, type.ToString()); + } + + // Elements of a string-keyed type carry no id either, and an owned list of a bare element is still refused. + Assert.That(Problems(typeof(Whole>)), Is.Empty); + RefusesBare(typeof(Whole>)); + } + + // MessagePack writes an AllowPrivate type with every member up its base chain, and the most derived [MessagePackObject] + // declaration decides whether that is so. + [Test] + public void AnAllowPrivateTypeIsWalkedByTheMembersMessagePackWrites() + { + Assert.That(CultElementIds.WrittenKeyedMembers(typeof(ApDerived)).Select(member => member.Name), Is.EquivalentTo(new[] { "baseStash", "own", "N", "Pg" })); + Assert.That(CultElementIds.WrittenKeyedMembers(typeof(NoApDerived)).Select(member => member.Name), Is.EquivalentTo(new[] { "N" })); + + var options = GameCult.Caching.MessagePack.CultDocumentMessagePackSerialization.Options; + var held = new HOptHidden(); + held.Peek.Add(new EIded()); + Assert.That(Assign(held, "r", deterministic: false), Is.EqualTo(1)); + var back = MessagePackSerializer.Deserialize(MessagePackSerializer.Serialize(held, options), options); + Assert.That(back.Peek.Single().Id, Is.EqualTo(held.Peek.Single().Id), "the minted id of a private list survives a round trip"); + + Assert.That(Problems(typeof(HApPrivSet)), Is.Empty, "MessagePack reads an AllowPrivate type's private setter back"); + var privSet = new HApPrivSet { Items = { new EApPrivSet(), new EApPrivSet() } }; + Assert.That(Assign(privSet, "r", deterministic: false), Is.EqualTo(2)); + var round = MessagePackSerializer.Deserialize(MessagePackSerializer.Serialize(privSet, options), options); + Assert.That(round.Items.Select(item => item.Id), Is.EqualTo(privSet.Items.Select(item => item.Id))); + Assert.That(round.Items.Select(item => item.Id), Has.All.Match("^[0-9a-f]{12}$")); + Assert.That(Problems(typeof(HPrivSet)).Single(), Does.Contain("setter is not public"), "without AllowPrivate it is still refused"); + } + private static string?[] Ids(object holder) => holder switch { HInit init => init.Items.Select(item => item.Id).ToArray(), @@ -885,12 +966,24 @@ namespace GameCult.Caching.Tests #pragma warning disable CS0169, CS0414, CS0649 [MessagePackObject] public class HHiddenList { [Key(0)] private List Stash { get; set; } = new(); } [MessagePackObject] public class HHiddenOwner { [Key(0)] private HHasIds stash = new(); } - [MessagePackObject(AllowPrivate = true)] public class HOptHidden { [Key(0)] private List stash = new(); } + [MessagePackObject(AllowPrivate = true)] public class HOptHidden { [Key(0)] private List stash = new(); [IgnoreMember] public List Peek => stash; } [MessagePackObject] public class HHiddenBase { [Key(0)] private List stash = new(); } [MessagePackObject] public class HHiddenFromBase : HHiddenBase { [Key(1)] public int N { get; set; } } [MessagePackObject] public class HHiddenLeaf { [Key(0)] public List Items { get; set; } = new(); [Key(1)] private int secret = 7; } + [MessagePackObject] public class ApBase { [Key(0)] private List baseStash = new(); } + [MessagePackObject(AllowPrivate = true)] public class ApDerived : ApBase { [Key(1)] private List own = new(); [Key(2)] public int N { get; set; } [Key(3)] public List Pg { private get; set; } = new(); } + [MessagePackObject(AllowPrivate = true)] public class ApBase2 { [Key(0)] private List baseStash = new(); } + [MessagePackObject] public class NoApDerived : ApBase2 { [Key(1)] public int N { get; set; } } #pragma warning restore CS0169, CS0414, CS0649 + [MessagePackObject] public class Whole { [Key(0)] public T? Value { get; set; } } + public class NkHold { public List Items = new(); } + [MessagePackObject] public class SkHold { [Key("items")] public List Items = new(); } + [MessagePackObject] public class SkElem { [Key("x")] public string X { get; set; } = ""; } + [System.Runtime.Serialization.DataContract] public class DcHold { [System.Runtime.Serialization.DataMember(Order = 0)] public List Items = new(); } + [MessagePackObject(AllowPrivate = true)] public class EApPrivSet { [Key(0)] [CultElementId] public string? Id { get; private set; } } + [MessagePackObject] public class HApPrivSet { [Key(0)] public List Items { get; set; } = new(); } + [MessagePackObject] public class EDerivedText { [Key(0)] public string? Name { get; set; } [Key(1)] [CultElementId(nameof(Name))] public string Id { get; set; } = ""; } [MessagePackObject] public class HDerivedText { [Key(0)] public List Items { get; set; } = new(); } diff --git a/tests/GameCult.Caching.Tests/ElementIdTests.cs b/tests/GameCult.Caching.Tests/ElementIdTests.cs index 70201d19..32a84677 100644 --- a/tests/GameCult.Caching.Tests/ElementIdTests.cs +++ b/tests/GameCult.Caching.Tests/ElementIdTests.cs @@ -1,6 +1,7 @@ #nullable enable using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.IO; using System.Linq; using System.Reflection; @@ -356,6 +357,48 @@ namespace GameCult.Caching.Tests private static byte[] OldShapedReels(string label) => MessagePackSerializer.Serialize(new List { new() { Label = label, Marks = { new OldMark { Text = "x" }, new OldMark { Text = "y" } } } }); + // A holder the walk does not own (ImmutableArray here) is one whole value: its elements carry no element ids, so it never + // marks the store, and a variant replaces it whole (q3 amended 2026-10-16). + [Test] + public void AWholeListHolderNeverMarksTheStoreAndAVariantReplacesItWhole() + { + Assert.That(CultElementIds.CanHoldIds(typeof(IdWholeHolder)), Is.False); + var path = PathOf("whole-holder.cc"); + using (var cache = OpenWith(path, false, typeof(IdWholeHolder))) + { + cache.Commit(batch => batch.Upsert(typeof(IdWholeHolder), + new IdWholeHolder { Name = "base", Reels = ImmutableArray.Create(new IdReel { Label = "set", Id = HexA }, new IdReel { Label = "unset" }) }, + new CultRecordKey("base"))); + } + + Assert.That(HeaderOf(path), Is.EqualTo("cultcache.store.v1"), "ids inside a whole value mark nothing"); + + using (var fresh = OpenWith(path, false, typeof(IdWholeHolder))) + { + fresh.Commit(batch => batch.Upsert(typeof(IdWholeHolder), new IdWholeHolder { Name = "other" }, new CultRecordKey("other"))); + var back = fresh.Get(new CultRecordKey("base"))!; + Assert.That(back.Reels.Select(reel => reel.Id), Is.EqualTo(new[] { HexA, "" }), "set ids are plain values, unset ids stay unset"); + } + + Assert.That(HeaderOf(path), Is.EqualTo("cultcache.store.v1")); + using (var cache = OpenWith(path, false, typeof(IdWholeHolder))) + { + var replacement = ImmutableArray.Create(new IdReel { Label = "v", Id = "eeeeeeeeeeee" }); + cache.Commit(batch => batch.UpsertVariant(new CultRecordKey("v"), new CultRecordKey("base"), + new[] { cache.Override(nameof(IdWholeHolder.Reels), replacement) })); + Assert.That(cache.Get(new CultRecordKey("v"))!.Reels.Select(reel => reel.Id), Is.EqualTo(new[] { "eeeeeeeeeeee" }), "replaced whole"); + Assert.That(cache.Get(new CultRecordKey("base"))!.Reels, Has.Length.EqualTo(2), "the base is untouched"); + } + + Assert.That(HeaderOf(path), Is.EqualTo("cultcache.store.v2"), "a variant makes v2, and nothing in it holds element ids"); + using var again = OpenWith(path, false, typeof(IdWholeHolder)); + Assert.That(again.Get(new CultRecordKey("v"))!.Reels.Single().Id, Is.EqualTo("eeeeeeeeeeee")); + + var step = new CultVariantOverride(CultOverrideOp.Set, new[] { new CultPathStep(1, HexA) }, string.Empty, MessagePackSerializer.Serialize("x")); + var refused = Assert.Catch(() => again.Commit(batch => batch.UpsertVariant(new CultRecordKey("p"), new CultRecordKey("base"), new[] { step })))!; + Assert.That(refused.ToString(), Does.Contain("top-level Set overrides only")); + } + [Test] public void AStoreThatCanHoldElementIdsCarriesTheV3MarkerAndOneThatCannotStaysV1() { @@ -1287,6 +1330,14 @@ namespace GameCult.Caching.Tests [Key(1)] public int Depth { get; set; } } + [CultDocument("tests.element_id_whole_holder", "tests.element_id_whole_holder.v1")] + [MessagePackObject] + public sealed class IdWholeHolder + { + [Key(0)] [CultName] public string Name { get; set; } = ""; + [Key(1)] public ImmutableArray Reels { get; set; } = ImmutableArray.Empty; + } + [CultDocument("tests.element_id_deck", "tests.element_id_deck.v1")] [MessagePackObject] public sealed class IdDeck diff --git a/tests/GameCult.Caching.Tests/UnionArmGuardTests.cs b/tests/GameCult.Caching.Tests/UnionArmGuardTests.cs index 760bf9ca..48038c9c 100644 --- a/tests/GameCult.Caching.Tests/UnionArmGuardTests.cs +++ b/tests/GameCult.Caching.Tests/UnionArmGuardTests.cs @@ -381,9 +381,7 @@ namespace GameCult.Caching.Tests Assert.That(back.Bag.Items.Single().Id, Is.EqualTo("00000000000d")); } - // Every collection shape the walk reads is one MessagePack writes through a collection formatter, so none is refused. A - // [DataContract] list subclass is still written by the generic resolver; an object formatter the object resolver could also - // build (DcColl: a different instance) or fails to build (DcBad) is no mismatch. + // Every collection shape the walk reads is one MessagePack writes through a collection formatter, so none is refused. [Test] public void EveryCollectionShapeTheWalkReadsIsWrittenByACollectionFormatter() { @@ -398,7 +396,6 @@ namespace GameCult.Caching.Tests typeof(G>), typeof(G>), typeof(G>), typeof(G>), typeof(G>), typeof(G), typeof(G), typeof(G), - typeof(G), typeof(G), }) { if (holder != typeof(G)) @@ -407,22 +404,45 @@ namespace GameCult.Caching.Tests } } - // The attribute formatter is of no known family, so the door passes it as it passes a source-generated one; registration - // owns the attribute. + // One recogniser decides what the walk owns and the door only verifies it: the same types give no registration problem and a + // passing door. A type formatted by attribute or written by the object resolver is one whole value, off the id path. [Test] - public void AFormatterOfNoKnownFamilyPassesTheDoor() + public void OneRecogniserDecidesAndTheDoorAgrees() { - Assert.DoesNotThrow(() => CultDocumentMessagePackSerialization.OptionsForDocument(typeof(G))); - var problems = CultElementIds.Inspect(typeof(G)).Problems; - Assert.That(problems, Has.Count.EqualTo(1)); - Assert.That(problems.Single(), Does.Contain("FormattedRoles").And.Contain("declares [MessagePackFormatter]")); + foreach (var holder in new[] { typeof(G), typeof(G), typeof(G), typeof(G), typeof(G) }) + { + Assert.That(CultElementIds.Inspect(holder).Problems, Is.Empty, holder.ToString()); + Assert.DoesNotThrow(() => CultDocumentMessagePackSerialization.OptionsForDocument(holder), holder.ToString()); + } + + foreach (var whole in new[] { typeof(G), typeof(G), typeof(G), typeof(G) }) + Assert.That(CultElementIds.Inspect(whole).IdPath, Is.Empty, whole.ToString()); + var ap = CultElementIds.Inspect(typeof(G)).IdPath; + Assert.That(ap, Does.Contain(typeof(ApTrueHold)).And.Contain(typeof(List))); + var seq = new G { Value = new DcSeq { Items = { new FbArm() } } }; + Assert.That(CultElementIds.Plan(seq.GetType(), seq, "r", deterministic: false).Count, Is.EqualTo(0)); } + // MessagePack throws for a type, or StandardResolver has no formatter for it (an interface or abstract class that is only + // IEnumerable): such a type cannot be persisted, so the door refuses it with the same typed text as a throwing build. [Test] - public void ACollectionMessagePackWritesAsAnObjectIsRefused() + public void ATypeMessagePackCannotBuildIsATypedRefusal() { - var thrown = Assert.Throws(() => CultDocumentMessagePackSerialization.OptionsForDocument(typeof(G)))!; - Assert.That(thrown.Message, Does.Contain("MessagePack writes DcSeq as an object, but the element-id walk reads it as a collection")); + foreach (var (holder, name) in new[] { (typeof(G), nameof(IArms)), (typeof(G), nameof(AbsArms)) }) + { + var thrown = Assert.Throws(() => CultDocumentMessagePackSerialization.OptionsForDocument(holder), holder.ToString())!; + Assert.That(thrown.Message, Does.Contain(name).And.Contain("cannot build a formatter")); + } + } + + // A keyed type that enumerates lists is read by key and is on the id path, so the door checks it: MessagePack writes it as a + // collection, which loses the ids the walk would mint. + [Test] + public void AKeyedTypeThatEnumeratesListsIsCheckedByTheDoor() + { + Assert.That(CultElementIds.Inspect(typeof(G)).IdPath, Does.Contain(typeof(KLL))); + var thrown = Assert.Throws(() => CultDocumentMessagePackSerialization.OptionsForDocument(typeof(G)))!; + Assert.That(thrown.Message, Does.Contain($"writes {nameof(KLL)} as a collection")); } // A resolver that throws while asked about an id-path type is a typed refusal, whether it is the AllowPrivate formatter @@ -519,6 +539,16 @@ namespace GameCult.Caching.Tests public IEnumerator GetEnumerator() => Items.GetEnumerator(); System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); } + public interface IArms : IEnumerable { } + public abstract class AbsArms : IEnumerable + { + public abstract IEnumerator GetEnumerator(); + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); + } + [MessagePackObject] public sealed class KLL : List> { [Key(0)] public int N; } +#pragma warning disable CS0169, CS0414, CS0649 + [MessagePackObject(AllowPrivate = true)] public sealed class ApTrueHold { [Key(0)] private List items = new(); } +#pragma warning restore CS0169, CS0414, CS0649 [DataContract] public sealed class DcColl : List { [DataMember(Order = 0)] public int N; } [DataContract] public sealed class DcBad : List { [DataMember(Order = 0)] public int A; [DataMember(Order = 0)] public int B; }