Skip to content

Revert #80698 and obsolete JsonSerializerOptions.AddContext #84022

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

Merged
merged 2 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions docs/project/list-of-diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ The PR that reveals the implementation of the `<IncludeInternalObsoleteAttribute
| __`SYSLIB0046`__ | ControlledExecution.Run method may corrupt the process and should not be used in production code. |
| __`SYSLIB0047`__ | XmlSecureResolver is obsolete. Use XmlResolver.ThrowingResolver instead when attempting to forbid XML external entity resolution. |
| __`SYSLIB0048`__ | RSA.EncryptValue and DecryptValue are not supported and throw NotSupportedException. Use RSA.Encrypt and RSA.Decrypt instead. |
| __`SYSLIB0049`__ | JsonSerializerOptions.AddContext is obsolete. To register a JsonSerializerContext, use either the TypeInfoResolver or TypeInfoResolverChain properties. |

## Analyzer Warnings

Expand Down
3 changes: 3 additions & 0 deletions src/libraries/Common/src/System/Obsoletions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -156,5 +156,8 @@ internal static class Obsoletions

internal const string RsaEncryptDecryptValueMessage = "RSA.EncryptValue and DecryptValue are not supported and throw NotSupportedException. Use RSA.Encrypt and RSA.Decrypt instead.";
internal const string RsaEncryptDecryptDiagId = "SYSLIB0048";

internal const string JsonSerializerOptionsAddContextMessage = "JsonSerializerOptions.AddContext is obsolete. To register a JsonSerializerContext, use either the TypeInfoResolver or TypeInfoResolverChain properties.";
internal const string JsonSerializerOptionsAddContextDiagId = "SYSLIB0049";
}
}
2 changes: 2 additions & 0 deletions src/libraries/System.Text.Json/ref/System.Text.Json.cs
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,8 @@ public JsonSerializerOptions(System.Text.Json.JsonSerializerOptions options) { }
public System.Text.Json.Serialization.JsonUnknownTypeHandling UnknownTypeHandling { get { throw null; } set { } }
public System.Text.Json.Serialization.JsonUnmappedMemberHandling UnmappedMemberHandling { get { throw null; } set { } }
public bool WriteIndented { get { throw null; } set { } }
[System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)]
[System.ObsoleteAttribute("JsonSerializerOptions.AddContext is obsolete. To register a JsonSerializerContext, use either the TypeInfoResolver or TypeInfoResolverChain properties.", DiagnosticId = "SYSLIB0049", UrlFormat = "https://aka.ms/dotnet-warnings/{0}")]
public void AddContext<TContext>() where TContext : System.Text.Json.Serialization.JsonSerializerContext, new() { }
[System.Diagnostics.CodeAnalysis.RequiresDynamicCodeAttribute("Getting a converter for a type may require reflection which depends on runtime code generation.")]
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Getting a converter for a type may require reflection which depends on unreferenced code.")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ public JsonSerializerOptions Options
}
}

internal void AssociateWithOptions(JsonSerializerOptions options)
{
Debug.Assert(!options.IsReadOnly);
options.TypeInfoResolver = this;
options.MakeReadOnly();
_options = options;
}

/// <summary>
/// Indicates whether pre-generated serialization logic for types in the context
/// is compatible with the run time specified <see cref="JsonSerializerOptions"/>.
Expand Down Expand Up @@ -94,9 +102,7 @@ protected JsonSerializerContext(JsonSerializerOptions? options)
if (options != null)
{
options.VerifyMutable();
options.TypeInfoResolver = this;
options.MakeReadOnly();
_options = options;
AssociateWithOptions(options);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,22 +162,20 @@ public JsonSerializerOptions(JsonSerializerDefaults defaults) : this()
}

/// <summary>
/// Appends a <see cref="Serialization.JsonSerializerContext"/> to the metadata resolution of the current <see cref="JsonSerializerOptions"/> instance.
/// Binds current <see cref="JsonSerializerOptions"/> instance with a new instance of the specified <see cref="Serialization.JsonSerializerContext"/> type.
/// </summary>
/// <typeparam name="TContext">The generic definition of the specified context type.</typeparam>
/// <remarks>
/// When serializing and deserializing types using the options
/// instance, metadata for the types will be fetched from the context instance.
///
/// The methods supports adding multiple contexts per options instance.
/// Metadata will be resolved in the order of configuration, similar to
/// how <see cref="JsonTypeInfoResolver.Combine(IJsonTypeInfoResolver?[])"/> resolves metadata.
/// </remarks>
[Obsolete(Obsoletions.JsonSerializerOptionsAddContextMessage, DiagnosticId = Obsoletions.JsonSerializerOptionsAddContextDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
[EditorBrowsable(EditorBrowsableState.Never)]
public void AddContext<TContext>() where TContext : JsonSerializerContext, new()
{
VerifyMutable();
TContext context = new();
TypeInfoResolver = JsonTypeInfoResolver.Combine(TypeInfoResolver, context);
context.AssociateWithOptions(this);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
<EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles>
<IncludeRemoteExecutor>true</IncludeRemoteExecutor>
<!-- SYSLIB0020: JsonSerializerOptions.IgnoreNullValues is obsolete -->
<!-- SYSLIB0049: JsonSerializerOptions.AddContext is obsolete -->
<!-- SYSLIB1037: Suppress init-only property deserialization warning -->
<!-- SYSLIB1038: Suppress JsonInclude on inaccessible members warning -->
<!-- SYSLIB1039: Suppress Polymorphic types not supported warning -->
<NoWarn>$(NoWarn);SYSLIB0020;SYSLIB1037;SYSLIB1038;SYSLIB1039</NoWarn>
<NoWarn>$(NoWarn);SYSLIB0020;SYSLIB0049;SYSLIB1037;SYSLIB1038;SYSLIB1039</NoWarn>
<IgnoreForCI Condition="'$(TargetsMobile)' == 'true' or '$(TargetsLinuxBionic)' == 'true' or '$(TargetArchitecture)' == 'ARMv6'">true</IgnoreForCI>
</PropertyGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Text;
using System.Text.Json.Serialization.Metadata;
using System.Threading;
using System.Threading.Tasks;
using Xunit;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ public void JsonSerializerContextCtor()
{
// Pass no options.
MyJsonContext context = new();
JsonSerializerOptions options = context.Options; // New options instance created and binded at this point.
JsonSerializerOptions options = context.Options; // New options instance created and bound at this point.
Assert.NotNull(options);

// Pass options.
options = new JsonSerializerOptions();
context = new MyJsonContext(options); // Provided options are binded at this point.
context = new MyJsonContext(options); // Provided options are bound at this point.
Assert.Same(options, context.Options);
}

Expand All @@ -29,30 +29,10 @@ public void AddContext()
{
JsonSerializerOptions options = new();
options.AddContext<MyJsonContext>();
Assert.IsType<MyJsonContext>(options.TypeInfoResolver);
}

[Fact]
public void AddContext_SupportsMultipleContexts()
{
JsonSerializerOptions options = new();
options.AddContext<SingleTypeContext<int>>();
options.AddContext<SingleTypeContext<string>>();

Assert.NotNull(options.GetTypeInfo(typeof(int)));
Assert.NotNull(options.GetTypeInfo(typeof(string)));
Assert.Throws<NotSupportedException>(() => options.GetTypeInfo(typeof(bool)));
}

[Fact]
public void AddContext_AppendsToExistingResolver()
{
JsonSerializerOptions options = new();
options.TypeInfoResolver = new DefaultJsonTypeInfoResolver();
options.AddContext<MyJsonContext>(); // this context always throws

// should always consult the default resolver, never falling back to the throwing resolver.
options.GetTypeInfo(typeof(int));
// Options can be bound only once.
CauseInvalidOperationException(() => options.AddContext<MyJsonContext>());
CauseInvalidOperationException(() => options.AddContext<MyJsonContextThatSetsOptionsInParameterlessCtor>());
}

private static void CauseInvalidOperationException(Action action)
Expand All @@ -68,15 +48,16 @@ public void AddContextOverwritesOptionsForFreshContext()
{
// Context binds with options when instantiated with parameterless ctor.
MyJsonContextThatSetsOptionsInParameterlessCtor context = new();
Assert.NotNull(context.Options);
Assert.Same(context, context.Options.TypeInfoResolver);
FieldInfo optionsField = typeof(JsonSerializerContext).GetField("_options", BindingFlags.NonPublic | BindingFlags.Instance);
Assert.NotNull(optionsField);
Assert.NotNull((JsonSerializerOptions)optionsField.GetValue(context));

// Those options are overwritten when context is binded via options.AddContext<TContext>();
// Those options are overwritten when context is bound via options.AddContext<TContext>();
JsonSerializerOptions options = new();
Assert.Null(options.TypeInfoResolver);
options.AddContext<MyJsonContextThatSetsOptionsInParameterlessCtor>(); // No error.
Assert.NotNull(options.TypeInfoResolver);
Assert.NotSame(options, ((JsonSerializerContext)options.TypeInfoResolver).Options);
FieldInfo resolverField = typeof(JsonSerializerOptions).GetField("_typeInfoResolver", BindingFlags.NonPublic | BindingFlags.Instance);
Assert.NotNull(resolverField);
Assert.Same(options, ((JsonSerializerContext)resolverField.GetValue(options)).Options);
}

[Fact]
Expand All @@ -85,26 +66,25 @@ public void AlreadyBindedOptions()
// Bind the options.
JsonSerializerOptions options = new();
options.AddContext<MyJsonContext>();
Assert.False(options.IsReadOnly);

// Pass the options to a context constructor
_ = new MyJsonContext(options);
Assert.True(options.IsReadOnly);
// Attempt to bind the instance again.
Assert.Throws<InvalidOperationException>(() => new MyJsonContext(options));
}

[Fact]
public void OptionsMutableAfterBinding()
public void OptionsImmutableAfterBinding()
{
// Bind via AddContext
JsonSerializerOptions options = new();
options.PropertyNameCaseInsensitive = true;
options.AddContext<MyJsonContext>();
Assert.False(options.IsReadOnly);
CauseInvalidOperationException(() => options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase);

// Bind via context ctor
options = new JsonSerializerOptions();
MyJsonContext context = new MyJsonContext(options);
Assert.True(options.IsReadOnly);
Assert.Same(options, context.Options);
CauseInvalidOperationException(() => options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase);
}

[Fact]
Expand Down Expand Up @@ -150,13 +130,5 @@ public EmptyContext(JsonSerializerOptions options) : base(options) { }
protected override JsonSerializerOptions? GeneratedSerializerOptions => null;
public override JsonTypeInfo? GetTypeInfo(Type type) => JsonTypeInfo.CreateJsonTypeInfo(type, Options);
}

private class SingleTypeContext<T> : JsonSerializerContext, IJsonTypeInfoResolver
{
public SingleTypeContext() : base(null) { }
protected override JsonSerializerOptions? GeneratedSerializerOptions => null;
public override JsonTypeInfo? GetTypeInfo(Type type) => GetTypeInfo(type, Options);
public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options) => type == typeof(T) ? JsonTypeInfo.CreateJsonTypeInfo(type, options) : null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -177,16 +177,16 @@ public static void NewDefaultOptions_MakeReadOnly_NoTypeInfoResolver_ThrowsInval
}

[Fact]
public static void TypeInfoResolverCanBeSetAfterAddingContext()
public static void TypeInfoResolverCannotBeSetAfterAddingContext()
{
var options = new JsonSerializerOptions();
Assert.False(options.IsReadOnly);

options.AddContext<JsonContext>();
Assert.False(options.IsReadOnly);
Assert.True(options.IsReadOnly);

options.TypeInfoResolver = new DefaultJsonTypeInfoResolver();
Assert.IsType<DefaultJsonTypeInfoResolver>(options.TypeInfoResolver);
Assert.IsType<JsonContext>(options.TypeInfoResolver);
Assert.Throws<InvalidOperationException>(() => options.TypeInfoResolver = new DefaultJsonTypeInfoResolver());
}

[Fact]
Expand All @@ -199,21 +199,19 @@ public static void TypeInfoResolverCannotBeSetOnOptionsCreatedFromContext()
}

[Fact]
public static void WhenAddingContextTypeInfoResolverAsContextOptionsAreNotSameAsOptions()
public static void WhenAddingContextTypeInfoResolverAsContextOptionsAreSameAsOptions()
{
var options = new JsonSerializerOptions();
options.AddContext<JsonContext>();
Assert.NotSame(options, (options.TypeInfoResolver as JsonContext).Options);
Assert.Same(options, (options.TypeInfoResolver as JsonContext).Options);
}

[Fact]
public static void WhenAddingContext_CanSetResolverToNull()
public static void WhenAddingContext_SettingResolverToNullThrowsInvalidOperationException()
{
var options = new JsonSerializerOptions();
options.AddContext<JsonContext>();

options.TypeInfoResolver = null;
Assert.Null(options.TypeInfoResolver);
Assert.Throws<InvalidOperationException>(() => options.TypeInfoResolver = null);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<!-- SYSLIB0020: JsonSerializerOptions.IgnoreNullValues is obsolete -->
<NoWarn>$(NoWarn);SYSLIB0020</NoWarn>
<!-- SYSLIB0049: JsonSerializerOptions.AddContext is obsolete -->
<NoWarn>$(NoWarn);SYSLIB0049</NoWarn>
<!-- Disable analyzers warnings about JSON being misformatted in string literals -->
<NoWarn>$(NoWarn);JSON001</NoWarn>

Expand Down