Skip to content

Commit

Permalink
Don't keep join entity type around because a ForeignKeyAttribute used…
Browse files Browse the repository at this point in the history
… to reference it. Instead, remove it and reattach the configuration.

Also, reuse the FKs that already were configured instead of creating new ones.

Fixes #27990
Fixes #26555
  • Loading branch information
AndriySvyryd committed Dec 19, 2023
1 parent 209865e commit cf303c8
Show file tree
Hide file tree
Showing 13 changed files with 419 additions and 57 deletions.
42 changes: 35 additions & 7 deletions src/EFCore/Metadata/Builders/CollectionCollectionBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ protected virtual IMutableEntityType UsingEntity(

var existingJoinEntityType = (EntityType?)(LeftNavigation.JoinEntityType ?? RightNavigation.JoinEntityType);
EntityType? newJoinEntityType = null;
EntityType.Snapshot? entityTypeSnapshot = null;
if (existingJoinEntityType != null)
{
if ((joinEntityType == null || existingJoinEntityType.ClrType == joinEntityType)
Expand All @@ -416,7 +417,13 @@ protected virtual IMutableEntityType UsingEntity(
}
else
{
ModelBuilder.RemoveImplicitJoinEntity(existingJoinEntityType);
ModelBuilder.RemoveImplicitJoinEntity(existingJoinEntityType, configurationSource: ConfigurationSource.DataAnnotation);

entityTypeSnapshot = InternalEntityTypeBuilder.DetachAllMembers(existingJoinEntityType);
if (entityTypeSnapshot != null)
{
ModelBuilder.HasNoEntityType(existingJoinEntityType, ConfigurationSource.Explicit);
}
}
}

Expand All @@ -440,15 +447,36 @@ protected virtual IMutableEntityType UsingEntity(
}
}

var rightForeignKey = configureRight != null
? configureRight(newJoinEntityType)
: GetOrCreateSkipNavigationForeignKey((SkipNavigation)RightNavigation, newJoinEntityType);
var leftForeignKey = configureLeft != null
? configureLeft(newJoinEntityType)
: GetOrCreateSkipNavigationForeignKey((SkipNavigation)LeftNavigation, newJoinEntityType);
if (entityTypeSnapshot != null)
{
entityTypeSnapshot.Attach(newJoinEntityType.Builder);
}

IMutableForeignKey? rightForeignKey;
if (configureRight != null)
{
newJoinEntityType.SetAnnotation(CoreAnnotationNames.SkipNavigationBeingConfigured, RightNavigation);
rightForeignKey = configureRight(newJoinEntityType);
newJoinEntityType.RemoveAnnotation(CoreAnnotationNames.SkipNavigationBeingConfigured);
}
else
{
rightForeignKey = GetOrCreateSkipNavigationForeignKey((SkipNavigation)RightNavigation, newJoinEntityType);
}
((SkipNavigation)RightNavigation).Builder
.HasForeignKey((ForeignKey)rightForeignKey, ConfigurationSource.Explicit);

IMutableForeignKey? leftForeignKey;
if (configureLeft != null)
{
newJoinEntityType.SetAnnotation(CoreAnnotationNames.SkipNavigationBeingConfigured, LeftNavigation);
leftForeignKey = configureLeft(newJoinEntityType);
newJoinEntityType.RemoveAnnotation(CoreAnnotationNames.SkipNavigationBeingConfigured);
}
else
{
leftForeignKey = GetOrCreateSkipNavigationForeignKey((SkipNavigation)LeftNavigation, newJoinEntityType);
}
((SkipNavigation)LeftNavigation).Builder
.HasForeignKey((ForeignKey)leftForeignKey, ConfigurationSource.Explicit);

Expand Down
17 changes: 13 additions & 4 deletions src/EFCore/Metadata/Builders/EntityTypeBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1055,8 +1055,7 @@ public virtual ReferenceNavigationBuilder HasOne(
Check.NullButNotEmpty(navigationName, nameof(navigationName));

var relatedEntityType = FindRelatedEntityType(relatedTypeName, navigationName);
var foreignKey = HasOneBuilder(
MemberIdentity.Create(navigationName), relatedEntityType);
var foreignKey = HasOneBuilder(MemberIdentity.Create(navigationName), relatedEntityType);

return new ReferenceNavigationBuilder(
Builder.Metadata,
Expand Down Expand Up @@ -1098,8 +1097,7 @@ public virtual ReferenceNavigationBuilder HasOne(
Check.NullButNotEmpty(navigationName, nameof(navigationName));

var relatedEntityType = FindRelatedEntityType(relatedType, navigationName);
var foreignKey = HasOneBuilder(
MemberIdentity.Create(navigationName), relatedEntityType);
var foreignKey = HasOneBuilder(MemberIdentity.Create(navigationName), relatedEntityType);

return new ReferenceNavigationBuilder(
Builder.Metadata,
Expand Down Expand Up @@ -1145,6 +1143,17 @@ protected virtual ForeignKey HasOneBuilder(
MemberIdentity navigationId,
EntityType relatedEntityType)
{
if (Metadata[CoreAnnotationNames.SkipNavigationBeingConfigured] is SkipNavigation skipNavigation
&& skipNavigation.DeclaringEntityType == relatedEntityType
&& skipNavigation.ForeignKey?.DeclaringEntityType == Builder.Metadata)
{
return navigationId.MemberInfo != null
? skipNavigation.ForeignKey.Builder.HasNavigation(navigationId.MemberInfo, pointsToPrincipal: true, ConfigurationSource.Explicit)
!.Metadata
: skipNavigation.ForeignKey.Builder.HasNavigation(navigationId.Name, pointsToPrincipal: true, ConfigurationSource.Explicit)
!.Metadata;
}

ForeignKey foreignKey;
if (navigationId.MemberInfo != null)
{
Expand Down
6 changes: 2 additions & 4 deletions src/EFCore/Metadata/Builders/EntityTypeBuilder`.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1336,8 +1336,7 @@ public virtual ReferenceNavigationBuilder<TEntity, TRelatedEntity> HasOne
where TRelatedEntity : class
{
var relatedEntityType = FindRelatedEntityType(typeof(TRelatedEntity), navigationName);
var foreignKey = HasOneBuilder(
MemberIdentity.Create(navigationName), relatedEntityType);
var foreignKey = HasOneBuilder(MemberIdentity.Create(navigationName), relatedEntityType);

return new ReferenceNavigationBuilder<TEntity, TRelatedEntity>(
Builder.Metadata,
Expand Down Expand Up @@ -1381,8 +1380,7 @@ public virtual ReferenceNavigationBuilder<TEntity, TRelatedEntity> HasOne
{
var navigationMember = navigationExpression?.GetMemberAccess();
var relatedEntityType = FindRelatedEntityType(typeof(TRelatedEntity), navigationMember?.GetSimpleMemberName());
var foreignKey = HasOneBuilder(
MemberIdentity.Create(navigationMember), relatedEntityType);
var foreignKey = HasOneBuilder(MemberIdentity.Create(navigationMember), relatedEntityType);

return new ReferenceNavigationBuilder<TEntity, TRelatedEntity>(
Builder.Metadata,
Expand Down
38 changes: 32 additions & 6 deletions src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -861,14 +861,13 @@ public virtual ReferenceNavigationBuilder HasOne(
Check.NullButNotEmpty(navigationName, nameof(navigationName));

var relatedEntityType = FindRelatedEntityType(relatedTypeName, navigationName);
var foreignKey = HasOneBuilder(MemberIdentity.Create(navigationName), relatedEntityType);

return new ReferenceNavigationBuilder(
DependentEntityType,
relatedEntityType,
navigationName,
DependentEntityType.Builder.HasRelationship(
relatedEntityType, navigationName, ConfigurationSource.Explicit,
targetIsPrincipal: DependentEntityType == relatedEntityType ? true : null)!.Metadata);
foreignKey);
}

/// <summary>
Expand Down Expand Up @@ -936,14 +935,41 @@ public virtual ReferenceNavigationBuilder HasOne(
Check.NullButNotEmpty(navigationName, nameof(navigationName));

var relatedEntityType = FindRelatedEntityType(relatedType, navigationName);
var foreignKey = HasOneBuilder(MemberIdentity.Create(navigationName), relatedEntityType);

return new ReferenceNavigationBuilder(
DependentEntityType,
relatedEntityType,
navigationName,
DependentEntityType.Builder.HasRelationship(
relatedEntityType, navigationName, ConfigurationSource.Explicit,
targetIsPrincipal: DependentEntityType == relatedEntityType ? true : null)!.Metadata);
foreignKey);
}

/// <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>
[EntityFrameworkInternal]
protected virtual ForeignKey HasOneBuilder(
MemberIdentity navigationId,
EntityType relatedEntityType)
{
ForeignKey foreignKey;
if (navigationId.MemberInfo != null)
{
foreignKey = DependentEntityType.Builder.HasRelationship(
relatedEntityType, navigationId.MemberInfo, ConfigurationSource.Explicit,
targetIsPrincipal: DependentEntityType == relatedEntityType ? true : null)!.Metadata;
}
else
{
foreignKey = DependentEntityType.Builder.HasRelationship(
relatedEntityType, navigationId.Name, ConfigurationSource.Explicit,
targetIsPrincipal: DependentEntityType == relatedEntityType ? true : null)!.Metadata;
}

return foreignKey;
}

/// <summary>
Expand Down
16 changes: 7 additions & 9 deletions src/EFCore/Metadata/Builders/OwnedNavigationBuilder`.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1104,14 +1104,13 @@ public virtual ReferenceNavigationBuilder<TDependentEntity, TNewRelatedEntity> H
where TNewRelatedEntity : class
{
var relatedEntityType = FindRelatedEntityType(typeof(TNewRelatedEntity), navigationName);
var foreignKey = HasOneBuilder(MemberIdentity.Create(navigationName), relatedEntityType);

return new ReferenceNavigationBuilder<TDependentEntity, TNewRelatedEntity>(
DependentEntityType,
relatedEntityType,
navigationName,
DependentEntityType.Builder.HasRelationship(
relatedEntityType, navigationName, ConfigurationSource.Explicit,
targetIsPrincipal: DependentEntityType == relatedEntityType ? true : null)!.Metadata);
foreignKey);
}

/// <summary>
Expand Down Expand Up @@ -1147,16 +1146,15 @@ public virtual ReferenceNavigationBuilder<TDependentEntity, TNewRelatedEntity> H
Expression<Func<TDependentEntity, TNewRelatedEntity?>>? navigationExpression = null)
where TNewRelatedEntity : class
{
var navigation = navigationExpression?.GetMemberAccess();
var relatedEntityType = FindRelatedEntityType(typeof(TNewRelatedEntity), navigation?.GetSimpleMemberName());
var navigationMember = navigationExpression?.GetMemberAccess();
var relatedEntityType = FindRelatedEntityType(typeof(TNewRelatedEntity), navigationMember?.GetSimpleMemberName());
var foreignKey = HasOneBuilder(MemberIdentity.Create(navigationMember), relatedEntityType);

return new ReferenceNavigationBuilder<TDependentEntity, TNewRelatedEntity>(
DependentEntityType,
relatedEntityType,
navigation,
DependentEntityType.Builder.HasRelationship(
relatedEntityType, navigation, ConfigurationSource.Explicit,
targetIsPrincipal: DependentEntityType == relatedEntityType ? true : null)!.Metadata);
navigationMember,
foreignKey);
}

/// <summary>
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore/Metadata/Conventions/KeyDiscoveryConvention.cs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,8 @@ public virtual void ProcessForeignKeyPropertiesChanged(
IConventionContext<IReadOnlyList<IConventionProperty>> context)
{
var foreignKey = relationshipBuilder.Metadata;
if (foreignKey.IsOwnership
if ((foreignKey.IsOwnership
|| foreignKey.GetReferencingSkipNavigations().Any(n => n.IsCollection))
&& !foreignKey.Properties.SequenceEqual(oldDependentProperties)
&& relationshipBuilder.Metadata.IsInModel)
{
Expand Down
11 changes: 10 additions & 1 deletion src/EFCore/Metadata/Internal/CoreAnnotationNames.cs
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,14 @@ public static class CoreAnnotationNames
/// </summary>
public const string ElementType = "ElementType";

/// <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 const string SkipNavigationBeingConfigured = "SkipNavigationBeingConfigured";

/// <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 Expand Up @@ -399,6 +407,7 @@ public static class CoreAnnotationNames
FullChangeTrackingNotificationsRequired,
AdHocModel,
JsonValueReaderWriterType,
ElementType
ElementType,
SkipNavigationBeingConfigured
};
}
11 changes: 4 additions & 7 deletions src/EFCore/Metadata/Internal/InternalModelBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -385,16 +385,13 @@ public virtual bool CanHaveEntity(
/// 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 virtual InternalModelBuilder? RemoveImplicitJoinEntity(EntityType joinEntityType)
{
Check.NotNull(joinEntityType, nameof(joinEntityType));

return !joinEntityType.IsInModel
public virtual InternalModelBuilder? RemoveImplicitJoinEntity(
EntityType joinEntityType, ConfigurationSource configurationSource = ConfigurationSource.Convention)
=> !Check.NotNull(joinEntityType, nameof(joinEntityType)).IsInModel
? this
: !joinEntityType.IsImplicitlyCreatedJoinEntityType
? null
: HasNoEntityType(joinEntityType, ConfigurationSource.Convention);
}
: HasNoEntityType(joinEntityType, configurationSource);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ protected override InternalSkipNavigationBuilder This
&& oldForeignKey != foreignKey
&& oldForeignKey.ReferencingSkipNavigations?.Any() != true)
{
oldForeignKey.DeclaringEntityType.Builder.HasNoRelationship(oldForeignKey, ConfigurationSource.Convention);
oldForeignKey.DeclaringEntityType.Builder.HasNoRelationship(oldForeignKey, ConfigurationSource.Explicit);
}

return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -920,23 +920,30 @@ public virtual void Can_use_shared_type_as_join_entity_with_partition_keys()
mb.HasPartitionKey("PartitionId");
});

modelBuilder.Entity<ManyToManyNavPrincipal>()
.HasMany(e => e.Dependents)
.WithMany(e => e.ManyToManyPrincipals)
.UsingEntity<Dictionary<string, object>>(
"JoinType",
e => e.HasOne<NavDependent>().WithMany().HasAnnotation("Right", "Foo"),
e => e.HasOne<ManyToManyNavPrincipal>().WithMany().HasAnnotation("Left", "Bar"));

modelBuilder.Entity<ManyToManyNavPrincipal>()
.HasMany(e => e.Dependents)
.WithMany(e => e.ManyToManyPrincipals)
.UsingEntity<Dictionary<string, object>>(
"JoinType",
e => e.HasOne<NavDependent>().WithMany().HasForeignKey("DependentId", "PartitionId"),
e => e.HasOne<ManyToManyNavPrincipal>().WithMany().HasForeignKey("PrincipalId", "PartitionId"),
e =>
{
e.HasPartitionKey("PartitionId");
});
e => e.HasPartitionKey("PartitionId"));

var model = modelBuilder.FinalizeModel();

var joinType = model.FindEntityType("JoinType")!;
Assert.NotNull(joinType);
Assert.Equal(2, joinType.GetForeignKeys().Count());
Assert.Collection(joinType.GetForeignKeys(),
fk => Assert.Equal("Foo", fk["Right"]),
fk => Assert.Equal("Bar", fk["Left"]));
Assert.Equal(3, joinType.FindPrimaryKey()!.Properties.Count);
Assert.Equal(6, joinType.GetProperties().Count());
Assert.Equal("DbContext", joinType.GetContainer());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ public override void AssertEqual(
bool assertOrder = false,
bool compareAnnotations = false)
{
expectedProperties = expectedProperties.Where(p => p.Name != "__jObject" && p.Name != "__id");
actualProperties = actualProperties.Where(p => p.Name != "__jObject" && p.Name != "__id");
expectedProperties = expectedProperties.Where(p => p.Name != "__jObject" && p.Name != "__id" && p.Name != "Discriminator");
actualProperties = actualProperties.Where(p => p.Name != "__jObject" && p.Name != "__id" && p.Name != "Discriminator");

base.AssertEqual(expectedProperties, actualProperties, assertOrder, compareAnnotations);
}
Expand Down
Loading

0 comments on commit cf303c8

Please sign in to comment.