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

System.Text.Json support to System.Runtime.Serialization #29975

Closed
bruno-garcia opened this issue Jun 21, 2019 · 89 comments
Closed

System.Text.Json support to System.Runtime.Serialization #29975

bruno-garcia opened this issue Jun 21, 2019 · 89 comments
Labels
area-System.Text.Json wishlist Issue we would like to prioritize, but we can't commit we will get to it yet
Milestone

Comments

@bruno-garcia
Copy link
Member

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.

@ahsonkhan
Copy link
Member

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

@bruno-garcia
Copy link
Member Author

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

@bruno-garcia bruno-garcia changed the title System.Text.Json support to System.ComponentModel System.Text.Json support to System.Runtime.Serialization Jun 22, 2019
@dazinator
Copy link

dazinator commented Sep 11, 2019

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?

@daerhiel
Copy link

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.

@ahsonkhan
Copy link
Member

ahsonkhan commented Sep 27, 2019

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).

@bruno-garcia
Copy link
Member Author

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. :)

@vitidev
Copy link

vitidev commented Nov 19, 2019

@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.

@bruno-garcia
Copy link
Member Author

@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.

@layomia
Copy link
Contributor

layomia commented Apr 6, 2020

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 layomia closed this as completed Apr 6, 2020
@bpicolo
Copy link

bpicolo commented Apr 15, 2020

@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?

@layomia
Copy link
Contributor

layomia commented Sep 24, 2020

Reopening this issue based on the comments from @steveharter in #30009 (comment):

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 mode\flag 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.

@layomia layomia reopened this Sep 24, 2020
@layomia layomia modified the milestones: 5.0.0, Future Sep 24, 2020
@bruno-garcia
Copy link
Member Author

we can add support for pre-existing attributes through an opt-in mode\flag 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.

@LukeTOBrien
Copy link

LukeTOBrien commented Jan 22, 2021

This is a majour issue for me (and a supprise, what's the point of [DataMember] if they are not used in JSON?) as I am consuming a third-party REST API that is using underscore_naming - So the logical way to overcome this is to use [DataMember].

I will switch back to JSON.Net until this is implemented.

@ahsonkhan
Copy link
Member

as I am consuming a third-party REST API that is using underscore_naming

If that is the only issue you are dealing with, you could implement a a snake case naming policy and set that in the JsonSerializerOptions, similar to how camel casing is set:
https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-customize-properties#use-camel-case-for-all-json-property-names
#42009

@reflectronic

This comment was marked as off-topic.

@edc-jacrys

This comment was marked as off-topic.

@edc-jacrys

This comment was marked as off-topic.

@eiriktsarpalis

This comment was marked as off-topic.

@dazinator
Copy link

dazinator commented Jun 17, 2022

@edc-jacrys - Not sure whether you would agree upon reflection, but I interpreted the tone of some of your recent comments to be a touch adversarial! As someone else who was also very surprised not to see this "feature" present, I worry its possibly not helpful for the cause to be quite so adversarial! Saying that, I appreciate you just care about this and that it can be frustrating, still - there is no "Argument" to he won here, we can only help influence, inform, or be informed. The best way to help influence is to remain objective and respectful imho.

It's great to see some specific examples / cases above where lacking this support is causing issues. I've raised one myself (about choice of serializer being passed to blazor wasm applications) which was meant to have been looked into but I've not seen any follow up. As long as those important use cases can be fulfilled it matters less to me whether its provided natively or via a suitable "extension" point + dependency. My only point against the extension point approach for this specific feature is that, relying on the community to maintain a package to allow S.T.J to support the runtime attributes, is a bit cheeky. It's Microsoft's stack, and customers having to glue it together with an external dependency is not a good look or use of time (especially for those having to produce and maintain the glue in the first place). If its a third party it might also suffer from quality bar or licencing issues in future. I'm sure this has already been factored into the decision making process however. So my "want" in this regard would be that this additional / optional dependency should be maintained by a suitable Microsoft team..

@edc-jacrys

This comment was marked as off-topic.

@edc-jacrys

This comment was marked as off-topic.

@eiriktsarpalis
Copy link
Member

My only point against the extension point approach for this specific feature is that, relying on the community to maintain a package to allow S.T.J to support the runtime attributes, is a bit cheeky. It's Microsoft's stack, and customers having to glue it together with an external dependency is not a good look or use of time (especially for those having to produce and maintain the glue in the first place).

That's pretty common though. To give you a contrived example, STJ does not support StringDictionary out of the box, and we would generally recommend authoring a custom converter if somebody does need support for the type. For many reasons, we tend to prioritize good extensibility over feature completeness.

I get that the DataContractSerializer attributes have become something of a de-facto standard in the ecosystem when it comes to defining dependency-free DTOs. I also get the appeal, the attributes provide good-enough contract customization without introducing third-party dependencies (I for one have written serializers that understand SRS attributes in my pre-Microsoft days). That being said, STJ attributes are part of the shared framework and can be applied without introducing unnecessary dependencies either.

I will reiterate that our decision to not include OOTB support for SRS attributes is not us not wanting to walk the final mile or being lazy; we have concerns about what an officially sanctioned SRS contract resolver might look like. Also, such decisions are always temporary and subject to reconsideration in light of new evidence. Letting the community drive experimentation would help a lot in that regard, and we have already seen this dynamic play out in other features, for example #69613.

@Grauenwolf
Copy link

Grauenwolf commented Jun 17, 2022

A possible resolution to that issue might be that we chose to ignore all the square pegs,

Which is exactly what all of the other serializers do. Everyone knows that the Data Contract attributes are a compromise. Even DataContractJsonSerializer ignored some parts as being not applicable.

It's worth keeping in mind that whatever component we ship with the shared framework automatically...

No one said it had be "with the shared framework". We already know that it has to be opt-in. If the way we turn it on is by referencing an optional package, that is perfectly acceptable.

All you have to say is, "We will include basic data contract support, not everything but the basics like DataMember and DataIgnore, as a separate library outside of the shared framework.", and most people are going to be happy.

We don't demand perfection, we just want the pain threshold to be reduced by a bit.

@dazinator
Copy link

dazinator commented Jun 17, 2022

That being said, STJ attributes are part of the shared framework and can be applied without introducing unnecessary dependencies either.

If I author a nuget package containing types that must be serialized / deserialized, I could previously use S.R.S attributes and know I wasn't coupling the consumer to any specific serialiser. So if they wanted to use newtonsoft that's fine, or myriad of others. Sure I can use new S.T.J attributes but then I am forcing the use of S.T.J as the serializer on consumers. I'd now need to address this in my design where previously the answer was S.R.S attributes to solve this problem. I haven't looked at how I'd do this specifically as this is a hypothetical but I am interested to know whether you have thought about this and have any recommendations.

@edc-jacrys
Copy link

edc-jacrys commented Jun 17, 2022

We don't demand perfection, we just want the pain threshold to be reduced by a bit.

Exactly. the option to self-implement support for something that has been the de-facto standard for 20 years is why we are concerned. And as @dazinator stated, STJ as it sits tightly couples the serialization to a specific provider rather than providing options. Which is completely antithetical to the core goal of Core in the first place, which is less coupled, not more.

Also, the entirety of the mentality of .NET 6+has been to make C# in general easier to use and more open to C# neophytes. I fail to see how this:

{
    public override JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options)
    {
        JsonTypeInfo jsonTypeInfo = base.GetJsonTypeInfo(type, options);

        if (jsonTypeInfo.Kind == JsonTypeInfoKind.Object &&
            type.GetCustomAttribute<DataContractAttribute>() is not null)
        {
            jsonTypeInfo.Properties.Clear();

            foreach ((PropertyInfo propertyInfo, DataMemberAttribute attr) in type.GetProperties(BindingFlags.Instance | BindingFlags.Public)
                .Select((prop) => (prop, prop.GetCustomAttribute<DataMemberAttribute>() as DataMemberAttribute))
                .Where((x) => x.Item2 != null)
                .OrderBy((x) => x.Item2!.Order))
            {
                JsonPropertyInfo jsonPropertyInfo = jsonTypeInfo.CreateJsonPropertyInfo(propertyInfo.PropertyType, attr.Name ?? propertyInfo.Name);
                jsonPropertyInfo.Get =
                    propertyInfo.CanRead
                    ? propertyInfo.GetValue
                    : null;

                jsonPropertyInfo.Set = propertyInfo.CanWrite
                    ? (obj, value) => propertyInfo.SetValue(obj, value)
                    : null;

                jsonTypeInfo.Properties.Add(jsonPropertyInfo);
            }
        }

        return jsonTypeInfo;
    }
}

makes it easier to approach STJ?

The attributes in System.Runtime.Serialization have been specifically designed for serializers like BinaryFormatter and DataContractSerializer, and are not general-purpose serialization annotations

This is missing the fact that SRS DOES support JSON and is designed to work with the serializer in SRS.Json, and has since Framework 3.5 back in 2007, 1.5 decades ago.

@edc-jacrys
Copy link

edc-jacrys commented Jun 17, 2022

Enabling SRS attributes by default would be a breaking change. It would either need to be a flag on JsonSerializerOptions or (most likely) exposed as a separate contract resolver implementation that users opt into.

Why would this necessarily be a breaking change.? Considering how you have seen that the community at large expects this to just work and it doesn't, wouldn't NOT doing it break more implementations than doing it?

@Grauenwolf
Copy link

This is missing the fact that SRS DOES support JSON and is designed to work with the serializer in SRS.Json, and has since Framework 3.5 back in 2002, 2 decades ago.

A slight error there. The Data Contract attributes that we care about are from .NET 3 in 2006.

@Grauenwolf
Copy link

Why would this necessarily be a breaking change.?

Any code that is currently written for System.Text.Json in mind will expect that the Data Contract attributes will be ignored. If they suddenly start working, then it changes the behavior of the code. A behavior the developer may have been relying on.

So yea, we're stuck with this being an opt-in feature that has to be explicitly turned on.

@edc-jacrys
Copy link

edc-jacrys commented Jun 17, 2022

@Grauenwolf Yeah I know. His point was the attributes were "specifically designed" for serializers like BinaryFormatter and I was pointing out that there is a JSON serializer and has been for 15 years that was designed to work with those. So the attributes are less specialized and more general purpose than he was making them appear to be.

@edc-jacrys
Copy link

Why would this necessarily be a breaking change.?

Any code that is currently written for System.Text.Json in mind will expect that the Data Contract attributes will be ignored. If they suddenly start working, then it changes the behavior of the code. A behavior the developer may have been relying on.

So yea, we're stuck with this being an opt-in feature that has to be explicitly turned on.

That is assuming a lot. One, it is obvious from this thread that most here assumed the opposite, Two, it could be as simple as an initialization setting in the startup, flipping a flag, not necessarily the contrivance which has so far been proposed.

@Grauenwolf
Copy link

Two, it could be as simple as an initialization setting in the startup

That's why I'm saying the "breaking change" argument is nonsense.

If they included this and they did turn it on by default, it would be a breaking change. But I don't think anyone here would object if we had to write .UseDataContracts = true in our startup file.

@eiriktsarpalis
Copy link
Member

The release of the contract customization feature in .NET 7 Preview 6 should make it possible to add support for System.Runtime.Serialization attributes via a custom contract resolver. Here is a functional test demonstrating how it could be implemented:

[Fact]
public static void DataContractResolverScenario()
{
var options = new JsonSerializerOptions { TypeInfoResolver = new DataContractResolver() };
var value = new DataContractResolver.TestClass { String = "str", Boolean = true, Int = 42, Ignored = "ignored" };
string json = JsonSerializer.Serialize(value, options);
Assert.Equal("""{"intValue":42,"boolValue":true,"stringValue":"str"}""", json);
DataContractResolver.TestClass result = JsonSerializer.Deserialize<DataContractResolver.TestClass>(json, options);
Assert.Equal("str", result.String);
Assert.Equal(42, result.Int);
Assert.True(result.Boolean);
}
internal class DataContractResolver : DefaultJsonTypeInfoResolver
{
[DataContract]
public class TestClass
{
[JsonIgnore] // ignored by the custom resolver
[DataMember(Name = "stringValue", Order = 2)]
public string String { get; set; }
[JsonPropertyName("BOOL_VALUE")] // ignored by the custom resolver
[DataMember(Name = "boolValue", Order = 1)]
public bool Boolean { get; set; }
[JsonPropertyOrder(int.MaxValue)] // ignored by the custom resolver
[DataMember(Name = "intValue", Order = 0)]
public int Int { get; set; }
[IgnoreDataMember]
public string Ignored { get; set; }
}
public override JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options)
{
JsonTypeInfo jsonTypeInfo = base.GetTypeInfo(type, options);
if (jsonTypeInfo.Kind == JsonTypeInfoKind.Object &&
type.GetCustomAttribute<DataContractAttribute>() is not null)
{
jsonTypeInfo.Properties.Clear();
foreach (PropertyInfo propInfo in type.GetProperties(BindingFlags.Instance | BindingFlags.Public))
{
if (propInfo.GetCustomAttribute<IgnoreDataMemberAttribute>() is not null)
{
continue;
}
DataMemberAttribute? attr = propInfo.GetCustomAttribute<DataMemberAttribute>();
JsonPropertyInfo jsonPropertyInfo = jsonTypeInfo.CreateJsonPropertyInfo(propInfo.PropertyType, attr?.Name ?? propInfo.Name);
jsonPropertyInfo.Order = attr?.Order ?? 0;
jsonPropertyInfo.Get =
propInfo.CanRead
? propInfo.GetValue
: null;
jsonPropertyInfo.Set = propInfo.CanWrite
? propInfo.SetValue
: null;
jsonTypeInfo.Properties.Add(jsonPropertyInfo);
}
}
return jsonTypeInfo;
}
}

@eiriktsarpalis
Copy link
Member

Per my earlier comment, adding support for DataContractAttribute and friends should be achievable via contract customization starting in .NET 7. We're not planning on adding out-of-the-box support for this, but it should still be possible for third-party extension libraries to ship contract resolvers offering this functionality.

@eiriktsarpalis eiriktsarpalis closed this as not planned Won't fix, can't repro, duplicate, stale Sep 2, 2022
@hkusulja
Copy link

hkusulja commented Sep 2, 2022

@eiriktsarpalis again, Microsoft is even not planning to implement / support cooperation between its own libraries (where in history .NET 4 this cooperation was supported and working), and is redirecting to use 3rd party solutions, again.

We already have 3rd party solution, and is called Newtonsoft Json , which is working for years...

I do not understand why this feature request / idea is not even considered , and not for the future of .NET 7 and supporting its own Microsoft latest libraries, even if those are from separate team.

So, please, @eiriktsarpalis can you please explain more / elaborate, why is Microsoft not implementing contract customization with DataContractAttribute to be native in .NET 7 as subject says - System.Text.Json support to System.Runtime.Serialization

Are there technical challenges or just team management and politics or not enough will to do it a proper way?

Thank you for your answer in advance

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Sep 2, 2022

@hkusulja please see my earlier responses here, here and here.

I do not understand why this feature request / idea is not even considered , and not for the future of .NET 7 and supporting its own Microsoft latest libraries, even if those are from separate team.

Respectfully, I do not agree with your assertion that the feature request was not considered. It was one key use cases driving development of #63686. It simply is the case that we believe that it shouldn't be supported out of the box.

@HeathJared
Copy link

As amazing as most of the .NET Core progress has been, it's equally amazing at how out of touch this project's team is.

This project could have had wide spread adoption 3 years ago. Instead it's going to continue being a fragmented solution with messy or limited real world use and 30 unsupported community NuGet packages from random authors all attempting to gap-fill the same exact basic requirements in order to try and make this package usable (with the most widely adopted, arguably standard, serialization library in the .NET Framework).

Extremely unfortunate for something that was so very desired and easily delivered. Massive shame.

@zcsizmadia
Copy link

DataContractResolver is trying to solve this. Unfortunately it requres STJ v7+.

Please provide test cases for data contracts with an expected serialized output.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json wishlist Issue we would like to prioritize, but we can't commit we will get to it yet
Projects
None yet
Development

No branches or pull requests