GameCult/CultLib
Watch
9
0
Fork
You've already forked CultLib
 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions

Make registry member visibility follow MessagePack's

Browse source 
The reflective registry now accepts what MessagePack 3.1.7 round-trips for a
[CultDocument] and refuses what it would silently lose or throw on:

- A property is writable when its setter is public, or any setter (private,
  internal, init-only) under AllowPrivate. A public getter with a non-public
  setter on a type without AllowPrivate is refused: MessagePack wrote it and
  loaded null. Private and init-only setters on AllowPrivate types, previously
  over-rejected, are accepted and round-trip.
- A [Key] on a non-public member of a type without AllowPrivate is refused;
  MessagePack skipped it silently.
- A class with no constructor MessagePack can pick is refused (the primary
  constructor case). The check mirrors DynamicObjectResolver as probed: the
  [SerializationConstructor] if marked, else any public (any, under
  AllowPrivate) constructor whose parameters take the keyed members by
  position with an assignable type.

Tests: InternalSetterWithoutAllowPrivate, KeyedNonPublicMemberWithoutAllowPrivate,
AllowPrivatePrimaryConstructor, AllowPrivateSettersRoundTrip (byte-matched
against MessagePackSerializer); InheritedPrivateSetter carries the new message.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Meta Vens 2026-09-14 00:03:23 +02:00
commit 9139bf3195
3 changed files with 136 additions and 21 deletions
Download patch file Download diff file Expand all files Collapse all files

21
src/GameCult.Caching/Contracts/cultcache-schema-compatibility.md
View file

@ -33,15 +33,24 @@ A member's slot is its MessagePack `[Key(n)]` integer. MessagePack's `[Key]` and
`MessagePack.Annotations`. Payloads are written by `MessagePackSerializer`, so
the registry refuses a document type without `[MessagePackObject]` and a
non-public document type without `[MessagePackObject(AllowPrivate = true)]`.
`AllowPrivate` makes MessagePack read non-public members too, so every
non-public instance field or property of such a type must be `[IgnoreMember]`;
the registry persists public members only. A
member's type name is the CLR full name; a nested type is written `Outer+Inner`.
A member's type name is the CLR full name; a nested type is written `Outer+Inner`.
The registry also refuses a persisted member without `[Key]`, a string
`[Key]`, two members sharing a slot (including `new`-hidden members), a hidden
persisted member, an override whose `[Key]` or `[IgnoreMember]` differs from its
base declaration's (MessagePack reads the base declaration), and a persisted
member without a public or internal setter.
base declaration's (MessagePack reads the base declaration), and a readonly
persisted field.
Visibility follows MessagePack's rule: the registry accepts what MessagePack
round-trips and refuses what it would silently lose. The registry persists
public fields and properties only. Without `AllowPrivate`, a persisted property
needs a public setter, and a `[Key]` on a non-public member is refused because
MessagePack skips that member. With `AllowPrivate`, private, internal and
init-only setters are all writable. But MessagePack then also reads non-public
members, so every non-public instance field or property must be
`[IgnoreMember]`. A class also needs a constructor MessagePack can call. That
is a parameterless one (public unless `AllowPrivate`), or one whose parameters
take the members at `[Key(0)]`, `[Key(1)]`, ... in order. A primary constructor
usually has neither.
## Which schema a record carries

63
src/GameCult.Caching/CultCache.cs
View file

@ -838,7 +838,11 @@ namespace GameCult.Caching
private static IReadOnlyList<PersistedMember> DiscoverMembers(Type type)
{
const BindingFlags declared = BindingFlags.DeclaredOnly | BindingFlags.Instance | BindingFlags.Public;
var messagePackObject = type.GetCustomAttribute<MessagePackObjectAttribute>(true);
var allowPrivate = messagePackObject?.AllowPrivate == true;
var candidates = new List<(MemberInfo Member, Type MemberType, int Depth, bool Writable, PropertyInfo? DivergentRoot)>();
// Keyed get-only properties: MessagePack writes them and fills them through a constructor parameter only.
var constructorOnly = new List<(Type MemberType, int Slot)>();
var seenRoots = new HashSet<(Type, string)>();
var depth = 0;
for (var current = type; current != null && current != typeof(object); current = current.BaseType, depth++)
@ -855,8 +859,14 @@ namespace GameCult.Caching
continue;
var chain = OverrideChain(property).ToArray();
var setter = chain.Select(declaration => declaration.SetMethod).FirstOrDefault(method => method != null);
if (chain.All(declaration => declaration.GetMethod == null) || setter == null)
if (chain.All(declaration => declaration.GetMethod == null))
continue;
if (setter == null)
{
if (!IsIgnored(property) && GetKeyValue(property) is int constructorSlot)
constructorOnly.Add((property.PropertyType, constructorSlot));
continue;
}
// MessagePack's resolver reads the base declaration's [Key]/[IgnoreMember]; an override that changes them is refused.
var root = chain[chain.Length - 1];
var divergentRoot = chain.Any(declaration =>
@ -865,30 +875,33 @@ namespace GameCult.Caching
: null;
if (divergentRoot == null && IsIgnored(property))
continue;
candidates.Add((property, property.PropertyType, depth, IsWritable(setter), divergentRoot));
// MessagePack's own rule: it calls a setter that is public, or any setter (init-only included) under AllowPrivate.
candidates.Add((property, property.PropertyType, depth, setter.IsPublic || allowPrivate, divergentRoot));
}
}
var rejections = new List<string>();
var messagePackObject = type.GetCustomAttribute<MessagePackObjectAttribute>(true);
if (messagePackObject == null)
rejections.Add(NotMessagePackObjectMessage(type.Name));
else if (!type.IsVisible && !messagePackObject.AllowPrivate)
else if (!type.IsVisible && !allowPrivate)
rejections.Add(NotVisibleMessage(type.Name));
else if (messagePackObject.AllowPrivate)
else
{
// AllowPrivate makes MessagePack read non-public members; the registry persists public members only.
// AllowPrivate makes MessagePack read non-public members, which the registry does not persist; without it MessagePack
// silently skips a keyed non-public member.
const BindingFlags nonPublic = BindingFlags.DeclaredOnly | BindingFlags.Instance | BindingFlags.NonPublic;
for (var current = type; current != null && current != typeof(object); current = current.BaseType)
{
foreach (var member in current.GetFields(nonPublic).Cast<MemberInfo>().Concat(current.GetProperties(nonPublic)))
{
if (!member.IsDefined(typeof(CompilerGeneratedAttribute)) && !IsIgnored(member))
if (allowPrivate && !member.IsDefined(typeof(CompilerGeneratedAttribute)) && !IsIgnored(member))
rejections.Add(UnmarkedNonPublicMemberMessage(type.Name, Qualified(member)));
else if (!allowPrivate && GetKeyValue(member) != null && !IsIgnored(member))
rejections.Add(KeyedNonPublicMemberMessage(type.Name, Qualified(member)));
}
}
}
var keyed =new List<(MemberInfo Member, Type MemberType, int Slot)>();
var keyed = new List<(MemberInfo Member, Type MemberType, int Slot)>();
foreach (var candidate in candidates.OrderBy(candidate => candidate.Depth).ThenBy(candidate => candidate.Member.Name, StringComparer.Ordinal))
{
var key = GetKeyValue(candidate.Member);
@ -901,11 +914,16 @@ namespace GameCult.Caching
else
{
if (!candidate.Writable)
rejections.Add(NotWritableMessage(type.Name, candidate.Member.Name));
rejections.Add(candidate.Member is FieldInfo
? NotWritableMessage(type.Name, candidate.Member.Name)
: NonPublicSetterMessage(type.Name, candidate.Member.Name));
keyed.Add((candidate.Member, candidate.MemberType, slot));
}
}
if (messagePackObject != null && !type.IsValueType && !HasMessagePackConstructor(type, allowPrivate, keyed.Select(entry => (entry.MemberType, entry.Slot)).Concat(constructorOnly)))
rejections.Add(NoConstructorMessage(type.Name));
foreach (var group in keyed.GroupBy(entry => entry.Slot).Where(group => group.Count() > 1).OrderBy(group => group.Key))
{
var pair = group.Take(2).ToArray();
@ -947,9 +965,19 @@ namespace GameCult.Caching
}
}
private static bool IsWritable(MethodInfo setter) =>
(setter.IsPublic || setter.IsAssembly || setter.IsFamilyOrAssembly) &&
!setter.ReturnParameter.GetRequiredCustomModifiers().Any(modifier => modifier.FullName == "System.Runtime.CompilerServices.IsExternalInit");
// MessagePack's DynamicObjectResolver: the [SerializationConstructor] if one is marked, otherwise any public constructor (any
// constructor under AllowPrivate) whose every parameter takes the keyed member at its position (slot == parameter index).
private static bool HasMessagePackConstructor(Type type, bool allowPrivate, IEnumerable<(Type MemberType, int Slot)> members)
{
var slots = new Dictionary<int, Type>();
foreach (var member in members)
slots.TryAdd(member.Slot, member.MemberType);
var constructors = type.GetConstructors(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
var marked = constructors.Where(constructor => constructor.IsDefined(typeof(SerializationConstructorAttribute))).ToArray();
return (marked.Length > 0 ? marked : constructors.Where(constructor => allowPrivate || constructor.IsPublic)).Any(constructor =>
constructor.GetParameters().All(parameter =>
slots.TryGetValue(parameter.Position, out var memberType) && parameter.ParameterType.IsAssignableFrom(memberType)));
}
private static string Qualified(MemberInfo member) => member.DeclaringType!.Name.Split('`')[0] + "." + member.Name;
@ -969,7 +997,16 @@ namespace GameCult.Caching
$"Cult document {documentTypeName} member {memberName} has a string [Key]; string keys are not supported; use integer [Key(n)].";
private static string NotWritableMessage(string documentTypeName, string memberName) =>
$"Cult document {documentTypeName} member {memberName} is not writable; a persisted member needs a non-readonly field or a public or internal set accessor.";
$"Cult document {documentTypeName} member {memberName} is a readonly field; a persisted field must not be readonly.";
private static string NonPublicSetterMessage(string documentTypeName, string memberName) =>
$"Cult document {documentTypeName} member {memberName} has a non-public set accessor; MessagePack writes it but never reads it back without AllowPrivate, so make the setter public or add [MessagePackObject(AllowPrivate = true)].";
private static string KeyedNonPublicMemberMessage(string documentTypeName, string memberName) =>
$"Cult document {documentTypeName} member {memberName} is non-public; MessagePack skips it silently, so [Key] on a non-public member requires [MessagePackObject(AllowPrivate = true)].";
private static string NoConstructorMessage(string documentTypeName) =>
$"Cult document {documentTypeName} has no constructor MessagePack can call; add a parameterless constructor (public unless AllowPrivate), or one whose parameters take the [Key(0)], [Key(1)], ... members in order.";
private static string DuplicateSlotMessage(string documentTypeName, string first, string second, int slot) =>
$"Cult document {documentTypeName} members {first} and {second} share [Key({slot})]; every persisted member needs a distinct [Key(n)].";

73
tests/GameCult.Caching.Tests/DocumentShapeTests.cs
View file

@ -15,7 +15,8 @@ namespace GameCult.Caching.Tests
// so a refused shape cannot break a registry built from the loaded assemblies.
public class DocumentShapeTests
{
private const string DivergentOverride = "overrides {0} with a different [Key] or [IgnoreMember]; MessagePack reads the base declaration's, so an override must repeat or omit them.";
private const string NonPublicSetter = "has a non-public set accessor; MessagePack writes it but never reads it back without AllowPrivate, so make the setter public or add [MessagePackObject(AllowPrivate = true)].";
private const string DivergentOverride ="overrides {0} with a different [Key] or [IgnoreMember]; MessagePack reads the base declaration's, so an override must repeat or omit them.";
private static readonly ModuleBuilder Rejected = AssemblyBuilder
.DefineDynamicAssembly(new AssemblyName("RejectedShapes"), AssemblyBuilderAccess.Run)
@ -169,7 +170,52 @@ namespace GameCult.Caching.Tests
Property(baseType, "Name", 0, MethodAttributes.Private, Key(0));
var type = Document("PrivSetDerived", baseType.CreateType());
Property(type, "X", 0, MethodAttributes.Public, Key(1));
Reject(type.CreateType()!, "Cult document PrivSetDerived member Name is not writable; a persisted member needs a non-readonly field or a public or internal set accessor.");
Reject(type.CreateType()!, "Cult document PrivSetDerived member Name " + NonPublicSetter);
}
// Without AllowPrivate MessagePack writes Name through the public getter and loads it back as null.
[Test]
public void InternalSetterWithoutAllowPrivate()
{
var type = Document("InternalSetter");
Property(type, "Name", 0, MethodAttributes.Assembly, Key(0));
Reject(type.CreateType()!, "Cult document InternalSetter member Name " + NonPublicSetter);
}
// Without AllowPrivate MessagePack neither writes nor reads a non-public member, [Key] or not.
[Test]
public void KeyedNonPublicMemberWithoutAllowPrivate()
{
var type = Document("KeyedPrivate");
Field(type, "Name", typeof(string), Key(0));
type.DefineField("_secret", typeof(int), FieldAttributes.Private).SetCustomAttribute(Key(1));
Reject(type.CreateType()!, "Cult document KeyedPrivate member KeyedPrivate._secret is non-public; MessagePack skips it silently, so [Key] on a non-public member requires [MessagePackObject(AllowPrivate = true)].");
}
// The shape of `class PrimaryCtor(int p)`: MessagePack throws "can't find matched constructor" on first serialize.
[Test]
public void AllowPrivatePrimaryConstructor()
{
var type = Rejected.DefineType("PrimaryCtor", TypeAttributes.NotPublic);
type.SetCustomAttribute(new CustomAttributeBuilder(
typeof(CultDocumentAttribute).GetConstructor(new[] { typeof(string), typeof(string) })!, new object[] { "shape.primary_ctor", "shape.primary_ctor.v1" }));
type.SetCustomAttribute(new CustomAttributeBuilder(
typeof(MessagePackObjectAttribute).GetConstructor(new[] { typeof(bool) })!, new object[] { false },
new[] { typeof(MessagePackObjectAttribute).GetProperty(nameof(MessagePackObjectAttribute.AllowPrivate))! }, new object[] { true }));
Field(type, "Name", typeof(string), Key(0));
var il = type.DefineConstructor(MethodAttributes.Public, CallingConventions.HasThis, new[] { typeof(int) }).GetILGenerator();
il.Emit(OpCodes.Ldarg_0);
il.Emit(OpCodes.Call, typeof(object).GetConstructor(Type.EmptyTypes)!);
il.Emit(OpCodes.Ret);
Reject(type.CreateType()!, "Cult document PrimaryCtor has no constructor MessagePack can call; add a parameterless constructor (public unless AllowPrivate), or one whose parameters take the [Key(0)], [Key(1)], ... members in order.");
}
[Test]
public void AllowPrivateSettersRoundTrip()
{
Assert.That(Convert.ToHexString(Accept(PrivateSetterNote.Make("n"))), Is.EqualTo("91A16E"));
Assert.That(Convert.ToHexString(Accept(new InitOnlyNote { Name = "n" })), Is.EqualTo("91A16E"));
Assert.That(Convert.ToHexString(Accept(new InternalSetterNote { Name = "n" })), Is.EqualTo("91A16E"));
}
[Test]
@ -272,6 +318,29 @@ namespace GameCult.Caching.Tests
[Key(1)] public int Count;
}
[CultDocument("shape.private_setter", "shape.private_setter.v1")]
[MessagePackObject(AllowPrivate = true)]
internal sealed class PrivateSetterNote
{
[Key(0)] public string Name { get; private set; } = "";
public static PrivateSetterNote Make(string name) => new() { Name = name };
}
[CultDocument("shape.init_only", "shape.init_only.v1")]
[MessagePackObject(AllowPrivate = true)]
internal sealed class InitOnlyNote
{
[Key(0)] public string Name { get; init; } = "";
}
[CultDocument("shape.internal_setter", "shape.internal_setter.v1")]
[MessagePackObject(AllowPrivate = true)]
internal sealed class InternalSetterNote
{
[Key(0)] public string Name { get; internal set; } = "";
}
[MessagePackObject]
public class OvrSameBase
{