Fix reference preservation for boxed structs (#56412)

* Fix reference preservation for boxed structs.

* add null assertion for BoxedStructReferenceId when writing a new instance

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs
index 9f7fa9d..ba83667 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs
@@ -16,6 +16,8 @@ namespace System.Text.Json.Serialization.Converters
         where TDictionary : IEnumerable<KeyValuePair<TKey, TValue>>
         where TKey : notnull
     {
+        internal override bool CanHaveIdMetadata => true;
+
         protected internal override bool OnWriteResume(
             Utf8JsonWriter writer,
             TDictionary value,

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableDefaultConverter.cs
index b623f42..c8d1c5f 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableDefaultConverter.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableDefaultConverter.cs
@@ -12,6 +12,8 @@ namespace System.Text.Json.Serialization.Converters
     internal abstract class IEnumerableDefaultConverter<TCollection, TElement> : JsonCollectionConverter<TCollection, TElement>
         where TCollection : IEnumerable<TElement>
     {
+        internal override bool CanHaveIdMetadata => true;
+
         protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options, ref WriteStack state)
         {
             Debug.Assert(value is not null);

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs
index 2f9bc99..a1f70ed 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs
@@ -14,6 +14,8 @@ namespace System.Text.Json.Serialization.Converters
     /// </summary>
     internal class ObjectDefaultConverter<T> : JsonObjectConverter<T> where T : notnull
     {
+        internal override bool CanHaveIdMetadata => true;
+
         internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, ref ReadStack state, [MaybeNullWhen(false)] out T value)
         {
             JsonTypeInfo jsonTypeInfo = state.Current.JsonTypeInfo;

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 f590292..f1626c4 100644 (file)
--- 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
@@ -30,7 +30,7 @@ namespace System.Text.Json.Serialization
         /// <summary>
         /// Can the converter have $id metadata.
         /// </summary>
-        internal virtual bool CanHaveIdMetadata => true;
+        internal virtual bool CanHaveIdMetadata => false;
 
         internal bool CanBePolymorphic { get; set; }
 

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 e083653..89d6c38 100644 (file)
--- 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
@@ -379,12 +379,26 @@ namespace System.Text.Json.Serialization
                         JsonConverter jsonConverter = state.Current.InitializeReEntry(type, options);
                         Debug.Assert(jsonConverter != this);
 
-                        if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.IgnoreCycles &&
-                            jsonConverter.IsValueType)
+                        // For boxed value types: invoke the reference handler
+                        // before the instance gets unboxed by the subtype converter.
+                        if (jsonConverter.IsValueType)
                         {
-                            // For boxed value types: push the value before it gets unboxed on TryWriteAsObject.
-                            state.ReferenceResolver.PushReferenceForCycleDetection(value);
-                            ignoreCyclesPopReference = true;
+                            switch (options.ReferenceHandlingStrategy)
+                            {
+                                case ReferenceHandlingStrategy.Preserve when (jsonConverter.CanHaveIdMetadata && !state.IsContinuation):
+                                    if (JsonSerializer.TryWriteReferenceForBoxedStruct(value, ref state, writer))
+                                    {
+                                        return true;
+                                    }
+                                    break;
+
+                                case ReferenceHandlingStrategy.IgnoreCycles:
+                                    state.ReferenceResolver.PushReferenceForCycleDetection(value);
+                                    ignoreCyclesPopReference = true;
+                                    break;
+                                default:
+                                    break;
+                            }
                         }
 
                         // We found a different converter; forward to that.

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleMetadata.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleMetadata.cs
index 45c1462..e792bcf 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleMetadata.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleMetadata.cs
@@ -21,9 +21,19 @@ namespace System.Text.Json
         {
             MetadataPropertyName writtenMetadataName;
 
-            // If the jsonConverter supports immutable dictionaries or value types, don't write any metadata
-            if (!jsonConverter.CanHaveIdMetadata || jsonConverter.IsValueType)
+            if (state.BoxedStructReferenceId != null)
             {
+                // We're serializing a struct that has been handled by a polymorphic converter;
+                // emit the reference id that was recorded for the boxed instance.
+
+                Debug.Assert(jsonConverter.IsValueType && jsonConverter.CanHaveIdMetadata);
+                writer.WriteString(s_metadataId, state.BoxedStructReferenceId);
+                writtenMetadataName = MetadataPropertyName.Id;
+                state.BoxedStructReferenceId = null;
+            }
+            else if (!jsonConverter.CanHaveIdMetadata || jsonConverter.IsValueType)
+            {
+                // If the jsonConverter supports immutable dictionaries or value types, don't write any metadata
                 writtenMetadataName = MetadataPropertyName.NoMetadata;
             }
             else
@@ -55,9 +65,22 @@ namespace System.Text.Json
         {
             MetadataPropertyName writtenMetadataName;
 
-            // If the jsonConverter supports immutable enumerables or value type collections, don't write any metadata
-            if (!jsonConverter.CanHaveIdMetadata || jsonConverter.IsValueType)
+            if (state.BoxedStructReferenceId != null)
             {
+                // We're serializing a struct that has been handled by a polymorphic converter;
+                // emit the reference id that was recorded for the boxed instance.
+
+                Debug.Assert(jsonConverter.IsValueType && jsonConverter.CanHaveIdMetadata);
+
+                writer.WriteStartObject();
+                writer.WriteString(s_metadataId, state.BoxedStructReferenceId);
+                writer.WriteStartArray(s_metadataValues);
+                writtenMetadataName = MetadataPropertyName.Id;
+                state.BoxedStructReferenceId = null;
+            }
+            else if (!jsonConverter.CanHaveIdMetadata || jsonConverter.IsValueType)
+            {
+                // If the jsonConverter supports immutable enumerables or value type collections, don't write any metadata
                 writer.WriteStartArray();
                 writtenMetadataName = MetadataPropertyName.NoMetadata;
             }
@@ -84,5 +107,32 @@ namespace System.Text.Json
 
             return writtenMetadataName;
         }
+
+        /// <summary>
+        /// Used by polymorphic converters that are handling references for values that are boxed structs.
+        /// </summary>
+        internal static bool TryWriteReferenceForBoxedStruct(object currentValue, ref WriteStack state, Utf8JsonWriter writer)
+        {
+            Debug.Assert(state.BoxedStructReferenceId == null);
+            Debug.Assert(currentValue.GetType().IsValueType);
+
+            string referenceId = state.ReferenceResolver.GetReference(currentValue, out bool alreadyExists);
+            Debug.Assert(referenceId != null);
+
+            if (alreadyExists)
+            {
+                writer.WriteStartObject();
+                writer.WriteString(s_metadataRef, referenceId);
+                writer.WriteEndObject();
+            }
+            else
+            {
+                // Since we cannot run `ReferenceResolver.GetReference` twice for newly encountered instances,
+                // need to store the reference id for use by the subtype converter we're dispatching to.
+                state.BoxedStructReferenceId = referenceId;
+            }
+
+            return alreadyExists;
+        }
     }
 }

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 e9a9f1e..bc313da 100644 (file)
--- 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
@@ -70,6 +70,11 @@ namespace System.Text.Json
         /// </summary>
         public bool SupportContinuation;
 
+        /// <summary>
+        /// Stores a reference id that has been calculated by a polymorphic converter handling a newly encountered boxed struct.
+        /// </summary>
+        public string? BoxedStructReferenceId;
+
         private void EnsurePushCapacity()
         {
             if (_stack is null)

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReferenceHandlerTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReferenceHandlerTests.cs
index 030f637..45e5ff4 100644 (file)
--- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReferenceHandlerTests.cs
+++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ReferenceHandlerTests.cs
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Collections;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.IO;
@@ -780,6 +781,56 @@ namespace System.Text.Json.Serialization.Tests
             Assert.Equal("$.Sibling", ex.Path);
         }
 
+        [Fact]
+        public static void BoxedStructReferencePreservation_NestedStructObject()
+        {
+            IBoxedStructWithObjectProperty value = new StructWithObjectProperty();
+            value.Property = new object[] { value };
+
+            string json = JsonSerializer.Serialize(value, new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.Preserve });
+            Assert.Equal(@"{""$id"":""1"",""Property"":[{""$ref"":""1""}]}", json);
+        }
+
+        [Fact]
+        public static void BoxedStructReferencePreservation_NestedStructCollection()
+        {
+            IBoxedStructWithObjectProperty value = new StructCollection();
+            value.Property = new object[] { value };
+
+            string json = JsonSerializer.Serialize(value, s_serializerOptionsPreserve);
+            Assert.Equal(@"{""$id"":""1"",""Property"":[{""$ref"":""1""}]}", json);
+        }
+
+        [Fact]
+        public static void BoxedStructReferencePreservation_SiblingStructObjects()
+        {
+            object box = new StructWithObjectProperty { Property = 42 };
+            var array = new object[] { box, box };
+
+            string json = JsonSerializer.Serialize(array, s_serializerOptionsPreserve);
+            Assert.Equal(@"[{""$id"":""1"",""Property"":42},{""$ref"":""1""}]", json);
+        }
+
+        [Fact]
+        public static void BoxedStructReferencePreservation_SiblingStructCollections()
+        {
+            object box = new StructCollection { Property = 42 };
+            var array = new object[] { box, box };
+
+            string json = JsonSerializer.Serialize(array, s_serializerOptionsPreserve);
+            Assert.Equal(@"[{""$id"":""1"",""$values"":[42]},{""$ref"":""1""}]", json);
+        }
+
+        [Fact]
+        public static void BoxedStructReferencePreservation_SiblingPrimitiveValues()
+        {
+            object box = 42;
+            var array = new object[] { box, box };
+
+            string json = JsonSerializer.Serialize(array, s_serializerOptionsPreserve);
+            Assert.Equal(@"[42,42]", json);
+        }
+
         private class ClassWithObjectProperty
         {
             public ClassWithObjectProperty Child { get; set; }
@@ -791,5 +842,28 @@ namespace System.Text.Json.Serialization.Tests
             public ClassWithListOfObjectProperty Child { get; set; }
             public List<object> ListOfObjects { get; set; }
         }
+
+        private interface IBoxedStructWithObjectProperty
+        {
+            object? Property { get; set; }
+        }
+
+        private struct StructWithObjectProperty : IBoxedStructWithObjectProperty
+        {
+            public object? Property { get; set; }
+        }
+
+        private struct StructCollection : IBoxedStructWithObjectProperty, IEnumerable<object?>
+        {
+            public object? Property { get; set; }
+
+            public IEnumerator<object?> GetEnumerator()
+            {
+                yield return Property;
+            }
+
+            IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+        }
+
     }
 }