diff --git a/src/libraries/System.Text.Json/ref/System.Text.Json.cs b/src/libraries/System.Text.Json/ref/System.Text.Json.cs index 81dd1d06194..7ab4dec1e92 100644 --- a/src/libraries/System.Text.Json/ref/System.Text.Json.cs +++ b/src/libraries/System.Text.Json/ref/System.Text.Json.cs @@ -1268,7 +1268,7 @@ namespace System.Text.Json.Serialization.Metadata } public static partial class JsonTypeInfoResolver { - public static System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver Combine(params System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver[] resolvers) { throw null; } + public static System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver Combine(params System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver?[] resolvers) { throw null; } } public abstract partial class JsonTypeInfo : System.Text.Json.Serialization.Metadata.JsonTypeInfo { diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx index 57076fff30a..fc3eb557c36 100644 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -659,9 +659,6 @@ The converter for type '{0}' does not support setting 'CreateObject' delegates. - - One of the provided resolvers is null. - JsonPropertyInfo with name '{0}' for type '{1}' is already bound to different JsonTypeInfo. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs index 4d04c55464f..b07c54438ef 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs @@ -35,14 +35,6 @@ namespace System.Text.Json.Serialization return options; } - - internal set - { - Debug.Assert(!value.IsReadOnly); - value.TypeInfoResolver = this; - value.MakeReadOnly(); - _options = value; - } } /// @@ -94,7 +86,9 @@ namespace System.Text.Json.Serialization if (options != null) { options.VerifyMutable(); - Options = options; + options.TypeInfoResolver = this; + options.MakeReadOnly(); + _options = options; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs index cfc36739317..ced99231fe5 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs @@ -161,18 +161,22 @@ namespace System.Text.Json } /// - /// Binds current instance with a new instance of the specified type. + /// Appends a to the metadata resolution of the current instance. /// /// The generic definition of the specified context type. /// /// 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 resolves metadata. /// public void AddContext() where TContext : JsonSerializerContext, new() { VerifyMutable(); TContext context = new(); - context.Options = this; + TypeInfoResolver = JsonTypeInfoResolver.Combine(TypeInfoResolver, context); } /// diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs index f7537104442..5ada5fd286d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; + namespace System.Text.Json.Serialization.Metadata { /// @@ -13,7 +15,7 @@ namespace System.Text.Json.Serialization.Metadata /// /// Sequence of contract resolvers to be queried for metadata. /// A combining results from . - /// or any of its elements is null. + /// is null. /// /// The combined resolver will query each of in the specified order, /// returning the first result that is non-null. If all return null, @@ -23,32 +25,42 @@ namespace System.Text.Json.Serialization.Metadata /// which typically define contract metadata for small subsets of types. /// It can also be used to fall back to wherever necessary. /// - public static IJsonTypeInfoResolver Combine(params IJsonTypeInfoResolver[] resolvers) + public static IJsonTypeInfoResolver Combine(params IJsonTypeInfoResolver?[] resolvers) { - if (resolvers == null) + if (resolvers is null) { throw new ArgumentNullException(nameof(resolvers)); } + var flattenedResolvers = new List(); + foreach (IJsonTypeInfoResolver? resolver in resolvers) { - if (resolver == null) + if (resolver is null) { - throw new ArgumentNullException(nameof(resolvers), SR.CombineOneOfResolversIsNull); + continue; + } + else if (resolver is CombiningJsonTypeInfoResolver nested) + { + flattenedResolvers.AddRange(nested._resolvers); + } + else + { + flattenedResolvers.Add(resolver); } } - return new CombiningJsonTypeInfoResolver(resolvers); + return flattenedResolvers.Count == 1 + ? flattenedResolvers[0] + : new CombiningJsonTypeInfoResolver(flattenedResolvers.ToArray()); } private sealed class CombiningJsonTypeInfoResolver : IJsonTypeInfoResolver { - private readonly IJsonTypeInfoResolver[] _resolvers; + internal readonly IJsonTypeInfoResolver[] _resolvers; public CombiningJsonTypeInfoResolver(IJsonTypeInfoResolver[] resolvers) - { - _resolvers = resolvers.AsSpan().ToArray(); - } + => _resolvers = resolvers; public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options) { diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/JsonTypeInfoResolverTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/JsonTypeInfoResolverTests.cs index 8b533b742fb..2fdf1a8c9e7 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/JsonTypeInfoResolverTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/JsonTypeInfoResolverTests.cs @@ -3,9 +3,11 @@ 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; @@ -14,18 +16,39 @@ namespace System.Text.Json.Serialization.Tests public static partial class JsonTypeInfoResolverTests { [Fact] - public static void GetTypeInfoNullArguments() + public static void CombineNullArgument() { IJsonTypeInfoResolver[] resolvers = null; Assert.Throws(() => JsonTypeInfoResolver.Combine(resolvers)); + } + [Fact] + public static void Combine_ShouldFlattenResolvers() + { DefaultJsonTypeInfoResolver nonNullResolver1 = new(); DefaultJsonTypeInfoResolver nonNullResolver2 = new(); - Assert.Throws(() => JsonTypeInfoResolver.Combine(null)); - Assert.Throws(() => JsonTypeInfoResolver.Combine(null, null)); - Assert.Throws(() => JsonTypeInfoResolver.Combine(nonNullResolver1, null)); - Assert.Throws(() => JsonTypeInfoResolver.Combine(nonNullResolver1, nonNullResolver2, null)); - Assert.Throws(() => JsonTypeInfoResolver.Combine(nonNullResolver1, null, nonNullResolver2)); + DefaultJsonTypeInfoResolver nonNullResolver3 = new(); + + ValidateCombinations(Array.Empty(), JsonTypeInfoResolver.Combine()); + ValidateCombinations(Array.Empty(), JsonTypeInfoResolver.Combine(new IJsonTypeInfoResolver[] { null })); + ValidateCombinations(Array.Empty(), JsonTypeInfoResolver.Combine(null, null)); + ValidateCombinations(new[] { nonNullResolver1 }, JsonTypeInfoResolver.Combine(nonNullResolver1, null)); + ValidateCombinations(new[] { nonNullResolver1, nonNullResolver2 }, JsonTypeInfoResolver.Combine(nonNullResolver1, nonNullResolver2, null)); + ValidateCombinations(new[] { nonNullResolver1, nonNullResolver2 }, JsonTypeInfoResolver.Combine(nonNullResolver1, null, nonNullResolver2)); + ValidateCombinations(new[] { nonNullResolver1, nonNullResolver2, nonNullResolver3 }, JsonTypeInfoResolver.Combine(JsonTypeInfoResolver.Combine(JsonTypeInfoResolver.Combine(nonNullResolver1), nonNullResolver2), nonNullResolver3)); + ValidateCombinations(new[] { nonNullResolver1, nonNullResolver2, nonNullResolver3 }, JsonTypeInfoResolver.Combine(JsonTypeInfoResolver.Combine(nonNullResolver1, null, nonNullResolver2), nonNullResolver3)); + + static void ValidateCombinations(IJsonTypeInfoResolver[] expectedResolvers, IJsonTypeInfoResolver combinedResolver) + { + if (expectedResolvers.Length == 1) + { + Assert.Same(expectedResolvers[0], combinedResolver); + } + else + { + Assert.Equal(expectedResolvers, GetCombinedResolvers(combinedResolver)); + } + } } [Fact] @@ -126,5 +149,24 @@ namespace System.Text.Json.Serialization.Tests Assert.Null(combined.GetTypeInfo(typeof(StringBuilder), options)); Assert.Equal(4, resolverId); } + + private static IJsonTypeInfoResolver[] GetCombinedResolvers(IJsonTypeInfoResolver resolver) + { + (Type combinedResolverType, FieldInfo underlyingResolverField) = s_combinedResolverMembers.Value; + Assert.IsType(combinedResolverType, resolver); + return (IJsonTypeInfoResolver[])underlyingResolverField.GetValue(resolver); + } + + private static Lazy<(Type, FieldInfo)> s_combinedResolverMembers = new Lazy<(Type, FieldInfo)> + ( + static () => + { + Type? combinedResolverType = typeof(JsonTypeInfoResolver).GetNestedType("CombiningJsonTypeInfoResolver", BindingFlags.NonPublic); + Assert.NotNull(combinedResolverType); + FieldInfo underlyingResolverField = combinedResolverType.GetField("_resolvers", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(underlyingResolverField); + return (combinedResolverType, underlyingResolverField); + } + ); } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/MetadataTests.Options.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/MetadataTests.Options.cs index 93c53a0f63f..2a0945814a0 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/MetadataTests.Options.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/MetadataTests.Options.cs @@ -29,10 +29,30 @@ namespace System.Text.Json.Serialization.Tests { JsonSerializerOptions options = new(); options.AddContext(); + Assert.IsType(options.TypeInfoResolver); + } - // Options can be binded only once. - CauseInvalidOperationException(() => options.AddContext()); - CauseInvalidOperationException(() => options.AddContext()); + [Fact] + public void AddContext_SupportsMultipleContexts() + { + JsonSerializerOptions options = new(); + options.AddContext>(); + options.AddContext>(); + + Assert.NotNull(options.GetTypeInfo(typeof(int))); + Assert.NotNull(options.GetTypeInfo(typeof(string))); + Assert.Throws(() => options.GetTypeInfo(typeof(bool))); + } + + [Fact] + public void AddContext_AppendsToExistingResolver() + { + JsonSerializerOptions options = new(); + options.TypeInfoResolver = new DefaultJsonTypeInfoResolver(); + options.AddContext(); // this context always throws + + // should always consult the default resolver, never falling back to the throwing resolver. + options.GetTypeInfo(typeof(int)); } private static void CauseInvalidOperationException(Action action) @@ -48,16 +68,15 @@ namespace System.Text.Json.Serialization.Tests { // Context binds with options when instantiated with parameterless ctor. MyJsonContextThatSetsOptionsInParameterlessCtor context = new(); - FieldInfo optionsField = typeof(JsonSerializerContext).GetField("_options", BindingFlags.NonPublic | BindingFlags.Instance); - Assert.NotNull(optionsField); - Assert.NotNull((JsonSerializerOptions)optionsField.GetValue(context)); + Assert.NotNull(context.Options); + Assert.Same(context, context.Options.TypeInfoResolver); // Those options are overwritten when context is binded via options.AddContext(); JsonSerializerOptions options = new(); + Assert.Null(options.TypeInfoResolver); options.AddContext(); // No error. - FieldInfo resolverField = typeof(JsonSerializerOptions).GetField("_typeInfoResolver", BindingFlags.NonPublic | BindingFlags.Instance); - Assert.NotNull(resolverField); - Assert.Same(options, ((JsonSerializerContext)resolverField.GetValue(options)).Options); + Assert.NotNull(options.TypeInfoResolver); + Assert.NotSame(options, ((JsonSerializerContext)options.TypeInfoResolver).Options); } [Fact] @@ -66,25 +85,26 @@ namespace System.Text.Json.Serialization.Tests // Bind the options. JsonSerializerOptions options = new(); options.AddContext(); + Assert.False(options.IsReadOnly); - // Attempt to bind the instance again. - Assert.Throws(() => new MyJsonContext(options)); + // Pass the options to a context constructor + _ = new MyJsonContext(options); + Assert.True(options.IsReadOnly); } [Fact] - public void OptionsImmutableAfterBinding() + public void OptionsMutableAfterBinding() { // Bind via AddContext JsonSerializerOptions options = new(); options.PropertyNameCaseInsensitive = true; options.AddContext(); - CauseInvalidOperationException(() => options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase); + Assert.False(options.IsReadOnly); // Bind via context ctor options = new JsonSerializerOptions(); MyJsonContext context = new MyJsonContext(options); - Assert.Same(options, context.Options); - CauseInvalidOperationException(() => options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase); + Assert.True(options.IsReadOnly); } [Fact] @@ -130,5 +150,13 @@ namespace System.Text.Json.Serialization.Tests protected override JsonSerializerOptions? GeneratedSerializerOptions => null; public override JsonTypeInfo? GetTypeInfo(Type type) => JsonTypeInfo.CreateJsonTypeInfo(type, Options); } + + private class SingleTypeContext : 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; + } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs index 869464ca968..306df7054ce 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs @@ -172,16 +172,16 @@ namespace System.Text.Json.Serialization.Tests } [Fact] - public static void TypeInfoResolverCannotBeSetAfterAddingContext() + public static void TypeInfoResolverCanBeSetAfterAddingContext() { var options = new JsonSerializerOptions(); Assert.False(options.IsReadOnly); options.AddContext(); - Assert.True(options.IsReadOnly); + Assert.False(options.IsReadOnly); - Assert.IsType(options.TypeInfoResolver); - Assert.Throws(() => options.TypeInfoResolver = new DefaultJsonTypeInfoResolver()); + options.TypeInfoResolver = new DefaultJsonTypeInfoResolver(); + Assert.IsType(options.TypeInfoResolver); } [Fact] @@ -194,19 +194,21 @@ namespace System.Text.Json.Serialization.Tests } [Fact] - public static void WhenAddingContextTypeInfoResolverAsContextOptionsAreSameAsOptions() + public static void WhenAddingContextTypeInfoResolverAsContextOptionsAreNotSameAsOptions() { var options = new JsonSerializerOptions(); options.AddContext(); - Assert.Same(options, (options.TypeInfoResolver as JsonContext).Options); + Assert.NotSame(options, (options.TypeInfoResolver as JsonContext).Options); } [Fact] - public static void WhenAddingContext_SettingResolverToNullThrowsInvalidOperationException() + public static void WhenAddingContext_CanSetResolverToNull() { var options = new JsonSerializerOptions(); options.AddContext(); - Assert.Throws(() => options.TypeInfoResolver = null); + + options.TypeInfoResolver = null; + Assert.Null(options.TypeInfoResolver); } [Fact]