Skip to content

Commit

Permalink
Revert #80698 and obsolete JsonSerializerOptions.AddContext (#84022)
Browse files Browse the repository at this point in the history
* Revert 9a6686b.

* Obsolete JsonSerializerOptions.AddContext
  • Loading branch information
eiriktsarpalis authored Mar 30, 2023
1 parent 57ae91c commit e43ddbf
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 68 deletions.
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

0 comments on commit e43ddbf

Please # to comment.