-
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
Proposal: Add mechanism to handle circular references when serializing #30820
Comments
@ahsonkhan @JamesNK thoughts? |
Consider changing "values" in the array metadata object to "$values" to indicate it isn't a real property. {
"$id": "1",
"Name": "Angela",
"Subordinates": {
"$id": "2",
"$values": [
{
"$id": "3",
"Name": "Bob",
"Manager": {
"$ref": "1"
}
}
]
}
} Instead of a dedicated property, what about adding ReferenceHandling as a property to [JsonReferenceHandling(ReferenceHandling.Preserve)]
// to
[JsonProperty(ReferenceHandling = ReferenceHandling.Preserve)] |
Agree.
|
Correct, we have cc @steveharter |
Is this proposal compatible with Json.Net? |
Why |
What determines if objects are equal? object.ReferenceEquals or object.Equals? Newtonsoft.Json uses object.Equals. Is there a way to override equality? Newtonsoft.Json has https://www.newtonsoft.com/json/help/html/P_Newtonsoft_Json_JsonSerializerSettings_EqualityComparer.htm |
What happens if there are additional properties in a JSON reference object? (has What happens if |
Yes for the metadata semantics that I am trying to follow; no for a feature parity, however we are not tied to follow the first one, just come up with what is already there since I have not found value in going on a different direction.
I was following existing enums in the namespace that also inherits from byte, thing that is made to reduce their size in the |
Currently the implementation relies on default equality of
Json.Net throws when finds that there are properties other than $ref in a JSON reference object, I was planning on sticking to that behavior. |
@JamesNK on "Ignore" - is this mode necessary and what scenarios would prefer that over "Preserve"? "Ignore" has potnential inconsistentcy w.r.t. what is serialized vs. what is ignored which would make me question using it in certain scenarios including:
In many scenarios the [JsonIgnore] could be used instead to prevent cycles which has the advantage of being deterministic. However if:
then "Ignore" makes sense. |
We seem to lean towards not having We'd like to see more detailed spec for the behavior, specifically error cases. |
Ok, I don't feel strongly that ignore is necessary. And it should never be the default setting so you can always add it in the future if desired. |
Hi @jozkee , is there a way to try PreserveReferencesHandling, like in a nuget preview? |
@paulovila this feature is still a work in progress, once is finished and merged into our master branch you can try it out by installing the latest bits of Net core. |
This is extremely disappointing, it was called out by numerous people while 3.0 was in preview as a complete blocker for a lot of people to use the new serializer and benefit from it. It’s even more disappointing this still isn’t being prioritized properly. It seems like by the time this gets addressed people will have just abandoned the new serializer altogether and likely won’t return to it. |
What gave you that impression? We are actively working on adding this feature for 5.0. |
IMO this is a major breaking change and it actually roadblocks a lot of people from following the recommended serializer setup from the docs. I’m frustrated because despite this being called out in preview there’s no plan to address in 3.0 or even 3.1 that I’ve seen. Waiting for 5.0 is way too far out on the roadmap and why I said it still isn’t being prioritized properly. |
Edit: Deleted the other comments. Are meaningless!! I was following the wrong crumbles... |
This comment has been minimized.
This comment has been minimized.
Isn't 5.0 a year away? |
No. We are currently in 3.0/3.1! Next year, around this time, .NET 5 will be released. They are gonna skip a number (the number 4) so we don't get confused with .NET Framework 4.* Next year, there will be one .NET to "rule us all" |
I understand that, but you mention that this change is earmarked for 5.0, which I'm assuming is .Net 5.0, which is a year away. |
This comment has been minimized.
This comment has been minimized.
"Next year, around this time" is just another way of saying "a year away"... |
Yes, this feature is for 5.0 (i.e. master), which is planned to ship near end of next year (hence the milestone). 3.1 is already done. That said, S.T.Json also ships as a NuGet package so once the feature is done, you could consider using a preview package on 3.1 before 5.0 officially ships. |
It's complicated to depend on the newtonsoft serialisation patch because It is not being updated at the same pace as the SignalR patch is, and that causes that we can’t upgrade to the intermediate versions that you provide. 🤷♂️ |
@paulovila - I am sorry, but I don't understand what you mean. What target framework are you using in your project?
Let's say you have a .NET Core/Asp.Net Core 3.1 application. The built-in |
@ahsonkhan the aforementioned patches are: <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="3.0.0" />
<PackageReference Include="Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson" Version="3.0.0" /> with that you can circumvent the circular references as follows: using Microsoft.AspNetCore.Mvc;
using Newtonsoft.Json;
...
services.AddMvc()
.AddNewtonsoftJson(options =>
{
options.SerializerSettings.NullValueHandling = NullValueHandling.Ignore;
options.SerializerSettings.PreserveReferencesHandling = PreserveReferencesHandling.Objects;
})
.SetCompatibilityVersion(CompatibilityVersion.Version_3_0);
services.AddSignalR().AddNewtonsoftJsonProtocol(options =>
{
options.PayloadSerializerSettings.NullValueHandling = NullValueHandling.Ignore;
options.PayloadSerializerSettings.PreserveReferencesHandling = PreserveReferencesHandling.Objects;
}); Is this patch strategy the right way to go? I can see now that there has been some alignment in the nuget preview versions since I tried version 3.0.0.0. |
Here: https://docs.microsoft.com/en-us/ef/core/querying/related-data/serialization Literally, straight out of the official MS documentation. |
@AlbertoPa I think for that example you can set the |
@BrunoBlanes I have tried it but it does not seem to work if there are many-to-many relationships. Currently I have two workarounds:
or
option on the server. In a Blazor app, the System.Text.Json deserializer seems to be OK with this trick so far. |
@AlbertoPa Could it be that you may be doing something wrong? I got it working fine o Blazor, I just had an issue with naming policies, but it was my fault. |
@BrunoBlanes certainly possible, but I don't see where. I have made a basic case where the join entity has only id's and navigation properties. If the EF query populates the IList of join entities and the corresponding navigation properties (meaning Include and ThenInclude), I can't get it to work properly even if I specify the Preserve option. What was the issue with naming policies? I am using the standard naming convention in EF Core. |
Forgot to set the naming policies: #45639 What error are you getting? |
This is what I get:
I haven't tried to play with naming policies. Will give it a try. |
Check if the JSON you are trying to deserialize is an actual list, then check your JSON for all of |
@AlbertoPa also if you could share a minimal standalone repro (potentially with data sanitization), with the JSON you are processing and the .NET object model used for serialization, folks on the thread might be able to help investigate. |
It appears I am having the same issue as AlbertoPa. I've posted the issue several places and so far have received no solution. I don't want to clutter up this post anymore than necessary so I'm wondering if we can start here... I have a Blazor WebAssembly application that so far has been working fine in everything I have implemented so far. The last thing I did was created a couple new classes in the Shared project that represent a parent/child relationship and my parent has a property for a collection of child objects and the child class has a property referencing its parent. I am also using EF. Once I implemented all of this and tried to run the app and browse to the page i got the following error at which point I fired up the Googler in search or a solution: • JsonException: A possible object cycle was detected which is not supported. This can either be due to a cycle or if the object depth is larger than the maximum allowed depth of 32. So my first question would be, is this expected behavior for a default implementation of a Blazor WebAssembly application? If so, can someone please post what the specific steps are to resolve this issue? Based on what I've read so far, it looks to me like this is the expected behavior and that a solution was implemented for it which is what this post represents. It looks to me like it states a solution was implemented for it as part of .NET 5 and that the solution that I would want would be to set ReferenceHandling = ReferenceHandling.Preserve. If wanted, i can go into the details of what I have tried so far, etc. and give as much detail as possible but the approach I would prefer taking is someone just posting the specific steps of what needs to be done. FYI, when i tried implementing the code changes that it looked to me like i needed to make based on this post one or two other sites I read all that did was instead of getting the error I mentioned above, I then got the error that AlbertPa mentioned. I would not THINK this would be related to naming policies since, as i mentioned, everything else up to this point had been working fine unless the issues resolved by naming policies would only come into play in my type situation when there are circular references involved. Thanks. |
@ryantgray That is the default behavior for |
I could somewhat address the issue with the change in naming convention you suggested. This however resulted in larger payload than with Ignore in Newtonsoft. Also, while not a bug, there is no way to have global options in the client, so it means changing the whole code base. Definitely not making Text.Json a plug in replacement, so for now I'll stay with my hybrid solution, using Newtonsoft on the server. |
Apparently there are some risks of data loss with
For this I think you could create an extension method. |
Yes, about extension methods. The rest has been standard practice for years without issues (it's literally recommended by EF doc). I'd say leave to the developer and implement the functionality. Until then, Text.Json isn't a replacement for Newtonsoft without extra work, and the benefit of performance may not be of interest due to the extra work needed. My two cents. |
If you are not happy you can always not use S.T.Json, it is completely optional. |
It is not a question of happiness, but of functionality. This is telling users to implement custom code for basic functionality, and I'd say having a global way to pass options to a serializer/deserializer also on the client is not asking too much for a framework which heavily relies on those operations. Frankly speaking, looking at all the threads and on StackOverflow here on this issue, there has been a lot of energy wasted arguing that "Ignore is not needed". Probably the same energy could have been channelled into implementing the functionality, and resolve the issue. |
@BrunoBlanes Here are the issue numbers I have read and/or commented on regarding this issue: It looked to me like the fix for the problem was to update System.Net.Http.Json to version 5.0.0 in the client project and then services.AddControllersWithViews().AddJsonOptions(options => or services.AddControllersWithViews(options => Am i on the right track here? Is there anything else that needs to be done? Do i also need to update my Server project |
@ryantgray I thionk you are on the right track. You shouldn't need to update to .Net 5, as long as you're running STJ 5 then you should be good. (I believe the ReferenceHandler flag didn't work at all in v3) Preserve is an excellent choice for retaining the full object tree, although it absolutely has drawbacks, as well as advantages: Drawbacks
Advantages
The conversation continues over on #40999This issue was closed a while ago so you're best posting in there. |
@butler1233 Thanks Lee but your comment did not address my issue. I don't really care about the advantages and disadvantages. I only care about getting it to work. It seems like it should be easy to have someone confirm that you guys implemented changes in JSON 5.0 to address the scenario I'm mentioning and, if so, what needs to be done in a standard default implementation of Blazor WebAssembly that Visual Studio creates to implement it. Also, you comment "this issue was closed a while ago so you're best posting in 40999" but that is closed as well and seems to have nothing to do with my issue. Can someone please just give me the specific steps including the code changes that need to be made, or point me to them, for what needs to be done in a default implementation of Blazor WebAssembly to set JSON Reference Handling to Preserve? Then, if i have done everything you guys say needs to be done and am still having issues I will take the necessary steps to address them at that point but the first thing i want to do is make sure I have all the necessary changes implemented correctly. Thanks. |
@ryantgray Again, as mentioned so many times in this thread, |
@BrunoBlanes It feels like there's a communication disconnect here. Maybe your getting me confused with AlbertoPa? I have never inquired about Ignore. My inquiry was always about Preserve and no, i have never gotten Preserve working. I understand this is not the place to learn code and did start out posting on places like Stackoverflow. However, noone has been able to provide any insight, possibly because you just released your changes to JSON within the last month and the combination of that and Blazor being so new...? If there is a better place/way to address my issue I'd be more than happy to pursue it if i knew what it was. However, I'm trying to implement some functionality released by Microsoft that is not working and so far, noone else seems to have any answers and i'm not even 100% sure the changes you guys made will address the situation in a Blazor WebAssembly application so I feel Microsoft may be the only entity that can address this. |
@ryantgray Didn't this fix it for you? |
That's the way it looked to me to based on the links I provided but no it did not fix it. |
If you are using Blazor, do also set |
@BrunoBlanes Thanks, I tried implementing your suggestion with both of the code options I posted and I still get the same errors. |
Serverservices.AddControllersWithViews().AddJsonOptions(options =>
{
options.JsonSerializerOptions.ReferenceHandler = System.Text.Json.Serialization.ReferenceHandler.Preserve;
options.JsonSerializerOptions.PropertyNamingPolicy = null;
}); Clientvar response = await Http.GetFromJsonAsync<T>("{Address}", new JsonSerializerOptions
{
ReferenceHandler = System.Text.Json.Serialization.ReferenceHandler.Preserve,
PropertyNamingPolicy = null
}); Try this and let me know what error are you getting. |
I observe a reduction in size in relatively small models or when the EF navigation properties are not fully populated. When pulling complex models, that does not seem to be the case. One example: if I pull an incomplete object with a query, to list objects for CRUD operations in a table, then System.Text.Json gives a smaller payload than Newtonsoft.Json (up to 11 Bytes smaller in my tests over 766 Bytes of objects: 1.4% difference). However, when populating all related data, the situation inverts: System.Text.Json generates a payload 8.27% larger than Newtonsoft. This may be OK if the data size is modest, but it isn't negligible for larger models.
This is absolutely the case, hence why it would be good to be able to use it without struggling 😆 . The advantage on the server side is clearly visible when serializing large amounts of data. Edit: Bruno's latest response works, also with
at least in my cases. I seem to have some issue when the objects are self-referencing, but I need to troubleshoot more to find what is causing it, because it is not happening systematically 🤷♂️ |
Ah, my apologies, it looks like I linked to the wrong issue. Should have been #40099.
They didnt make the changes to add something like Ignore in 5, although as discussed in that (hopefully correctly linked) issue it's being considered for 6.
…________________________________
From: Ryan Gray <notifications@github.com>
Sent: Sunday, 13 December 2020, 17:01
To: dotnet/runtime
Cc: Lee Butler; Mention
Subject: Re: [dotnet/runtime] Proposal: Add mechanism to handle circular references when serializing (#30820)
@butler1233<https://github.com/butler1233> Thanks Lee but your comment did not address my issue. I don't really care about the advantages and disadvantages. I only care about getting it to work. It seems like it should be easy to have someone confirm that you guys implemented changes in JSON 5.0 to address the scenario I'm mentioning and, if so, what needs to be done in a standard default implementation of Blazor WebAssembly that Visual Studio creates to implement it. Also, you comment "this issue was closed a while ago so you're best posting in 40999" but that is closed as well and seems to have nothing to do with my issue.
Can someone please just give me the specific steps including the code changes that need to be made, or point me to them, for what needs to be done in a default implementation of Blazor WebAssembly to set JSON Reference Handling to Preserve? Then, if i have done everything you guys say needs to be done and am still having issues I will take the necessary steps to address them at that point but the first thing i want to do is make sure I have all the necessary changes implemented correctly. Thanks.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#30820 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADZLJXK4TPBN7VEBZIYGF6DSUTXONANCNFSM4KY4YW5Q>.
|
@ryantgray, it seems to me that @BrunoBlanes' suggestion should cover your problem.
You need to use at least version 5.0.0 of System.Text.Json in order to be able to use the in case you are still having issues, kindly can you please share it with us here and/or in a new issue tagging me. A standalone repro (using the Blazor template for example) would also be very helpful for us. |
See initial proposal with extended comments here:
https://github.com/dotnet/runtime/pull/354/files
See proposal extension for
ReferenceResolver
on #30820 (comment).Rationale and Usage
Currently there is no mechanism to prevent infinite looping in circular objects nor to preserve references when using System.Text.Json.
Community is heavily requesting this feature since is consider by many as a very common scenario, specially when serializing POCOs that came from an ORM Framework, such as Entity Framework; even though JSON specifiacation does not support reference loops by default. Therefore this will be shipped as an opt-in feature.
The current solution to deal with reference loops is to rely in MaxDepth and throw a
JsonException
after it is exceeded. Now, this is a decent and cheap solution but we will also offer other not-so-cheap options to deal with this problem while keeping the current one in order to not affect the out-of-the-box performance.Proposed API
EDIT:
We considered having
ReferenceHandlig.Ignore
but it was cut out of the final design due the lack of scenarios where you would really needIgnore
overPreserve
.Although is not part of the shipping API, the samples and definitions of
Ignore
remain in this description for their informative value.In depth
Default:
Ignore:
Preserve:
Preserve
does affect its behavior with the following: Metadata will be expected (although is not mandatory) and the deserializer will try to understand it.Feature Parity (Examples of System.Text.Json vs Newtonsoft's Json.Net)
Having the following class:
Using Ignore on Serialize
On System.Text.Json:
On Newtonsoft's Json.Net:
Output:
Using Preserve on Serialize
On System.Text.Json:
On Newtonsoft's Json.Net:
Output:
Using Preserve on Deserialize
On System.Text.Json:
On Newtonsoft's Json.Net:
Notes:
ReferenceHandling.Ignore
orReferenceHandling.Preserve
.ReferenceLoopHandling
andPreserveReferencesHandling
(we are also not including the granularity on this one) into one single enum;ReferenceHandling
.System.Array
s can be Serialized with Preserve semantics, they will not be supported when trying to Deserialize them as a reference.ReferenceResolver
,JsonPropertyAttribute.IsReference
andJsonPropertyAttribute.ReferenceLoopHandling
, that build on top ofReferenceLoopHandling
andPreserveReferencesHandling
were considered but they will not be included in this first effort.ReferenceHandling.Ignore
.Issues related:
The text was updated successfully, but these errors were encountered: