Skip to content

Commit

Permalink
Add FK constraints between TPT tables (dotnet#22136)
Browse files Browse the repository at this point in the history 
Only added if there isn't an equivalent FK already in the model
The new FKs are used to sort the TPT update commands instead of the ad-hoc solution
Fix recursive issues exposed by the new FKs

Fixes dotnet#21943
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Aug 20, 2020
1 parent cc65d11 commit e567b4d
Show file tree
Hide file tree
Showing 30 changed files with 333 additions and 243 deletions.
10 changes: 10 additions & 0 deletions src/EFCore.Relational/Metadata/Conventions/EntityTypeHierarchyMappingConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Linq;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;
Expand Down Expand Up @@ -42,6 +43,15 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
&& (tableName != entityType.BaseType.GetTableName()
|| schema != entityType.BaseType.GetSchema()))
{
var pk = entityType.FindPrimaryKey();
if (pk != null
&& !entityType.FindDeclaredForeignKeys(pk.Properties)
.Any(fk => fk.PrincipalKey.IsPrimaryKey() && fk.PrincipalEntityType.IsAssignableFrom(entityType)))
{
entityType.Builder.HasRelationship(entityType.BaseType, pk.Properties, entityType.BaseType.FindPrimaryKey())
.IsUnique(true);
}

nonTphRoots.Add(entityType.GetRootType());
}

Expand Down
14 changes: 13 additions & 1 deletion src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,14 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
in StoreObjectIdentifier storeObject,
int maxLength)
{
foreach (var foreignKey in entityType.GetDeclaredForeignKeys())
foreach (var foreignKey in entityType.GetForeignKeys())
{
if (foreignKey.DeclaringEntityType != entityType
&& StoreObjectIdentifier.Create(foreignKey.DeclaringEntityType, StoreObjectType.Table) == storeObject)
{
continue;
}

var principalTable = foreignKey.PrincipalKey.IsPrimaryKey()
? StoreObjectIdentifier.Create(foreignKey.PrincipalEntityType, StoreObjectType.Table)
: StoreObjectIdentifier.Create(foreignKey.PrincipalKey.DeclaringEntityType, StoreObjectType.Table);
Expand Down Expand Up @@ -408,6 +414,12 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
continue;
}

if (!otherForeignKey.DeclaringEntityType.IsAssignableFrom(entityType)
&& !entityType.IsAssignableFrom(otherForeignKey.DeclaringEntityType))
{
continue;
}

var newOtherForeignKeyName = TryUniquify(otherForeignKey, foreignKeyName, foreignKeys, maxLength);
if (newOtherForeignKeyName != null)
{
Expand Down
8 changes: 5 additions & 3 deletions src/EFCore.Relational/Metadata/Internal/RelationalForeignKeyExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ public static class RelationalForeignKeyExtensions
return false;
}

if (foreignKey.DeleteBehavior != duplicateForeignKey.DeleteBehavior)
var referentialAction = RelationalModel.ToReferentialAction(foreignKey.DeleteBehavior);
var duplicateReferentialAction = RelationalModel.ToReferentialAction(duplicateForeignKey.DeleteBehavior);
if (referentialAction != duplicateReferentialAction)
{
if (shouldThrow)
{
Expand All @@ -151,8 +153,8 @@ public static class RelationalForeignKeyExtensions
duplicateForeignKey.DeclaringEntityType.DisplayName(),
foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(),
foreignKey.GetConstraintName(storeObject, principalTable.Value),
foreignKey.DeleteBehavior,
duplicateForeignKey.DeleteBehavior));
referentialAction,
duplicateReferentialAction));
}

return false;
Expand Down
14 changes: 13 additions & 1 deletion src/EFCore.Relational/Metadata/Internal/RelationalModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,12 @@ private static void PopulateConstraints(Table table)
var storeObject = StoreObjectIdentifier.Table(table.Name, table.Schema);
foreach (var entityTypeMapping in ((ITable)table).EntityTypeMappings)
{
if (!entityTypeMapping.IncludesDerivedTypes
&& entityTypeMapping.EntityType.GetTableMappings().Any(m => m.IncludesDerivedTypes))
{
continue;
}

var entityType = (IConventionEntityType)entityTypeMapping.EntityType;
foreach (var foreignKey in entityType.GetForeignKeys())
{
Expand Down Expand Up @@ -1061,7 +1067,13 @@ private static void PopulateRowInternalForeignKeys(TableBase table)
}
}

private static ReferentialAction ToReferentialAction(DeleteBehavior deleteBehavior)
/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public static ReferentialAction ToReferentialAction(DeleteBehavior deleteBehavior)
{
switch (deleteBehavior)
{
Expand Down
8 changes: 7 additions & 1 deletion src/EFCore.Relational/Query/SqlExpressionFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -948,9 +948,15 @@ bool HasSiblings(IEntityType entityType)
// other dependents.
foreach (var referencingFk in entityType.GetReferencingForeignKeys())
{
if (referencingFk.PrincipalEntityType.IsAssignableFrom(entityType))
{
continue;
}

var otherSelectExpression = new SelectExpression(entityType, this);

var sameTable = table.IsOptional(referencingFk.DeclaringEntityType);
var sameTable = table.EntityTypeMappings.Any(m => m.EntityType == referencingFk.DeclaringEntityType)
&& table.IsOptional(referencingFk.DeclaringEntityType);
AddInnerJoin(
otherSelectExpression, referencingFk,
sameTable ? table : null);
Expand Down
33 changes: 2 additions & 31 deletions src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ public CommandBatchPreparer([NotNull] CommandBatchPreparerDependencies dependenc
var mappings = (IReadOnlyCollection<ITableMapping>)entry.EntityType.GetTableMappings();
var mappingCount = mappings.Count;
ModificationCommand firstCommand = null;
var relatedCommands = mappingCount > 1 ? new List<ModificationCommand>(mappingCount) : null;
foreach (var mapping in mappings)
{
var table = mapping.Table;
Expand Down Expand Up @@ -216,33 +215,12 @@ public CommandBatchPreparer([NotNull] CommandBatchPreparerDependencies dependenc
Check.DebugAssert(firstCommand == null, "firstCommand == null");
firstCommand = command;
}

if (relatedCommands != null)
{
relatedCommands.Add(command);
}
}

if (firstCommand == null)
{
throw new InvalidOperationException(RelationalStrings.ReadonlyEntitySaved(entry.EntityType.DisplayName()));
}

if (relatedCommands != null)
{
if (firstCommand.EntityState == EntityState.Deleted)
{
relatedCommands.Reverse();
}

var previousCommand = relatedCommands[0];
for (var i = 1; i < relatedCommands.Count; i++)
{
var relatedCommand = relatedCommands[i];
relatedCommand.Predecessor = previousCommand;
previousCommand = relatedCommand;
}
}
}

if (sharedTablesCommandsMap != null)
Expand Down Expand Up @@ -569,12 +547,6 @@ private void Format(IIndex index, ModificationCommand source, ModificationComman
{
foreach (var command in commandGraph.Vertices)
{
if (command.Predecessor != null)
{
// This is usually an implicit relationship between TPT rows for the same entity
commandGraph.AddEdge(command.Predecessor, command, null);
}

switch (command.EntityState)
{
case EntityState.Modified:
Expand All @@ -585,9 +557,8 @@ private void Format(IIndex index, ModificationCommand source, ModificationComman
var entry = command.Entries[entryIndex];
foreach (var foreignKey in entry.EntityType.GetForeignKeys())
{
var constraints = foreignKey.GetMappedConstraints();

if (!constraints.Any()
if (!foreignKey.GetMappedConstraints()
.Any(c => c.Table.Name == command.TableName && c.Table.Schema == command.Schema)
|| (command.EntityState == EntityState.Modified
&& !foreignKey.Properties.Any(p => entry.IsModified(p))))
{
Expand Down
5 changes: 0 additions & 5 deletions src/EFCore.Relational/Update/ModificationCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,6 @@ public class ModificationCommand
/// </summary>
public virtual string Schema { get; }

/// <summary>
/// The command that needs to be executed before this one.
/// </summary>
public virtual ModificationCommand Predecessor { get; [param: CanBeNull] set; }

/// <summary>
/// The <see cref="IUpdateEntry" />s that represent the entities that are mapped to the row
/// to update.
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore.SqlServer/Extensions/SqlServerPropertyExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Linq;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata;
Expand Down Expand Up @@ -335,7 +336,7 @@ public static SqlServerValueGenerationStrategy GetValueGenerationStrategy([NotNu
}

if (property.ValueGenerated != ValueGenerated.OnAdd
|| property.IsForeignKey()
|| property.GetContainingForeignKeys().Any(fk => !fk.IsBaseLinking())
|| property.GetDefaultValue(storeObject) != null
|| property.GetDefaultValueSql(storeObject) != null
|| property.GetComputedColumnSql(storeObject) != null)
Expand Down
5 changes: 5 additions & 0 deletions src/EFCore.SqlServer/Metadata/Conventions/SqlServerOnDeleteConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ protected override DeleteBehavior GetTargetDeleteBehavior(IConventionForeignKey
return deleteBehavior;
}

if (foreignKey.IsBaseLinking())
{
return DeleteBehavior.ClientCascade;
}

var selfReferencingSkipNavigation = foreignKey.GetReferencingSkipNavigations()
.FirstOrDefault(s => s.Inverse != null && s.TargetEntityType == s.DeclaringEntityType);
if (selfReferencingSkipNavigation == null)
Expand Down
5 changes: 5 additions & 0 deletions src/EFCore/ChangeTracking/Internal/StateManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,11 @@ public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumer
foreach (InternalEntityEntry dependent in (GetDependentsFromNavigation(entry, fk)
?? GetDependents(entry, fk)).ToList())
{
if (dependent.SharedIdentityEntry == entry)
{
continue;
}

changeDetector?.DetectChanges(dependent);

if (dependent.EntityState != EntityState.Deleted
Expand Down
13 changes: 13 additions & 0 deletions src/EFCore/Extensions/ForeignKeyExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Linq;
using System.Text;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.ChangeTracking;
Expand Down Expand Up @@ -71,6 +72,18 @@ public static IEntityType GetRelatedEntityType([NotNull] this IForeignKey foreig
public static INavigation GetNavigation([NotNull] this IForeignKey foreignKey, bool pointsToPrincipal)
=> pointsToPrincipal ? foreignKey.DependentToPrincipal : foreignKey.PrincipalToDependent;

/// <summary>
/// Returns a value indicating whether the foreign key is defined on the primary key and pointing to the same primary key.
/// </summary>
/// <param name="foreignKey"> The foreign key. </param>
/// <returns> A value indicating whether the foreign key is defined on the primary key and pointing to the same primary key. </returns>
public static bool IsBaseLinking([NotNull] this IForeignKey foreignKey)
{
var primaryKey = foreignKey.DeclaringEntityType.FindPrimaryKey();
return primaryKey == foreignKey.PrincipalKey
&& foreignKey.Properties.SequenceEqual(primaryKey.Properties);
}

/// <summary>
/// <para>
/// Creates a human-readable representation of the given metadata.
Expand Down
7 changes: 4 additions & 3 deletions src/EFCore/Infrastructure/ModelValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ private Type FindCandidateNavigationPropertyType(PropertyInfo propertyInfo)
var principalType = foreignKey.PrincipalEntityType;
if (!foreignKey.PrincipalKey.IsPrimaryKey()
|| !unvalidatedEntityTypes.Contains(principalType)
|| foreignKey.PrincipalEntityType == entityType
|| foreignKey.PrincipalEntityType.IsAssignableFrom(entityType)
|| !PropertyListComparer.Instance.Equals(foreignKey.Properties, primaryKey.Properties))
{
continue;
Expand Down Expand Up @@ -770,12 +770,13 @@ private bool Contains(IForeignKey inheritedFk, IForeignKey derivedFk)
foreach (var declaredForeignKey in entityType.GetDeclaredForeignKeys())
{
if (declaredForeignKey.PrincipalEntityType == declaredForeignKey.DeclaringEntityType
&& PropertyListComparer.Instance.Equals(declaredForeignKey.PrincipalKey.Properties, declaredForeignKey.Properties))
&& declaredForeignKey.PrincipalKey.Properties.SequenceEqual(declaredForeignKey.Properties))
{
logger.RedundantForeignKeyWarning(declaredForeignKey);
}

if (entityType.BaseType == null)
if (entityType.BaseType == null
|| declaredForeignKey.IsBaseLinking())
{
continue;
}
Expand Down
4 changes: 3 additions & 1 deletion src/EFCore/Metadata/Conventions/ForeignKeyPropertyDiscoveryConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ public ForeignKeyPropertyDiscoveryConvention([NotNull] ProviderConventionSetBuil
|| foreignKey.DeclaringEntityType.IsKeyless
|| (!foreignKey.IsUnique && !ConfigurationSource.Convention.Overrides(foreignKey.GetIsUniqueConfigurationSource()))
|| foreignKey.PrincipalToDependent?.IsCollection == true
|| foreignKey.DeclaringEntityType.FindOwnership() != null)
|| foreignKey.DeclaringEntityType.FindOwnership() != null
|| (foreignKey.IsBaseLinking()
&& foreignKey.PrincipalEntityType.IsAssignableFrom(foreignKey.DeclaringEntityType)))
{
relationshipBuilder = relationshipBuilder.HasEntityTypes(
foreignKey.PrincipalEntityType, foreignKey.DeclaringEntityType);
Expand Down
13 changes: 9 additions & 4 deletions src/EFCore/Metadata/Conventions/ValueGenerationConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,13 @@ public ValueGenerationConvention([NotNull] ProviderConventionSetBuilderDependenc
public virtual void ProcessForeignKeyAdded(
IConventionForeignKeyBuilder relationshipBuilder, IConventionContext<IConventionForeignKeyBuilder> context)
{
foreach (var property in relationshipBuilder.Metadata.Properties)
var foreignKey = relationshipBuilder.Metadata;
if (!foreignKey.IsBaseLinking())
{
property.Builder.ValueGenerated(ValueGenerated.Never);
foreach (var property in foreignKey.Properties)
{
property.Builder.ValueGenerated(ValueGenerated.Never);
}
}
}

Expand Down Expand Up @@ -82,7 +86,8 @@ public ValueGenerationConvention([NotNull] ProviderConventionSetBuilderDependenc
{
OnForeignKeyRemoved(oldDependentProperties);

if (relationshipBuilder.Metadata.Builder != null)
if (relationshipBuilder.Metadata.Builder != null
&& !foreignKey.IsBaseLinking())
{
foreach (var property in foreignKey.Properties)
{
Expand Down Expand Up @@ -179,7 +184,7 @@ private void OnForeignKeyRemoved(IReadOnlyList<IConventionProperty> foreignKeyPr
/// <param name="property"> The property. </param>
/// <returns> The store value generation strategy to set for the given property. </returns>
public static ValueGenerated? GetValueGenerated([NotNull] IProperty property)
=> !property.IsForeignKey()
=> !property.GetContainingForeignKeys().Any(fk => !fk.IsBaseLinking())
&& ShouldHaveGeneratedProperty(property.FindContainingPrimaryKey())
&& CanBeGenerated(property)
? ValueGenerated.OnAdd
Expand Down
9 changes: 0 additions & 9 deletions src/EFCore/Metadata/Internal/ForeignKeyExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,6 @@ public static class ForeignKeyExtensions
public static bool IsSelfReferencing([NotNull] this IForeignKey foreignKey)
=> foreignKey.DeclaringEntityType == foreignKey.PrincipalEntityType;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public static bool IsSelfPrimaryKeyReferencing([NotNull] this IForeignKey foreignKey)
=> foreignKey.DeclaringEntityType.FindPrimaryKey() == foreignKey.PrincipalKey;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down
Loading

0 comments on commit e567b4d

Please sign in to comment.