From 81844ec96d93767a3876eb5fd4bb91dc9e625c91 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Sat, 15 Aug 2020 15:38:51 -0700 Subject: [PATCH] Use local DetectChanges when cascade-deleting Fixes #17828 --- .../ChangeTracking/Internal/StateManager.cs | 6 + test/EFCore.Tests/DbContextTrackingTest.cs | 119 ++++++++++++++++++ 2 files changed, 125 insertions(+) diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs index 35a2a57979c..1de65f14d08 100644 --- a/src/EFCore/ChangeTracking/Internal/StateManager.cs +++ b/src/EFCore/ChangeTracking/Internal/StateManager.cs @@ -980,6 +980,10 @@ public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumer { var doCascadeDelete = force || CascadeDeleteTiming != CascadeTiming.Never; var principalIsDetached = entry.EntityState == EntityState.Detached; + var changeDetector = Context.ChangeTracker.AutoDetectChangesEnabled + && (string)Context.Model[CoreAnnotationNames.SkipDetectChangesAnnotation] != "true" + ? Context.GetDependencies().ChangeDetector + : null; foreignKeys ??= entry.EntityType.GetReferencingForeignKeys(); foreach (var fk in foreignKeys) @@ -992,6 +996,8 @@ public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumer foreach (InternalEntityEntry dependent in (GetDependentsFromNavigation(entry, fk) ?? GetDependents(entry, fk)).ToList()) { + changeDetector?.DetectChanges(dependent); + if (dependent.EntityState != EntityState.Deleted && dependent.EntityState != EntityState.Detached && (dependent.EntityState == EntityState.Added diff --git a/test/EFCore.Tests/DbContextTrackingTest.cs b/test/EFCore.Tests/DbContextTrackingTest.cs index 89bdfd7226b..0bf1f28935f 100644 --- a/test/EFCore.Tests/DbContextTrackingTest.cs +++ b/test/EFCore.Tests/DbContextTrackingTest.cs @@ -1696,5 +1696,124 @@ public void Can_set_set_to_Unchanged_with_inconsistent_FK_dependent_first_refere Assert.Equal(EntityState.Unchanged, context.Entry(category).State); Assert.Equal(EntityState.Unchanged, context.Entry(product).State); } + + [ConditionalTheory] // Issue #17828 + [InlineData(CascadeTiming.Immediate)] + [InlineData(CascadeTiming.Never)] + [InlineData(CascadeTiming.OnSaveChanges)] + public void Can_re_parent_optional_without_DetectChanges(CascadeTiming cascadeTiming) + { + using var context = new Parent77Context(); + + context.ChangeTracker.CascadeDeleteTiming = cascadeTiming; + + var parent1 = new Parent77(); + var parent2 = new Parent77(); + var child = new Optional77(); + + child.Parent77 = parent1; + context.AddRange(parent1, parent2, child); + context.SaveChanges(); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Unchanged, context.Entry(parent1).State); + Assert.Equal(EntityState.Unchanged, context.Entry(parent2).State); + Assert.Equal(EntityState.Unchanged, context.Entry(child).State); + + child.Parent77 = parent2; + context.Remove(parent1); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Deleted, context.Entry(parent1).State); + Assert.Equal(EntityState.Unchanged, context.Entry(parent2).State); + Assert.Equal(EntityState.Modified, context.Entry(child).State); + Assert.Same(parent2, child.Parent77); + + context.SaveChanges(); + + Assert.Equal(2, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Detached, context.Entry(parent1).State); + Assert.Equal(EntityState.Unchanged, context.Entry(parent2).State); + Assert.Equal(EntityState.Unchanged, context.Entry(child).State); + Assert.Same(parent2, child.Parent77); + } + + [ConditionalTheory] // Issue #17828 + [InlineData(CascadeTiming.Immediate)] + [InlineData(CascadeTiming.Never)] + [InlineData(CascadeTiming.OnSaveChanges)] + public void Can_re_parent_required_without_DetectChanges(CascadeTiming cascadeTiming) + { + using var context = new Parent77Context(); + + context.ChangeTracker.CascadeDeleteTiming = cascadeTiming; + + var parent1 = new Parent77(); + var parent2 = new Parent77(); + var child = new Required77(); + + child.Parent77 = parent1; + context.AddRange(parent1, parent2, child); + context.SaveChanges(); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Unchanged, context.Entry(parent1).State); + Assert.Equal(EntityState.Unchanged, context.Entry(parent2).State); + Assert.Equal(EntityState.Unchanged, context.Entry(child).State); + + child.Parent77 = parent2; + context.Remove(parent1); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Deleted, context.Entry(parent1).State); + Assert.Equal(EntityState.Unchanged, context.Entry(parent2).State); + Assert.Equal(EntityState.Modified, context.Entry(child).State); + Assert.Same(parent2, child.Parent77); + + context.SaveChanges(); + + Assert.Equal(2, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Detached, context.Entry(parent1).State); + Assert.Equal(EntityState.Unchanged, context.Entry(parent2).State); + Assert.Equal(EntityState.Unchanged, context.Entry(child).State); + Assert.Same(parent2, child.Parent77); + } + + private class Parent77Context : DbContext + { + protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseInMemoryDatabase(nameof(Parent77Context)); + + protected internal override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity( + b => + { + b.HasMany().WithOne(e => e.Parent77); + b.HasMany().WithOne(e => e.Parent77); + }); + } + } + + private class Parent77 + { + public int Id { get; set; } + } + + private class Optional77 + { + public int Id { get; set; } + + public int? Parent77Id { get; set; } + public Parent77 Parent77 { get; set; } + } + + private class Required77 + { + public int Id { get; set; } + + public int Parent77Id { get; set; } + public Parent77 Parent77 { get; set; } + } } }