From 0304f1f5157a8280fa093bdfc7cfb8d9f62e016f Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 8 Feb 2023 19:44:45 +0000 Subject: [PATCH] Support use of fast-path serialization in combined JsonSerializerContexts. (#80741) * Support use of fast-path serialization in combined JsonSerializerContexts. * Strengthen assertions in the nested JsonTypeInfo accessors. --- .../gen/JsonSourceGenerator.Emitter.cs | 4 + .../System.Text.Json/ref/System.Text.Json.cs | 2 + .../System.Text.Json/src/Resources/Strings.resx | 3 - .../Text/Json/Serialization/ConfigurationList.cs | 7 +- .../Converters/JsonMetadataServicesConverter.cs | 1 - .../Json/Serialization/JsonSerializerContext.cs | 28 ++- .../Json/Serialization/JsonSerializerOptions.cs | 34 ++- .../Metadata/DefaultJsonTypeInfoResolver.cs | 5 + .../Metadata/JsonMetadataServices.Helpers.cs | 2 +- .../Serialization/Metadata/JsonPropertyInfo.cs | 15 +- .../Json/Serialization/Metadata/JsonTypeInfo.cs | 133 +++++++++++- .../Metadata/JsonTypeInfoOfT.WriteHelpers.cs | 1 - .../Json/Serialization/Metadata/JsonTypeInfoOfT.cs | 2 +- .../Serialization/Metadata/JsonTypeInfoResolver.cs | 10 +- .../System/Text/Json/ThrowHelper.Serialization.cs | 6 - .../JsonSerializerContextTests.cs | 230 +++++++++++++++++++-- ...efaultJsonTypeInfoResolverTests.JsonTypeInfo.cs | 33 +++ .../MetadataTests/JsonTypeInfoResolverTests.cs | 12 +- 18 files changed, 465 insertions(+), 63 deletions(-) diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs index 5455d3a..ffa31a0 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs @@ -26,6 +26,7 @@ namespace System.Text.Json.SourceGeneration private const string DefaultOptionsStaticVarName = "s_defaultOptions"; private const string DefaultContextBackingStaticVarName = "s_defaultContext"; internal const string GetConverterFromFactoryMethodName = "GetConverterFromFactory"; + private const string OriginatingResolverPropertyName = "OriginatingResolver"; private const string InfoVarName = "info"; private const string PropertyInfoVarName = "propertyInfo"; internal const string JsonContextVarName = "jsonContext"; @@ -1160,6 +1161,9 @@ private {typeInfoPropertyTypeRef} {typeMetadata.CreateTypeInfoMethodName}({JsonS {typeInfoPropertyTypeRef}? {JsonTypeInfoReturnValueLocalVariableName} = null; {WrapWithCheckForCustomConverter(metadataInitSource, typeCompilableName)} + { /* NB OriginatingResolver should be the last property set by the source generator. */ ""} + {JsonTypeInfoReturnValueLocalVariableName}.{OriginatingResolverPropertyName} = this; + return {JsonTypeInfoReturnValueLocalVariableName}; }} {additionalSource}"; 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 6916f71..f01bdab 100644 --- a/src/libraries/System.Text.Json/ref/System.Text.Json.cs +++ b/src/libraries/System.Text.Json/ref/System.Text.Json.cs @@ -1245,6 +1245,8 @@ namespace System.Text.Json.Serialization.Metadata public System.Action? OnSerialized { get { throw null; } set { } } public System.Action? OnSerializing { get { throw null; } set { } } public System.Text.Json.JsonSerializerOptions Options { get { throw null; } } + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] + public System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver? OriginatingResolver { get { throw null; } set { } } public System.Text.Json.Serialization.Metadata.JsonPolymorphismOptions? PolymorphismOptions { get { throw null; } set { } } public System.Collections.Generic.IList Properties { get { throw null; } } public System.Type Type { get { throw null; } } diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx index cb48851..b36235c 100644 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -252,9 +252,6 @@ This JsonTypeInfo instance is marked read-only or has already been used in serialization or deserialization. - - This JsonTypeInfo instance is marked read-only or has already been used in serialization or deserialization. - Max depth must be positive. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ConfigurationList.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ConfigurationList.cs index a5c9397..c3f888a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ConfigurationList.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ConfigurationList.cs @@ -73,7 +73,7 @@ namespace System.Text.Json.Serialization _list.CopyTo(array, arrayIndex); } - public IEnumerator GetEnumerator() + public List.Enumerator GetEnumerator() { return _list.GetEnumerator(); } @@ -107,6 +107,11 @@ namespace System.Text.Json.Serialization _list.RemoveAt(index); } + IEnumerator IEnumerable.GetEnumerator() + { + return _list.GetEnumerator(); + } + IEnumerator IEnumerable.GetEnumerator() { return _list.GetEnumerator(); 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 bd0f50e..f7eb2f7 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 @@ -69,7 +69,6 @@ namespace System.Text.Json.Serialization.Converters !state.CurrentContainsMetadata) // Do not use the fast path if state needs to write metadata. { Debug.Assert(jsonTypeInfo is JsonTypeInfo typeInfo && typeInfo.SerializeHandler != null); - Debug.Assert(jsonTypeInfo.CanUseSerializeHandler); ((JsonTypeInfo)jsonTypeInfo).SerializeHandler!(writer, value); return true; } 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 014af6f..eedeb31 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 @@ -41,12 +41,20 @@ namespace System.Text.Json.Serialization /// Indicates whether pre-generated serialization logic for types in the context /// is compatible with the run time specified . /// - internal bool CanUseFastPathSerializationLogic(JsonSerializerOptions options) + internal bool IsCompatibleWithGeneratedOptions(JsonSerializerOptions options) { - Debug.Assert(options.TypeInfoResolver == this); + Debug.Assert(options != null); + + JsonSerializerOptions? generatedSerializerOptions = GeneratedSerializerOptions; + + if (ReferenceEquals(options, generatedSerializerOptions)) + { + // Fast path for the 99% case + return true; + } return - GeneratedSerializerOptions is not null && + generatedSerializerOptions is not null && // Guard against unsupported features options.Converters.Count == 0 && options.Encoder == null && @@ -59,13 +67,13 @@ namespace System.Text.Json.Serialization #pragma warning restore SYSLIB0020 // Ensure options values are consistent with expected defaults. - options.DefaultIgnoreCondition == GeneratedSerializerOptions.DefaultIgnoreCondition && - options.IgnoreReadOnlyFields == GeneratedSerializerOptions.IgnoreReadOnlyFields && - options.IgnoreReadOnlyProperties == GeneratedSerializerOptions.IgnoreReadOnlyProperties && - options.IncludeFields == GeneratedSerializerOptions.IncludeFields && - options.PropertyNamingPolicy == GeneratedSerializerOptions.PropertyNamingPolicy && - options.DictionaryKeyPolicy == GeneratedSerializerOptions.DictionaryKeyPolicy && - options.WriteIndented == GeneratedSerializerOptions.WriteIndented; + options.DefaultIgnoreCondition == generatedSerializerOptions.DefaultIgnoreCondition && + options.IgnoreReadOnlyFields == generatedSerializerOptions.IgnoreReadOnlyFields && + options.IgnoreReadOnlyProperties == generatedSerializerOptions.IgnoreReadOnlyProperties && + options.IncludeFields == generatedSerializerOptions.IncludeFields && + options.PropertyNamingPolicy == generatedSerializerOptions.PropertyNamingPolicy && + options.DictionaryKeyPolicy == generatedSerializerOptions.DictionaryKeyPolicy && + options.WriteIndented == generatedSerializerOptions.WriteIndented; } /// 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 ced9923..9a08620 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 @@ -605,18 +605,48 @@ namespace System.Text.Json } } + /// + /// Returns true if options uses compatible built-in resolvers or a combination of compatible built-in resolvers. + /// internal bool CanUseFastPathSerializationLogic { get { Debug.Assert(IsReadOnly); - return _canUseFastPathSerializationLogic ??= _typeInfoResolver is JsonSerializerContext ctx ? ctx.CanUseFastPathSerializationLogic(this) : false; + Debug.Assert(TypeInfoResolver != null); + return _canUseFastPathSerializationLogic ??= CanUseFastPath(TypeInfoResolver); + + bool CanUseFastPath(IJsonTypeInfoResolver resolver) + { + switch (resolver) + { + case DefaultJsonTypeInfoResolver defaultResolver: + return defaultResolver.GetType() == typeof(DefaultJsonTypeInfoResolver) && + defaultResolver.Modifiers.Count == 0; + case JsonSerializerContext ctx: + return ctx.IsCompatibleWithGeneratedOptions(this); + case JsonTypeInfoResolver.CombiningJsonTypeInfoResolver combiningResolver: + foreach (IJsonTypeInfoResolver component in combiningResolver.Resolvers) + { + Debug.Assert(component is not JsonTypeInfoResolver.CombiningJsonTypeInfoResolver, "recurses at most once."); + if (!CanUseFastPath(component)) + { + return false; + } + } + + return true; + + default: + return false; + } + } } } private bool? _canUseFastPathSerializationLogic; - // The cached value used to determine if ReferenceHandler should use Preserve or IgnoreCycles semanitcs or None of them. + // The cached value used to determine if ReferenceHandler should use Preserve or IgnoreCycles semantics or None of them. internal ReferenceHandlingStrategy ReferenceHandlingStrategy = ReferenceHandlingStrategy.None; // Workaround https://github.com/dotnet/linker/issues/2715 [UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode", diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs index ae19c96..578bf5c 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs @@ -67,6 +67,11 @@ namespace System.Text.Json.Serialization.Metadata JsonTypeInfo.ValidateType(type); JsonTypeInfo typeInfo = CreateJsonTypeInfo(type, options); + typeInfo.OriginatingResolver = this; + + // We've finished configuring the metadata, brand the instance as user-unmodified. + // This should be the last update operation in the resolver to avoid resetting the flag. + typeInfo.IsCustomized = false; if (_modifiers != null) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.Helpers.cs index 564c6aa..e57fdf4 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.Helpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.Helpers.cs @@ -124,7 +124,7 @@ namespace System.Text.Json.Serialization.Metadata Debug.Assert(typeInfo.Kind is JsonTypeInfoKind.Object); Debug.Assert(!typeInfo.IsReadOnly); - JsonSerializerContext? context = typeInfo.Options.TypeInfoResolver as JsonSerializerContext; + JsonSerializerContext? context = (typeInfo.OriginatingResolver ?? typeInfo.Options.TypeInfoResolver) as JsonSerializerContext; if (propInitFunc?.Invoke(context!) is not JsonPropertyInfo[] properties) { if (typeInfo.Type == JsonTypeInfo.ObjectType) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs index 57bd626..90bfd38 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs @@ -18,7 +18,6 @@ namespace System.Text.Json.Serialization.Metadata internal static readonly JsonPropertyInfo s_missingProperty = GetPropertyPlaceholder(); internal JsonTypeInfo? ParentTypeInfo { get; private set; } - private JsonTypeInfo? _jsonTypeInfo; /// /// Converter after applying CustomConverter (i.e. JsonConverterAttribute) @@ -266,10 +265,7 @@ namespace System.Text.Json.Serialization.Metadata private protected void VerifyMutable() { - if (ParentTypeInfo?.IsReadOnly == true) - { - ThrowHelper.ThrowInvalidOperationException_PropertyInfoImmutable(); - } + ParentTypeInfo?.VerifyMutable(); } internal bool IsConfigured { get; private set; } @@ -806,6 +802,15 @@ namespace System.Text.Json.Serialization.Metadata } } + private JsonTypeInfo? _jsonTypeInfo; + + /// + /// Returns true if has been configured. + /// This might be false even if is true + /// in cases of recursive types or is true. + /// + internal bool IsPropertyTypeInfoConfigured => _jsonTypeInfo?.IsConfigured == true; + /// /// Property was marked JsonIgnoreCondition.Always and also hasn't been configured by the user. /// 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 397d5c6..123b5a1 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 @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.ComponentModel; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.IO; @@ -295,8 +296,11 @@ namespace System.Text.Json.Serialization.Metadata internal PolymorphicTypeResolver? PolymorphicTypeResolver { get; private set; } - // Flag indicating that JsonTypeInfo.SerializeHandler is populated and is compatible with the associated Options instance. - internal bool CanUseSerializeHandler { get; private protected set; } + // Indicates that SerializeHandler is populated. + internal bool HasSerializeHandler { get; private protected set; } + + // Indicates that SerializeHandler is populated and is compatible with the associated contract metadata. + internal bool CanUseSerializeHandler { get; private set; } // Configure would normally have thrown why initializing properties for source gen but type had SerializeHandler // so it is allowed to be used for fast-path serialization but it will throw if used for metadata-based serialization @@ -496,14 +500,54 @@ namespace System.Text.Json.Serialization.Metadata internal JsonUnmappedMemberHandling EffectiveUnmappedMemberHandling { get; private set; } + /// + /// Gets or sets the from which this metadata instance originated. + /// + /// + /// The instance has been locked for further modification. + /// + /// + /// Metadata used to determine the + /// configuration for the current metadata instance. + /// + [EditorBrowsable(EditorBrowsableState.Never)] + public IJsonTypeInfoResolver? OriginatingResolver + { + get => _originatingResolver; + set + { + VerifyMutable(); + + if (value is JsonSerializerContext) + { + // The source generator uses this property setter to brand the metadata instance as user-unmodified. + // Even though users could call the same property setter to unset this flag, this is generally speaking fine. + // This flag is only used to determine fast-path invalidation, worst case scenario this would lead to a false negative. + IsCustomized = false; + } + + _originatingResolver = value; + } + } + + private IJsonTypeInfoResolver? _originatingResolver; + internal void VerifyMutable() { if (IsReadOnly) { ThrowHelper.ThrowInvalidOperationException_TypeInfoImmutable(); } + + IsCustomized = true; } + /// + /// Indicates that the current JsonTypeInfo might contain user modifications. + /// Defaults to true, and is only unset by the built-in contract resolvers. + /// + internal bool IsCustomized { get; set; } = true; + internal bool IsConfigured => _configurationState == ConfigurationState.Configured; internal bool IsConfigurationStarted => _configurationState is not ConfigurationState.NotConfigured; private volatile ConfigurationState _configurationState; @@ -559,7 +603,6 @@ namespace System.Text.Json.Serialization.Metadata Debug.Assert(IsReadOnly); PropertyInfoForTypeInfo.Configure(); - CanUseSerializeHandler &= Options.CanUseFastPathSerializationLogic; if (Kind == JsonTypeInfoKind.Object) { @@ -583,12 +626,96 @@ namespace System.Text.Json.Serialization.Metadata _keyTypeInfo.EnsureConfigured(); } + DetermineIsCompatibleWithCurrentOptions(); + CanUseSerializeHandler = HasSerializeHandler && IsCompatibleWithCurrentOptions; + if (PolymorphismOptions != null) { PolymorphicTypeResolver = new PolymorphicTypeResolver(this); } } + /// + /// Determines if the transitive closure of all JsonTypeInfo metadata referenced + /// by the current type (property types, key types, element types, ...) are + /// compatible with the settings as specified in JsonSerializerOptions. + /// + private void DetermineIsCompatibleWithCurrentOptions() + { + // Defines a recursive algorithm validating that the `IsCurrentNodeCompatible` + // predicate is valid for every node in the type graph. This method only checks + // the immediate children, with recursion being driven by the Configure() method. + // Therefore, this method must be called _after_ the child nodes have been configured. + + Debug.Assert(IsReadOnly); + Debug.Assert(!IsConfigured); + + if (!IsCurrentNodeCompatible()) + { + IsCompatibleWithCurrentOptions = false; + return; + } + + if (_properties != null) + { + foreach (JsonPropertyInfo property in _properties) + { + Debug.Assert(property.IsConfigured); + + if (!property.IsPropertyTypeInfoConfigured) + { + // Either an ignored property or property is part of a cycle. + // In both cases we can ignore these instances. + continue; + } + + if (!property.JsonTypeInfo.IsCompatibleWithCurrentOptions) + { + IsCompatibleWithCurrentOptions = false; + return; + } + } + } + + if (_elementTypeInfo?.IsCompatibleWithCurrentOptions == false || + _keyTypeInfo?.IsCompatibleWithCurrentOptions == false) + { + IsCompatibleWithCurrentOptions = false; + return; + } + + Debug.Assert(IsCompatibleWithCurrentOptions); + + // Defines the core predicate that must be checked for every node in the type graph. + bool IsCurrentNodeCompatible() + { + if (Options.CanUseFastPathSerializationLogic) + { + // Simple case/backward compatibility: options uses a combination of compatible built-in converters. + return true; + } + + if (IsCustomized) + { + // Return false if we have detected contract customization by the user. + return false; + } + + return OriginatingResolver switch + { + JsonSerializerContext ctx => ctx.IsCompatibleWithGeneratedOptions(Options), + DefaultJsonTypeInfoResolver => true, // generates default contracts by definition + _ => false + }; + } + } + + /// + /// Holds the result of the above algorithm -- NB must default to true + /// to establish a base case for recursive types and any JsonIgnored property types. + /// + private bool IsCompatibleWithCurrentOptions { get; set; } = true; + #if DEBUG internal string GetPropertyDebugInfo(ReadOnlySpan unescapedPropertyName) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.WriteHelpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.WriteHelpers.cs index d16480d..1d3bdd8 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.WriteHelpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.WriteHelpers.cs @@ -32,7 +32,6 @@ namespace System.Text.Json.Serialization.Metadata // this avoids creating a WriteStack and calling into the converter infrastructure. Debug.Assert(SerializeHandler != null); - Debug.Assert(CanUseSerializeHandler); Debug.Assert(Converter is JsonMetadataServicesConverter); SerializeHandler(writer, rootValue!); 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 6c2a0a5..1b2062b 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 @@ -115,7 +115,7 @@ namespace System.Text.Json.Serialization.Metadata { Debug.Assert(!IsReadOnly, "We should not mutate read-only JsonTypeInfo"); _serialize = value; - CanUseSerializeHandler = value != null; + HasSerializeHandler = value != null; } } 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 5ada5fd..2484592 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 @@ -42,7 +42,7 @@ namespace System.Text.Json.Serialization.Metadata } else if (resolver is CombiningJsonTypeInfoResolver nested) { - flattenedResolvers.AddRange(nested._resolvers); + flattenedResolvers.AddRange(nested.Resolvers); } else { @@ -55,16 +55,16 @@ namespace System.Text.Json.Serialization.Metadata : new CombiningJsonTypeInfoResolver(flattenedResolvers.ToArray()); } - private sealed class CombiningJsonTypeInfoResolver : IJsonTypeInfoResolver + internal sealed class CombiningJsonTypeInfoResolver : IJsonTypeInfoResolver { - internal readonly IJsonTypeInfoResolver[] _resolvers; + internal IJsonTypeInfoResolver[] Resolvers { get; } public CombiningJsonTypeInfoResolver(IJsonTypeInfoResolver[] resolvers) - => _resolvers = resolvers; + => Resolvers = resolvers; public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options) { - foreach (IJsonTypeInfoResolver resolver in _resolvers) + foreach (IJsonTypeInfoResolver resolver in Resolvers) { JsonTypeInfo? typeInfo = resolver.GetTypeInfo(type, options); if (typeInfo != null) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs index 6ba9bb0..4b235f7 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs @@ -185,12 +185,6 @@ namespace System.Text.Json } [DoesNotReturn] - public static void ThrowInvalidOperationException_PropertyInfoImmutable() - { - throw new InvalidOperationException(SR.PropertyInfoImmutable); - } - - [DoesNotReturn] public static void ThrowInvalidOperationException_SerializerPropertyNameConflict(Type type, string propertyName) { throw new InvalidOperationException(SR.Format(SR.SerializerPropertyNameConflict, type, propertyName)); 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 edce3f1..fd30c56 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 @@ -2,9 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.IO; using System.Reflection; using System.Text.Json.Serialization; using System.Text.Json.Serialization.Metadata; +using System.Threading.Tasks; using Microsoft.DotNet.RemoteExecutor; using Xunit; @@ -252,41 +254,233 @@ namespace System.Text.Json.SourceGeneration.Tests Assert.Throws(() => JsonSerializer.Deserialize(expectedJson, options)); } - [Fact] - public static void FastPathSerialization_CombinedContext_ThrowsInvalidOperationException() + [Theory] + [MemberData(nameof(GetFastPathCompatibleResolvers))] + [MemberData(nameof(GetFastPathIncompatibleResolvers))] + public static void FastPathSerialization_AppendedResolver_WorksAsExpected(IJsonTypeInfoResolver appendedResolver) { - // TODO change exception assertions once https://github.com/dotnet/runtime/issues/71933 is fixed. + // Resolvers appended after ours will never introduce metadata to the type graph, + // therefore the fast path should always be used regardless of what they are doing. + var fastPathContext = new ContextWithInstrumentedFastPath(); var options = new JsonSerializerOptions { - TypeInfoResolver = JsonTypeInfoResolver.Combine(FastPathSerializationContext.Default, new DefaultJsonTypeInfoResolver()) + TypeInfoResolver = JsonTypeInfoResolver.Combine(fastPathContext, appendedResolver, new DefaultJsonTypeInfoResolver()) }; - JsonTypeInfo jsonMessageInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(JsonMessage)); + JsonTypeInfo jsonMessageInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(PocoWithInteger)); Assert.NotNull(jsonMessageInfo.SerializeHandler); - var value = new JsonMessage { Message = "Hi" }; - Assert.Throws(() => JsonSerializer.Serialize(value, jsonMessageInfo)); - Assert.Throws(() => JsonSerializer.Serialize(value, options)); + var value = new PocoWithInteger { Value = 42 }; + string expectedJson = """{"Value":42}"""; + + string json = JsonSerializer.Serialize(value, jsonMessageInfo); + Assert.Equal(expectedJson, json); + Assert.Equal(1, fastPathContext.FastPathInvocationCount); - JsonTypeInfo classInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(ClassWithJsonMessage)); + json = JsonSerializer.Serialize(value, options); + Assert.Equal(expectedJson, json); + Assert.Equal(2, fastPathContext.FastPathInvocationCount); + + JsonTypeInfo classInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(ContainingClass)); Assert.Null(classInfo.SerializeHandler); - var largerValue = new ClassWithJsonMessage { Message = value }; - Assert.Throws(() => JsonSerializer.Serialize(largerValue, classInfo)); - Assert.Throws(() => JsonSerializer.Serialize(largerValue, options)); + var largerValue = new ContainingClass { Message = value }; + expectedJson = $$"""{"Message":{{expectedJson}}}"""; + + json = JsonSerializer.Serialize(largerValue, classInfo); + Assert.Equal(expectedJson, json); + Assert.Equal(3, fastPathContext.FastPathInvocationCount); + + json = JsonSerializer.Serialize(largerValue, options); + Assert.Equal(expectedJson, json); + Assert.Equal(4, fastPathContext.FastPathInvocationCount); } - [JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Serialization)] - [JsonSerializable(typeof(JsonMessage))] - public partial class FastPathSerializationContext : JsonSerializerContext + [Theory] + [MemberData(nameof(GetFastPathCompatibleResolvers))] + public static void FastPathSerialization_PrependedResolver_CompatibleResolvers_WorksAsExpected(IJsonTypeInfoResolver prependedResolver) + { + // We're prepending a resolver that generates metadata for the property of our type, + // but because the two sources use compatible configuration the fast path should still be used. + + var fastPathContext = new ContextWithInstrumentedFastPath(); + var options = new JsonSerializerOptions + { + TypeInfoResolver = JsonTypeInfoResolver.Combine(prependedResolver, fastPathContext, new DefaultJsonTypeInfoResolver()) + }; + + JsonTypeInfo jsonMessageInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(PocoWithInteger)); + Assert.NotNull(jsonMessageInfo.SerializeHandler); + + var value = new PocoWithInteger { Value = 42 }; + string expectedJson = """{"Value":42}"""; + + string json = JsonSerializer.Serialize(value, jsonMessageInfo); + Assert.Equal(expectedJson, json); + Assert.Equal(1, fastPathContext.FastPathInvocationCount); + + json = JsonSerializer.Serialize(value, options); + Assert.Equal(expectedJson, json); + Assert.Equal(2, fastPathContext.FastPathInvocationCount); + + JsonTypeInfo classInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(ContainingClass)); + Assert.Null(classInfo.SerializeHandler); + + var largerValue = new ContainingClass { Message = value }; + expectedJson = $$"""{"Message":{{expectedJson}}}"""; + + json = JsonSerializer.Serialize(largerValue, classInfo); + Assert.Equal(expectedJson, json); + Assert.Equal(3, fastPathContext.FastPathInvocationCount); + + json = JsonSerializer.Serialize(largerValue, options); + Assert.Equal(expectedJson, json); + Assert.Equal(4, fastPathContext.FastPathInvocationCount); + } + + [Theory] + [MemberData(nameof(GetFastPathIncompatibleResolvers))] + public static void FastPathSerialization_PrependedResolver_IncompatibleResolvers_FallsBackToMetadata(IJsonTypeInfoResolver prependedResolver) + { + // We're prepending a resolver that generates metadata for the property of our type, + // because the two sources use incompatible configuration the fast path should not be used. + + var fastPathContext = new ContextWithInstrumentedFastPath(); + var options = new JsonSerializerOptions + { + TypeInfoResolver = JsonTypeInfoResolver.Combine(prependedResolver, fastPathContext, new DefaultJsonTypeInfoResolver()) + }; + + JsonTypeInfo jsonMessageInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(PocoWithInteger)); + Assert.NotNull(jsonMessageInfo.SerializeHandler); + + var value = new PocoWithInteger { Value = 42 }; + string expectedJson = """{"Value":42}"""; + + string json = JsonSerializer.Serialize(value, jsonMessageInfo); + Assert.Equal(expectedJson, json); + Assert.Equal(0, fastPathContext.FastPathInvocationCount); + + json = JsonSerializer.Serialize(value, options); + Assert.Equal(expectedJson, json); + Assert.Equal(0, fastPathContext.FastPathInvocationCount); + + JsonTypeInfo classInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(ContainingClass)); + Assert.Null(classInfo.SerializeHandler); + + var largerValue = new ContainingClass { Message = value }; + expectedJson = $$"""{"Message":{{expectedJson}}}"""; + + json = JsonSerializer.Serialize(largerValue, classInfo); + Assert.Equal(expectedJson, json); + Assert.Equal(0, fastPathContext.FastPathInvocationCount); + + json = JsonSerializer.Serialize(largerValue, options); + Assert.Equal(expectedJson, json); + Assert.Equal(0, fastPathContext.FastPathInvocationCount); + } + + public static IEnumerable GetFastPathCompatibleResolvers() + { + yield return new object[] { CompatibleWithInstrumentedFastPathContext.Default }; + yield return new object[] { new CustomWrappingResolver { Resolver = new DefaultJsonTypeInfoResolver() } }; + yield return new object[] { new CustomWrappingResolver { Resolver = CompatibleWithInstrumentedFastPathContext.Default } }; + yield return new object[] { new CustomWrappingResolver { Resolver = new ContextWithInstrumentedFastPath() } }; + } + + public static IEnumerable GetFastPathIncompatibleResolvers() + { + yield return new object[] { NotCompatibleWithInstrumentedFastPathContext.Default }; + yield return new object[] { new CustomWrappingResolver { Resolver = new DefaultJsonTypeInfoResolver { Modifiers = { static jti => jti.PolymorphismOptions = null } } } }; + yield return new object[] { new CustomWrappingResolver { Resolver = NotCompatibleWithInstrumentedFastPathContext.Default } }; + } + + public class PocoWithInteger + { + public int Value { get; set; } + } + + public class ContainingClass + { + public PocoWithInteger Message { get; set; } + } + + public class ContextWithInstrumentedFastPath : JsonSerializerContext, IJsonTypeInfoResolver + { + public int FastPathInvocationCount { get; private set; } + + public ContextWithInstrumentedFastPath() : base(null) + { } + + protected override JsonSerializerOptions? GeneratedSerializerOptions => Options; + public override JsonTypeInfo? GetTypeInfo(Type type) => GetTypeInfo(type, Options); + public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options) + { + JsonTypeInfo? typeInfo = null; + + if (type == typeof(int)) + { + typeInfo = JsonMetadataServices.CreateValueInfo(options, JsonMetadataServices.Int32Converter); + } + + if (type == typeof(PocoWithInteger)) + { + typeInfo = JsonMetadataServices.CreateObjectInfo(options, + new JsonObjectInfoValues + { + PropertyMetadataInitializer = _ => new JsonPropertyInfo[1] + { + JsonMetadataServices.CreatePropertyInfo(options, + new JsonPropertyInfoValues + { + IsProperty = true, + IsPublic = true, + DeclaringType = typeof(PocoWithInteger), + PropertyName = "Value", + Getter = obj => ((PocoWithInteger)obj).Value, + Setter = (obj, value) => ((PocoWithInteger)obj).Value = value, + }) + }, + + SerializeHandler = (writer, value) => + { + writer.WriteStartObject(); + writer.WriteNumber("Value", value.Value); + writer.WriteEndObject(); + FastPathInvocationCount++; + } + }); + } + + if (typeInfo != null) + typeInfo.OriginatingResolver = this; + + return typeInfo; + } + } + + [JsonSerializable(typeof(int))] + public partial class CompatibleWithInstrumentedFastPathContext : JsonSerializerContext { } - public class ClassWithJsonMessage + [JsonSourceGenerationOptions(IncludeFields = true)] + [JsonSerializable(typeof(int))] + public partial class NotCompatibleWithInstrumentedFastPathContext : JsonSerializerContext + { } + + public class CustomWrappingResolver : IJsonTypeInfoResolver { - public JsonMessage Message { get; set; } + public required IJsonTypeInfoResolver Resolver { get; init; } + public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options) + => type == typeof(T) ? Resolver.GetTypeInfo(type, options) : null; } + [JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Serialization)] + [JsonSerializable(typeof(JsonMessage))] + public partial class FastPathSerializationContext : JsonSerializerContext + { } + [Theory] [MemberData(nameof(GetCombiningContextsData))] public static void CombiningContexts_Serialization(T value, string expectedJson) @@ -306,7 +500,7 @@ namespace System.Text.Json.SourceGeneration.Tests JsonSerializer.Deserialize(json, options); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [Fact] public static void CombiningContextWithCustomResolver_ReplacePoco() { TestResolver customResolver = new((type, options) => diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs index f172260..ed36a73 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs @@ -422,6 +422,7 @@ namespace System.Text.Json.Serialization.Tests Assert.Throws(() => typeInfo.Properties.Clear()); Assert.Throws(() => typeInfo.PolymorphismOptions = null); Assert.Throws(() => typeInfo.PolymorphismOptions = new()); + Assert.Throws(() => typeInfo.OriginatingResolver = new DefaultJsonTypeInfoResolver()); if (typeInfo.Properties.Count > 0) { @@ -1431,5 +1432,37 @@ namespace System.Text.Json.Serialization.Tests JsonTypeInfo jsonTypeInfo = JsonTypeInfo.CreateJsonTypeInfo(type, new()); Assert.Throws(() => jsonTypeInfo.UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip); } + + [Theory] + [InlineData(typeof(int))] + [InlineData(typeof(string))] + [InlineData(typeof(int[]))] + [InlineData(typeof(Dictionary))] + public static void DefaultJsonTypeInfo_OriginatingResolver_GetterReturnsResolver(Type type) + { + var resolver = new DefaultJsonTypeInfoResolver(); + var options = new JsonSerializerOptions(); + + JsonTypeInfo typeInfo = resolver.GetTypeInfo(type, options); + Assert.Same(resolver, typeInfo.OriginatingResolver); + } + + [Theory] + [InlineData(typeof(int))] + [InlineData(typeof(string))] + [InlineData(typeof(int[]))] + [InlineData(typeof(Dictionary))] + public static void OriginatingResolver_GetterReturnsTheSetValue(Type type) + { + var resolver = new DefaultJsonTypeInfoResolver(); + var options = new JsonSerializerOptions(); + + JsonTypeInfo typeInfo = resolver.GetTypeInfo(type, options); + typeInfo.OriginatingResolver = null; + Assert.Null(typeInfo.OriginatingResolver); + + typeInfo.OriginatingResolver = JsonSerializerOptions.Default.TypeInfoResolver; + Assert.Same(JsonSerializerOptions.Default.TypeInfoResolver, typeInfo.OriginatingResolver); + } } } 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 2fdf1a8..43d1b6a 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 @@ -152,20 +152,20 @@ namespace System.Text.Json.Serialization.Tests private static IJsonTypeInfoResolver[] GetCombinedResolvers(IJsonTypeInfoResolver resolver) { - (Type combinedResolverType, FieldInfo underlyingResolverField) = s_combinedResolverMembers.Value; + (Type combinedResolverType, PropertyInfo underlyingResolverProperty) = s_combinedResolverMembers.Value; Assert.IsType(combinedResolverType, resolver); - return (IJsonTypeInfoResolver[])underlyingResolverField.GetValue(resolver); + return (IJsonTypeInfoResolver[])underlyingResolverProperty.GetValue(resolver); } - private static Lazy<(Type, FieldInfo)> s_combinedResolverMembers = new Lazy<(Type, FieldInfo)> + private static Lazy<(Type, PropertyInfo)> s_combinedResolverMembers = new Lazy<(Type, PropertyInfo)> ( 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); + PropertyInfo underlyingResolverProperty = combinedResolverType.GetProperty("Resolvers", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(underlyingResolverProperty); + return (combinedResolverType, underlyingResolverProperty); } ); } -- 2.7.4