diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs index 12202e395b67ac..184492ae5c4c6e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs @@ -97,52 +97,58 @@ public JsonClassInfo(Type type, JsonSerializerOptions options) case ClassType.Object: { CreateObject = options.MemberAccessorStrategy.CreateConstructor(type); + Dictionary cache = new Dictionary( + Options.PropertyNameCaseInsensitive + ? StringComparer.OrdinalIgnoreCase + : StringComparer.Ordinal); - PropertyInfo[] properties = type.GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); - - Dictionary cache = CreatePropertyCache(properties.Length); - - foreach (PropertyInfo propertyInfo in properties) + for (Type? currentType = type; currentType != null; currentType = currentType.BaseType) { - // Ignore indexers - if (propertyInfo.GetIndexParameters().Length > 0) + foreach (PropertyInfo propertyInfo in currentType.GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly)) { - continue; - } - - if (IsNonPublicProperty(propertyInfo)) - { - if (JsonPropertyInfo.GetAttribute(propertyInfo) != null) + // Ignore indexers + if (propertyInfo.GetIndexParameters().Length > 0) { - ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(propertyInfo, Type); + continue; } - // Non-public properties should not be included for (de)serialization. - continue; - } + if (IsNonPublicProperty(propertyInfo)) + { + if (JsonPropertyInfo.GetAttribute(propertyInfo) != null) + { + ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(propertyInfo, currentType); + } - // For now we only support public getters\setters - if (propertyInfo.GetMethod?.IsPublic == true || - propertyInfo.SetMethod?.IsPublic == true) - { - JsonPropertyInfo jsonPropertyInfo = AddProperty(propertyInfo, type, options); - Debug.Assert(jsonPropertyInfo != null && jsonPropertyInfo.NameAsString != null); + // Non-public properties should not be included for (de)serialization. + continue; + } - // If the JsonPropertyNameAttribute or naming policy results in collisions, throw an exception. - if (!JsonHelpers.TryAdd(cache, jsonPropertyInfo.NameAsString, jsonPropertyInfo)) + // For now we only support public getters\setters + if (propertyInfo.GetMethod?.IsPublic == true || + propertyInfo.SetMethod?.IsPublic == true) { - JsonPropertyInfo other = cache[jsonPropertyInfo.NameAsString]; + JsonPropertyInfo jsonPropertyInfo = AddProperty(propertyInfo, currentType, options); + Debug.Assert(jsonPropertyInfo != null && jsonPropertyInfo.NameAsString != null); - if (other.ShouldDeserialize == false && other.ShouldSerialize == false) - { - // Overwrite the one just added since it has [JsonIgnore]. - cache[jsonPropertyInfo.NameAsString] = jsonPropertyInfo; - } - else if (jsonPropertyInfo.ShouldDeserialize == true || jsonPropertyInfo.ShouldSerialize == true) + // If the JsonPropertyNameAttribute or naming policy results in collisions, throw an exception. + if (!JsonHelpers.TryAdd(cache, jsonPropertyInfo.NameAsString, jsonPropertyInfo)) { - ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameConflict(Type, jsonPropertyInfo); + JsonPropertyInfo other = cache[jsonPropertyInfo.NameAsString]; + + if (other.ShouldDeserialize == false && other.ShouldSerialize == false) + { + // Overwrite the one just added since it has [JsonIgnore]. + cache[jsonPropertyInfo.NameAsString] = jsonPropertyInfo; + } + else if (other.PropertyInfo?.Name != jsonPropertyInfo.PropertyInfo?.Name && + (jsonPropertyInfo.ShouldDeserialize == true || jsonPropertyInfo.ShouldSerialize == true)) + { + // Check for name equality is required to determine when a new slot is used for the member. + // Therefore, if names are not the same, there is conflict due to the name policy or attributes. + ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameConflict(Type, jsonPropertyInfo); + } + // else ignore jsonPropertyInfo since it has [JsonIgnore] or it's hidden by a new slot. } - // else ignore jsonPropertyInfo since it has [JsonIgnore]. } } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs b/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs index 9aa9cd6bb8ba7c..2cc311764cb8e1 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs @@ -11,6 +11,469 @@ namespace System.Text.Json.Serialization.Tests { public static partial class PropertyVisibilityTests { + [Fact] + public static void Serialize_NewSlotPublicProperty() + { + // Serialize + var obj = new ClassWithNewSlotProperty(); + string json = JsonSerializer.Serialize(obj); + + Assert.Equal(@"{""MyString"":""NewDefaultValue""}", json); + + // Deserialize + json = @"{""MyString"":""NewValue""}"; + obj = JsonSerializer.Deserialize(json); + + Assert.Equal("NewValue", ((ClassWithNewSlotProperty)obj).MyString); + Assert.Equal("DefaultValue", ((ClassWithInternalProperty)obj).MyString); + } + + [Fact] + public static void Serialize_BasePublicProperty_ConflictWithDerivedPrivate() + { + // Serialize + var obj = new ClassWithNewSlotInternalProperty(); + string json = JsonSerializer.Serialize(obj); + + Assert.Equal(@"{""MyString"":""DefaultValue""}", json); + + // Deserialize + json = @"{""MyString"":""NewValue""}"; + obj = JsonSerializer.Deserialize(json); + + Assert.Equal("NewValue", ((ClassWithPublicProperty)obj).MyString); + Assert.Equal("NewDefaultValue", ((ClassWithNewSlotInternalProperty)obj).MyString); + } + + [Fact] + public static void Serialize_PublicProperty_ConflictWithPrivateDueAttributes() + { + // Serialize + var obj = new ClassWithPropertyNamingConflict(); + string json = JsonSerializer.Serialize(obj); + + Assert.Equal(@"{""MyString"":""DefaultValue""}", json); + + // Deserialize + json = @"{""MyString"":""NewValue""}"; + obj = JsonSerializer.Deserialize(json); + + Assert.Equal("NewValue", obj.MyString); + Assert.Equal("ConflictingValue", obj.ConflictingString); + } + + [Fact] + public static void Serialize_PublicProperty_ConflictWithPrivateDuePolicy() + { + var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; + + // Serialize + var obj = new ClassWithPropertyPolicyConflict(); + string json = JsonSerializer.Serialize(obj, options); + + Assert.Equal(@"{""myString"":""DefaultValue""}", json); + + // Deserialize + json = @"{""myString"":""NewValue""}"; + obj = JsonSerializer.Deserialize(json, options); + + Assert.Equal("NewValue", obj.MyString); + Assert.Equal("ConflictingValue", obj.myString); + } + + [Fact] + public static void Serealize_NewSlotPublicProperty_ConflictWithBasePublicProperty() + { + // Serialize + var obj = new ClassWithNewSlotDecimalProperty(); + string json = JsonSerializer.Serialize(obj); + + Assert.Equal(@"{""MyNumeric"":1.5}", json); + + // Deserialize + json = @"{""MyNumeric"":2.5}"; + obj = JsonSerializer.Deserialize(json); + + Assert.Equal(2.5M, obj.MyNumeric); + } + + [Fact] + public static void Serealize_NewSlotPublicProperty_SpecifiedJsonPropertyName() + { + // Serialize + var obj = new ClassWithNewSlotAttributedDecimalProperty(); + string json = JsonSerializer.Serialize(obj); + + Assert.Contains(@"""MyNewNumeric"":1.5", json); + Assert.Contains(@"""MyNumeric"":1", json); + + // Deserialize + json = @"{""MyNewNumeric"":2.5,""MyNumeric"":4}"; + obj = JsonSerializer.Deserialize(json); + + Assert.Equal(4, ((ClassWithHiddenByNewSlotIntProperty)obj).MyNumeric); + Assert.Equal(2.5M, ((ClassWithNewSlotAttributedDecimalProperty)obj).MyNumeric); + } + + [Fact] + public static void Ignore_NonPublicProperty() + { + // Serialize + var obj = new ClassWithInternalProperty(); + string json = JsonSerializer.Serialize(obj); + + Assert.Equal(@"{}", json); + + // Deserialize + json = @"{""MyString"":""NewValue""}"; + obj = JsonSerializer.Deserialize(json); + + Assert.Equal("DefaultValue", obj.MyString); + } + + [Fact] + public static void Ignore_NewSlotPublicPropertyIgnored() + { + // Serialize + var obj = new ClassWithIgnoredNewSlotProperty(); + string json = JsonSerializer.Serialize(obj); + + Assert.Equal(@"{}", json); + + // Deserialize + json = @"{""MyString"":""NewValue""}"; + obj = JsonSerializer.Deserialize(json); + + Assert.Equal("NewDefaultValue", ((ClassWithIgnoredNewSlotProperty)obj).MyString); + Assert.Equal("DefaultValue", ((ClassWithInternalProperty)obj).MyString); + } + + [Fact] + public static void Ignore_BasePublicPropertyIgnored_ConflictWithDerivedPrivate() + { + // Serialize + var obj = new ClassWithIgnoredPublicPropertyAndNewSlotPrivate(); + string json = JsonSerializer.Serialize(obj); + + Assert.Equal(@"{}", json); + + // Deserialize + json = @"{""MyString"":""NewValue""}"; + obj = JsonSerializer.Deserialize(json); + + Assert.Equal("DefaultValue", ((ClassWithIgnoredPublicProperty)obj).MyString); + Assert.Equal("NewDefaultValue", ((ClassWithIgnoredPublicPropertyAndNewSlotPrivate)obj).MyString); + } + + [Fact] + public static void Ignore_PublicProperty_ConflictWithPrivateDueAttributes() + { + // Serialize + var obj = new ClassWithIgnoredPropertyNamingConflictPrivate(); + string json = JsonSerializer.Serialize(obj); + + Assert.Equal(@"{}", json); + + // Deserialize + json = @"{""MyString"":""NewValue""}"; + obj = JsonSerializer.Deserialize(json); + + Assert.Equal("DefaultValue", obj.MyString); + Assert.Equal("ConflictingValue", obj.ConflictingString); + } + + [Fact] + public static void Ignore_PublicProperty_ConflictWithPrivateDuePolicy() + { + var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; + + // Serialize + var obj = new ClassWithIgnoredPropertyPolicyConflictPrivate(); + string json = JsonSerializer.Serialize(obj, options); + + Assert.Equal(@"{}", json); + + // Deserialize + json = @"{""myString"":""NewValue""}"; + obj = JsonSerializer.Deserialize(json, options); + + Assert.Equal("DefaultValue", obj.MyString); + Assert.Equal("ConflictingValue", obj.myString); + } + + [Fact] + public static void Ignore_PublicProperty_ConflictWithPublicDueAttributes() + { + // Serialize + var obj = new ClassWithIgnoredPropertyNamingConflictPublic(); + string json = JsonSerializer.Serialize(obj); + + Assert.Equal(@"{""MyString"":""ConflictingValue""}", json); + + // Deserialize + json = @"{""MyString"":""NewValue""}"; + obj = JsonSerializer.Deserialize(json); + + Assert.Equal("DefaultValue", obj.MyString); + Assert.Equal("NewValue", obj.ConflictingString); + } + + [Fact] + public static void Ignore_PublicProperty_ConflictWithPublicDuePolicy() + { + var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; + + // Serialize + var obj = new ClassWithIgnoredPropertyPolicyConflictPublic(); + string json = JsonSerializer.Serialize(obj, options); + + Assert.Equal(@"{""myString"":""ConflictingValue""}", json); + + // Deserialize + json = @"{""myString"":""NewValue""}"; + obj = JsonSerializer.Deserialize(json, options); + + Assert.Equal("DefaultValue", obj.MyString); + Assert.Equal("NewValue", obj.myString); + } + + [Fact] + public static void Throw_PublicProperty_ConflictDueAttributes() + { + // Serialize + var obj = new ClassWithPropertyNamingConflictWhichThrows(); + Assert.Throws( + () => JsonSerializer.Serialize(obj)); + + // Deserialize + string json = @"{""MyString"":""NewValue""}"; + Assert.Throws( + () => JsonSerializer.Deserialize(json)); + } + + [Fact] + public static void Throw_PublicProperty_ConflictDueAttributes_SingleInheritance() + { + // Serialize + var obj = new ClassInheritedWithPropertyNamingConflictWhichThrows(); + Assert.Throws( + () => JsonSerializer.Serialize(obj)); + + // Deserialize + string json = @"{""MyString"":""NewValue""}"; + Assert.Throws( + () => JsonSerializer.Deserialize(json)); + } + + [Fact] + public static void Throw_PublicProperty_ConflictDueAttributes_DoubleInheritance() + { + // Serialize + var obj = new ClassTwiceInheritedWithPropertyNamingConflictWhichThrows(); + Assert.Throws( + () => JsonSerializer.Serialize(obj)); + + // Deserialize + string json = @"{""MyString"":""NewValue""}"; + Assert.Throws( + () => JsonSerializer.Deserialize(json)); + } + + [Fact] + public static void Throw_PublicProperty_ConflictDuePolicy() + { + var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; + + // Serialize + var obj = new ClassWithPropertyPolicyConflictWhichThrows(); + Assert.Throws( + () => JsonSerializer.Serialize(obj, options)); + + // Deserialize + string json = @"{""MyString"":""NewValue""}"; + Assert.Throws( + () => JsonSerializer.Deserialize(json, options)); + } + + [Fact] + public static void Throw_PublicProperty_ConflictDuePolicy_SingleInheritance() + { + var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; + + // Serialize + var obj = new ClassInheritedWithPropertyPolicyConflictWhichThrows(); + Assert.Throws( + () => JsonSerializer.Serialize(obj, options)); + + // Deserialize + string json = @"{""MyString"":""NewValue""}"; + Assert.Throws( + () => JsonSerializer.Deserialize(json, options)); + } + + [Fact] + public static void Throw_PublicProperty_ConflictDuePolicy_DobuleInheritance() + { + var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; + + // Serialize + var obj = new ClassTwiceInheritedWithPropertyPolicyConflictWhichThrows(); + Assert.Throws( + () => JsonSerializer.Serialize(obj, options)); + + // Deserialize + string json = @"{""MyString"":""NewValue""}"; + Assert.Throws( + () => JsonSerializer.Deserialize(json, options)); + } + + public class ClassWithInternalProperty + { + internal string MyString { get; set; } = "DefaultValue"; + } + + public class ClassWithNewSlotProperty : ClassWithInternalProperty + { + public new string MyString { get; set; } = "NewDefaultValue"; + } + + public class ClassWithPublicProperty + { + public string MyString { get; set; } = "DefaultValue"; + } + + public class ClassWithNewSlotInternalProperty : ClassWithPublicProperty + { + internal new string MyString { get; set; } = "NewDefaultValue"; + } + + public class ClassWithPropertyNamingConflict + { + public string MyString { get; set; } = "DefaultValue"; + + [JsonPropertyName(nameof(MyString))] + internal string ConflictingString { get; set; } = "ConflictingValue"; + } + + public class ClassWithPropertyNamingConflictWhichThrows + { + public string MyString { get; set; } = "DefaultValue"; + + [JsonPropertyName(nameof(MyString))] + public string ConflictingString { get; set; } = "ConflictingValue"; + } + + public class ClassInheritedWithPropertyNamingConflictWhichThrows : ClassWithPublicProperty + { + [JsonPropertyName(nameof(MyString))] + public string ConflictingString { get; set; } = "ConflictingValue"; + } + + public class ClassTwiceInheritedWithPropertyNamingConflictWhichThrowsDummy : ClassWithPublicProperty + { + } + + public class ClassTwiceInheritedWithPropertyNamingConflictWhichThrows : ClassTwiceInheritedWithPropertyNamingConflictWhichThrowsDummy + { + [JsonPropertyName(nameof(MyString))] + public string ConflictingString { get; set; } = "ConflictingValue"; + } + + public class ClassWithPropertyPolicyConflict + { + public string MyString { get; set; } = "DefaultValue"; + + internal string myString { get; set; } = "ConflictingValue"; + } + + public class ClassWithPropertyPolicyConflictWhichThrows + { + public string MyString { get; set; } = "DefaultValue"; + + public string myString { get; set; } = "ConflictingValue"; + } + + public class ClassInheritedWithPropertyPolicyConflictWhichThrows : ClassWithPublicProperty + { + public string myString { get; set; } = "ConflictingValue"; + } + + public class ClassInheritedWithPropertyPolicyConflictWhichThrowsDummy : ClassWithPublicProperty + { + } + + public class ClassTwiceInheritedWithPropertyPolicyConflictWhichThrows : ClassInheritedWithPropertyPolicyConflictWhichThrowsDummy + { + public string myString { get; set; } = "ConflictingValue"; + } + + public class ClassWithIgnoredNewSlotProperty : ClassWithInternalProperty + { + [JsonIgnore] + public new string MyString { get; set; } = "NewDefaultValue"; + } + + public class ClassWithIgnoredPublicProperty + { + [JsonIgnore] + public string MyString { get; set; } = "DefaultValue"; + } + + public class ClassWithIgnoredPublicPropertyAndNewSlotPrivate : ClassWithIgnoredPublicProperty + { + internal new string MyString { get; set; } = "NewDefaultValue"; + } + + public class ClassWithIgnoredPropertyNamingConflictPrivate + { + [JsonIgnore] + public string MyString { get; set; } = "DefaultValue"; + + [JsonPropertyName(nameof(MyString))] + internal string ConflictingString { get; set; } = "ConflictingValue"; + } + + public class ClassWithIgnoredPropertyPolicyConflictPrivate + { + [JsonIgnore] + public string MyString { get; set; } = "DefaultValue"; + + internal string myString { get; set; } = "ConflictingValue"; + } + + public class ClassWithIgnoredPropertyNamingConflictPublic + { + [JsonIgnore] + public string MyString { get; set; } = "DefaultValue"; + + [JsonPropertyName(nameof(MyString))] + public string ConflictingString { get; set; } = "ConflictingValue"; + } + + public class ClassWithIgnoredPropertyPolicyConflictPublic + { + [JsonIgnore] + public string MyString { get; set; } = "DefaultValue"; + + public string myString { get; set; } = "ConflictingValue"; + } + + public class ClassWithHiddenByNewSlotIntProperty + { + public int MyNumeric { get; set; } = 1; + } + + public class ClassWithNewSlotDecimalProperty : ClassWithHiddenByNewSlotIntProperty + { + public new decimal MyNumeric { get; set; } = 1.5M; + } + + public class ClassWithNewSlotAttributedDecimalProperty : ClassWithHiddenByNewSlotIntProperty + { + [JsonPropertyName("MyNewNumeric")] + public new decimal MyNumeric { get; set; } = 1.5M; + } + [Fact] public static void NoSetter() {