-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Comments
We don't currently have plans to support the attributes from |
@ahsonkhan Is there a plan to provide an API to define an arbitrary name for a serialized property? |
Not supporting this is a fail for the .NET library ecosystem. 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? |
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. |
@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.
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.
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.
Part of the reservation was concerns around binary serialization/supporting attributes like
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). |
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. :) |
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). |
As far as supporting attributes from |
@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? |
…ssues: 1. Lacks support for DataContract/DataMember attributes: dotnet/runtime#29975 dotnet/runtime#30009 2. Deserializes the primitive values as JsonElement instances.
Reopening this issue based on the comments from @steveharter in #30009 (comment):
|
That's all we want, thanks.
If it's not landing on 6.0, that means 2022 the earliest. I really hope I can unsubscribe from this issue before that. :) |
This is a majour issue for me (and a supprise, what's the point of I will switch back to JSON.Net until this is implemented. |
If that is the only issue you are dealing with, you could implement a a snake case naming policy and set that in the |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
@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.. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
That's pretty common though. To give you a contrived example, STJ does not support 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. |
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.
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. |
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. |
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?
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. |
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? |
A slight error there. The Data Contract attributes that we care about are from .NET 3 in 2006. |
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. |
@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. |
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. |
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 |
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: Lines 656 to 726 in 1958812
|
Per my earlier comment, adding support for |
@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 |
@hkusulja please see my earlier responses here, here and here.
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. |
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. |
DataContractResolver is trying to solve this. Unfortunately it requres STJ v7+. Please provide test cases for data contracts with an expected serialized output. |
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 isnull
.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
andDataContractSerializer
at this point.The text was updated successfully, but these errors were encountered: