From 0d3ce648da3b64e124c668f5e259fe9e76a62f94 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 21 Jul 2022 19:53:22 +0100 Subject: [PATCH] Fix #72614. --- .../JsonMetadataServicesConverter.cs | 6 ++- ...ctWithParameterizedConstructorConverter.cs | 2 +- .../JsonSerializer.Write.Helpers.cs | 2 +- .../Metadata/JsonTypeInfo.Cache.cs | 2 +- .../Serialization/Metadata/JsonTypeInfo.cs | 10 ++-- .../Serialization/Metadata/JsonTypeInfoOfT.cs | 2 +- .../Metadata/SourceGenJsonTypeInfoOfT.cs | 16 +++--- .../JsonSerializerContextTests.cs | 54 +++++++++++++++++++ 8 files changed, 77 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonMetadataServicesConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonMetadataServicesConverter.cs index bec6fddd9ef79..d921c7b6e8e98 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonMetadataServicesConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonMetadataServicesConverter.cs @@ -61,7 +61,7 @@ public JsonMetadataServicesConverter(JsonConverter converter) } internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, ref ReadStack state, out T? value) - => Converter.OnTryRead(ref reader, typeToConvert, options, ref state, out value); + => Converter.OnTryRead(ref reader, typeToConvert, options, ref state, out value); internal override bool OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializerOptions options, ref WriteStack state) { @@ -70,15 +70,17 @@ internal override bool OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializer Debug.Assert(options == jsonTypeInfo.Options); if (!state.SupportContinuation && + jsonTypeInfo.HasSerializeHandler && jsonTypeInfo is JsonTypeInfo info && - info.SerializeHandler != null && !state.CurrentContainsMetadata && // Do not use the fast path if state needs to write metadata. info.Options.SerializerContext?.CanUseSerializationLogic == true) { + Debug.Assert(info.SerializeHandler != null); info.SerializeHandler(writer, value); return true; } + jsonTypeInfo.ValidateCanBeUsedForMetadataSerialization(); return Converter.OnTryWrite(writer, value, options, ref state); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs index 2f6945c4574a2..9a2b85469f982 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs @@ -525,7 +525,7 @@ private void BeginRead(ref ReadStack state, ref Utf8JsonReader reader, JsonSeria { JsonTypeInfo jsonTypeInfo = state.Current.JsonTypeInfo; - jsonTypeInfo.ValidateCanBeUsedForDeserialization(); + jsonTypeInfo.ValidateCanBeUsedForMetadataSerialization(); if (jsonTypeInfo.ParameterCount != jsonTypeInfo.ParameterCache!.Count) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs index 7d7a8ee09d5cf..d80d5fbc34ef2 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs @@ -39,7 +39,7 @@ private static void WriteUsingGeneratedSerializer(Utf8JsonWriter writer, { Debug.Assert(writer != null); - if (jsonTypeInfo.HasSerialize && + if (jsonTypeInfo.HasSerializeHandler && jsonTypeInfo is JsonTypeInfo typedInfo && typedInfo.Options.SerializerContext?.CanUseSerializationLogic == true) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs index 7efaef692cd8e..af6b4cf10104d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs @@ -97,7 +97,7 @@ internal JsonPropertyInfo GetProperty( { PropertyRef propertyRef; - ValidateCanBeUsedForDeserialization(); + ValidateCanBeUsedForMetadataSerialization(); ulong key = GetKey(propertyName); // Keep a local copy of the cache in case it changes by another thread. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs index 03ad3653a7f16..99e75e28c6be7 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs @@ -267,16 +267,16 @@ public JsonPolymorphismOptions? PolymorphismOptions private JsonTypeInfo? _elementTypeInfo; // Avoids having to perform an expensive cast to JsonTypeInfo to check if there is a Serialize method. - internal bool HasSerialize { get; private protected set; } + internal bool HasSerializeHandler { get; private protected set; } // Configure would normally have thrown why initializing properties for source gen but type had SerializeHandler - // so it is allowed to be used for serialization but it will throw if used for deserialization - internal bool ThrowOnDeserialize { get; private protected set; } + // so it is allowed to be used for fast-path serialization but it will throw if used for metadata-based serialization + internal bool MetadataSerializationNotSupported { get; private protected set; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal void ValidateCanBeUsedForDeserialization() + internal void ValidateCanBeUsedForMetadataSerialization() { - if (ThrowOnDeserialize) + if (MetadataSerializationNotSupported) { ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs index 95ac5c21a34ae..005c67fe578e0 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs @@ -99,7 +99,7 @@ private protected set { Debug.Assert(!IsConfigured, "We should not mutate configured JsonTypeInfo"); _serialize = value; - HasSerialize = value != null; + HasSerializeHandler = value != null; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs index 0d3ec62fa11d9..72df4963cd2e7 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs @@ -107,8 +107,13 @@ internal override JsonParameterInfoValues[] GetParameterInfoValues() JsonParameterInfoValues[] array; if (CtorParamInitFunc == null || (array = CtorParamInitFunc()) == null) { - ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeCtorParams(Options.TypeInfoResolver, Type); - return null!; + if (SerializeHandler == null) + { + ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeCtorParams(Options.TypeInfoResolver, Type); + } + + array = Array.Empty(); + MetadataSerializationNotSupported = true; } return array; @@ -139,13 +144,12 @@ internal override void LateAddProperties() return; } - if (SerializeHandler != null && context?.CanUseSerializationLogic == true) + if (SerializeHandler == null) { - ThrowOnDeserialize = true; - return; + ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type); } - ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type); + MetadataSerializationNotSupported = true; return; } 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 6b533a8da1234..f839c7a840344 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 @@ -145,6 +145,60 @@ public static void CombiningContexts_ResolveJsonTypeInfo_DifferentCasing() Assert.Equal("lastName", personInfo.Properties[1].Name); } + [Fact] + public static void FastPathSerialization_ResolvingJsonTypeInfo() + { + JsonSerializerOptions options = FastPathSerializationContext.Default.Options; + + JsonTypeInfo jsonMessageInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(JsonMessage)); + Assert.NotNull(jsonMessageInfo.SerializeHandler); + + var value = new JsonMessage { Message = "Hi" }; + string expectedJson = """{"Message":"Hi","Length":2}"""; + + Assert.Equal(expectedJson, JsonSerializer.Serialize(value, jsonMessageInfo)); + Assert.Equal(expectedJson, JsonSerializer.Serialize(value, options)); + + // Throws since deserialization without metadata is not supported + Assert.Throws(() => JsonSerializer.Deserialize(expectedJson, jsonMessageInfo)); + Assert.Throws(() => JsonSerializer.Deserialize(expectedJson, options)); + } + + [Fact] + public static void FastPathSerialization_CombinedContext_ThrowsInvalidOperationException() + { + // TODO change exception assertions once https://github.com/dotnet/runtime/issues/71933 is fixed. + + var options = new JsonSerializerOptions + { + TypeInfoResolver = JsonTypeInfoResolver.Combine(FastPathSerializationContext.Default, new DefaultJsonTypeInfoResolver()) + }; + + JsonTypeInfo jsonMessageInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(JsonMessage)); + Assert.NotNull(jsonMessageInfo.SerializeHandler); + + var value = new JsonMessage { Message = "Hi" }; + Assert.Throws(() => JsonSerializer.Serialize(value, jsonMessageInfo)); + Assert.Throws(() => JsonSerializer.Serialize(value, options)); + + JsonTypeInfo classInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(ClassWithJsonMessage)); + Assert.Null(classInfo.SerializeHandler); + + var largerValue = new ClassWithJsonMessage { Message = value }; + Assert.Throws(() => JsonSerializer.Serialize(largerValue, classInfo)); + Assert.Throws(() => JsonSerializer.Serialize(largerValue, options)); + } + + [JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Serialization)] + [JsonSerializable(typeof(JsonMessage))] + public partial class FastPathSerializationContext : JsonSerializerContext + { } + + public class ClassWithJsonMessage + { + public JsonMessage Message { get; set; } + } + [Theory] [MemberData(nameof(GetCombiningContextsData))] public static void CombiningContexts_Serialization(T value, string expectedJson)