diff --git a/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs b/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs index dd3d11b7b65..b34c3fa04cf 100644 --- a/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs @@ -50,6 +50,14 @@ public static string GetColumnName([NotNull] this IProperty property, in StoreOb return overrides.ColumnName; } + if (!property.IsPrimaryKey() + && storeObject.StoreObjectType != StoreObjectType.Function + && storeObject.StoreObjectType != StoreObjectType.SqlQuery + && StoreObjectIdentifier.Create(property.DeclaringEntityType, storeObject.StoreObjectType) != storeObject) + { + return null; + } + var columnAnnotation = property.FindAnnotation(RelationalAnnotationNames.ColumnName); if (columnAnnotation != null) { diff --git a/src/EFCore.Relational/Metadata/Conventions/EntityTypeHierarchyMappingConvention.cs b/src/EFCore.Relational/Metadata/Conventions/EntityTypeHierarchyMappingConvention.cs index bbfb2b6c302..76c191d9a47 100644 --- a/src/EFCore.Relational/Metadata/Conventions/EntityTypeHierarchyMappingConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/EntityTypeHierarchyMappingConvention.cs @@ -43,15 +43,6 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder, || schema != entityType.BaseType.GetSchema())) { nonTphRoots.Add(entityType.GetRootType()); - foreach (var property in entityType.BaseType.GetProperties()) - { - if (property.IsPrimaryKey()) - { - continue; - } - - property.Builder.HasColumnName(null, StoreObjectIdentifier.Table(tableName, schema)); - } } var viewName = entityType.GetViewName(); @@ -61,15 +52,6 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder, || viewSchema != entityType.BaseType.GetViewSchema())) { nonTphRoots.Add(entityType.GetRootType()); - foreach (var property in entityType.BaseType.GetProperties()) - { - if (property.IsPrimaryKey()) - { - continue; - } - - property.Builder.HasColumnName(null, StoreObjectIdentifier.View(viewName, viewSchema)); - } } } diff --git a/src/EFCore.Relational/Metadata/Internal/RelationalForeignKeyExtensions.cs b/src/EFCore.Relational/Metadata/Internal/RelationalForeignKeyExtensions.cs index 719044bcd56..1a6e7a20c42 100644 --- a/src/EFCore.Relational/Metadata/Internal/RelationalForeignKeyExtensions.cs +++ b/src/EFCore.Relational/Metadata/Internal/RelationalForeignKeyExtensions.cs @@ -31,9 +31,10 @@ public static bool AreCompatible( bool shouldThrow) { var principalType = foreignKey.PrincipalEntityType; + var principalTable = StoreObjectIdentifier.Create(principalType, StoreObjectType.Table); var duplicatePrincipalType = duplicateForeignKey.PrincipalEntityType; - if (!string.Equals(principalType.GetSchema(), duplicatePrincipalType.GetSchema(), StringComparison.OrdinalIgnoreCase) - || !string.Equals(principalType.GetTableName(), duplicatePrincipalType.GetTableName(), StringComparison.OrdinalIgnoreCase)) + var duplicatePrincipalTable = StoreObjectIdentifier.Create(duplicatePrincipalType, StoreObjectType.Table); + if (principalTable != duplicatePrincipalTable) { if (shouldThrow) { @@ -44,9 +45,7 @@ public static bool AreCompatible( duplicateForeignKey.Properties.Format(), duplicateForeignKey.DeclaringEntityType.DisplayName(), foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(), - foreignKey.GetConstraintName(storeObject, - StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(), - foreignKey.PrincipalEntityType.GetSchema())), + foreignKey.GetConstraintName(storeObject, principalTable.Value), principalType.GetSchemaQualifiedTableName(), duplicatePrincipalType.GetSchemaQualifiedTableName())); } @@ -65,9 +64,7 @@ public static bool AreCompatible( duplicateForeignKey.Properties.Format(), duplicateForeignKey.DeclaringEntityType.DisplayName(), foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(), - foreignKey.GetConstraintName(storeObject, - StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(), - foreignKey.PrincipalEntityType.GetSchema())), + foreignKey.GetConstraintName(storeObject, principalTable.Value), foreignKey.Properties.FormatColumns(storeObject), duplicateForeignKey.Properties.FormatColumns(storeObject))); } @@ -75,7 +72,7 @@ public static bool AreCompatible( return false; } - if (!SameColumnNames(foreignKey.PrincipalKey.Properties, duplicateForeignKey.PrincipalKey.Properties, storeObject)) + if (!SameColumnNames(foreignKey.PrincipalKey.Properties, duplicateForeignKey.PrincipalKey.Properties, principalTable.Value)) { if (shouldThrow) { @@ -86,11 +83,9 @@ public static bool AreCompatible( duplicateForeignKey.Properties.Format(), duplicateForeignKey.DeclaringEntityType.DisplayName(), foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(), - foreignKey.GetConstraintName(storeObject, - StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(), - foreignKey.PrincipalEntityType.GetSchema())), - foreignKey.PrincipalKey.Properties.FormatColumns(storeObject), - duplicateForeignKey.PrincipalKey.Properties.FormatColumns(storeObject))); + foreignKey.GetConstraintName(storeObject, principalTable.Value), + foreignKey.PrincipalKey.Properties.FormatColumns(principalTable.Value), + duplicateForeignKey.PrincipalKey.Properties.FormatColumns(principalTable.Value))); } return false; @@ -107,9 +102,7 @@ public static bool AreCompatible( duplicateForeignKey.Properties.Format(), duplicateForeignKey.DeclaringEntityType.DisplayName(), foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(), - foreignKey.GetConstraintName(storeObject, - StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(), - foreignKey.PrincipalEntityType.GetSchema())))); + foreignKey.GetConstraintName(storeObject, principalTable.Value))); } return false; @@ -126,9 +119,7 @@ public static bool AreCompatible( duplicateForeignKey.Properties.Format(), duplicateForeignKey.DeclaringEntityType.DisplayName(), foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(), - foreignKey.GetConstraintName(storeObject, - StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(), - foreignKey.PrincipalEntityType.GetSchema())), + foreignKey.GetConstraintName(storeObject, principalTable.Value), foreignKey.DeleteBehavior, duplicateForeignKey.DeleteBehavior)); } diff --git a/src/EFCore.Relational/Metadata/StoreObjectIdentifier.cs b/src/EFCore.Relational/Metadata/StoreObjectIdentifier.cs index e3303a2369c..ca4cc93d01f 100644 --- a/src/EFCore.Relational/Metadata/StoreObjectIdentifier.cs +++ b/src/EFCore.Relational/Metadata/StoreObjectIdentifier.cs @@ -10,7 +10,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata /// /// A type that represents the id of a store object /// - public readonly struct StoreObjectIdentifier : IComparable + public readonly struct StoreObjectIdentifier : IComparable, IEquatable { private StoreObjectIdentifier(StoreObjectType storeObjectType, string name, string schema = null) { @@ -19,6 +19,35 @@ private StoreObjectIdentifier(StoreObjectType storeObjectType, string name, stri Schema = schema; } + /// + /// Creates an id for the store object that the given entity type is mapped to />. + /// + /// The entity type. + /// The store object type. + /// The store object id. + public static StoreObjectIdentifier? Create([NotNull] IEntityType entityType, StoreObjectType type) + { + Check.NotNull(entityType, nameof(entityType)); + + switch (type) + { + case StoreObjectType.Table: + var tableName = entityType.GetTableName(); + return tableName == null ? (StoreObjectIdentifier?)null : Table(tableName, entityType.GetSchema()); + case StoreObjectType.View: + var viewName = entityType.GetViewName(); + return viewName == null ? (StoreObjectIdentifier?)null : View(viewName, entityType.GetViewSchema()); + case StoreObjectType.SqlQuery: + var query = entityType.GetSqlQuery(); + return query == null ? (StoreObjectIdentifier?)null : SqlQuery(entityType); + case StoreObjectType.Function: + var functionName = entityType.GetFunctionName(); + return functionName == null ? (StoreObjectIdentifier?)null : DbFunction(functionName); + default: + return null; + } + } + /// /// Creates a table id. /// @@ -121,5 +150,37 @@ public int CompareTo(StoreObjectIdentifier other) /// public override string ToString() => StoreObjectType.ToString() + " " + DisplayName(); + + /// + public override bool Equals(object obj) + => obj is StoreObjectIdentifier identifier && Equals(identifier); + + /// + public bool Equals(StoreObjectIdentifier other) + => StoreObjectType == other.StoreObjectType && + Name == other.Name && + Schema == other.Schema; + + /// + public override int GetHashCode() + => HashCode.Combine(StoreObjectType, Name, Schema); + + /// + /// Compares one id to another id to see if they represent the same store object. + /// + /// The first id. + /// The second id. + /// if they represent the same store object; otherwise. + public static bool operator ==(StoreObjectIdentifier left, StoreObjectIdentifier right) + => left.Equals(right); + + /// + /// Compares one id to another id to see if they represent the same store object. + /// + /// The first id. + /// The second id. + /// if they represent the same store object; otherwise. + public static bool operator !=(StoreObjectIdentifier left, StoreObjectIdentifier right) + => !(left == right); } } diff --git a/src/EFCore/Diagnostics/CoreEventId.cs b/src/EFCore/Diagnostics/CoreEventId.cs index 79f6d40f036..1fc9d33b9b2 100644 --- a/src/EFCore/Diagnostics/CoreEventId.cs +++ b/src/EFCore/Diagnostics/CoreEventId.cs @@ -88,8 +88,8 @@ private enum Id ShadowPropertyCreated = CoreBaseId + 600, RedundantIndexRemoved, IncompatibleMatchingForeignKeyProperties, - RequiredAttributeOnDependent, // Unused - RequiredAttributeOnBothNavigations, // Unused + Obsolete_RequiredAttributeOnDependent, + Obsolete_RequiredAttributeOnBothNavigations, ConflictingShadowForeignKeysWarning, MultiplePrimaryKeyCandidates, MultipleNavigationProperties, @@ -100,10 +100,10 @@ private enum Id ForeignKeyAttributesOnBothNavigationsWarning, ConflictingForeignKeyAttributesOnNavigationAndPropertyWarning, RedundantForeignKeyWarning, - NonNullableInverted, // Unused - NonNullableReferenceOnBothNavigations, // Unused - NonNullableReferenceOnDependent, // Unused - RequiredAttributeInverted, // Unused + Obsolete_NonNullableInverted, + Obsolete_NonNullableReferenceOnBothNavigations, + Obsolete_NonNullableReferenceOnDependent, + Obsolete_RequiredAttributeInverted, RequiredAttributeOnCollection, CollectionWithoutComparer, ConflictingKeylessAndKeyAttributesWarning, @@ -475,7 +475,7 @@ public static readonly EventId FirstWithoutOrderByAndFilterWarning /// /// [Obsolete] - public static readonly EventId RequiredAttributeInverted = MakeModelId(Id.RequiredAttributeInverted); + public static readonly EventId RequiredAttributeInverted = MakeModelId(Id.Obsolete_RequiredAttributeInverted); /// /// @@ -490,7 +490,7 @@ public static readonly EventId FirstWithoutOrderByAndFilterWarning /// /// [Obsolete] - public static readonly EventId NonNullableInverted = MakeModelId(Id.NonNullableInverted); + public static readonly EventId NonNullableInverted = MakeModelId(Id.Obsolete_NonNullableInverted); /// /// @@ -504,7 +504,7 @@ public static readonly EventId FirstWithoutOrderByAndFilterWarning /// /// [Obsolete] - public static readonly EventId RequiredAttributeOnBothNavigations = MakeModelId(Id.RequiredAttributeOnBothNavigations); + public static readonly EventId RequiredAttributeOnBothNavigations = MakeModelId(Id.Obsolete_RequiredAttributeOnBothNavigations); /// /// @@ -518,7 +518,7 @@ public static readonly EventId FirstWithoutOrderByAndFilterWarning /// /// [Obsolete] - public static readonly EventId NonNullableReferenceOnBothNavigations = MakeModelId(Id.NonNullableReferenceOnBothNavigations); + public static readonly EventId NonNullableReferenceOnBothNavigations = MakeModelId(Id.Obsolete_NonNullableReferenceOnBothNavigations); /// /// @@ -532,7 +532,7 @@ public static readonly EventId FirstWithoutOrderByAndFilterWarning /// /// [Obsolete] - public static readonly EventId RequiredAttributeOnDependent = MakeModelId(Id.RequiredAttributeOnDependent); + public static readonly EventId RequiredAttributeOnDependent = MakeModelId(Id.Obsolete_RequiredAttributeOnDependent); /// /// @@ -546,7 +546,7 @@ public static readonly EventId FirstWithoutOrderByAndFilterWarning /// /// [Obsolete] - public static readonly EventId NonNullableReferenceOnDependent = MakeModelId(Id.NonNullableReferenceOnDependent); + public static readonly EventId NonNullableReferenceOnDependent = MakeModelId(Id.Obsolete_NonNullableReferenceOnDependent); /// /// diff --git a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTestBase.cs b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTestBase.cs index 5acc884b957..50762900d19 100644 --- a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTestBase.cs +++ b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTestBase.cs @@ -147,10 +147,10 @@ protected virtual ModelBuilder CreateModelBuilder(bool skipConventions) private ConventionSet CreateEmptyConventionSet() { var conventions = new ConventionSet(); - var conventionSetDependencies = TestHelpers.CreateContextServices().GetRequiredService(); - var relationalConventionSetDependencies = new RelationalConventionSetBuilderDependencies( - new RelationalAnnotationProvider( - new RelationalAnnotationProviderDependencies())); + var conventionSetDependencies = TestHelpers.CreateContextServices() + .GetRequiredService(); + var relationalConventionSetDependencies = TestHelpers.CreateContextServices() + .GetRequiredService(); conventions.ModelFinalizingConventions.Add(new TypeMappingConvention(conventionSetDependencies)); conventions.ModelFinalizedConventions.Add(new RelationalModelConvention(conventionSetDependencies, relationalConventionSetDependencies)); return conventions; @@ -158,16 +158,16 @@ private ConventionSet CreateEmptyConventionSet() protected virtual MigrationsModelDiffer CreateModelDiffer(DbContextOptions options) { - var ctx = TestHelpers.CreateContext(options); + var context = TestHelpers.CreateContext(options); return new MigrationsModelDiffer( new TestRelationalTypeMappingSource( TestServiceFactory.Instance.Create(), TestServiceFactory.Instance.Create()), new MigrationsAnnotationProvider( new MigrationsAnnotationProviderDependencies()), - ctx.GetService(), - ctx.GetService(), - ctx.GetService()); + context.GetService(), + context.GetService(), + context.GetService()); } } } diff --git a/test/EFCore.SqlServer.Tests/Migrations/SqlServerModelDifferTest.cs b/test/EFCore.SqlServer.Tests/Migrations/SqlServerModelDifferTest.cs index 02d0d9585a2..ca98643251b 100644 --- a/test/EFCore.SqlServer.Tests/Migrations/SqlServerModelDifferTest.cs +++ b/test/EFCore.SqlServer.Tests/Migrations/SqlServerModelDifferTest.cs @@ -927,6 +927,153 @@ public void Rebuild_index_when_changing_online_option() }); } + [ConditionalFact] + public void Noop_TPT_with_FKs_and_seed_data() + { + Execute( + modelBuilder => + { + }, + source => + { + source.Entity("Animal", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("int") + .UseIdentityColumn(); + + b.Property("MouseId") + .HasColumnType("int"); + + b.HasKey("Id"); + + b.HasIndex("MouseId"); + + b.ToTable("Animal"); + }); + + source.Entity("Cat", b => + { + b.HasBaseType("Animal"); + + b.Property("PreyId") + .HasColumnType("int") + .HasColumnName("PreyId"); + + b.HasIndex("PreyId"); + + b.ToTable("Cats"); + + b.HasData( + new + { + Id = 11, + MouseId = 31 + }); + }); + + source.Entity("Dog", b => + { + b.HasBaseType("Animal"); + + b.Property("PreyId") + .HasColumnType("int") + .HasColumnName("PreyId"); + + b.HasIndex("PreyId"); + + b.ToTable("Dogs"); + + b.HasData( + new + { + Id = 21, + PreyId = 31 + }); + }); + + source.Entity("Mouse", b => + { + b.HasBaseType("Animal"); + + b.ToTable("Mice"); + + b.HasData( + new + { + Id = 31 + }); + }); + + source.Entity("Animal", b => + { + b.HasOne("Mouse", null) + .WithMany() + .HasForeignKey("MouseId"); + }); + + source.Entity("Cat", b => + { + b.HasOne("Animal", null) + .WithMany() + .HasForeignKey("PreyId"); + }); + + source.Entity("Dog", b => + { + b.HasOne("Animal", null) + .WithMany() + .HasForeignKey("PreyId"); + }); + }, + modelBuilder => + { + modelBuilder.Entity( + "Animal", x => + { + x.Property("Id"); + x.Property("MouseId"); + + x.HasOne("Mouse").WithMany().HasForeignKey("MouseId"); + }); + modelBuilder.Entity( + "Cat", x => + { + x.HasBaseType("Animal"); + x.ToTable("Cats"); + x.Property("PreyId").HasColumnName("PreyId"); + + x.HasOne("Animal").WithMany().HasForeignKey("PreyId"); + x.HasData( + new { Id = 11, MouseId = 31 }); + }); + modelBuilder.Entity( + "Dog", x => + { + x.HasBaseType("Animal"); + x.ToTable("Dogs"); + x.Property("PreyId").HasColumnName("PreyId"); + + x.HasOne("Animal").WithMany().HasForeignKey("PreyId"); + x.HasData( + new { Id = 21, PreyId = 31 }); + }); + modelBuilder.Entity( + "Mouse", x => + { + x.HasBaseType("Animal"); + x.ToTable("Mice"); + + x.HasData( + new { Id = 31 }); + }); + }, + upOps => Assert.Empty(upOps), + downOps => Assert.Empty(downOps), + skipSourceConventions: true); + } + protected override TestHelpers TestHelpers => SqlServerTestHelpers.Instance; protected override MigrationsModelDiffer CreateModelDiffer(DbContextOptions options) diff --git a/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderGenericTest.cs b/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderGenericTest.cs index 713cc23d6c3..a4b72de3749 100644 --- a/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderGenericTest.cs +++ b/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderGenericTest.cs @@ -262,13 +262,9 @@ public virtual void Owned_types_use_table_splitting_by_default() Assert.Equal(4, model.GetEntityTypes().Count(e => e.ClrType == typeof(AnotherBookLabel))); Assert.Equal(4, model.GetEntityTypes().Count(e => e.ClrType == typeof(SpecialBookLabel))); - Assert.Equal( - nameof(BookLabel.Id), - bookOwnership1.DeclaringEntityType.FindProperty(nameof(BookLabel.Id)) + Assert.Null(bookOwnership1.DeclaringEntityType.FindProperty(nameof(BookLabel.Id)) .GetColumnName(StoreObjectIdentifier.Table("Label", null))); - Assert.Equal( - nameof(BookLabel.Id), - bookLabel2Ownership1.DeclaringEntityType.FindProperty(nameof(BookLabel.Id)) + Assert.Null(bookLabel2Ownership1.DeclaringEntityType.FindProperty(nameof(BookLabel.Id)) .GetColumnName(StoreObjectIdentifier.Table("AlternateLabel", null))); modelBuilder.Entity().OwnsOne(b => b.Label).ToTable("Label");