Support completing request units of work before responses start - #26017
Support completing request units of work before responses start#26017maliming wants to merge 11 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR changes AbpUnitOfWorkMiddleware to complete (commit) the ambient unit of work on HttpResponse.OnStarting, ensuring data written during a request is committed before the first response bytes can be flushed to the client. It also adds MVC integration tests to cover response-flush timing, post-flush exceptions, and behavior of repository/raw provider access after the response has started.
Changes:
- Complete the ambient UoW via
HttpResponse.OnStarting(and keep the existing post-pipeline completion as a fallback). - Add integration-test endpoints that flush the response mid-pipeline and then validate UoW completion / post-flush behavior.
- Add integration tests verifying commit-before-flush, commit persistence on post-flush exception, and post-flush UoW behavior for repositories vs raw provider access.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs | Commits the ambient UoW on Response.OnStarting before response bytes are sent. |
| framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs | Adds endpoints that flush responses mid-request to exercise UoW completion timing and post-flush access patterns. |
| framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs | Adds integration tests validating commit-before-flush and post-flush semantics. |
| framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWorkConfig.cs | Adds a test-only flag to assert the UoW completion state after a response flush. |
Suppressed comments (1)
framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs:62
- This assertion hard-codes the expected seed count ("(1)"), which can make the test fail if the test data builder changes or additional records are seeded in the future. It’s enough to assert that the count is unchanged across the flush boundary and that
ambient=null.
// After the response starts the request uow is gone; a repository still works via its
// own implicit uow (ambient=null), so it no longer joins the request transaction.
var body = await GetResponseAsStringAsync("/api/unitofwork-test/ReadRepositoryAfterResponseFlush");
body.ShouldBe("before=ok(1);after=ok(1,ambient=null)");
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Enable globally via CompleteUnitOfWorkOnResponseStarting or per path via the Urls list - OpenIddict opts in its endpoints so token/session rows commit before the response
- Rename completedOnResponseStarting and fix response-start completion comments and docs
maliming
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (1)
framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs:77
- This assertion cannot hold with the response-start completion implemented here. Once
uow.CompleteAsync()runs,AmbientUnitOfWork.GetCurrentByChecking()hides the completed UOW, andMemoryDbRepository.GetListAsync()callsGetDatabaseAsync(), which requires an ambient UOW and throwsAbpException; the controller therefore writesafter=threw:AbpException, notafter=ok(1,ambient=null). Update the expected body (or explicitly start a new UOW if a successful post-response query is what this test intends to cover).
body.ShouldBe("before=ok(1);after=ok(1,ambient=null)");
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## rel-10.7 #26017 +/- ##
============================================
+ Coverage 48.97% 49.22% +0.24%
============================================
Files 3805 3805
Lines 132259 132421 +162
Branches 10027 10038 +11
============================================
+ Hits 64774 65183 +409
+ Misses 65559 65277 -282
- Partials 1926 1961 +35 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AbpUnitOfWorkMiddlewarecompletes the request's unit of work afterawait next(context)returns. But a response can reach the client before that: for the OpenIddict token endpoint, the ABP controller returns aSignInResultand OpenIddict's response handler writes the token JSON to the response while the downstream pipeline is still running.A client that receives the token and immediately calls an API can then read data written while the token was issued before it is committed:
This adds an opt-in that completes the request's unit of work on
HttpResponse.OnStarting, before the first response byte is sent, so the data is committed before the client receives the response. It only completes the middleware's own request unit of work; if a nestedrequiresNewunit of work is current, the request unit of work is completed afternextreturns, as before.It is off by default. Enable it for every request the middleware handles with
CompleteUnitOfWorkOnResponseStarting, or for selected path prefixes withCompleteUnitOfWorkOnResponseStartingUrls. The OpenIddict module enables it for its authorization, token, device authorization, pushed authorization, end-session, revocation, and end-user verification endpoints. Other requests keep the existing behavior.Covered by added integration tests.
nextreturns, so an immediate follow-up request may not see it yetnextreturns