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

#6560 - EFRepositoryBase - Unnecessary call to Update or UpdateAsync causes whole entity to be marked dirty #6561

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

gentledepp
Copy link

#6560 - EFRepositoryonlyBase - if entity had to be attached, we do set its state to "Modified"

this ensures, that unnecessary calls to "UpdateAsync" result in the whole entity being set "dirty" as opposed to just the properties that changed!

@ismcagdas ismcagdas added this to the v8.0 milestone Oct 10, 2022
@ismcagdas ismcagdas modified the milestones: v8.0, v8.1 Dec 8, 2022
@ismcagdas
Copy link
Member

@ismcagdas ismcagdas modified the milestones: v8.1, v8.2 Feb 16, 2023
@gentledepp gentledepp force-pushed the bugfix/6560_donotsetattachedentitytomodified branch from 344e852 to e8e683e Compare April 26, 2023 08:56
@gentledepp
Copy link
Author

Please check my changes on the tests. I tried to add comments to make my point clear.

  1. Should_Write_History_For_Audited_Property_Foreign_Key_Collection
    Here, the test was wrong: it expected the Blog to be changed. But since only the FK on post was edited, there was actually no change on Blog.
    I am not sure though, if this is a requirement (having an entitychange entry for the blog with changetype "Updated" and zero edited properties)

  2. Should_Write_History_For_Audited_Property_Foreign_Key_Shadow
    This test fails because of a bug in the EntityHistoryHelper!

The line comment1.Post = post2 is expected to change the entity comment1.
However, as there is no FK (PostId) defined, the entity is not touched.
Rather, the Relationship is changed.
You can read more about this here:
https://stackoverflow.com/questions/11670212/entity-framework-wont-detect-changes-of-navigation-properties/11671707#11671707

How should this be resolved?
Should we also go through the changed relationships?
In this case, an entitychange entry for the blog with changetype "Updated" and zero edited properties for testcase 1 would also be possible

…ate to "Modified"

this ensures, that unnecessary calls to "UpdateAsync" result in the whole entity being set "dirty" as opposed to just the properties that changed!
@gentledepp gentledepp force-pushed the bugfix/6560_donotsetattachedentitytomodified branch from e8e683e to 798cd96 Compare April 26, 2023 09:38
// in case no entity was changed, but only a relationship between entities...
foreach(var relationshipOnlyChange in relationshipChanges.Where(rc => context.ChangeTracker.Entries().All(ent => ent.Entity != rc.Entity)))
{
// TODO: implement this
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem lies here.
In case only a relationship was changed, but none of the actual entities, we need to track the changes too.

Because comments and posts are connected using an implicit N:M table (Comments_Posts) I have no way to get all the details necessary, though.
Can you give me a suggestion in how to implement this?

I would need:
to get from the Comments_Posts EntitySet to the entities Comment and Post, but the API of ObjectStateEntry seems not to allow for that.
Otherwise I have no way of knowing whether one of those entities is actually audited or not :-|

@ismcagdas ismcagdas modified the milestones: v8.2, v9.0 May 2, 2023
@ismcagdas ismcagdas modified the milestones: v9.0, v9.1 Nov 15, 2023
@ismcagdas ismcagdas modified the milestones: v9.1, Backlog Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants