Skip to content

Commit

Permalink
Revert 3.0 fix for consistent reload and replace with fix that doesn'…
Browse files Browse the repository at this point in the history
…t dramatically change detach behavior

Fixes #17784

In 3.0 we made a [simple change](7e65b82#diff-7767de8ee880eb46efbfc9d2153dd33a) for #15645 that started clearing navigation properties when they referenced detached entities. In retrospect this was the wrong thing to do as people are relying on this to be able to detach all entities from the context.

The new fix restores the pre-3.0 detach behavior, and sends the reload of a deleted entity through a simulated to delete instead.
  • Loading branch information
ajcvickers committed Oct 22, 2019
1 parent f325945 commit 5844092
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 24 deletions.
1 change: 1 addition & 0 deletions src/EFCore/ChangeTracking/EntityEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ private void Reload(PropertyValues storeValues)
{
if (State != EntityState.Added)
{
State = EntityState.Deleted;
State = EntityState.Detached;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore/ChangeTracking/Internal/NavigationFixer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,8 @@ public virtual void StateChanged(
{
InitialFixup(entry, fromQuery: false);
}
else if (entry.EntityState == EntityState.Detached)
else if (oldState == EntityState.Deleted
&& entry.EntityState == EntityState.Detached)
{
DeleteFixup(entry);
}
Expand Down
42 changes: 21 additions & 21 deletions test/EFCore.Specification.Tests/GraphUpdatesTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -836,7 +836,7 @@ public virtual void Detaching_principal_entity_will_remove_references_to_it()
}

[ConditionalFact]
public virtual void Detaching_dependent_entity_will_remove_references_to_it()
public virtual void Detaching_dependent_entity_will_not_remove_references_to_it()
{
ExecuteWithStrategyInTransaction(
context =>
Expand Down Expand Up @@ -944,26 +944,26 @@ public virtual void Detaching_dependent_entity_will_remove_references_to_it()
Assert.True(requiredChildrenAk.All(e => e.Parent == root));
Assert.True(requiredCompositeChildren.All(e => e.Parent == root));
Assert.Null(root.OptionalSingle);
Assert.Null(root.RequiredSingle);
Assert.Null(root.OptionalSingleAk);
Assert.Null(root.OptionalSingleDerived);
Assert.Null(root.RequiredSingleAk);
Assert.Null(root.OptionalSingleAkDerived);
Assert.Null(root.OptionalSingleMoreDerived);
Assert.Null(root.RequiredNonPkSingle);
Assert.Null(root.OptionalSingleAkMoreDerived);
Assert.Null(root.RequiredNonPkSingleAk);
Assert.Null(root.RequiredNonPkSingleDerived);
Assert.Null(root.RequiredNonPkSingleAkDerived);
Assert.Null(root.RequiredNonPkSingleMoreDerived);
Assert.Null(root.RequiredNonPkSingleAkMoreDerived);
Assert.DoesNotContain(optionalChild, root.OptionalChildren);
Assert.DoesNotContain(requiredChild, root.RequiredChildren);
Assert.DoesNotContain(optionalChildAk, root.OptionalChildrenAk);
Assert.DoesNotContain(requieredChildAk, root.RequiredChildrenAk);
Assert.DoesNotContain(requiredCompositeChild, root.RequiredCompositeChildren);
Assert.NotNull(root.OptionalSingle);
Assert.NotNull(root.RequiredSingle);
Assert.NotNull(root.OptionalSingleAk);
Assert.NotNull(root.OptionalSingleDerived);
Assert.NotNull(root.RequiredSingleAk);
Assert.NotNull(root.OptionalSingleAkDerived);
Assert.NotNull(root.OptionalSingleMoreDerived);
Assert.NotNull(root.RequiredNonPkSingle);
Assert.NotNull(root.OptionalSingleAkMoreDerived);
Assert.NotNull(root.RequiredNonPkSingleAk);
Assert.NotNull(root.RequiredNonPkSingleDerived);
Assert.NotNull(root.RequiredNonPkSingleAkDerived);
Assert.NotNull(root.RequiredNonPkSingleMoreDerived);
Assert.NotNull(root.RequiredNonPkSingleAkMoreDerived);
Assert.Contains(optionalChild, root.OptionalChildren);
Assert.Contains(requiredChild, root.RequiredChildren);
Assert.Contains(optionalChildAk, root.OptionalChildrenAk);
Assert.Contains(requieredChildAk, root.RequiredChildrenAk);
Assert.Contains(requiredCompositeChild, root.RequiredCompositeChildren);
});
}

Expand Down
4 changes: 3 additions & 1 deletion test/EFCore.Specification.Tests/PropertyValuesTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1301,7 +1301,9 @@ public async Task Reload_when_entity_deleted_in_store_can_happen_for_any_state(E
{
Assert.Equal(EntityState.Detached, entry.State);
Assert.Null(mailRoom.Building);
Assert.Same(state == EntityState.Deleted ? building : null, office.Building);

Assert.Equal(EntityState.Detached, context.Entry(office.Building).State);
Assert.Same(building, office.Building);
}

Assert.Same(mailRoom, building.PrincipalMailRoom);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ public virtual void Save_required_one_to_one_changed_by_reference(ChangeMechanis
Assert.Same(root, new1.Root);
Assert.Same(new1, new2.Back);
Assert.Null(old1.Root);
Assert.NotNull(old1.Root);
Assert.Null(old2.Back);
Assert.Equal(old1.Id, old2.Id);
});
Expand Down

0 comments on commit 5844092

Please sign in to comment.