Skip to content

Commit

Permalink
Use Json TypeInfoResolverChain (dotnet#47450)
Browse files Browse the repository at this point in the history
* Use Json TypeInfoResolverChain

- Removes all usages of JsonSerializerOptions.AddContext, which will be obsoleted by dotnet/runtime#83280
- Update ProblemDetailsJsonContext to be added in a Configure<JsonOptions>() call back and to always be added to the beginning of the resolver chain at that time
    - This gives us the simplest, most understandable pattern for all libraries to follow.
- Small clean up in the API template's usings

* Change ProblemDetailsJsonOptionsSetup to use Configure instead of PostConfigure.

When adding the ProblemDetailsJsonContext, we always prepend it to the beginning of the chain at the time the configure step is executed, no matter the state of the chain.

This allows for a simpler, more understandable policy for all libraries that want to add their JsonContext into the JsonSerializerOptions resolver chain. The order of the chain is now determined by the order that the configure steps were registered in DI (for example when AddProblemDetails() was called).
  • Loading branch information
eerhardt authored Mar 29, 2023
1 parent 61787bc commit 7c0091a
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 49 deletions.
10 changes: 4 additions & 6 deletions src/Http/Http.Extensions/src/JsonOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
using System.Text.Json.Serialization.Metadata;
using Microsoft.AspNetCore.Internal;

#nullable enable

namespace Microsoft.AspNetCore.Http.Json;

/// <summary>
Expand All @@ -18,23 +16,23 @@ public class JsonOptions
{
internal static readonly JsonSerializerOptions DefaultSerializerOptions = new JsonSerializerOptions(JsonSerializerDefaults.Web)
{
// Web defaults don't use the relex JSON escaping encoder.
// Web defaults don't use the relaxed JSON escaping encoder.
//
// Because these options are for producing content that is written directly to the request
// (and not embedded in an HTML page for example), we can use UnsafeRelaxedJsonEscaping.
Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,

// The JsonSerializerOptions.GetTypeInfo method is called directly and needs a defined resolver
// setting the default resolver (reflection-based) but the user can overwrite it directly or calling
// .AddContext<TContext>()
// setting the default resolver (reflection-based) but the user can overwrite it directly or by modifying
// the TypeInfoResolverChain
TypeInfoResolver = TrimmingAppContextSwitches.EnsureJsonTrimmability ? null : CreateDefaultTypeResolver()
};

// Use a copy so the defaults are not modified.
/// <summary>
/// Gets the <see cref="JsonSerializerOptions"/>.
/// </summary>
public JsonSerializerOptions SerializerOptions { get; internal set; } = new JsonSerializerOptions(DefaultSerializerOptions);
public JsonSerializerOptions SerializerOptions { get; } = new JsonSerializerOptions(DefaultSerializerOptions);

#pragma warning disable IL2026 // Suppressed in Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml
#pragma warning disable IL3050 // Calling members annotated with 'RequiresDynamicCodeAttribute' may break functionality when AOT compiling.
Expand Down
35 changes: 14 additions & 21 deletions src/Http/Http.Extensions/src/ProblemDetailsJsonOptionsSetup.cs
Original file line number Diff line number Diff line change
@@ -1,33 +1,26 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Text.Json.Serialization.Metadata;
using Microsoft.AspNetCore.Http.Json;
using Microsoft.Extensions.Options;

namespace Microsoft.AspNetCore.Http;

internal sealed class ProblemDetailsJsonOptionsSetup : IPostConfigureOptions<JsonOptions>
/// <summary>
/// Adds the ProblemDetailsJsonContext to the current JsonSerializerOptions.
///
/// This allows for consistent serialization behavior for ProblemDetails regardless if
/// the default reflection-based serializer is used or not. And makes it trim/NativeAOT compatible.
/// </summary>
internal sealed class ProblemDetailsJsonOptionsSetup : IConfigureOptions<JsonOptions>
{
public void PostConfigure(string? name, JsonOptions options)
public void Configure(JsonOptions options)
{
switch (options.SerializerOptions.TypeInfoResolver)
{
case DefaultJsonTypeInfoResolver:
// In this case, the current configuration is using a reflection-based resolver
// and we are prepending our internal problem details context to be evaluated
// first.
options.SerializerOptions.TypeInfoResolver = JsonTypeInfoResolver.Combine(ProblemDetailsJsonContext.Default, options.SerializerOptions.TypeInfoResolver);
break;
case not null:
// Combine the current resolver with our internal problem details context (adding last)
options.SerializerOptions.AddContext<ProblemDetailsJsonContext>();
break;
default:
// Not adding our source gen context when TypeInfoResolver == null
// since adding it will skip the reflection-based resolver and potentially
// cause unexpected serialization problems
break;
}
// Always insert the ProblemDetailsJsonContext to the beginning of the chain at the time
// this Configure is invoked. This JsonTypeInfoResolver will be before the default reflection-based resolver,
// and before any other resolvers currently added.
// If apps need to customize ProblemDetails serialization, they can prepend a custom ProblemDetails resolver
// to the chain in an IConfigureOptions<JsonOptions> registered after the call to AddProblemDetails().
options.SerializerOptions.TypeInfoResolverChain.Insert(0, new ProblemDetailsJsonContext());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ public static IServiceCollection AddProblemDetails(
// Adding default services;
services.TryAddSingleton<IProblemDetailsService, ProblemDetailsService>();
services.TryAddEnumerable(ServiceDescriptor.Singleton<IProblemDetailsWriter, DefaultProblemDetailsWriter>());
services.TryAddEnumerable(ServiceDescriptor.Singleton<IPostConfigureOptions<JsonOptions>, ProblemDetailsJsonOptionsSetup>());
// Use IConfigureOptions (instead of post-configure) so the registration gets added/invoked relative to when AddProblemDetails() is called.
services.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<JsonOptions>, ProblemDetailsJsonOptionsSetup>());

if (configure != null)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Text.Json;
using System.Text.Json.Serialization;
using System.Text.Json.Serialization.Metadata;
using Microsoft.AspNetCore.Http.Json;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
Expand All @@ -27,7 +27,7 @@ public void AddProblemDetails_AddsNeededServices()
// Assert
Assert.Single(collection, (sd) => sd.ServiceType == typeof(IProblemDetailsService) && sd.ImplementationType == typeof(ProblemDetailsService));
Assert.Single(collection, (sd) => sd.ServiceType == typeof(IProblemDetailsWriter) && sd.ImplementationType == typeof(DefaultProblemDetailsWriter));
Assert.Single(collection, (sd) => sd.ServiceType == typeof(IPostConfigureOptions<JsonOptions>) && sd.ImplementationType == typeof(ProblemDetailsJsonOptionsSetup));
Assert.Single(collection, (sd) => sd.ServiceType == typeof(IConfigureOptions<JsonOptions>) && sd.ImplementationType == typeof(ProblemDetailsJsonOptionsSetup));
}

[Fact]
Expand All @@ -43,7 +43,7 @@ public void AddProblemDetails_DoesNotDuplicate_WhenMultipleCalls()
// Assert
Assert.Single(collection, (sd) => sd.ServiceType == typeof(IProblemDetailsService) && sd.ImplementationType == typeof(ProblemDetailsService));
Assert.Single(collection, (sd) => sd.ServiceType == typeof(IProblemDetailsWriter) && sd.ImplementationType == typeof(DefaultProblemDetailsWriter));
Assert.Single(collection, (sd) => sd.ServiceType == typeof(IPostConfigureOptions<JsonOptions>) && sd.ImplementationType == typeof(ProblemDetailsJsonOptionsSetup));
Assert.Single(collection, (sd) => sd.ServiceType == typeof(IConfigureOptions<JsonOptions>) && sd.ImplementationType == typeof(ProblemDetailsJsonOptionsSetup));
}

[Fact]
Expand Down Expand Up @@ -109,7 +109,8 @@ public void AddProblemDetails_Throws_ForReadOnlyJsonOptions()
// Arrange
var collection = new ServiceCollection();
collection.AddOptions<JsonOptions>();
collection.ConfigureAll<JsonOptions>(options => {
collection.ConfigureAll<JsonOptions>(options =>
{
options.SerializerOptions.TypeInfoResolver = new TestExtensionsJsonContext();
options.SerializerOptions.MakeReadOnly();
});
Expand All @@ -124,13 +125,35 @@ public void AddProblemDetails_Throws_ForReadOnlyJsonOptions()
Assert.Throws<InvalidOperationException>(() => jsonOptions.Value);
}

[Fact]
public void AddProblemDetails_CombinesProblemDetailsContext_WhenAddContext()
public enum CustomContextBehavior
{
Prepend,
Append,
Replace,
}

[Theory]
[InlineData(CustomContextBehavior.Prepend)]
[InlineData(CustomContextBehavior.Append)]
[InlineData(CustomContextBehavior.Replace)]
public void AddProblemDetails_CombinesProblemDetailsContext_WhenAddingCustomContext(CustomContextBehavior behavior)
{
// Arrange
var collection = new ServiceCollection();
collection.AddOptions<JsonOptions>();
collection.ConfigureAll<JsonOptions>(options => options.SerializerOptions.AddContext<TestExtensionsJsonContext>());

if (behavior == CustomContextBehavior.Prepend)
{
collection.ConfigureAll<JsonOptions>(options => options.SerializerOptions.TypeInfoResolverChain.Insert(0, TestExtensionsJsonContext.Default));
}
else if (behavior == CustomContextBehavior.Append)
{
collection.ConfigureAll<JsonOptions>(options => options.SerializerOptions.TypeInfoResolverChain.Add(TestExtensionsJsonContext.Default));
}
else
{
collection.ConfigureAll<JsonOptions>(options => options.SerializerOptions.TypeInfoResolver = TestExtensionsJsonContext.Default);
}

// Act
collection.AddProblemDetails();
Expand All @@ -146,7 +169,7 @@ public void AddProblemDetails_CombinesProblemDetailsContext_WhenAddContext()
}

[Fact]
public void AddProblemDetails_DoesNotCombineProblemDetailsContext_WhenNullTypeInfoResolver()
public void AddProblemDetails_CombinesProblemDetailsContext_EvenWhenNullTypeInfoResolver()
{
// Arrange
var collection = new ServiceCollection();
Expand All @@ -161,7 +184,8 @@ public void AddProblemDetails_DoesNotCombineProblemDetailsContext_WhenNullTypeIn
var jsonOptions = services.GetService<IOptions<JsonOptions>>();

Assert.NotNull(jsonOptions.Value);
Assert.Null(jsonOptions.Value.SerializerOptions.TypeInfoResolver);
Assert.NotNull(jsonOptions.Value.SerializerOptions.TypeInfoResolver);
Assert.NotNull(jsonOptions.Value.SerializerOptions.TypeInfoResolver.GetTypeInfo(typeof(ProblemDetails), jsonOptions.Value.SerializerOptions));
}

[Fact]
Expand All @@ -186,9 +210,55 @@ public void AddProblemDetails_CombineProblemDetailsContext_WhenDefaultTypeInfoRe
Assert.NotNull(jsonOptions.Value.SerializerOptions.TypeInfoResolver.GetTypeInfo(typeof(TypeA), jsonOptions.Value.SerializerOptions));
}

[Fact]
public void AddProblemDetails_CanHaveCustomJsonTypeInfo()
{
// Arrange
var collection = new ServiceCollection();
collection.AddOptions<JsonOptions>();

// Act
collection.AddProblemDetails();

// add any custom ProblemDetails TypeInfoResolvers after calling AddProblemDetails()
var customProblemDetailsResolver = new CustomProblemDetailsTypeInfoResolver();
collection.ConfigureAll<JsonOptions>(options => options.SerializerOptions.TypeInfoResolverChain.Insert(0, customProblemDetailsResolver));

// Assert
var services = collection.BuildServiceProvider();
var jsonOptions = services.GetService<IOptions<JsonOptions>>();

Assert.NotNull(jsonOptions.Value);
Assert.NotNull(jsonOptions.Value.SerializerOptions.TypeInfoResolver);

Assert.Equal(3, jsonOptions.Value.SerializerOptions.TypeInfoResolverChain.Count);
Assert.IsType<CustomProblemDetailsTypeInfoResolver>(jsonOptions.Value.SerializerOptions.TypeInfoResolverChain[0]);
Assert.Equal("Microsoft.AspNetCore.Http.ProblemDetailsJsonContext", jsonOptions.Value.SerializerOptions.TypeInfoResolverChain[1].GetType().FullName);
Assert.IsType<DefaultJsonTypeInfoResolver>(jsonOptions.Value.SerializerOptions.TypeInfoResolverChain[2]);

var pdTypeInfo = jsonOptions.Value.SerializerOptions.GetTypeInfo(typeof(ProblemDetails));
Assert.Same(customProblemDetailsResolver.LastProblemDetailsInfo, pdTypeInfo);
}

[JsonSerializable(typeof(TypeA))]
internal partial class TestExtensionsJsonContext : JsonSerializerContext
{ }

public class TypeA { }

internal class CustomProblemDetailsTypeInfoResolver : IJsonTypeInfoResolver
{
public JsonTypeInfo LastProblemDetailsInfo { get; set; }

public JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options)
{
if (type == typeof(ProblemDetails))
{
LastProblemDetailsInfo = JsonTypeInfo.CreateJsonTypeInfo<ProblemDetails>(options);
return LastProblemDetailsInfo;
}

return null;
}
}
}
10 changes: 5 additions & 5 deletions src/Http/Http.Results/test/HttpResultsHelperTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public async Task WriteResultAsJsonAsync_Works_ForValueTypes(bool useJsonContext

if (useJsonContext)
{
serializerOptions.AddContext<TestJsonContext>();
serializerOptions.TypeInfoResolver = TestJsonContext.Default;
}

// Act
Expand Down Expand Up @@ -61,7 +61,7 @@ public async Task WriteResultAsJsonAsync_Works_ForReferenceTypes(bool useJsonCon

if (useJsonContext)
{
serializerOptions.AddContext<TestJsonContext>();
serializerOptions.TypeInfoResolver = TestJsonContext.Default;
}

// Act
Expand Down Expand Up @@ -94,7 +94,7 @@ public async Task WriteResultAsJsonAsync_Works_ForChildTypes(bool useJsonContext

if (useJsonContext)
{
serializerOptions.AddContext<TestJsonContext>();
serializerOptions.TypeInfoResolver = TestJsonContext.Default;
}

// Act
Expand Down Expand Up @@ -128,7 +128,7 @@ public async Task WriteResultAsJsonAsync_Works_UsingBaseType_ForChildTypes(bool

if (useJsonContext)
{
serializerOptions.AddContext<TestJsonContext>();
serializerOptions.TypeInfoResolver = TestJsonContext.Default;
}

// Act
Expand Down Expand Up @@ -162,7 +162,7 @@ public async Task WriteResultAsJsonAsync_Works_UsingBaseType_ForChildTypes_WithJ

if (useJsonContext)
{
serializerOptions.AddContext<TestJsonContext>();
serializerOptions.TypeInfoResolver = TestJsonContext.Default;
}

// Act
Expand Down
4 changes: 2 additions & 2 deletions src/Mvc/Mvc.Core/src/JsonOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ public class JsonOptions
MaxDepth = MvcOptions.DefaultMaxModelBindingRecursionDepth,

// The JsonSerializerOptions.GetTypeInfo method is called directly and needs a defined resolver
// setting the default resolver (reflection-based) but the user can overwrite it directly or calling
// .AddContext<TContext>()
// setting the default resolver (reflection-based) but the user can overwrite it directly or by modifying
// the TypeInfoResolverChain
TypeInfoResolver = TrimmingAppContextSwitches.EnsureJsonTrimmability ? null : CreateDefaultTypeResolver()
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System.Text.Json.Serialization;
#if NativeAot
using Microsoft.AspNetCore.Http.Json;
using Microsoft.Extensions.Options;
using System.Text.Json.Serialization;

#endif
namespace Company.ApiApplication1;

Expand All @@ -15,7 +14,7 @@ public static void Main(string[] args)
#if (NativeAot)
builder.Services.ConfigureHttpJsonOptions(options =>
{
options.SerializerOptions.AddContext<AppJsonSerializerContext>();
options.SerializerOptions.TypeInfoResolverChain.Insert(0, AppJsonSerializerContext.Default);
});

#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#if (NativeAot)
builder.Services.ConfigureHttpJsonOptions(options =>
{
options.SerializerOptions.AddContext<AppJsonSerializerContext>();
options.SerializerOptions.TypeInfoResolverChain.Insert(0, AppJsonSerializerContext.Default);
});

#endif
Expand Down

0 comments on commit 7c0091a

Please sign in to comment.