Support filtering property names on dictionary deserialization #87868

AlexRadch · 2023-06-21T14:13:54Z

See #79059
I need some help with implementation.

dotnet-issue-labeler · 2023-06-21T14:14:03Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

ghost · 2023-06-21T14:14:09Z

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

See #79059
I need some help with implementation.

Author:	AlexRadch
Assignees:	-
Labels:	`area-System.Text.Json`, `new-api-needs-documentation`, `community-contribution`
Milestone:	-

eiriktsarpalis · 2023-06-26T14:57:49Z

...ext.Json/src/System/Text/Json/Serialization/Converters/Collection/JsonDictionaryConverter.cs

@@ -16,6 +17,12 @@ internal abstract class JsonDictionaryConverter<TDictionary> : JsonResumableConv
        private protected sealed override ConverterStrategy GetDefaultConverterStrategy() => ConverterStrategy.Dictionary;

        protected internal abstract bool OnWriteResume(Utf8JsonWriter writer, TDictionary dictionary, JsonSerializerOptions options, ref WriteStack state);
+
+        internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)


Seems like this isn't doing anything?

eiriktsarpalis · 2023-06-26T15:00:13Z

...ext.Json/src/System/Text/Json/Serialization/Converters/Collection/JsonDictionaryConverter.cs

+                    }
+                }
+
+                TKey key;


This is repeating the logic of the existing ReadDictionaryKey method. It seems like you could simply extract the DictionaryKeyFilter logic into a standalone helper?

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonDictionaryKeyFilter.cs

eiriktsarpalis · 2023-06-26T15:03:12Z

...m.Text.Json/src/System/Text/Json/Serialization/JsonIgnoreMetadataNamesDictionaryKeyFilter.cs

+{
+
+    /// <summary>
+    /// Ignores any metadata keys starting with $, such as `$schema`.


Rather than ignoring every key starting with $, I would recommend only checking for metadata property names that have built-in support in JSON:

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleMetadata.cs

Lines 19 to 22 in 5ca6826

private static readonly byte[] s_idPropertyName = "$id"u8.ToArray();

private static readonly byte[] s_refPropertyName = "$ref"u8.ToArray();

private static readonly byte[] s_typePropertyName = "$type"u8.ToArray();

private static readonly byte[] s_valuesPropertyName = "$values"u8.ToArray();

eiriktsarpalis · 2023-06-26T15:04:26Z

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs

@@ -105,6 +106,7 @@ public JsonSerializerOptions(JsonSerializerOptions options)
            // 2. _typeInfoResolverChain can be created lazily as it relies on
            //    _typeInfoResolver as its source of truth.

+            _dictionaryKeyFilter = options._dictionaryKeyFilter;


You should also update the EqualityComparer implementation in JsonSerializerOptions.Caching to account for the new property:

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs

Line 480 in 5ca6826

private sealed class EqualityComparer : IEqualityComparer<JsonSerializerOptions>

eiriktsarpalis · 2023-06-26T15:04:50Z

...raries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Dictionary.KeyFilter.cs

+            Assert.Equal(JsonValueKind.Null, myClass.ExtensionData["$metadataNull"].ValueKind);
+        }
+
+        //[Fact]


Commented out code?

…ion/JsonDictionaryKeyFilter.cs Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>

ghost · 2023-07-13T15:02:34Z

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

ghost · 2023-07-27T18:02:00Z

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.

AlexRadch added 4 commits June 20, 2023 12:21

Create JsonDictionaryKeyFilter class

ea6d4c7

Created DictionaryKeyFilterUnitTests

14c1e4d

Some tests are added

91e66c9

ConfigureJsonTypeInfo

f8da4b9

ghost added the community-contribution Indicates that the PR has been added by a community member label Jun 21, 2023

dotnet-issue-labeler bot added area-System.Text.Json new-api-needs-documentation labels Jun 21, 2023

Delete test.result.txt

cd441ee

This was referenced Jun 21, 2023

System.Text.Json failing some large file tests #59678

Closed

System.Net.Http.WinHttpHandlerFunctional.Tests.BidirectionStreamingTest.AfterReadResponseServerError_ClientWrite failing #86262

Closed

AlexRadch mentioned this pull request Jun 22, 2023

Support filtering property names on dictionary deserialization. #79059

Open

eiriktsarpalis reviewed Jun 26, 2023

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonDictionaryKeyFilter.cs Outdated Show resolved Hide resolved

eiriktsarpalis reviewed Jun 26, 2023

View reviewed changes

Update src/libraries/System.Text.Json/src/System/Text/Json/Serializat…

4bd8bec

…ion/JsonDictionaryKeyFilter.cs Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>

This was referenced Jun 26, 2023

LibraryImportGenerator.Unit.Tests crashed in CI #87951

Closed

[browser][MT] Wasm.Browser.Threads.Minimal sample is failing #88057

Closed

eiriktsarpalis added the needs-author-action An issue or pull request that requires more info or actions from the author. label Jun 28, 2023

ghost added the no-recent-activity label Jul 13, 2023

ghost closed this Jul 27, 2023

ghost locked as resolved and limited conversation to collaborators Aug 26, 2023

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support filtering property names on dictionary deserialization #87868

Support filtering property names on dictionary deserialization #87868

AlexRadch commented Jun 21, 2023

dotnet-issue-labeler bot commented Jun 21, 2023

ghost commented Jun 21, 2023

eiriktsarpalis Jun 26, 2023

eiriktsarpalis Jun 26, 2023

eiriktsarpalis Jun 26, 2023

eiriktsarpalis Jun 26, 2023

eiriktsarpalis Jun 26, 2023

ghost commented Jul 13, 2023

ghost commented Jul 27, 2023

	private static readonly byte[] s_idPropertyName = "$id"u8.ToArray();
	private static readonly byte[] s_refPropertyName = "$ref"u8.ToArray();
	private static readonly byte[] s_typePropertyName = "$type"u8.ToArray();
	private static readonly byte[] s_valuesPropertyName = "$values"u8.ToArray();

Support filtering property names on dictionary deserialization #87868

Support filtering property names on dictionary deserialization #87868

Conversation

AlexRadch commented Jun 21, 2023

dotnet-issue-labeler bot commented Jun 21, 2023

ghost commented Jun 21, 2023

eiriktsarpalis Jun 26, 2023

Choose a reason for hiding this comment

eiriktsarpalis Jun 26, 2023

Choose a reason for hiding this comment

eiriktsarpalis Jun 26, 2023

Choose a reason for hiding this comment

eiriktsarpalis Jun 26, 2023

Choose a reason for hiding this comment

eiriktsarpalis Jun 26, 2023

Choose a reason for hiding this comment

ghost commented Jul 13, 2023

ghost commented Jul 27, 2023