From 70ffa2af9a201b5ab127c47d24fbdfc1a6a05ce1 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Thu, 15 Oct 2026 01:32:25 +0200 Subject: [PATCH] Element ids: Classify keeps only the string special case; tests pin nullable structs, unkeyed structs, inheritance and an AllowPrivate formatter that cannot be built Stryker found the primitive, enum and object tests in Classify redundant with the value-type and unkeyed branches, so they are gone. The new tests kill the survivors that were behaviour: a nullable keyed struct walked as its struct, an unannotated struct treated as a value, [MessagePackObject] reaching a subclass, and a claim on a type StandardResolverAllowPrivate cannot format. Co-Authored-By: Claude Sonnet 5.5 --- src/GameCult.Caching/CultElementIds.cs | 7 ++-- .../ElementIdEngineTests.cs | 20 +++++++++ .../UnionArmGuardTests.cs | 41 +++++++++++++++++++ 3 files changed, 65 insertions(+), 3 deletions(-) diff --git a/src/GameCult.Caching/CultElementIds.cs b/src/GameCult.Caching/CultElementIds.cs index fcbf8079..c27fef5f 100644 --- a/src/GameCult.Caching/CultElementIds.cs +++ b/src/GameCult.Caching/CultElementIds.cs @@ -567,8 +567,9 @@ namespace GameCult.Caching // 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. - // Leaf a value written whole that holds no element list: primitives, enums, strings, object (untyped on disk), - // a struct with no [MessagePackObject] or [Union], and a class with neither. + // 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). // 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 @@ -576,7 +577,7 @@ namespace GameCult.Caching private static (Kind Kind, Type Inner) Classify(Type declared) => Classes.GetOrAdd(declared, static type => { type = Nullable.GetUnderlyingType(type) ?? type; - if (type.IsPrimitive || type.IsEnum || type == typeof(string) || type == typeof(object)) + if (type == typeof(string)) return (Kind.Leaf, type); if (type.IsValueType) return IsKeyed(type) ? (Kind.Owner, type) : (Kind.Leaf, type); diff --git a/tests/GameCult.Caching.Tests/ElementIdEngineTests.cs b/tests/GameCult.Caching.Tests/ElementIdEngineTests.cs index 4e9bb460..45ab621f 100644 --- a/tests/GameCult.Caching.Tests/ElementIdEngineTests.cs +++ b/tests/GameCult.Caching.Tests/ElementIdEngineTests.cs @@ -402,6 +402,21 @@ namespace GameCult.Caching.Tests Is.EquivalentTo(new[] { typeof(HStructValues), typeof(List), typeof(EIded) }), "a list of keyed structs is a value list"); var values = new HStructValues { Structs = { new SValue { N = 1 } }, Objects = { new EIded() } }; Assert.That(Assign(values, "r", deterministic: false), Is.EqualTo(1)); + + var nullable = new HNullableStructIded { Hold = new SHoldIded { Items = new List { new EIded() } } }; + Assert.That(Assign(nullable, "r", deterministic: false), Is.EqualTo(1), "a nullable struct is walked as its struct"); + Assert.That(CultElementIds.Holds(typeof(HNullableStructIded), nullable), Is.True); + } + + // A type is an owner or an element only when MessagePack serializes it by key ([MessagePackObject], inherited, or + // [Union]). A struct without either is a value however many [Key] members it carries. + [Test] + public void OnlyAKeyedTypeIsWalked() + { + Assert.That(Problems(typeof(HUnkeyedStruct)), Is.Empty); + Assert.That(CultElementIds.Inspect(typeof(HUnkeyedStruct)).IdPath, Is.Empty); + Assert.That(Problems(typeof(HInheritedBare)).Single(), Does.Contain("InheritedBare").And.Contain("[CultElementId]"), + "[MessagePackObject] reaches a subclass through inheritance"); } // Every dictionary interface MessagePack writes as a map is walked by one path, and the minted path text is the one a @@ -615,6 +630,11 @@ namespace GameCult.Caching.Tests [MessagePackObject] public struct SHold { [Key(0)] public List Items; } [MessagePackObject] public struct SHoldIded { [Key(0)] public List Items; } [MessagePackObject] public struct SValue { [Key(0)] public int N; } + public struct UnkeyedS { [Key(0)] public List Items; } + [MessagePackObject] public class HUnkeyedStruct { [Key(0)] public UnkeyedS Value; } + public class InheritedBare : EBare { } + [MessagePackObject] public class HInheritedBare { [Key(0)] public List Items { get; set; } = new(); } + [MessagePackObject] public class HNullableStructIded { [Key(0)] public SHoldIded? Hold; } [MessagePackObject] public class HStruct { [Key(0)] public SHold Hold; } [MessagePackObject] public class HStructList { [Key(0)] public List Holds { get; set; } = new(); } [MessagePackObject] public class HNullableStruct { [Key(0)] public SHold? Hold; } diff --git a/tests/GameCult.Caching.Tests/UnionArmGuardTests.cs b/tests/GameCult.Caching.Tests/UnionArmGuardTests.cs index 7c98927b..e6a11309 100644 --- a/tests/GameCult.Caching.Tests/UnionArmGuardTests.cs +++ b/tests/GameCult.Caching.Tests/UnionArmGuardTests.cs @@ -357,6 +357,46 @@ namespace GameCult.Caching.Tests Assert.That(Convert.ToHexString(plainBytes), Is.EqualTo(Convert.ToHexString(MessagePackSerializer.Serialize(plain, CultDocumentMessagePackSerialization.Options)))); } + // StandardResolverAllowPrivate cannot always build a formatter (a private readonly [Key] field has no constructor to + // fill it). That is not an escape from the claim check: a resolver formatting such a type is refused like any other. + [Test] + public void AFormatterThatStandardResolverAllowPrivateCannotBuildStillClaims() + { + var registry = CultDocumentRegistry.ForTypes(new[] { typeof(ApRoDoc) }); + var doc = new ApRoDoc { Id = "r1", Items = { new ApRoArm { Id = "00000000000c" } } }; + + var thrown = Assert.Throws(() => CultDocumentMessagePackSerialization.SerializeUntyped(doc, typeof(ApRoDoc), registry))!; + + Assert.That(thrown.Message, Does.Contain(nameof(ListClaimResolver)).And.Contain(nameof(ApRoArm))); + } + + [MessagePackObject] + public sealed class ApRoArm + { + [Key(0)] [CultElementId] public string Id = string.Empty; + [Key(1)] private readonly int fixedValue = 3; + [IgnoreMember] public int FixedValue => fixedValue; + } + + [CultDocument("tests.ap_ro_doc", "tests.ap_ro_doc.v1")] + [MessagePackObject] + public sealed class ApRoDoc + { + [Key(0)] [CultName] public string Id = string.Empty; + [Key(1)] public List Items = new(); + } + + private sealed class ApRoFormatter : IMessagePackFormatter + { + public void Serialize(ref MessagePackWriter writer, ApRoArm value, MessagePackSerializerOptions options) => writer.WriteNil(); + + public ApRoArm Deserialize(ref MessagePackReader reader, MessagePackSerializerOptions options) + { + reader.Skip(); + return new ApRoArm(); + } + } + [MessagePackObject] public sealed class ApArm { @@ -457,6 +497,7 @@ namespace GameCult.Caching.Tests if (typeof(T) == typeof(List)) return (IMessagePackFormatter)(object)new ValuesFormatter(); if (typeof(T) == typeof(List)) return (IMessagePackFormatter)(object)new PointsFormatter(); if (typeof(T) == typeof(FbArm)) return global::MessagePack.Resolvers.StandardResolver.Instance.GetFormatter(); + if (typeof(T) == typeof(ApRoArm)) return (IMessagePackFormatter)(object)new ApRoFormatter(); if (typeof(T) == typeof(ApArm) || typeof(T) == typeof(ApDoc) || typeof(T) == typeof(List) || typeof(T) == typeof(ApPlainArm) || typeof(T) == typeof(ApPlainDoc) || typeof(T) == typeof(List)) return global::MessagePack.Resolvers.StandardResolverAllowPrivate.Instance.GetFormatter();