Throw if an entity type in table-sharing uses TPC

Don't configure table name for abstract types in TPC that have a corresponding DbSet property Make ToTable(b => {}) map the entity type to the default table Fixes #3170 Fixes #27608 Fixes #27945 Fixes #27947
dotnet · May 11, 2022 · 979c898 · 979c898
1 parent f6988cd
commit 979c898
Show file tree

Hide file tree

Showing 14 changed files with 2,942 additions and 290 deletions.
diff --git a/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs b/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs
@@ -187,6 +187,12 @@ public static EntityTypeBuilder<TEntity> ToTable<TEntity>(
     {
         Check.NotNull(buildAction, nameof(buildAction));
 
+        var entityTypeConventionBuilder = entityTypeBuilder.GetInfrastructure();
+        if (entityTypeConventionBuilder.Metadata[RelationalAnnotationNames.TableName] == null)
+        {
+            entityTypeConventionBuilder.ToTable(entityTypeBuilder.Metadata.GetDefaultTableName());
+        }
+
         buildAction(new TableBuilder<TEntity>(null, null, entityTypeBuilder));
 
         return entityTypeBuilder;

diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs
@@ -1364,17 +1364,26 @@ protected override void ValidateInheritanceMapping(
             }
             else
             {
-                var pk = entityType.FindPrimaryKey();
+                var primaryKey = entityType.FindPrimaryKey();
                 if (mappingStrategy == RelationalAnnotationNames.TpcMappingStrategy)
                 {
-                    var storeGeneratedProperty = pk?.Properties.FirstOrDefault(p => (p.ValueGenerated & ValueGenerated.OnAdd) != 0);
+                    var storeGeneratedProperty = primaryKey?.Properties.FirstOrDefault(p => (p.ValueGenerated & ValueGenerated.OnAdd) != 0);
                     if (storeGeneratedProperty != null
                         && entityType.GetTableName() != null)
                     {
                         logger.TpcStoreGeneratedIdentityWarning(storeGeneratedProperty);
                     }
+
+                    if (entityType.GetDirectlyDerivedTypes().Any())
+                    {
+                        foreach (var fk in entityType.GetDeclaredReferencingForeignKeys())
+                        {
+                            AssertNonInternal(fk, StoreObjectType.View);
+                            AssertNonInternal(fk, StoreObjectType.Table);
+                        }
+                    }
                 }
-                else if (pk == null)
+                else if (primaryKey == null)
                 {
                     throw new InvalidOperationException(
                        RelationalStrings.KeylessMappingStrategy(mappingStrategy ?? RelationalAnnotationNames.TptMappingStrategy, entityType.DisplayName()));
@@ -1384,6 +1393,31 @@ protected override void ValidateInheritanceMapping(
                 ValidateNonTphMapping(entityType, forTables: true);
             }
         }
+
+        static void AssertNonInternal(IForeignKey foreignKey, StoreObjectType storeObjectType)
+        {
+            if (!foreignKey.PrincipalKey.IsPrimaryKey()
+                || foreignKey.PrincipalEntityType == foreignKey.DeclaringEntityType
+                || !foreignKey.IsUnique
+#pragma warning disable EF1001 // Internal EF Core API usage.
+                || !PropertyListComparer.Instance.Equals(foreignKey.Properties, foreignKey.PrincipalKey.Properties))
+#pragma warning restore EF1001 // Internal EF Core API usage.
+            {
+                return;
+            }
+
+            var storeObjectId = StoreObjectIdentifier.Create(foreignKey.DeclaringEntityType, storeObjectType);
+            if (storeObjectId == null
+                || storeObjectId != StoreObjectIdentifier.Create(foreignKey.PrincipalEntityType, storeObjectType))
+            {
+                return;
+            }
+
+            throw new InvalidOperationException(RelationalStrings.TpcTableSharing(
+                foreignKey.DeclaringEntityType.DisplayName(),
+                storeObjectId.Value.DisplayName(),
+                foreignKey.PrincipalEntityType.DisplayName()));
+        }
     }
 
     /// <summary>
@@ -1429,6 +1463,26 @@ private static void ValidateNonTphMapping(IEntityType rootEntityType, bool forTa
 
             derivedTypes[(name, schema)] = entityType;
         }
+
+        var storeObject = StoreObjectIdentifier.Create(rootEntityType, forTables ? StoreObjectType.Table : StoreObjectType.View);
+        if (storeObject == null)
+        {
+            return;
+        }
+
+        var internalForeignKey = rootEntityType.FindRowInternalForeignKeys(storeObject.Value).FirstOrDefault();
+        if (internalForeignKey != null
+            && derivedTypes.Count > 1
+            && rootEntityType.GetMappingStrategy() == RelationalAnnotationNames.TpcMappingStrategy)
+        {
+            var derivedTypePair = derivedTypes.First(kv => kv.Value != rootEntityType);
+            var (derivedName, derivedSchema) = derivedTypePair.Key;
+            throw new InvalidOperationException(RelationalStrings.TpcTableSharingDependent(
+                rootEntityType.DisplayName(),
+                storeObject.Value.DisplayName(),
+                derivedTypePair.Value.DisplayName(),
+                derivedSchema == null ? derivedName : $"{derivedSchema}.{derivedName}"));
+        }
     }
 
     private static void ValidateTphMapping(IEntityType rootEntityType, bool forTables)

diff --git a/src/EFCore.Relational/Metadata/Conventions/TableNameFromDbSetConvention.cs b/src/EFCore.Relational/Metadata/Conventions/TableNameFromDbSetConvention.cs
@@ -9,7 +9,10 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
 /// <remarks>
 ///     See <see href="https://aka.ms/efcore-docs-conventions">Model building conventions</see> for more information and examples.
 /// </remarks>
-public class TableNameFromDbSetConvention : IEntityTypeAddedConvention, IEntityTypeBaseTypeChangedConvention, IModelFinalizingConvention
+public class TableNameFromDbSetConvention :
+    IEntityTypeAddedConvention,
+    IEntityTypeBaseTypeChangedConvention,
+    IModelFinalizingConvention
 {
     private readonly IDictionary<Type, string> _sets;
 
@@ -115,11 +118,20 @@ public virtual void ProcessModelFinalizing(
         foreach (var entityType in modelBuilder.Metadata.GetEntityTypes())
         {
             if (entityType.GetTableName() != null
-                && entityType.GetViewNameConfigurationSource() != null
                 && _sets.ContainsKey(entityType.ClrType))
             {
-                // Undo the convention change if the entity type is mapped to a view
-                entityType.Builder.HasNoAnnotation(RelationalAnnotationNames.TableName);
+                if (entityType.GetViewNameConfigurationSource() != null)
+                {
+                    // Undo the convention change if the entity type is mapped to a view
+                    entityType.Builder.HasNoAnnotation(RelationalAnnotationNames.TableName);
+                }
+
+                if (entityType.GetMappingStrategy() == RelationalAnnotationNames.TpcMappingStrategy
+                    && entityType.IsAbstract())
+                {
+                    // Undo the convention change if the entity type is mapped using TPC
+                    entityType.Builder.HasNoAnnotation(RelationalAnnotationNames.TableName);
+                }
             }
         }
     }

diff --git a/src/EFCore.Relational/Metadata/ITable.cs b/src/EFCore.Relational/Metadata/ITable.cs
@@ -108,8 +108,9 @@ string ToDebugString(MetadataDebugStringOptions options = MetadataDebugStringOpt
         }
 
         builder.Append(Name);
-
-        if (IsExcludedFromMigrations)
+
+        if (EntityTypeMappings.First().EntityType is not RuntimeEntityType
+            && IsExcludedFromMigrations)
         {
             builder.Append(" ExcludedFromMigrations");
         }

diff --git a/src/EFCore.Relational/Metadata/Internal/DbFunction.cs b/src/EFCore.Relational/Metadata/Internal/DbFunction.cs
@@ -538,7 +538,7 @@ public virtual RelationalTypeMapping? TypeMapping
                     var relationalTypeMappingSource =
                         (IRelationalTypeMappingSource)((IModel)dbFunction.Model).GetModelDependencies().TypeMappingSource;
                     return !string.IsNullOrEmpty(dbFunction._storeType)
-                        ? relationalTypeMappingSource.FindMapping(dbFunction._storeType)!
+                        ? relationalTypeMappingSource.FindMapping(dbFunction.ReturnType, dbFunction._storeType)!
                         : relationalTypeMappingSource.FindMapping(dbFunction.ReturnType, (IModel)dbFunction.Model)!;
                 })
             : _typeMapping;

diff --git a/src/EFCore.Relational/Metadata/RuntimeDbFunction.cs b/src/EFCore.Relational/Metadata/RuntimeDbFunction.cs
@@ -98,7 +98,7 @@ public virtual RelationalTypeMapping? TypeMapping
                     var relationalTypeMappingSource =
                         (IRelationalTypeMappingSource)((IModel)dbFunction.Model).GetModelDependencies().TypeMappingSource;
                     return !string.IsNullOrEmpty(dbFunction._storeType)
-                        ? relationalTypeMappingSource.FindMapping(dbFunction._storeType)!
+                        ? relationalTypeMappingSource.FindMapping(dbFunction._returnType, dbFunction._storeType)!
                         : relationalTypeMappingSource.FindMapping(dbFunction._returnType, dbFunction.Model)!;
                 })
             : _typeMapping;

diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs
diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx
@@ -838,6 +838,12 @@
   <data name="TooFewReaderFields" xml:space="preserve">
     <value>The underlying reader doesn't have as many fields as expected. Expected: {expected}, actual: {actual}.</value>
   </data>
+  <data name="TpcTableSharing" xml:space="preserve">
+    <value>The entity type '{dependentType}' is mapped to '{storeObject}'. However the principal entity type '{principalEntityType}' is also mapped to '{storeObject}' and it's using the TPC mapping strategy. Only leaf entity types in a TPC hierarchy can use table-sharing.</value>
+  </data>
+  <data name="TpcTableSharingDependent" xml:space="preserve">
+    <value>The entity type '{dependentType}' is mapped to '{storeObject}'. However one of its derived types '{derivedType}' is mapped to '{otherStoreObject}'. Hierarchies using table-sharing cannot be mapped using the TPC mapping strategy.</value>
+  </data>
   <data name="TphTableMismatch" xml:space="preserve">
     <value>'{entityType}' is mapped to the table '{table}' while '{otherEntityType}' is mapped to the table '{otherTable}'. Map all the entity types in the hierarchy to the same table, or remove the discriminator and map them all to different tables. See https://go.microsoft.com/fwlink/?linkid=2130430 for more information.</value>
   </data>
@@ -916,4 +922,4 @@
   <data name="VisitChildrenMustBeOverridden" xml:space="preserve">
     <value>'VisitChildren' must be overridden in the class deriving from 'SqlExpression'.</value>
   </data>
-</root>
+</root>
diff --git a/test/EFCore.Relational.Specification.Tests/Query/TPCGearsOfWarQueryRelationalFixture.cs b/test/EFCore.Relational.Specification.Tests/Query/TPCGearsOfWarQueryRelationalFixture.cs
@@ -31,9 +31,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
         modelBuilder.Entity<Faction>().UseTpcMappingStrategy();
         modelBuilder.Entity<LocustLeader>().UseTpcMappingStrategy();
 
-        // Work-around for issue#27947
-        modelBuilder.Entity<Faction>().ToTable((string)null);
-
         modelBuilder.Entity<Gear>().ToTable("Gears");
         modelBuilder.Entity<Officer>().ToTable("Officers");
 

diff --git a/test/EFCore.Relational.Specification.Tests/Query/TPCInheritanceQueryFixture.cs b/test/EFCore.Relational.Specification.Tests/Query/TPCInheritanceQueryFixture.cs
@@ -25,10 +25,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
         modelBuilder.Entity<Animal>().UseTpcMappingStrategy();
         modelBuilder.Entity<Drink>().UseTpcMappingStrategy();
 
-        // Work-around for issue#27947
-        modelBuilder.Entity<Animal>().ToTable((string)null);
-        modelBuilder.Entity<Plant>().ToTable((string)null);
-
         modelBuilder.Entity<Flower>().ToTable("Flowers");
         modelBuilder.Entity<Rose>().ToTable("Roses");
         modelBuilder.Entity<Daisy>().ToTable("Daisies");

diff --git a/test/EFCore.Relational.Specification.Tests/Query/TPCRelationshipsQueryRelationalFixture.cs b/test/EFCore.Relational.Specification.Tests/Query/TPCRelationshipsQueryRelationalFixture.cs
@@ -11,14 +11,16 @@ public abstract class TPCRelationshipsQueryRelationalFixture : InheritanceRelati
 
     public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
         => base.AddOptions(builder).ConfigureWarnings(
-            w =>
-                w.Log(RelationalEventId.ForeignKeyTpcPrincipalWarning));
+            w => w.Log(RelationalEventId.ForeignKeyTpcPrincipalWarning));
 
     protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
     {
         base.OnModelCreating(modelBuilder, context);
 
-        modelBuilder.Entity<BaseInheritanceRelationshipEntity>().UseTpcMappingStrategy();
+        modelBuilder.Entity<BaseInheritanceRelationshipEntity>().UseTpcMappingStrategy()
+            // Table-sharing is not supported in TPC mapping
+            .OwnsMany(e => e.OwnedCollectionOnBase, e => e.ToTable("OwnedCollections"))
+            .OwnsOne(e => e.OwnedReferenceOnBase, e => e.ToTable("OwnedReferences"));
         modelBuilder.Entity<BaseReferenceOnBase>().UseTpcMappingStrategy();
         modelBuilder.Entity<BaseCollectionOnBase>().UseTpcMappingStrategy();
         modelBuilder.Entity<BaseReferenceOnDerived>().UseTpcMappingStrategy();

diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs
@@ -1908,6 +1908,22 @@ public virtual void Detects_store_generated_PK_in_TPC()
             LogLevel.Warning);
     }
 
+    [ConditionalFact]
+    public virtual void Detects_table_sharing_with_TPC_on_dependent()
+    {
+        var modelBuilder = CreateConventionalModelBuilder();
+
+        modelBuilder.Entity<Animal>()
+            .ToTable("Animal")
+            .HasOne(c => c.FavoritePerson).WithOne().HasForeignKey<Person>(c => c.Id);
+
+        modelBuilder.Entity<Person>().ToTable("Animal").UseTpcMappingStrategy();
+        modelBuilder.Entity<Employee>().ToTable("Employee");
+
+        VerifyError(RelationalStrings.TpcTableSharingDependent("Person", "Animal", "Employee", "Employee"),
+            modelBuilder);
+    }
+
     [ConditionalFact]
     public virtual void Passes_on_valid_view_sharing_with_TPC()
     {
@@ -1929,6 +1945,24 @@ public virtual void Passes_on_valid_view_sharing_with_TPC()
 
         Validate(modelBuilder);
     }
+
+    [ConditionalFact]
+    public virtual void Detects_view_sharing_on_base_with_TPC()
+    {
+        var modelBuilder = CreateConventionalModelBuilder();
+
+        modelBuilder.Entity<Animal>()
+            .UseTpcMappingStrategy()
+            .ToView("Animal")
+            .HasOne(c => c.FavoritePerson).WithOne().HasForeignKey<Person>(c => c.Id);
+
+        modelBuilder.Entity<Cat>(x => x.ToView("Cat"));
+
+        modelBuilder.Entity<Person>().ToView("Animal");
+
+        VerifyError(RelationalStrings.TpcTableSharing("Person", "Animal", "Animal"),
+            modelBuilder);
+    }
 
     [ConditionalFact]
     public virtual void Detects_linking_relationship_on_derived_type_in_TPC()

diff --git a/test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs b/test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs
@@ -919,7 +919,7 @@ private IRelationalModel CreateTestModel(bool mapToTables = false, bool mapToVie
 
                         if (mapToTables)
                         {
-                            cb.ToTable("AbstractBase");
+                            cb.ToTable(t => { });
                         }
                     }