Feature request: constructor injection

[nzbart]

Thanks for this really nice implementation of MessagePack!

We use a lot of immutable objects in our code, and these are currently not very compatible with this implementation. It would be nice if the serializer could detect the possibility of deserializing an object via the constructor if there are no settable properties. Current, this implementation throws the following exception in this scenario:

Cannot serialize type 'ConstructorInjectionDemo' because it does not have any serializable fields nor properties

I've written a sample test that demonstrates the feature I'm proposing here:

using System.IO;
using MsgPack.Serialization;
using NUnit.Framework;

public class ConstructorInjectionDemo
{
    public string Value { get; private set; }

    public ConstructorInjectionDemo(string value)
    {
        Value = value;
    }
}

[TestFixture]
public class MessagePackConstructorInjectionTests
{
    [Test]
    public void Given_a_class_without_public_setters_When_MessagePack_deserializes_the_object_Then_the_constructor_should_be_used()
    {
        var serializer = MessagePackSerializer.Get<ConstructorInjectionDemo>();
        using(var stream = new MemoryStream())
        {
            serializer.Pack(stream, new ConstructorInjectionDemo("Hello!"));
            stream.Position = 0;
            var deserialisedObject = serializer.Unpack(stream);
            Assert.That(deserialisedObject.Value, Is.EqualTo("Hello!"));
        }
    }
}

Is there any appetite to implement this feature?

Thanks,
Bart

[yfakariya]

It is great idea! I'll try to implement it in next major release, but there are some problems to be resolved:

• What order of constructor parameters is valid? I think the order of members (lexical or explicitly specified via attributes) are good. And qualifying attributes will also be good.
• What about naming convention using Map serialization method? I think just lower-casing initial char is best.
• How about overloading? It may be solved by qualifying the constructor with custom attribute, and then such construtor only become candidate of deserialization constructor.
• How about versioniong? If we add new member to the type, a new constructor should be added? If so, the overload problem appears again. I think versioning will be soled a simple rule - only qualified constructor will be allowed per their paramters count.

Any idea?

[nzbart]

1. It would be nice if the order of parameters were not critical, and instead the names of the parameters were used. This is consistent with the current property deserialization concepts. It would allow for better separation of concerns if a simple POCO could be serialized and deserialized without applying any attributes.
2. I agree with the lower case initial char for parameter names. This is the Microsoft naming convention.
3. It seem that some (Unity, Autofac at least) IoC containers prefer the constructor with the most parameters. Could we do the same?
4. I agree with keeping the rules simple and clear. Perhaps any parameters that do not have values to be deserialized should have their value set to default(T) because I believe that is consistent with what happens if you add a new property currently?
5. An additional consideration is that this feature would need to be backwards compatible. Perhaps it should only be activated if / when there are no settable properties? That would mean that it would be impossible to serialise the type in previous versions because the exception would be thrown: Cannot serialize type 'xxx' because it does not have any serializable fields nor properties.

There are some serious discussions around record types in C#, so this feature would be invaluable if these are implemented in a future version of the language. This blog entry simplifies the draft spec somewhat.

[fritzhu]

@yfakariya Yes, please implement! 👍

[jamezor]

This would be a big help to me as well, my use case is virtually identical to nzbart. Thanks!

[yfakariya]

@nzbart
Thank you for reply! I agree your opinions.
BTW, there is a too conservative spec that array-based serialization does not take effort to backward compatiblity now. I also fixes this problem in the next major release.

[jakesays-old]

Any news on this issue? I'm facing this exact issue - immutable types initialized via constructor injection. I've had to break immutability on my models and I'm not comfortable with that.

[yfakariya]

@JakeSays
Sorry, long time passed since last answer. I've just pushed local work and put pre-release version (0.6.0-Alpha1) on NuGet. I'm happy if you try it and post feedback :)

[jakesays-old]

@yfakariya Excellent. Thanks! i'll give it a try now.

Which method did you choose? After my message yesterday I implemented ctor based deserialization using matching ctor parameter names/property names.

[jakesays-old]

@yfakariya,

I gave your changes a try and came up with an issue - the unpacker is throwing FatalExecutionEngineError.

First, here's my test app:

using MsgPack.Serialization;

namespace MsgPackTester
{
    public struct ObjectId
    {
        public long Number { get; private set; }
        public int Version { get; private set; }

        public ObjectId(long number, int version)
            : this()
        {
            Number = number;
            Version = version;
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            var serializer = MessagePackSerializer.Get<ObjectId>();

            var id = new ObjectId(10, 1);

            //works. produces a 3 byte array.
            var bits = serializer.PackSingleObject(id);

            //crashes with FatalExecutionEngineError
            var id2 = serializer.UnpackSingleObject(bits);
        }
    }
}

And here is the decompiled serializer for ObjectId:

using MsgPack;
using MsgPack.Serialization;
using MsgPackTester;
using System;
using System.Reflection;

namespace MsgPack.Serialization.EmittingSerializers.Generated
{
    public sealed class MsgPackTester_ObjectIdSerializer : MessagePackSerializer<ObjectId>
    {
        private readonly MessagePackSerializer<long> _serializer0;

        private readonly MessagePackSerializer<int> _serializer1;

        private readonly FieldInfo _functionObjectId_set_Number0;

        private readonly FieldInfo _functionObjectId_set_Version1;

        public MsgPackTester_ObjectIdSerializer() : this(null)
        {
        }

        public MsgPackTester_ObjectIdSerializer(SerializationContext serializationContext) : base(serializationContext)
        {
            this._serializer0 = serializationContext.GetSerializer<long>();
            this._serializer1 = serializationContext.GetSerializer<int>();
            this._functionObjectId_set_Number0 = MethodBase.GetMethodFromHandle(typeof(ObjectId).GetMethod("set_Number", new Type[] { typeof(long) }).MethodHandle);
            this._functionObjectId_set_Version1 = MethodBase.GetMethodFromHandle(typeof(ObjectId).GetMethod("set_Version", new Type[] { typeof(int) }).MethodHandle);
        }

        protected sealed override void PackToCore(Packer packer, ObjectId objectId)
        {
            packer.PackArrayHeader(2);
            this._serializer0.PackTo(packer, objectId.Number);
            this._serializer1.PackTo(packer, objectId.Version);
        }

        protected sealed override ObjectId UnpackFromCore(Unpacker unpackers)
        {
            ObjectId objectId = new ObjectId();
            int num = 0;
            long? nullable = new long?();
            int? nullable1 = new int?();
            int num1 = 0;
            if (!unpackers.IsArrayHeader)
            {
                int itemsCount = UnpackHelpers.GetItemsCount(unpackers);
                while (num1 < itemsCount)
                {
                    string str = UnpackHelpers.UnpackStringValue(unpackers, typeof(ObjectId), "MemberName");
                    if (str == null)
                    {
                        throw SerializationExceptions.NewNullIsProhibited("MemberName");
                    }
                    string str1 = str;
                    if (str1 == "Version")
                    {
                        int? nullable2 = UnpackHelpers.UnpackNullableInt32Value(unpackers, typeof(ObjectId), "Int32 Version");
                        if (nullable2.HasValue)
                        {
                            FieldInfo _functionObjectIdSetVersion1 = this._functionObjectId_set_Version1;
                            object[] value = new object[] { nullable2.Value };
                            ((MethodBase)_functionObjectIdSetVersion1).Invoke(objectId, value);
                        }
                    }
                    else if (str1 != "Number")
                    {
                        unpackers.Skip();
                    }
                    else
                    {
                        long? nullable3 = UnpackHelpers.UnpackNullableInt64Value(unpackers, typeof(ObjectId), "Int64 Number");
                        if (nullable3.HasValue)
                        {
                            FieldInfo _functionObjectIdSetNumber0 = this._functionObjectId_set_Number0;
                            object[] objArray = new object[] { nullable3.Value };
                            ((MethodBase)_functionObjectIdSetNumber0).Invoke(objectId, objArray);
                        }
                    }
                    num1++;
                }
            }
            else
            {
                int itemsCount1 = UnpackHelpers.GetItemsCount(unpackers);
                if (num < itemsCount1)
                {
                    nullable = UnpackHelpers.UnpackNullableInt64Value(unpackers, typeof(ObjectId), "Int64 Number");
                }
                if (nullable.HasValue)
                {
                    FieldInfo fieldInfo = this._functionObjectId_set_Number0;
                    object[] value1 = new object[] { nullable.Value };
                    ((MethodBase)fieldInfo).Invoke(objectId, value1);
                }
                num++;
                if (num < itemsCount1)
                {
                    nullable1 = UnpackHelpers.UnpackNullableInt32Value(unpackers, typeof(ObjectId), "Int32 Version");
                }
                if (nullable1.HasValue)
                {
                    FieldInfo _functionObjectIdSetVersion11 = this._functionObjectId_set_Version1;
                    object[] objArray1 = new object[] { nullable1.Value };
                    ((MethodBase)_functionObjectIdSetVersion11).Invoke(objectId, objArray1);
                }
                num++;
            }
            return objectId;
        }
    }
}

I suspect the issue is private readonly FieldInfo _functionObjectId_set_Number0. ObjectId exposes properties only, you're treating them as fields.

I also noticed the unpacker is using reflection to set the members directly. Is this what you expected? I was hoping the ObjectId(long number, int version) would be used to initialize the unpacked instance.

Thanks for your help!
-Jake

[jakesays-old]

@yfakariya,

I think I found the issue.

If I change SerializationTarget.cs:310 from
if ( asProperty.GetSetMethod( true ) != null )
to
if ( asProperty.GetSetMethod( false ) != null )

then I get the proper constructor being called.

The FieldInfo thing is still going to be an issue, though.
-Jake

[yfakariya]

Thank you for reporting. I'll add more test for private setter and will fix it in the next release, sorry.

[yfakariya]

@JakeSays I released this feature in 0.6.0-beta2. Please try it. Thanks!