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

Implement the JsonSerializer.IsReflectionEnabledByDefault feature switch #83844

Merged
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address feedback.
eiriktsarpalis committed Apr 3, 2023
commit ae5af278f47577950d83cb2674a5ef1b09649db0
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
<linker>
<assembly fullname="System.Text.Json">
<type fullname="System.Text.Json.AppContextSwitchHelper">
<method signature="System.Boolean get_IsDefaultReflectionDisabled()" body="stub" value="true" feature="System.Text.Json.Serialization.DisableDefaultReflection" featurevalue="true"/>
<method signature="System.Boolean get_UseReflectionDefault()" body="stub" value="true"
feature="System.Text.Json.Serialization.UseReflectionDefault" featurevalue="true"/>

<method signature="System.Boolean get_IsSourceGenReflectionFallbackEnabled()" body="stub" value="true"
feature="System.Text.Json.Serialization.EnableSourceGenReflectionFallback" featurevalue="true"/>
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
</type>
</assembly>
</linker>
Original file line number Diff line number Diff line change
@@ -5,11 +5,11 @@ namespace System.Text.Json
{
internal static class AppContextSwitchHelper
{
public static bool IsDefaultReflectionDisabled { get; } =
public static bool UseReflectionDefault { get; } =
AppContext.TryGetSwitch(
switchName: "System.Text.Json.Serialization.DisableDefaultReflection",
switchName: "System.Text.Json.Serialization.UseReflectionDefault",
isEnabled: out bool value)
? value : false;
? value : true;

public static bool IsSourceGenReflectionFallbackEnabled { get; } =
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
AppContext.TryGetSwitch(
Original file line number Diff line number Diff line change
@@ -45,7 +45,7 @@ public JsonConverter GetConverter(Type typeToConvert)
ThrowHelper.ThrowArgumentNullException(nameof(typeToConvert));
}

if (!AppContextSwitchHelper.IsDefaultReflectionDisabled && _typeInfoResolver is null)
if (AppContextSwitchHelper.UseReflectionDefault && _typeInfoResolver is null)
{
// Backward compatibility -- root & query the default reflection converters
// but do not populate the TypeInfoResolver setting.
Original file line number Diff line number Diff line change
@@ -705,7 +705,7 @@ public void MakeReadOnly()
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
internal void ConfigureForJsonSerializer()
{
if (!AppContextSwitchHelper.IsDefaultReflectionDisabled)
if (AppContextSwitchHelper.UseReflectionDefault)
{
// Even if a resolver has already been specified, we need to root
// the default resolver to gain access to the default converters.
@@ -855,13 +855,13 @@ private static JsonSerializerOptions GetOrCreateDefaultOptionsInstance()
{
var options = new JsonSerializerOptions
{
// Because're marking the default instance as read-only,
// Because we're marking the default instance as read-only,
// we need to specify a resolver instance for the case where
// reflection is disabled by default: use one that returns null for all types.

TypeInfoResolver = AppContextSwitchHelper.IsDefaultReflectionDisabled
? JsonTypeInfoResolver.Combine()
: DefaultJsonTypeInfoResolver.RootDefaultInstance(),
TypeInfoResolver = AppContextSwitchHelper.UseReflectionDefault
? DefaultJsonTypeInfoResolver.RootDefaultInstance()
: new JsonTypeInfoResolverChain(),

_isReadOnly = true,
};
Original file line number Diff line number Diff line change
@@ -483,13 +483,13 @@ public static void Options_JsonSerializerContext_DoesNotFallbackToReflection()

[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void Options_DisableDefaultReflectionSwitch_DefaultOptionsDoesNotSupportReflection()
public static void Options_DisablingUseReflectionDefaultSwitch_DefaultOptionsDoesNotSupportReflection()
{
var options = new RemoteInvokeOptions
{
RuntimeConfigurationOptions =
{
["System.Text.Json.Serialization.DisableDefaultReflection"] = true
["System.Text.Json.Serialization.UseReflectionDefault"] = false
}
};

@@ -519,13 +519,13 @@ public static void Options_DisableDefaultReflectionSwitch_DefaultOptionsDoesNotS

[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void Options_DisableDefaultReflectionSwitch_NewOptionsDoesNotSupportReflection()
public static void Options_DisablingUseReflectionDefaultSwitch_NewOptionsDoesNotSupportReflection()
{
var options = new RemoteInvokeOptions
{
RuntimeConfigurationOptions =
{
["System.Text.Json.Serialization.DisableDefaultReflection"] = true
["System.Text.Json.Serialization.UseReflectionDefault"] = false
}
};

@@ -559,6 +559,30 @@ public static void Options_DisableDefaultReflectionSwitch_NewOptionsDoesNotSuppo
}, options).Dispose();
}

[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void Options_DisablingUseReflectionDefaultSwitch_CanUseSourceGen()
{
var options = new RemoteInvokeOptions
{
RuntimeConfigurationOptions =
{
["System.Text.Json.Serialization.UseReflectionDefault"] = false
}
};

RemoteExecutor.Invoke(static () =>
{
var options = new JsonSerializerOptions();
options.TypeInfoResolverChain.Add(JsonContext.Default);

string json = JsonSerializer.Serialize(new WeatherForecastWithPOCOs(), options);
WeatherForecastWithPOCOs result = JsonSerializer.Deserialize<WeatherForecastWithPOCOs>(json, options);
Assert.NotNull(result);

}, options).Dispose();
}

[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[InlineData(false)]
@@ -630,13 +654,13 @@ public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_FallsBa

[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_IsOverriddenByDisableDefaultReflection()
public static void Options_JsonSerializerContext_Net6CompatibilitySwitch_IsOverriddenByDisablingUseReflectionDefault()
{
var options = new RemoteInvokeOptions
{
RuntimeConfigurationOptions =
{
["System.Text.Json.Serialization.DisableDefaultReflection"] = true,
["System.Text.Json.Serialization.UseReflectionDefault"] = false,
["System.Text.Json.Serialization.EnableSourceGenReflectionFallback"] = true
}
};