Circular reference detection reference equality handling #423
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… instances when checking for circular references in a object graph.
…changing the current bahavior unconditionally would cause a breaking change, this change set add the ability to override the behavior by providing a new setting when configuring the serializer. All the configuration paths are supported(JsonSerializerSettings, Json*Attribute, JsonContract, JsonProperty, etc...). See the ReferenceComparisonHandling enum. [ADDED]: ReferenceComparisonHandlingTests
… specific framework versions
… instances when checking for circular references in a object graph.
…changing the current bahavior unconditionally would cause a breaking change, this change set add the ability to override the behavior by providing a new setting when configuring the serializer. All the configuration paths are supported(JsonSerializerSettings, Json*Attribute, JsonContract, JsonProperty, etc...). See the ReferenceComparisonHandling enum. [ADDED]: ReferenceComparisonHandlingTests
… specific framework versions
JsonContainerAttribute.ReferenceComparisonHandling and JsonSerializerSettings.ReferenceComparisonHandling [FIXED]: In JsonSerializer.applySerializerSettings, settings.ReferenceComparisonHandling was used when checking if the user provided a value for the setting instead of using settings._referenceLoopHandling.
|
I agree. I think the default behavior is wrong, but this solution is agreeable. Perhaps 7.0 can make the default use I'm on a project that needs this enhancement - any idea when it will be merged? |
|
Added EqualityComparer to JsonSerializer 3cc797c |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I' ve closed the previous pull request and opened a new one.
I still think that the current behavior(see #401) IS very fragile.
I undestand that if people are depending on it, the default
behavior cannot be changed, but maybe it should be at least overridable.
In this pull request i've added a new enum(ReferenceComparisonHandling) and a new property that allow to override the behavior, available to all the various levels available for other settings like ReferenceLoopHandling, ItemReferenceLoopHandling etc... that specify the method that will be used to compare references during the circular reference detection phase.
If the settings is not provided the old behavior is used.
and of course all the other classes that partecipate in the serialization pipeline(JsonProperty,JsonContract, etc...).
I've added a new set of unit test that verify the intended behavior as the guidelines indicates.
This could have also been implemented in a different way, by creating a new service (like IReferenceResolver) that allow to change the default behavior, but i didn't want to change the code to much. If you prefer this solution let me know , i can make another pull request.
Thanks.