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

Ensure metadata originating from converters is surfaced while JsonTypeInfo is mutable. #72483

Merged
merged 1 commit into from
Jul 20, 2022

Conversation

eiriktsarpalis
Copy link
Member

@eiriktsarpalis eiriktsarpalis commented Jul 19, 2022

Discovered while investigating #60560.

Some of our internal converter implementations override the JsonConverter.ConfigureJsonTypeInfo method when needing to modify contract metadata. Currently, this configuration step is run after the metadata instance gets locked, so none of these changes are visible to contract resolver authors. This PR changes this so that the method in question is invoked at metadata construction time, and only when said metadata originates from DefaultJsonTypeInfoResolver or JsonSerializerContext.

Contributes to #63686.

@ghost
Copy link

ghost commented Jul 19, 2022

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Discovered while investigating #60560.

Some of our internal converter implementations use the JsonConverter.ConfigureJsonTypeInfo method for injecting contract metadata configuration. Currently, this configuration step is done after the metadata instance gets locked, so this configuration is not visible to contract resolver authors. This PR changes this so that the method in question is invoked at metadata construction time, and only when said metadata originates from DefaultJsonTypeInfoResolver or JsonSerializerContext.

Contributes to #63686.

Author: eiriktsarpalis
Assignees: eiriktsarpalis
Labels:

area-System.Text.Json

Milestone: -

@@ -27,7 +28,7 @@ internal override object CreateObject(JsonSerializerOptions options)
JsonObject jObject = (JsonObject)obj;

Debug.Assert(value == null || value is JsonNode);
JsonNode? jNodeValue = (JsonNode?)value;
JsonNode? jNodeValue = value;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider removing assert above

@eiriktsarpalis eiriktsarpalis merged commit dfb934a into dotnet:main Jul 20, 2022
@eiriktsarpalis eiriktsarpalis deleted the jsonconverter-metadata branch July 20, 2022 11:49
@ghost ghost locked as resolved and limited conversation to collaborators Aug 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants