MessagePack blindly respects ignore attributes used for WCF

[Haihg]

Is your feature request related to a problem? Please describe.

A past improvement has enabled MessagePack to support [IgnoreDataMember] attribute, along with existing [IgnoreMember] to define properties we want to exclude for serializing. Please see
#9

We are looking to have MessagePack implemented in our system to gradually replace Json.Net for serialization in multiple areas. In our system, there is a legacy WCF service. WCF and Json.Net are doing serialization on the same entities. WCF serialization makes use of [IgnoreDataMember] attributes to exclude properties from serializing. Json.Net uses "JsonIgnore" attribute for this purpose. The properties that we want to exclude for WCF and Json.Net are not the same on the same entity (in one case, we want to reduce data load, so we exclude a few list properties). So far this has been without issue. However, when we test run MessagePack to replace Json.Net, it becomes an issue because properties we previously didn't want to exclude by Json.Net now automatically get excluded by MessagePack, as MessagePack respects "IgnoreDataMember" attribute used by WCF. The data being serialized by MessagePack accidentally lacks some properties we don't want to miss due to the [IgnoreDataMember] attribute, which we cannot remove.

Describe the solution you'd like

Recommend MessagePack to have a stricter set of attributes for data contract. User should be able to specify to use either the MessagePackObject/Key/IgnoreMember or DataContract/DataMember/IgnoreDataMember set of attributes. The usage can be set by a global setting. Support for both set of attributes is also fine (and can be even set as default), as long as we have an option to exclusively select either one of the above attribute sets.

[AArnott]

That sounds interesting and reasonable, @Haihg. Can you provide an example C# class definition that is problematic? I guess it will have a mix of two sets of attributes.

[Haihg]

Hi AArnott

A simple class definition is like:

[MessagePack.MessagePackObject(true)]
public class MyData
{
	public long Id { get; set; }

	public string Name { get; set; }

	[MessagePack.IgnoreMember] // We want MessagePack to ignore this property
	public string Alias { get; set; }

	public DateTime LastUpdated { get; set; }

	// Issue with MessagePack, this property will not get serialized
	// but we cannot remove this attribute as it is needed by existing code
	// We currently use Json.Net for serialization and there is no issue with it
	[IgnoreDataMember]
	public IList<RawData1> RawData1 { get; set; }
}

A more complicated one looks like:

[MessagePack.MessagePackObject]
public class MyData2
{
	[MessagePack.Key(0)]
	public long Id { get; set; }

	[field: NonSerialized] // I was doing this when trying out another serializer, e.g. Apex.Serialization
	[MessagePack.Key(1)] // No issue with MessagePack
	public string Name { get; set; }

	[MessagePack.IgnoreMember] // We want MessagePack to ignore this property
	public string Alias { get; set; }
	
	[CsvPropertyIgnore] // For our CSV export library
	[MessagePack.Key(2)] // No issue with MessagePack
	public DateTime LastUpdated { get; set; }

	[IgnoreDataMember] // For our WCF service
	[MessagePack.Key(3)] // Issue, property not serialised by MessagePack
	public IList<RawData1> RawData1 { get; set; }
}

[AArnott]

OK, that makes sense. Fixing this will be an observable behavioral change and therefore potentially breaking, but it totally makes sense that as a serializer that supports multiple attribute sets, we should pick one and honor just that for any given type.

@neuecc any concerns?

[LiorBanai]

I suggest to allow multiple attributes since this will give most flexibility. The user should set all the types to support so one can choose to ignore both or one of them.

[AArnott]

The way I was thinking of going is to honor only the messagepack-specific attributes when the type is tagged as [MessagePackObject].
In the absence of such an attribute, it would look for the [DataContract] attribute and honor just those attributes if it found one.
In absence of either, it doesn't work at all today unless you specify the Contractless resolver, and we could keep that behavior the same.

[LiorBanai]

Sounds good actually.

[Haihg]

Thanks all for quickly looking into this!

I'm wondering with the proposed fix, will it add a tiny amount of performance loss to the seririalization for reading and processing the existing attributes? Also, if the user can EXPLICITLY tell what approach they want via some configuration, then I think that's clearer for both developer on implementation, and for users on using the library.

Otherwise if you guys don't see any performance concern then I look forward to seeing the proposed fix implemented in the next releases.

The way I was thinking of going is to honor only the messagepack-specific attributes when the type is tagged as [MessagePackObject].
In the absence of such an attribute, it would look for the [DataContract] attribute and honor just those attributes if it found one.
In absence of either, it doesn't work at all today unless you specify the Contractless resolver, and we could keep that behavior the same.

[AArnott]

I predict a positive perf impact, if anything, since we won't look for as many attributes. But it will be neglible either way.

[neuecc]

In the first case, you can use JsonIgnoreAttribute instead of IgnoreDataMember.
I don't feel much need, but ok to impl #800 (comment)

[github-actions]

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]

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]

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.

[AArnott]

I'm going to close this now but this will be tracked as part of #1060.