Skip to content

Commit

Permalink
Stop deleting orphans for optional relationships configured for casca…
Browse files Browse the repository at this point in the history 
…de deletes

Fixes #27217
Fixes #27218

The 5.0/6.0 behavior here is:
- Orphans are deleted if the relationship is severed by navigation property
- Orphans are not deleted if the relationship is severed by setting the FK to null

This change means orphans are not deleted in either case. The change that causes this to happen is to not set a conceptual null for a nullable FK property. Deletion can still be forced by explicitly forcing a conceptual null or just by setting the dependent state explicitly to Deleted.
  • Loading branch information
@ajcvickers
ajcvickers committed Mar 2, 2022
1 parent 786cb40 commit 2e64a62
Show file tree
Hide file tree
Showing 2 changed files with 267 additions and 29 deletions.
3 changes: 2 additions & 1 deletion src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1250,7 +1250,8 @@ private void SetProperty(
&& valueType == CurrentValueType.Normal
&& (!asProperty.ClrType.IsNullableType()
|| asProperty.GetContainingForeignKeys().Any(
fk => (fk.DeleteBehavior == DeleteBehavior.Cascade
fk => fk.IsRequired
&& (fk.DeleteBehavior == DeleteBehavior.Cascade
|| fk.DeleteBehavior == DeleteBehavior.ClientCascade)
&& fk.DeclaringEntityType.IsAssignableFrom(EntityType))))
{
Expand Down
293 changes: 265 additions & 28 deletions test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2016,27 +2016,70 @@ public void Dependent_FKs_are_not_nulled_when_principal_is_detached(bool delayCa
}
}

[ConditionalTheory] // Issues #16546 #25360; Change reverted in #27174.
[InlineData(false, false, false, true, false)]
[InlineData(true, false, false, true, false)]
[InlineData(false, true, false, true, false)]
[InlineData(true, true, false, true, false)]
[InlineData(false, false, true, true, false)]
[InlineData(true, false, true, true, false)]
[InlineData(false, true, false, false, true)]
[InlineData(true, true, false, false, true)]
[InlineData(false, false, true, false, true)]
[InlineData(true, false, true, false, true)]
[InlineData(false, true, false, true, true)]
[InlineData(true, true, false, true, true)]
[InlineData(false, false, true, true, true)]
[InlineData(true, false, true, true, true)]
public void Optional_relationship_with_cascade_still_cascades(
bool delayCascade,
[ConditionalTheory] // Issues #16546 #25360; Change reverted in #27174.
[InlineData(null, false, false, true, false, false)]
[InlineData(null, true, false, true, false, false)]
[InlineData(null, false, true, true, false, false)]
[InlineData(null, true, false, false, true, false)]
[InlineData(null, false, true, false, true, false)]
[InlineData(null, true, false, true, true, false)]
[InlineData(null, false, true, true, true, false)]
[InlineData(CascadeTiming.Immediate, false, false, true, false, false)]
[InlineData(CascadeTiming.Immediate, true, false, true, false, false)]
[InlineData(CascadeTiming.Immediate, false, true, true, false, false)]
[InlineData(CascadeTiming.Immediate, true, false, false, true, false)]
[InlineData(CascadeTiming.Immediate, false, true, false, true, false)]
[InlineData(CascadeTiming.Immediate, true, false, true, true, false)]
[InlineData(CascadeTiming.Immediate, false, true, true, true, false)]
[InlineData(CascadeTiming.OnSaveChanges, false, false, true, false, false)]
[InlineData(CascadeTiming.OnSaveChanges, true, false, true, false, false)]
[InlineData(CascadeTiming.OnSaveChanges, false, true, true, false, false)]
[InlineData(CascadeTiming.OnSaveChanges, true, false, false, true, false)]
[InlineData(CascadeTiming.OnSaveChanges, false, true, false, true, false)]
[InlineData(CascadeTiming.OnSaveChanges, true, false, true, true, false)]
[InlineData(CascadeTiming.OnSaveChanges, false, true, true, true, false)]
[InlineData(CascadeTiming.Never, false, false, true, false, false)]
[InlineData(CascadeTiming.Never, true, false, true, false, false)]
[InlineData(CascadeTiming.Never, false, true, true, false, false)]
[InlineData(CascadeTiming.Never, true, false, false, true, false)]
[InlineData(CascadeTiming.Never, false, true, false, true, false)]
[InlineData(CascadeTiming.Never, true, false, true, true, false)]
[InlineData(CascadeTiming.Never, false, true, true, true, false)]
[InlineData(null, false, false, true, false, true)]
[InlineData(null, true, false, true, false, true)]
[InlineData(null, false, true, true, false, true)]
[InlineData(null, true, false, false, true, true)]
[InlineData(null, false, true, false, true, true)]
[InlineData(null, true, false, true, true, true)]
[InlineData(null, false, true, true, true, true)]
[InlineData(CascadeTiming.Immediate, false, false, true, false, true)]
[InlineData(CascadeTiming.Immediate, true, false, true, false, true)]
[InlineData(CascadeTiming.Immediate, false, true, true, false, true)]
[InlineData(CascadeTiming.Immediate, true, false, false, true, true)]
[InlineData(CascadeTiming.Immediate, false, true, false, true, true)]
[InlineData(CascadeTiming.Immediate, true, false, true, true, true)]
[InlineData(CascadeTiming.Immediate, false, true, true, true, true)]
[InlineData(CascadeTiming.OnSaveChanges, false, false, true, false, true)]
[InlineData(CascadeTiming.OnSaveChanges, true, false, true, false, true)]
[InlineData(CascadeTiming.OnSaveChanges, false, true, true, false, true)]
[InlineData(CascadeTiming.OnSaveChanges, true, false, false, true, true)]
[InlineData(CascadeTiming.OnSaveChanges, false, true, false, true, true)]
[InlineData(CascadeTiming.OnSaveChanges, true, false, true, true, true)]
[InlineData(CascadeTiming.OnSaveChanges, false, true, true, true, true)]
[InlineData(CascadeTiming.Never, false, false, true, false, true)]
[InlineData(CascadeTiming.Never, true, false, true, false, true)]
[InlineData(CascadeTiming.Never, false, true, true, false, true)]
[InlineData(CascadeTiming.Never, true, false, false, true, true)]
[InlineData(CascadeTiming.Never, false, true, false, true, true)]
[InlineData(CascadeTiming.Never, true, false, true, true, true)]
[InlineData(CascadeTiming.Never, false, true, true, true, true)]
public void Optional_relationship_with_cascade_does_not_delete_orphans(
CascadeTiming? orphanTiming,
bool setProperty,
bool setCurrentValue,
bool useForeignKey,
bool useNavigation)
bool useNavigation,
bool forceCascade)
{
Kontainer detachedContainer;
using (var context = new KontainerContext())
Expand Down Expand Up @@ -2076,9 +2119,9 @@ public void Optional_relationship_with_cascade_still_cascades(
Assert.Equal(EntityState.Unchanged, context.Entry(attachedRoom).State);
Assert.Equal(EntityState.Unchanged, context.Entry(attachedTroduct).State);

if (delayCascade)
if (orphanTiming != null)
{
context.ChangeTracker.DeleteOrphansTiming = CascadeTiming.OnSaveChanges;
context.ChangeTracker.DeleteOrphansTiming = orphanTiming.Value;
}

if (setProperty)
Expand Down Expand Up @@ -2115,30 +2158,224 @@ public void Optional_relationship_with_cascade_still_cascades(
Assert.Equal(3, context.ChangeTracker.Entries().Count());
Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer).State);
Assert.Equal(EntityState.Unchanged, context.Entry(attachedTroduct).State);
Assert.Equal(EntityState.Modified, context.Entry(attachedRoom).State);

if (delayCascade
|| (useForeignKey && setProperty))
if (forceCascade)
{
Assert.Equal(EntityState.Modified, context.Entry(attachedRoom).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(attachedTroduct).State);
Assert.Equal(EntityState.Modified, context.Entry(attachedRoom).State);

context.SaveChanges();

Assert.Equal(3, context.ChangeTracker.Entries().Count());
Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer).State);
Assert.Equal(EntityState.Unchanged, context.Entry(attachedTroduct).State);
Assert.Equal(EntityState.Unchanged, context.Entry(attachedRoom).State);
}
}

[ConditionalTheory] // Issues #16546 #25360; Change reverted in #27174.
[InlineData(null, false, false, true, false, false)]
[InlineData(null, true, false, true, false, false)]
[InlineData(null, false, true, true, false, false)]
[InlineData(null, true, false, false, true, false)]
[InlineData(null, false, true, false, true, false)]
[InlineData(null, true, false, true, true, false)]
[InlineData(null, false, true, true, true, false)]
[InlineData(CascadeTiming.Immediate, false, false, true, false, false)]
[InlineData(CascadeTiming.Immediate, true, false, true, false, false)]
[InlineData(CascadeTiming.Immediate, false, true, true, false, false)]
[InlineData(CascadeTiming.Immediate, true, false, false, true, false)]
[InlineData(CascadeTiming.Immediate, false, true, false, true, false)]
[InlineData(CascadeTiming.Immediate, true, false, true, true, false)]
[InlineData(CascadeTiming.Immediate, false, true, true, true, false)]
[InlineData(CascadeTiming.OnSaveChanges, false, false, true, false, false)]
[InlineData(CascadeTiming.OnSaveChanges, true, false, true, false, false)]
[InlineData(CascadeTiming.OnSaveChanges, false, true, true, false, false)]
[InlineData(CascadeTiming.OnSaveChanges, true, false, false, true, false)]
[InlineData(CascadeTiming.OnSaveChanges, false, true, false, true, false)]
[InlineData(CascadeTiming.OnSaveChanges, true, false, true, true, false)]
[InlineData(CascadeTiming.OnSaveChanges, false, true, true, true, false)]
[InlineData(CascadeTiming.Never, false, false, true, false, false)]
[InlineData(CascadeTiming.Never, true, false, true, false, false)]
[InlineData(CascadeTiming.Never, false, true, true, false, false)]
[InlineData(CascadeTiming.Never, true, false, false, true, false)]
[InlineData(CascadeTiming.Never, false, true, false, true, false)]
[InlineData(CascadeTiming.Never, true, false, true, true, false)]
[InlineData(CascadeTiming.Never, false, true, true, true, false)]
[InlineData(null, false, false, true, false, true)]
[InlineData(null, true, false, true, false, true)]
[InlineData(null, false, true, true, false, true)]
[InlineData(null, true, false, false, true, true)]
[InlineData(null, false, true, false, true, true)]
[InlineData(null, true, false, true, true, true)]
[InlineData(null, false, true, true, true, true)]
[InlineData(CascadeTiming.Immediate, false, false, true, false, true)]
[InlineData(CascadeTiming.Immediate, true, false, true, false, true)]
[InlineData(CascadeTiming.Immediate, false, true, true, false, true)]
[InlineData(CascadeTiming.Immediate, true, false, false, true, true)]
[InlineData(CascadeTiming.Immediate, false, true, false, true, true)]
[InlineData(CascadeTiming.Immediate, true, false, true, true, true)]
[InlineData(CascadeTiming.Immediate, false, true, true, true, true)]
[InlineData(CascadeTiming.OnSaveChanges, false, false, true, false, true)]
[InlineData(CascadeTiming.OnSaveChanges, true, false, true, false, true)]
[InlineData(CascadeTiming.OnSaveChanges, false, true, true, false, true)]
[InlineData(CascadeTiming.OnSaveChanges, true, false, false, true, true)]
[InlineData(CascadeTiming.OnSaveChanges, false, true, false, true, true)]
[InlineData(CascadeTiming.OnSaveChanges, true, false, true, true, true)]
[InlineData(CascadeTiming.OnSaveChanges, false, true, true, true, true)]
[InlineData(CascadeTiming.Never, false, false, true, false, true)]
[InlineData(CascadeTiming.Never, true, false, true, false, true)]
[InlineData(CascadeTiming.Never, false, true, true, false, true)]
[InlineData(CascadeTiming.Never, true, false, false, true, true)]
[InlineData(CascadeTiming.Never, false, true, false, true, true)]
[InlineData(CascadeTiming.Never, true, false, true, true, true)]
[InlineData(CascadeTiming.Never, false, true, true, true, true)]
public void Optional_relationship_with_cascade_can_be_forced_to_delete_orphans(
CascadeTiming? orphanTiming,
bool setProperty,
bool setCurrentValue,
bool useForeignKey,
bool useNavigation,
bool forceCascade)
{
Kontainer detachedContainer;
using (var context = new KontainerContext())
{
context.Database.EnsureDeleted();
context.Database.EnsureCreated();

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())
{
var attachedContainer = context.Set<Kontainer>()
.Include(container => container.Rooms)
.ThenInclude(room => room.Troduct)
.Single();

var attachedRoom = attachedContainer.Rooms.Single();
var attachedTroduct = attachedRoom.Troduct;

Assert.Equal(3, context.ChangeTracker.Entries().Count());
Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer).State);
Assert.Equal(EntityState.Unchanged, context.Entry(attachedRoom).State);
Assert.Equal(EntityState.Unchanged, context.Entry(attachedTroduct).State);

if (orphanTiming != null)
{
context.ChangeTracker.DeleteOrphansTiming = orphanTiming.Value;
}

if (setProperty)
{
if (useForeignKey)
{
attachedRoom.TroductId = null;
}

if (useNavigation)
{
attachedRoom.Troduct = null;
}
}
else if (setCurrentValue)
{
if (useForeignKey)
{
context.Entry(attachedRoom).Property(e => e.TroductId).CurrentValue = null;
}

if (useNavigation)
{
context.Entry(attachedRoom).Reference(e => e.Troduct).CurrentValue = null;
}
}
else
{
// Deleted because FK with cascade has been set to null
var detachedRoom = detachedContainer.Rooms.Single();
detachedRoom.TroductId = null;
context.Entry(attachedRoom).CurrentValues.SetValues(detachedRoom);
}

context.Entry(attachedRoom).GetInfrastructure()
.HandleNullForeignKey(context.Entry(attachedRoom).Property(e => e.TroductId).Metadata);

Assert.Equal(3, context.ChangeTracker.Entries().Count());
Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer).State);
Assert.Equal(EntityState.Unchanged, context.Entry(attachedTroduct).State);

if (orphanTiming == null
|| orphanTiming == CascadeTiming.Immediate)
{
Assert.Equal(EntityState.Deleted, context.Entry(attachedRoom).State);
}
else
{
Assert.Equal(EntityState.Modified, context.Entry(attachedRoom).State);
}

context.ChangeTracker.CascadeChanges();
if (forceCascade)
{
context.ChangeTracker.CascadeChanges();
}

Assert.Equal(3, context.ChangeTracker.Entries().Count());
Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer).State);
Assert.Equal(EntityState.Unchanged, context.Entry(attachedTroduct).State);

if (orphanTiming == null
|| orphanTiming == CascadeTiming.Immediate
|| forceCascade)
{
Assert.Equal(EntityState.Deleted, context.Entry(attachedRoom).State);
}
else
{
Assert.Equal(EntityState.Modified, context.Entry(attachedRoom).State);
}

if (orphanTiming == CascadeTiming.Never
&& !forceCascade)
{
Assert.Equal(
useForeignKey && setProperty ? EntityState.Modified : EntityState.Deleted,
context.Entry(attachedRoom).State);
CoreStrings.RelationshipConceptualNull(nameof(Troduct), nameof(KontainerRoom)),
Assert.Throws<InvalidOperationException>(
() => context.SaveChanges()).Message);

context.SaveChanges();
Assert.Equal(3, context.ChangeTracker.Entries().Count());
Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer).State);
Assert.Equal(EntityState.Unchanged, context.Entry(attachedTroduct).State);
Assert.Equal(EntityState.Modified, context.Entry(attachedRoom).State);
}
else
{
context.SaveChanges();
Assert.Equal(2, context.ChangeTracker.Entries().Count());
Assert.Equal(EntityState.Unchanged, context.Entry(attachedContainer).State);
Assert.Equal(EntityState.Unchanged, context.Entry(attachedTroduct).State);
Assert.Equal(EntityState.Detached, context.Entry(attachedRoom).State);
}
}
}

Expand Down

0 comments on commit 2e64a62

Please sign in to comment.