From 11e14b85a7f425531ec5c7862c0197e7d22e18c7 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Mon, 27 Mar 2023 16:26:16 -0500 Subject: [PATCH 1/2] Use Json TypeInfoResolverChain - Removes all usages of JsonSerializerOptions.AddContext, which will be obsoleted by https://github.com/dotnet/runtime/issues/83280 - Update ProblemDetailsJsonContext to be added just before the DefaultJsonTypeInfoResolver if it is the last resolver in the chain. Otherwise, add it to the end of a non-empty resolver chain. - Small clean up in the API template's usings --- src/Http/Http.Extensions/src/JsonOptions.cs | 8 +-- .../src/ProblemDetailsJsonOptionsSetup.cs | 36 +++++----- ...mDetailsServiceCollectionExtensionsTest.cs | 65 +++++++++++++++++-- .../test/HttpResultsHelperTests.cs | 10 +-- src/Mvc/Mvc.Core/src/JsonOptions.cs | 4 +- .../content/Api-CSharp/Program.Main.cs | 7 +- .../content/Api-CSharp/Program.cs | 2 +- 7 files changed, 94 insertions(+), 38 deletions(-) diff --git a/src/Http/Http.Extensions/src/JsonOptions.cs b/src/Http/Http.Extensions/src/JsonOptions.cs index 5ca45dc9faff..4b02f982b686 100644 --- a/src/Http/Http.Extensions/src/JsonOptions.cs +++ b/src/Http/Http.Extensions/src/JsonOptions.cs @@ -6,8 +6,6 @@ using System.Text.Json.Serialization.Metadata; using Microsoft.AspNetCore.Internal; -#nullable enable - namespace Microsoft.AspNetCore.Http.Json; /// @@ -18,15 +16,15 @@ 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() + // setting the default resolver (reflection-based) but the user can overwrite it directly or by modifying + // the TypeInfoResolverChain TypeInfoResolver = TrimmingAppContextSwitches.EnsureJsonTrimmability ? null : CreateDefaultTypeResolver() }; diff --git a/src/Http/Http.Extensions/src/ProblemDetailsJsonOptionsSetup.cs b/src/Http/Http.Extensions/src/ProblemDetailsJsonOptionsSetup.cs index a2ef31b611d3..0f1317eddb5e 100644 --- a/src/Http/Http.Extensions/src/ProblemDetailsJsonOptionsSetup.cs +++ b/src/Http/Http.Extensions/src/ProblemDetailsJsonOptionsSetup.cs @@ -11,23 +11,27 @@ internal sealed class ProblemDetailsJsonOptionsSetup : IPostConfigureOptions(); - 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; + // Not adding our source gen context when the TypeInfoResolverChain is empty, + // since adding it will skip the reflection-based resolver and potentially + // cause unexpected serialization problems + return; + } + + var lastResolver = typeInfoResolverChain[typeInfoResolverChain.Count - 1]; + if (lastResolver is DefaultJsonTypeInfoResolver) + { + // In this case, the current configuration has a reflection-based resolver at the end + // and we are inserting our internal ProblemDetails context to be evaluated + // just before the reflection-based resolver. + typeInfoResolverChain.Insert(typeInfoResolverChain.Count - 1, ProblemDetailsJsonContext.Default); + } + else + { + // Combine the current resolver with our internal problem details context (adding last) + typeInfoResolverChain.Add(ProblemDetailsJsonContext.Default); } } } diff --git a/src/Http/Http.Extensions/test/ProblemDetailsServiceCollectionExtensionsTest.cs b/src/Http/Http.Extensions/test/ProblemDetailsServiceCollectionExtensionsTest.cs index d19a27256cb1..979ec5d56726 100644 --- a/src/Http/Http.Extensions/test/ProblemDetailsServiceCollectionExtensionsTest.cs +++ b/src/Http/Http.Extensions/test/ProblemDetailsServiceCollectionExtensionsTest.cs @@ -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; @@ -109,7 +109,8 @@ public void AddProblemDetails_Throws_ForReadOnlyJsonOptions() // Arrange var collection = new ServiceCollection(); collection.AddOptions(); - collection.ConfigureAll(options => { + collection.ConfigureAll(options => + { options.SerializerOptions.TypeInfoResolver = new TestExtensionsJsonContext(); options.SerializerOptions.MakeReadOnly(); }); @@ -124,13 +125,23 @@ public void AddProblemDetails_Throws_ForReadOnlyJsonOptions() Assert.Throws(() => jsonOptions.Value); } - [Fact] - public void AddProblemDetails_CombinesProblemDetailsContext_WhenAddContext() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void AddProblemDetails_CombinesProblemDetailsContext_WhenAddingCustomContext(bool prepend) { // Arrange var collection = new ServiceCollection(); collection.AddOptions(); - collection.ConfigureAll(options => options.SerializerOptions.AddContext()); + + if (prepend) + { + collection.ConfigureAll(options => options.SerializerOptions.TypeInfoResolverChain.Insert(0, TestExtensionsJsonContext.Default)); + } + else + { + collection.ConfigureAll(options => options.SerializerOptions.TypeInfoResolverChain.Add(TestExtensionsJsonContext.Default)); + } // Act collection.AddProblemDetails(); @@ -186,9 +197,53 @@ 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(); + var customProblemDetailsResolver = new CustomProblemDetailsTypeInfoResolver(); + collection.ConfigureAll(options => options.SerializerOptions.TypeInfoResolverChain.Insert(0, customProblemDetailsResolver)); + + // Act + collection.AddProblemDetails(); + + // Assert + var services = collection.BuildServiceProvider(); + var jsonOptions = services.GetService>(); + + Assert.NotNull(jsonOptions.Value); + Assert.NotNull(jsonOptions.Value.SerializerOptions.TypeInfoResolver); + + Assert.Equal(3, jsonOptions.Value.SerializerOptions.TypeInfoResolverChain.Count); + Assert.IsType(jsonOptions.Value.SerializerOptions.TypeInfoResolverChain[0]); + Assert.Equal("Microsoft.AspNetCore.Http.ProblemDetailsJsonContext", jsonOptions.Value.SerializerOptions.TypeInfoResolverChain[1].GetType().FullName); + Assert.IsType(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(options); + return LastProblemDetailsInfo; + } + + return null; + } + } } diff --git a/src/Http/Http.Results/test/HttpResultsHelperTests.cs b/src/Http/Http.Results/test/HttpResultsHelperTests.cs index 0ebbdbe017a9..3c2531f57b51 100644 --- a/src/Http/Http.Results/test/HttpResultsHelperTests.cs +++ b/src/Http/Http.Results/test/HttpResultsHelperTests.cs @@ -30,7 +30,7 @@ public async Task WriteResultAsJsonAsync_Works_ForValueTypes(bool useJsonContext if (useJsonContext) { - serializerOptions.AddContext(); + serializerOptions.TypeInfoResolver = TestJsonContext.Default; } // Act @@ -61,7 +61,7 @@ public async Task WriteResultAsJsonAsync_Works_ForReferenceTypes(bool useJsonCon if (useJsonContext) { - serializerOptions.AddContext(); + serializerOptions.TypeInfoResolver = TestJsonContext.Default; } // Act @@ -94,7 +94,7 @@ public async Task WriteResultAsJsonAsync_Works_ForChildTypes(bool useJsonContext if (useJsonContext) { - serializerOptions.AddContext(); + serializerOptions.TypeInfoResolver = TestJsonContext.Default; } // Act @@ -128,7 +128,7 @@ public async Task WriteResultAsJsonAsync_Works_UsingBaseType_ForChildTypes(bool if (useJsonContext) { - serializerOptions.AddContext(); + serializerOptions.TypeInfoResolver = TestJsonContext.Default; } // Act @@ -162,7 +162,7 @@ public async Task WriteResultAsJsonAsync_Works_UsingBaseType_ForChildTypes_WithJ if (useJsonContext) { - serializerOptions.AddContext(); + serializerOptions.TypeInfoResolver = TestJsonContext.Default; } // Act diff --git a/src/Mvc/Mvc.Core/src/JsonOptions.cs b/src/Mvc/Mvc.Core/src/JsonOptions.cs index 44c2522924f2..57ea12c97814 100644 --- a/src/Mvc/Mvc.Core/src/JsonOptions.cs +++ b/src/Mvc/Mvc.Core/src/JsonOptions.cs @@ -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() + // setting the default resolver (reflection-based) but the user can overwrite it directly or by modifying + // the TypeInfoResolverChain TypeInfoResolver = TrimmingAppContextSwitches.EnsureJsonTrimmability ? null : CreateDefaultTypeResolver() }; diff --git a/src/ProjectTemplates/Web.ProjectTemplates/content/Api-CSharp/Program.Main.cs b/src/ProjectTemplates/Web.ProjectTemplates/content/Api-CSharp/Program.Main.cs index dc6deae4de45..69f7df3c6280 100644 --- a/src/ProjectTemplates/Web.ProjectTemplates/content/Api-CSharp/Program.Main.cs +++ b/src/ProjectTemplates/Web.ProjectTemplates/content/Api-CSharp/Program.Main.cs @@ -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; @@ -15,7 +14,7 @@ public static void Main(string[] args) #if (NativeAot) builder.Services.ConfigureHttpJsonOptions(options => { - options.SerializerOptions.AddContext(); + options.SerializerOptions.TypeInfoResolverChain.Insert(0, AppJsonSerializerContext.Default); }); #endif diff --git a/src/ProjectTemplates/Web.ProjectTemplates/content/Api-CSharp/Program.cs b/src/ProjectTemplates/Web.ProjectTemplates/content/Api-CSharp/Program.cs index 5f12bdd3c5ba..50d640ff22ca 100644 --- a/src/ProjectTemplates/Web.ProjectTemplates/content/Api-CSharp/Program.cs +++ b/src/ProjectTemplates/Web.ProjectTemplates/content/Api-CSharp/Program.cs @@ -9,7 +9,7 @@ #if (NativeAot) builder.Services.ConfigureHttpJsonOptions(options => { - options.SerializerOptions.AddContext(); + options.SerializerOptions.TypeInfoResolverChain.Insert(0, AppJsonSerializerContext.Default); }); #endif From 52c3abec24412c04e6fcff31e190d37c53e8e022 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Tue, 28 Mar 2023 16:01:39 -0500 Subject: [PATCH 2/2] 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). --- src/Http/Http.Extensions/src/JsonOptions.cs | 2 +- .../src/ProblemDetailsJsonOptionsSetup.cs | 39 +++++++------------ ...oblemDetailsServiceCollectionExtensions.cs | 3 +- ...mDetailsServiceCollectionExtensionsTest.cs | 39 +++++++++++++------ 4 files changed, 44 insertions(+), 39 deletions(-) diff --git a/src/Http/Http.Extensions/src/JsonOptions.cs b/src/Http/Http.Extensions/src/JsonOptions.cs index 4b02f982b686..540e54ef51e2 100644 --- a/src/Http/Http.Extensions/src/JsonOptions.cs +++ b/src/Http/Http.Extensions/src/JsonOptions.cs @@ -32,7 +32,7 @@ public class JsonOptions /// /// Gets the . /// - 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. diff --git a/src/Http/Http.Extensions/src/ProblemDetailsJsonOptionsSetup.cs b/src/Http/Http.Extensions/src/ProblemDetailsJsonOptionsSetup.cs index 0f1317eddb5e..21ba496347f9 100644 --- a/src/Http/Http.Extensions/src/ProblemDetailsJsonOptionsSetup.cs +++ b/src/Http/Http.Extensions/src/ProblemDetailsJsonOptionsSetup.cs @@ -1,37 +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 +/// +/// 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. +/// +internal sealed class ProblemDetailsJsonOptionsSetup : IConfigureOptions { - public void PostConfigure(string? name, JsonOptions options) + public void Configure(JsonOptions options) { - var typeInfoResolverChain = options.SerializerOptions.TypeInfoResolverChain; - if (typeInfoResolverChain.Count == 0) - { - // Not adding our source gen context when the TypeInfoResolverChain is empty, - // since adding it will skip the reflection-based resolver and potentially - // cause unexpected serialization problems - return; - } - - var lastResolver = typeInfoResolverChain[typeInfoResolverChain.Count - 1]; - if (lastResolver is DefaultJsonTypeInfoResolver) - { - // In this case, the current configuration has a reflection-based resolver at the end - // and we are inserting our internal ProblemDetails context to be evaluated - // just before the reflection-based resolver. - typeInfoResolverChain.Insert(typeInfoResolverChain.Count - 1, ProblemDetailsJsonContext.Default); - } - else - { - // Combine the current resolver with our internal problem details context (adding last) - typeInfoResolverChain.Add(ProblemDetailsJsonContext.Default); - } + // 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 registered after the call to AddProblemDetails(). + options.SerializerOptions.TypeInfoResolverChain.Insert(0, new ProblemDetailsJsonContext()); } } diff --git a/src/Http/Http.Extensions/src/ProblemDetailsServiceCollectionExtensions.cs b/src/Http/Http.Extensions/src/ProblemDetailsServiceCollectionExtensions.cs index f76540a67838..7b7ba4bb5530 100644 --- a/src/Http/Http.Extensions/src/ProblemDetailsServiceCollectionExtensions.cs +++ b/src/Http/Http.Extensions/src/ProblemDetailsServiceCollectionExtensions.cs @@ -40,7 +40,8 @@ public static IServiceCollection AddProblemDetails( // Adding default services; services.TryAddSingleton(); services.TryAddEnumerable(ServiceDescriptor.Singleton()); - services.TryAddEnumerable(ServiceDescriptor.Singleton, ProblemDetailsJsonOptionsSetup>()); + // Use IConfigureOptions (instead of post-configure) so the registration gets added/invoked relative to when AddProblemDetails() is called. + services.TryAddEnumerable(ServiceDescriptor.Singleton, ProblemDetailsJsonOptionsSetup>()); if (configure != null) { diff --git a/src/Http/Http.Extensions/test/ProblemDetailsServiceCollectionExtensionsTest.cs b/src/Http/Http.Extensions/test/ProblemDetailsServiceCollectionExtensionsTest.cs index 979ec5d56726..d06b980a0592 100644 --- a/src/Http/Http.Extensions/test/ProblemDetailsServiceCollectionExtensionsTest.cs +++ b/src/Http/Http.Extensions/test/ProblemDetailsServiceCollectionExtensionsTest.cs @@ -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) && sd.ImplementationType == typeof(ProblemDetailsJsonOptionsSetup)); + Assert.Single(collection, (sd) => sd.ServiceType == typeof(IConfigureOptions) && sd.ImplementationType == typeof(ProblemDetailsJsonOptionsSetup)); } [Fact] @@ -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) && sd.ImplementationType == typeof(ProblemDetailsJsonOptionsSetup)); + Assert.Single(collection, (sd) => sd.ServiceType == typeof(IConfigureOptions) && sd.ImplementationType == typeof(ProblemDetailsJsonOptionsSetup)); } [Fact] @@ -125,23 +125,35 @@ public void AddProblemDetails_Throws_ForReadOnlyJsonOptions() Assert.Throws(() => jsonOptions.Value); } + public enum CustomContextBehavior + { + Prepend, + Append, + Replace, + } + [Theory] - [InlineData(true)] - [InlineData(false)] - public void AddProblemDetails_CombinesProblemDetailsContext_WhenAddingCustomContext(bool prepend) + [InlineData(CustomContextBehavior.Prepend)] + [InlineData(CustomContextBehavior.Append)] + [InlineData(CustomContextBehavior.Replace)] + public void AddProblemDetails_CombinesProblemDetailsContext_WhenAddingCustomContext(CustomContextBehavior behavior) { // Arrange var collection = new ServiceCollection(); collection.AddOptions(); - if (prepend) + if (behavior == CustomContextBehavior.Prepend) { collection.ConfigureAll(options => options.SerializerOptions.TypeInfoResolverChain.Insert(0, TestExtensionsJsonContext.Default)); } - else + else if (behavior == CustomContextBehavior.Append) { collection.ConfigureAll(options => options.SerializerOptions.TypeInfoResolverChain.Add(TestExtensionsJsonContext.Default)); } + else + { + collection.ConfigureAll(options => options.SerializerOptions.TypeInfoResolver = TestExtensionsJsonContext.Default); + } // Act collection.AddProblemDetails(); @@ -157,7 +169,7 @@ public void AddProblemDetails_CombinesProblemDetailsContext_WhenAddingCustomCont } [Fact] - public void AddProblemDetails_DoesNotCombineProblemDetailsContext_WhenNullTypeInfoResolver() + public void AddProblemDetails_CombinesProblemDetailsContext_EvenWhenNullTypeInfoResolver() { // Arrange var collection = new ServiceCollection(); @@ -172,7 +184,8 @@ public void AddProblemDetails_DoesNotCombineProblemDetailsContext_WhenNullTypeIn var jsonOptions = services.GetService>(); 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] @@ -203,12 +216,14 @@ public void AddProblemDetails_CanHaveCustomJsonTypeInfo() // Arrange var collection = new ServiceCollection(); collection.AddOptions(); - var customProblemDetailsResolver = new CustomProblemDetailsTypeInfoResolver(); - collection.ConfigureAll(options => options.SerializerOptions.TypeInfoResolverChain.Insert(0, customProblemDetailsResolver)); - + // Act collection.AddProblemDetails(); + // add any custom ProblemDetails TypeInfoResolvers after calling AddProblemDetails() + var customProblemDetailsResolver = new CustomProblemDetailsTypeInfoResolver(); + collection.ConfigureAll(options => options.SerializerOptions.TypeInfoResolverChain.Insert(0, customProblemDetailsResolver)); + // Assert var services = collection.BuildServiceProvider(); var jsonOptions = services.GetService>();