From c8b3051399fdf2d6d904be66e859df7135bf9afd Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Tue, 13 Jul 2021 00:27:06 -0700 Subject: [PATCH] Keep looking up derived chain for Property.Get if it doesn't exist. (#52509) --- .../ReflectionXmlSerializationReader.cs | 2 +- .../ReflectionXmlSerializationWriter.cs | 66 ++++++++++++++++++- .../src/System/Xml/Serialization/Types.cs | 5 +- .../XmlSerializerTests.RuntimeOnly.cs | 7 +- .../tests/DataContractSerializer.cs | 2 +- .../tests/SerializationTypes.RuntimeOnly.cs | 4 ++ 6 files changed, 78 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationReader.cs b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationReader.cs index 4e7b4e8e6e92d..eca311b4d31d9 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationReader.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationReader.cs @@ -637,7 +637,7 @@ private static ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate Get (Type, string) typeMemberNameTuple = (o.GetType(), memberName); if (!s_setMemberValueDelegateCache.TryGetValue(typeMemberNameTuple, out ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate? result)) { - MemberInfo memberInfo = ReflectionXmlSerializationHelper.GetMember(o.GetType(), memberName); + MemberInfo memberInfo = ReflectionXmlSerializationHelper.GetEffectiveSetInfo(o.GetType(), memberName); Debug.Assert(memberInfo != null, "memberInfo could not be retrieved"); Type memberType; if (memberInfo is PropertyInfo propInfo) diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationWriter.cs b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationWriter.cs index d138146708e52..c40076aa83861 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationWriter.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationWriter.cs @@ -660,7 +660,7 @@ private void WriteStructMethod(StructMapping mapping, string n, string? ns, obje [RequiresUnreferencedCode("Calls GetType on object")] private object? GetMemberValue(object o, string memberName) { - MemberInfo memberInfo = ReflectionXmlSerializationHelper.GetMember(o.GetType(), memberName); + MemberInfo memberInfo = ReflectionXmlSerializationHelper.GetEffectiveGetInfo(o.GetType(), memberName); object? memberValue = GetMemberValue(o, memberInfo); return memberValue; } @@ -1367,7 +1367,7 @@ private enum WritePrimitiveMethodRequirement internal static class ReflectionXmlSerializationHelper { [RequiresUnreferencedCode("Reflects over base members")] - public static MemberInfo GetMember(Type declaringType, string memberName) + public static MemberInfo? GetMember(Type declaringType, string memberName, bool throwOnNotFound) { MemberInfo[] memberInfos = declaringType.GetMember(memberName); if (memberInfos == null || memberInfos.Length == 0) @@ -1388,7 +1388,11 @@ public static MemberInfo GetMember(Type declaringType, string memberName) if (!foundMatchedMember) { - throw new InvalidOperationException(SR.Format(SR.XmlInternalErrorDetails, $"Could not find member named {memberName} of type {declaringType}")); + if (throwOnNotFound) + { + throw new InvalidOperationException(SR.Format(SR.XmlInternalErrorDetails, $"Could not find member named {memberName} of type {declaringType}")); + } + return null; } declaringType = currentType!; @@ -1409,5 +1413,61 @@ public static MemberInfo GetMember(Type declaringType, string memberName) return memberInfo; } + + [RequiresUnreferencedCode(XmlSerializer.TrimSerializationWarning)] + public static MemberInfo GetEffectiveGetInfo(Type declaringType, string memberName) + { + MemberInfo memberInfo = GetMember(declaringType, memberName, true)!; + + // For properties, we might have a PropertyInfo that does not have a valid + // getter at this level of inheritance. If that's the case, we need to look + // up the chain to find the right PropertyInfo for the getter. + if (memberInfo is PropertyInfo propInfo && propInfo.GetMethod == null) + { + var parent = declaringType.BaseType; + + while (parent != null) + { + var mi = GetMember(parent, memberName, false); + + if (mi is PropertyInfo pi && pi.GetMethod != null && pi.PropertyType == propInfo.PropertyType) + { + return pi; + } + + parent = parent.BaseType; + } + } + + return memberInfo; + } + + [RequiresUnreferencedCode(XmlSerializer.TrimSerializationWarning)] + public static MemberInfo GetEffectiveSetInfo(Type declaringType, string memberName) + { + MemberInfo memberInfo = GetMember(declaringType, memberName, true)!; + + // For properties, we might have a PropertyInfo that does not have a valid + // setter at this level of inheritance. If that's the case, we need to look + // up the chain to find the right PropertyInfo for the setter. + if (memberInfo is PropertyInfo propInfo && propInfo.SetMethod == null) + { + var parent = declaringType.BaseType; + + while (parent != null) + { + var mi = GetMember(parent, memberName, false); + + if (mi is PropertyInfo pi && pi.SetMethod != null && pi.PropertyType == propInfo.PropertyType) + { + return pi; + } + + parent = parent.BaseType; + } + } + + return memberInfo; + } } } diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/Types.cs b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/Types.cs index 84a3d736550fd..1971544633bf6 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/Types.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/Types.cs @@ -1219,7 +1219,10 @@ private static bool ShouldBeReplaced( replacedInfo = info; if (replacedInfo != memberInfoToBeReplaced) { - if (!info.GetMethod!.IsPublic + // The property name is a match. It might be an override, or + // it might be hiding. Either way, check to see if the derived + // property has a getter that is useable for serialization. + if (info.GetMethod != null && !info.GetMethod!.IsPublic && memberInfoToBeReplaced is PropertyInfo && ((PropertyInfo)memberInfoToBeReplaced).GetMethod!.IsPublic ) diff --git a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.RuntimeOnly.cs b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.RuntimeOnly.cs index f83571e7ebf3e..167e812048c2c 100644 --- a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.RuntimeOnly.cs +++ b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.RuntimeOnly.cs @@ -2836,13 +2836,14 @@ public static void DerivedTypeWithDifferentOverrides() [Fact] public static void DerivedTypeWithDifferentOverrides2() { - DerivedTypeWithDifferentOverrides2 value = new DerivedTypeWithDifferentOverrides2() { Name1 = "Name1", Name2 = "Name2", Name3 = "Name3", Name4 = "Name4", Name5 = "Name5", Name6 = "Name6" }; + DerivedTypeWithDifferentOverrides2 value = new DerivedTypeWithDifferentOverrides2() { Name1 = "Name1", Name2 = "Name2", Name3 = "Name3", Name4 = "Name4", Name5 = "Name5", Name6 = "Name6", Name7 = "Name7" }; ((DerivedTypeWithDifferentOverrides)value).Name5 = "MidLevelName5"; ((DerivedTypeWithDifferentOverrides)value).Name4 = "MidLevelName4"; ((SerializationTypes.BaseType)value).Name4 = "BaseLevelName4"; ((DerivedTypeWithDifferentOverrides)value).Name6 = "MidLevelName6"; ((SerializationTypes.BaseType)value).Name6 = "BaseLevelName6"; - DerivedTypeWithDifferentOverrides2 actual = SerializeAndDeserialize(value, @"Name1Name2Name3BaseLevelName4MidLevelName5BaseLevelName6"); + ((DerivedTypeWithDifferentOverrides)value).Name7 = "MidLevelName7"; + DerivedTypeWithDifferentOverrides2 actual = SerializeAndDeserialize(value, @"Name1Name2Name3BaseLevelName4MidLevelName5BaseLevelName6MidLevelName7"); Assert.Equal(value.Name1, actual.Name1); Assert.Equal(value.Name2, actual.Name2); Assert.Equal(value.Name3, actual.Name3); @@ -2855,6 +2856,8 @@ public static void DerivedTypeWithDifferentOverrides2() Assert.Null(actual.Name6); Assert.Equal(((DerivedTypeWithDifferentOverrides)actual).Name6, ((SerializationTypes.BaseType)actual).Name6); Assert.Equal(((SerializationTypes.BaseType)actual).Name6, ((SerializationTypes.BaseType)actual).Name6); + Assert.Equal(((DerivedTypeWithDifferentOverrides)actual).Name7, ((SerializationTypes.BaseType)actual).Name7); + Assert.Equal(actual.Name7, ((SerializationTypes.BaseType)actual).Name7); } [Fact] diff --git a/src/libraries/System.Runtime.Serialization.Xml/tests/DataContractSerializer.cs b/src/libraries/System.Runtime.Serialization.Xml/tests/DataContractSerializer.cs index 2a39358aa09bb..038b9130d4dc8 100644 --- a/src/libraries/System.Runtime.Serialization.Xml/tests/DataContractSerializer.cs +++ b/src/libraries/System.Runtime.Serialization.Xml/tests/DataContractSerializer.cs @@ -1084,7 +1084,7 @@ public static void DCS_WithListOfXElement() public static void DCS_DerivedTypeWithDifferentOverrides() { var x = new DerivedTypeWithDifferentOverrides() { Name1 = "Name1", Name2 = "Name2", Name3 = "Name3", Name4 = "Name4", Name5 = "Name5" }; - var y = DataContractSerializerHelper.SerializeAndDeserialize(x, @"Name1Name2Name3Name5"); + var y = DataContractSerializerHelper.SerializeAndDeserialize(x, @"Name1Name2Name3Name5"); Assert.Equal(x.Name1, y.Name1); Assert.Equal(x.Name2, y.Name2); diff --git a/src/libraries/System.Runtime.Serialization.Xml/tests/SerializationTypes.RuntimeOnly.cs b/src/libraries/System.Runtime.Serialization.Xml/tests/SerializationTypes.RuntimeOnly.cs index 70e080bae6e06..de485ec10b570 100644 --- a/src/libraries/System.Runtime.Serialization.Xml/tests/SerializationTypes.RuntimeOnly.cs +++ b/src/libraries/System.Runtime.Serialization.Xml/tests/SerializationTypes.RuntimeOnly.cs @@ -837,6 +837,8 @@ public class BaseType public string @Name5 { get; set; } public virtual string Name6 { get; set; } + + public virtual string Name7 { get; set; } } public class DerivedTypeWithDifferentOverrides : BaseType @@ -852,6 +854,8 @@ public class DerivedTypeWithDifferentOverrides : BaseType public new string Name5 { get; set; } public override string Name6 { get; set; } + + public override string Name7 { set { base.Name7 = value; } } } public class DerivedTypeWithDifferentOverrides2 : DerivedTypeWithDifferentOverrides