Merge pull request #1065 from aws/asmarp/fix-ddb-dictionary-error

fix: fix ddb error when derived and base class have same member name

generator/.DevConfigs/65e61bbf-6a61-4066-b297-b670b5e7e422.json

@@ -0,0 +1,11 @@
+{
+    "services": [
+        {
+            "serviceName": "DynamoDBv2",
+            "type": "patch",
+            "changeLogMessages": [
+                "Fixed an issue causing DynamoDB context save to fail if the model object inherits a base class that contains members with the same name"
+            ]
+        }
+    ]
+}

sdk/src/Services/DynamoDBv2/Custom/DataModel/InternalModel.cs

@@ -304,28 +304,13 @@ private bool FindSingleProperty(Func<PropertyStorage, bool> match, string errorM
 
             throw new InvalidOperationException(errorMessage);
         }
-
-        public static bool IsValidMemberInfo(MemberInfo member)
-        {
-            // filter out non-fields and non-properties
-            if (!(member is FieldInfo || member is PropertyInfo))
-                return false;
-
-            // filter out properties that aren't both read and write
-            if (!Utils.IsReadWrite(member))
-                return false;
-
-            return true;
-        }
 
         private static Dictionary<string, MemberInfo> GetMembersDictionary(Type type)
         {
             Dictionary<string, MemberInfo> dictionary = new Dictionary<string, MemberInfo>(StringComparer.Ordinal);
 
-            var members = type
-                .GetMembers(BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public)
-                .Where(IsValidMemberInfo)
-                .ToList();
+            var members = Utils.GetMembersFromType(type);
+
             foreach (var member in members)
             {
                 InternalSDKUtils.AddToDictionary(dictionary, member.Name, member);
@@ -773,11 +758,10 @@ private static void PopulateConfigFromType(ItemStorageConfig config, Type type)
             if (AWSConfigsDynamoDB.Context.TableAliases.TryGetValue(config.TableName, out tableAlias))
                 config.TableName = tableAlias;
 
-            foreach (var member in type.GetMembers(BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public))
+            var members = Utils.GetMembersFromType(type);
+
+            foreach (var member in members)
             {
-                if (!StorageConfig.IsValidMemberInfo(member))
-                    continue;
-
                 // prepare basic info
                 PropertyStorage propertyStorage = new PropertyStorage(member);
                 propertyStorage.AttributeName = GetAccurateCase(config, member.Name);

sdk/src/Services/DynamoDBv2/Custom/DataModel/Utils.cs

@@ -376,6 +376,64 @@ public static bool ImplementsInterface(Type targetType, Type interfaceType)
             }
             return false;
         }
+
+        /// <summary>
+        /// Apply a set of filters to a determine whether a member should be returned.
+        /// In terms of DynamoDb, we want to return members that are fields or properties
+        /// and are both read and write members.
+        /// </summary>
+        private static bool IsValidMemberInfo(MemberInfo member)
+        {
+            // filter out non-fields and non-properties
+            if (!(member is FieldInfo || member is PropertyInfo))
+                return false;
+
+            // filter out properties that aren't both read and write
+            if (!IsReadWrite(member))
+                return false;
+
+            return true;
+        }
+
+        /// <summary>
+        /// Retrieves a list of members that exist in a given type.
+        /// The function goes over all the declared members of a given type
+        /// and recurses into any base types and the declared members of those types.
+        /// In case of members existing in both derived and base types,
+        /// members from the derived types will be used while ignoring same-name members
+        /// in base types to avoid returning duplicate members.
+        /// </summary>
+        public static List<MemberInfo> GetMembersFromType(Type type)
+        {
+            Dictionary<string, MemberInfo> members = new Dictionary<string, MemberInfo>();
+
+            Type currentType = type;
+            while (
+                currentType != null && 
+                currentType != typeof(object))
+            {
+                // Previous implementation used GetMembers to return the valid members for a type, but in certain class configurations
+                // invalid members were returned. To account for that, we are going over each type separately.
+                // Using 'DeclaredOnly' binding flag to return the members that were declared in the current type and not any inherited members
+                // since the iteration is going over each base type separately.
+                var currentMembers = currentType
+                    .GetMembers(BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.DeclaredOnly)
+                    .Where(IsValidMemberInfo)
+                    .ToList();
+
+                foreach (var member in currentMembers)
+                {
+                    if (!members.ContainsKey(member.Name))
+                    {
+                        members[member.Name] = member;
+                    }
+                }
+
+                currentType = currentType.BaseType;
+            }
+
+            return members.Values.ToList();
+        }
 
 #endregion
 

sdk/test/NetStandard/IntegrationTests/IntegrationTests/DynamoDB/DataModelDerivedClassTest.cs

@@ -35,6 +35,25 @@ public async Task TestDerivedPropertyOverrides()
                 await SharedTestFixture.Context.DeleteAsync<Derived>(loaded);
             }
         }
+
+        [Fact]
+        [Trait(CategoryAttribute, "DynamoDB")]
+        public async Task TestDerivedProperties_MaskedUsingNew()
+        {
+            foreach (var conversion in new DynamoDBEntryConversion[] { DynamoDBEntryConversion.V1, DynamoDBEntryConversion.V2 })
+            {
+                var d = new DerivedClassExample { Name = "Name", Age = 200, Data = "Test data"};
+
+                await SharedTestFixture.Context.SaveAsync<DerivedClassExample>(d);
+
+                var loaded = await SharedTestFixture.Context.LoadAsync<DerivedClassExample>(d.Name, d.Age);
+                Assert.Equal(d.Name, loaded.Name);
+                Assert.Equal(d.Age, loaded.Age);
+                Assert.Equal(d.Data, loaded.Data);
+
+                await SharedTestFixture.Context.DeleteAsync<DerivedClassExample>(loaded);
+            }
+        }
 
         private class Base
         {
@@ -51,5 +70,26 @@ private class Derived : Base
 
             public string Data { get; set; }
         }
+
+        public class BaseClassExample
+        {
+            public string Name { get; set; }
+
+            public int Age { get; set; }
+
+            public string Data { get; set; }
+        }
+
+        [DynamoDBTable("HashRangeTable")]
+        public class DerivedClassExample: BaseClassExample
+        {
+            [DynamoDBHashKey]
+            public new string Name
+            {
+                get => base.Name;
+                set => base.Name = value;
+            }
+            public new int Age { get; set; }
+        }
     }
 }