From 7cfcfb0a2e7104d20ec54dbe0fc2f9eb21d86b9c Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Wed, 26 Aug 2020 22:43:58 -0700 Subject: [PATCH 1/2] Fixup JSON equality checks in polymorphic converter tests --- .../System.Text.Json/tests/AssertHelper.cs | 20 ---------- .../CustomConverterTests.Object.cs | 39 +++++++++---------- .../CustomConverterTests.ValueTypedMember.cs | 13 +++---- .../TestClasses.ValueTypedMember.cs | 3 -- .../tests/System.Text.Json.Tests.csproj | 1 - 5 files changed, 24 insertions(+), 52 deletions(-) delete mode 100644 src/libraries/System.Text.Json/tests/AssertHelper.cs diff --git a/src/libraries/System.Text.Json/tests/AssertHelper.cs b/src/libraries/System.Text.Json/tests/AssertHelper.cs deleted file mode 100644 index 0821027726a8aa..00000000000000 --- a/src/libraries/System.Text.Json/tests/AssertHelper.cs +++ /dev/null @@ -1,20 +0,0 @@ -// 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.Generic; -using Xunit; - -namespace System.Text.Json.Tests -{ - public static class AssertHelper - { - public static void ValidateJson(IEnumerable expectedProperties, string json) - { - Assert.StartsWith("{", json); - Assert.EndsWith("}", json); - foreach (string expectedProperty in expectedProperties) - Assert.Contains(expectedProperty, json); - } - - } -} diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests/CustomConverterTests.Object.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests/CustomConverterTests.Object.cs index 1ecdfebc935ada..0ab7ad020de5d2 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests/CustomConverterTests.Object.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests/CustomConverterTests.Object.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Text.Json.Tests; using Xunit; namespace System.Text.Json.Serialization.Tests @@ -396,15 +395,14 @@ private class ClassWithPrimitives [Fact] public static void ClassWithPrimitivesObjectConverter() { - string[] expected = new[] - { - @"""MyIntProperty"":123", - @"""MyBoolProperty"":true", - @"""MyStringProperty"":""Hello""", - @"""MyIntField"":321", - @"""MyBoolField"":true", - @"""MyStringField"":""World""" - }; + string expected = @"{ +""MyIntProperty"":123, +""MyBoolProperty"":true, +""MyStringProperty"":""Hello"", +""MyIntField"":321, +""MyBoolField"":true, +""MyStringField"":""World"" +}"; string json; var converter = new PrimitiveConverter(); @@ -428,7 +426,7 @@ public static void ClassWithPrimitivesObjectConverter() json = JsonSerializer.Serialize(obj, options); Assert.Equal(6, converter.WriteCallCount); - AssertHelper.ValidateJson(expected, json); + JsonTestHelper.AssertJsonEqual(expected, json); } { var obj = JsonSerializer.Deserialize(json, options); @@ -459,15 +457,14 @@ private class ClassWithNullablePrimitives [Fact] public static void ClassWithNullablePrimitivesObjectConverter() { - string[] expected = new[] - { - @"""MyIntProperty"":123", - @"""MyBoolProperty"":true", - @"""MyStringProperty"":""Hello""", - @"""MyIntField"":321", - @"""MyBoolField"":true", - @"""MyStringField"":""World""" - }; + string expected = @"{ +""MyIntProperty"":123, +""MyBoolProperty"":true, +""MyStringProperty"":""Hello"", +""MyIntField"":321, +""MyBoolField"":true, +""MyStringField"":""World"" +}"; string json; var converter = new PrimitiveConverter(); @@ -491,7 +488,7 @@ public static void ClassWithNullablePrimitivesObjectConverter() json = JsonSerializer.Serialize(obj, options); Assert.Equal(6, converter.WriteCallCount); - AssertHelper.ValidateJson(expected, json); + JsonTestHelper.AssertJsonEqual(expected, json); } { var obj = JsonSerializer.Deserialize(json, options); diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests/CustomConverterTests.ValueTypedMember.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests/CustomConverterTests.ValueTypedMember.cs index a4fa41de979f79..9501b00a04a870 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests/CustomConverterTests.ValueTypedMember.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests/CustomConverterTests.ValueTypedMember.cs @@ -208,7 +208,7 @@ public static void ValueTypedMemberToInterfaceConverter() json = JsonSerializer.Serialize(obj, options); Assert.Equal(4, converter.WriteCallCount); - Assert.Equal(expected, json); + JsonTestHelper.AssertJsonEqual(expected, json); } { @@ -240,7 +240,7 @@ public static void ValueTypedMemberToObjectConverter() json = JsonSerializer.Serialize(obj, options); Assert.Equal(4, converter.WriteCallCount); - Assert.Equal(expected, json); + JsonTestHelper.AssertJsonEqual(expected, json); } { @@ -272,7 +272,7 @@ public static void NullableValueTypedMemberToInterfaceConverter() json = JsonSerializer.Serialize(obj, options); Assert.Equal(4, converter.WriteCallCount); - Assert.Equal(expected, json); + JsonTestHelper.AssertJsonEqual(expected, json); } { @@ -304,7 +304,7 @@ public static void NullableValueTypedMemberToObjectConverter() json = JsonSerializer.Serialize(obj, options); Assert.Equal(4, converter.WriteCallCount); - Assert.Equal(expected, json); + JsonTestHelper.AssertJsonEqual(expected, json); } { @@ -334,7 +334,7 @@ public static void NullableValueTypedMemberWithNullsToInterfaceConverter() json = JsonSerializer.Serialize(obj, options); Assert.Equal(4, converter.WriteCallCount); - Assert.Equal(expected, json); + JsonTestHelper.AssertJsonEqual(expected, json); } { @@ -367,7 +367,7 @@ public static void NullableValueTypedMemberWithNullsToObjectConverter() json = JsonSerializer.Serialize(obj, options); Assert.Equal(4, converter.WriteCallCount); - Assert.Equal(expected, json); + JsonTestHelper.AssertJsonEqual(expected, json); } { @@ -380,6 +380,5 @@ public static void NullableValueTypedMemberWithNullsToObjectConverter() Assert.Null(obj.MyRefTypedField); } } - } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/TestClasses/TestClasses.ValueTypedMember.cs b/src/libraries/System.Text.Json/tests/Serialization/TestClasses/TestClasses.ValueTypedMember.cs index ab7384afce90e1..31e51649cf3a03 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/TestClasses/TestClasses.ValueTypedMember.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/TestClasses/TestClasses.ValueTypedMember.cs @@ -1,8 +1,6 @@ // 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.Generic; -using System.Linq; using Xunit; namespace System.Text.Json.Serialization.Tests @@ -105,5 +103,4 @@ public OtherRTMember(string value) Value = value; } } - } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj index adb637c9244b8f..2f8a4ffcbae617 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj @@ -9,7 +9,6 @@ - From 3141b60493b30dd5b4d956a32d7e2638f2d9d82a Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Thu, 27 Aug 2020 11:34:07 -0700 Subject: [PATCH 2/2] Limit deserialization value validation to custom _polymorphic_ converters --- .../Text/Json/Serialization/JsonPropertyInfoOfT.cs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfT.cs index ed3da3ac632967..48f952a8a19a78 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfT.cs @@ -15,6 +15,14 @@ namespace System.Text.Json /// or a type's converter, if the current instance is a . internal sealed class JsonPropertyInfo : JsonPropertyInfo { + /// + /// Returns true if the property's converter is external (a user's custom converter) + /// and the type to convert is not the same as the declared property type (polymorphic). + /// Used to determine whether to perform additional validation on the value returned by the + /// converter on deserialization. + /// + private bool _converterIsExternalAndPolymorphic; + public Func? Get { get; private set; } public Action? Set { get; private set; } @@ -91,6 +99,7 @@ public override void Initialize( } } + _converterIsExternalAndPolymorphic = !converter.IsInternalConverter && DeclaredPropertyType != converter.TypeToConvert; GetPolicies(ignoreCondition, parentTypeNumberHandling, defaultValueIsNull: Converter.CanBeNull); } @@ -230,7 +239,9 @@ public override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref U success = Converter.TryRead(ref reader, RuntimePropertyType!, Options, ref state, out T value); if (success) { - if (!Converter.IsInternalConverter) +#if !DEBUG + if (_converterIsExternalAndPolymorphic) +#endif { if (value != null) { @@ -245,6 +256,7 @@ public override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref U ThrowHelper.ThrowInvalidOperationException_DeserializeUnableToAssignNull(DeclaredPropertyType); } } + Set!(obj, value!); } }