Don't use PropertyInfo from unmapped base type

Fixes #22092
dotnet · Aug 25, 2020 · c6ef53f · c6ef53f
1 parent e3a2651
commit c6ef53f
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 24 deletions.
diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs
@@ -510,8 +510,7 @@ private InternalPropertyBuilder Property(
                     entityType = existingProperty.DeclaringEntityType;
                 }
 
-                var existingMember = existingProperty.GetIdentifyingMemberInfo();
-                if ((memberInfo == null || existingMember.IsOverridenBy(memberInfo))
+                if (IsCompatible(memberInfo, existingProperty)
                     && (propertyType == null || propertyType == existingProperty.ClrType))
                 {
                     if (configurationSource.HasValue)
@@ -699,6 +698,50 @@ private InternalPropertyBuilder Property(
                 : builder;
         }
 
+        private bool IsCompatible(MemberInfo newMemberInfo, Property existingProperty)
+        {
+            if (newMemberInfo == null)
+            {
+                return true;
+            }
+
+            var existingMemberInfo = existingProperty.GetIdentifyingMemberInfo();
+            if (existingMemberInfo == null)
+            {
+                return false;
+            }
+
+            if (newMemberInfo == existingMemberInfo)
+            {
+                return true;
+            }
+
+            var declaringType = (IMutableEntityType)existingProperty.DeclaringType;
+            if (!newMemberInfo.DeclaringType.IsAssignableFrom(declaringType.ClrType))
+            {
+                return existingMemberInfo.IsOverridenBy(newMemberInfo);
+            }
+
+            IMutableEntityType existingMemberDeclaringEntityType = null;
+            foreach (var baseType in declaringType.GetAllBaseTypes())
+            {
+                if (newMemberInfo.DeclaringType == baseType.ClrType)
+                {
+                    return existingMemberDeclaringEntityType != null
+                        && existingMemberInfo.IsOverridenBy(newMemberInfo);
+                }
+
+                if (existingMemberDeclaringEntityType == null
+                    && existingMemberInfo.DeclaringType == baseType.ClrType)
+                {
+                    existingMemberDeclaringEntityType = baseType;
+                }
+            }
+
+            // newMemberInfo is declared on an unmapped base type, existingMemberInfo should be kept
+            return newMemberInfo.IsOverridenBy(existingMemberInfo);
+        }
+
         private bool CanRemoveProperty(
             [NotNull] Property property,
             ConfigurationSource configurationSource,

diff --git a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs
@@ -2501,6 +2501,7 @@ protected class One : OneBase
         {
             [Key]
             [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
+            [Column("Unique_No")]
             public override int UniqueNo { get; set; }
 
             [ConcurrencyCheck]

diff --git a/test/EFCore.SqlServer.FunctionalTests/DataAnnotationSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/DataAnnotationSqlServerTest.cs
@@ -129,11 +129,11 @@ public override ModelBuilder DatabaseGeneratedOption_configures_the_property_cor
         public virtual void ColumnAttribute_configures_the_property_correctly()
         {
             var modelBuilder = CreateModelBuilder();
-            modelBuilder.Entity<One>();
+            modelBuilder.Entity<One>().HasKey(o => o.UniqueNo);
 
             Assert.Equal(
-                "Name",
-                modelBuilder.Model.FindEntityType(typeof(One)).FindProperty(nameof(One.RequiredColumn)).GetColumnName());
+                "Unique_No",
+                modelBuilder.Model.FindEntityType(typeof(One)).FindProperty(nameof(One.UniqueNo)).GetColumnName());
         }
 
         public override ModelBuilder DatabaseGeneratedOption_Identity_does_not_throw_on_noninteger_properties()
@@ -159,13 +159,13 @@ public override void ConcurrencyCheckAttribute_throws_if_value_in_database_chang
             base.ConcurrencyCheckAttribute_throws_if_value_in_database_changed();
 
             AssertSql(
-                @"SELECT TOP(1) [s].[UniqueNo], [s].[MaxLengthProperty], [s].[Name], [s].[RowVersion], [s].[AdditionalDetails_Name], [s].[Details_Name]
+                @"SELECT TOP(1) [s].[Unique_No], [s].[MaxLengthProperty], [s].[Name], [s].[RowVersion], [s].[AdditionalDetails_Name], [s].[Details_Name]
 FROM [Sample] AS [s]
-WHERE [s].[UniqueNo] = 1",
+WHERE [s].[Unique_No] = 1",
                 //
-                @"SELECT TOP(1) [s].[UniqueNo], [s].[MaxLengthProperty], [s].[Name], [s].[RowVersion], [s].[AdditionalDetails_Name], [s].[Details_Name]
+                @"SELECT TOP(1) [s].[Unique_No], [s].[MaxLengthProperty], [s].[Name], [s].[RowVersion], [s].[AdditionalDetails_Name], [s].[Details_Name]
 FROM [Sample] AS [s]
-WHERE [s].[UniqueNo] = 1",
+WHERE [s].[Unique_No] = 1",
                 //
                 @"@p2='1'
 @p0='ModifiedData' (Nullable = false) (Size = 4000)
@@ -174,7 +174,7 @@ FROM [Sample] AS [s]
 
 SET NOCOUNT ON;
 UPDATE [Sample] SET [Name] = @p0, [RowVersion] = @p1
-WHERE [UniqueNo] = @p2 AND [RowVersion] = @p3;
+WHERE [Unique_No] = @p2 AND [RowVersion] = @p3;
 SELECT @@ROWCOUNT;",
                 //
                 @"@p2='1'
@@ -184,7 +184,7 @@ FROM [Sample] AS [s]
 
 SET NOCOUNT ON;
 UPDATE [Sample] SET [Name] = @p0, [RowVersion] = @p1
-WHERE [UniqueNo] = @p2 AND [RowVersion] = @p3;
+WHERE [Unique_No] = @p2 AND [RowVersion] = @p3;
 SELECT @@ROWCOUNT;");
         }
 
@@ -202,9 +202,9 @@ public override void DatabaseGeneratedAttribute_autogenerates_values_when_set_to
 SET NOCOUNT ON;
 INSERT INTO [Sample] ([MaxLengthProperty], [Name], [RowVersion], [AdditionalDetails_Name], [Details_Name])
 VALUES (@p0, @p1, @p2, @p3, @p4);
-SELECT [UniqueNo]
+SELECT [Unique_No]
 FROM [Sample]
-WHERE @@ROWCOUNT = 1 AND [UniqueNo] = scope_identity();");
+WHERE @@ROWCOUNT = 1 AND [Unique_No] = scope_identity();");
         }
 
         public override void MaxLengthAttribute_throws_while_inserting_value_longer_than_max_length()
@@ -221,9 +221,9 @@ public override void MaxLengthAttribute_throws_while_inserting_value_longer_than
 SET NOCOUNT ON;
 INSERT INTO [Sample] ([MaxLengthProperty], [Name], [RowVersion], [AdditionalDetails_Name], [Details_Name])
 VALUES (@p0, @p1, @p2, @p3, @p4);
-SELECT [UniqueNo]
+SELECT [Unique_No]
 FROM [Sample]
-WHERE @@ROWCOUNT = 1 AND [UniqueNo] = scope_identity();",
+WHERE @@ROWCOUNT = 1 AND [Unique_No] = scope_identity();",
                 //
                 @"@p0='VeryVeryVeryVeryVeryVeryLongString' (Size = 4000)
 @p1='ValidString' (Nullable = false) (Size = 4000)
@@ -234,9 +234,9 @@ FROM [Sample]
 SET NOCOUNT ON;
 INSERT INTO [Sample] ([MaxLengthProperty], [Name], [RowVersion], [AdditionalDetails_Name], [Details_Name])
 VALUES (@p0, @p1, @p2, @p3, @p4);
-SELECT [UniqueNo]
+SELECT [Unique_No]
 FROM [Sample]
-WHERE @@ROWCOUNT = 1 AND [UniqueNo] = scope_identity();");
+WHERE @@ROWCOUNT = 1 AND [Unique_No] = scope_identity();");
         }
 
         public override void StringLengthAttribute_throws_while_inserting_value_longer_than_max_length()

diff --git a/test/EFCore.Sqlite.FunctionalTests/DataAnnotationSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/DataAnnotationSqliteTest.cs
@@ -97,14 +97,14 @@ public override void ConcurrencyCheckAttribute_throws_if_value_in_database_chang
             base.ConcurrencyCheckAttribute_throws_if_value_in_database_changed();
 
             AssertSql(
-                @"SELECT ""s"".""UniqueNo"", ""s"".""MaxLengthProperty"", ""s"".""Name"", ""s"".""RowVersion"", ""s"".""AdditionalDetails_Name"", ""s"".""Details_Name""
+                @"SELECT ""s"".""Unique_No"", ""s"".""MaxLengthProperty"", ""s"".""Name"", ""s"".""RowVersion"", ""s"".""AdditionalDetails_Name"", ""s"".""Details_Name""
 FROM ""Sample"" AS ""s""
-WHERE ""s"".""UniqueNo"" = 1
+WHERE ""s"".""Unique_No"" = 1
 LIMIT 1",
                 //
-                @"SELECT ""s"".""UniqueNo"", ""s"".""MaxLengthProperty"", ""s"".""Name"", ""s"".""RowVersion"", ""s"".""AdditionalDetails_Name"", ""s"".""Details_Name""
+                @"SELECT ""s"".""Unique_No"", ""s"".""MaxLengthProperty"", ""s"".""Name"", ""s"".""RowVersion"", ""s"".""AdditionalDetails_Name"", ""s"".""Details_Name""
 FROM ""Sample"" AS ""s""
-WHERE ""s"".""UniqueNo"" = 1
+WHERE ""s"".""Unique_No"" = 1
 LIMIT 1",
                 //
                 @"@p2='1' (DbType = String)
@@ -113,7 +113,7 @@ public override void ConcurrencyCheckAttribute_throws_if_value_in_database_chang
 @p3='00000001-0000-0000-0000-000000000001' (DbType = String)
 
 UPDATE ""Sample"" SET ""Name"" = @p0, ""RowVersion"" = @p1
-WHERE ""UniqueNo"" = @p2 AND ""RowVersion"" = @p3;
+WHERE ""Unique_No"" = @p2 AND ""RowVersion"" = @p3;
 SELECT changes();",
                 //
                 @"@p2='1' (DbType = String)
@@ -122,7 +122,7 @@ public override void ConcurrencyCheckAttribute_throws_if_value_in_database_chang
 @p3='00000001-0000-0000-0000-000000000001' (DbType = String)
 
 UPDATE ""Sample"" SET ""Name"" = @p0, ""RowVersion"" = @p1
-WHERE ""UniqueNo"" = @p2 AND ""RowVersion"" = @p3;
+WHERE ""Unique_No"" = @p2 AND ""RowVersion"" = @p3;
 SELECT changes();");
         }
 
@@ -139,7 +139,7 @@ public override void DatabaseGeneratedAttribute_autogenerates_values_when_set_to
 
 INSERT INTO ""Sample"" (""MaxLengthProperty"", ""Name"", ""RowVersion"", ""AdditionalDetails_Name"", ""Details_Name"")
 VALUES (@p0, @p1, @p2, @p3, @p4);
-SELECT ""UniqueNo""
+SELECT ""Unique_No""
 FROM ""Sample""
 WHERE changes() = 1 AND ""rowid"" = last_insert_rowid();");
         }