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.

Sign up for GitHub

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

Jump to bottom

Detach entity with same identity when attaching an equivalent entity as Unchanged #19477

Merged
merged 1 commit into from
Jan 7, 2020
Merged

Detach entity with same identity when attaching an equivalent entity as Unchanged #19477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,11 @@ private void SetEntityState(EntityState oldState, EntityState newState, bool acc

FireStateChanged(oldState);

if (newState == EntityState.Unchanged)
{
SharedIdentityEntry?.SetEntityState(EntityState.Detached);
}

if ((newState == EntityState.Deleted
|| newState == EntityState.Detached)
&& StateManager.CascadeDeleteTiming == CascadeTiming.Immediate)
Expand Down
86 changes: 86 additions & 0 deletions test/EFCore.Specification.Tests/GraphUpdatesTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,18 @@ public virtual void Changes_to_Added_relationships_are_picked_up(ChangeMechanism
Assert.Null(entity.RootId);
Assert.Null(entity.Root);

Assert.True(context.ChangeTracker.HasChanges());

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());

id = entity.Id;
},
context =>
{
var entity = context.Set<OptionalSingle1>().Include(e => e.Root).Single(e => e.Id == id);

Assert.Null(entity.Root);
Assert.Null(entity.RootId);
});
Expand Down Expand Up @@ -111,6 +116,8 @@ public virtual void New_FK_is_not_cleared_on_old_dependent_delete(

context.Remove(removed);

Assert.True(context.ChangeTracker.HasChanges());

if (Fixture.ForceClientNoAction)
{
Assert.Throws<DbUpdateException>(() => context.SaveChanges());
Expand All @@ -119,6 +126,8 @@ public virtual void New_FK_is_not_cleared_on_old_dependent_delete(
{
context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());

Assert.Equal(EntityState.Detached, context.Entry(removed).State);
Assert.Equal(newFk, child.ParentId);

Expand Down Expand Up @@ -154,6 +163,8 @@ public virtual void New_FK_is_not_cleared_on_old_dependent_delete(
{
Assert.Null((child.Parent));
}

Assert.False(context.ChangeTracker.HasChanges());
}
});
}
Expand All @@ -173,8 +184,12 @@ public virtual void Optional_One_to_one_relationships_are_one_to_one(

var root = context.Set<Root>().Single(IsTheRoot);

Assert.False(context.ChangeTracker.HasChanges());

root.OptionalSingle = new OptionalSingle1();

Assert.True(context.ChangeTracker.HasChanges());

Assert.Throws<DbUpdateException>(() => context.SaveChanges());
});
}
Expand All @@ -194,8 +209,12 @@ public virtual void Required_One_to_one_relationships_are_one_to_one(

var root = context.Set<Root>().Single(IsTheRoot);

Assert.False(context.ChangeTracker.HasChanges());

root.RequiredSingle = new RequiredSingle1();

Assert.True(context.ChangeTracker.HasChanges());

Assert.Throws<DbUpdateException>(() => context.SaveChanges());
});
}
Expand All @@ -215,8 +234,12 @@ public virtual void Optional_One_to_one_with_AK_relationships_are_one_to_one(

var root = context.Set<Root>().Single(IsTheRoot);

Assert.False(context.ChangeTracker.HasChanges());

root.OptionalSingleAk = new OptionalSingleAk1();

Assert.True(context.ChangeTracker.HasChanges());

Assert.Throws<DbUpdateException>(() => context.SaveChanges());
});
}
Expand All @@ -236,8 +259,12 @@ public virtual void Required_One_to_one_with_AK_relationships_are_one_to_one(

var root = context.Set<Root>().Single(IsTheRoot);

Assert.False(context.ChangeTracker.HasChanges());

root.RequiredSingleAk = new RequiredSingleAk1();

Assert.True(context.ChangeTracker.HasChanges());

Assert.Throws<DbUpdateException>(() => context.SaveChanges());
});
}
Expand All @@ -256,12 +283,16 @@ public virtual void No_fixup_to_Deleted_entities(
var root = LoadOptionalGraph(context);
var existing = root.OptionalChildren.OrderBy(e => e.Id).First();

Assert.False(context.ChangeTracker.HasChanges());

existing.Parent = null;
existing.ParentId = null;
((ICollection<Optional1>)root.OptionalChildren).Remove(existing);

context.Entry(existing).State = EntityState.Deleted;

Assert.True(context.ChangeTracker.HasChanges());

var queried = context.Set<Optional1>().ToList();

Assert.Null(existing.Parent);
Expand Down Expand Up @@ -737,8 +768,12 @@ public virtual void Notification_entities_can_have_indexes()

Assert.Equal(EntityState.Added, context.Entry(produce).State);

Assert.True(context.ChangeTracker.HasChanges());

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());

Assert.Equal(EntityState.Unchanged, context.Entry(produce).State);
Assert.NotEqual(Guid.Empty, context.Entry(produce).Property(e => e.ProduceId).OriginalValue);
Assert.Equal(77, context.Entry(produce).Property(e => e.BarCode).OriginalValue);
Expand All @@ -748,8 +783,12 @@ public virtual void Notification_entities_can_have_indexes()
Assert.NotEqual(Guid.Empty, context.Entry(produce).Property(e => e.ProduceId).OriginalValue);
Assert.Equal(77, context.Entry(produce).Property(e => e.BarCode).OriginalValue);

Assert.True(context.ChangeTracker.HasChanges());

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());

Assert.Equal(EntityState.Detached, context.Entry(produce).State);
});
}
Expand All @@ -770,6 +809,8 @@ public virtual void Resetting_a_deleted_reference_fixes_up_again()

context.Remove(bloog);

Assert.True(context.ChangeTracker.HasChanges());

Assert.Equal(2, bloog.Poosts.Count());

if (Fixture.ForceClientNoAction)
Expand Down Expand Up @@ -815,8 +856,12 @@ public virtual void Resetting_a_deleted_reference_fixes_up_again()

if (!Fixture.ForceClientNoAction)
{
Assert.True(context.ChangeTracker.HasChanges());

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());

Assert.Equal(2, bloog.Poosts.Count());
Assert.Null(poost1.Bloog);
Assert.Null(poost2.Bloog);
Expand Down Expand Up @@ -875,6 +920,8 @@ public virtual void Detaching_principal_entity_will_remove_references_to_it()
Assert.True(root.RequiredChildrenAk.All(e => e.Parent == root));
Assert.True(root.RequiredCompositeChildren.All(e => e.Parent == root));

Assert.False(context.ChangeTracker.HasChanges());

context.Entry(optionalSingle).State = EntityState.Detached;
context.Entry(requiredSingle).State = EntityState.Detached;
context.Entry(optionalSingleAk).State = EntityState.Detached;
Expand All @@ -890,6 +937,8 @@ public virtual void Detaching_principal_entity_will_remove_references_to_it()
context.Entry(requiredNonPkSingleMoreDerived).State = EntityState.Detached;
context.Entry(requiredNonPkSingleAkMoreDerived).State = EntityState.Detached;

Assert.False(context.ChangeTracker.HasChanges());

Assert.NotNull(optionalSingle.Root);
Assert.NotNull(requiredSingle.Root);
Assert.NotNull(optionalSingleAk.Root);
Expand Down Expand Up @@ -975,6 +1024,8 @@ public virtual void Detaching_dependent_entity_will_not_remove_references_to_it(
Assert.True(requiredChildrenAk.All(e => e.Parent == root));
Assert.True(requiredCompositeChildren.All(e => e.Parent == root));

Assert.False(context.ChangeTracker.HasChanges());

context.Entry(optionalSingle).State = EntityState.Detached;
context.Entry(requiredSingle).State = EntityState.Detached;
context.Entry(optionalSingleAk).State = EntityState.Detached;
Expand All @@ -1001,6 +1052,8 @@ public virtual void Detaching_dependent_entity_will_not_remove_references_to_it(

context.Entry(requiredCompositeChild).State = EntityState.Detached;

Assert.False(context.ChangeTracker.HasChanges());

Assert.Same(root, optionalSingle.Root);
Assert.Same(root, requiredSingle.Root);
Assert.Same(root, optionalSingleAk.Root);
Expand Down Expand Up @@ -2232,6 +2285,8 @@ public virtual void Reparent_required_one_to_one(

var root = LoadRequiredGraph(context);

Assert.False(context.ChangeTracker.HasChanges());

context.Entry(newRoot).State = useExistingRoot ? EntityState.Unchanged : EntityState.Added;

Assert.Equal(
Expand Down Expand Up @@ -2547,6 +2602,8 @@ public virtual void Reparent_to_different_one_to_many(
{
var loadedRoot = LoadOptionalOneToManyGraph(context);

Assert.False(context.ChangeTracker.HasChanges());

AssertKeys(root, loadedRoot);
AssertNavigations(loadedRoot);

Expand Down Expand Up @@ -3708,7 +3765,11 @@ public virtual void Save_changed_optional_one_to_one_with_alternate_key_in_store
root2.OptionalSingleAkDerived = new1d;
root2.OptionalSingleAkMoreDerived = new1dd;

Assert.True(context2.ChangeTracker.HasChanges());

context2.SaveChanges();

Assert.False(context2.ChangeTracker.HasChanges());
}

new1 = context.Set<OptionalSingleAk1>().Single(e => e.Id == new1.Id);
Expand Down Expand Up @@ -3751,8 +3812,12 @@ public virtual void Save_changed_optional_one_to_one_with_alternate_key_in_store
Assert.Equal(old1d.AlternateId, old2d.BackId);
Assert.Equal(old1dd.AlternateId, old2dd.BackId);

Assert.True(context.ChangeTracker.HasChanges());

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());

Assert.Equal(root.AlternateId, new1.RootId);
Assert.Equal(root.AlternateId, new1d.DerivedRootId);
Assert.Equal(root.AlternateId, new1dd.MoreDerivedRootId);
Expand Down Expand Up @@ -8673,8 +8738,12 @@ public virtual void Re_childing_parent_to_new_child_with_delete(
var newChild = new ChildAsAParent();
parent.ChildAsAParent = newChild;

Assert.True(context.ChangeTracker.HasChanges());

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());

if (cascadeDeleteTiming == null)
{
context.ChangeTracker.CascadeChanges();
Expand All @@ -8693,6 +8762,7 @@ public virtual void Re_childing_parent_to_new_child_with_delete(
context =>
{
var parent = context.Set<ParentAsAChild>().Include(p => p.ChildAsAParent).Single();

Assert.Equal(newId, parent.ChildAsAParentId);
Assert.Equal(newId, parent.ChildAsAParent.Id);
Assert.Null(context.Set<ChildAsAParent>().Find(oldId));
Expand All @@ -8717,8 +8787,12 @@ public virtual void Sometimes_not_calling_DetectChanges_when_required_does_not_t
Assert.Null(dependent.BadCustomer);
Assert.Empty(principal.BadOrders);

Assert.True(context.ChangeTracker.HasChanges());

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());

Assert.Null(dependent.BadCustomerId);
Assert.Null(dependent.BadCustomer);
Assert.Empty(principal.BadOrders);
Expand All @@ -8745,7 +8819,11 @@ public virtual void Can_add_valid_first_dependent_when_multiple_possible_princip

context.Add(quizTask);

Assert.True(context.ChangeTracker.HasChanges());

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());
},
context =>
{
Expand All @@ -8770,7 +8848,11 @@ public virtual void Can_add_valid_second_dependent_when_multiple_possible_princi

context.Add(hiddenAreaTask);

Assert.True(context.ChangeTracker.HasChanges());

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());
},
context =>
{
Expand Down Expand Up @@ -8802,7 +8884,11 @@ public virtual void Can_add_multiple_dependents_when_multiple_possible_principal

context.Add(hiddenAreaTask);

Assert.True(context.ChangeTracker.HasChanges());

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());
},
context =>
{
Expand Down
12 changes: 12 additions & 0 deletions test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,8 @@ public void Clearing_optional_FK_does_not_leave_temporary_value()
var product = new OptionalProduct();
context.Add(product);

Assert.True(context.ChangeTracker.HasChanges());

var productEntry = context.Entry(product);
Assert.Equal(EntityState.Added, productEntry.State);

Expand All @@ -530,6 +532,8 @@ public void Clearing_optional_FK_does_not_leave_temporary_value()

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());

productEntry = context.Entry(product);
Assert.Equal(EntityState.Unchanged, productEntry.State);

Expand All @@ -544,6 +548,8 @@ public void Clearing_optional_FK_does_not_leave_temporary_value()
var category = new OptionalCategory();
product.Category = category;

Assert.True(context.ChangeTracker.HasChanges());

productEntry = context.Entry(product);
Assert.Equal(EntityState.Modified, productEntry.State);

Expand All @@ -563,6 +569,8 @@ public void Clearing_optional_FK_does_not_leave_temporary_value()

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());

productEntry = context.Entry(product);
Assert.Equal(EntityState.Unchanged, productEntry.State);

Expand Down Expand Up @@ -598,8 +606,12 @@ public void Clearing_optional_FK_does_not_leave_temporary_value()
Assert.Equal(1, category.Id);
Assert.Equal(1, categoryEntry.Property(e => e.Id).CurrentValue);

Assert.True(context.ChangeTracker.HasChanges());

context.SaveChanges();

Assert.False(context.ChangeTracker.HasChanges());

productEntry = context.Entry(product);
Assert.Equal(EntityState.Unchanged, productEntry.State);

Expand Down
Loading