From 1dcbd57795b42fddc0aef0556e3da1b8a1b5aa5f Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 6 Jun 2022 13:01:45 +0100 Subject: [PATCH 1/2] Fix JsonTypeInfoResolver.Combine for JsonSerializerContext --- .../gen/JsonSourceGenerator.Emitter.cs | 36 +++++++++++++++---- .../System.Text.Json/ref/System.Text.Json.cs | 1 + .../Serialization/JsonSerializerContext.cs | 23 +++++++++++- .../JsonSerializerContextTests.cs | 17 +++++++++ 4 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs index f18b452c91135..209c255a5d146 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs @@ -50,6 +50,7 @@ private sealed partial class Emitter private const string TypeTypeRef = "global::System.Type"; private const string UnsafeTypeRef = "global::System.Runtime.CompilerServices.Unsafe"; private const string NullableTypeRef = "global::System.Nullable"; + private const string ConditionalWeakTableTypeRef = "global::System.Runtime.CompilerServices.ConditionalWeakTable"; private const string EqualityComparerTypeRef = "global::System.Collections.Generic.EqualityComparer"; private const string IListTypeRef = "global::System.Collections.Generic.IList"; private const string KeyValuePairTypeRef = "global::System.Collections.Generic.KeyValuePair"; @@ -70,6 +71,7 @@ private sealed partial class Emitter private const string JsonPropertyInfoTypeRef = "global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo"; private const string JsonPropertyInfoValuesTypeRef = "global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues"; private const string JsonTypeInfoTypeRef = "global::System.Text.Json.Serialization.Metadata.JsonTypeInfo"; + private const string JsonTypeInfoResolverTypeRef = "global::System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver"; private static DiagnosticDescriptor TypeNotSupported { get; } = new DiagnosticDescriptor( id: "SYSLIB1030", @@ -131,14 +133,14 @@ public void Emit() isRootContextDef: true); // Add GetJsonTypeInfo override implementation. - AddSource($"{contextName}.GetJsonTypeInfo.g.cs", GetGetTypeInfoImplementation()); + AddSource($"{contextName}.GetJsonTypeInfo.g.cs", GetGetTypeInfoImplementation(), interfaceImplementation: JsonTypeInfoResolverTypeRef); // Add property name initialization. AddSource($"{contextName}.PropertyNames.g.cs", GetPropertyNameInitialization()); } } - private void AddSource(string fileName, string source, bool isRootContextDef = false) + private void AddSource(string fileName, string source, bool isRootContextDef = false, string? interfaceImplementation = null) { string? generatedCodeAttributeSource = isRootContextDef ? s_generatedCodeAttributeSource : null; @@ -175,7 +177,7 @@ namespace {@namespace} // Add the core implementation for the derived context class. string partialContextImplementation = $@" -{generatedCodeAttributeSource}{declarationList[0]} +{generatedCodeAttributeSource}{declarationList[0]}{(interfaceImplementation is null ? "" : ": " + interfaceImplementation)} {{ {IndentSource(source, Math.Max(1, declarationCount - 1))} }}"; @@ -338,7 +340,7 @@ private static string GenerateForTypeWithUnknownConverter(TypeGenerationSpec typ {{ // Allow nullable handling to forward to the underlying type's converter. converter = {JsonMetadataServicesTypeRef}.GetNullableConverter<{typeCompilableName}>(this.{typeFriendlyName})!; - converter = (({ JsonConverterFactoryTypeRef })converter).CreateConverter(typeToConvert, { OptionsInstanceVariableName })!; + converter = (({JsonConverterFactoryTypeRef})converter).CreateConverter(typeToConvert, {OptionsInstanceVariableName})!; }} else {{ @@ -356,7 +358,7 @@ private static string GenerateForTypeWithUnknownConverter(TypeGenerationSpec typ } metadataInitSource.Append($@" - _{typeFriendlyName} = { JsonMetadataServicesTypeRef }.{ GetCreateValueInfoMethodRef(typeCompilableName)} ({ OptionsInstanceVariableName}, converter); "); + _{typeFriendlyName} = {JsonMetadataServicesTypeRef}.{GetCreateValueInfoMethodRef(typeCompilableName)} ({OptionsInstanceVariableName}, converter); "); return GenerateForType(typeMetadata, metadataInitSource.ToString()); } @@ -1176,6 +1178,10 @@ private string GetRootJsonContextImplementation() {{ }} +private {contextTypeName}({JsonSerializerOptionsTypeRef} options, bool bindOptionsToContext) : base(options, bindOptionsToContext) +{{ +}} + {GetFetchLogicForRuntimeSpecifiedCustomConverter()}"); if (_generateGetConverterMethodForProperties) @@ -1291,10 +1297,28 @@ private string GetGetTypeInfoImplementation() } } - sb.Append(@" + sb.AppendLine(@" return null!; }"); + // Explicit IJsonTypeInfoResolver implementation + string contextTypeName = _currentContext.ContextType.Name; + + sb.AppendLine(); + sb.Append(@$"{JsonTypeInfoTypeRef}? {JsonTypeInfoResolverTypeRef}.GetTypeInfo({TypeTypeRef} type, {JsonSerializerOptionsTypeRef} options) +{{ + {contextTypeName} context = this; + + if (options != null && {OptionsInstanceVariableName} != options) + {{ + context = (s_resolverCache ??= new()).GetValue(options, static options => new {contextTypeName}(options, bindOptionsToContext: false)); + }} + + return context.GetTypeInfo(type); +}} + +private {ConditionalWeakTableTypeRef}<{JsonSerializerOptionsTypeRef},{contextTypeName}>? s_resolverCache;"); + return sb.ToString(); } 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 07e3cdf07c243..3c2466038a5d8 100644 --- a/src/libraries/System.Text.Json/ref/System.Text.Json.cs +++ b/src/libraries/System.Text.Json/ref/System.Text.Json.cs @@ -960,6 +960,7 @@ public JsonSerializableAttribute(System.Type type) { } public abstract partial class JsonSerializerContext : System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver { protected JsonSerializerContext(System.Text.Json.JsonSerializerOptions? options) { } + protected JsonSerializerContext(System.Text.Json.JsonSerializerOptions? options, bool bindOptionsToContext) { } protected abstract System.Text.Json.JsonSerializerOptions? GeneratedSerializerOptions { get; } public System.Text.Json.JsonSerializerOptions Options { get { throw null; } } public abstract System.Text.Json.Serialization.Metadata.JsonTypeInfo? GetTypeInfo(System.Type type); 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 a9f6c90714020..cc1da1f048f9b 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 @@ -81,10 +81,31 @@ internal bool CanUseSerializationLogic /// or until is called, where a new options instance is created and bound. /// protected JsonSerializerContext(JsonSerializerOptions? options) + : this(options, bindOptionsToContext: true) + { + } + + /// + /// Creates an instance of and optionally binds it with the indicated . + /// + /// The run time provided options for the context instance. + /// Specify whether the options instance should be bound to the new context. + /// + /// If no instance options are passed, then no options are set until the context is bound using , + /// or until is called, where a new options instance is created and bound. + /// + protected JsonSerializerContext(JsonSerializerOptions? options, bool bindOptionsToContext) { if (options != null) { - options.TypeInfoResolver = this; + if (bindOptionsToContext) + { + options.TypeInfoResolver = this; + } + else + { + _options = options; + } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs index 3e6304f588ae5..ce4a1cf263c79 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs @@ -68,6 +68,23 @@ public static void SupportsPositionalRecords() Assert.Equal("Doe", person.LastName); } + [Fact] + public static void CombiningContexts() + { + IJsonTypeInfoResolver combined = JsonTypeInfoResolver.Combine(NestedContext.Default, PersonJsonContext.Default); + var options = new JsonSerializerOptions { TypeInfoResolver = combined }; + + JsonTypeInfo messageInfo = combined.GetTypeInfo(typeof(JsonMessage), options); + Assert.IsAssignableFrom>(messageInfo); + Assert.Same(options, messageInfo.Options); + JsonSerializer.Serialize(new JsonMessage { Message = "Hi" }, (JsonTypeInfo) messageInfo); + + JsonTypeInfo personInfo = combined.GetTypeInfo(typeof(Person), options); + Assert.IsAssignableFrom>(personInfo); + Assert.Same(options, personInfo.Options); + JsonSerializer.Serialize(new Person("John", "Doe"), (JsonTypeInfo) personInfo); + } + [JsonSerializable(typeof(JsonMessage))] internal partial class NestedContext : JsonSerializerContext { } From 17d134dd5cc93196067451d2dda917d02a9816fa Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 6 Jun 2022 15:26:47 +0100 Subject: [PATCH 2/2] Break up failing test --- .../JsonSerializerContextTests.cs | 32 +++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs index ce4a1cf263c79..da18a517a8493 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs @@ -69,20 +69,46 @@ public static void SupportsPositionalRecords() } [Fact] - public static void CombiningContexts() + public static void CombiningContexts_ResolveJsonTypeInfo() { + // Basic smoke test establishing combination of JsonSerializerContext classes. IJsonTypeInfoResolver combined = JsonTypeInfoResolver.Combine(NestedContext.Default, PersonJsonContext.Default); var options = new JsonSerializerOptions { TypeInfoResolver = combined }; JsonTypeInfo messageInfo = combined.GetTypeInfo(typeof(JsonMessage), options); Assert.IsAssignableFrom>(messageInfo); Assert.Same(options, messageInfo.Options); - JsonSerializer.Serialize(new JsonMessage { Message = "Hi" }, (JsonTypeInfo) messageInfo); JsonTypeInfo personInfo = combined.GetTypeInfo(typeof(Person), options); Assert.IsAssignableFrom>(personInfo); Assert.Same(options, personInfo.Options); - JsonSerializer.Serialize(new Person("John", "Doe"), (JsonTypeInfo) personInfo); + } + + [Theory] + [MemberData(nameof(GetCombiningContextsData))] + public static void CombiningContexts_Serialization(T value, string expectedJson) + { + // Basic smoke test establishing combination of JsonSerializerContext classes. + IJsonTypeInfoResolver combined = JsonTypeInfoResolver.Combine(NestedContext.Default, PersonJsonContext.Default); + var options = new JsonSerializerOptions { TypeInfoResolver = combined }; + + JsonTypeInfo typeInfo = (JsonTypeInfo)combined.GetTypeInfo(typeof(T), options)!; + + string json = JsonSerializer.Serialize(value, typeInfo); + JsonTestHelper.AssertJsonEqual(expectedJson, json); + + json = JsonSerializer.Serialize(value, options); + JsonTestHelper.AssertJsonEqual(expectedJson, json); + + JsonSerializer.Deserialize(json, typeInfo); + JsonSerializer.Deserialize(json, options); + } + + public static IEnumerable GetCombiningContextsData() + { + yield return WrapArgs(new JsonMessage { Message = "Hi" }, """{ "Message" : { "Hi" } }"""); + yield return WrapArgs(new Person("John", "Doe"), """{ "FirstName" : "John", "LastName" : "Doe" }"""); + static object[] WrapArgs(T value, string expectedJson) => new object[] { value, expectedJson }; } [JsonSerializable(typeof(JsonMessage))]