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

DataMember attribute for properties with inheritance #886

Closed
OlegNadymov opened this issue Apr 14, 2020 · 9 comments · Fixed by #1083
Closed

DataMember attribute for properties with inheritance #886

OlegNadymov opened this issue Apr 14, 2020 · 9 comments · Fixed by #1083
Assignees
Labels
Milestone

Comments

@OlegNadymov
Copy link

Bug description

There is a problem with objects with inheritance. In this case I have virtual properties with DataMember attribute in base class. Then I have derived class with overridden properties for adding some custom attributes for these properties. It should work for standard DataContract serializer and for MessagePack serializer. But it only works for DataContract serializer.

Repro steps

Demo example:

[DataContract]
public class BaseClass
{
  [DataMember]
  public virtual int VirtualProperty { get; set; }
}

[DataContract]
public class DerivedClass : BaseClass
{
 //[SomeCustomAttribute]
  [DataMember]
  public override int VirtualProperty { get; set; }
}

var obj = new DerivedClass {VirtualProperty = 100};
var bin = MessagePackSerializer.Serialize(obj);
var obj2 = MessagePackSerializer.Deserialize<DerivedClass>(bin);

Expected behavior

It should work as well as on DataContract serializer.

Actual behavior

I got exception:
MessagePack.Internal.MessagePackDynamicObjectResolverException : key is duplicated, all members key must be unique. type: MyLibTest+DerivedClass member:VirtualProperty

  • MessagePack v.2.1.115
  • Runtime: .Net Core 3.1, Visual Studio 2019.

Additional context

Is it possible to add support for this case?
I can delete DataMember attribute from property in base class. In this case MessagePack serializer start working, but DataContract serializer stops working.

@OlegNadymov
Copy link
Author

I dived deeper in this topic and found out that if I omit DataMember attribute from property in derived class the DataContract serializer will work anyway. It was unpredictable for me because DataMember attribute has flag Inherited = false. But anyway it works for me and I think it is quite appropriated way for me. The MessagePack serializer also works in this case. I'm gonna research this topic for a while and if I don't find better solution I will use this way.

@OlegNadymov
Copy link
Author

Does the MessagePack serializer respect to DataMember.EmitDefaultValue flag?

@OlegNadymov
Copy link
Author

Does the MessagePack serializer respect to DataMember.EmitDefaultValue flag?

I checked it and I found out that it does not. Do you have any plans to support this flag in MessagePack serializer? Or maybe MessagePack serializer has alternative option for this?

@AArnott
Copy link
Collaborator

AArnott commented Apr 15, 2020

Does the MessagePack serializer respect to DataMember.EmitDefaultValue flag?

I don't think so. Can you share why it's important to you? If you're using the Order property of the DataMember attribute, we can't omit a null value anyway because we have to put something in that array position. If you're using named keys, we could technically omit the value though.

@OlegNadymov
Copy link
Author

I want to use these business classes for DataContract serializer and MessagePack serializer also. But I made up only single solution for this. It's to remain DataMember attribute for properties on base class. I'm gonna use EmitDefaultValue to reduce XML/Json size. I use named keys for MessagePack.

@AArnott
Copy link
Collaborator

AArnott commented Apr 15, 2020

OK, so with named keys, omitting null is a reasonable request. Appeasing that might make sense to do at the same time as #678.

But let's have this issue focus on the inheritance issue as its title says. I agree this is a bug, and I'm glad you have a workaround. FWIW though, your sample has a (small) bug too. The way you have it implemented you have two fields backing VirtualProperty (one for each class). The way to avoid that and have just one field is this:

[DataContract]
public class BaseClass
{
  [DataMember]
  public virtual int VirtualProperty { get; set; }
}

[DataContract]
public class DerivedClass : BaseClass
{
 //[SomeCustomAttribute]
  [DataMember]
  public override int VirtualProperty {
     get => base.VirtualProperty;
     set => base.VirtualProperty = value;
  }
}

@AArnott AArnott added the bug label Apr 15, 2020
@OlegNadymov
Copy link
Author

OK. Thanks for reasonable comment about a bug in my example.
I'm looking forward when you will be able to fix the bug from this issue and implement feature from #678

@github-actions
Copy link

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions
Copy link

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 5 days.

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