Add JsonDictionaryKeyFilter to support filtering property names on dictionary deserialization#124648
Add JsonDictionaryKeyFilter to support filtering property names on dictionary deserialization#124648
Conversation
…keys Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
|
@copilot please address the failing tests. |
…ests Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
Fixed in 0f1bab6. The two failing tests were:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot please address the merge conflicts |
Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
Resolved in f236d93. The only conflict was in |
|
@copilot please address the most recent merge conflicts |
Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
Resolved in 0597bb2. Three files had conflicts from the
|
JSON documents containing schema metadata (e.g.
$schema,$id) fail to deserialize into generic dictionary types because all JSON property values must conform toTValue. This adds aJsonDictionaryKeyFilterAPI that lets callers skip arbitrary keys during dictionary deserialization.Description
New API:
System.Text.Json.Serialization.JsonDictionaryKeyFilter— abstract filter class:IgnoreMetadataNamesstatic property: built-in filter that skips any key starting with$abstract bool IgnoreKey(ReadOnlySpan<byte> utf8JsonPropertyName)— override to implement custom filteringJsonSerializerOptions.DictionaryKeyFilter— wires the filter into deserializationUsage:
Implementation details:
!SupportContinuation && !CanContainMetadata): filter check +SkipWithVerify— safe since the entire JSON is buffered.TrySkipPartialwith a newReadStackFrame.SkipCurrentDictionaryKeybool for resumable continuation across buffer boundaries. Filtered keys bypassReadDictionaryKey, so unconvertible key strings (e.g."$schema"withTKey = int) never cause conversion errors.JsonSerializerOptionsequality comparer and hash code updated to include_dictionaryKeyFilter.Existing test updates:
Two existing tests that enumerate all public settable properties of
JsonSerializerOptionsvia reflection were updated to account for the newDictionaryKeyFilterproperty:CacheTests.JsonSerializerOptions_EqualityComparer_ChangingAnySettingShouldReturnFalse— addedDictionaryKeyFilterentry toGetPropertiesWithSettersAndNonDefaultValues.OptionsTests.CopyConstructor_CopiesAllPublicProperties— added an explicitJsonDictionaryKeyFilterhandler inGetFullyPopulatedOptionsInstance(the method usesActivator.CreateInstancefor unknown types, which fails for abstract classes).Merge conflict resolutions:
Merge conflicts with main were resolved twice as main introduced new features in the
System.Text.Jsonarea:ref/System.Text.Json.cs— preservedJsonDictionaryKeyFilteralongside new entries from main (JsonIgnoreAttributeattribute usage expansion,JsonNamingPolicyAttribute).gen/JsonSourceGenerator.Parser.cs— accepted main's version (our PR does not modify this file; conflicts arose from a newtypeNamingPolicyparameter and pre-existing helper methods).src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs— accepted main's version (our PR does not modify this file; conflicts arose from a newtypeNamingPolicyparameter threading).Testing
CollectionTests.Dictionary.KeyFilter.cscoveringIgnoreMetadataNames, custom filters, null filter, all-keys-filtered,IReadOnlyDictionary, copy constructor independence, and small-buffer async stream deserialization.Original prompt
This section details on the original issue you should resolve
<issue_title>Support filtering property names on dictionary deserialization.</issue_title>
<issue_description>EDIT See #79059 (comment) for API proposal.
It seems there is currently no way to make JSON documents that reference a schema just deserializa.
Repro
{ "$schema": "http://example.org/myschema.json", "Key1": { "V1": 10, "V2": "Ten" }, "Key2": { "V1": 20, "V2": "Twenty" } }Expected Behavior
It just works. Or, based on
JsonSerializerOptions, it could be configured to just work.Actual Behavior
It causes an exception:
In order to make it deserializable, the easiest option is to manually remove the schema node from the document:
The more performant alternative would be to write a custom converter, but that seems like a lot of work. If the root type of the document weren't dictionary but a type I control, then presumably I could work this around by adding a property to my class and decorate it with
[JsonPropertyName("$schema")]but that feels equally hacky.Customer Evidence
It’s not entirely clear to me if this is worth prioritizing. On the one hand, it seems a decent amount of our customers might use JSON schemas (based on this super scientific poll below).
On the other hand, it would only impact customers who use dictionaries as the document type. That combination might be quite rare and may not be be worth handling directly. And based on James’s comments below, JSON.NET never handled that either. I suggest we leave it open for a bit and see what responses we get but my gut feel says cutting seems fine.
</issue_description>
Comments on the Issue (you are @copilot in this section)
@ Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis See info in [area-owners.md](https://github.com/dotnet/runtime/blob/main/docs/area-owners.md) if you want to be subscribed.Issue Details
It seems there is currently no way to make JSON documents that reference a schema just deserializa.
Repro
{ "$schema": "http://example.org/myschema.json", "Key1": "Value1", "Key2": "Value2" }Expected Behavior
It just works. Or, based on
JsonSerializerOptions, it could be configured to just work.Actual Behavior
It causes an exception:
In order to make it deserializable, the easiest option is to manually remove the schema node from the document:
The more performant alternative would be to write a custom converter, but that seems like a lot of work. If the root type of the document weren't dictionary but a type I control, then presumably I could work this around by adding a property to my class and decorate it with
[JsonPropertyName("$schema")]but that feels equally hacky.area-System.Text.JsonI'd recommend people load the JSON into a JObject and remove the
$schemaproperty and then deserialize it.</comment_new><comment_new>@eiriktsarpalis
A few observations:
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.