Fix serialization of private fields in base classes

[AArnott]

Fixes #820

[neuecc]

Should this line fix? https://github.com/neuecc/MessagePack-CSharp/blob/268896d89e1565ed6cac1e685bbd4689ca5c3fcf/src/MessagePack.UnityClient/Assets/Scripts/MessagePack/Resolvers/DynamicObjectResolver.cs#L1426

this line run by forceStringKey || contractless || (contractAttr != null && contractAttr.KeyAsPropertyName

[AArnott]

I don't think contractless wants to serialize private fields anyway, should it?

[neuecc]

At least, ContractlessStandardResolverAllowPrivate.Options currently exists.

[AArnott]

Ah! OK, I'll add a test for that and fix.

[AArnott]

So it's actually DynamicContractlessObjectResolverAllowPrivate that is required to exercise the other code path. And it's not clear to me that the fix should be to serialize all private fields. If we're serializing all private fields and properties, then every single property gets serialized twice (once for its field and once for its property). Since every property must have a backing field if it is worth serializing at all, it seems to me we should stop serializing properties altogether if we're going to serialize every single field. Thoughts?

[AArnott]

Given that enabling private fields in base classes greatly expand the problem of DynamicContractlessObjectResolverAllowPrivate serializing every property twice, and that when I tried it, it also breaks because it looks for a constructor with these extra members, I think we should consider resolving that scenario as a separate issue/PR. It's unrelated to the original issue which we already have a fix for.

[neuecc]

// [MessagePackObject(true)] is same as contractless.
[MessagePackObject(true)]
public class Base_1
{
    private int field;

    public int Field
    {
        get { return field; }

        set { field = value; }
    }
}

[MessagePackObject(true)]
public class Inheritance_1 : Base_1
{
    private int field2;

    public int Field2
    {
        get { return field2; }

        set { field2 = value; }
    }
}

var data1 = new Base_1 { Field = 100 };
var data2 = new Inheritance_1 { Field = 100, Field2 = 200 };

// {"Field":100,"field":100}
Console.WriteLine(MessagePackSerializer.ConvertToJson(MessagePackSerializer.Serialize(data1, StandardResolverAllowPrivate.Options)));

// { "Field":100,"Field2":200,"field2":200}
Console.WriteLine(MessagePackSerializer.ConvertToJson(MessagePackSerializer.Serialize(data2, StandardResolverAllowPrivate.Options)));

when data1 serialized result is {"Field":100,"field":100},
data2 serialized result should be { "Field":100,"Field2":200, "field1":100, "field2":200}
but currently no( { "Field":100,"Field2":200,"field2":200} ).

If you want to change the behavior of this data1, it's another PR,
If you want to follow the current behavior, you should fix it in this PR.

I think that double serialization can be tolerated as it is now because Contractless cannot grasp the identity of field and property.

[AArnott]

If you want to change the behavior of this data1, it's another PR,
If you want to follow the current behavior, you should fix it in this PR.

I think we should consider changing the behavior for contractless, but I see that as orthogonal to the issue being fixed in this PR. This PR doesn't impact the contractless behavior, so can't we design any behavior change as a separate issue and PR?

[neuecc]

@AArnott
Ok to merge, and let discuss in another issue.