STJ: Support serialization callbacks for collection and dictionary ty…

…pes (#104120) * STJ: Support serialization callbacks for collection and dictionary types * Fix tests * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/JsonCollectionConverter.cs Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com> * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/JsonDictionaryConverter.cs Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com> * Reverse Kind logic to be more future proof * Trigger callbacks before writing any JSON or metadata content * Add JsonTypeInfo tests * Call OnSerializing before any writing operation * Keep result as variable name for partial operations * Prevent setting OnDeserialize callback on immutable types * Avoid using reflection when possible * Set IsImmutableType for all converters overriding ConvertCollection * Rename to IsImmutableCollectionType * Remove extra empty lines * Rename exception message * Rename immutable -> convertible and fix issue around callback use for struct types --------- Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
dotnet · Jul 12, 2024 · 856a0a6 · 856a0a6
1 parent 45f3250
commit 856a0a6
Show file tree

Hide file tree

Showing 17 changed files with 379 additions and 29 deletions.
diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx
@@ -674,6 +674,9 @@
   <data name="InvalidJsonTypeInfoOperationForKind" xml:space="preserve">
     <value>Invalid JsonTypeInfo operation for JsonTypeInfoKind '{0}'.</value>
   </data>
+  <data name="OnDeserializingCallbacksNotSupported" xml:space="preserve">
+    <value>The type '{0}' does not support setting OnDeserializing callbacks.</value>
+  </data>
   <data name="CreateObjectConverterNotCompatible" xml:space="preserve">
     <value>The converter for type '{0}' does not support setting 'CreateObject' delegates.</value>
   </data>

diff --git a/...stem.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ArrayConverter.cs b/...stem.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ArrayConverter.cs
@@ -24,6 +24,7 @@ protected override void CreateCollection(ref Utf8JsonReader reader, scoped ref R
             state.Current.ReturnValue = new List<TElement>();
         }
 
+        internal sealed override bool IsConvertibleCollection => true;
         protected override void ConvertCollection(ref ReadStack state, JsonSerializerOptions options)
         {
             List<TElement> list = (List<TElement>)state.Current.ReturnValue!;

diff --git a/...Text/Json/Serialization/Converters/Collection/ImmutableDictionaryOfTKeyTValueConverter.cs b/...Text/Json/Serialization/Converters/Collection/ImmutableDictionaryOfTKeyTValueConverter.cs
@@ -25,6 +25,7 @@ protected sealed override void CreateCollection(ref Utf8JsonReader reader, scope
             state.Current.ReturnValue = new Dictionary<TKey, TValue>();
         }
 
+        internal sealed override bool IsConvertibleCollection => true;
         protected sealed override void ConvertCollection(ref ReadStack state, JsonSerializerOptions options)
         {
             Func<IEnumerable<KeyValuePair<TKey, TValue>>, TDictionary>? creator =

diff --git a/...c/System/Text/Json/Serialization/Converters/Collection/ImmutableEnumerableOfTConverter.cs b/...c/System/Text/Json/Serialization/Converters/Collection/ImmutableEnumerableOfTConverter.cs
@@ -24,6 +24,7 @@ protected sealed override void CreateCollection(ref Utf8JsonReader reader, scope
             state.Current.ReturnValue = new List<TElement>();
         }
 
+        internal sealed override bool IsConvertibleCollection => true;
         protected sealed override void ConvertCollection(ref ReadStack state, JsonSerializerOptions options)
         {
             JsonTypeInfo typeInfo = state.Current.JsonTypeInfo;

diff --git a/....Json/src/System/Text/Json/Serialization/Converters/Collection/JsonCollectionConverter.cs b/....Json/src/System/Text/Json/Serialization/Converters/Collection/JsonCollectionConverter.cs
@@ -41,6 +41,10 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, scoped ref Re
             Debug.Assert(state.Current.ReturnValue is TCollection);
         }
 
+        /// <summary>
+        /// When overridden, converts the temporary collection held in state.Current.ReturnValue to the final collection.
+        /// The <see cref="JsonConverter.IsConvertibleCollection"/> property must also be set to <see langword="true"/>.
+        /// </summary>
         protected virtual void ConvertCollection(ref ReadStack state, JsonSerializerOptions options) { }
 
         protected static JsonConverter<TElement> GetElementConverter(JsonTypeInfo elementTypeInfo)
@@ -61,7 +65,8 @@ internal override bool OnTryRead(
             scoped ref ReadStack state,
             [MaybeNullWhen(false)] out TCollection value)
         {
-            JsonTypeInfo elementTypeInfo = state.Current.JsonTypeInfo.ElementTypeInfo!;
+            JsonTypeInfo jsonTypeInfo = state.Current.JsonTypeInfo;
+            JsonTypeInfo elementTypeInfo = jsonTypeInfo.ElementTypeInfo!;
 
             if (!state.SupportContinuation && !state.Current.CanContainMetadata)
             {
@@ -74,6 +79,8 @@ internal override bool OnTryRead(
 
                 CreateCollection(ref reader, ref state, options);
 
+                jsonTypeInfo.OnDeserializing?.Invoke(state.Current.ReturnValue!);
+
                 state.Current.JsonPropertyInfo = elementTypeInfo.PropertyInfoForTypeInfo;
                 JsonConverter<TElement> elementConverter = GetElementConverter(elementTypeInfo);
                 if (elementConverter.CanUseDirectReadOrWrite && state.Current.NumberHandling == null)
@@ -112,8 +119,6 @@ internal override bool OnTryRead(
             else
             {
                 // Slower path that supports continuation and reading metadata.
-                JsonTypeInfo jsonTypeInfo = state.Current.JsonTypeInfo;
-
                 if (state.Current.ObjectState == StackFrameObjectState.None)
                 {
                     if (reader.TokenType == JsonTokenType.StartArray)
@@ -183,6 +188,8 @@ internal override bool OnTryRead(
                         state.ReferenceId = null;
                     }
 
+                    jsonTypeInfo.OnDeserializing?.Invoke(state.Current.ReturnValue!);
+
                     state.Current.ObjectState = StackFrameObjectState.CreatedObject;
                 }
 
@@ -274,7 +281,10 @@ internal override bool OnTryRead(
             }
 
             ConvertCollection(ref state, options);
-            value = (TCollection)state.Current.ReturnValue!;
+            object returnValue = state.Current.ReturnValue!;
+            jsonTypeInfo.OnDeserialized?.Invoke(returnValue);
+            value = (TCollection)returnValue;
+
             return true;
         }
 
@@ -293,18 +303,22 @@ internal override bool OnTryWrite(
             }
             else
             {
+                JsonTypeInfo jsonTypeInfo = state.Current.JsonTypeInfo;
+
                 if (!state.Current.ProcessedStartToken)
                 {
                     state.Current.ProcessedStartToken = true;
 
+                    jsonTypeInfo.OnSerializing?.Invoke(value);
+
                     if (state.CurrentContainsMetadata && CanHaveMetadata)
                     {
                         state.Current.MetadataPropertyName = JsonSerializer.WriteMetadataForCollection(this, ref state, writer);
                     }
 
                     // Writing the start of the array must happen after any metadata
                     writer.WriteStartArray();
-                    state.Current.JsonPropertyInfo = state.Current.JsonTypeInfo.ElementTypeInfo!.PropertyInfoForTypeInfo;
+                    state.Current.JsonPropertyInfo = jsonTypeInfo.ElementTypeInfo!.PropertyInfoForTypeInfo;
                 }
 
                 success = OnWriteResume(writer, value, options, ref state);
@@ -321,6 +335,8 @@ internal override bool OnTryWrite(
                             writer.WriteEndObject();
                         }
                     }
+
+                    jsonTypeInfo.OnSerialized?.Invoke(value);
                 }
             }
 

diff --git a/....Json/src/System/Text/Json/Serialization/Converters/Collection/JsonDictionaryConverter.cs b/....Json/src/System/Text/Json/Serialization/Converters/Collection/JsonDictionaryConverter.cs
@@ -76,8 +76,9 @@ internal sealed override bool OnTryRead(
             scoped ref ReadStack state,
             [MaybeNullWhen(false)] out TDictionary value)
         {
-            JsonTypeInfo keyTypeInfo = state.Current.JsonTypeInfo.KeyTypeInfo!;
-            JsonTypeInfo elementTypeInfo = state.Current.JsonTypeInfo.ElementTypeInfo!;
+            JsonTypeInfo jsonTypeInfo = state.Current.JsonTypeInfo;
+            JsonTypeInfo keyTypeInfo = jsonTypeInfo.KeyTypeInfo!;
+            JsonTypeInfo elementTypeInfo = jsonTypeInfo.ElementTypeInfo!;
 
             if (!state.SupportContinuation && !state.Current.CanContainMetadata)
             {
@@ -90,6 +91,8 @@ internal sealed override bool OnTryRead(
 
                 CreateCollection(ref reader, ref state);
 
+                jsonTypeInfo.OnDeserializing?.Invoke(state.Current.ReturnValue!);
+
                 _keyConverter ??= GetConverter<TKey>(keyTypeInfo);
                 _valueConverter ??= GetConverter<TValue>(elementTypeInfo);
 
@@ -149,8 +152,6 @@ internal sealed override bool OnTryRead(
             else
             {
                 // Slower path that supports continuation and reading metadata.
-                JsonTypeInfo jsonTypeInfo = state.Current.JsonTypeInfo;
-
                 if (state.Current.ObjectState == StackFrameObjectState.None)
                 {
                     if (reader.TokenType != JsonTokenType.StartObject)
@@ -210,6 +211,8 @@ internal sealed override bool OnTryRead(
                         state.ReferenceId = null;
                     }
 
+                    jsonTypeInfo.OnDeserializing?.Invoke(state.Current.ReturnValue!);
+
                     state.Current.ObjectState = StackFrameObjectState.CreatedObject;
                 }
 
@@ -302,7 +305,10 @@ internal sealed override bool OnTryRead(
             }
 
             ConvertCollection(ref state, options);
-            value = (TDictionary)state.Current.ReturnValue!;
+            object result = state.Current.ReturnValue!;
+            jsonTypeInfo.OnDeserialized?.Invoke(result);
+            value = (TDictionary)result;
+
             return true;
 
             static TKey ReadDictionaryKey(JsonConverter<TKey> keyConverter, ref Utf8JsonReader reader, scoped ref ReadStack state, JsonSerializerOptions options)
@@ -337,17 +343,22 @@ internal sealed override bool OnTryWrite(
                 return true;
             }
 
+            JsonTypeInfo jsonTypeInfo = state.Current.JsonTypeInfo;
+
             if (!state.Current.ProcessedStartToken)
             {
                 state.Current.ProcessedStartToken = true;
+
+                jsonTypeInfo.OnSerializing?.Invoke(dictionary);
+
                 writer.WriteStartObject();
 
                 if (state.CurrentContainsMetadata && CanHaveMetadata)
                 {
                     JsonSerializer.WriteMetadataForObject(this, ref state, writer);
                 }
 
-                state.Current.JsonPropertyInfo = state.Current.JsonTypeInfo.ElementTypeInfo!.PropertyInfoForTypeInfo;
+                state.Current.JsonPropertyInfo = jsonTypeInfo.ElementTypeInfo!.PropertyInfoForTypeInfo;
             }
 
             bool success = OnWriteResume(writer, dictionary, options, ref state);
@@ -358,6 +369,8 @@ internal sealed override bool OnTryWrite(
                     state.Current.ProcessedEndToken = true;
                     writer.WriteEndObject();
                 }
+
+                jsonTypeInfo.OnSerialized?.Invoke(dictionary);
             }
 
             return success;

diff --git a/...tem.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs b/...tem.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/MemoryConverter.cs
@@ -37,6 +37,7 @@ protected override void CreateCollection(ref Utf8JsonReader reader, scoped ref R
             state.Current.ReturnValue = new List<T>();
         }
 
+        internal sealed override bool IsConvertibleCollection => true;
         protected override void ConvertCollection(ref ReadStack state, JsonSerializerOptions options)
         {
             Memory<T> memory = ((List<T>)state.Current.ReturnValue!).ToArray().AsMemory();

diff --git a/....Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs b/....Json/src/System/Text/Json/Serialization/Converters/Collection/ReadOnlyMemoryConverter.cs
@@ -37,6 +37,7 @@ protected override void CreateCollection(ref Utf8JsonReader reader, scoped ref R
             state.Current.ReturnValue = new List<T>();
         }
 
+        internal sealed override bool IsConvertibleCollection => true;
         protected override void ConvertCollection(ref ReadStack state, JsonSerializerOptions options)
         {
             ReadOnlyMemory<T> memory = ((List<T>)state.Current.ReturnValue!).ToArray().AsMemory();

diff --git a/...tem.Text.Json/src/System/Text/Json/Serialization/Converters/FSharp/FSharpListConverter.cs b/...tem.Text.Json/src/System/Text/Json/Serialization/Converters/FSharp/FSharpListConverter.cs
@@ -31,6 +31,7 @@ protected override void CreateCollection(ref Utf8JsonReader reader, scoped ref R
             state.Current.ReturnValue = new List<TElement>();
         }
 
+        internal sealed override bool IsConvertibleCollection => true;
         protected override void ConvertCollection(ref ReadStack state, JsonSerializerOptions options)
         {
             state.Current.ReturnValue = _listConstructor((List<TElement>)state.Current.ReturnValue!);

diff --git a/...stem.Text.Json/src/System/Text/Json/Serialization/Converters/FSharp/FSharpMapConverter.cs b/...stem.Text.Json/src/System/Text/Json/Serialization/Converters/FSharp/FSharpMapConverter.cs
@@ -34,6 +34,7 @@ protected override void CreateCollection(ref Utf8JsonReader reader, scoped ref R
             state.Current.ReturnValue = new List<Tuple<TKey, TValue>>();
         }
 
+        internal sealed override bool IsConvertibleCollection => true;
         protected override void ConvertCollection(ref ReadStack state, JsonSerializerOptions options)
         {
             state.Current.ReturnValue = _mapConstructor((List<Tuple<TKey, TValue>>)state.Current.ReturnValue!);

diff --git a/...stem.Text.Json/src/System/Text/Json/Serialization/Converters/FSharp/FSharpSetConverter.cs b/...stem.Text.Json/src/System/Text/Json/Serialization/Converters/FSharp/FSharpSetConverter.cs
@@ -31,6 +31,7 @@ protected override void CreateCollection(ref Utf8JsonReader reader, scoped ref R
             state.Current.ReturnValue = new List<TElement>();
         }
 
+        internal sealed override bool IsConvertibleCollection => true;
         protected override void ConvertCollection(ref ReadStack state, JsonSerializerOptions options)
         {
             state.Current.ReturnValue = _setConstructor((List<TElement>)state.Current.ReturnValue!);

diff --git a/....Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs b/....Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs
@@ -326,15 +326,15 @@ internal sealed override bool OnTryWrite(
 
             if (!state.SupportContinuation)
             {
+                jsonTypeInfo.OnSerializing?.Invoke(obj);
+
                 writer.WriteStartObject();
 
                 if (state.CurrentContainsMetadata && CanHaveMetadata)
                 {
                     JsonSerializer.WriteMetadataForObject(this, ref state, writer);
                 }
 
-                jsonTypeInfo.OnSerializing?.Invoke(obj);
-
                 foreach (JsonPropertyInfo jsonPropertyInfo in jsonTypeInfo.PropertyCache)
                 {
                     if (jsonPropertyInfo.CanSerialize)

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
@@ -179,6 +179,13 @@ internal JsonConverter<TTarget> CreateCastingConverter<TTarget>()
         /// </summary>
         internal bool IsInternalConverterForNumberType { get; init; }
 
+        /// <summary>
+        /// Whether the converter handles collection deserialization by converting from
+        /// an intermediate buffer such as immutable collections, arrays or memory types.
+        /// Used in conjunction with <see cref="JsonCollectionConverter{TCollection, TElement}.ConvertCollection(ref ReadStack, JsonSerializerOptions)"/>.
+        /// </summary>
+        internal virtual bool IsConvertibleCollection => false;
+
         internal static bool ShouldFlush(ref WriteStack state, Utf8JsonWriter writer)
         {
             Debug.Assert(state.FlushThreshold == 0 || (state.PipeWriter is { CanGetUnflushedBytes: true }),

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
@@ -123,7 +123,7 @@ public Action<object>? OnSerializing
             {
                 VerifyMutable();
 
-                if (Kind != JsonTypeInfoKind.Object)
+                if (Kind is not (JsonTypeInfoKind.Object or JsonTypeInfoKind.Enumerable or JsonTypeInfoKind.Dictionary))
                 {
                     ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoOperationNotPossibleForKind(Kind);
                 }
@@ -153,7 +153,7 @@ public Action<object>? OnSerialized
             {
                 VerifyMutable();
 
-                if (Kind != JsonTypeInfoKind.Object)
+                if (Kind is not (JsonTypeInfoKind.Object or JsonTypeInfoKind.Enumerable or JsonTypeInfoKind.Dictionary))
                 {
                     ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoOperationNotPossibleForKind(Kind);
                 }
@@ -183,11 +183,17 @@ public Action<object>? OnDeserializing
             {
                 VerifyMutable();
 
-                if (Kind != JsonTypeInfoKind.Object)
+                if (Kind is not (JsonTypeInfoKind.Object or JsonTypeInfoKind.Enumerable or JsonTypeInfoKind.Dictionary))
                 {
                     ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoOperationNotPossibleForKind(Kind);
                 }
 
+                if (Converter.IsConvertibleCollection)
+                {
+                    // The values for convertible collections aren't available at the start of deserialization.
+                    ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoOnDeserializingCallbacksNotSupported(Type);
+                }
+
                 _onDeserializing = value;
             }
         }
@@ -213,7 +219,7 @@ public Action<object>? OnDeserialized
             {
                 VerifyMutable();
 
-                if (Kind != JsonTypeInfoKind.Object)
+                if (Kind is not (JsonTypeInfoKind.Object or JsonTypeInfoKind.Enumerable or JsonTypeInfoKind.Dictionary))
                 {
                     ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoOperationNotPossibleForKind(Kind);
                 }
@@ -1256,9 +1262,7 @@ internal void MapInterfaceTypesToCallbacks()
         {
             Debug.Assert(!IsReadOnly);
 
-            // Callbacks currently only supported in object kinds
-            // TODO: extend to collections/dictionaries
-            if (Kind == JsonTypeInfoKind.Object)
+            if (Kind is JsonTypeInfoKind.Object or JsonTypeInfoKind.Enumerable or JsonTypeInfoKind.Dictionary)
             {
                 if (typeof(IJsonOnSerializing).IsAssignableFrom(Type))
                 {

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
@@ -414,6 +414,12 @@ public static void ThrowInvalidOperationException_JsonTypeInfoOperationNotPossib
             throw new InvalidOperationException(SR.Format(SR.InvalidJsonTypeInfoOperationForKind, kind));
         }
 
+        [DoesNotReturn]
+        public static void ThrowInvalidOperationException_JsonTypeInfoOnDeserializingCallbacksNotSupported(Type type)
+        {
+            throw new InvalidOperationException(SR.Format(SR.OnDeserializingCallbacksNotSupported, type));
+        }
+
         [DoesNotReturn]
         public static void ThrowInvalidOperationException_CreateObjectConverterNotCompatible(Type type)
         {