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

Accessing Target context via IQueryExpressionInterceptor #30004

Closed
pinkfloydx33 opened this issue Jan 7, 2023 · 4 comments
Closed

Accessing Target context via IQueryExpressionInterceptor #30004

pinkfloydx33 opened this issue Jan 7, 2023 · 4 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@pinkfloydx33
Copy link

pinkfloydx33 commented Jan 7, 2023

Ask a question

Is it possible to access a property of the target context when using the IQueryExpressionInterceptor?

We have a custom DbContext subclass. As part of SaveChanges it iterates over all modified entities in the change tracker, and for those implementing a particular interface it sets a LastModifiedDate and LastModifiedBy property. The former is set to the current UTC time while the latter is queried from a property of our context used to track the current Tenant and User. Essentially something like this:

foreach (var entry in ChangeTracker.Entries().ToList())
{
  if (entry.State == EntityState.Modified && entry.Entity is IModified entity)
  {
    entity.LastModifiedDate = DateTime.UtcNow;
    entity.LastModifiedBy = this.UserInfoAccessor?.User.Id ?? "SYSTEM";
  } 
} 

This has been working great for us, however tit obviously does not work with the new Bulk Update features of EF7.

To try and get around this, I wrote a custom IQueryExpressionInterceptor that walks the expression to determine if the target entity is of the proper type and if so, checks to see if the SetProperty calls includes both of the target properties. If either are missing, I dynamically build the SetProperty call for the corresponding fields.

However this didn't work. I originally passed the DbContext through to my expression visitor to access its UserInfoAccessor property. The first test attempt threw a Context Disposed exception. It was then that I realized it was using the original context, since the interceptor only appears to run once (and not per context).

I was a little confused at first since the interceptor includes a parameter of type QueryExpressionEventData which has a Context property. But since the interceptor only seems to run once, that property is filled in with the context that first triggered the query compilation.

For now, I've pared back my logic so that it sets the modified date (if absent) and hardcodes the user to "system". This at least partially covers us if the developer neglects to call SetProperty themselves, but I was hoping for something more robust. We don't expect them to set the values during non-bulk updates, so I don't think they'd remember/realize it needs to be done for the bulk operations. Heck, I'm the one this bit initially hence me trying to find a way to "automate" it away.

Is there a way to access/indicate the target context from within the new expression? I was hoping there'd be some mechanism by which EF internals recognized some sentinel in the returned expression and would replace it on the fly...but there doesn't seem to be anything like this.

If this isn't achievable with the IQueryExpressionInterceptor, is there any other mechanism outside of a Trigger that I can plug into (either instead of, or in addition to) to achieve parity with non-bulk operations?

Include provider and version information

EF Core version: 7.0.1
Database provider: efcore.pg
Target framework: NET7.0
Operating system: Windows/Debian

@roji
Copy link
Member

roji commented Jan 7, 2023

@pinkfloydx33 by-design, IQueryExpressionInterceptor operates when the query is compiled; the results of that compilation are then cached and reused when the same query is re-executed. Therefore, the interceptor cannot access contextual information that can change between executions of the same query.

I'm not aware of a built-in EF way to do this (but @ajcvickers may have an idea). But you could introduce ExecuteUpdateWithDefaults (or similar) which modifies the tree and then calls ExecuteUpdate.

@pinkfloydx33
Copy link
Author

@roji an extension method sounds like it could be a workable solution, thanks for the idea. I could also introduce an analyzer to catch mis-uses of ExecuteUpdate (or both even).

What purpose does the Context property of the QueryExpressionEventData serve? It seems like the original context wouldn't ever be of much use. Maybe for accessing other services?

@roji
Copy link
Member

roji commented Jan 8, 2023

What purpose does the Context property of the QueryExpressionEventData serve? It seems like the original context wouldn't ever be of much use. Maybe for accessing other services?

Yeah, probably - it may be better to directly expose the (singleton) service provider rather than the context, to avoid this kind of error. But @ajcvickers probably has more context/insight on all this.

@ajcvickers
Copy link
Contributor

#29573 is tracking making this a singleton service.

@ajcvickers ajcvickers added the closed-no-further-action The issue is closed and no further action is planned. label Jan 13, 2023
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

3 participants