Conversation
This empty file was accidentally committed in 8aeccd2.
I've used `CancellationToken.None` for async calls after the 'point of no cancellation' (i.e. after we've incurred side-effects). This ensures, for example, that we always send the password reset notification after a password change has been committed. In `SetPassword` I could have theoretically used delayed the 'point of no cancellation' to the deletion of password reset tokens. However, doing that safely would have required using a transaction to ensure that the user and user audit changes were rolled back when cancellation occurred during the deletion of password reset tokens, and that would introduced more complexity and overhead than I felt was justifiable for such a marginal extension to the cancellation window.
There was a problem hiding this comment.
Pull request overview
This PR propagates CancellationToken support through the Buttercup web MVC layer, GraphQL API, application managers, security services, and email sending so request cancellation can be honored during async operations (especially EF Core and HTTP calls).
Changes:
- Add
CancellationTokenparameters to MVC controller actions and controller-query interfaces/implementations, passing tokens into EF Core async queries. - Extend application/service interfaces and implementations (managers, auth/token services, mailers, email senders, queryable extensions) to accept and forward
CancellationTokens. - Update unit/integration tests to pass
TestContext.Current.CancellationTokenand adjust mocks accordingly.
Reviewed changes
Copilot reviewed 56 out of 57 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Buttercup.Web/Controllers/RecipesController.cs | MVC actions now accept/forward request cancellation. |
| src/Buttercup.Web/Controllers/Queries/RecipesControllerQueries.cs | EF queries now accept and use cancellation tokens. |
| src/Buttercup.Web/Controllers/Queries/IRecipesControllerQueries.cs | Update query interface to require cancellation tokens. |
| src/Buttercup.Web/Controllers/Queries/IHomeControllerQueries.cs | Update query interface to require cancellation tokens. |
| src/Buttercup.Web/Controllers/Queries/ICommentsControllerQueries.cs | Update query interface to require cancellation tokens. |
| src/Buttercup.Web/Controllers/Queries/HomeControllerQueries.cs | Forward cancellation tokens to EF Core async calls. |
| src/Buttercup.Web/Controllers/Queries/CommentsControllerQueries.cs | Forward cancellation tokens to EF Core async calls. |
| src/Buttercup.Web/Controllers/HomeController.cs | MVC action now accepts/forwards cancellation. |
| src/Buttercup.Web/Controllers/CommentsController.cs | MVC actions now accept/forward cancellation. |
| src/Buttercup.Web/Controllers/AuthenticationController.cs | Auth endpoints now accept/forward cancellation. |
| src/Buttercup.Web/Controllers/AccountController.cs | Account endpoints now accept/forward cancellation. |
| src/Buttercup.Web/Areas/Admin/Controllers/UsersController.cs | Admin endpoints now accept/forward cancellation. |
| src/Buttercup.Web/Areas/Admin/Controllers/Queries/UsersControllerQueries.cs | Admin EF query now uses cancellation token. |
| src/Buttercup.Web/Areas/Admin/Controllers/Queries/IUsersControllerQueries.cs | Update admin query interface to require cancellation tokens. |
| src/Buttercup.Web/Api/UserMutations.cs | GraphQL mutations now accept/forward cancellation. |
| src/Buttercup.Web/Api/RecipeMutations.cs | GraphQL mutations now accept/forward cancellation. |
| src/Buttercup.Web/Api/CommentMutations.cs | GraphQL mutations now accept/forward cancellation. |
| src/Buttercup.Web/Api/AuthenticationMutations.cs | GraphQL auth mutation now forwards cancellation. |
| src/Buttercup.Web.Tests/Controllers/RecipesControllerTests.cs | Update MVC controller tests for new token parameters. |
| src/Buttercup.Web.Tests/Controllers/Queries/RecipesControllerQueriesTests.cs | Update query tests to pass cancellation tokens. |
| src/Buttercup.Web.Tests/Controllers/Queries/HomeControllerQueriesTests.cs | Update query tests to pass cancellation tokens. |
| src/Buttercup.Web.Tests/Controllers/Queries/CommentsControllerQueriesTests.cs | Update query tests to pass cancellation tokens. |
| src/Buttercup.Web.Tests/Controllers/HomeControllerTests.cs | Update MVC controller tests for new token parameters. |
| src/Buttercup.Web.Tests/Controllers/CommentsControllerTests.cs | Update MVC controller tests for new token parameters. |
| src/Buttercup.Web.Tests/Controllers/AuthenticationControllerTests.cs | Update auth controller tests for new token parameters. |
| src/Buttercup.Web.Tests/Controllers/AccountControllerTests.cs | Update account controller tests for new token parameters. |
| src/Buttercup.Web.Tests/Areas/Admin/Controllers/UsersControllerTests.cs | Update admin controller tests for new token parameters. |
| src/Buttercup.Web.Tests/Areas/Admin/Controllers/Queries/UsersControllerQueriesTests.cs | Update admin query tests to pass cancellation tokens. |
| src/Buttercup.Web.Tests/Api/CreateTestUserTests.cs | Update API test to pass cancellation into auth service. |
| src/Buttercup.Web.Tests/Api/AuthenticateTests.cs | Update API test to pass cancellation into token validation. |
| src/Buttercup.Web.Tests/Api/AccessTokenInvalidationTests.cs | Update E2E test to pass cancellation into user manager. |
| src/Buttercup.Security/TokenAuthenticationService.cs | Token issuance/validation now accepts/uses cancellation tokens. |
| src/Buttercup.Security/PasswordAuthenticationService.cs | Password flows now accept/forward cancellation tokens to EF. |
| src/Buttercup.Security/ITokenAuthenticationService.cs | Add optional cancellation tokens to interface methods. |
| src/Buttercup.Security/IPasswordAuthenticationService.cs | Add optional cancellation tokens to interface methods. |
| src/Buttercup.Security/IAuthenticationMailer.cs | Add optional cancellation tokens to mailer interface. |
| src/Buttercup.Security/AuthenticationMailer.cs | Forward cancellation tokens to email sender. |
| src/Buttercup.Security.Tests/TokenAuthenticationServiceTests.cs | Update token service tests for new token parameters. |
| src/Buttercup.Security.Tests/PasswordAuthenticationServiceTests.cs | Update password service tests for new token parameters. |
| src/Buttercup.Security.Tests/AuthenticationMailerTests.cs | Update mailer tests for new token parameters. |
| src/Buttercup.EntityModel/QueryableExtensions.cs | Add cancellation support to async query helpers. |
| src/Buttercup.EntityModel.Tests/QueryableExtensionsTests.cs | Update tests for cancellation-aware query helpers. |
| src/Buttercup.Email/MailpitSender.cs | Pass cancellation token through HTTP send/read operations. |
| src/Buttercup.Email/IEmailSender.cs | Add optional cancellation token to email sender contract. |
| src/Buttercup.Email/AzureEmailSender.cs | Forward cancellation token into Azure EmailClient. |
| src/Buttercup.Email.Tests/MailpitSenderTests.cs | Update tests for new token parameter. |
| src/Buttercup.Email.Tests/AzureEmailSenderTests.cs | Update tests for new token parameter. |
| src/Buttercup.Application/UserManager.cs | Add cancellation tokens and forward to EF async operations. |
| src/Buttercup.Application/RecipeManager.cs | Add cancellation tokens and forward to EF async operations. |
| src/Buttercup.Application/IUserManager.cs | Add optional cancellation tokens to manager contract. |
| src/Buttercup.Application/IRecipeManager.cs | Add optional cancellation tokens to manager contract. |
| src/Buttercup.Application/ICommentManager.cs | Add optional cancellation tokens to manager contract. |
| src/Buttercup.Application/CommentManager.cs | Add cancellation tokens and forward to EF async operations. |
| src/Buttercup.Application.Tests/UserManagerTests.cs | Update tests for new token parameters. |
| src/Buttercup.Application.Tests/RecipeManagerTests.cs | Update tests for new token parameters. |
| src/Buttercup.Application.Tests/CommentManagerTests.cs | Update tests for new token parameters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
I'm deliberately not passing the cancellation token through to the `InsertUserAuditEntry` method as we don't want to cancel between performing and auditable action and recording that action in the audit log.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.