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 a23e89fd6d7ed..db80617e678d2 100644 --- a/src/libraries/System.Text.Json/ref/System.Text.Json.cs +++ b/src/libraries/System.Text.Json/ref/System.Text.Json.cs @@ -356,6 +356,7 @@ public JsonSerializerOptions(System.Text.Json.JsonSerializerOptions options) { } public System.Text.Json.JsonNamingPolicy? PropertyNamingPolicy { get { throw null; } set { } } public System.Text.Json.JsonCommentHandling ReadCommentHandling { get { throw null; } set { } } public System.Text.Json.Serialization.ReferenceHandler? ReferenceHandler { get { throw null; } set { } } + [System.Diagnostics.CodeAnalysis.AllowNullAttribute] public System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver TypeInfoResolver { [System.Diagnostics.CodeAnalysis.RequiresDynamicCodeAttribute("JSON serialization and deserialization might require types that cannot be statically analyzed and might need runtime code generation. Use System.Text.Json source generation for native AOT applications."), System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved.")] get { throw null; } set { } } public System.Text.Json.Serialization.JsonUnknownTypeHandling UnknownTypeHandling { get { throw null; } set { } } public bool WriteIndented { get { throw null; } set { } } diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx index 4d329769110a0..ed5b4f159a555 100644 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -400,10 +400,10 @@ The converter '{0}' is not compatible with the type '{1}'. - TypeInfoResolver expected to return JsonTypeInfo of type '{0}' but returned JsonTypeInfo of type '{1}'. + The IJsonTypeInfoResolver returned an incompatible JsonTypeInfo instance of type '{0}', expected type '{1}'. - TypeInfoResolver expected to return JsonTypeInfo options bound to the JsonSerializerOptions provided in the argument. + The IJsonTypeInfoResolver returned a JsonTypeInfo instance whose JsonSerializerOptions setting does not match the provided argument. The converter '{0}' wrote too much or not enough. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs index 3f38f57941220..02f9cb940970b 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs @@ -65,7 +65,7 @@ internal JsonConverter GetConverterInternal(Type typeToConvert, JsonSerializerOp break; } - return converter!; + return converter; } internal sealed override object ReadCoreAsObject( diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs index 3fdad107df10a..0b7afd0c555f0 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs @@ -84,6 +84,7 @@ internal sealed override JsonParameterInfo CreateJsonParameterInfo() internal sealed override JsonConverter CreateCastingConverter() { + JsonSerializerOptions.CheckConverterNullabilityIsSameAsPropertyType(this, typeof(TTarget)); return new CastingConverter(this); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs index 7e56f52f29ab9..5cd5bcd3c9b59 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs @@ -20,12 +20,9 @@ private static JsonTypeInfo GetTypeInfo(JsonSerializerOptions? options, Type run Debug.Assert(runtimeType != null); options ??= JsonSerializerOptions.Default; - if (!options.IsInitializedForReflectionSerializer) - { - options.InitializeForReflectionSerializer(); - } + options.InitializeForReflectionSerializer(); - return options.GetOrAddJsonTypeInfoForRootType(runtimeType); + return options.GetJsonTypeInfoForRootType(runtimeType); } private static JsonTypeInfo GetTypeInfo(JsonSerializerContext context, Type type) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs index 16415a1cdb634..ae344a1831d3a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs @@ -366,13 +366,7 @@ public static partial class JsonSerializer ThrowHelper.ThrowArgumentNullException(nameof(utf8Json)); } - options ??= JsonSerializerOptions.Default; - if (!options.IsInitializedForReflectionSerializer) - { - options.InitializeForReflectionSerializer(); - } - - JsonTypeInfo jsonTypeInfo = options.GetOrAddJsonTypeInfoForRootType(typeof(TValue)); + JsonTypeInfo jsonTypeInfo = GetTypeInfo(options, typeof(TValue)); return CreateAsyncEnumerableDeserializer(utf8Json, CreateQueueTypeInfo(jsonTypeInfo), cancellationToken); } 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 42da3e131bd4d..7d7a8ee09d5cf 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 @@ -57,11 +57,7 @@ private static void WriteUsingSerializer(Utf8JsonWriter writer, in TValu { Debug.Assert(writer != null); - Debug.Assert(!jsonTypeInfo.HasSerialize || - jsonTypeInfo is not JsonTypeInfo || - jsonTypeInfo.Options.SerializerContext == null || - !jsonTypeInfo.Options.SerializerContext.CanUseSerializationLogic, - "Incorrect method called. WriteUsingGeneratedSerializer() should have been called instead."); + // TODO unify method with WriteUsingGeneratedSerializer WriteStack state = default; jsonTypeInfo.EnsureConfigured(); 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 7e05b6347d1b6..b8159c3de583c 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 @@ -13,16 +13,27 @@ public abstract partial class JsonSerializerContext : IJsonTypeInfoResolver { private bool? _canUseSerializationLogic; - internal JsonSerializerOptions? _options; + private JsonSerializerOptions? _options; /// /// Gets the run time specified options of the context. If no options were passed /// when instanciating the context, then a new instance is bound and returned. /// /// - /// The instance cannot be mutated once it is bound to the context instance. + /// The options instance cannot be mutated once it is bound to the context instance. /// - public JsonSerializerOptions Options => _options ??= new JsonSerializerOptions { TypeInfoResolver = this }; + public JsonSerializerOptions Options + { + get => _options ??= new JsonSerializerOptions { TypeInfoResolver = this, IsLockedInstance = true }; + + internal set + { + Debug.Assert(!value.IsLockedInstance); + value.TypeInfoResolver = this; + value.IsLockedInstance = true; + _options = value; + } + } /// /// Indicates whether pre-generated serialization logic for types in the context @@ -84,8 +95,8 @@ protected JsonSerializerContext(JsonSerializerOptions? options) { if (options != null) { - options.TypeInfoResolver = this; - Debug.Assert(_options == options, "options.TypeInfoResolver setter did not assign options"); + options.VerifyMutable(); + Options = options; } } @@ -98,10 +109,9 @@ protected JsonSerializerContext(JsonSerializerOptions? options) JsonTypeInfo? IJsonTypeInfoResolver.GetTypeInfo(Type type, JsonSerializerOptions options) { - if (options != null && _options != options) + if (options != null && options != _options) { - // TODO is this the appropriate exception message to throw? - ThrowHelper.ThrowInvalidOperationException_SerializerContextOptionsImmutable(); + ThrowHelper.ThrowInvalidOperationException_ResolverTypeInfoOptionsNotCompatible(); } return GetTypeInfo(type); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs index e06683aebf243..1b670c0a18dbe 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs @@ -26,15 +26,15 @@ public sealed partial class JsonSerializerOptions /// /// This method returns configured non-null JsonTypeInfo /// - internal JsonTypeInfo GetOrAddJsonTypeInfo(Type type) + internal JsonTypeInfo GetJsonTypeInfoCached(Type type) { - if (_cachingContext == null) + JsonTypeInfo? typeInfo = null; + + if (IsLockedInstance) { - InitializeCachingContext(); + typeInfo = GetCachingContext()?.GetOrAddJsonTypeInfo(type); } - JsonTypeInfo? typeInfo = _cachingContext.GetOrAddJsonTypeInfo(type); - if (typeInfo == null) { ThrowHelper.ThrowNotSupportedException_NoMetadataForType(type); @@ -42,11 +42,10 @@ internal JsonTypeInfo GetOrAddJsonTypeInfo(Type type) } typeInfo.EnsureConfigured(); - return typeInfo; } - internal bool TryGetJsonTypeInfo(Type type, [NotNullWhen(true)] out JsonTypeInfo? typeInfo) + internal bool TryGetJsonTypeInfoCached(Type type, [NotNullWhen(true)] out JsonTypeInfo? typeInfo) { if (_cachingContext == null) { @@ -57,20 +56,18 @@ internal bool TryGetJsonTypeInfo(Type type, [NotNullWhen(true)] out JsonTypeInfo return _cachingContext.TryGetJsonTypeInfo(type, out typeInfo); } - internal bool IsJsonTypeInfoCached(Type type) => _cachingContext?.IsJsonTypeInfoCached(type) == true; - /// /// Return the TypeInfo for root API calls. /// This has an LRU cache that is intended only for public API calls that specify the root type. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal JsonTypeInfo GetOrAddJsonTypeInfoForRootType(Type type) + internal JsonTypeInfo GetJsonTypeInfoForRootType(Type type) { JsonTypeInfo? jsonTypeInfo = _lastTypeInfo; if (jsonTypeInfo?.Type != type) { - jsonTypeInfo = GetOrAddJsonTypeInfo(type); + jsonTypeInfo = GetJsonTypeInfoCached(type); _lastTypeInfo = jsonTypeInfo; } @@ -83,11 +80,16 @@ internal void ClearCaches() _lastTypeInfo = null; } - [MemberNotNull(nameof(_cachingContext))] - private void InitializeCachingContext() + private CachingContext? GetCachingContext() { - _isLockedInstance = true; - _cachingContext = TrackedCachingContexts.GetOrCreate(this); + Debug.Assert(IsLockedInstance); + + if (_cachingContext is null && _typeInfoResolver is not null) + { + _cachingContext = TrackedCachingContexts.GetOrCreate(this); + } + + return _cachingContext; } /// @@ -98,7 +100,7 @@ private void InitializeCachingContext() /// internal sealed class CachingContext { - private readonly ConcurrentDictionary _jsonTypeInfoCache = new(); + private readonly ConcurrentDictionary _jsonTypeInfoCache = new(); public CachingContext(JsonSerializerOptions options) { @@ -110,24 +112,8 @@ public CachingContext(JsonSerializerOptions options) // If changing please ensure that src/ILLink.Descriptors.LibraryBuild.xml is up-to-date. public int Count => _jsonTypeInfoCache.Count; - public JsonTypeInfo? GetOrAddJsonTypeInfo(Type type) - { - if (_jsonTypeInfoCache.TryGetValue(type, out JsonTypeInfo? typeInfo)) - { - return typeInfo; - } - - typeInfo = Options.GetTypeInfoInternal(type); - if (typeInfo != null) - { - return _jsonTypeInfoCache.GetOrAdd(type, _ => typeInfo); - } - - return null; - } - + public JsonTypeInfo? GetOrAddJsonTypeInfo(Type type) => _jsonTypeInfoCache.GetOrAdd(type, Options.GetTypeInfoNoCaching); public bool TryGetJsonTypeInfo(Type type, [NotNullWhen(true)] out JsonTypeInfo? typeInfo) => _jsonTypeInfoCache.TryGetValue(type, out typeInfo); - public bool IsJsonTypeInfoCached(Type type) => _jsonTypeInfoCache.ContainsKey(type); public void Clear() { @@ -147,12 +133,14 @@ internal static class TrackedCachingContexts new(concurrencyLevel: 1, capacity: MaxTrackedContexts, new EqualityComparer()); private const int EvictionCountHistory = 16; - private static Queue s_recentEvictionCounts = new(EvictionCountHistory); + private static readonly Queue s_recentEvictionCounts = new(EvictionCountHistory); private static int s_evictionRunsToSkip; public static CachingContext GetOrCreate(JsonSerializerOptions options) { - Debug.Assert(options._isLockedInstance, "Cannot create caching contexts for mutable JsonSerializerOptions instances"); + Debug.Assert(options.IsLockedInstance, "Cannot create caching contexts for mutable JsonSerializerOptions instances"); + Debug.Assert(options._typeInfoResolver != null); + ConcurrentDictionary> cache = s_cache; if (cache.TryGetValue(options, out WeakReference? wr) && wr.TryGetTarget(out CachingContext? ctx)) @@ -187,12 +175,7 @@ public static CachingContext GetOrCreate(JsonSerializerOptions options) // Use a defensive copy of the options instance as key to // avoid capturing references to any caching contexts. - var key = new JsonSerializerOptions(options) - { - // Copy fields ignored by the copy constructor - // but are necessary to determine equivalence. - _typeInfoResolver = options._typeInfoResolver, - }; + var key = new JsonSerializerOptions(options); Debug.Assert(key._cachingContext == null); ctx = new CachingContext(options); @@ -312,7 +295,7 @@ public bool Equals(JsonSerializerOptions? left, JsonSerializerOptions? right) left._includeFields == right._includeFields && left._propertyNameCaseInsensitive == right._propertyNameCaseInsensitive && left._writeIndented == right._writeIndented && - NormalizeResolver(left._typeInfoResolver) == NormalizeResolver(right._typeInfoResolver) && + left._typeInfoResolver == right._typeInfoResolver && CompareLists(left._converters, right._converters); static bool CompareLists(ConfigurationList left, ConfigurationList right) @@ -356,7 +339,7 @@ public int GetHashCode(JsonSerializerOptions options) hc.Add(options._includeFields); hc.Add(options._propertyNameCaseInsensitive); hc.Add(options._writeIndented); - hc.Add(NormalizeResolver(options._typeInfoResolver)); + hc.Add(options._typeInfoResolver); GetHashCode(ref hc, options._converters); static void GetHashCode(ref HashCode hc, ConfigurationList list) @@ -370,10 +353,6 @@ static void GetHashCode(ref HashCode hc, ConfigurationList list) return hc.ToHashCode(); } - // An options instance might be locked but not initialized for reflection serialization yet. - private static IJsonTypeInfoResolver? NormalizeResolver(IJsonTypeInfoResolver? resolver) - => resolver ?? DefaultJsonTypeInfoResolver.DefaultInstance; - #if !NETCOREAPP /// /// Polyfill for System.HashCode. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index 8b5119015f4ff..cfef45c315620 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -4,12 +4,9 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Reflection; using System.Text.Json.Reflection; using System.Text.Json.Serialization; -using System.Text.Json.Serialization.Converters; using System.Text.Json.Serialization.Metadata; -using System.Threading; namespace System.Text.Json { @@ -26,67 +23,6 @@ public sealed partial class JsonSerializerOptions /// public IList Converters => _converters; - // This may return factory converter - internal JsonConverter? GetCustomConverterFromMember(Type typeToConvert, MemberInfo memberInfo) - { - Debug.Assert(memberInfo.DeclaringType != null, "Properties and fields always have a declaring type."); - JsonConverter? converter = null; - - JsonConverterAttribute? converterAttribute = memberInfo.GetUniqueCustomAttribute(inherit: false); - if (converterAttribute != null) - { - converter = GetConverterFromAttribute(converterAttribute, typeToConvert, memberInfo); - } - - return converter; - } - - /// - /// Gets converter for type but does not use TypeInfoResolver - /// - internal JsonConverter GetConverterForType(Type typeToConvert) - { - JsonConverter converter = GetConverterFromOptionsOrReflectionConverter(typeToConvert); - Debug.Assert(converter != null); - - converter = ExpandFactoryConverter(converter, typeToConvert); - - CheckConverterNullabilityIsSameAsPropertyType(converter, typeToConvert); - - return converter; - } - - [return: NotNullIfNotNull("converter")] - internal JsonConverter? ExpandFactoryConverter(JsonConverter? converter, Type typeToConvert) - { - if (converter is JsonConverterFactory factory) - { - converter = factory.GetConverterInternal(typeToConvert, this); - - // A factory cannot return null; GetConverterInternal checked for that. - Debug.Assert(converter != null); - } - - return converter; - } - - internal static void CheckConverterNullabilityIsSameAsPropertyType(JsonConverter converter, Type propertyType) - { - // User has indicated that either: - // a) a non-nullable-struct handling converter should handle a nullable struct type or - // b) a nullable-struct handling converter should handle a non-nullable struct type. - // User should implement a custom converter for the underlying struct and remove the unnecessary CanConvert method override. - // The serializer will automatically wrap the custom converter with NullableConverter. - // - // We also throw to avoid passing an invalid argument to setters for nullable struct properties, - // which would cause an InvalidProgramException when the generated IL is invoked. - if (propertyType.IsValueType && converter.IsValueType && - (propertyType.IsNullableOfT() ^ converter.TypeToConvert.IsNullableOfT())) - { - ThrowHelper.ThrowInvalidOperationException_ConverterCanConvertMultipleTypes(propertyType, converter); - } - } - /// /// Returns the converter for the specified type. /// @@ -110,7 +46,7 @@ public JsonConverter GetConverter(Type typeToConvert) ThrowHelper.ThrowArgumentNullException(nameof(typeToConvert)); } - DefaultJsonTypeInfoResolver.RootDefaultInstance(); + _typeInfoResolver ??= DefaultJsonTypeInfoResolver.RootDefaultInstance(); return GetConverterFromTypeInfo(typeToConvert); } @@ -119,39 +55,25 @@ public JsonConverter GetConverter(Type typeToConvert) /// internal JsonConverter GetConverterFromTypeInfo(Type typeToConvert) { - if (_cachingContext == null) - { - if (_isLockedInstance) - { - InitializeCachingContext(); - } - else - { - // We do not want to lock options instance here but we need to return correct answer - // which means we need to go through TypeInfoResolver but without caching because that's the - // only place which will have correct converter for JsonSerializerContext and reflection - // based resolver. It will also work correctly for combined resolvers. - return GetTypeInfoInternal(typeToConvert)?.Converter - ?? GetConverterFromOptionsOrReflectionConverter(typeToConvert); + JsonConverter? converter; - } + if (IsLockedInstance) + { + converter = GetCachingContext()?.GetOrAddJsonTypeInfo(typeToConvert)?.Converter; } - - JsonConverter? converter = _cachingContext.GetOrAddJsonTypeInfo(typeToConvert)?.Converter; - - // we can get here if resolver returned null but converter was added for the type - converter ??= GetConverterFromOptions(typeToConvert); - - if (converter == null) + else { - ThrowHelper.ThrowNotSupportedException_BuiltInConvertersNotRooted(typeToConvert); - return null!; + // We do not want to lock options instance here but we need to return correct answer + // which means we need to go through TypeInfoResolver but without caching because that's the + // only place which will have correct converter for JsonSerializerContext and reflection + // based resolver. It will also work correctly for combined resolvers. + converter = GetTypeInfoNoCaching(typeToConvert)?.Converter; } - return converter; + return converter ?? GetConverterFromListOrBuiltInConverter(typeToConvert); } - private JsonConverter? GetConverterFromOptions(Type typeToConvert) + internal JsonConverter? GetConverterFromList(Type typeToConvert) { foreach (JsonConverter item in _converters) { @@ -164,93 +86,54 @@ internal JsonConverter GetConverterFromTypeInfo(Type typeToConvert) return null; } - private JsonConverter GetConverterFromOptionsOrReflectionConverter(Type typeToConvert) + internal JsonConverter GetConverterFromListOrBuiltInConverter(Type typeToConvert) { - Debug.Assert(typeToConvert != null); + JsonConverter? converter = GetConverterFromList(typeToConvert); + return GetCustomOrBuiltInConverter(typeToConvert, converter); + } - // Priority 1: Attempt to get custom converter from the Converters list. - JsonConverter? converter = GetConverterFromOptions(typeToConvert); + internal JsonConverter GetCustomOrBuiltInConverter(Type typeToConvert, JsonConverter? converter) + { + // Attempt to get built-in converter. + converter ??= DefaultJsonTypeInfoResolver.GetBuiltInConverter(typeToConvert); + // Expand potential convert converter factory. + converter = ExpandConverterFactory(converter, typeToConvert); - // Priority 2: Attempt to get converter from [JsonConverter] on the type being converted. - if (converter == null) + if (!converter.TypeToConvert.IsInSubtypeRelationshipWith(typeToConvert)) { - JsonConverterAttribute? converterAttribute = typeToConvert.GetUniqueCustomAttribute(inherit: false); - if (converterAttribute != null) - { - converter = GetConverterFromAttribute(converterAttribute, typeToConvert: typeToConvert, memberInfo: null); - } + ThrowHelper.ThrowInvalidOperationException_SerializationConverterNotCompatible(converter.GetType(), converter.TypeToConvert); } - // Priority 3: Attempt to get built-in converter. - converter ??= DefaultJsonTypeInfoResolver.GetDefaultConverter(typeToConvert); + CheckConverterNullabilityIsSameAsPropertyType(converter, typeToConvert); + return converter; + } - // Allow redirection for generic types or the enum converter. + [return: NotNullIfNotNull("converter")] + internal JsonConverter? ExpandConverterFactory(JsonConverter? converter, Type typeToConvert) + { if (converter is JsonConverterFactory factory) { converter = factory.GetConverterInternal(typeToConvert, this); - - // A factory cannot return null; GetConverterInternal checked for that. - Debug.Assert(converter != null); - } - - Type converterTypeToConvert = converter.TypeToConvert; - - if (!converterTypeToConvert.IsInSubtypeRelationshipWith(typeToConvert)) - { - ThrowHelper.ThrowInvalidOperationException_SerializationConverterNotCompatible(converter.GetType(), typeToConvert); } return converter; } - // This suppression needs to be removed. https://github.com/dotnet/runtime/issues/68878 - [UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode", Justification = "The factory constructors are only invoked in the context of reflection serialization code paths " + - "and are marked RequiresDynamicCode")] - private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, MemberInfo? memberInfo) + internal static void CheckConverterNullabilityIsSameAsPropertyType(JsonConverter converter, Type propertyType) { - JsonConverter? converter; - - Type declaringType = memberInfo?.DeclaringType ?? typeToConvert; - Type? converterType = converterAttribute.ConverterType; - if (converterType == null) - { - // Allow the attribute to create the converter. - converter = converterAttribute.CreateConverter(typeToConvert); - if (converter == null) - { - ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(declaringType, memberInfo, typeToConvert); - } - } - else - { - ConstructorInfo? ctor = converterType.GetConstructor(Type.EmptyTypes); - if (!typeof(JsonConverter).IsAssignableFrom(converterType) || ctor == null || !ctor.IsPublic) - { - ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeInvalid(declaringType, memberInfo); - } - - converter = (JsonConverter)Activator.CreateInstance(converterType)!; - } - - Debug.Assert(converter != null); - if (!converter.CanConvert(typeToConvert)) + // User has indicated that either: + // a) a non-nullable-struct handling converter should handle a nullable struct type or + // b) a nullable-struct handling converter should handle a non-nullable struct type. + // User should implement a custom converter for the underlying struct and remove the unnecessary CanConvert method override. + // The serializer will automatically wrap the custom converter with NullableConverter. + // + // We also throw to avoid passing an invalid argument to setters for nullable struct properties, + // which would cause an InvalidProgramException when the generated IL is invoked. + if (propertyType.IsValueType && converter.IsValueType && + (propertyType.IsNullableOfT() ^ converter.TypeToConvert.IsNullableOfT())) { - Type? underlyingType = Nullable.GetUnderlyingType(typeToConvert); - if (underlyingType != null && converter.CanConvert(underlyingType)) - { - if (converter is JsonConverterFactory converterFactory) - { - converter = converterFactory.GetConverterInternal(underlyingType, this); - } - - // Allow nullable handling to forward to the underlying type's converter. - return NullableConverterFactory.CreateValueConverter(underlyingType, converter); - } - - ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(declaringType, memberInfo, typeToConvert); + ThrowHelper.ThrowInvalidOperationException_ConverterCanConvertMultipleTypes(propertyType, converter); } - - return converter; } } } 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 fe9bd9e2df955..33eb11b76bb28 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 @@ -57,7 +57,7 @@ public sealed partial class JsonSerializerOptions private bool _propertyNameCaseInsensitive; private bool _writeIndented; - private bool _isLockedInstance; + private volatile bool _isLockedInstance; /// /// Constructs a new instance. @@ -102,9 +102,7 @@ public JsonSerializerOptions(JsonSerializerOptions options) _includeFields = options._includeFields; _propertyNameCaseInsensitive = options._propertyNameCaseInsensitive; _writeIndented = options._writeIndented; - // Preserve backward compatibility with .NET 6 - // This should almost certainly be changed, cf. https://github.com/dotnet/aspnetcore/issues/38720 - _typeInfoResolver = options._typeInfoResolver is JsonSerializerContext ? null : options._typeInfoResolver; + _typeInfoResolver = options._typeInfoResolver; EffectiveMaxDepth = options.EffectiveMaxDepth; ReferenceHandlingStrategy = options.ReferenceHandlingStrategy; @@ -149,51 +147,35 @@ public JsonSerializerOptions(JsonSerializerDefaults defaults) : this() /// Binds current instance with a new instance of the specified type. /// /// The generic definition of the specified context type. - /// When serializing and deserializing types using the options + /// + /// When serializing and deserializing types using the options /// instance, metadata for the types will be fetched from the context instance. /// public void AddContext() where TContext : JsonSerializerContext, new() { VerifyMutable(); TContext context = new(); - _typeInfoResolver = context; - _isLockedInstance = true; - context._options = this; + context.Options = this; } /// - /// Gets or sets JsonTypeInfo resolver. + /// Gets or sets a contract resolver. /// + /// + /// A setting is equivalent to using the reflection-based . + /// + [AllowNull] public IJsonTypeInfoResolver TypeInfoResolver { [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] get { - return _typeInfoResolver ?? DefaultJsonTypeInfoResolver.RootDefaultInstance(); + return _typeInfoResolver ??= DefaultJsonTypeInfoResolver.RootDefaultInstance(); } set { VerifyMutable(); - - if (value is null) - { - ThrowHelper.ThrowArgumentNullException(nameof(value)); - } - - if (value is JsonSerializerContext ctx) - { - if (ctx._options != null && ctx._options != this) - { - // TODO evaluate if this is the appropriate behaviour; - ThrowHelper.ThrowInvalidOperationException_SerializerContextOptionsImmutable(); - } - - // Associate options instance with context and lock for further modification - ctx._options = this; - _isLockedInstance = true; - } - _typeInfoResolver = value; } } @@ -616,10 +598,15 @@ internal MemberAccessor MemberAccessorStrategy } } - internal bool IsInitializedForReflectionSerializer { get; private set; } - // Effective resolver, populated when enacting reflection-based fallback - // Should not be taken into account when calculating options equality. - private IJsonTypeInfoResolver? _effectiveJsonTypeInfoResolver; + internal bool IsLockedInstance + { + get => _isLockedInstance; + set + { + Debug.Assert(value, "cannot unlock options instances"); + _isLockedInstance = true; + } + } /// /// Initializes the converters for the reflection-based serializer. @@ -628,44 +615,47 @@ internal MemberAccessor MemberAccessorStrategy [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] internal void InitializeForReflectionSerializer() { - if (_typeInfoResolver is JsonSerializerContext ctx) + if (!_isInitializedForReflectionSerializer) { - // .NET 6 backward compatibility; use fallback to reflection serialization - // TODO: Consider removing this behaviour (needs to be filed as a breaking change). - _effectiveJsonTypeInfoResolver = JsonTypeInfoResolver.Combine(ctx, DefaultJsonTypeInfoResolver.RootDefaultInstance()); - } - else - { - _typeInfoResolver ??= DefaultJsonTypeInfoResolver.RootDefaultInstance(); - } + DefaultJsonTypeInfoResolver defaultResolver = DefaultJsonTypeInfoResolver.RootDefaultInstance(); + _typeInfoResolver ??= defaultResolver; + IsLockedInstance = true; - if (_cachingContext != null && _cachingContext.Options != this) - { - // We're using a shared caching context deriving from a different options instance; - // for coherence ensure that it has been opted in for reflection-based serialization as well. - _cachingContext.Options.InitializeForReflectionSerializer(); - } + CachingContext? context = GetCachingContext(); + Debug.Assert(context != null); + + if (context.Options != this) + { + // We're using a shared caching context deriving from a different options instance; + // for coherence ensure that it has been opted in for reflection-based serialization as well. + context.Options.InitializeForReflectionSerializer(); + } - IsInitializedForReflectionSerializer = true; + _isInitializedForReflectionSerializer = true; + } } - internal bool IsInitializedForMetadataGeneration { get; private set; } + private volatile bool _isInitializedForReflectionSerializer; + internal void InitializeForMetadataGeneration() { - IJsonTypeInfoResolver? resolver = _effectiveJsonTypeInfoResolver ?? _typeInfoResolver; - if (resolver == null) + if (!_isInitializedForMetadataGeneration) { - ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoUsedButTypeInfoResolverNotSet(); - } + if (_typeInfoResolver is null) + { + ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoUsedButTypeInfoResolverNotSet(); + } - _isLockedInstance = true; - IsInitializedForMetadataGeneration = true; + IsLockedInstance = true; + _isInitializedForMetadataGeneration = true; + } } - private JsonTypeInfo? GetTypeInfoInternal(Type type) + private volatile bool _isInitializedForMetadataGeneration; + + private JsonTypeInfo? GetTypeInfoNoCaching(Type type) { - IJsonTypeInfoResolver? resolver = _effectiveJsonTypeInfoResolver ?? _typeInfoResolver; - JsonTypeInfo? info = resolver?.GetTypeInfo(type, this); + JsonTypeInfo? info = _typeInfoResolver?.GetTypeInfo(type, this); if (info != null) { @@ -742,13 +732,13 @@ public ConverterList(JsonSerializerOptions options, IList? source _options = options; } - protected override bool IsLockedInstance => _options._isLockedInstance; + protected override bool IsLockedInstance => _options.IsLockedInstance; protected override void VerifyMutable() => _options.VerifyMutable(); } private static JsonSerializerOptions CreateDefaultImmutableInstance() { - var options = new JsonSerializerOptions { _isLockedInstance = true }; + var options = new JsonSerializerOptions { IsLockedInstance = true }; return options; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/CustomJsonTypeInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/CustomJsonTypeInfoOfT.cs index 10599434e6713..53e0a5a68bc01 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/CustomJsonTypeInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/CustomJsonTypeInfoOfT.cs @@ -19,7 +19,7 @@ internal sealed class CustomJsonTypeInfo : JsonTypeInfo /// Creates serialization metadata for a type using a simple converter. /// internal CustomJsonTypeInfo(JsonSerializerOptions options) - : base(options.GetConverterForType(typeof(T)), options) + : base(options.GetConverterFromListOrBuiltInConverter(typeof(T)), options) { } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Converters.cs index 1ab5bff497acf..dc8fbc5e188ff 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Converters.cs @@ -4,6 +4,8 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Reflection; +using System.Text.Json.Reflection; using System.Text.Json.Serialization.Converters; namespace System.Text.Json.Serialization.Metadata @@ -79,7 +81,7 @@ void Add(JsonConverter converter) => converters.Add(converter.TypeToConvert, converter); } - internal static JsonConverter GetDefaultConverter(Type typeToConvert) + internal static JsonConverter GetBuiltInConverter(Type typeToConvert) { if (s_defaultSimpleConverters == null || s_defaultFactoryConverters == null) { @@ -122,5 +124,99 @@ internal static bool TryGetDefaultSimpleConverter(Type typeToConvert, [NotNullWh return s_defaultSimpleConverters.TryGetValue(typeToConvert, out converter); } + + // This method gets the runtime information for a given type or property. + // The runtime information consists of the following: + // - class type, + // - element type (if the type is a collection), + // - the converter (either native or custom), if one exists. + [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] + [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] + internal static JsonConverter GetConverterForMember( + Type typeToConvert, + MemberInfo memberInfo, + JsonSerializerOptions options, + out JsonConverter? customConverter) + { + Debug.Assert(memberInfo is FieldInfo or PropertyInfo); + Debug.Assert(typeToConvert != null); + + JsonConverterAttribute? converterAttribute = memberInfo.GetUniqueCustomAttribute(inherit: false); + customConverter = converterAttribute is null ? null : GetConverterFromAttribute(converterAttribute, typeToConvert, memberInfo, options); + + return options.TryGetJsonTypeInfoCached(typeToConvert, out JsonTypeInfo? typeInfo) + ? typeInfo.Converter + : GetConverterForType(typeToConvert, options); + } + + [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] + [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] + internal static JsonConverter GetConverterForType(Type typeToConvert, JsonSerializerOptions options) + { + // Priority 1: Attempt to get custom converter from the Converters list. + JsonConverter? converter = options.GetConverterFromList(typeToConvert); + + // Priority 2: Attempt to get converter from [JsonConverter] on the type being converted. + if (converter == null) + { + JsonConverterAttribute? converterAttribute = typeToConvert.GetUniqueCustomAttribute(inherit: false); + if (converterAttribute != null) + { + converter = GetConverterFromAttribute(converterAttribute, typeToConvert: typeToConvert, memberInfo: null, options); + } + } + + // Priority 3: Fall back to built-in converters and validate result + return options.GetCustomOrBuiltInConverter(typeToConvert, converter); + } + + [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] + [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] + private static JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, MemberInfo? memberInfo, JsonSerializerOptions options) + { + JsonConverter? converter; + + Type declaringType = memberInfo?.DeclaringType ?? typeToConvert; + Type? converterType = converterAttribute.ConverterType; + if (converterType == null) + { + // Allow the attribute to create the converter. + converter = converterAttribute.CreateConverter(typeToConvert); + if (converter == null) + { + ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(declaringType, memberInfo, typeToConvert); + } + } + else + { + ConstructorInfo? ctor = converterType.GetConstructor(Type.EmptyTypes); + if (!typeof(JsonConverter).IsAssignableFrom(converterType) || ctor == null || !ctor.IsPublic) + { + ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeInvalid(declaringType, memberInfo); + } + + converter = (JsonConverter)Activator.CreateInstance(converterType)!; + } + + Debug.Assert(converter != null); + if (!converter.CanConvert(typeToConvert)) + { + Type? underlyingType = Nullable.GetUnderlyingType(typeToConvert); + if (underlyingType != null && converter.CanConvert(underlyingType)) + { + if (converter is JsonConverterFactory converterFactory) + { + converter = converterFactory.GetConverterInternal(underlyingType, options); + } + + // Allow nullable handling to forward to the underlying type's converter. + return NullableConverterFactory.CreateValueConverter(underlyingType, converter); + } + + ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(declaringType, memberInfo, typeToConvert); + } + + return converter; + } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs index 4d9403b81d022..9a262ed3648e1 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs @@ -39,7 +39,7 @@ public JsonTypeInfo JsonTypeInfo { Debug.Assert(Options != null); Debug.Assert(ShouldDeserialize); - return _jsonTypeInfo ??= Options.GetOrAddJsonTypeInfo(PropertyType); + return _jsonTypeInfo ??= Options.GetJsonTypeInfoCached(PropertyType); } set { @@ -97,7 +97,7 @@ public static JsonParameterInfo CreateIgnoredParameterPlaceholder( Type parameterType = parameterInfo.ParameterType; DefaultValueHolder holder; - if (matchingProperty.Options.TryGetJsonTypeInfo(parameterType, out JsonTypeInfo? typeInfo)) + if (matchingProperty.Options.TryGetJsonTypeInfoCached(parameterType, out JsonTypeInfo? typeInfo)) { holder = typeInfo.DefaultValueHolder; } 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 51c39366e586e..857e0f86aad24 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 @@ -757,7 +757,7 @@ internal JsonTypeInfo JsonTypeInfo else { // GetOrAddJsonTypeInfo already ensures it's configured. - _jsonTypeInfo = Options.GetOrAddJsonTypeInfo(PropertyType); + _jsonTypeInfo = Options.GetJsonTypeInfoCached(PropertyType); } return _jsonTypeInfo; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs index b57245ba49279..faa58bd81fc1e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs @@ -184,8 +184,7 @@ private protected override void DetermineEffectiveConverter() JsonConverter? customConverter = CustomConverter; if (customConverter != null) { - customConverter = Options.ExpandFactoryConverter(customConverter, PropertyType); - JsonSerializerOptions.CheckConverterNullabilityIsSameAsPropertyType(customConverter, PropertyType); + customConverter = Options.ExpandConverterFactory(customConverter, PropertyType); } JsonConverter converter = customConverter ?? DefaultConverterForType ?? Options.GetConverterFromTypeInfo(PropertyType); 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 5727026b9a0b4..861b693b27e3b 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 @@ -226,7 +226,7 @@ internal JsonTypeInfo? ElementTypeInfo { // GetOrAddJsonTypeInfo already ensures JsonTypeInfo is configured // also see comment on JsonPropertyInfo.JsonTypeInfo - _elementTypeInfo = Options.GetOrAddJsonTypeInfo(ElementType); + _elementTypeInfo = Options.GetJsonTypeInfoCached(ElementType); } } else @@ -268,7 +268,7 @@ internal JsonTypeInfo? KeyTypeInfo // GetOrAddJsonTypeInfo already ensures JsonTypeInfo is configured // also see comment on JsonPropertyInfo.JsonTypeInfo - _keyTypeInfo = Options.GetOrAddJsonTypeInfo(KeyType); + _keyTypeInfo = Options.GetJsonTypeInfoCached(KeyType); } } else @@ -400,6 +400,8 @@ internal void VerifyMutable() internal void EnsureConfigured() { + Debug.Assert(!Monitor.IsEntered(_configureLock), "recursive locking detected."); + if (!_isConfigured) ConfigureLocked(); @@ -432,11 +434,7 @@ void ConfigureLocked() internal virtual void Configure() { Debug.Assert(Monitor.IsEntered(_configureLock), "Configure called directly, use EnsureConfigured which locks this method"); - - if (!Options.IsInitializedForMetadataGeneration) - { - Options.InitializeForMetadataGeneration(); - } + Options.InitializeForMetadataGeneration(); PropertyInfoForTypeInfo.EnsureChildOf(this); PropertyInfoForTypeInfo.EnsureConfigured(); @@ -582,7 +580,7 @@ public JsonPropertyInfo CreateJsonPropertyInfo(Type propertyType, string name) ThrowHelper.ThrowArgumentException_CannotSerializeInvalidType(nameof(propertyType), propertyType, Type, name); } - JsonConverter converter = Options.GetConverterForType(propertyType); + JsonConverter converter = Options.GetConverterFromListOrBuiltInConverter(propertyType); JsonPropertyInfo propertyInfo = CreatePropertyUsingReflection(propertyType, converter); propertyInfo.Name = name; @@ -891,23 +889,6 @@ internal JsonPropertyDictionary CreatePropertyCache(int capaci return new JsonPropertyDictionary(Options.PropertyNameCaseInsensitive, capacity); } - // This method gets the runtime information for a given type or property. - // The runtime information consists of the following: - // - class type, - // - element type (if the type is a collection), - // - the converter (either native or custom), if one exists. - private protected static JsonConverter GetConverterFromMember( - Type typeToConvert, - MemberInfo memberInfo, - JsonSerializerOptions options, - out JsonConverter? customConverter) - { - Debug.Assert(typeToConvert != null); - Debug.Assert(!IsInvalidForSerialization(typeToConvert), $"Type `{typeToConvert.FullName}` should already be validated."); - customConverter = options.GetCustomConverterFromMember(typeToConvert, memberInfo); - return options.GetConverterForType(typeToConvert); - } - private static JsonParameterInfo CreateConstructorParameter( JsonParameterInfoValues parameterInfo, JsonPropertyInfo jsonPropertyInfo, diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PolymorphicTypeResolver.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PolymorphicTypeResolver.cs index 6f50d0298b1f7..c2a73ea6ee928 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PolymorphicTypeResolver.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PolymorphicTypeResolver.cs @@ -249,7 +249,7 @@ public DerivedJsonTypeInfo(Type type, object? typeDiscriminator) public Type DerivedType { get; } public object? TypeDiscriminator { get; } public JsonTypeInfo GetJsonTypeInfo(JsonSerializerOptions options) - => _jsonTypeInfo ??= options.GetOrAddJsonTypeInfo(DerivedType); + => _jsonTypeInfo ??= options.GetJsonTypeInfoCached(DerivedType); } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs index 4d4557408b0f6..a0efc0629a26d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs @@ -6,6 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Text.Json.Reflection; +using System.Text.Json.Serialization.Converters; namespace System.Text.Json.Serialization.Metadata { @@ -17,7 +18,7 @@ internal sealed class ReflectionJsonTypeInfo : JsonTypeInfo [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] internal ReflectionJsonTypeInfo(JsonSerializerOptions options) - : this(options.GetConverterForType(typeof(T)), options) + : this(DefaultJsonTypeInfoResolver.GetConverterForType(typeof(T), options), options) { } @@ -196,7 +197,7 @@ private void CacheMember( try { - converter = GetConverterFromMember( + converter = DefaultJsonTypeInfoResolver.GetConverterForMember( typeToConvert, memberInfo, options, diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs index ee15c3fecb00e..5ff6e12662862 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs @@ -95,7 +95,7 @@ private void EnsurePushCapacity() public void Initialize(Type type, JsonSerializerOptions options, bool supportContinuation) { - JsonTypeInfo jsonTypeInfo = options.GetOrAddJsonTypeInfoForRootType(type); + JsonTypeInfo jsonTypeInfo = options.GetJsonTypeInfoForRootType(type); Initialize(jsonTypeInfo, supportContinuation); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs index b6f448100d07b..f3fbe1a7ad817 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs @@ -138,7 +138,7 @@ private void EnsurePushCapacity() /// public JsonConverter Initialize(Type type, JsonSerializerOptions options, bool supportContinuation, bool supportAsync) { - JsonTypeInfo jsonTypeInfo = options.GetOrAddJsonTypeInfoForRootType(type); + JsonTypeInfo jsonTypeInfo = options.GetJsonTypeInfoForRootType(type); return Initialize(jsonTypeInfo, supportContinuation, supportAsync); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs index 5ecd1603d9434..43bf3ef4daa22 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs @@ -126,7 +126,7 @@ public JsonConverter InitializePolymorphicReEntry(Type runtimeType, JsonSerializ // if the current element is the same type as the previous element. if (PolymorphicJsonTypeInfo?.PropertyType != runtimeType) { - JsonTypeInfo typeInfo = options.GetOrAddJsonTypeInfo(runtimeType); + JsonTypeInfo typeInfo = options.GetJsonTypeInfoCached(runtimeType); PolymorphicJsonTypeInfo = typeInfo.PropertyInfoForTypeInfo; } 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 5c4967107a1d5..cb15bfe034ffb 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 @@ -120,7 +120,7 @@ public static void ThrowInvalidOperationException_SerializationConverterNotCompa [DoesNotReturn] public static void ThrowInvalidOperationException_ResolverTypeNotCompatible(Type requestedType, Type actualType) { - throw new InvalidOperationException(SR.Format(SR.ResolverTypeNotCompatible, requestedType, actualType)); + throw new InvalidOperationException(SR.Format(SR.ResolverTypeNotCompatible, actualType, requestedType)); } [DoesNotReturn] diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.cs index 1bcbef4ff58ae..51c8904f16cfa 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.cs @@ -197,6 +197,7 @@ public static void GetConverter_Poco_WriteThrowsNotSupportedException() // since it can't resolve reflection-based metadata. Assert.Throws(() => converter.Write(writer, value, options)); Assert.Equal(0, writer.BytesCommitted + writer.BytesPending); + options.IncludeFields = false; // options should still be mutable JsonSerializer.Serialize(42, options); @@ -205,6 +206,8 @@ public static void GetConverter_Poco_WriteThrowsNotSupportedException() Assert.NotEqual(0, writer.BytesCommitted + writer.BytesPending); writer.Reset(); + Assert.Throws(() => options.IncludeFields = false); + // State change should not leak into unrelated options instances. var options2 = new JsonSerializerOptions(); options2.AddContext(); 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 12178942288e4..8db7c72e00e96 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 @@ -1101,5 +1101,32 @@ private class RecursiveType public int Value { get; set; } public RecursiveType? Next { get; set; } } + + [Fact] + public static void CreateJsonTypeInfo_ClassWithConverterAttribute_ShouldNotResolveConverterAttribute() + { + JsonTypeInfo jsonTypeInfo = JsonTypeInfo.CreateJsonTypeInfo(typeof(ClassWithConverterAttribute), JsonSerializerOptions.Default); + Assert.Equal(typeof(ClassWithConverterAttribute), jsonTypeInfo.Type); + Assert.IsNotType(jsonTypeInfo.Converter); + } + + [Fact] + public static void DefaultJsonTypeInfoResolver_ClassWithConverterAttribute_ShouldResolveConverterAttribute() + { + var options = JsonSerializerOptions.Default; + JsonTypeInfo jsonTypeInfo = options.TypeInfoResolver.GetTypeInfo(typeof(ClassWithConverterAttribute), options); + Assert.Equal(typeof(ClassWithConverterAttribute), jsonTypeInfo.Type); + Assert.IsType(jsonTypeInfo.Converter); + } + + [JsonConverter(typeof(CustomConverter))] + public class ClassWithConverterAttribute + { + public class CustomConverter : JsonConverter + { + public override ClassWithConverterAttribute? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => throw new NotImplementedException(); + public override void Write(Utf8JsonWriter writer, ClassWithConverterAttribute value, JsonSerializerOptions options) => throw new NotImplementedException(); + } + } } } 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 c1f53298ac5df..93c53a0f63f17 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 @@ -87,6 +87,25 @@ public void OptionsImmutableAfterBinding() CauseInvalidOperationException(() => options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase); } + [Fact] + public void PassingImmutableOptionsThrowsException() + { + JsonSerializerOptions defaultOptions = JsonSerializerOptions.Default; + Assert.Throws(() => new MyJsonContext(defaultOptions)); + } + + [Fact] + public void PassingWrongOptionsInstanceToResolverThrowsException() + { + JsonSerializerOptions defaultOptions = JsonSerializerOptions.Default; + JsonSerializerOptions contextOptions = new(); + IJsonTypeInfoResolver context = new EmptyContext(contextOptions); + + Assert.IsAssignableFrom>(context.GetTypeInfo(typeof(int), contextOptions)); + Assert.IsAssignableFrom>(context.GetTypeInfo(typeof(int), null)); + Assert.Throws(() => context.GetTypeInfo(typeof(int), defaultOptions)); + } + private class MyJsonContext : JsonSerializerContext { public MyJsonContext() : base(null) { } @@ -104,5 +123,12 @@ public MyJsonContextThatSetsOptionsInParameterlessCtor() : base(new JsonSerializ public override JsonTypeInfo? GetTypeInfo(Type type) => throw new NotImplementedException(); protected override JsonSerializerOptions? GeneratedSerializerOptions => null; } + + private class EmptyContext : JsonSerializerContext + { + public EmptyContext(JsonSerializerOptions options) : base(options) { } + protected override JsonSerializerOptions? GeneratedSerializerOptions => null; + public override JsonTypeInfo? GetTypeInfo(Type type) => JsonTypeInfo.CreateJsonTypeInfo(type, Options); + } } } 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 56c1ccdbef95b..40764bf7d5859 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 @@ -7,7 +7,9 @@ using System.Reflection; using System.Text.Encodings.Web; using System.Text.Json.Serialization.Metadata; +using System.Text.Json.Tests; using System.Text.Unicode; +using Microsoft.DotNet.RemoteExecutor; using Xunit; namespace System.Text.Json.Serialization.Tests @@ -34,9 +36,6 @@ public static void SetOptionsFail() TestIListNonThrowingOperationsWhenMutable(options.Converters, () => new TestConverter()); - // Verify TypeInfoResolver throws on null resolver - Assert.Throws(() => options.TypeInfoResolver = null); - // Verify default TypeInfoResolver throws Action tiModifier = (ti) => { }; Assert.Throws(() => (options.TypeInfoResolver as DefaultJsonTypeInfoResolver).Modifiers.Clear()); @@ -171,7 +170,15 @@ public static void WhenAddingContextTypeInfoResolverAsContextOptionsAreSameAsOpt } [Fact] - public static void TypeInfoResolverCannotBeSetAfterContextIsSetThroughTypeInfoResolver() + public static void WhenAddingContext_SettingResolverToNullThrowsInvalidOperationException() + { + var options = new JsonSerializerOptions(); + options.AddContext(); + Assert.Throws(() => options.TypeInfoResolver = null); + } + + [Fact] + public static void TypeInfoResolverCanBeSetAfterContextIsSetThroughTypeInfoResolver() { var options = new JsonSerializerOptions(); IJsonTypeInfoResolver resolver = new JsonContext(); @@ -179,7 +186,8 @@ public static void TypeInfoResolverCannotBeSetAfterContextIsSetThroughTypeInfoRe Assert.Same(resolver, options.TypeInfoResolver); resolver = new DefaultJsonTypeInfoResolver(); - Assert.Throws(() => options.TypeInfoResolver = resolver); + options.TypeInfoResolver = resolver; + Assert.Same(resolver, options.TypeInfoResolver); } [Fact] @@ -424,6 +432,54 @@ public static void Options_GetConverterForObjectJsonElement_GivesCorrectConverte GenericObjectOrJsonElementConverterTestHelper("JsonElementConverter", element, "[3]"); } + [Fact] + public static void Options_JsonSerializerContext_DoesNotFallbackToReflection() + { + var options = JsonContext.Default.Options; + JsonSerializer.Serialize(new WeatherForecastWithPOCOs(), options); // type supported by context should succeed serialization + + var unsupportedValue = new MyClass(); + Assert.Null(JsonContext.Default.GetTypeInfo(unsupportedValue.GetType())); + Assert.Throws(() => JsonSerializer.Serialize(unsupportedValue, unsupportedValue.GetType(), JsonContext.Default)); + Assert.Throws(() => JsonSerializer.Serialize(unsupportedValue, options)); + } + + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public static void Options_JsonSerializerContext_GetConverter_FallsBackToReflectionConverter() + { + RemoteExecutor.Invoke(static () => + { + JsonContext context = JsonContext.Default; + var unsupportedValue = new MyClass(); + + // Default converters have not been rooted yet + Assert.Null(context.GetTypeInfo(typeof(MyClass))); + Assert.Throws(() => context.Options.GetConverter(typeof(MyClass))); + + // Root converters process-wide by calling a Serialize overload accepting options + Assert.Throws(() => JsonSerializer.Serialize(unsupportedValue, context.Options)); + + // We still can't resolve metadata for MyClass, but we can now resolve a converter using the rooted converters. + Assert.Null(context.GetTypeInfo(typeof(MyClass))); + Assert.IsAssignableFrom>(context.Options.GetConverter(typeof(MyClass))); + + }).Dispose(); + } + + [Fact] + public static void Options_JsonSerializerContext_Combine_FallbackToReflection() + { + var options = new JsonSerializerOptions + { + TypeInfoResolver = JsonTypeInfoResolver.Combine(JsonContext.Default, new DefaultJsonTypeInfoResolver()) + }; + + var value = new MyClass(); + string json = JsonSerializer.Serialize(value, options); + JsonTestHelper.AssertJsonEqual("""{"Value":null,"Thing":null}""", json); + } + private static void GenericObjectOrJsonElementConverterTestHelper(string converterName, object objectValue, string stringValue) { var options = new JsonSerializerOptions(); @@ -596,6 +652,26 @@ public static void CopyConstructor_CopiesAllPublicProperties() VerifyOptionsEqual(options, newOptions); } + [Fact] + public static void CopyConstructor_CopiesJsonSerializerContext() + { + JsonSerializerOptions options = new JsonSerializerOptions(); + options.AddContext(); + JsonContext original = Assert.IsType(options.TypeInfoResolver); + + // copy constructor copies the JsonSerializerContext + var newOptions = new JsonSerializerOptions(options); + Assert.Same(original, newOptions.TypeInfoResolver); + + // resolving metadata returns metadata tied to the new options + JsonTypeInfo typeInfo = newOptions.TypeInfoResolver.GetTypeInfo(typeof(int), newOptions); + Assert.Same(typeInfo.Options, newOptions); + + // it is possible to reset the resolver + newOptions.TypeInfoResolver = null; + Assert.IsType(newOptions.TypeInfoResolver); + } + [Fact] public static void CopyConstructor_NullInput() {