From 15be294a494f58edf51c45391a65ac47bf2ef317 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Mon, 26 Oct 2020 10:22:14 -0700 Subject: [PATCH 1/2] Set default value for value-type ctor params properly --- .../Serialization/JsonParameterInfoOfT.cs | 14 ++++- .../Json/Serialization/JsonPropertyInfo.cs | 3 ++ .../Json/Serialization/JsonPropertyInfoOfT.cs | 19 ++++--- .../ConstructorTests.ParameterMatching.cs | 51 +++++++++++++++++++ 4 files changed, 75 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonParameterInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonParameterInfoOfT.cs index 3ff8c169dede5..cca6dfaedb6b2 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonParameterInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonParameterInfoOfT.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.Diagnostics; using System.Reflection; namespace System.Text.Json @@ -25,10 +26,19 @@ public override void Initialize( matchingProperty, options); + Debug.Assert(parameterInfo.ParameterType == matchingProperty.DeclaredPropertyType); + if (parameterInfo.HasDefaultValue) { - DefaultValue = parameterInfo.DefaultValue; - TypedDefaultValue = (T)parameterInfo.DefaultValue!; + if (parameterInfo.DefaultValue == null && !matchingProperty.PropertyTypeCanBeNull) + { + DefaultValue = TypedDefaultValue; + } + else + { + DefaultValue = parameterInfo.DefaultValue; + TypedDefaultValue = (T)parameterInfo.DefaultValue!; + } } else { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs index 4d7d9f40f2c81..21c501252aa6b 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs @@ -435,5 +435,8 @@ public JsonClassInfo RuntimeClassInfo public bool IsIgnored { get; private set; } public JsonNumberHandling? NumberHandling { get; private set; } + + // Whether the property type can be null. + public bool PropertyTypeCanBeNull { get; protected set; } } } 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 114858d4e32ba..992f28b4bcfea 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 @@ -26,7 +26,6 @@ internal sealed class JsonPropertyInfo : JsonPropertyInfo // Since a converter's TypeToConvert (which is the T value in this type) can be different than // the property's type, we track that and whether the property type can be null. private bool _propertyTypeEqualsTypeToConvert; - private bool _propertyTypeCanBeNull; public Func? Get { get; private set; } public Action? Set { get; private set; } @@ -105,10 +104,10 @@ public override void Initialize( } _converterIsExternalAndPolymorphic = !converter.IsInternalConverter && DeclaredPropertyType != converter.TypeToConvert; - _propertyTypeCanBeNull = DeclaredPropertyType.CanBeNull(); + PropertyTypeCanBeNull = DeclaredPropertyType.CanBeNull(); _propertyTypeEqualsTypeToConvert = typeof(T) == DeclaredPropertyType; - GetPolicies(ignoreCondition, parentTypeNumberHandling, defaultValueIsNull: _propertyTypeCanBeNull); + GetPolicies(ignoreCondition, parentTypeNumberHandling, defaultValueIsNull: PropertyTypeCanBeNull); } public override JsonConverter ConverterBase @@ -147,7 +146,7 @@ public override bool GetMemberAndWriteJson(object obj, ref WriteStack state, Utf return true; } - if (!_propertyTypeCanBeNull) + if (!PropertyTypeCanBeNull) { if (_propertyTypeEqualsTypeToConvert) { @@ -172,7 +171,7 @@ public override bool GetMemberAndWriteJson(object obj, ref WriteStack state, Utf if (value == null) { - Debug.Assert(_propertyTypeCanBeNull); + Debug.Assert(PropertyTypeCanBeNull); if (Converter.HandleNullOnWrite) { @@ -235,7 +234,7 @@ public override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref U bool isNullToken = reader.TokenType == JsonTokenType.Null; if (isNullToken && !Converter.HandleNullOnRead && !state.IsContinuation) { - if (!_propertyTypeCanBeNull) + if (!PropertyTypeCanBeNull) { ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(Converter.TypeToConvert); } @@ -255,7 +254,7 @@ public override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref U // CanUseDirectReadOrWrite == false when using streams Debug.Assert(!state.IsContinuation); - if (!isNullToken || !IgnoreDefaultValuesOnRead || !_propertyTypeCanBeNull) + if (!isNullToken || !IgnoreDefaultValuesOnRead || !PropertyTypeCanBeNull) { // Optimize for internal converters by avoiding the extra call to TryRead. T fastValue = Converter.Read(ref reader, RuntimePropertyType!, Options); @@ -267,7 +266,7 @@ public override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref U else { success = true; - if (!isNullToken || !IgnoreDefaultValuesOnRead || !_propertyTypeCanBeNull || state.IsContinuation) + if (!isNullToken || !IgnoreDefaultValuesOnRead || !PropertyTypeCanBeNull || state.IsContinuation) { success = Converter.TryRead(ref reader, RuntimePropertyType!, Options, ref state, out T value); if (success) @@ -284,7 +283,7 @@ public override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref U ThrowHelper.ThrowInvalidCastException_DeserializeUnableToAssignValue(typeOfValue, DeclaredPropertyType); } } - else if (!_propertyTypeCanBeNull) + else if (!PropertyTypeCanBeNull) { ThrowHelper.ThrowInvalidOperationException_DeserializeUnableToAssignNull(DeclaredPropertyType); } @@ -304,7 +303,7 @@ public override bool ReadJsonAsObject(ref ReadStack state, ref Utf8JsonReader re bool isNullToken = reader.TokenType == JsonTokenType.Null; if (isNullToken && !Converter.HandleNullOnRead && !state.IsContinuation) { - if (!_propertyTypeCanBeNull) + if (!PropertyTypeCanBeNull) { ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(Converter.TypeToConvert); } diff --git a/src/libraries/System.Text.Json/tests/Serialization/ConstructorTests/ConstructorTests.ParameterMatching.cs b/src/libraries/System.Text.Json/tests/Serialization/ConstructorTests/ConstructorTests.ParameterMatching.cs index 4b28b5771201e..577fdc4e29255 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/ConstructorTests/ConstructorTests.ParameterMatching.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/ConstructorTests/ConstructorTests.ParameterMatching.cs @@ -1139,5 +1139,56 @@ public void PocoWithSamePropertyNameDifferentTypes() AgePoco obj = JsonSerializer.Deserialize(@"{""age"":1}"); Assert.Equal(1, obj.age); } + + [Theory] + [InlineData(typeof(ClassWithGuid))] + [InlineData(typeof(ClassWithNullableGuid))] + public void DefaultForValueTypeCtorParam(Type type) + { + string json = @"{""MyGuid"":""edc421bf-782a-4a95-ad67-3d73b5d7db6f""}"; + object obj = JsonSerializer.Deserialize(json, type); + Assert.Equal(json, JsonSerializer.Serialize(obj)); + + json = @"{}"; + obj = JsonSerializer.Deserialize(json, type); + + var options = new JsonSerializerOptions { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault }; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + } + + private class ClassWithGuid + { + public Guid MyGuid { get; } + + public ClassWithGuid(Guid myGuid = default) => MyGuid = myGuid; + } + + private class ClassWithNullableGuid + { + public Guid? MyGuid { get; } + + public ClassWithNullableGuid(Guid? myGuid = default) => MyGuid = myGuid; + } + + [Fact] + public void DefaultForReferenceTypeCtorParam() + { + string json = @"{""MyUri"":""http://hello""}"; + object obj = JsonSerializer.Deserialize(json, typeof(ClassWithUri)); + Assert.Equal(json, JsonSerializer.Serialize(obj)); + + json = @"{}"; + obj = JsonSerializer.Deserialize(json, typeof(ClassWithUri)); + + var options = new JsonSerializerOptions { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault }; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + } + + private class ClassWithUri + { + public Uri MyUri { get; } + + public ClassWithUri(Uri myUri = default) => MyUri = myUri; + } } } From 9f23df88f0e81d5b642646c56b2a0f3566a8d2c8 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Thu, 29 Oct 2020 17:03:06 -0700 Subject: [PATCH 2/2] Address review feedback --- .../ConstructorTests.ParameterMatching.cs | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Text.Json/tests/Serialization/ConstructorTests/ConstructorTests.ParameterMatching.cs b/src/libraries/System.Text.Json/tests/Serialization/ConstructorTests/ConstructorTests.ParameterMatching.cs index 577fdc4e29255..ac6718a447eec 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/ConstructorTests/ConstructorTests.ParameterMatching.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/ConstructorTests/ConstructorTests.ParameterMatching.cs @@ -1141,8 +1141,8 @@ public void PocoWithSamePropertyNameDifferentTypes() } [Theory] - [InlineData(typeof(ClassWithGuid))] - [InlineData(typeof(ClassWithNullableGuid))] + [InlineData(typeof(TypeWithGuid))] + [InlineData(typeof(TypeWithNullableGuid))] public void DefaultForValueTypeCtorParam(Type type) { string json = @"{""MyGuid"":""edc421bf-782a-4a95-ad67-3d73b5d7db6f""}"; @@ -1156,39 +1156,40 @@ public void DefaultForValueTypeCtorParam(Type type) Assert.Equal(json, JsonSerializer.Serialize(obj, options)); } - private class ClassWithGuid + private class TypeWithGuid { public Guid MyGuid { get; } - public ClassWithGuid(Guid myGuid = default) => MyGuid = myGuid; + public TypeWithGuid(Guid myGuid = default) => MyGuid = myGuid; } - private class ClassWithNullableGuid + private struct TypeWithNullableGuid { public Guid? MyGuid { get; } - public ClassWithNullableGuid(Guid? myGuid = default) => MyGuid = myGuid; + [JsonConstructor] + public TypeWithNullableGuid(Guid? myGuid = default) => MyGuid = myGuid; } [Fact] public void DefaultForReferenceTypeCtorParam() { string json = @"{""MyUri"":""http://hello""}"; - object obj = JsonSerializer.Deserialize(json, typeof(ClassWithUri)); + object obj = JsonSerializer.Deserialize(json, typeof(TypeWithUri)); Assert.Equal(json, JsonSerializer.Serialize(obj)); json = @"{}"; - obj = JsonSerializer.Deserialize(json, typeof(ClassWithUri)); + obj = JsonSerializer.Deserialize(json, typeof(TypeWithUri)); var options = new JsonSerializerOptions { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault }; Assert.Equal(json, JsonSerializer.Serialize(obj, options)); } - private class ClassWithUri + private class TypeWithUri { public Uri MyUri { get; } - public ClassWithUri(Uri myUri = default) => MyUri = myUri; + public TypeWithUri(Uri myUri = default) => MyUri = myUri; } } }