diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs index 80d011b4f30..0d14863adb9 100644 --- a/src/EFCore/ChangeTracking/Internal/StateManager.cs +++ b/src/EFCore/ChangeTracking/Internal/StateManager.cs @@ -959,6 +959,7 @@ public virtual void CascadeChanges(bool force) public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumerable foreignKeys = null) { var doCascadeDelete = force || CascadeDeleteTiming != CascadeTiming.Never; + var principalIsDetached = entry.EntityState == EntityState.Detached; foreignKeys ??= entry.EntityType.GetReferencingForeignKeys(); foreach (var fk in foreignKeys) @@ -980,9 +981,10 @@ public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumer || fk.DeleteBehavior == DeleteBehavior.ClientCascade) && doCascadeDelete) { - var cascadeState = dependent.EntityState == EntityState.Added - ? EntityState.Detached - : EntityState.Deleted; + var cascadeState = principalIsDetached + || dependent.EntityState == EntityState.Added + ? EntityState.Detached + : EntityState.Deleted; if (SensitiveLoggingEnabled) { diff --git a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs index d1fc0da51e0..00af8790b8d 100644 --- a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs +++ b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs @@ -1257,7 +1257,8 @@ public async Task Reload_when_entity_deleted_in_store_can_happen_for_any_state(E else { Assert.Equal(EntityState.Detached, entry.State); - Assert.Null(mailRoom.Building); + Assert.Same(mailRoom, building.PrincipalMailRoom); + Assert.Contains(office, building.Offices); Assert.Equal(EntityState.Detached, context.Entry(office.Building).State); Assert.Same(building, office.Building); diff --git a/test/EFCore.Specification.Tests/ProxyGraphUpdatesTestBase.cs b/test/EFCore.Specification.Tests/ProxyGraphUpdatesTestBase.cs index ff6784c4c13..747347bbe94 100644 --- a/test/EFCore.Specification.Tests/ProxyGraphUpdatesTestBase.cs +++ b/test/EFCore.Specification.Tests/ProxyGraphUpdatesTestBase.cs @@ -681,7 +681,7 @@ public virtual void Save_required_one_to_one_changed_by_reference(ChangeMechanis Assert.Same(new1, new2.Back); Assert.NotNull(old1.Root); - Assert.Null(old2.Back); + Assert.Same(old1, old2.Back); Assert.Equal(old1.Id, old2.Id); }); } diff --git a/test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs b/test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs index 7f5827349db..39f52e7dd88 100644 --- a/test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs +++ b/test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs @@ -2085,6 +2085,323 @@ public void Can_add_owned_dependent_with_reference_to_parent(bool useAdd, bool s Assert.Equal(1, dependentEntry2b.Property(dependentEntry2b.Metadata.FindPrimaryKey().Properties[0].Name).CurrentValue); } + [ConditionalTheory] // Issue #17828 + [InlineData(false)] + [InlineData(true)] + public void DetectChanges_reparents_even_when_immediate_cascade_enabled(bool delayCascade) + { + using var context = new EarlyLearningCenter(); + + // Construct initial state + var parent1 = new Category { Id = 1 }; + var parent2 = new Category { Id = 2 }; + var child = new Product { Id = 3, Category = parent1 }; + + context.AddRange(parent1, parent2, child); + context.ChangeTracker.AcceptAllChanges(); + + 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); + + if (delayCascade) + { + context.ChangeTracker.CascadeDeleteTiming = CascadeTiming.OnSaveChanges; + } + + child.Category = parent2; + + context.ChangeTracker.DetectChanges(); + + 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); + } + + [ConditionalTheory] // Issue #12590 + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void Dependents_are_detached_not_deleted_when_principal_is_detached(bool delayCascade, bool trackNewDependents) + { + using var context = new EarlyLearningCenter(); + + var category = new Category + { + Id = 1, + Products = new List + { + new Product { Id = 1 }, + new Product { Id = 2 }, + new Product { Id = 3 } + } + }; + + context.Attach(category); + + var categoryEntry = context.Entry(category); + var product0Entry = context.Entry(category.Products[0]); + var product1Entry = context.Entry(category.Products[1]); + var product2Entry = context.Entry(category.Products[2]); + + Assert.Equal(EntityState.Unchanged, categoryEntry.State); + Assert.Equal(EntityState.Unchanged, product0Entry.State); + Assert.Equal(EntityState.Unchanged, product1Entry.State); + Assert.Equal(EntityState.Unchanged, product2Entry.State); + + if (delayCascade) + { + context.ChangeTracker.CascadeDeleteTiming = CascadeTiming.OnSaveChanges; + } + + context.Entry(category).State = EntityState.Detached; + + Assert.Equal(EntityState.Detached, categoryEntry.State); + + if (delayCascade) + { + Assert.Equal(EntityState.Unchanged, product0Entry.State); + Assert.Equal(EntityState.Unchanged, product1Entry.State); + Assert.Equal(EntityState.Unchanged, product2Entry.State); + } + else + { + Assert.Equal(EntityState.Detached, product0Entry.State); + Assert.Equal(EntityState.Detached, product1Entry.State); + Assert.Equal(EntityState.Detached, product2Entry.State); + } + + var newCategory = new Category { Id = 1, }; + + if (trackNewDependents) + { + newCategory.Products = new List + { + new Product { Id = 1 }, + new Product { Id = 2 }, + new Product { Id = 3 } + }; + } + + var traversal = new List(); + + if (delayCascade && trackNewDependents) + { + Assert.Equal( + CoreStrings.IdentityConflict(nameof(Product), "{'Id'}"), + Assert.Throws(TrackGraph).Message); + } + else + { + TrackGraph(); + + Assert.Equal( + trackNewDependents + ? new List + { + " -----> Category:1", + "Category:1 ---Products--> Product:1", + "Category:1 ---Products--> Product:2", + "Category:1 ---Products--> Product:3" + } + : new List + { + " -----> Category:1" + }, + traversal); + + if (trackNewDependents || delayCascade) + { + Assert.Equal(4, context.ChangeTracker.Entries().Count()); + + categoryEntry = context.Entry(newCategory); + product0Entry = context.Entry(newCategory.Products[0]); + product1Entry = context.Entry(newCategory.Products[1]); + product2Entry = context.Entry(newCategory.Products[2]); + + Assert.Equal(EntityState.Modified, categoryEntry.State); + + if (trackNewDependents) + { + Assert.Equal(EntityState.Modified, product0Entry.State); + Assert.Equal(EntityState.Modified, product1Entry.State); + Assert.Equal(EntityState.Modified, product2Entry.State); + + Assert.NotSame(newCategory.Products[0], category.Products[0]); + Assert.NotSame(newCategory.Products[1], category.Products[1]); + Assert.NotSame(newCategory.Products[2], category.Products[2]); + } + else + { + Assert.Equal(EntityState.Unchanged, product0Entry.State); + Assert.Equal(EntityState.Unchanged, product1Entry.State); + Assert.Equal(EntityState.Unchanged, product2Entry.State); + + Assert.Same(newCategory.Products[0], category.Products[0]); + Assert.Same(newCategory.Products[1], category.Products[1]); + Assert.Same(newCategory.Products[2], category.Products[2]); + } + + Assert.Same(newCategory, newCategory.Products[0].Category); + Assert.Same(newCategory, newCategory.Products[1].Category); + Assert.Same(newCategory, newCategory.Products[2].Category); + + Assert.Equal(newCategory.Id, product0Entry.Property("CategoryId").CurrentValue); + Assert.Equal(newCategory.Id, product1Entry.Property("CategoryId").CurrentValue); + Assert.Equal(newCategory.Id, product2Entry.Property("CategoryId").CurrentValue); + } + else + { + Assert.Single(context.ChangeTracker.Entries()); + + categoryEntry = context.Entry(newCategory); + + Assert.Equal(EntityState.Modified, categoryEntry.State); + Assert.Null(newCategory.Products); + } + } + + void TrackGraph() + { + context.ChangeTracker.TrackGraph( + newCategory, n => + { + n.Entry.State = EntityState.Modified; + traversal.Add(NodeString(n)); + }); + } + } + + [ConditionalTheory] // Issue #16546 + [InlineData(false)] + [InlineData(true)] + public void Optional_relationship_with_cascade_still_cascades(bool delayCascade) + { + Kontainer detachedContainer; + var databaseName = "K" + delayCascade; + using (var context = new KontainerContext(databaseName)) + { + context.Add( + new Kontainer + { + Name = "C1", + Rooms = { new KontainerRoom { Number = 1, Troduct = new Troduct { Description = "Heavy Engine XT3" } } } + } + ); + + context.SaveChanges(); + + detachedContainer = context.Set() + .Include(container => container.Rooms) + .ThenInclude(room => room.Troduct) + .AsNoTracking() + .Single(); + } + + using (var context = new KontainerContext(databaseName)) + { + var attachedContainer = context.Set() + .Include(container => container.Rooms) + .ThenInclude(room => room.Troduct) + .Single(); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer).State); + Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer.Rooms.Single()).State); + Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer.Rooms.Single().Troduct).State); + + var detachedRoom = detachedContainer.Rooms.Single(); + detachedRoom.Troduct = null; + detachedRoom.TroductId = null; + + var attachedRoom = attachedContainer.Rooms.Single(); + + if (delayCascade) + { + context.ChangeTracker.DeleteOrphansTiming = CascadeTiming.OnSaveChanges; + } + + context.Entry(attachedRoom).CurrentValues.SetValues(detachedRoom); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer).State); + Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer.Rooms.Single().Troduct).State); + + if (delayCascade) + { + Assert.Equal(EntityState.Modified, context.Entry(attachedContainer.Rooms.Single()).State); + } + else + { + // Deleted because FK with cascade has been set to null + Assert.Equal(EntityState.Deleted, context.Entry(attachedContainer.Rooms.Single()).State); + } + + context.ChangeTracker.CascadeChanges(); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer).State); + Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer.Rooms.Single().Troduct).State); + Assert.Equal(EntityState.Deleted, context.Entry(attachedContainer.Rooms.Single()).State); + + context.SaveChanges(); + } + } + + private class Kontainer + { + public int Id { get; set; } + public string Name { get; set; } + public List Rooms { get; set; } = new List(); + } + + private class KontainerRoom + { + public int Id { get; set; } + public int Number { get; set; } + public int KontainerId { get; set; } + public Kontainer Kontainer { get; set; } + public int? TroductId { get; set; } + public Troduct Troduct { get; set; } + } + + private class Troduct + { + public int Id { get; set; } + public string Description { get; set; } + public List Rooms { get; set; } = new List(); + } + + private class KontainerContext : DbContext + { + private readonly string _databaseName; + + public KontainerContext(string databaseName) + { + _databaseName = databaseName; + } + + protected internal override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity() + .HasOne(room => room.Troduct) + .WithMany(product => product.Rooms) + .HasForeignKey(room => room.TroductId) + .IsRequired(false) + .OnDelete(DeleteBehavior.Cascade); + } + + protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder + .UseInternalServiceProvider(InMemoryFixture.DefaultServiceProvider) + .UseInMemoryDatabase(_databaseName); + } + [ConditionalTheory] [InlineData(false)] [InlineData(true)] diff --git a/test/EFCore.Tests/ChangeTracking/Internal/OwnedFixupTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/OwnedFixupTest.cs index 2be8a77b339..5ec48473b9a 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/OwnedFixupTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/OwnedFixupTest.cs @@ -22,6 +22,72 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal { public class OwnedFixupTest { + private class Thing + { + public Guid ThingId { get; set; } + public List OwnedByThings { get; set; } = new List(); + } + + private class OwnedByThing + { + public Guid OwnedByThingId { get; set; } + public Guid ThingId { get; set; } + public Thing Thing { get; set; } + } + + [ConditionalTheory] // Issue #18982 + [InlineData(false)] + [InlineData(true)] + public void Detaching_owner_does_not_delete_owned_entities(bool delayCascade) + { + using var context = new FixupContext(); + + var thing = new Thing + { + ThingId = Guid.NewGuid(), + OwnedByThings = new List + { + new OwnedByThing + { + OwnedByThingId = Guid.NewGuid() + }, + new OwnedByThing + { + OwnedByThingId = Guid.NewGuid() + } + } + }; + + context.Attach(thing); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Unchanged, context.Entry(thing).State); + Assert.Equal(EntityState.Unchanged, context.Entry(thing.OwnedByThings[0]).State); + Assert.Equal(EntityState.Unchanged, context.Entry(thing.OwnedByThings[1]).State); + + if (delayCascade) + { + context.ChangeTracker.CascadeDeleteTiming = CascadeTiming.OnSaveChanges; + } + + context.Entry(thing).State = EntityState.Detached; + + if (delayCascade) + { + Assert.Equal(2, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Detached, context.Entry(thing).State); + Assert.Equal(EntityState.Unchanged, context.Entry(thing.OwnedByThings[0]).State); + Assert.Equal(EntityState.Unchanged, context.Entry(thing.OwnedByThings[1]).State); + } + else + { + Assert.Empty(context.ChangeTracker.Entries()); + Assert.Equal(EntityState.Detached, context.Entry(thing).State); + Assert.Equal(EntityState.Detached, context.Entry(thing.OwnedByThings[0]).State); + Assert.Equal(EntityState.Detached, context.Entry(thing.OwnedByThings[1]).State); + } + } + [ConditionalFact] public void Can_detach_Added_owner_referencing_detached_weak_owned_entity() { @@ -3259,8 +3325,8 @@ public void Parent_and_identity_swapped_bidirectional_collection(EntityState ent public void Fixup_works_when_changing_state_from_Detached_to_Modified(bool detachDependent) { using var context = new OwnedModifiedContext(Guid.NewGuid().ToString()); - var details = new ProductDetails { Color = "C1", Size = "S1" }; + var details = new ProductDetails { Color = "C1", Size = "S1" }; var product = new Product { Name = "Product1", Details = details }; context.Add(product); @@ -3276,15 +3342,8 @@ public void Fixup_works_when_changing_state_from_Detached_to_Modified(bool detac context.Entry(details).State = EntityState.Detached; } - if (detachDependent) - { - Assert.Empty(context.ChangeTracker.Entries()); - } - else - { - Assert.Single(context.ChangeTracker.Entries()); - Assert.Equal(EntityState.Deleted, context.Entry(details).State); - } + Assert.Empty(context.ChangeTracker.Entries()); + Assert.Equal(EntityState.Detached, context.Entry(details).State); var newDetails = new ProductDetails { Color = "C2", Size = "S2" }; @@ -3297,15 +3356,7 @@ public void Fixup_works_when_changing_state_from_Detached_to_Modified(bool detac context.Update(newProduct); - if (detachDependent) - { - Assert.Equal(2, context.ChangeTracker.Entries().Count()); - } - else - { - Assert.Equal(3, context.ChangeTracker.Entries().Count()); - Assert.Equal(EntityState.Deleted, context.Entry(details).State); - } + Assert.Equal(2, context.ChangeTracker.Entries().Count()); Assert.Equal(EntityState.Modified, context.Entry(newProduct).State); Assert.Equal(EntityState.Modified, context.Entry(newDetails).State); @@ -3315,19 +3366,11 @@ public void Fixup_works_when_changing_state_from_Detached_to_Modified(bool detac Assert.Same(newDetails, newProduct.Details); Assert.Equal("C2", newProduct.Details.Color); - if (detachDependent) - { - context.SaveChanges(); + context.SaveChanges(); - Assert.Equal(2, context.ChangeTracker.Entries().Count()); - Assert.Equal(EntityState.Unchanged, context.Entry(newProduct).State); - Assert.Equal(EntityState.Unchanged, context.Entry(newDetails).State); - } - else - { - // Because attempting to update an entity after it has been deleted - Assert.Throws(() => context.SaveChanges()); - } + Assert.Equal(2, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Unchanged, context.Entry(newProduct).State); + Assert.Equal(EntityState.Unchanged, context.Entry(newDetails).State); } private class Product @@ -4210,6 +4253,12 @@ protected internal override void OnModelCreating(ModelBuilder modelBuilder) }); }); }); + + modelBuilder.Entity().OwnsMany(p => p.OwnedByThings, a => + { + a.WithOwner().HasForeignKey(e => e.ThingId); + a.HasKey(e => e.OwnedByThingId); + }); } protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)