JsonSerializerOptions.AddContext should allow context composition

[brunolins16]

Description

In ASP.NET Core, JsonSerializerOptions are resolved from DI and is possible to configure JsonTypeResolver/Context by:

builder.Services.ConfigureHttpJsonOptions(
    options => options.SerializerOptions.AddContext<CustomAppContext>());

or

builder.Services.ConfigureHttpJsonOptions(
    options => options.SerializerOptions.TypeInfoResolver = CustomAppContext.Default);

Also, users and internally can combine additional JsonTypeInfoResolvers using Options [post-]configuration.

Eg.:

        services.PostConfigure<JsonOptions>(options =>
        {
            if (options.SerializerOptions.TypeInfoResolver is not null)
            {
                _serializerOptions.TypeInfoResolver = JsonTypeInfoResolver.Combine(_serializerOptions.TypeInfoResolver, ProblemDetailsJsonContext.Default);
            }

        });

The biggest challenge is how JsonTypeResolver/Context composition works, since a call to AddContext makes the JsonSerializerOptions read only and locks it to any further modification related to the Resolver.

Related to: dotnet/aspnetcore#45906

cc @davidfowl @eiriktsarpalis

Reproduction Steps

using Microsoft.AspNetCore.Http.Json;
using System.Text.Json.Serialization;

var builder = WebApplication.CreateBuilder(args);

// Add services to the container.
builder.Services.ConfigureHttpJsonOptions(
    options => options.SerializerOptions.AddContext<CustomAppContext>());

// Additional JsonSerializerOptions configuration
builder.Services.PostConfigure<JsonOptions>(options =>
{
    options.SerializerOptions.AddContext<AdditionalAppContext>();
});

var app = builder.Build();

app.Run();

public class TypeA { }

[JsonSerializable(typeof(TypeA))]
public partial class CustomAppContext : JsonSerializerContext {}

public class TypeB { }

[JsonSerializable(typeof(TypeB))]
public partial class AdditionalAppContext : JsonSerializerContext { }

Expected behavior

The expected behavior is multiple calls to AddContext or a new API (eg.: AppendContext) combine JsonSerializerContext in the order it is called while keeping the possibility to keep the fast path performance.

Actual behavior

The second call to AddContext throws an InvalidOperationException:

System.InvalidOperationException: JsonSerializerOptions instances cannot be modified once encapsulated by a JsonSerializerContext. Such encapsulation can happen either when calling 'JsonSerializerOptions.AddContext' or when passing the options instance to a JsonSerializerContext constructor.
   at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_SerializerOptionsReadOnly(JsonSerializerContext context)
   at System.Text.Json.JsonSerializerOptions.VerifyMutable()
   at System.Text.Json.JsonSerializerOptions.AddContext[TContext]()
   at Program.<>c.<<Main>$>b__0_1(JsonOptions options) in C:\Users\brolivei\source\repos\WebApplication24\Program.cs:line 11
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.UnnamedOptionsManager`1.get_Value()
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl..ctor(RequestDelegate next, IOptions`1 options, ILoggerFactory loggerFactory, IWebHostEnvironment hostingEnvironment, DiagnosticSource diagnosticSource, IEnumerable`1 filters, IOptions`1 jsonOptions, IProblemDetailsService problemDetailsService)
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.ConstructorInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
   at System.Reflection.RuntimeConstructorInfo.InvokeWithManyArguments(RuntimeConstructorInfo ci, Int32 argCount, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at Microsoft.Extensions.Internal.ActivatorUtilities.ConstructorMatcher.CreateInstance(IServiceProvider provider)
   at Microsoft.Extensions.Internal.ActivatorUtilities.CreateInstance(IServiceProvider provider, Type instanceType, Object[] parameters)
   at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.<>c__DisplayClass5_0.<UseMiddleware>b__0(RequestDelegate next)
   at Microsoft.AspNetCore.Builder.ApplicationBuilder.Build()
   at Microsoft.AspNetCore.Hosting.GenericWebHostService.StartAsync(CancellationToken cancellationToken)
   at Microsoft.Extensions.Hosting.Internal.Host.StartAsync(CancellationToken cancellationToken)
   at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.RunAsync(IHost host, CancellationToken token)
   at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.RunAsync(IHost host, CancellationToken token)
   at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.Run(IHost host)
   at Program.<Main>$(String[] args) in C:\Users\brolivei\source\repos\WebApplication24\Program.cs:line 16

Regression?

No response

Known Workarounds

If you have control of the JsonSerializerOptions (not possible in ASP.NET Core right now) you could create a new JsonSerializerOptions using the copy constructor and combine the additional context:

    var serializerOptions = options.SerializerOptions;
    if (serializerOptions.TypeInfoResolver != null)
    {
        serializerOptions = new(options.SerializerOptions);
        serializerOptions.TypeInfoResolver = JsonTypeInfoResolver.Combine(serializerOptions.TypeInfoResolver!, AdditionalAppContext.Default);
    }

Configuration

.NET 7

Other information

No response

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

---

Description

In ASP.NET Core, JsonSerializerOptions are resolved from DI and is possible to configure JsonTypeResolver/Context by:

builder.Services.ConfigureHttpJsonOptions(
    options => options.SerializerOptions.AddContext<CustomAppContext>());

or

builder.Services.ConfigureHttpJsonOptions(
    options => options.SerializerOptions.TypeInfoResolver = CustomAppContext.Default);

Also, users and internally can combine additional JsonTypeInfoResolvers using Options [post-]configuration.

Eg.:

        services.PostConfigure<JsonOptions>(options =>
        {
            if (options.SerializerOptions.TypeInfoResolver is not null)
            {
                _serializerOptions.TypeInfoResolver = JsonTypeInfoResolver.Combine(_serializerOptions.TypeInfoResolver, ProblemDetailsJsonContext.Default);
            }

        });

The biggest challenge is how JsonTypeResolver/Context composition works, since a call to AddContext makes the JsonSerializerOptions read only and locks it to any further modification related to the Resolver.

Related to: dotnet/aspnetcore#45906

cc @davidfowl @eiriktsarpalis

Reproduction Steps

using Microsoft.AspNetCore.Http.Json;
using System.Text.Json.Serialization;

var builder = WebApplication.CreateBuilder(args);

// Add services to the container.
builder.Services.ConfigureHttpJsonOptions(
    options => options.SerializerOptions.AddContext<CustomAppContext>());

// Additional JsonSerializerOptions configuration
builder.Services.PostConfigure<JsonOptions>(options =>
{
    options.SerializerOptions.AddContext<AdditionalAppContext>();
});

var app = builder.Build();

app.Run();

public class TypeA { }

[JsonSerializable(typeof(TypeA))]
public partial class CustomAppContext : JsonSerializerContext {}

public class TypeB { }

[JsonSerializable(typeof(TypeB))]
public partial class AdditionalAppContext : JsonSerializerContext { }

Expected behavior

The expected behavior is multiple calls to AddContext or a new API (eg.: AppendContext) combine JsonSerializerContext in the order it is called while keeping the possibility to keep the fast path performance.

Actual behavior

The second call to AddContext throws an InvalidOperationException:

System.InvalidOperationException: JsonSerializerOptions instances cannot be modified once encapsulated by a JsonSerializerContext. Such encapsulation can happen either when calling 'JsonSerializerOptions.AddContext' or when passing the options instance to a JsonSerializerContext constructor.
   at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_SerializerOptionsReadOnly(JsonSerializerContext context)
   at System.Text.Json.JsonSerializerOptions.VerifyMutable()
   at System.Text.Json.JsonSerializerOptions.AddContext[TContext]()
   at Program.<>c.<<Main>$>b__0_1(JsonOptions options) in C:\Users\brolivei\source\repos\WebApplication24\Program.cs:line 11
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.UnnamedOptionsManager`1.get_Value()
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl..ctor(RequestDelegate next, IOptions`1 options, ILoggerFactory loggerFactory, IWebHostEnvironment hostingEnvironment, DiagnosticSource diagnosticSource, IEnumerable`1 filters, IOptions`1 jsonOptions, IProblemDetailsService problemDetailsService)
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.ConstructorInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
   at System.Reflection.RuntimeConstructorInfo.InvokeWithManyArguments(RuntimeConstructorInfo ci, Int32 argCount, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at Microsoft.Extensions.Internal.ActivatorUtilities.ConstructorMatcher.CreateInstance(IServiceProvider provider)
   at Microsoft.Extensions.Internal.ActivatorUtilities.CreateInstance(IServiceProvider provider, Type instanceType, Object[] parameters)
   at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.<>c__DisplayClass5_0.<UseMiddleware>b__0(RequestDelegate next)
   at Microsoft.AspNetCore.Builder.ApplicationBuilder.Build()
   at Microsoft.AspNetCore.Hosting.GenericWebHostService.StartAsync(CancellationToken cancellationToken)
   at Microsoft.Extensions.Hosting.Internal.Host.StartAsync(CancellationToken cancellationToken)
   at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.RunAsync(IHost host, CancellationToken token)
   at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.RunAsync(IHost host, CancellationToken token)
   at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.Run(IHost host)
   at Program.<Main>$(String[] args) in C:\Users\brolivei\source\repos\WebApplication24\Program.cs:line 16

Regression?

No response

Known Workarounds

If you have control of the JsonSerializerOptions (not possible in ASP.NET Core right now) you could create a new JsonSerializerOptions using the copy constructor and combine the additional context:

    var serializerOptions = options.SerializerOptions;
    if (serializerOptions.TypeInfoResolver != null)
    {
        serializerOptions = new(options.SerializerOptions);
        serializerOptions.TypeInfoResolver = JsonTypeInfoResolver.Combine(serializerOptions.TypeInfoResolver!, AdditionalAppContext.Default);
    }

Configuration

.NET 7

Other information

No response

Author:	brunolins16
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[krwq]

Note: the workaround also has perf hit from fast path not being used currently

[eiriktsarpalis]

Note: the workaround also has perf hit from fast path not being used currently

cc #71933

[eiriktsarpalis]

Following offline conversation, there is desire to introduce the following (breaking change) to the AddContext method:

public void AddContext<TContext>() where TContext : JsonSerializerContext, new()
{
    VerifyMutable();
    TContext context = new();
-    // create bidirectional link between this options instance and the context, 
-    // making options read-only and overwriting any existing resolver configuration.
-    context.Options = this;
-    TypeInfoResolver = context;
+    // Do not associate the context with the current options and 
+    // chain the current context after any pre-existing resolvers.
+    TypeInfoResolver = TypeInfoResolver is null ? context : JsonTypeInfoResolver.Combine(TypeInfoResolver, context);
}

This would introduce the following breaking change:

options.TypeInfoResolver = FooContext.Default;
options.AddContext<BarContext>();

Assert.IsType<BarContext>(options.TypeInforesolver); // passes in .NET 7, would start to fail in .NET 8

I feel that this type of breaking change is acceptable, since more likely than not the user is doing something wrong here. We should document the breaking change regardless.

[layomia]

there is desire to introduce the following (breaking change) to the AddContext method

👍. Also we should be able to just combine the contexts even if one exists already; still a breaking change. Also noting that we discussed keeping track of the originating context to enable using fast-path for combined contexts.