From 2eaf1d20e7622486266b912d4bbc53b132a717a1 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 7 Jun 2023 15:16:19 +0100 Subject: [PATCH] Move the CreateCastingConverter() method to the base JsonConverter type. (#87211) --- .../Serialization/Converters/CastingConverter.cs | 8 ++-- .../Text/Json/Serialization/JsonConverter.cs | 36 +++++++++++++++- .../Json/Serialization/JsonConverterFactory.cs | 6 --- .../Text/Json/Serialization/JsonConverterOfT.cs | 50 ++++------------------ 4 files changed, 47 insertions(+), 53 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/CastingConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/CastingConverter.cs index 3d6ff9b..ece6207 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/CastingConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/CastingConverter.cs @@ -19,7 +19,7 @@ namespace System.Text.Json.Serialization.Converters public override bool HandleNull { get; } internal override bool SupportsCreateObjectDelegate => _sourceConverter.SupportsCreateObjectDelegate; - internal CastingConverter(JsonConverter sourceConverter, bool handleNull, bool handleNullOnRead, bool handleNullOnWrite) + internal CastingConverter(JsonConverter sourceConverter) { Debug.Assert(typeof(T).IsInSubtypeRelationshipWith(sourceConverter.TypeToConvert)); Debug.Assert(sourceConverter.SourceConverterForCastingConverter is null, "casting converters should not be layered."); @@ -31,9 +31,9 @@ namespace System.Text.Json.Serialization.Converters CanBePolymorphic = sourceConverter.CanBePolymorphic; // Ensure HandleNull values reflect the exact configuration of the source converter - HandleNullOnRead = handleNullOnRead; - HandleNullOnWrite = handleNullOnWrite; - HandleNull = handleNull; + HandleNullOnRead = sourceConverter.HandleNullOnRead; + HandleNullOnWrite = sourceConverter.HandleNullOnWrite; + HandleNull = sourceConverter.HandleNullOnWrite; } internal override JsonConverter? SourceConverterForCastingConverter => _sourceConverter; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs index c5bda10..54d2561 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs @@ -102,7 +102,41 @@ namespace System.Text.Json.Serialization throw new InvalidOperationException(); } - internal abstract JsonConverter CreateCastingConverter(); + internal JsonConverter CreateCastingConverter() + { + Debug.Assert(this is not JsonConverterFactory); + + if (this is JsonConverter conv) + { + return conv; + } + else + { + JsonSerializerOptions.CheckConverterNullabilityIsSameAsPropertyType(this, typeof(TTarget)); + + // Avoid layering casting converters by consulting any source converters directly. + return + SourceConverterForCastingConverter?.CreateCastingConverter() + ?? new CastingConverter(this); + } + } + + /// + /// Tracks whether the JsonConverter<T>.HandleNull property has been overridden by a derived converter. + /// + internal bool UsesDefaultHandleNull { get; private protected set; } + + /// + /// Does the converter want to be called when reading null tokens. + /// When JsonConverter<T>.HandleNull isn't overridden this can still be true for non-nullable structs. + /// + internal bool HandleNullOnRead { get; private protected init; } + + /// + /// Does the converter want to be called for null values. + /// Should always match the precise value of the JsonConverter<T>.HandleNull virtual property. + /// + internal bool HandleNullOnWrite { get; private protected init; } /// /// Set if this converter is itself a casting converter. 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 6d492a1..4ac5516 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 @@ -161,11 +161,5 @@ namespace System.Text.Json.Serialization throw new InvalidOperationException(); } - - internal sealed override JsonConverter CreateCastingConverter() - { - ThrowHelper.ThrowInvalidOperationException_ConverterCanConvertMultipleTypes(typeof(TTarget), this); - return null!; - } } } 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 e318ca7..34265f4 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 @@ -26,13 +26,14 @@ namespace System.Text.Json.Serialization HandleNullOnRead = true; HandleNullOnWrite = true; } - else + else if (UsesDefaultHandleNull) { - // For the HandleNull == false case, either: - // 1) The default values are assigned in this type's virtual HandleNull property - // or - // 2) A converter overrode HandleNull and returned false so HandleNullOnRead and HandleNullOnWrite - // will be their default values of false. + // If the type doesn't support null, allow the converter a chance to modify. + // These semantics are backwards compatible with 3.0. + HandleNullOnRead = default(T) is not null; + + // The framework handles null automatically on writes. + HandleNullOnWrite = false; } } @@ -56,21 +57,6 @@ namespace System.Text.Json.Serialization return new JsonTypeInfo(this, options); } - internal sealed override JsonConverter CreateCastingConverter() - { - if (this is JsonConverter conv) - { - return conv; - } - - JsonSerializerOptions.CheckConverterNullabilityIsSameAsPropertyType(this, typeof(TTarget)); - - // Avoid layering casting converters by consulting any source converters directly. - return - SourceConverterForCastingConverter?.CreateCastingConverter() - ?? new CastingConverter(this, handleNull: HandleNull, handleNullOnRead: HandleNullOnRead, handleNullOnWrite: HandleNullOnWrite); - } - internal override Type? KeyType => null; internal override Type? ElementType => null; @@ -86,31 +72,11 @@ namespace System.Text.Json.Serialization { get { - // HandleNull is only called by the framework once during initialization and any - // subsequent calls elsewhere would just re-initialize to the same values (we don't - // track a "hasInitialized" flag since that isn't necessary). - - // If the type doesn't support null, allow the converter a chance to modify. - // These semantics are backwards compatible with 3.0. - HandleNullOnRead = default(T) is not null; - - // The framework handles null automatically on writes. - HandleNullOnWrite = false; - + UsesDefaultHandleNull = true; return false; } } - /// - /// Does the converter want to be called when reading null tokens. - /// - internal bool HandleNullOnRead { get; private protected set; } - - /// - /// Does the converter want to be called for null values. - /// - internal bool HandleNullOnWrite { get; private protected set; } - // This non-generic API is sealed as it just forwards to the generic version. internal sealed override void WriteAsObject(Utf8JsonWriter writer, object? value, JsonSerializerOptions options) { -- 2.7.4