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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -191,15 +191,23 @@ public override async Task<TPrimaryKey> InsertOrUpdateAndGetIdAsync(TEntity enti

public override TEntity Update(TEntity entity)
{
AttachIfNot(entity);
Context.Entry(entity).State = EntityState.Modified;
// only if entity had to be attached, we do set its state to "Modified"
// this ensures, that unnecessary calls to "Update" result in the whole entity being set "dirty"
// as opposed to just the properties that changed!
if(AttachIfNot(entity))
Context.Entry(entity).State = EntityState.Modified;

return entity;
}

public override Task<TEntity> UpdateAsync(TEntity entity)
{
AttachIfNot(entity);
Context.Entry(entity).State = EntityState.Modified;
// only 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!
if(AttachIfNot(entity))
Context.Entry(entity).State = EntityState.Modified;

return Task.FromResult(entity);
}

Expand Down Expand Up @@ -248,12 +256,15 @@ public override async Task<long> LongCountAsync(Expression<Func<TEntity, bool>>
return await query.Where(predicate).LongCountAsync(CancellationTokenProvider.Token);
}

protected virtual void AttachIfNot(TEntity entity)
protected virtual bool AttachIfNot(TEntity entity)
{
if (!Table.Local.Contains(entity))
{
Table.Attach(entity);
return true;
}

return false;
}

public DbContext GetDbContext()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ public virtual EntityChangeSet CreateEntityChangeSet(DbContext context)
changeSet.EntityChanges.Add(entityChange);
}

// 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 :-|

}

return changeSet;
}

Expand Down Expand Up @@ -154,7 +160,7 @@ protected virtual string GetEntityId(DbEntityEntry entityEntry, EntityType entit
var property = entityEntry.Property(primaryKey.Name);
return (property.GetNewValue() ?? property.GetOriginalValue())?.ToJsonString();
}

[CanBeNull]
private EntityChange CreateEntityChange(DbEntityEntry entityEntry, EntityType entityType)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ public void Should_Write_History_For_Audited_Property_Foreign_Key_Collection()

Predicate<EntityChangeSet> predicate = s =>
{
s.EntityChanges.Count.ShouldBe(2);
s.EntityChanges.Count.ShouldBe(1, "only the FK on post10 was set, so blog1 should not be modified");

/* Post is not in Configuration.Selectors */
/* Post.Blog has Audited attribute */
Expand All @@ -528,9 +528,8 @@ public void Should_Write_History_For_Audited_Property_Foreign_Key_Collection()
propertyChange1.NewValue.ShouldNotBeNull();

/* Blog has Audited attribute. */
var entityChangeBlog = s.EntityChanges.Single(ec => ec.EntityTypeFullName == typeof(Blog).FullName);
entityChangeBlog.ChangeType.ShouldBe(EntityChangeType.Updated);
entityChangeBlog.PropertyChanges.Count.ShouldBe(0);
var entityChangeBlog = s.EntityChanges.SingleOrDefault(ec => ec.EntityTypeFullName == typeof(Blog).FullName);
entityChangeBlog.ShouldBeNull("only the FK on post10 was set, so blog1 should not be modified");

return true;
};
Expand Down