From c934df0017f16770be41c1e397bc49629bcb7337 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Oct 2025 17:42:44 +0000 Subject: [PATCH 1/7] Initial plan From 01aa977ccef7d1feb0773cd86b7a6fb839813bd6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Oct 2025 18:29:03 +0000 Subject: [PATCH 2/7] Add IReadOnlyDictionary support for JsonExtensionData attribute Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../Attributes/JsonExtensionDataAttribute.cs | 8 ++- .../JsonSerializer.Read.HandlePropertyName.cs | 20 +++++- .../Serialization/Metadata/JsonTypeInfo.cs | 5 ++ .../tests/Common/ExtensionDataTests.cs | 69 +++++++++++++++++++ ...nTypeInfoResolverTests.JsonPropertyInfo.cs | 2 + 5 files changed, 100 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonExtensionDataAttribute.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonExtensionDataAttribute.cs index 42b9957d3e32de..21da84f2976293 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonExtensionDataAttribute.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonExtensionDataAttribute.cs @@ -4,12 +4,14 @@ namespace System.Text.Json.Serialization { /// - /// When placed on a property or field of type or - /// , any properties that do not have a + /// When placed on a property or field of type , + /// , or + /// , any properties that do not have a /// matching property or field are added during deserialization and written during serialization. /// /// - /// When using , the TKey value must be + /// When using or + /// , the TKey value must be /// and TValue must be or . /// /// During deserializing with a extension property with TValue as diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs index 64a9de11e893a7..d018aef43c8e16 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs @@ -114,7 +114,9 @@ internal static void CreateExtensionDataProperty( { // Create the appropriate dictionary type. We already verified the types. #if DEBUG - Type underlyingIDictionaryType = jsonPropertyInfo.PropertyType.GetCompatibleGenericInterface(typeof(IDictionary<,>))!; + Type? underlyingIDictionaryType = jsonPropertyInfo.PropertyType.GetCompatibleGenericInterface(typeof(IDictionary<,>)) + ?? jsonPropertyInfo.PropertyType.GetCompatibleGenericInterface(typeof(IReadOnlyDictionary<,>)); + Debug.Assert(underlyingIDictionaryType is not null); Type[] genericArgs = underlyingIDictionaryType.GetGenericArguments(); Debug.Assert(underlyingIDictionaryType.IsGenericType); @@ -136,6 +138,22 @@ internal static void CreateExtensionDataProperty( { ThrowHelper.ThrowInvalidOperationException_NodeJsonObjectCustomConverterNotAllowedOnExtensionProperty(); } + // For IReadOnlyDictionary or IReadOnlyDictionary, + // create a Dictionary instance + else if (typeof(IReadOnlyDictionary).IsAssignableFrom(jsonPropertyInfo.PropertyType)) + { + extensionData = new Dictionary(); + Debug.Assert(jsonPropertyInfo.Set != null); + jsonPropertyInfo.Set(obj, extensionData); + return; + } + else if (typeof(IReadOnlyDictionary).IsAssignableFrom(jsonPropertyInfo.PropertyType)) + { + extensionData = new Dictionary(); + Debug.Assert(jsonPropertyInfo.Set != null); + jsonPropertyInfo.Set(obj, extensionData); + return; + } else { ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(jsonPropertyInfo.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 10e6db00dca22b..3e0e40b5e823ab 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 @@ -1326,6 +1326,11 @@ internal static bool IsValidExtensionDataProperty(Type propertyType) { return typeof(IDictionary).IsAssignableFrom(propertyType) || typeof(IDictionary).IsAssignableFrom(propertyType) || + // IReadOnlyDictionary is supported only if a Dictionary can be assigned to it (e.g., the interface itself) + (typeof(IReadOnlyDictionary).IsAssignableFrom(propertyType) && + propertyType.IsAssignableFrom(typeof(Dictionary))) || + (typeof(IReadOnlyDictionary).IsAssignableFrom(propertyType) && + propertyType.IsAssignableFrom(typeof(Dictionary))) || // Avoid a reference to typeof(JsonNode) to support trimming. (propertyType.FullName == JsonObjectTypeName && ReferenceEquals(propertyType.Assembly, typeof(JsonTypeInfo).Assembly)); } diff --git a/src/libraries/System.Text.Json/tests/Common/ExtensionDataTests.cs b/src/libraries/System.Text.Json/tests/Common/ExtensionDataTests.cs index 5a675f2851df04..ea28be6938735b 100644 --- a/src/libraries/System.Text.Json/tests/Common/ExtensionDataTests.cs +++ b/src/libraries/System.Text.Json/tests/Common/ExtensionDataTests.cs @@ -1483,5 +1483,74 @@ public class ClassWithEmptyPropertyNameAndExtensionProperty [JsonExtensionData] public IDictionary MyOverflow { get; set; } } + + [Fact] + public async Task IReadOnlyDictionary_ObjectExtensionPropertyRoundTrip() + { + string json = @"{""MyIntMissing"":2, ""MyInt"":1}"; + ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty obj = await Serializer.DeserializeWrapper(json); + + Assert.NotNull(obj.MyOverflow); + Assert.Equal(1, obj.MyInt); + Assert.IsType(obj.MyOverflow["MyIntMissing"]); + Assert.Equal(2, ((JsonElement)obj.MyOverflow["MyIntMissing"]).GetInt32()); + + string jsonSerialized = await Serializer.SerializeWrapper(obj); + Assert.Contains("\"MyIntMissing\"", jsonSerialized); + Assert.Contains("\"MyInt\"", jsonSerialized); + Assert.DoesNotContain(nameof(ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty.MyOverflow), jsonSerialized); + } + + [Fact] + public async Task IReadOnlyDictionary_JsonElementExtensionPropertyRoundTrip() + { + string json = @"{""MyIntMissing"":2, ""MyInt"":1}"; + ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty obj = await Serializer.DeserializeWrapper(json); + + Assert.NotNull(obj.MyOverflow); + Assert.Equal(1, obj.MyInt); + Assert.Equal(2, obj.MyOverflow["MyIntMissing"].GetInt32()); + + string jsonSerialized = await Serializer.SerializeWrapper(obj); + Assert.Contains("\"MyIntMissing\"", jsonSerialized); + Assert.Contains("\"MyInt\"", jsonSerialized); + Assert.DoesNotContain(nameof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty.MyOverflow), jsonSerialized); + } + + [Fact] + public async Task IReadOnlyDictionary_ExtensionPropertyIgnoredWhenWritingDefault() + { + string expected = @"{}"; + string actual = await Serializer.SerializeWrapper(new ClassWithIReadOnlyDictionaryExtensionPropertyAsObject()); + Assert.Equal(expected, actual); + } + + public class ClassWithIReadOnlyDictionaryExtensionPropertyAsObject + { + [JsonExtensionData] + public IReadOnlyDictionary MyOverflow { get; set; } + } + + public class ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElement + { + [JsonExtensionData] + public IReadOnlyDictionary MyOverflow { get; set; } + } + + public class ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty + { + public int MyInt { get; set; } + + [JsonExtensionData] + public IReadOnlyDictionary MyOverflow { get; set; } + } + + public class ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty + { + public int MyInt { get; set; } + + [JsonExtensionData] + public IReadOnlyDictionary MyOverflow { get; set; } + } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs index 4f263a7c882895..f3ac893e9e5c19 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs @@ -1285,6 +1285,8 @@ public static void ClassWithExtensionDataAttribute_RemovingExtensionDataProperty [Theory] [InlineData(typeof(IDictionary))] [InlineData(typeof(IDictionary))] + [InlineData(typeof(IReadOnlyDictionary))] + [InlineData(typeof(IReadOnlyDictionary))] [InlineData(typeof(Dictionary))] [InlineData(typeof(Dictionary))] [InlineData(typeof(ConcurrentDictionary))] From 9a2287e43e9bde5599c313e04fded4b1ea1f4087 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Oct 2025 18:34:20 +0000 Subject: [PATCH 3/7] Fix IReadOnlyDictionary validation to prevent ImmutableDictionary regression Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../JsonSerializer.Read.HandlePropertyName.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs index d018aef43c8e16..f442f58d5baba0 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs @@ -138,16 +138,19 @@ internal static void CreateExtensionDataProperty( { ThrowHelper.ThrowInvalidOperationException_NodeJsonObjectCustomConverterNotAllowedOnExtensionProperty(); } - // For IReadOnlyDictionary or IReadOnlyDictionary, - // create a Dictionary instance - else if (typeof(IReadOnlyDictionary).IsAssignableFrom(jsonPropertyInfo.PropertyType)) + // For IReadOnlyDictionary or IReadOnlyDictionary interface types, + // create a Dictionary instance. We only do this if Dictionary can be assigned back + // to the property (i.e., the property is the interface type itself, not a concrete implementation). + else if (typeof(IReadOnlyDictionary).IsAssignableFrom(jsonPropertyInfo.PropertyType) && + jsonPropertyInfo.PropertyType.IsAssignableFrom(typeof(Dictionary))) { extensionData = new Dictionary(); Debug.Assert(jsonPropertyInfo.Set != null); jsonPropertyInfo.Set(obj, extensionData); return; } - else if (typeof(IReadOnlyDictionary).IsAssignableFrom(jsonPropertyInfo.PropertyType)) + else if (typeof(IReadOnlyDictionary).IsAssignableFrom(jsonPropertyInfo.PropertyType) && + jsonPropertyInfo.PropertyType.IsAssignableFrom(typeof(Dictionary))) { extensionData = new Dictionary(); Debug.Assert(jsonPropertyInfo.Set != null); From c0c8cf6c256ca85762113b14f78b36226e12560b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Oct 2025 02:00:53 +0000 Subject: [PATCH 4/7] Fix source generator validation to support IReadOnlyDictionary extension data Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../gen/Helpers/KnownTypeSymbols.cs | 10 ++++++ .../gen/JsonSourceGenerator.Parser.cs | 35 ++++++++++++++++--- .../Serialization/ExtensionDataTests.cs | 8 +++++ 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Text.Json/gen/Helpers/KnownTypeSymbols.cs b/src/libraries/System.Text.Json/gen/Helpers/KnownTypeSymbols.cs index 0837c6f42f8871..395b2e598e3654 100644 --- a/src/libraries/System.Text.Json/gen/Helpers/KnownTypeSymbols.cs +++ b/src/libraries/System.Text.Json/gen/Helpers/KnownTypeSymbols.cs @@ -191,6 +191,16 @@ public KnownTypeSymbols(Compilation compilation) public INamedTypeSymbol? JsonElementType => GetOrResolveType("System.Text.Json.JsonElement", ref _JsonElementType); private Option _JsonElementType; + public INamedTypeSymbol? StringObjectDictionaryType => _StringObjectDictionaryType.HasValue + ? _StringObjectDictionaryType.Value + : (_StringObjectDictionaryType = new(DictionaryOfTKeyTValueType?.Construct(StringType, ObjectType))).Value; + private Option _StringObjectDictionaryType; + + public INamedTypeSymbol? StringJsonElementDictionaryType => _StringJsonElementDictionaryType.HasValue + ? _StringJsonElementDictionaryType.Value + : (_StringJsonElementDictionaryType = new(DictionaryOfTKeyTValueType?.Construct(StringType, JsonElementType))).Value; + private Option _StringJsonElementDictionaryType; + public INamedTypeSymbol? JsonNodeType => GetOrResolveType("System.Text.Json.Nodes.JsonNode", ref _JsonNodeType); private Option _JsonNodeType; diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs index fc64ef50b766f3..9d4d1eacd6d2db 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs @@ -1105,14 +1105,39 @@ private bool IsValidDataExtensionPropertyType(ITypeSymbol type) } INamedTypeSymbol? actualDictionaryType = type.GetCompatibleGenericBaseType(_knownSymbols.IDictionaryOfTKeyTValueType); - if (actualDictionaryType == null) + if (actualDictionaryType != null) { - return false; + if (SymbolEqualityComparer.Default.Equals(actualDictionaryType.TypeArguments[0], _knownSymbols.StringType) && + (SymbolEqualityComparer.Default.Equals(actualDictionaryType.TypeArguments[1], _knownSymbols.ObjectType) || + SymbolEqualityComparer.Default.Equals(actualDictionaryType.TypeArguments[1], _knownSymbols.JsonElementType))) + { + return true; + } } - return SymbolEqualityComparer.Default.Equals(actualDictionaryType.TypeArguments[0], _knownSymbols.StringType) && - (SymbolEqualityComparer.Default.Equals(actualDictionaryType.TypeArguments[1], _knownSymbols.ObjectType) || - SymbolEqualityComparer.Default.Equals(actualDictionaryType.TypeArguments[1], _knownSymbols.JsonElementType)); + // Also check for IReadOnlyDictionary or IReadOnlyDictionary + // but only if Dictionary can be assigned to it (to exclude ImmutableDictionary and similar types) + INamedTypeSymbol? actualReadOnlyDictionaryType = type.GetCompatibleGenericBaseType(_knownSymbols.IReadonlyDictionaryOfTKeyTValueType); + if (actualReadOnlyDictionaryType != null) + { + if (SymbolEqualityComparer.Default.Equals(actualReadOnlyDictionaryType.TypeArguments[0], _knownSymbols.StringType) && + (SymbolEqualityComparer.Default.Equals(actualReadOnlyDictionaryType.TypeArguments[1], _knownSymbols.ObjectType) || + SymbolEqualityComparer.Default.Equals(actualReadOnlyDictionaryType.TypeArguments[1], _knownSymbols.JsonElementType))) + { + // Check if Dictionary can be assigned to this type + INamedTypeSymbol? dictionaryType = SymbolEqualityComparer.Default.Equals(actualReadOnlyDictionaryType.TypeArguments[1], _knownSymbols.ObjectType) + ? _knownSymbols.StringObjectDictionaryType + : _knownSymbols.StringJsonElementDictionaryType; + + if (dictionaryType != null) + { + Conversion conversion = _knownSymbols.Compilation.ClassifyConversion(dictionaryType, type); + return conversion.IsImplicit || conversion.IsIdentity; + } + } + } + + return false; } private PropertyGenerationSpec? ParsePropertyGenerationSpec( diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ExtensionDataTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ExtensionDataTests.cs index ba67748368ff3b..a7d4e221e63367 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ExtensionDataTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ExtensionDataTests.cs @@ -68,6 +68,10 @@ public ExtensionDataTests_Metadata() [JsonSerializable(typeof(int))] [JsonSerializable(typeof(DummyObj))] [JsonSerializable(typeof(DummyStruct))] + [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsObject))] + [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElement))] + [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty))] + [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty))] internal sealed partial class ExtensionDataTestsContext_Metadata : JsonSerializerContext { } @@ -132,6 +136,10 @@ public ExtensionDataTests_Default() [JsonSerializable(typeof(int))] [JsonSerializable(typeof(DummyObj))] [JsonSerializable(typeof(DummyStruct))] + [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsObject))] + [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElement))] + [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty))] + [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty))] internal sealed partial class ExtensionDataTestsContext_Default : JsonSerializerContext { } From c98fa0d48975200e7520577eefda76ef10c33b1c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Oct 2025 12:16:43 +0000 Subject: [PATCH 5/7] Fix null reference warning in KnownTypeSymbols for JsonElementType Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../System.Text.Json/gen/Helpers/KnownTypeSymbols.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/gen/Helpers/KnownTypeSymbols.cs b/src/libraries/System.Text.Json/gen/Helpers/KnownTypeSymbols.cs index 395b2e598e3654..3827f707ee4763 100644 --- a/src/libraries/System.Text.Json/gen/Helpers/KnownTypeSymbols.cs +++ b/src/libraries/System.Text.Json/gen/Helpers/KnownTypeSymbols.cs @@ -198,7 +198,9 @@ public KnownTypeSymbols(Compilation compilation) public INamedTypeSymbol? StringJsonElementDictionaryType => _StringJsonElementDictionaryType.HasValue ? _StringJsonElementDictionaryType.Value - : (_StringJsonElementDictionaryType = new(DictionaryOfTKeyTValueType?.Construct(StringType, JsonElementType))).Value; + : (_StringJsonElementDictionaryType = new(DictionaryOfTKeyTValueType is { } dictType && JsonElementType is { } jsonElemType + ? dictType.Construct(StringType, jsonElemType) + : null)).Value; private Option _StringJsonElementDictionaryType; public INamedTypeSymbol? JsonNodeType => GetOrResolveType("System.Text.Json.Nodes.JsonNode", ref _JsonNodeType); From 4e5e8e2966f0b199e0fcbd1d2fef035a1ef60bcc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Oct 2025 15:34:51 +0000 Subject: [PATCH 6/7] Simplify IReadOnlyDictionary validation logic Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../Text/Json/Serialization/Metadata/JsonTypeInfo.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) 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 3e0e40b5e823ab..5456030b9bcaea 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 @@ -1326,11 +1326,8 @@ internal static bool IsValidExtensionDataProperty(Type propertyType) { return typeof(IDictionary).IsAssignableFrom(propertyType) || typeof(IDictionary).IsAssignableFrom(propertyType) || - // IReadOnlyDictionary is supported only if a Dictionary can be assigned to it (e.g., the interface itself) - (typeof(IReadOnlyDictionary).IsAssignableFrom(propertyType) && - propertyType.IsAssignableFrom(typeof(Dictionary))) || - (typeof(IReadOnlyDictionary).IsAssignableFrom(propertyType) && - propertyType.IsAssignableFrom(typeof(Dictionary))) || + propertyType == typeof(IReadOnlyDictionary) || + propertyType == typeof(IReadOnlyDictionary) || // Avoid a reference to typeof(JsonNode) to support trimming. (propertyType.FullName == JsonObjectTypeName && ReferenceEquals(propertyType.Assembly, typeof(JsonTypeInfo).Assembly)); } From 7cde7a7cd9c4921a1ee872aa4de7dd800df44109 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Oct 2025 16:16:55 +0000 Subject: [PATCH 7/7] Support pre-populated IReadOnlyDictionary extension data properties Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../JsonSerializer.Read.HandlePropertyName.cs | 47 ++++++++++--- .../tests/Common/ExtensionDataTests.cs | 68 +++++++++++++++++++ .../Serialization/ExtensionDataTests.cs | 4 ++ 3 files changed, 110 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs index f442f58d5baba0..5daf1c73d346c8 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs @@ -110,7 +110,13 @@ internal static void CreateExtensionDataProperty( Debug.Assert(jsonPropertyInfo != null); object? extensionData = jsonPropertyInfo.GetValueAsObject(obj); - if (extensionData == null) + + // For IReadOnlyDictionary, if there's an existing non-null instance, we need to create a new mutable + // Dictionary seeded with the existing contents so we can add the deserialized extension data to it. + bool isReadOnlyDictionary = jsonPropertyInfo.PropertyType == typeof(IReadOnlyDictionary) || + jsonPropertyInfo.PropertyType == typeof(IReadOnlyDictionary); + + if (extensionData == null || (isReadOnlyDictionary && extensionData != null)) { // Create the appropriate dictionary type. We already verified the types. #if DEBUG @@ -139,20 +145,43 @@ internal static void CreateExtensionDataProperty( ThrowHelper.ThrowInvalidOperationException_NodeJsonObjectCustomConverterNotAllowedOnExtensionProperty(); } // For IReadOnlyDictionary or IReadOnlyDictionary interface types, - // create a Dictionary instance. We only do this if Dictionary can be assigned back - // to the property (i.e., the property is the interface type itself, not a concrete implementation). - else if (typeof(IReadOnlyDictionary).IsAssignableFrom(jsonPropertyInfo.PropertyType) && - jsonPropertyInfo.PropertyType.IsAssignableFrom(typeof(Dictionary))) + // create a Dictionary instance seeded with any existing contents. + else if (jsonPropertyInfo.PropertyType == typeof(IReadOnlyDictionary)) { - extensionData = new Dictionary(); + if (extensionData != null) + { + var existing = (IReadOnlyDictionary)extensionData; + var newDict = new Dictionary(); + foreach (KeyValuePair kvp in existing) + { + newDict[kvp.Key] = kvp.Value; + } + extensionData = newDict; + } + else + { + extensionData = new Dictionary(); + } Debug.Assert(jsonPropertyInfo.Set != null); jsonPropertyInfo.Set(obj, extensionData); return; } - else if (typeof(IReadOnlyDictionary).IsAssignableFrom(jsonPropertyInfo.PropertyType) && - jsonPropertyInfo.PropertyType.IsAssignableFrom(typeof(Dictionary))) + else if (jsonPropertyInfo.PropertyType == typeof(IReadOnlyDictionary)) { - extensionData = new Dictionary(); + if (extensionData != null) + { + var existing = (IReadOnlyDictionary)extensionData; + var newDict = new Dictionary(); + foreach (KeyValuePair kvp in existing) + { + newDict[kvp.Key] = kvp.Value; + } + extensionData = newDict; + } + else + { + extensionData = new Dictionary(); + } Debug.Assert(jsonPropertyInfo.Set != null); jsonPropertyInfo.Set(obj, extensionData); return; diff --git a/src/libraries/System.Text.Json/tests/Common/ExtensionDataTests.cs b/src/libraries/System.Text.Json/tests/Common/ExtensionDataTests.cs index ea28be6938735b..570624a2a229df 100644 --- a/src/libraries/System.Text.Json/tests/Common/ExtensionDataTests.cs +++ b/src/libraries/System.Text.Json/tests/Common/ExtensionDataTests.cs @@ -1525,6 +1525,50 @@ public async Task IReadOnlyDictionary_ExtensionPropertyIgnoredWhenWritingDefault Assert.Equal(expected, actual); } + [Fact] + public async Task IReadOnlyDictionary_PrePopulated_SeedsNewInstance() + { + string json = @"{""MyIntMissing"":2, ""KeyToOverwrite"":""NewValue"", ""MyInt"":1}"; + var obj = await Serializer.DeserializeWrapper(json); + + Assert.NotNull(obj.MyOverflow); + Assert.Equal(1, obj.MyInt); + + // Should have the existing key from the initializer + Assert.True(obj.MyOverflow.ContainsKey("ExistingKey")); + Assert.Equal("ExistingValue", ((JsonElement)obj.MyOverflow["ExistingKey"]).GetString()); + + // Should have the new key from deserialization + Assert.True(obj.MyOverflow.ContainsKey("MyIntMissing")); + Assert.Equal(2, ((JsonElement)obj.MyOverflow["MyIntMissing"]).GetInt32()); + + // Existing key should be overwritten with new value from deserialization + Assert.True(obj.MyOverflow.ContainsKey("KeyToOverwrite")); + Assert.Equal("NewValue", ((JsonElement)obj.MyOverflow["KeyToOverwrite"]).GetString()); + } + + [Fact] + public async Task IReadOnlyDictionary_PrePopulated_JsonElement_SeedsNewInstance() + { + string json = @"{""MyIntMissing"":2, ""KeyToOverwrite"":""NewValue"", ""MyInt"":1}"; + var obj = await Serializer.DeserializeWrapper(json); + + Assert.NotNull(obj.MyOverflow); + Assert.Equal(1, obj.MyInt); + + // Should have the existing key from the initializer + Assert.True(obj.MyOverflow.ContainsKey("ExistingKey")); + Assert.Equal("ExistingValue", obj.MyOverflow["ExistingKey"].GetString()); + + // Should have the new key from deserialization + Assert.True(obj.MyOverflow.ContainsKey("MyIntMissing")); + Assert.Equal(2, obj.MyOverflow["MyIntMissing"].GetInt32()); + + // Existing key should be overwritten with new value from deserialization + Assert.True(obj.MyOverflow.ContainsKey("KeyToOverwrite")); + Assert.Equal("NewValue", obj.MyOverflow["KeyToOverwrite"].GetString()); + } + public class ClassWithIReadOnlyDictionaryExtensionPropertyAsObject { [JsonExtensionData] @@ -1552,5 +1596,29 @@ public class ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithPrope [JsonExtensionData] public IReadOnlyDictionary MyOverflow { get; set; } } + + public class ClassWithIReadOnlyDictionaryAlreadyInstantiated + { + public int MyInt { get; set; } + + [JsonExtensionData] + public IReadOnlyDictionary MyOverflow { get; set; } = new Dictionary + { + ["ExistingKey"] = JsonDocument.Parse("\"ExistingValue\"").RootElement, + ["KeyToOverwrite"] = JsonDocument.Parse("\"OldValue\"").RootElement + }; + } + + public class ClassWithIReadOnlyDictionaryJsonElementAlreadyInstantiated + { + public int MyInt { get; set; } + + [JsonExtensionData] + public IReadOnlyDictionary MyOverflow { get; set; } = new Dictionary + { + ["ExistingKey"] = JsonDocument.Parse("\"ExistingValue\"").RootElement, + ["KeyToOverwrite"] = JsonDocument.Parse("\"OldValue\"").RootElement + }; + } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ExtensionDataTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ExtensionDataTests.cs index a7d4e221e63367..89829b304bf4b2 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ExtensionDataTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ExtensionDataTests.cs @@ -72,6 +72,8 @@ public ExtensionDataTests_Metadata() [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElement))] [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty))] [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty))] + [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryAlreadyInstantiated))] + [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryJsonElementAlreadyInstantiated))] internal sealed partial class ExtensionDataTestsContext_Metadata : JsonSerializerContext { } @@ -140,6 +142,8 @@ public ExtensionDataTests_Default() [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElement))] [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty))] [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty))] + [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryAlreadyInstantiated))] + [JsonSerializable(typeof(ClassWithIReadOnlyDictionaryJsonElementAlreadyInstantiated))] internal sealed partial class ExtensionDataTestsContext_Default : JsonSerializerContext { }