From c6ef53f995bcbf20e626772807e8ca9d5d236ef4 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Tue, 25 Aug 2020 00:33:53 -0700 Subject: [PATCH] Don't use PropertyInfo from unmapped base type Fixes #22092 --- .../Internal/InternalEntityTypeBuilder.cs | 47 ++++++++++++++++++- .../DataAnnotationTestBase.cs | 1 + .../DataAnnotationSqlServerTest.cs | 30 ++++++------ .../DataAnnotationSqliteTest.cs | 14 +++--- 4 files changed, 68 insertions(+), 24 deletions(-) diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs index 49c0b7a2b8f..ce641948166 100644 --- 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 index 658981e6208..ddc860c2cf2 100644 --- 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 index b7a8a869b08..b78d4275618 100644 --- 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(); + modelBuilder.Entity().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 index b13a112458e..2ed67884db3 100644 --- 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();"); }