Implement the JsonSerializer.IsReflectionEnabledByDefault feature switch #83844

eiriktsarpalis · 2023-03-23T19:52:49Z

ghost · 2023-03-23T19:53:03Z

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

Fix #83279.

Note that this scales back the scope of the API as approved, since populating a resolver at the default constructor creates usability issues when used in conjunction with TypeInfoResolverChain collection initializers:

var options = new JsonSerializerOptions { TypeInfoResolverChain = { MyContextA.Default, MyContextB.Default } };
Assert.Equal(new [] { DefaultJsonTypeInfoResolver, MyContextA.Default, MyContextB.Default }, options.TypeInfoResolverChain);

Instead, this PR simply wires the feature switch to the JsonSerializer methods, disabling the delayed TypeInfoResolver population logic.

Author:	eiriktsarpalis
Assignees:	eiriktsarpalis
Labels:	`area-System.Text.Json`
Milestone:	-

src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs

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

krwq

LGTM

src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs

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

src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml

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

src/libraries/System.Text.Json/src/System.Text.Json.csproj

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs

src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs

src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs

.../System.Text.Json/tests/System.Text.Json.TrimmingTests/System.Text.Json.TrimmingTests.csproj

...libraries/System.Text.Json/tests/System.Text.Json.TrimmingTests/UseReflectionDefaultFalse.cs

.../System.Text.Json/tests/System.Text.Json.TrimmingTests/System.Text.Json.TrimmingTests.csproj

…eature switch to match namespace.

eiriktsarpalis · 2023-04-03T14:47:04Z

Holding off from merging until the revised API has been approved.

src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs

...em.Text.Json/tests/System.Text.Json.Tests/TrimmingTests/IsReflectionEnabledByDefaultFalse.cs

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj

…ion/JsonSerializer.Helpers.cs Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

…immingTests/IsReflectionEnabledByDefaultFalse.cs Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

eerhardt

Nice work, @eiriktsarpalis.

LGTM

Adds a msbuild property that ties to the feature switch implemented in dotnet/runtime#83844. Default JsonSerializerIsReflectionEnabledByDefault in ASP.NET PublishAot apps to start the replacement of the ASP.NET JSON feature switch. Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

eiriktsarpalis self-assigned this Mar 23, 2023

dotnet-issue-labeler bot added the area-System.Text.Json label Mar 23, 2023

eiriktsarpalis requested review from eerhardt, vitek-karas and krwq March 23, 2023 19:53

eiriktsarpalis added this to the 8.0.0 milestone Mar 23, 2023

eiriktsarpalis commented Mar 23, 2023

View reviewed changes

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

eiriktsarpalis commented Mar 23, 2023

View reviewed changes

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

brunolins16 mentioned this pull request Mar 23, 2023

Changing reflection-based json configuration dotnet/aspnetcore#46303

Closed

This was referenced Mar 23, 2023

Roslyn source generator crash on mono/linux/arm64 #81123

Closed

WasmTestOnBrowser-System.* test failures in CI #83655

Closed

krwq approved these changes Mar 24, 2023

View reviewed changes

vitek-karas reviewed Mar 24, 2023

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs Show resolved Hide resolved

vitek-karas reviewed Mar 24, 2023

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs Show resolved Hide resolved

vitek-karas reviewed Mar 24, 2023

View reviewed changes

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

vitek-karas reviewed Mar 24, 2023

View reviewed changes

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

jkotas reviewed Mar 24, 2023

View reviewed changes

src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml Outdated Show resolved Hide resolved

eerhardt reviewed Mar 24, 2023

View reviewed changes

Sergio0694 reviewed Mar 25, 2023

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/AppContextSwitchHelper.cs Show resolved Hide resolved

eiriktsarpalis mentioned this pull request Mar 27, 2023

Add an AppContext switch that disables using reflection by default in JsonSerializerOptions #83279

Closed

eiriktsarpalis changed the title ~~Implement the STJ.DisableDefaultReflection feature switch~~ Implement the STJ.UseReflectionDefault feature switch Mar 27, 2023

eerhardt reviewed Mar 27, 2023

View reviewed changes

src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml Outdated Show resolved Hide resolved

src/libraries/System.Text.Json/src/ILLink/ILLink.Substitutions.xml Outdated Show resolved Hide resolved

build-analysis bot mentioned this pull request Mar 27, 2023

CI failure STRICT_JS doesn't work with MODULARIZE or EXPORT_ES6 #83986

Closed

brunolins16 reviewed Mar 28, 2023

View reviewed changes

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs Outdated Show resolved Hide resolved

eiriktsarpalis force-pushed the feature/json-reflection-flag branch from ca49b6c to cae73ce Compare March 28, 2023 15:49

eerhardt reviewed Mar 28, 2023

View reviewed changes

.../System.Text.Json/tests/System.Text.Json.TrimmingTests/System.Text.Json.TrimmingTests.csproj Outdated Show resolved Hide resolved

eerhardt reviewed Mar 29, 2023

View reviewed changes

eiriktsarpalis added 2 commits April 3, 2023 15:45

Implement the STJ.DisableDefaultReflection feature switch.

7fc40ff

Reinstate accidentally stripped attribute

6d2af10

eiriktsarpalis added 7 commits April 3, 2023 15:45

Address feedback.

ae5af27

Address feedback.

6095616

Add a trimming test for STJ

67535b3

Move trimming test to existing trimming tests folder.

c7dac06

Add source gen serialization test case to Trimming test.

dbf6d36

Fix style.

145207c

Expose the feature switch as a property on JsonSerializer -- rename f…

1d9f86b

…eature switch to match namespace.

eiriktsarpalis force-pushed the feature/json-reflection-flag branch from 920da20 to 1d9f86b Compare April 3, 2023 14:45

eiriktsarpalis changed the title ~~Implement the STJ.UseReflectionDefault feature switch~~ Implement the JsonSerializer.IsReflectionEnabledByDefault feature switch Apr 3, 2023

eiriktsarpalis added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 3, 2023

build-analysis bot mentioned this pull request Apr 3, 2023

[wasm] interpreter timeouts when WebSocket closes unexpectedly #84101

Closed

eerhardt mentioned this pull request Apr 3, 2023

Stop Calling JSON GetTypeInfo with the runtime type dotnet/aspnetcore#47548

Closed

eiriktsarpalis removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 4, 2023

eerhardt reviewed Apr 4, 2023

View reviewed changes

eiriktsarpalis and others added 4 commits April 5, 2023 15:35

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

7dedca9

…ion/JsonSerializer.Helpers.cs Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

Update src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Tr…

4615116

…immingTests/IsReflectionEnabledByDefaultFalse.cs Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

Address feedback.

c9db1f3

Address feedback.

e643ec2

eiriktsarpalis mentioned this pull request Apr 5, 2023

Add a JsonSerializerIsReflectionEnabledByDefault property dotnet/sdk#31626

Merged

Add entry to feature-switches.md

af0131b

eerhardt approved these changes Apr 5, 2023

View reviewed changes

eiriktsarpalis mentioned this pull request Apr 5, 2023

Disable JsonSerializer.IsReflectionEnabledByDefault on PublishTrimmed=true #84378

Closed

eiriktsarpalis merged commit 0f11212 into dotnet:main Apr 5, 2023

eiriktsarpalis deleted the feature/json-reflection-flag branch April 5, 2023 20:21

ghost locked as resolved and limited conversation to collaborators May 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the JsonSerializer.IsReflectionEnabledByDefault feature switch #83844

Implement the JsonSerializer.IsReflectionEnabledByDefault feature switch #83844

eiriktsarpalis commented Mar 23, 2023 •

edited

Loading

ghost commented Mar 23, 2023

krwq left a comment •

edited

Loading

eiriktsarpalis commented Apr 3, 2023

eerhardt left a comment

Implement the JsonSerializer.IsReflectionEnabledByDefault feature switch #83844

Implement the JsonSerializer.IsReflectionEnabledByDefault feature switch #83844

Conversation

eiriktsarpalis commented Mar 23, 2023 • edited Loading

ghost commented Mar 23, 2023

krwq left a comment • edited Loading

Choose a reason for hiding this comment

eiriktsarpalis commented Apr 3, 2023

eerhardt left a comment

Choose a reason for hiding this comment

eiriktsarpalis commented Mar 23, 2023 •

edited

Loading

krwq left a comment •

edited

Loading