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

Strange Behaviour on detaching entries #17784

Closed
arturgromek opened this issue Sep 12, 2019 · 8 comments · Fixed by #18515
Closed

Strange Behaviour on detaching entries #17784

arturgromek opened this issue Sep 12, 2019 · 8 comments · Fixed by #18515
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression type-bug
Milestone

Comments

@arturgromek
Copy link

On detaching entries from the change tracker, I am getting an Invalid Operation Exception. It seems that detaching entries from the tracker is setting foreign keys or parents to null in child objects. In the past(preview 8) detaching has only stopped tracking the entries, without modifying/altering any properties.

Note: Nullable Reference Types is not enabled in this project

Steps to reproduce

Add multiple linked entries to a dbcontext using Update.
SaveChanges.
Clear entries with:

var entries = DbContext.ChangeTracker.Entries();
foreach (var entry in entries)
{
    entry.State = Microsoft.EntityFrameworkCore.EntityState.Detached;
}

Relevant modelbuilder code:

entity.HasOne(d => d.Person)
    .WithMany(p => p.Entrant)
    .HasForeignKey(d => d.PersonId)
    .OnDelete(DeleteBehavior.ClientSetNull)
    .HasConstraintName("PersonEntrant");

Exception

'The association between entity types 'Person' and 'Entrant' has been severed but the relationship is either marked as 'Required' or is implicitly required because the foreign key is not nullable. If the dependent/child entity should be deleted when a required relationship is severed, then setup the relationship to use cascade deletes.  Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values.'
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.HandleConceptualNulls(Boolean sensitiveLoggingEnabled, Boolean force, Boolean isCascadeDelete)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.CascadeDelete(InternalEntityEntry entry, Boolean force, IEnumerable`1 foreignKeys)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState entityState, Boolean acceptChanges, Boolean modifyProperties, Nullable`1 forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.ChangeTracking.EntityEntry.set_State(EntityState value)
   at [MyFile].<Commit>d__10.MoveNext() in [MyFile]:line 62

Further technical details

EF Core version: 3.0.0-preview9.19423.6
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.0
Operating system: Windows 10 64 bit
IDE: Visual Studio 2019 16.3

@AndriySvyryd
Copy link
Member

This is a change in behavior for 3.0

You can mitigate it:

context.ChangeTracker.CascadeDeleteTiming = CascadeTiming.OnSaveChanges;
context.ChangeTracker.DeleteOrphansTiming = CascadeTiming.OnSaveChanges;

@arturgromek
Copy link
Author

I was under the impression that detaching is a completely seperate action to deletion. Doesn't the detached state only relate to the entity not being tracked?

I am only intending to stop the context from tracking these entries, not modify anything about them, or their relationships.

Also that change has been in since preview 3 according to the docs, but the change I've experienced has only happened since preview 9.

@Confusedfish
Copy link

I have just upgraded from preview-6 to preview-9 and am hitting this problem.

I am caching user permissions, using the UserManager to get a user's role membership and then detaching the user from the change tracker. Rather alarmingly, a short while later and the user's role membership is deleted from the DB. The log shows:

The 'IdentityUserRole' entity with key '{UserId: ..., RoleId: ...}' tracked by 'ApplicationDbContext' changed from 'Deleted' to 'Detached'.

I can confirm that the mitigation steps outlined above do get around the problem but agree that this is some sort of regression as I didn't have the issue when I was on preview-6.

resp.User = await _userManager.FindByIdAsync(userId); // These two lines get round the problem //_db.ChangeTracker.CascadeDeleteTiming = CascadeTiming.OnSaveChanges; //_db.ChangeTracker.DeleteOrphansTiming = CascadeTiming.OnSaveChanges; resp.IsAdmin = await _userManager.IsInRoleAsync(resp.User, "Admin"); _db.Entry(resp.User).State = EntityState.Detached;

@jwbats
Copy link

jwbats commented Oct 17, 2019

I've come across this too. Detaching entities after saving will now remove their nav props.

This causes me to have to re-retrieve them. I wish there were a way of turning off tracking globally, not just for queries, but also for saving.

I've never used EF's tracking capability. I consider it a hassle that gets in my way. I never want to run into any entity tracking collissions, so no tracking for me.

I have an Update function in a generic repository, which I use to call Update(), SaveChanges() and then DetachAll().

I want to be able to send in whatever entity I like, in whatever state I like, at any time I like. I want it to come out saved to the database and still having its nav props, so I can continue to do more things with it in the calling code.

Real life use case I am currently dealing with:

A 'Package' entity is an entity that defines a package of graphical social media template designs. A user can view a package page. Viewing this must also increase the NrViews property on the package.

Serverside, it should go like this:

  1. Retrieve Package, including nav props, which I intend to serialize to the clientside, because they are used there.
  2. Increase NrViews on the Package.
  3. Update/Save/DetachAll the Package so that the increased NrViews is now in the DB.
  4. Package including nav props is serialized to client side, where it is used to build the page.

As of EF3, I now have to do it like this:

  1. Retrieve Package including nav props.
  2. Increase NrViews.
  3. Update/Save/DetachAll.
  4. Re-retrieve Package including nav props.
  5. Serialize to client.

Is this really by design?

Strange observation related to this new behavior:

  1. Create entity including nav props.
  2. Update/Save/DetachAll.
  3. Entity is stripped of nav props.

Now drag the debug pointer back to 1, and then this happens:

  1. Create entity including nav props.
  2. Update/Save/DetachAll.
  3. Entity still has nav props.

Why would EF3 behave differently the 2nd time of doing the exact same thing?

This leads me to believe something buggy is going on.

@AndriySvyryd AndriySvyryd removed their assignment Oct 18, 2019
@tom-mckinney
Copy link

+1 Detaching an entity from the change tracker should not trigger a cascading delete. This behavior does not seem to align with the description of the change in the documentation.

@jwbats
Copy link

jwbats commented Oct 22, 2019

@tom-mckinney You can turn that part off. The solution is posted in this very thread.

It's the removal from the nav props that's grinding my gears.

ajcvickers added a commit that referenced this issue Oct 22, 2019
…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.
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 22, 2019
@ajcvickers
Copy link
Contributor

I have just submitted a PR that reverts the behavior change in 3.0 where navigation properties are cleared when they reference an entity that is detached.

That being said, detaching entities on mass is a very slow process--much slower than creating a new context instance. If creating a new instance is too hard based on other design constraints in the application, then #15577 when implemented would be a better choice.

@ajcvickers
Copy link
Contributor

@jwbats Following up your comments here and on #18406. Considering this scenario:

Retrieve Package, including nav props, which I intend to serialize to the clientside, because they are used there.
Increase NrViews on the Package.
Update/Save/DetachAll the Package so that the increased NrViews is now in the DB.
Package including nav props is serialized to client side, where it is used to build the page.

This implies that you are first using a no-tracking query, then attaching the graph with Update before calling SaveChanges. This is not a recommended way of doing things. First, performing a no-tracking query followed by a call to Update to start tracking is less efficient than doing a tracking query in the first place. Second, calling Update marks the entire graph of entities as modified, which means SaveChanges will send updates to the database even for entities/properties that have not changed. Note that Update is not required at all in this case. Third, other information can also be lost in certain cases when attached a graph of untracked entities. For example, any shadow FK properties will need to be handled appropriately.

Now drag the debug pointer back to 1

I never trust this. How do you really know what the state of everything is?

First, what you're trying to tell me is that you're supposed to call Update() once on an entity to add it to the context, then SaveChanges() to actually save the changes and also track it... and then you can make more changes to the now-tracked object, and then you only have to call SaveChanges() if you want to save further changes. Do I understand that correctly?

Not completely. Update is a way to start tracking untracked entities such that they are marked as modified. Update is not needed when the entities are retrieved with a tracking query since EF is tracking which changes have been made and therefore which entities/properties need to be marked as modified. SaveChanges works with tracked entities; it doesn't start tracking anything itself.

Once SaveChanges has been called it is fine to make further changes, which will then also be detected, and then will be saved on the next call to SaveChanges. However, this can be complicated to manage over a long-lived context with many disparate changes, which is why one context for one unit-of-work is recommended.

ajcvickers added a commit that referenced this issue Oct 22, 2019
…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.
@ajcvickers ajcvickers modified the milestones: 3.1.0, 3.1.0-preview2 Oct 24, 2019
@ajcvickers ajcvickers modified the milestones: 3.1.0-preview2, 3.1.0 Dec 2, 2019
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression type-bug
Projects
None yet
7 participants