Runtime: System.Text.Json support to System.Runtime.Serialization

Created on 21 Jun 2019  路  12Comments  路  Source: dotnet/runtime

This is a follow up to this comment:

Is there a plan to support [DataContract], [DataMember] and friends?

Today this is a way to define how types should serialize/deserialize (e.g. the field name) in a way that doesn't bring a dependency to any serialization library to the project using it.

The current JsonNamingPolicy takes a string only so there's no way to inspect the member's attributes. So it seems so far there's no way to support annotations (.NET Attributes) and as such no way to define the serialized name except from the original property name.

Motivation

System.Runtime.Serialization is part of .NET Standard and it allows us to annotate members of a class in a way to hint a serialization library what name to use and whether to include it or not if the value is null.

This is valuable because it allows us to have NuGet packages that don't depend on a serialization library directly.

Ensuring that a serialized version of a type matches a certain protocol could be done by means of tests only. This way we can make sure more than one library would be supported (like Newtonsoft.Json and DataContractSerializer at this point.

area-System.Text.Json

Most helpful comment

Not supporting this is a fail for the .NET library ecosystem.
It means nuget package authors that provide serialisable models in their libraries (with Blazor in the picture now, there are nuget packages that provide server side and client side code, and models that must be serialised in between) - must pass on their choice of serializer to their consumers - i.e System.Text.Json vs Newtonsoft.Json. This will result in blazor client applications being forced to consume both dependencies (not ideal as blazor wasm applications need as few dll's as possible to avoid poor startup performance in the browser), or nuget package authors having to publish two versions of their solution, one for each serialiser - no one will do that.

I'd be fine if this wasn't implemented yet - but there were plans to. But the fact there are't even any plans to implement it makes me think this is an oversight. Why would you not plan to implement this?

All 12 comments

We don't currently have plans to support the attributes from System.Runtime.Serialization

@ahsonkhan Is there a plan to provide an API to define an arbitrary name for a serialized property?
Not necessarily via attributes.

Not supporting this is a fail for the .NET library ecosystem.
It means nuget package authors that provide serialisable models in their libraries (with Blazor in the picture now, there are nuget packages that provide server side and client side code, and models that must be serialised in between) - must pass on their choice of serializer to their consumers - i.e System.Text.Json vs Newtonsoft.Json. This will result in blazor client applications being forced to consume both dependencies (not ideal as blazor wasm applications need as few dll's as possible to avoid poor startup performance in the browser), or nuget package authors having to publish two versions of their solution, one for each serialiser - no one will do that.

I'd be fine if this wasn't implemented yet - but there were plans to. But the fact there are't even any plans to implement it makes me think this is an oversight. Why would you not plan to implement this?

We don't currently have plans to support the attributes from System.Runtime.Serialization

There will be no use of System.Text.Json then. I expect to be able to transfer the serialization model when migrating over versions, so I won't have to redesign existing app architecture.

Is there a plan to provide an API to define an arbitrary name for a serialized property?
Not necessarily via attributes.

@bruno-garcia, you could try and register a converter for a particular type/property and override the behavior. I don't know how you can customize any arbitrary property name though without attributes. Do you have a a scenario in mind where you'd need that without attributes? @steveharter - do you have any thoughts on how we can support this? @bruno-garcia - it would be good if you could file a separate issue for this with your expectation/use case.

This will result in blazor client applications being forced to consume both dependencies (not ideal as blazor wasm applications need as few dll's as possible to avoid poor startup performance in the browser), or nuget package authors having to publish two versions of their solution, one for each serialiser - no one will do that.

That's a good point, and I agree that decoupling the dependency to a particular serializer is useful. I'll spend some time taking a look at this space in the near future.

I'd be fine if this wasn't implemented yet - but there were plans to. But the fact there are't even any plans to implement it makes me think this is an oversight.

There are a lot of feature/capability requests when it comes to serialization and we prioritized enabling key scenarios first and hence only certain features can be supported for the given release. To clarify, I am not suggesting we would never support this, just that we haven't gone through and spec'd out this particular feature (the requirements/constraints/etc.). Which is why I said _currently_ and which is why the issue is still open. We are still doing 5.0 planning.

Why would you not plan to implement this?

Part of the reservation was concerns around binary serialization/supporting attributes like [OnSerializing] (which is something I need to get clarity on anyway, and as you mentioned S.R.Serialization has general purpose attributes too - https://github.com/dotnet/corefx/issues/37503#issuecomment-500969445). But it's primarily scheduling and filing the gaps for other features took precedence (see current backlog - https://github.com/dotnet/corefx/issues?q=is%3Aopen+is%3Aissue+label%3Aarea-System.Text.Json+milestone%3A5.0). Feedback here about usage/scenarios being blocked certainly helps motivate enabling this. I appreciate folks sharing their current usage patterns and current pain points of migrating.

I expect to be able to transfer the serialization model when migrating over versions, so I won't have to redesign existing app architecture.

Given the current release of the JSON stack has bare-bones feature-set with an emphasis on performance first, this isn't feasible for all cases already (for example if your model had fields instead of properties). Going from using Newtonsoft.Json where you may be relying on all the features it offers, you will certainly find gaps where migration would require changes (or it would be non-feasible) for the foreseeable future. I doubt we will ever have complete parity here, but we will focus on closing the most significant gaps in subsequent releases (especially where the new stack is better suited to solve the problem).

Do you have a a scenario in mind where you'd need that without attributes?
I don't. Ideally we'd be able to use it as-is. I can use DataContractSerializer or Newtonsoft.Json with my types annotated with System.Runtime.Serialization and it works fine.
When I raised the ticket the only thing the converter was able to do is to give the json name based on the property name (not the type's member instance, so that i could look for the attribute myself.

I understand the design here was focused on perf and doing reflection to look for the attribute would defeat the purpose. I'm accepting that fact we'll never get the support but I won't close the ticket myself. :)

@bruno-garcia

I understand the design here was focused on perf and doing reflection to look for the attribute would defeat the purpose.

There is no difference between resolving of attribute [DataMember] or [JsonPropertyName]. This is doing once and does not affect the final performance.

@vitidev when I raised this, if I didn't get it wrong, no attribute was resolved. The only way to change the name of the property in the resulting JSON was by inspecting the object's property name (IIRC it took a string and returned a string).
If instead it returned the Property object (reflection) we could inspect the attribute to look for the alt name.
Ideally we could register some Func<> or type that would be called and upon registration we could do the reflection bit (once) and register some compiled expressions to do the conversion. So the reflection perf hit would happen only at startup time.

As far as supporting attributes from System.Runtime.Serialization is concerned, it is not on the System.Text.Json roadmap to do this en masse. Please open individual issues for each attribute/logical group of attributes for which support is needed, and include use cases.

@layomia Should #30009 be reopened, then? It was closed due to this ticket being open. A lot of useful libraries in the ecosystem depend on this for serialization to/from many formats, json included. That sort of compatibility seems to be what dotnet strives for.

If performance impact is a concern, could it potentially be factored into JsonSerializerOptions?

Reopening this issue based on the comments from @steveharter in https://github.com/dotnet/runtime/issues/30009#issuecomment-697936065:

Early-on during design of STJ in 3.0, it was decided not to support pre-existing attributes mainly because they would only be partially supported and it would be hit-and-miss meaning STJ would only support some of those in the first release and additional support added in future releases. This would cause endless confusion over what is and what is not supported. STJ can't just throw NotSupportedException for the unsupported cases since that would mean the attribute usage would need to be removed from the corresponding types (not feasible unless the types are owned), or have a way to turn off the exception.

Also since STJ would need to explicitly look for those attributes, it would cause some slowdown during warm-up.

Consider just the DataMemberAttribute properties:

  • EmitDefaultValue: in 3.0, we couldn't support since we didn't have that feature. Now in 5.0 it is.
  • IsNameSetExplicitly: not supported yet.
  • IsRequired: not supported yet.
  • Name: supported through STJ.PropertyNameAttribute.
  • Order: not supported yet.

Then there's also a few other attributes including CollectionDataContractAttribute, OnDeserializedAttribute that would also contribute to hit-and-miss. STJ also didn't consider other areas including System.IConvertible.

So we went with a new explicit model that is intuitive (if it's not in the STJ namespace then it's not supported) that is high-performance with the thought we can add support for pre-existing attributes through an opt-in modeflag of some sort or perhaps a pluggable metadata provider. These have not been implemented yet.

Moving-forward, having a pluggable metadata provider along with an opt-in System.Runtime.Serialization "compat" implementation of that provider makes sense to me. Not sure if that has enough priority for 6.0, but it could be considered along with the feature to expose metadata and more flexible converters.

we can add support for pre-existing attributes through an opt-in modeflag of some sort or perhaps a pluggable metadata provider.

That's all we want, thanks.

Not sure if that has enough priority for 6.0, but it could be considered along with the feature to expose metadata and more flexible converters.

If it's not landing on 6.0, that means 2022 the earliest. I really hope I can unsubscribe from this issue before that. :)
But I'm sure it'll be helpful to many folks that have large code bases that rely on these.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

v0l picture v0l  路  3Comments

jamesqo picture jamesqo  路  3Comments

jkotas picture jkotas  路  3Comments

Timovzl picture Timovzl  路  3Comments

matty-hall picture matty-hall  路  3Comments