Closed bruno-garcia closed 2 years ago
Instead, we aim to provide good extensibility points so that third-party extensions like the one you cited can be successful.
Way to read the room, good job /s
No one is requesting the "kitchen sink" approach. The point is, the extensibility points aren't good enough to accomplish what needs to be done. The options at the moment for controlling serialization is to
JsonIgnore
attributesI'm not advocating for the design to go nuts and provide call backs like the old XML serialization ShouldSerializeXXX
methods, but supporting DataContract
and associated attributes should not be a big deal. This has been the standard way of doing this since like .NET Fx 3.
I'll explain my use case so hopefully it might make sense to you as to why we need this functionality. We have a library of data contracts to support our internal applications and exposing our surface API. These data contracts currently support both WCF and REST via Web API, using Newtonsoft
. We are also moving towards gRPC (which supports DataContract
).
We are largely on .NET (only the WCF service is .NET FX) and we want to keep our runtime stack modern and up-to-date. I don't think that Newtonsoft is going to disappear any time soon but clearly the emphasis is on System.Text.Json
. We want to use it but we can't at the moment.
I expect any one else operating in the "enterprise real world" is going to be facing similar hurdles to adoption. My DTO layer should not need to know implementation details for what is going to consume it and as such we are reluctant to litter JsonIgnore
all through it.
If they start adding things left right and center, the library will get bloated and more complicated every time.
Again, nobody is asking for this. It seems the position keeps getting pushed to the extreme of "add everything and make it newtonsoft again" when that's far from the case.
99% of the community's wishes are to support the "standard" attributes found in System.Runtime.Serialization
so that existing code can be used with a new serializer without modification. If there are fundamental incompatibilities then that's fine, but the vast majority of POCOs and simple types should just work.
Why can't we have that? Not everything... but just that?
So if I've got this right, the consensus from the implementation team is: "screw the users and developers that actually made this framework successful, we're Microsoft, we're smarter and better than you in every conceivable way, so obviously we know what you actually want, and you'll take what we give you and like it. " Congratulations, that's very Balmer of you. Does the .NET Foundation know you think so little of your customers? I imagine they and Satya would be very interested in this dismissive and frankly toxic attitude.
Here is a good example of why this is important. EF Core has to add a serialization attribute specifically for System.Text.Json
because it isn't honoring IgnoreDataMember
.
And what does that attribute do? Exactly the same thing as IgnoreDataMember
, but it's called JsonIgnore
just to be different.
Another example is when a REST API allows the client to specify the Accepts
header. The API models can use [DataMember]
for all formats except JSON, where we need to specifically add attributes from System.Text.Json
which do the same thing.
A very common mantra for .NET is that users should fall into a "pit of success". When a developer uses a library, their first instinct for how to use a library says a lot about the API design the library should have, unless there is a very good reason for the design to break away from that intuition.
When I switched to System.Text.Json
, using the DataMemberAttribute
was one of the first things I tried. I actually just expected it to work out of the box and was pretty surprised when it didn't, since almost all popular serializers written for .NET support it in some way. So I changed over to the more specific JsonPropertyNameAttribute
, and moved on.
Then I wanted to control the string values that enum values were serialized as. So I tried to use EnumMemberAttribute
, since that's the way it's done in every other serialization library, but again it didn't work. So I went to look for the System.Text.Json
specific version and realized that there isn't one. That's when I found issue https://github.com/dotnet/runtime/issues/31081 (which was closed unresolved in favor of this issue). So, three years on, System.Text.Json
still cannot change enum string values. Not only does System.Text.Json
not support the existing de-facto standard, it doesn't provide a replacement for a common usecase. Not a happy developer experience.
Performance is one of the good reasons a design can be initially unintuitive, because it's a compromise that leads to a better product. However, I see no reason why the performance goals of System.Text.Json
should be hampered by supporting System.Runtime.Serialization
, because it deals with its own built-in attributes just fine. So, there must be some other motive for this design decision, which as far as I can tell has not been specified besides the nebulous goal of keeping the library lean and unbloated.
providing built-in support for everything is an explicit anti-goal for System.Text.Json. Instead, we aim to provide good extensibility points so that third-party extensions like the one you cited can be successful.
So what usecases does System.Text.Json
actually intend to cover out of the box? Ultimately it's just a JSON serializer. It needs to actually provide enough flexibility to be able to be a JSON serializer that works in common usecases. It has been frequently stated that the goal of System.Text.Json
isn't to replace Newtonsoft.Json
, you don't want everything and "the kitchen sink" supported. However, Newtonsoft's development has been driven by actual developer needs based on real usecases for a decade. Does Newtonsoft actually have that many niche and outlandish features, so much so that it shouldn't be considered a solid reference for how System.Text.Json
should operate? How many features are in Newtonsoft that System.Text.Json
can actually justify not supporting?
If you provide "extensibility points", and then the majority of developers need to immediately bolt-on a third-party library to fulfill their usecase, is that a desirable outcome? I could understand if System.Runtime.Serialization isn't included in the core library to maintain separation of concerns, but I certainly think that the functionality is widely used enough to justify first party support via a package, at minimum.
Here's another way to think about it. We can divide the list of serializers into two categories:
.NET 1.0 Serialization Libraries
Serialization Libraries that Support Data Contracts
Json.Net is the only one that doesn't fit.
If you expect
System.Runtime.Serialization
attributes to work, a good extension apparently exists already.They say we may get the ability to write such an extension in .NET 7.
Perhaps I didn't make myself clear, this issue is very clearly listed in the use cases under the acceptance criteria for #63686. It even literally includes a code snippet showing how to do this for DataMemberAttribute
. When the feature does ship, there will be acceptance testing specifically validating that contract resolution using common System.Runtime.Serialization attributes is feasible.
When the feature does ship, there will be acceptance testing specifically validating that contract resolution using common System.Runtime.Serialization attributes is feasible.
Then take the next step and actually make it a feature.
Then take the next step and actually make it a feature.
Here's a few reasons why I think we shouldn't ship SRS attribute support OOTB:
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.
Potential quirks mapping SRS attribute data defining behaviour that is either:
NameSpace
and its variants), IsRequired
) or,DataContractSerializer
(type-level IsReference
annotation vs. global JsonSerializerOptions.ReferenceHandler
).A possible resolution to that issue might be that we chose to ignore all the square pegs, but it does somewhat undermine the seamless transition narrative. It might be an acceptable compromise in the context of a team wishing to migrate their serialization layer, but I don't think it meets the bar for quality in a component that's shipped with the shared framework.
The SRS namespace contains attributes specific to BinaryFormatter (e.g. OptionalFieldAttribute
, OnSerializingAttribute
and friends). Clearly we can't support everything under that namespace, but communicating such caveats might be tricky.
Ambiguity with DTOs mixing and matching STJ and SRS annotations. An SRS contract resolver implementation could either choose to completely ignore STJ annotations when SRS attributes are detected, or it could employ some form of ad hoc conflict resolution. I don't believe there is a canonical implementation that would absolutely satisfy every use case.
Authoring custom contract resolvers that add support for custom attributes is trivial, and there is a lot of prior art in the Json.NET ecosystem to be adapted in this context.
cc @GrabYourPitchforks
It's worth keeping in mind that whatever component we ship with the shared framework automatically crosses a threshold of immortality, as it were. It will need to preserve backward compatibility for however long .NET exists. Almost any amendment -including fixes to obviously incorrect behaviour- is a potential breaking change and as such the bar for making it is very high.
This is an important factor constraining the evolution of shared framework libraries like System.Text.Json, which unlike third-party libraries cannot adopt versioning strategies like SemVer. If System.Text.Json v7 somehow breaks handling of DataMemberAttribute.IsRequired
for a user, then that user would be blocked from upgrading their application to .NET 7.
If System.Text.Json v7 somehow breaks handling of
DataMemberAttribute.IsRequired
for a user, then that user would be blocked from upgrading their application to .NET 7.
So what you are saying is, you when you release a feature, you fail to develop adequate tests for that feature to ensure that regressions don't happen? What did this comment mean then, in which you were SO confident about the quality of your testing?
@edc-jacrys I was very specifically referring to breaking changes, not regressions. There is a difference.
@eiriktsarpalis It breaks the handling of DataMemberAttribute.ISRequired
NOW. So in essence, no there isn't, not in this case. If x doesn't work, you make a change and it does work, then you make another and it doesn't work again, that is both a breaking change AND a regression. So please stop trying to use semantic traps to justify blindness to the needs of your users.
It's worth keeping in mind that whatever component we ship with the shared framework automatically crosses a threshold of immortality, as it were. It will need to preserve backward compatibility for however long .NET exists. Almost any amendment -including fixes to obviously incorrect behaviour- is a potential breaking change and as such the bar for making it is very high.
This is an important factor constraining the evolution of shared framework libraries like System.Text.Json, which unlike third-party libraries cannot adopt versioning strategies like SemVer. If System.Text.Json v7 somehow breaks handling of
DataMemberAttribute.IsRequired
for a user, then that user would be blocked from upgrading their application to .NET 7.
@eiriktsarpalis And is this unlike when System.Text.Json v3.1 broke the functionality of DataMemberAttribute.IsRequired
and users were blocked from upgrading to .NET 3.1 because their code no longer behaved in a .NET API compliant way?
@edc-jacrys I was very specifically referring to breaking changes, not regressions. There is a difference.
@eiriktsarpalis Failing to apply proper regression testing AND breaking change testing is setting yourself up for failure, and setting the library up for failure. So again, what we are hearing from you in this thread is that
It's not a semantic trap, you are just creating a straw man. He is talking about intentional breaking changes (that is why he mentioned the "threshold of immortality") -- there will be no chance to correct any suboptimal or incorrect behavior that is discovered, nor will there be any room for the team to "change their minds" on how the feature interacts with System.Text.Json, once this is published in a stable release. Please consider reading other peoples' comments before responding to them. He is actually trying to help you -- instead of burning a policy that's opinionated by definition into this library until the end of time, he is suggesting that you use this feature they're working on to trivially define this policy yourself, so that you can tweak it to your heart's content without having to come back and disparage the .NET team for not having done exactly as you commanded.
@reflectronic It IS a semantic trap, and I am not misrepresenting what he said in any way. Please consider going back and reading other people's comments before responding to them. So what you are saying is supporting a Core API in a logical way is opinionated by definition? and putting the onus to enable support "trivially" for a Core API on the implementor is good design practice? And as far as tweaking it to your heart's content, Having built in support would actually do that. That is LITERALLY the design implementation of the System.Runtime.Serialization
library. To have a built-in serialization framework that is extensible and customizable. It is a framework DESIGNED to "be used for serializing and deserializing objects. Serialization is the process of converting an object or a graph of objects into a linear sequence of bytes for either storage or transmission to another location. Deserialization is the process of taking in stored information and recreating objects from it." Are you telling me that the above description does not describe what System.Text.Json aims to do?
He is talking about intentional breaking changes
No, he LITERALLY isn't. now you are the one not reading comments before responding He said, and I quote:
If System.Text.Json v7 somehow breaks handling
the qualifier somehow
literally describes unintentional AND intentional breaking changes
Intentional change is any change that you make on purpose, with a specific goal in mind.
You have to try to make them. If you "somehow" break them, that implies an unknown component to the change, an unintended side effect. Therefore the breaking change was not intentional, but unintentional. You did not set out with the explicit notion nor did you realize along the way you would have to break it to continue the development, you "somehow" broke it without knowing how.
@edc-jacrys I'm guessing use of the term "somehow" put you off, but yes I was talking about intentional breaking changes. Regressions are a very different conversation, I would say not particularly relevant to this discussion.
the team (or maybe just you) does NOT care that they broke anything,
Even though I'm happy to respond to your feedback, casually throwing personal insults will not help your case. Please be respectful, or leave this conversation.
@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..
@eiriktsarpalis That isn't a personal insult, that is an observation about the attitude displayed about this topic. If the statement insulted you, I would suggest a re-evaluation of your stance, or at least your approach, since that is the distinct impression you are giving throughout this conversation.
@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.
I am blunt, I prefer not dancing around using double speak to avoid offending someone. That being said, I match tone with tone. a dismissive attitude toward the people who support you is adversarial. Add that to an already adversarial tone from @eiriktsarpalis, I may have been a touch adversarial, sure.
That being said, A serialization interface (that's what this is) that doesn't support the Core API serialization contract model is a baffling design decision.
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.
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.
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.
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.
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?
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.
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.
@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.
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.
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.
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:
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 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.
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.
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.