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

Fine-grained query intercepting #3404

Merged
merged 8 commits into from
Jan 13, 2025
Merged

Fine-grained query intercepting #3404

merged 8 commits into from
Jan 13, 2025

Conversation

shroff
Copy link
Contributor

@shroff shroff commented Jan 11, 2025

I want to be able to intercept queries executed only in certain transaction contexts, and I couldn't find another/easier way.

I thought of filing an issue for this, but it just seemed easier to do.

Copy link
Owner

@simolus3 simolus3 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! The approach looks ok as an easy way to only apply the interceptor in some scopes.

We'll also need an test for this (see interceptor_test.dart as an example), something that applies an interceptor for a transaction and asserts that it was applied for the methods within the transaction block.

drift/lib/src/runtime/api/connection_user.dart Outdated Show resolved Hide resolved
drift/lib/src/runtime/api/connection_user.dart Outdated Show resolved Hide resolved
@simolus3
Copy link
Owner

How about an approach that allows applying a QueryInterceptor on some arbitrary scope? E.g., what if we had an API like this:

await database.withInterceptor(interceptor: yourInterceptor, () async {
  // In this block, the interceptor is active
  await database.transaction(() async {
    // intercepted beginTransaction(), and so on...
  });
});

This has the advantage that it doesn't just apply to transactions, but may also be used for batches or any other drift APIs. Whenever some specific operation should run with an interceptor, you'd just use that method. It also gives you control over whether beginTransaction should be intercepted (if you call transaction inside withInterceptor, yes; if you call withInterceptor inside transaction, no).

I think it can be implemented by adding this to DatabaseConnectionUser:

Future<T> withQueryInterceptor<T>(
  Future<T> Function() action, {
  required QueryInterceptor interceptor,
}) async {
  return await resolvedEngine.doWhenOpened((executor) {
    final inner = _ExclusiveExecutor(this,
        executor: executor.interceptWith(interceptor));
    return _runConnectionZoned(inner, action);
  });
}

(Think because I didn't test this at all).

@shroff
Copy link
Contributor Author

shroff commented Jan 12, 2025

That's a great idea! And the API is much cleaner and doesn't complicate existing functionality.

In terms of naming, withInterceptor or withQueryInterceptor might lead to confusion with QueryExecutor.interceptWith. How about .intercept (like .transaction, or .exclusively), or .runWithInterceptor?

@simolus3
Copy link
Owner

runWithInterceptor sounds good 👍

@shroff shroff changed the title Allow running transactions with QueryInterceptor Fine-grained query intercepting Jan 12, 2025
Copy link
Owner

@simolus3 simolus3 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, this looks good to me 👍 I've added a brief mention of this feature in the documentation.

@simolus3 simolus3 merged commit 7ded602 into simolus3:develop Jan 13, 2025
16 checks passed
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.

2 participants