Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent dependents from being deleted when principal is detached #19379

Merged
merged 1 commit into from
Dec 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/EFCore/ChangeTracking/Internal/StateManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,7 @@ public virtual void CascadeChanges(bool force)
public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumerable<IForeignKey> foreignKeys = null)
{
var doCascadeDelete = force || CascadeDeleteTiming != CascadeTiming.Never;
var principalIsDetached = entry.EntityState == EntityState.Detached;

foreignKeys ??= entry.EntityType.GetReferencingForeignKeys();
foreach (var fk in foreignKeys)
Expand All @@ -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)
{
Expand Down
3 changes: 2 additions & 1 deletion test/EFCore.Specification.Tests/PropertyValuesTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
Expand Down
317 changes: 317 additions & 0 deletions test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Product>
{
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<Product>
{
new Product { Id = 1 },
new Product { Id = 2 },
new Product { Id = 3 }
};
}

var traversal = new List<string>();

if (delayCascade && trackNewDependents)
{
Assert.Equal(
CoreStrings.IdentityConflict(nameof(Product), "{'Id'}"),
Assert.Throws<InvalidOperationException>(TrackGraph).Message);
}
else
{
TrackGraph();

Assert.Equal(
trackNewDependents
? new List<string>
{
"<None> -----> Category:1",
"Category:1 ---Products--> Product:1",
"Category:1 ---Products--> Product:2",
"Category:1 ---Products--> Product:3"
}
: new List<string>
{
"<None> -----> 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<Kontainer>()
.Include(container => container.Rooms)
.ThenInclude(room => room.Troduct)
.AsNoTracking()
.Single();
}

using (var context = new KontainerContext(databaseName))
{
var attachedContainer = context.Set<Kontainer>()
.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<KontainerRoom> Rooms { get; set; } = new List<KontainerRoom>();
}

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<KontainerRoom> Rooms { get; set; } = new List<KontainerRoom>();
}

private class KontainerContext : DbContext
{
private readonly string _databaseName;

public KontainerContext(string databaseName)
{
_databaseName = databaseName;
}

protected internal override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<KontainerRoom>()
.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)]
Expand Down
Loading