Skip to content

Commit

Permalink
Throw if table-sharing principal is in TPC and has derived types or t…
Browse files Browse the repository at this point in the history
…he dependent doesn't use TPH

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
  • Loading branch information
AndriySvyryd committed May 11, 2022
1 parent b4f8800 commit a6d82d1
Show file tree
Hide file tree
Showing 14 changed files with 2,942 additions and 290 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
60 changes: 57 additions & 3 deletions src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand All @@ -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>
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/EFCore.Relational/Metadata/ITable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Metadata/Internal/DbFunction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Metadata/RuntimeDbFunction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 17 additions & 1 deletion src/EFCore.Relational/Properties/RelationalStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ private IRelationalModel CreateTestModel(bool mapToTables = false, bool mapToVie

if (mapToTables)
{
cb.ToTable("AbstractBase");
cb.ToTable(t => { });
}
}

Expand Down
Loading

0 comments on commit a6d82d1

Please sign in to comment.