Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add FK constraints between TPT tables #22136

Merged
1 commit merged into from
Aug 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,14 @@ private void TryUniquifyForeignKeyNames(
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 @@ private void TryUniquifyForeignKeyNames(
continue;
}

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

var newOtherForeignKeyName = TryUniquify(otherForeignKey, foreignKeyName, foreignKeys, maxLength);
if (newOtherForeignKeyName != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ public static bool AreCompatible(
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 bool AreCompatible(
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -948,9 +948,15 @@ private void AddOptionalDependentConditions(
// 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
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ protected virtual IEnumerable<ModificationCommand> CreateModificationCommands(
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 @@ protected virtual IEnumerable<ModificationCommand> CreateModificationCommands(
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 AddForeignKeyEdges(
{
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 AddForeignKeyEdges(
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
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,6 @@ public 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
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(
}

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
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
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
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dotnet/efteam - For new API review.

(looks fine to me)

{
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
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ protected virtual void ValidateNoCycles(
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 @@ protected virtual void ValidateForeignKeys(
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
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ private IConventionForeignKeyBuilder DiscoverProperties(
|| 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
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 virtual void ProcessForeignKeyPropertiesChanged(
{
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 @@ public virtual void ProcessEntityTypeBaseTypeChanged(
/// <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
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
8 changes: 4 additions & 4 deletions src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2796,7 +2796,7 @@ private InternalForeignKeyBuilder HasRelationship(
principalKey: null,
propertyBaseName: navigationProperty?.GetSimpleMemberName(),
required: required,
configurationSource: configurationSource);
configurationSource);
}
else
{
Expand All @@ -2812,7 +2812,7 @@ private InternalForeignKeyBuilder HasRelationship(
principalKey: null,
propertyBaseName: navigationProperty?.GetSimpleMemberName(),
required: null,
configurationSource: configurationSource);
configurationSource);
}

relationship = newRelationship;
Expand Down Expand Up @@ -3884,8 +3884,8 @@ private IReadOnlyList<Property> CreateUniqueProperties(
using var principalPropertyTypesEnumerator = principalPropertyTypes.GetEnumerator();
for (var i = 0;
i < propertyCount
&& principalPropertyNamesEnumerator.MoveNext()
&& principalPropertyTypesEnumerator.MoveNext();
&& principalPropertyNamesEnumerator.MoveNext()
&& principalPropertyTypesEnumerator.MoveNext();
i++)
{
var keyPropertyName = principalPropertyNamesEnumerator.Current;
Expand Down
Loading