Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Private fields on base class do not get serialized #820

Closed
OlegNadymov opened this issue Feb 12, 2020 · 13 comments · Fixed by #821
Closed

Private fields on base class do not get serialized #820

OlegNadymov opened this issue Feb 12, 2020 · 13 comments · Fixed by #821
Assignees
Labels
Milestone

Comments

@OlegNadymov
Copy link

I have business classes derived from a base class. The base class has several private fields that should to be serialized.
Simplified demo example:

[DataContract]
public class BaseClass
{
  [DataMember]
  private int _intField;

  public BaseClass()
  {
     _intField = SomeMethod();
  }

 private int SomeMethod() => 100; 

 public int IntField => _intField;
}

[DataContract]
public class DerivedClass : BaseClass
{
  [DataMember]
  public string Name {get; set;}
}

Is it possible to serialize/deserialize DerivedClass with _intField from BaseClass? I tried with this code:

var obj = new DerivedClass {Name = "name"};
var bin = MessagePackSerializer.Serialize(obj);
var obj2 = MessagePackSerializer.Deserialize<DerivedClass>(bin);
//or
var obj = new DerivedClass {Name = "name"};
var bin = MessagePackSerializer.Serialize(obj, StandardResolverAllowPrivate.Options);
var obj2 = MessagePackSerializer.Deserialize<DerivedClass>(bin, StandardResolverAllowPrivate.Options);

But I had _intField = 0.
If it is not possible could you suggest some workaround for my situation.
Important addition: I can't change BaseClass.

@AArnott
Copy link
Collaborator

AArnott commented Feb 13, 2020

I feel like StandardResolverAllowPrivate should have worked. @neuecc what do you think? That may be a bug.

In the meantime, you can get anything to work with a custom formatter.

@neuecc
Copy link
Member

neuecc commented Feb 14, 2020

I don't understand what data i confirm.

image

@AArnott
Copy link
Collaborator

AArnott commented Feb 14, 2020

Maybe @OlegNadymov can clarify what version of MessagePack produced the error.

@OlegNadymov
Copy link
Author

@AArnott @neuecc thank you for the feedback!
It's my bad. I simplified the demo code too much. Here is example where the error is reproduced:

[DataContract]
public class BaseClass
{
	[DataMember]
	private int _intField;

	public int IntField => _intField;

	internal void SetIntField(int intField)
	{
	  _intField = intField;
	}
}

[DataContract]
public class DerivedClass : BaseClass
{
	[DataMember]
	public string Name { get; set; }
}

The test:

[Fact]
public void PrivateFieldFromBaseClassSerializeTest()
{
	// Arrange 
	var obj = new DerivedClass { Name = "name" };
	obj.SetIntField(100);

	// Act
	var bin = MessagePackSerializer.Serialize(obj, StandardResolverAllowPrivate.Options);
	var obj2 = MessagePackSerializer.Deserialize<DerivedClass>(bin, StandardResolverAllowPrivate.Options);

	// Assert
	Assert.Equal(obj.IntField, obj2.IntField);
}

The test result:
msgpack_issue
I updated MessagePack to the latest version 2.1.90.
The main idea here is support for serialization of private fields from base classes.

@neuecc
Copy link
Member

neuecc commented Feb 15, 2020

Ok.
Actually, it is a bug.
https://github.com/neuecc/MessagePack-CSharp/blob/56fa862/src/MessagePack.UnityClient/Assets/Scripts/MessagePack/Resolvers/DynamicObjectResolver.cs#L1426
GetRuntimeFields does not provides non public field on base classes.

Specify BindingFlags.NonPublic to include non-public fields (that is, private, internal, and protected fields) in the search. Only protected and internal fields on base classes are returned;

Replace iterate method like this.

static IEnumerable<FieldInfo> GetAllFields(Type type)
{
    if (type.BaseType != null)
    {
        foreach (var item in GetAllFields(type.BaseType))
        {
            yield return item;
        }
    }

    // with declared only
    foreach (var item in type.GetFields(BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly))
    {
        yield return item;
    }
}

@AArnott AArnott self-assigned this Feb 15, 2020
@AArnott AArnott added the bug label Feb 15, 2020
@AArnott AArnott added this to the v2.1 milestone Feb 15, 2020
AArnott added a commit to AArnott/MessagePack-CSharp that referenced this issue Feb 15, 2020
@AArnott AArnott changed the title Private fields from base class Private fields on base class do not get serialized Feb 15, 2020
@OlegNadymov
Copy link
Author

@AArnott @neuecc thank you for the quick fix 👍 Could you tell when will the fix be on NuGet?

@OlegNadymov
Copy link
Author

One more addition. I don't know if anybody uses private properties. But if some developers use private properties and these properties are declared in base classes, then the properties do not get serialized. I think I don't use private properties so I can't realize whether it is necessary to add support for this case or not. What do you think?

@AArnott
Copy link
Collaborator

AArnott commented Feb 17, 2020

I added a test for private properties on a base class and so far, the tests as already passing.

AArnott added a commit to AArnott/MessagePack-CSharp that referenced this issue Feb 18, 2020
@OlegNadymov
Copy link
Author

I added a test for private properties on a base class and so far, the tests as already passing.

@AArnott Private properties on base classes do not get serialized. I mean this case:

[DataContract]
public class BaseClass
{
    [DataMember]
    private string PrivateBaseNote { get; set; }

    internal void SetPrivateBaseNote(string note)
    {
        PrivateBaseNote = note;
    }
}

[DataContract]
public class DerivedClass : BaseClass
{
    [DataMember]
    public int DerivedClassName { get; set; }
}

If you serialize DerivedClass the property PrivateBaseNote will not get serialized. But as I've mentioned before I don't have private properties with DataMember attribute. And so I don't know if it is should be serialized or not.

@neuecc, anyway, could tell me when you will be able to release the fix for the original issue on NuGet?

@AArnott
Copy link
Collaborator

AArnott commented Feb 26, 2020

@OlegNadymov In my PR I added a test that shows private properties on the base class do get serialized.
Did I miss some particular trait about your repro that leads the properties in your code snippet above to not be serialized?

@OlegNadymov
Copy link
Author

@AArnott in your PR there are public properties with private fields. But in my example there is private auto-implemented property (without backing fields)

Your test:

// private field (not property)
[DataMember]
private int baseClassField;

My example

// private property (not field)
[DataMember]
private string PrivateBaseNote { get; set; }

I hope it will be clear for understanding particular trait in my example.

@AArnott
Copy link
Collaborator

AArnott commented Feb 26, 2020

Ah, crap. I meant for that property in my test to be private. That's what I overlooked. Thanks!

@OlegNadymov
Copy link
Author

I was happy to help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants