[release/9.0] Don't remove non-existent foreign keys. (#34614)

Fixes #34329
dotnet · Sep 5, 2024 · 1c40112 · 1c40112
1 parent f2eb7db
commit 1c40112
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 14 deletions.
diff --git 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<ForeignKey>(ForeignKeyComparer.Instance) { foreignKey };
+            principalEntityType._declaredReferencingForeignKeys = new SortedSet<ForeignKey>(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<ForeignKey> FindForeignKeysInHierarchy(
     /// </summary>
     public virtual IEnumerable<ForeignKey> 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<ForeignKey> GetReferencingForeignKeys()
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public virtual IEnumerable<ForeignKey> GetDeclaredReferencingForeignKeys()
-        => DeclaredReferencingForeignKeys ?? Enumerable.Empty<ForeignKey>();
+        => _declaredReferencingForeignKeys ?? Enumerable.Empty<ForeignKey>();
 
-    private SortedSet<ForeignKey>? DeclaredReferencingForeignKeys { get; set; }
+    private SortedSet<ForeignKey>? _declaredReferencingForeignKeys;
 
     #endregion
 
@@ -1673,14 +1673,14 @@ public virtual IEnumerable<Navigation> GetNavigations()
 
         _skipNavigations.Add(name, skipNavigation);
 
-        if (targetEntityType.DeclaredReferencingSkipNavigations == null)
+        if (targetEntityType._declaredReferencingSkipNavigations == null)
         {
-            targetEntityType.DeclaredReferencingSkipNavigations =
+            targetEntityType._declaredReferencingSkipNavigations =
                 new SortedSet<SkipNavigation>(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<SkipNavigation> 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<SkipNavigation> GetSkipNavigations()
     /// </summary>
     public virtual IEnumerable<SkipNavigation> 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<SkipNavigation> GetReferencingSkipNavigations()
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public virtual IEnumerable<SkipNavigation> GetDeclaredReferencingSkipNavigations()
-        => DeclaredReferencingSkipNavigations ?? Enumerable.Empty<SkipNavigation>();
+        => _declaredReferencingSkipNavigations ?? Enumerable.Empty<SkipNavigation>();
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -1880,7 +1880,7 @@ public virtual IEnumerable<SkipNavigation> GetDerivedReferencingSkipNavigations(
             ? Enumerable.Empty<SkipNavigation>()
             : GetDerivedTypes<EntityType>().SelectMany(et => et.GetDeclaredReferencingSkipNavigations());
 
-    private SortedSet<SkipNavigation>? DeclaredReferencingSkipNavigations { get; set; }
+    private SortedSet<SkipNavigation>? _declaredReferencingSkipNavigations;
 
     #endregion
 

diff --git 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
@@ -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<EntityType1>().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<ModelConfigurationBuilder>? configure = null)
             => new GenericTestModelBuilder(Fixture, configure);
     }