Git Product home page Git Product logo

Comments (13)

yfakariya avatar yfakariya commented on July 18, 2024

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?

from msgpack-cli.

nzbart avatar nzbart commented on July 18, 2024
  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.

from msgpack-cli.

fritzhu avatar fritzhu commented on July 18, 2024

@yfakariya Yes, please implement! 👍

from msgpack-cli.

jamezor avatar jamezor commented on July 18, 2024

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

from msgpack-cli.

yfakariya avatar yfakariya commented on July 18, 2024

@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.

from msgpack-cli.

jakesays-old avatar jakesays-old commented on July 18, 2024

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.

from msgpack-cli.

yfakariya avatar yfakariya commented on July 18, 2024

@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 :)

from msgpack-cli.

jakesays-old avatar jakesays-old commented on July 18, 2024

@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.

from msgpack-cli.

jakesays-old avatar jakesays-old commented on July 18, 2024

@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

from msgpack-cli.

jakesays-old avatar jakesays-old commented on July 18, 2024

@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

from msgpack-cli.

yfakariya avatar yfakariya commented on July 18, 2024

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

from msgpack-cli.

yfakariya avatar yfakariya commented on July 18, 2024

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

from msgpack-cli.

yfakariya avatar yfakariya commented on July 18, 2024

Fixed in 0.6.0.

from msgpack-cli.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.