From de85553036c96b721dfa125be7d7a827790ae195 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Wed, 4 Sep 2024 17:48:19 -0700 Subject: [PATCH] Don't remove non-existant foreign keys. Fixes #34329 --- src/EFCore/Metadata/Internal/EntityType.cs | 28 ++++++------- .../Metadata/Internal/InternalModelBuilder.cs | 5 +++ .../CosmosModelBuilderGenericTest.cs | 41 +++++++++++++++++++ 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/src/EFCore/Metadata/Internal/EntityType.cs b/src/EFCore/Metadata/Internal/EntityType.cs index 2fc7085df50..7cadc45f845 100644 --- a/src/EFCore/Metadata/Internal/EntityType.cs +++ b/src/EFCore/Metadata/Internal/EntityType.cs @@ -990,7 +990,7 @@ public virtual void OnForeignKeyUpdating(ForeignKey foreignKey) removed = foreignKey.PrincipalKey.ReferencingForeignKeys!.Remove(foreignKey); Check.DebugAssert(removed, "removed is false"); - removed = foreignKey.PrincipalEntityType.DeclaredReferencingForeignKeys!.Remove(foreignKey); + removed = foreignKey.PrincipalEntityType._declaredReferencingForeignKeys!.Remove(foreignKey); Check.DebugAssert(removed, "removed is false"); } @@ -1029,13 +1029,13 @@ public virtual void OnForeignKeyUpdated(ForeignKey foreignKey) } var principalEntityType = foreignKey.PrincipalEntityType; - if (principalEntityType.DeclaredReferencingForeignKeys == null) + if (principalEntityType._declaredReferencingForeignKeys == null) { - principalEntityType.DeclaredReferencingForeignKeys = new SortedSet(ForeignKeyComparer.Instance) { foreignKey }; + principalEntityType._declaredReferencingForeignKeys = new SortedSet(ForeignKeyComparer.Instance) { foreignKey }; } else { - added = principalEntityType.DeclaredReferencingForeignKeys.Add(foreignKey); + added = principalEntityType._declaredReferencingForeignKeys.Add(foreignKey); Check.DebugAssert(added, "added is false"); } } @@ -1371,7 +1371,7 @@ public virtual IEnumerable FindForeignKeysInHierarchy( /// public virtual IEnumerable GetReferencingForeignKeys() => BaseType != null - ? (DeclaredReferencingForeignKeys?.Count ?? 0) == 0 + ? (_declaredReferencingForeignKeys?.Count ?? 0) == 0 ? BaseType.GetReferencingForeignKeys() : BaseType.GetReferencingForeignKeys().Concat(GetDeclaredReferencingForeignKeys()) : GetDeclaredReferencingForeignKeys(); @@ -1383,9 +1383,9 @@ public virtual IEnumerable GetReferencingForeignKeys() /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual IEnumerable GetDeclaredReferencingForeignKeys() - => DeclaredReferencingForeignKeys ?? Enumerable.Empty(); + => _declaredReferencingForeignKeys ?? Enumerable.Empty(); - private SortedSet? DeclaredReferencingForeignKeys { get; set; } + private SortedSet? _declaredReferencingForeignKeys; #endregion @@ -1673,14 +1673,14 @@ public virtual IEnumerable GetNavigations() _skipNavigations.Add(name, skipNavigation); - if (targetEntityType.DeclaredReferencingSkipNavigations == null) + if (targetEntityType._declaredReferencingSkipNavigations == null) { - targetEntityType.DeclaredReferencingSkipNavigations = + targetEntityType._declaredReferencingSkipNavigations = new SortedSet(SkipNavigationComparer.Instance) { skipNavigation }; } else { - var added = targetEntityType.DeclaredReferencingSkipNavigations.Add(skipNavigation); + var added = targetEntityType._declaredReferencingSkipNavigations.Add(skipNavigation); Check.DebugAssert(added, "added is false"); } @@ -1826,7 +1826,7 @@ public virtual IEnumerable FindSkipNavigationsInHierarchy(string || foreignKey.ReferencingSkipNavigations!.Remove(navigation); Check.DebugAssert(removed, "removed is false"); - removed = navigation.TargetEntityType.DeclaredReferencingSkipNavigations!.Remove(navigation); + removed = navigation.TargetEntityType._declaredReferencingSkipNavigations!.Remove(navigation); Check.DebugAssert(removed, "removed is false"); navigation.SetRemovedFromModel(); @@ -1855,7 +1855,7 @@ public virtual IEnumerable GetSkipNavigations() /// public virtual IEnumerable GetReferencingSkipNavigations() => BaseType != null - ? (DeclaredReferencingSkipNavigations?.Count ?? 0) == 0 + ? (_declaredReferencingSkipNavigations?.Count ?? 0) == 0 ? BaseType.GetReferencingSkipNavigations() : BaseType.GetReferencingSkipNavigations().Concat(GetDeclaredReferencingSkipNavigations()) : GetDeclaredReferencingSkipNavigations(); @@ -1867,7 +1867,7 @@ public virtual IEnumerable GetReferencingSkipNavigations() /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual IEnumerable GetDeclaredReferencingSkipNavigations() - => DeclaredReferencingSkipNavigations ?? Enumerable.Empty(); + => _declaredReferencingSkipNavigations ?? Enumerable.Empty(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -1880,7 +1880,7 @@ public virtual IEnumerable GetDerivedReferencingSkipNavigations( ? Enumerable.Empty() : GetDerivedTypes().SelectMany(et => et.GetDeclaredReferencingSkipNavigations()); - private SortedSet? DeclaredReferencingSkipNavigations { get; set; } + private SortedSet? _declaredReferencingSkipNavigations; #endregion diff --git a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs index 69c2235a753..8f32abaa469 100644 --- a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs @@ -687,6 +687,11 @@ private bool CanIgnore(in TypeIdentity type, ConfigurationSource configurationSo { foreach (var foreignKey in entityType.GetDeclaredReferencingForeignKeys().ToList()) { + if (!foreignKey.IsInModel) + { + continue; + } + if (foreignKey.IsOwnership && configurationSource.Overrides(foreignKey.DeclaringEntityType.GetConfigurationSource())) { diff --git a/test/EFCore.Cosmos.FunctionalTests/ModelBuilding/CosmosModelBuilderGenericTest.cs b/test/EFCore.Cosmos.FunctionalTests/ModelBuilding/CosmosModelBuilderGenericTest.cs index 4d8994ef478..82b70d4313b 100644 --- a/test/EFCore.Cosmos.FunctionalTests/ModelBuilding/CosmosModelBuilderGenericTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/ModelBuilding/CosmosModelBuilderGenericTest.cs @@ -855,6 +855,47 @@ public override void Navigation_to_shared_type_is_not_discovered_by_convention() owned.DisplayName()); } + [ConditionalFact] // Issue #34329 + public virtual void Navigation_cycle_can_be_broken() + { + var modelBuilder = CreateModelBuilder(); + + modelBuilder.Entity().Ignore(x => x.E2); + + var model = modelBuilder.FinalizeModel(); + + var principal = model.FindEntityType(typeof(EntityType1))!; + Assert.Null(principal.FindNavigation(nameof(EntityType1.E2))); + Assert.Null(model.FindEntityType(typeof(EntityType2))); + } + + protected class EntityType1 + { + public int Id { get; set; } + public EntityType2? E2 { get; set; } + } + + + protected class EntityType2 + { + public int Id { get; set; } + public EntityType3? E3 { get; set; } + + public EntityType4? E4 { get; set; } + } + + protected class EntityType3 + { + public int Id { get; set; } + public EntityType2? U2 { get; set; } + } + + protected class EntityType4 + { + public int Id { get; set; } + public EntityType3? E3 { get; set; } + } + protected override TestModelBuilder CreateModelBuilder(Action? configure = null) => new GenericTestModelBuilder(Fixture, configure); }