Add the ability to break self-ref cycles between added and removed en…

…tities

Fixes #26750

src/Shared/Multigraph.cs

@@ -17,6 +17,9 @@ internal class Multigraph<TVertex, TEdge> : Graph<TVertex>
         private readonly Dictionary<TVertex, Dictionary<TVertex, object?>> _successorMap = new();
         private readonly Dictionary<TVertex, HashSet<TVertex>> _predecessorMap = new();
 
+        private readonly bool _useOldBehavior
+            = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue26750", out var enabled) && enabled;
+
         public IEnumerable<TEdge> GetEdges(TVertex from, TVertex to)
         {
             if (_successorMap.TryGetValue(from, out var successorSet))
@@ -369,7 +372,9 @@ public IReadOnlyList<List<TVertex>> BatchingTopologicalSort(
                         && tryBreakEdge != null)
                     {
                         var candidateVertex = candidateVertices[candidateIndex];
-                        if (predecessorCounts[candidateVertex] != 1)
+                        if (_useOldBehavior
+                            ? predecessorCounts[candidateVertex] != 1
+                            : predecessorCounts[candidateVertex] == 0)
                         {
                             candidateIndex++;
                             continue;
@@ -386,10 +391,14 @@ public IReadOnlyList<List<TVertex>> BatchingTopologicalSort(
                             _successorMap[incomingNeighbor].Remove(candidateVertex);
                             _predecessorMap[candidateVertex].Remove(incomingNeighbor);
                             predecessorCounts[candidateVertex]--;
-                            currentRootsQueue.Add(candidateVertex);
-                            nextRootsQueue = new List<TVertex>();
-                            broken = true;
-                            break;
+                            if (_useOldBehavior
+                                || predecessorCounts[candidateVertex] == 0)
+                            {
+                                currentRootsQueue.Add(candidateVertex);
+                                nextRootsQueue = new List<TVertex>();
+                                broken = true;
+                            }
+                            continue;
                         }
 
                         candidateIndex++;

test/EFCore.Specification.Tests/TestModels/UpdatesModel/Person.cs

@@ -0,0 +1,23 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel
+{
+    public class Person
+    {
+        protected Person()
+        {
+        }
+
+        public Person(string name, Person parent)
+        {
+            Name = name;
+            Parent = parent;
+        }
+
+        public int PersonId { get; set; }
+        public string Name { get; set; }
+        public int? ParentId { get; set; }
+        public Person Parent { get; set; }
+    }
+}

test/EFCore.Specification.Tests/UpdatesFixtureBase.cs

@@ -23,6 +23,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                 .HasForeignKey(e => e.DependentId)
                 .HasPrincipalKey(e => e.PrincipalId);
 
+            modelBuilder.Entity<Person>()
+                .HasOne(p => p.Parent)
+                .WithMany()
+                .OnDelete(DeleteBehavior.Restrict);
+
             modelBuilder.Entity<Category>()
                 .Property(e => e.Id)
                 .ValueGeneratedNever();

test/EFCore.Specification.Tests/UpdatesTestBase.cs

@@ -278,6 +278,66 @@ public virtual void Remove_on_bytes_concurrency_token_original_value_matches_doe
                 context => Assert.Null(context.ProductWithBytes.Find(productId)));
         }
 
+        [ConditionalFact]
+        public virtual void Can_add_and_remove_self_refs()
+        {
+            ExecuteWithStrategyInTransaction(
+                context =>
+                {
+                    var parent = new Person("1", null);
+                    var child1 = new Person("2", parent);
+                    var child2 = new Person("3", parent);
+                    var grandchild1 = new Person("4", child1);
+                    var grandchild2 = new Person("5", child1);
+                    var grandchild3 = new Person("6", child2);
+                    var grandchild4 = new Person("7", child2);
+
+                    context.Add(parent);
+                    context.Add(child1);
+                    context.Add(child2);
+                    context.Add(grandchild1);
+                    context.Add(grandchild2);
+                    context.Add(grandchild3);
+                    context.Add(grandchild4);
+
+                    context.SaveChanges();
+
+                    context.Remove(parent);
+                    context.Remove(child1);
+                    context.Remove(child2);
+                    context.Remove(grandchild1);
+                    context.Remove(grandchild2);
+                    context.Remove(grandchild3);
+                    context.Remove(grandchild4);
+
+                    parent = new Person("1", null);
+                    child1 = new Person("2", parent);
+                    child2 = new Person("3", parent);
+                    grandchild1 = new Person("4", child1);
+                    grandchild2 = new Person("5", child1);
+                    grandchild3 = new Person("6", child2);
+                    grandchild4 = new Person("7", child2);
+
+                    context.Add(parent);
+                    context.Add(child1);
+                    context.Add(child2);
+                    context.Add(grandchild1);
+                    context.Add(grandchild2);
+                    context.Add(grandchild3);
+                    context.Add(grandchild4);
+
+                    context.SaveChanges();
+                },
+                context =>
+                {
+                    var people = context.Set<Person>()
+                        .Include(p => p.Parent).ThenInclude(c => c.Parent).ThenInclude(c => c.Parent)
+                        .ToList();
+                    Assert.Equal(7, people.Count);
+                    Assert.Equal("1", people.Single(p => p.Parent == null).Name);
+                });
+        }
+
         [ConditionalFact]
         public virtual void Can_remove_partial()
         {

test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTest.cs

@@ -65,6 +65,68 @@ INTO @inserted0
 SELECT [i].[Id] FROM @inserted0 i;");
         }
 
+        [ConditionalFact]
+        public override void Can_add_and_remove_self_refs()
+        {
+            base.Can_add_and_remove_self_refs();
+
+            AssertContainsSql(
+                @"@p0='1' (Size = 4000)
+@p1=NULL (DbType = Int32)
+
+SET NOCOUNT ON;
+INSERT INTO [Person] ([Name], [ParentId])
+VALUES (@p0, @p1);
+SELECT [PersonId]
+FROM [Person]
+WHERE @@ROWCOUNT = 1 AND [PersonId] = scope_identity();",
+                //
+                @"@p0='2' (Size = 4000)
+@p1='1' (Nullable = true)
+
+SET NOCOUNT ON;
+INSERT INTO [Person] ([Name], [ParentId])
+VALUES (@p0, @p1);
+SELECT [PersonId]
+FROM [Person]
+WHERE @@ROWCOUNT = 1 AND [PersonId] = scope_identity();",
+                //
+                @"@p0='3' (Size = 4000)
+@p1='1' (Nullable = true)
+
+SET NOCOUNT ON;
+INSERT INTO [Person] ([Name], [ParentId])
+VALUES (@p0, @p1);
+SELECT [PersonId]
+FROM [Person]
+WHERE @@ROWCOUNT = 1 AND [PersonId] = scope_identity();",
+                //
+                @"@p2='4' (Size = 4000)
+@p3='2' (Nullable = true)
+@p4='5' (Size = 4000)
+@p5='2' (Nullable = true)
+@p6='6' (Size = 4000)
+@p7='3' (Nullable = true)
+@p8='7' (Size = 4000)
+@p9='3' (Nullable = true)
+
+SET NOCOUNT ON;
+DECLARE @inserted0 TABLE ([PersonId] int, [_Position] [int]);
+MERGE [Person] USING (
+VALUES (@p2, @p3, 0),
+(@p4, @p5, 1),
+(@p6, @p7, 2),
+(@p8, @p9, 3)) AS i ([Name], [ParentId], _Position) ON 1=0
+WHEN NOT MATCHED THEN
+INSERT ([Name], [ParentId])
+VALUES (i.[Name], i.[ParentId])
+OUTPUT INSERTED.[PersonId], i._Position
+INTO @inserted0;
+
+SELECT [i].[PersonId] FROM @inserted0 i
+ORDER BY [i].[_Position];");
+        }
+
         public override void Save_replaced_principal()
         {
             base.Save_replaced_principal();