From 30ee4f8a49fcf2215cd459df609ff8f0441a7696 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Mon, 23 Nov 2020 10:00:04 -0700 Subject: [PATCH] Avoid `MissingMethodException` on generic classes with `init` property setters Fixes #1134 as much as we can while .NET has the underlying bug. When .NET 6 ships with the fix, we can add a .NET 6 target that re-allows setting `init` property setters from the `DynamicObjectResolver`. --- README.md | 20 +++++++++++ .../Resolvers/DynamicObjectResolver.cs | 10 ++++-- .../ShareTests/DynamicObjectResolverTests.cs | 36 +++++++++++++++++-- 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 454f47927..8e36e79da 100644 --- a/README.md +++ b/README.md @@ -404,6 +404,8 @@ public struct Point } ``` +### C# 9 `record` types + C# 9.0 record with primary constructor is similar immutable object, also supports serialize/deserialize. ```csharp @@ -412,8 +414,26 @@ C# 9.0 record with primary constructor is similar immutable object, also support // use property: to set KeyAttribute [MessagePackObject] public record Point([property:Key(0)] int X, [property: Key(1)] int Y); + +// Or use explicit properties +[MessagePackObject] +public record Person +{ + [Key(0)] + public string FirstName { get; init; } + + [Key(1)] + public string LastName { get; init; } +} ``` +### C# 9 `init` property setter limitations + +When using `init` property setters in _generic_ classes, [a CLR bug](https://github.com/neuecc/MessagePack-CSharp/issues/1134) prevents our most efficient code generation from invoking the property setter. +As a result, you should avoid using `init` on property setters in generic classes when using the public-only `DynamicObjectResolver`/`StandardResolver`. + +When using the `DynamicObjectResolverAllowPrivate`/`StandardResolverAllowPrivate` resolver the bug does not apply and you may use `init` without restriction. + ## Serialization Callback Objects implementing the `IMessagePackSerializationCallbackReceiver` interface will received `OnBeforeSerialize` and `OnAfterDeserialize` calls during serialization/deserialization. diff --git a/src/MessagePack.UnityClient/Assets/Scripts/MessagePack/Resolvers/DynamicObjectResolver.cs b/src/MessagePack.UnityClient/Assets/Scripts/MessagePack/Resolvers/DynamicObjectResolver.cs index 73af68261..6b0cbb71c 100644 --- a/src/MessagePack.UnityClient/Assets/Scripts/MessagePack/Resolvers/DynamicObjectResolver.cs +++ b/src/MessagePack.UnityClient/Assets/Scripts/MessagePack/Resolvers/DynamicObjectResolver.cs @@ -1739,11 +1739,14 @@ public static ObjectSerializationInfo CreateOrNull(Type type, bool forceStringKe MethodInfo getMethod = property.GetGetMethod(true); MethodInfo setMethod = property.GetSetMethod(true); + // init property setters only work with DynamicMethods: https://github.com/neuecc/MessagePack-CSharp/issues/1134 + bool isInit = setMethod?.ReturnParameter.GetRequiredCustomModifiers().Any(modifierType => modifierType.FullName == "System.Runtime.CompilerServices.IsExternalInit") ?? false; + member = new EmittableMember { PropertyInfo = property, IsReadable = (getMethod != null) && (allowPrivate || getMethod.IsPublic) && !getMethod.IsStatic, - IsWritable = (setMethod != null) && (allowPrivate || setMethod.IsPublic) && !setMethod.IsStatic, + IsWritable = (setMethod != null) && (allowPrivate || (setMethod.IsPublic && !isInit)) && !setMethod.IsStatic, StringKey = firstMemberByName ? item.Name : $"{item.DeclaringType.FullName}.{item.Name}", }; } @@ -1817,11 +1820,14 @@ public static ObjectSerializationInfo CreateOrNull(Type type, bool forceStringKe MethodInfo getMethod = item.GetGetMethod(true); MethodInfo setMethod = item.GetSetMethod(true); + // init property setters only work with DynamicMethods: https://github.com/neuecc/MessagePack-CSharp/issues/1134 + bool isInit = setMethod?.ReturnParameter.GetRequiredCustomModifiers().Any(modifierType => modifierType.FullName == "System.Runtime.CompilerServices.IsExternalInit") ?? false; + var member = new EmittableMember { PropertyInfo = item, IsReadable = (getMethod != null) && (allowPrivate || getMethod.IsPublic) && !getMethod.IsStatic, - IsWritable = (setMethod != null) && (allowPrivate || setMethod.IsPublic) && !setMethod.IsStatic, + IsWritable = (setMethod != null) && (allowPrivate || (setMethod.IsPublic && !isInit)) && !setMethod.IsStatic, }; if (!member.IsReadable && !member.IsWritable) { diff --git a/src/MessagePack.UnityClient/Assets/Scripts/Tests/ShareTests/DynamicObjectResolverTests.cs b/src/MessagePack.UnityClient/Assets/Scripts/Tests/ShareTests/DynamicObjectResolverTests.cs index b317cee99..4ccbf3f4c 100644 --- a/src/MessagePack.UnityClient/Assets/Scripts/Tests/ShareTests/DynamicObjectResolverTests.cs +++ b/src/MessagePack.UnityClient/Assets/Scripts/Tests/ShareTests/DynamicObjectResolverTests.cs @@ -314,11 +314,32 @@ public void DefaultValueIntKeyStructWithExplicitConstructorSetPropertyTest() #if NET5_0 [Fact] - public void RoundtripGenericClass() + public void RoundtripGenericClass_StandardResolverConsidersInitPropertyReadOnly() { var person = new GenericPerson { Name = "bob" }; - byte[] msgpack = MessagePackSerializer.Serialize(person, MessagePackSerializerOptions.Standard); - var deserialized = MessagePackSerializer.Deserialize>(msgpack, MessagePackSerializerOptions.Standard); + var options = StandardResolver.Options; + byte[] msgpack = MessagePackSerializer.Serialize(person, options); + var deserialized = MessagePackSerializer.Deserialize>(msgpack, options); + Assert.Null(deserialized.Name); + } + + [Fact] + public void RoundtripGenericClass_StandardResolverWorksWithDeserializingCtor() + { + var person = new GenericPersonWithCtor("bob"); + var options = StandardResolver.Options; + byte[] msgpack = MessagePackSerializer.Serialize(person, options); + var deserialized = MessagePackSerializer.Deserialize>(msgpack, options); + Assert.Equal(person.Name, deserialized.Name); + } + + [Fact] + public void RoundtripGenericClass_AllowPrivateStandardResolver() + { + var person = new GenericPerson { Name = "bob" }; + var options = StandardResolverAllowPrivate.Options; + byte[] msgpack = MessagePackSerializer.Serialize(person, options); + var deserialized = MessagePackSerializer.Deserialize>(msgpack, options); Assert.Equal(person.Name, deserialized.Name); } #endif @@ -616,6 +637,15 @@ public class GenericPerson [Key(0)] public string Name { get; init; } } + + [MessagePackObject] + public class GenericPersonWithCtor + { + public GenericPersonWithCtor(string name) => this.Name = name; + + [Key(0)] + public string Name { get; init; } + } #endif } }