From 011258793ff80549f1ea0f85a9e6bf43acc27b09 Mon Sep 17 00:00:00 2001 From: maliming Date: Wed, 19 Aug 2026 21:02:25 +0800 Subject: [PATCH 01/11] Commit ambient unit of work before the response starts --- .../AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs | 17 +++- .../Mvc/Uow/TestUnitOfWorkConfig.cs | 2 + .../Mvc/Uow/UnitOfWorkMiddleware_Tests.cs | 43 ++++++++- .../Mvc/Uow/UnitOfWorkTestController.cs | 89 ++++++++++++++++++- 4 files changed, 148 insertions(+), 3 deletions(-) diff --git a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs index 74833bf748..4c1b11fc1d 100644 --- a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs +++ b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs @@ -37,8 +37,23 @@ public class AbpUnitOfWorkMiddleware : AbpMiddlewareBase, ITransientDependency using (var uow = _unitOfWorkManager.Reserve(UnitOfWork.UnitOfWorkReservationName)) { + // Commit the ambient unit of work before the response starts, so data written + // during the request is committed before the response is flushed to the client. + context.Response.OnStarting(async () => + { + var currentUow = _unitOfWorkManager.Current; + if (currentUow != null && !currentUow.IsCompleted) + { + await currentUow.CompleteAsync(_cancellationTokenProvider.Token); + } + }); + await next(context); - await uow.CompleteAsync(_cancellationTokenProvider.Token); + + if (!uow.IsCompleted) + { + await uow.CompleteAsync(_cancellationTokenProvider.Token); + } } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWorkConfig.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWorkConfig.cs index 8f40f4b8e0..9dd87b56b1 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWorkConfig.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWorkConfig.cs @@ -7,4 +7,6 @@ public class TestUnitOfWorkConfig : ISingletonDependency public const string ExceptionOnCompleteMessage = "TestUnitOfWork configured for exception"; public bool ThrowExceptionOnComplete { get; set; } + + public bool? UowCompletedAfterResponseFlush { get; set; } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs index 05e5d7b524..b829df769d 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs @@ -1,5 +1,7 @@ -using System.Net.Http; +using System.Net; +using System.Net.Http; using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; using Shouldly; using Xunit; @@ -27,4 +29,43 @@ public class UnitOfWorkMiddleware_Tests : AspNetCoreMvcTestBase var result = await Client.SendAsync(requestMessage); result.IsSuccessStatusCode.ShouldBeTrue(); } + + [Fact] + public async Task Ambient_Uow_Should_Be_Completed_Before_Response_Is_Flushed() + { + var result = await GetResponseAsStringAsync("/api/unitofwork-test/CommitBeforeResponseFlush"); + result.ShouldBe("first:completed"); + } + + [Fact] + public async Task Exception_After_Response_Flush_Should_Not_Undo_Committed_Work() + { + // Once the response has started, an exception can't turn it into an error response + // (the connection is reset). What matters: the uow was committed before the throw. + await Should.ThrowAsync(async () => + { + var response = await Client.GetAsync("/api/unitofwork-test/CommitThenThrowAfterResponseFlush"); + await response.Content.ReadAsStringAsync(); + }); + + ServiceProvider.GetRequiredService() + .UowCompletedAfterResponseFlush.ShouldBe(true); + } + + [Fact] + public async Task Repository_Access_After_Response_Flush_Runs_Outside_The_Request_Uow() + { + // 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)"); + } + + [Fact] + public async Task Raw_Database_Provider_After_Response_Flush_Throws() + { + // Unlike repositories, raw provider access after the response started has no uow and throws. + var body = await GetResponseAsStringAsync("/api/unitofwork-test/RawDatabaseProviderAfterResponseFlush"); + body.ShouldBe("first:threw-AbpException"); + } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs index ebf2c12a6a..106946a238 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs @@ -1,5 +1,14 @@ -using Microsoft.AspNetCore.Mvc; +using System; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; using Shouldly; +using Volo.Abp; +using Volo.Abp.Domain.Repositories; +using Volo.Abp.MemoryDb; +using Volo.Abp.TestApp.MemoryDb; +using Volo.Abp.TestApp.Domain; using Volo.Abp.Uow; namespace Volo.Abp.AspNetCore.Mvc.Uow; @@ -64,4 +73,82 @@ public class UnitOfWorkTestController : AbpController _testUnitOfWorkConfig.ThrowExceptionOnComplete = true; } + + [HttpGet] + [Route("CommitBeforeResponseFlush")] + public async Task CommitBeforeResponseFlush() + { + var uow = CurrentUnitOfWork; + uow.ShouldNotBeNull(); + + // Start the response from inside the pipeline, before the middleware would commit. + await Response.WriteAsync("first"); + await Response.Body.FlushAsync(); + + await Response.WriteAsync(uow.IsCompleted ? ":completed" : ":not-completed"); + } + + [HttpGet] + [Route("CommitThenThrowAfterResponseFlush")] + public async Task CommitThenThrowAfterResponseFlush() + { + var uow = CurrentUnitOfWork; + + await Response.WriteAsync("first"); + await Response.Body.FlushAsync(); + + // Record the commit state so the test can assert the throw below doesn't undo it. + _testUnitOfWorkConfig.UowCompletedAfterResponseFlush = uow.IsCompleted; + + throw new UserFriendlyException("boom after the response was already flushed"); + } + + [HttpGet] + [Route("ReadRepositoryAfterResponseFlush")] + public async Task ReadRepositoryAfterResponseFlush() + { + var repository = LazyServiceProvider.LazyGetRequiredService>(); + + var before = (await repository.GetListAsync()).Count; + await Response.WriteAsync($"before=ok({before})"); + await Response.Body.FlushAsync(); + + string after; + try + { + var count = (await repository.GetListAsync()).Count; + after = $";after=ok({count},ambient={(UnitOfWorkManager.Current == null ? "null" : "present")})"; + } + catch (Exception ex) + { + after = $";after=threw:{ex.GetType().Name}"; + } + + await Response.WriteAsync(after); + } + + [HttpGet] + [Route("RawDatabaseProviderAfterResponseFlush")] + public async Task RawDatabaseProviderAfterResponseFlush() + { + var databaseProvider = LazyServiceProvider + .LazyGetRequiredService>(); + + await Response.WriteAsync("first"); + await Response.Body.FlushAsync(); + + string outcome; + try + { + await databaseProvider.GetDatabaseAsync(); + outcome = ":ok"; + } + catch (AbpException) + { + // Raw provider access has no ambient uow once the response started, so it throws. + outcome = ":threw-AbpException"; + } + + await Response.WriteAsync(outcome); + } } From 4bd160565d7d754b322e09f1bfc8d66d1fcddc82 Mon Sep 17 00:00:00 2001 From: maliming Date: Thu, 20 Aug 2026 15:08:51 +0800 Subject: [PATCH 02/11] Complete only the reserved unit of work on response start --- .../AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs | 7 +++--- .../Mvc/Uow/UnitOfWorkMiddleware_Tests.cs | 7 ++++++ .../Mvc/Uow/UnitOfWorkTestController.cs | 24 +++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs index 4c1b11fc1d..534d8f9828 100644 --- a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs +++ b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs @@ -39,12 +39,13 @@ public class AbpUnitOfWorkMiddleware : AbpMiddlewareBase, ITransientDependency { // Commit the ambient unit of work before the response starts, so data written // during the request is committed before the response is flushed to the client. + // Only when this reserved unit of work is the current one: if an explicit nested + // unit of work is in progress, it is the current one and must be left to its owner. context.Response.OnStarting(async () => { - var currentUow = _unitOfWorkManager.Current; - if (currentUow != null && !currentUow.IsCompleted) + if (_unitOfWorkManager.Current == uow) { - await currentUow.CompleteAsync(_cancellationTokenProvider.Token); + await uow.CompleteAsync(_cancellationTokenProvider.Token); } }); diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs index b829df769d..7953162f40 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs @@ -68,4 +68,11 @@ public class UnitOfWorkMiddleware_Tests : AspNetCoreMvcTestBase var body = await GetResponseAsStringAsync("/api/unitofwork-test/RawDatabaseProviderAfterResponseFlush"); body.ShouldBe("first:threw-AbpException"); } + + [Fact] + public async Task Response_Flush_Inside_Nested_Uow_Should_Not_Complete_The_Nested_Uow() + { + var body = await GetResponseAsStringAsync("/api/unitofwork-test/NestedUowDuringResponseFlush"); + body.ShouldBe("first:nested-completed-by-owner"); + } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs index 106946a238..cdb7c80917 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs @@ -151,4 +151,28 @@ public class UnitOfWorkTestController : AbpController await Response.WriteAsync(outcome); } + + [HttpGet] + [Route("NestedUowDuringResponseFlush")] + public async Task NestedUowDuringResponseFlush() + { + using (var nested = UnitOfWorkManager.Begin(requiresNew: true, isTransactional: false)) + { + await Response.WriteAsync("first"); + await Response.Body.FlushAsync(); + + string outcome; + try + { + await nested.CompleteAsync(); + outcome = ":nested-completed-by-owner"; + } + catch (AbpException) + { + outcome = ":nested-already-completed"; + } + + await Response.WriteAsync(outcome); + } + } } From eaf1bf6fe362f86f63d84bb493523153774d365d Mon Sep 17 00:00:00 2001 From: maliming Date: Fri, 21 Aug 2026 10:30:14 +0800 Subject: [PATCH 03/11] Add opt-in options for completing the unit of work on response start - Enable globally via CompleteUnitOfWorkOnResponseStarting or per path via the Urls list - OpenIddict opts in its endpoints so token/session rows commit before the response --- framework/Volo.Abp.slnx | 1 + .../Uow/AbpAspNetCoreUnitOfWorkOptions.cs | 27 +++- .../AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs | 50 ++++-- .../Mvc/Uow/UnitOfWorkMiddleware_Tests.cs | 82 ++++++++-- .../Mvc/Uow/UnitOfWorkTestController.cs | 14 +- .../Volo.Abp.AspNetCore.Uow.Tests.csproj | 26 +++ .../Uow/AbpAspNetCoreUowTestModule.cs | 61 +++++++ .../Volo/Abp/AspNetCore/Uow/Program.cs | 15 ++ .../UnitOfWorkMiddleware_Relational_Tests.cs | 120 ++++++++++++++ .../AspNetCore/Uow/UowVisibilityController.cs | 107 +++++++++++++ .../AspNetCore/Uow/UowVisibilityTestEntity.cs | 46 ++++++ .../AbpOpenIddictAspNetCoreModule.cs | 40 ++++- ...olo.Abp.OpenIddict.AspNetCore.Tests.csproj | 6 + ...enIddictTokenEndpoint_Integration_Tests.cs | 58 +++++++ .../OpenIddictTokenIntegrationTestModule.cs | 150 ++++++++++++++++++ .../Abp/OpenIddict/Integration/Program.cs | 15 ++ 16 files changed, 796 insertions(+), 22 deletions(-) create mode 100644 framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo.Abp.AspNetCore.Uow.Tests.csproj create mode 100644 framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUowTestModule.cs create mode 100644 framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/Program.cs create mode 100644 framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UnitOfWorkMiddleware_Relational_Tests.cs create mode 100644 framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UowVisibilityController.cs create mode 100644 framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UowVisibilityTestEntity.cs create mode 100644 modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs create mode 100644 modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenIntegrationTestModule.cs create mode 100644 modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/Program.cs diff --git a/framework/Volo.Abp.slnx b/framework/Volo.Abp.slnx index 5f4adbd622..6b9ad8af61 100644 --- a/framework/Volo.Abp.slnx +++ b/framework/Volo.Abp.slnx @@ -194,6 +194,7 @@ + diff --git a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs index 3949bb6639..67bfbfaec5 100644 --- a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs +++ b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs @@ -1,4 +1,4 @@ -using System.Collections.Generic; +using System.Collections.Generic; namespace Volo.Abp.AspNetCore.Uow; @@ -11,4 +11,29 @@ public class AbpAspNetCoreUnitOfWorkOptions /// starting with an ignored URL. /// public List IgnoredUrls { get; } = new List(); + + /// + /// Completes the request unit of work just before the response starts (on + /// HttpResponse.OnStarting) instead of at the end of the pipeline, so data written during + /// the request is committed before the response is flushed. Disabled by default; enable it here + /// globally or opt-in per endpoint via . + /// + /// Trade-offs when it applies: an exception after the response starts can no longer roll back the + /// committed data (commit and network response are not atomic); database access after the response + /// starts is outside the request unit of work (unsuitable for streaming responses); unit of work + /// events and completed handlers run before the first response byte (adding to its latency); a + /// nested (requiresNew) unit of work that is current when the response starts is left to its owner + /// and the request unit of work then completes at the end of the pipeline as usual. + /// + /// + public bool CompleteUnitOfWorkOnResponseStarting { get; set; } = false; + + /// + /// Absolute request path prefixes (matched by segment) that opt-in to + /// even when it is globally disabled (for example + /// "/connect" matches "/connect/token" but not "/connections"). A trailing slash is normalized; blank, + /// non-absolute, and root ("/") entries are ignored - use + /// to enable it for every request. + /// + public List CompleteUnitOfWorkOnResponseStartingUrls { get; } = new List(); } diff --git a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs index 534d8f9828..36c00208f9 100644 --- a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs +++ b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs @@ -37,21 +37,25 @@ public class AbpUnitOfWorkMiddleware : AbpMiddlewareBase, ITransientDependency using (var uow = _unitOfWorkManager.Reserve(UnitOfWork.UnitOfWorkReservationName)) { - // Commit the ambient unit of work before the response starts, so data written - // during the request is committed before the response is flushed to the client. - // Only when this reserved unit of work is the current one: if an explicit nested - // unit of work is in progress, it is the current one and must be left to its owner. - context.Response.OnStarting(async () => + var completedOnResponseStarting = false; + + if (!context.Response.HasStarted && ShouldCompleteOnResponseStarting(context)) { - if (_unitOfWorkManager.Current == uow) + context.Response.OnStarting(async () => { - await uow.CompleteAsync(_cancellationTokenProvider.Token); - } - }); + // A nested (requiresNew) unit of work that is current is left to its owner. + if (_unitOfWorkManager.Current == uow) + { + // Set before completing so a post-commit failure isn't masked by the completion below. + completedOnResponseStarting = true; + await uow.CompleteAsync(_cancellationTokenProvider.Token); + } + }); + } await next(context); - if (!uow.IsCompleted) + if (!completedOnResponseStarting) { await uow.CompleteAsync(_cancellationTokenProvider.Token); } @@ -64,6 +68,32 @@ public class AbpUnitOfWorkMiddleware : AbpMiddlewareBase, ITransientDependency _options.IgnoredUrls.Any(x => context.Request.Path.Value.StartsWith(x, StringComparison.OrdinalIgnoreCase)); } + private bool ShouldCompleteOnResponseStarting(HttpContext context) + { + if (_options.CompleteUnitOfWorkOnResponseStarting) + { + return true; + } + + foreach (var url in _options.CompleteUnitOfWorkOnResponseStartingUrls) + { + if (string.IsNullOrWhiteSpace(url)) + { + continue; + } + + // Normalize a trailing slash ("/connect/" behaves like "/connect") and ignore non-absolute entries. + var prefix = url.TrimEnd('/'); + if (prefix.StartsWith("/", StringComparison.Ordinal) && + context.Request.Path.StartsWithSegments(prefix, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; + } + protected async override Task ShouldSkipAsync(HttpContext context, RequestDelegate next) { // Blazor components will render concurrently, so we need to skip the middleware for them. diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs index 7953162f40..4e241cccf0 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs @@ -1,14 +1,19 @@ -using System.Net; +using System.Net; using System.Net.Http; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Shouldly; +using Volo.Abp.AspNetCore.Uow; using Xunit; namespace Volo.Abp.AspNetCore.Mvc.Uow; public class UnitOfWorkMiddleware_Tests : AspNetCoreMvcTestBase { + private AbpAspNetCoreUnitOfWorkOptions Options => + ServiceProvider.GetRequiredService>().Value; + [Fact] public async Task Get_Actions_Should_Not_Be_Transactional() { @@ -31,17 +36,28 @@ public class UnitOfWorkMiddleware_Tests : AspNetCoreMvcTestBase } [Fact] - public async Task Ambient_Uow_Should_Be_Completed_Before_Response_Is_Flushed() + public async Task Ambient_Uow_Should_Be_Completed_Before_Response_Is_Flushed_When_Enabled() { + Options.CompleteUnitOfWorkOnResponseStarting = true; + var result = await GetResponseAsStringAsync("/api/unitofwork-test/CommitBeforeResponseFlush"); result.ShouldBe("first:completed"); } [Fact] - public async Task Exception_After_Response_Flush_Should_Not_Undo_Committed_Work() + public async Task Ambient_Uow_Is_Not_Completed_On_Response_Start_By_Default() { - // Once the response has started, an exception can't turn it into an error response - // (the connection is reset). What matters: the uow was committed before the throw. + var result = await GetResponseAsStringAsync("/api/unitofwork-test/CommitBeforeResponseFlush"); + result.ShouldBe("first:not-completed"); + } + + [Fact] + public async Task Ambient_Uow_Is_Already_Completed_When_An_Exception_Is_Raised_After_The_Response_Started() + { + Options.CompleteUnitOfWorkOnResponseStarting = true; + + // Once the response has started, an exception can't turn it into an error response (the + // connection is reset). Database-level rollback/commit is covered by the relational tests. await Should.ThrowAsync(async () => { var response = await Client.GetAsync("/api/unitofwork-test/CommitThenThrowAfterResponseFlush"); @@ -55,8 +71,8 @@ public class UnitOfWorkMiddleware_Tests : AspNetCoreMvcTestBase [Fact] public async Task Repository_Access_After_Response_Flush_Runs_Outside_The_Request_Uow() { - // 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. + Options.CompleteUnitOfWorkOnResponseStarting = true; + var body = await GetResponseAsStringAsync("/api/unitofwork-test/ReadRepositoryAfterResponseFlush"); body.ShouldBe("before=ok(1);after=ok(1,ambient=null)"); } @@ -64,7 +80,8 @@ public class UnitOfWorkMiddleware_Tests : AspNetCoreMvcTestBase [Fact] public async Task Raw_Database_Provider_After_Response_Flush_Throws() { - // Unlike repositories, raw provider access after the response started has no uow and throws. + Options.CompleteUnitOfWorkOnResponseStarting = true; + var body = await GetResponseAsStringAsync("/api/unitofwork-test/RawDatabaseProviderAfterResponseFlush"); body.ShouldBe("first:threw-AbpException"); } @@ -72,7 +89,54 @@ public class UnitOfWorkMiddleware_Tests : AspNetCoreMvcTestBase [Fact] public async Task Response_Flush_Inside_Nested_Uow_Should_Not_Complete_The_Nested_Uow() { + Options.CompleteUnitOfWorkOnResponseStarting = true; + var body = await GetResponseAsStringAsync("/api/unitofwork-test/NestedUowDuringResponseFlush"); - body.ShouldBe("first:nested-completed-by-owner"); + body.ShouldBe("first:outer-not-completed:nested-completed-by-owner"); + } + + [Fact] + public async Task Completing_The_Uow_In_The_Action_Still_Fails_At_End_Of_Pipeline_By_Default() + { + var response = await Client.GetAsync("/api/unitofwork-test/CompleteCurrentUow"); + response.StatusCode.ShouldBe(HttpStatusCode.InternalServerError); + } + + [Fact] + public async Task Opt_In_Url_Enables_The_Feature_For_A_Matching_Path() + { + Options.CompleteUnitOfWorkOnResponseStartingUrls.Add("/api/unitofwork-test/CommitBeforeResponseFlush"); + + var result = await GetResponseAsStringAsync("/api/unitofwork-test/CommitBeforeResponseFlush"); + result.ShouldBe("first:completed"); + } + + [Fact] + public async Task Opt_In_Url_With_A_Trailing_Slash_Still_Matches() + { + Options.CompleteUnitOfWorkOnResponseStartingUrls.Add("/api/unitofwork-test/"); + + var result = await GetResponseAsStringAsync("/api/unitofwork-test/CommitBeforeResponseFlush"); + result.ShouldBe("first:completed"); + } + + [Fact] + public async Task Opt_In_Url_With_A_Non_Segment_Prefix_Should_Not_Match() + { + Options.CompleteUnitOfWorkOnResponseStartingUrls.Add("/api/unitofwork-test/Commit"); + + var result = await GetResponseAsStringAsync("/api/unitofwork-test/CommitBeforeResponseFlush"); + result.ShouldBe("first:not-completed"); + } + + [Fact] + public async Task Blank_Or_Malformed_Opt_In_Urls_Are_Ignored() + { + Options.CompleteUnitOfWorkOnResponseStartingUrls.Add(""); + Options.CompleteUnitOfWorkOnResponseStartingUrls.Add(" "); + Options.CompleteUnitOfWorkOnResponseStartingUrls.Add("api/no-leading-slash"); + + var result = await GetResponseAsStringAsync("/api/unitofwork-test/CommitBeforeResponseFlush"); + result.ShouldBe("first:not-completed"); } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs index cdb7c80917..435a44d9e8 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs @@ -97,7 +97,6 @@ public class UnitOfWorkTestController : AbpController await Response.WriteAsync("first"); await Response.Body.FlushAsync(); - // Record the commit state so the test can assert the throw below doesn't undo it. _testUnitOfWorkConfig.UowCompletedAfterResponseFlush = uow.IsCompleted; throw new UserFriendlyException("boom after the response was already flushed"); @@ -161,6 +160,10 @@ public class UnitOfWorkTestController : AbpController await Response.WriteAsync("first"); await Response.Body.FlushAsync(); + // The outer request unit of work (nested.Outer) must not have been completed on response + // start while a nested unit of work is current. + await Response.WriteAsync(nested.Outer!.IsCompleted ? ":outer-completed" : ":outer-not-completed"); + string outcome; try { @@ -175,4 +178,13 @@ public class UnitOfWorkTestController : AbpController await Response.WriteAsync(outcome); } } + + [HttpGet] + [Route("CompleteCurrentUow")] + public async Task CompleteCurrentUow() + { + // Complete the request unit of work inside the action, without writing the response yet. + // The middleware must still try to complete it at the end of the pipeline (original behavior). + await CurrentUnitOfWork.CompleteAsync(); + } } diff --git a/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo.Abp.AspNetCore.Uow.Tests.csproj b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo.Abp.AspNetCore.Uow.Tests.csproj new file mode 100644 index 0000000000..08b17d2002 --- /dev/null +++ b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo.Abp.AspNetCore.Uow.Tests.csproj @@ -0,0 +1,26 @@ + + + + + + net10.0 + Volo.Abp.AspNetCore.Uow.Tests + Volo.Abp.AspNetCore.Uow.Tests + true + false + false + false + true + + + + + + + + + + + + + diff --git a/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUowTestModule.cs b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUowTestModule.cs new file mode 100644 index 0000000000..df2256258f --- /dev/null +++ b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUowTestModule.cs @@ -0,0 +1,61 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Volo.Abp.AspNetCore.Mvc; +using Volo.Abp.AspNetCore.TestBase; +using Volo.Abp.Autofac; +using Volo.Abp.Data; +using Volo.Abp.EntityFrameworkCore; +using Volo.Abp.EntityFrameworkCore.Sqlite; +using Volo.Abp.Modularity; + +namespace Volo.Abp.AspNetCore.Uow; + +[DependsOn( + typeof(AbpAspNetCoreTestBaseModule), + typeof(AbpAspNetCoreMvcModule), + typeof(AbpEntityFrameworkCoreSqliteModule), + typeof(AbpAutofacModule) + )] +public class AbpAspNetCoreUowTestModule : AbpModule +{ + private readonly AbpUnitTestSqliteDatabase _database = new AbpUnitTestSqliteDatabase(); + + public override void ConfigureServices(ServiceConfigurationContext context) + { + context.Services.AddAbpDbContext(options => + { + options.AddDefaultRepositories(includeAllEntities: true); + }); + + Configure(options => + { + options.ConnectionStrings.Default = _database.ConnectionString; + }); + + Configure(options => + { + options.Configure(dbContext => dbContext.UseSqlite().AddAbpDbContextOptionsExtension()); + }); + + _database.CreateTables(new UowVisibilityTestDbContext( + new DbContextOptionsBuilder() + .UseSqlite(_database.ConnectionString) + .AddAbpDbContextOptionsExtension() + .Options)); + } + + public override void OnApplicationInitialization(ApplicationInitializationContext context) + { + var app = context.GetApplicationBuilder(); + + app.UseRouting(); + app.UseUnitOfWork(); + app.UseConfiguredEndpoints(); + } + + public override void OnApplicationShutdown(ApplicationShutdownContext context) + { + _database.Dispose(); + } +} diff --git a/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/Program.cs b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/Program.cs new file mode 100644 index 0000000000..98ab220169 --- /dev/null +++ b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/Program.cs @@ -0,0 +1,15 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.Hosting; +using Volo.Abp.AspNetCore.TestBase; +using Volo.Abp.AspNetCore.Uow; + +var builder = WebApplication.CreateBuilder(new WebApplicationOptions +{ + EnvironmentName = Environments.Staging +}); + +await builder.RunAbpModuleAsync(); + +public partial class Program +{ +} diff --git a/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UnitOfWorkMiddleware_Relational_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UnitOfWorkMiddleware_Relational_Tests.cs new file mode 100644 index 0000000000..00d4cdd2ec --- /dev/null +++ b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UnitOfWorkMiddleware_Relational_Tests.cs @@ -0,0 +1,120 @@ +using System; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Shouldly; +using Volo.Abp.AspNetCore.TestBase; +using Xunit; + +namespace Volo.Abp.AspNetCore.Uow; + +public class UnitOfWorkMiddleware_Relational_Tests : AbpWebApplicationFactoryIntegratedTest +{ + private void EnableCompleteOnResponseStarting() + { + ServiceProvider.GetRequiredService>() + .Value.CompleteUnitOfWorkOnResponseStarting = true; + } + + private async Task CountAsync(string name) + { + var response = await Client.GetAsync("/api/uow-visibility/count?name=" + name); + response.EnsureSuccessStatusCode(); + return int.Parse(await response.Content.ReadAsStringAsync()); + } + + [Fact] + public async Task Row_Written_During_Request_Is_Visible_From_An_Independent_Connection_On_Response_Start() + { + EnableCompleteOnResponseStarting(); + + var response = await Client.GetAsync("/api/uow-visibility/insert-then-read"); + response.EnsureSuccessStatusCode(); + (await response.Content.ReadAsStringAsync()).ShouldBe("inserted:visible"); + } + + [Fact] + public async Task Row_Written_During_Request_Is_Still_Committed_When_The_Feature_Is_Disabled() + { + var name = Guid.NewGuid().ToString("N"); + + var insert = await Client.GetAsync("/api/uow-visibility/insert?name=" + name); + insert.EnsureSuccessStatusCode(); + + (await CountAsync(name)).ShouldBe(1); + } + + [Fact] + public async Task Exception_Before_Response_Rolls_Back_The_Written_Row() + { + EnableCompleteOnResponseStarting(); + var name = Guid.NewGuid().ToString("N"); + + var insert = await Client.GetAsync("/api/uow-visibility/insert-then-throw?name=" + name); + insert.IsSuccessStatusCode.ShouldBeFalse(); + + (await CountAsync(name)).ShouldBe(0); + } + + [Fact] + public async Task Committed_Row_Survives_An_Exception_Raised_After_The_Response_Started() + { + EnableCompleteOnResponseStarting(); + var name = Guid.NewGuid().ToString("N"); + + await Should.ThrowAsync(async () => + { + var response = await Client.GetAsync("/api/uow-visibility/insert-flush-then-throw?name=" + name); + await response.Content.ReadAsStringAsync(); + }); + + (await CountAsync(name)).ShouldBe(1); + } + [Fact] + public async Task Committed_Row_Survives_A_Completed_Handler_Failing_On_Response_Start() + { + EnableCompleteOnResponseStarting(); + var name = Guid.NewGuid().ToString("N"); + + // The handler's error must surface as-is (not masked by a second "already requested" completion); + // the row is committed regardless, since the handler runs after commit. + Exception surfaced = null; + try + { + var response = await Client.GetAsync("/api/uow-visibility/insert-flush-throwing-completed-handler?name=" + name); + await response.Content.ReadAsStringAsync(); + } + catch (Exception ex) + { + surfaced = ex; + } + + surfaced.ShouldNotBeNull(); + surfaced.ToString().ShouldContain("boom in a completed handler"); + surfaced.ToString().ShouldNotContain("already"); + + (await CountAsync(name)).ShouldBe(1); + } + [Fact] + public async Task A_Failing_Commit_On_Response_Start_Does_Not_Persist_Data() + { + EnableCompleteOnResponseStarting(); + var name = Guid.NewGuid().ToString("N"); + + Exception surfaced = null; + try + { + var response = await Client.GetAsync("/api/uow-visibility/insert-then-fail-commit?name=" + name); + await response.Content.ReadAsStringAsync(); + } + catch (Exception ex) + { + surfaced = ex; + } + + // The commit fails on response start, so the request must surface an error and persist nothing. + surfaced.ShouldNotBeNull(); + (await CountAsync(name)).ShouldBe(0); + } +} diff --git a/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UowVisibilityController.cs b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UowVisibilityController.cs new file mode 100644 index 0000000000..83d30d8410 --- /dev/null +++ b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UowVisibilityController.cs @@ -0,0 +1,107 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Volo.Abp; +using Volo.Abp.AspNetCore.Mvc; +using Volo.Abp.Domain.Repositories; +using Volo.Abp.Uow; + +namespace Volo.Abp.AspNetCore.Uow; + +[Route("api/uow-visibility")] +public class UowVisibilityController : AbpController +{ + private readonly IRepository _repository; + + public UowVisibilityController(IRepository repository) + { + _repository = repository; + } + + [HttpGet] + [Route("insert-then-read")] + [UnitOfWork(isTransactional: true)] + public async Task InsertThenRead() + { + var name = Guid.NewGuid().ToString("N"); + await _repository.InsertAsync(new UowVisibilityTestEntity(Guid.NewGuid(), name)); + + await Response.WriteAsync("inserted"); + await Response.Body.FlushAsync(); + + int count; + using (var uow = UnitOfWorkManager.Begin(requiresNew: true, isTransactional: false)) + { + count = await _repository.CountAsync(x => x.Name == name); + await uow.CompleteAsync(); + } + + await Response.WriteAsync(count == 1 ? ":visible" : ":not-visible"); + } + + [HttpGet] + [Route("insert")] + [UnitOfWork(isTransactional: true)] + public async Task Insert(string name) + { + await _repository.InsertAsync(new UowVisibilityTestEntity(Guid.NewGuid(), name)); + await Response.WriteAsync("inserted"); + } + + [HttpGet] + [Route("count")] + public async Task Count(string name) + { + var count = await _repository.CountAsync(x => x.Name == name); + await Response.WriteAsync(count.ToString()); + } + + // Insert (autoSave sends the INSERT to the transaction) then throw before the response: must roll back. + [HttpGet] + [Route("insert-then-throw")] + [UnitOfWork(isTransactional: true)] + public async Task InsertThenThrow(string name) + { + await _repository.InsertAsync(new UowVisibilityTestEntity(Guid.NewGuid(), name), autoSave: true); + throw new AbpException("boom before the response started"); + } + + // Insert, flush the response (committed here when enabled), then throw: the committed row survives. + [HttpGet] + [Route("insert-flush-then-throw")] + [UnitOfWork(isTransactional: true)] + public async Task InsertFlushThenThrow(string name) + { + await _repository.InsertAsync(new UowVisibilityTestEntity(Guid.NewGuid(), name)); + + await Response.WriteAsync("inserted"); + await Response.Body.FlushAsync(); + + throw new AbpException("boom after the response started"); + } + // Insert, register a completed handler that throws (runs after commit), then flush. + [HttpGet] + [Route("insert-flush-throwing-completed-handler")] + [UnitOfWork(isTransactional: true)] + public async Task InsertFlushWithThrowingCompletedHandler(string name) + { + await _repository.InsertAsync(new UowVisibilityTestEntity(Guid.NewGuid(), name)); + CurrentUnitOfWork.OnCompleted(() => throw new AbpException("boom in a completed handler")); + + await Response.WriteAsync("inserted"); + await Response.Body.FlushAsync(); + } + // Insert a valid row plus an invalid one (Name is required): the commit at response start fails. + [HttpGet] + [Route("insert-then-fail-commit")] + [UnitOfWork(isTransactional: true)] + public async Task InsertThenFailCommit(string name) + { + await _repository.InsertAsync(new UowVisibilityTestEntity(Guid.NewGuid(), name)); + await _repository.InsertAsync(new UowVisibilityTestEntity(Guid.NewGuid(), null)); + + await Response.WriteAsync("inserted"); + await Response.Body.FlushAsync(); + } +} diff --git a/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UowVisibilityTestEntity.cs b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UowVisibilityTestEntity.cs new file mode 100644 index 0000000000..f3595ddd33 --- /dev/null +++ b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UowVisibilityTestEntity.cs @@ -0,0 +1,46 @@ +using System; +using Microsoft.EntityFrameworkCore; +using Volo.Abp.Data; +using Volo.Abp.Domain.Entities; +using Volo.Abp.EntityFrameworkCore; +using Volo.Abp.EntityFrameworkCore.Modeling; + +namespace Volo.Abp.AspNetCore.Uow; + +public class UowVisibilityTestEntity : AggregateRoot +{ + public string Name { get; set; } + + protected UowVisibilityTestEntity() + { + } + + public UowVisibilityTestEntity(Guid id, string name) + : base(id) + { + Name = name; + } +} + +[ConnectionStringName("Default")] +public class UowVisibilityTestDbContext : AbpDbContext +{ + public DbSet UowVisibilityTestEntities { get; set; } + + public UowVisibilityTestDbContext(DbContextOptions options) + : base(options) + { + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + base.OnModelCreating(modelBuilder); + + modelBuilder.Entity(b => + { + b.ToTable("UowVisibilityTestEntities"); + b.ConfigureByConvention(); + b.Property(x => x.Name).IsRequired(); + }); + } +} diff --git a/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs b/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs index 26729669e7..0826f7a3ae 100644 --- a/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs +++ b/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs @@ -1,10 +1,14 @@ -using Microsoft.AspNetCore.Identity; +using System.Collections.Generic; +using System.Linq; +using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc.Razor; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using OpenIddict.Abstractions; using OpenIddict.Server; using Volo.Abp.AspNetCore.MultiTenancy; using Volo.Abp.AspNetCore.Mvc.UI.Theme.Shared; +using Volo.Abp.AspNetCore.Uow; using Volo.Abp.Modularity; using Volo.Abp.OpenIddict.Globalization; using Volo.Abp.OpenIddict.Scopes; @@ -45,6 +49,40 @@ public class AbpOpenIddictAspNetCoreModule : AbpModule { options.RemoveClientIdClaim(); }); + + // Commit tokens/authorizations/sessions written during sign-in before the response is flushed. + // Derived from the configured OpenIddict server endpoint paths (including the device endpoint). + context.Services.AddOptions() + .Configure>((uowOptions, serverOptions) => + { + foreach (var path in GetServerEndpointPaths(serverOptions.Value)) + { + if (!uowOptions.CompleteUnitOfWorkOnResponseStartingUrls.Contains(path)) + { + uowOptions.CompleteUnitOfWorkOnResponseStartingUrls.Add(path); + } + } + }); + } + + private static IEnumerable GetServerEndpointPaths(OpenIddictServerOptions serverOptions) + { + var endpoints = serverOptions.TokenEndpointUris + .Concat(serverOptions.AuthorizationEndpointUris) + .Concat(serverOptions.DeviceAuthorizationEndpointUris) + .Concat(serverOptions.PushedAuthorizationEndpointUris) + .Concat(serverOptions.EndSessionEndpointUris) + .Concat(serverOptions.RevocationEndpointUris) + .Concat(serverOptions.EndUserVerificationEndpointUris); + + foreach (var uri in endpoints) + { + var path = uri.IsAbsoluteUri ? uri.AbsolutePath : uri.OriginalString; + if (!string.IsNullOrWhiteSpace(path)) + { + yield return "/" + path.TrimStart('/'); + } + } } private void AddOpenIddictServer(IServiceCollection services) diff --git a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo.Abp.OpenIddict.AspNetCore.Tests.csproj b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo.Abp.OpenIddict.AspNetCore.Tests.csproj index f4c6815fd6..003b78894c 100644 --- a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo.Abp.OpenIddict.AspNetCore.Tests.csproj +++ b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo.Abp.OpenIddict.AspNetCore.Tests.csproj @@ -5,6 +5,8 @@ net10.0 + true + true @@ -16,6 +18,10 @@ + + + + diff --git a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs new file mode 100644 index 0000000000..cf11cffdf3 --- /dev/null +++ b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs @@ -0,0 +1,58 @@ +using System.Collections.Generic; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Shouldly; +using Volo.Abp.AspNetCore.TestBase; +using Volo.Abp.AspNetCore.Uow; +using Xunit; + +namespace Volo.Abp.OpenIddict.Integration; + +// A real "/connect/token" (client_credentials) request through the OpenIddict server. A probe registered +// outside UseUnitOfWork reads the token count from an independent connection at response start. +public class OpenIddictTokenEndpoint_Integration_Tests : AbpWebApplicationFactoryIntegratedTest +{ + private AbpAspNetCoreUnitOfWorkOptions Options => + ServiceProvider.GetRequiredService>().Value; + + private long? TokenCountAtResponseStart => + ServiceProvider.GetRequiredService().TokenCountAtResponseStart; + + private Task RequestTokenAsync() + { + return Client.PostAsync("/connect/token", new FormUrlEncodedContent(new Dictionary + { + ["grant_type"] = "client_credentials", + ["client_id"] = "test-client", + ["client_secret"] = "test-secret" + })); + } + + [Fact] + public async Task Token_Row_Is_Committed_Before_The_Connect_Token_Response_Is_Sent() + { + // The OpenIddict module opts "/connect" in by default. + var response = await RequestTokenAsync(); + + response.StatusCode.ShouldBe(HttpStatusCode.OK); + (await response.Content.ReadAsStringAsync()).ShouldContain("access_token"); + TokenCountAtResponseStart.ShouldBe(1); + } + + [Fact] + public async Task Without_The_Opt_In_The_Token_Is_Not_Committed_When_The_Response_Starts() + { + // Negative control: without the opt-in the token is committed only at the end of the pipeline, + // so the probe reads 0. This proves the positive case genuinely observes response-start timing. + Options.CompleteUnitOfWorkOnResponseStartingUrls.Clear(); + Options.CompleteUnitOfWorkOnResponseStarting = false; + + var response = await RequestTokenAsync(); + + response.StatusCode.ShouldBe(HttpStatusCode.OK); + TokenCountAtResponseStart.ShouldBe(0); + } +} diff --git a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenIntegrationTestModule.cs b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenIntegrationTestModule.cs new file mode 100644 index 0000000000..1319045ca6 --- /dev/null +++ b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenIntegrationTestModule.cs @@ -0,0 +1,150 @@ +using System; +using System.IO; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc.ApplicationParts; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using OpenIddict.Abstractions; +using OpenIddict.Server; +using Volo.Abp.AspNetCore.TestBase; +using Volo.Abp.AspNetCore.Uow; +using Volo.Abp.Data; +using Volo.Abp.Domain.Repositories; +using Volo.Abp.EntityFrameworkCore; +using Volo.Abp.EntityFrameworkCore.Sqlite; +using Volo.Abp.Modularity; +using Volo.Abp.OpenIddict.Applications; +using Volo.Abp.OpenIddict.EntityFrameworkCore; +using Volo.Abp.OpenIddict.Tokens; +using Volo.Abp.Uow; +using Volo.Abp.Autofac; + +namespace Volo.Abp.OpenIddict.Integration; + +public class TokenVisibilityRecorder +{ + public long? TokenCountAtResponseStart { get; set; } +} + +[DependsOn( + typeof(AbpAspNetCoreTestBaseModule), + typeof(AbpOpenIddictAspNetCoreModule), + typeof(AbpOpenIddictEntityFrameworkCoreModule), + typeof(AbpEntityFrameworkCoreSqliteModule), + typeof(AbpAutofacModule) + )] +public class OpenIddictTokenIntegrationTestModule : AbpModule +{ + // File-based SQLite (not shared-cache in-memory) so an independent connection can read committed + // state while another holds an open write transaction, without the shared-cache single-writer deadlock. + private readonly string _databasePath = Path.Combine(Path.GetTempPath(), $"abp-oidc-uow-{Guid.NewGuid():N}.db"); + private string ConnectionString => $"Data Source={_databasePath};Pooling=False"; + + public override void PreConfigureServices(ServiceConfigurationContext context) + { + PreConfigure(options => + { + options.AddDevelopmentEncryptionAndSigningCertificate = false; + }); + + PreConfigure(builder => + { + builder.AddEphemeralEncryptionKey(); + builder.AddEphemeralSigningKey(); + builder.UseAspNetCore().DisableTransportSecurityRequirement(); + }); + } + + public override void ConfigureServices(ServiceConfigurationContext context) + { + context.Services.AddSingleton(); + + // The OpenIddict controllers (including the token endpoint) live in a referenced assembly. + context.Services.GetSingletonInstance() + .ApplicationParts.AddIfNotContains(typeof(AbpOpenIddictAspNetCoreModule).Assembly); + + using (var dbContext = new OpenIddictDbContext( + new DbContextOptionsBuilder().UseSqlite(ConnectionString).Options)) + { + dbContext.Database.EnsureCreated(); + } + + Configure(options => + { + options.ConnectionStrings.Default = ConnectionString; + }); + + Configure(options => + { + options.Configure(c => c.UseSqlite()); + }); + } + + public override void OnApplicationInitialization(ApplicationInitializationContext context) + { + SeedClientAsync(context.ServiceProvider).GetAwaiter().GetResult(); + + var app = context.GetApplicationBuilder(); + app.UseRouting(); + + // Registered before UseUnitOfWork so its OnStarting runs after the unit of work commits + // (callbacks run in reverse order): reads the token count from an independent connection. + app.Use(async (ctx, next) => + { + if (ctx.Request.Path.StartsWithSegments("/connect/token")) + { + ctx.Response.OnStarting(async () => + { + var recorder = ctx.RequestServices.GetRequiredService(); + var uowManager = ctx.RequestServices.GetRequiredService(); + using var uow = uowManager.Begin(requiresNew: true, isTransactional: false); + var repository = ctx.RequestServices.GetRequiredService>(); + recorder.TokenCountAtResponseStart = await repository.GetCountAsync(); + await uow.CompleteAsync(); + }); + } + + await next(); + }); + + app.UseAuthentication(); + app.UseUnitOfWork(); + app.UseAuthorization(); + app.UseConfiguredEndpoints(); + } + + public override void OnApplicationShutdown(ApplicationShutdownContext context) + { + if (File.Exists(_databasePath)) + { + File.Delete(_databasePath); + } + } + + private static async Task SeedClientAsync(IServiceProvider serviceProvider) + { + using var scope = serviceProvider.CreateScope(); + var uowManager = scope.ServiceProvider.GetRequiredService(); + using var uow = uowManager.Begin(); + + var applicationManager = scope.ServiceProvider.GetRequiredService(); + if (await applicationManager.FindByClientIdAsync("test-client") == null) + { + await applicationManager.CreateAsync(new AbpApplicationDescriptor + { + ClientId = "test-client", + ClientSecret = "test-secret", + DisplayName = "Test Client", + ClientType = OpenIddictConstants.ClientTypes.Confidential, + Permissions = + { + OpenIddictConstants.Permissions.Endpoints.Token, + OpenIddictConstants.Permissions.GrantTypes.ClientCredentials + } + }); + } + + await uow.CompleteAsync(); + } +} diff --git a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/Program.cs b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/Program.cs new file mode 100644 index 0000000000..a2ac64130a --- /dev/null +++ b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/Program.cs @@ -0,0 +1,15 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.Hosting; +using Volo.Abp.AspNetCore.TestBase; +using Volo.Abp.OpenIddict.Integration; + +var builder = WebApplication.CreateBuilder(new WebApplicationOptions +{ + EnvironmentName = Environments.Staging +}); + +await builder.RunAbpModuleAsync(); + +public partial class Program +{ +} From bb413e65fbe6a69c2441937c40f4b0a2a1bb4fdb Mon Sep 17 00:00:00 2001 From: maliming Date: Fri, 21 Aug 2026 10:37:16 +0800 Subject: [PATCH 04/11] Remove unused using in UnitOfWorkTestController --- .../Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs index 435a44d9e8..9185b41e03 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs @@ -1,5 +1,4 @@ using System; -using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; From f1e28e0ebcca3c9c11c8f6d98950b70f4bee55a2 Mon Sep 17 00:00:00 2001 From: maliming Date: Fri, 21 Aug 2026 12:37:04 +0800 Subject: [PATCH 05/11] Match the path base and normalize OpenIddict opt-in endpoint paths - Rename completedOnResponseStarting and fix response-start completion comments and docs --- .../Uow/AbpAspNetCoreUnitOfWorkOptions.cs | 2 +- .../AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs | 17 ++++++++++++----- .../OpenIddict/AbpOpenIddictAspNetCoreModule.cs | 14 +++++++++----- ...OpenIddictTokenEndpoint_Integration_Tests.cs | 2 +- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs index 67bfbfaec5..3784cc9005 100644 --- a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs +++ b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs @@ -33,7 +33,7 @@ public class AbpAspNetCoreUnitOfWorkOptions /// even when it is globally disabled (for example /// "/connect" matches "/connect/token" but not "/connections"). A trailing slash is normalized; blank, /// non-absolute, and root ("/") entries are ignored - use - /// to enable it for every request. + /// to enable it for every request handled by the middleware. /// public List CompleteUnitOfWorkOnResponseStartingUrls { get; } = new List(); } diff --git a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs index 36c00208f9..211a5be2cf 100644 --- a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs +++ b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs @@ -37,7 +37,7 @@ public class AbpUnitOfWorkMiddleware : AbpMiddlewareBase, ITransientDependency using (var uow = _unitOfWorkManager.Reserve(UnitOfWork.UnitOfWorkReservationName)) { - var completedOnResponseStarting = false; + var completionAttemptedOnResponseStarting = false; if (!context.Response.HasStarted && ShouldCompleteOnResponseStarting(context)) { @@ -47,7 +47,7 @@ public class AbpUnitOfWorkMiddleware : AbpMiddlewareBase, ITransientDependency if (_unitOfWorkManager.Current == uow) { // Set before completing so a post-commit failure isn't masked by the completion below. - completedOnResponseStarting = true; + completionAttemptedOnResponseStarting = true; await uow.CompleteAsync(_cancellationTokenProvider.Token); } }); @@ -55,7 +55,7 @@ public class AbpUnitOfWorkMiddleware : AbpMiddlewareBase, ITransientDependency await next(context); - if (!completedOnResponseStarting) + if (!completionAttemptedOnResponseStarting) { await uow.CompleteAsync(_cancellationTokenProvider.Token); } @@ -84,8 +84,15 @@ public class AbpUnitOfWorkMiddleware : AbpMiddlewareBase, ITransientDependency // Normalize a trailing slash ("/connect/" behaves like "/connect") and ignore non-absolute entries. var prefix = url.TrimEnd('/'); - if (prefix.StartsWith("/", StringComparison.Ordinal) && - context.Request.Path.StartsWithSegments(prefix, StringComparison.OrdinalIgnoreCase)) + if (!prefix.StartsWith("/", StringComparison.Ordinal)) + { + continue; + } + + // Match both the request path and the path base + path, so an absolute endpoint that includes + // the path base still matches when the path base is stripped from Request.Path. + if (context.Request.Path.StartsWithSegments(prefix, StringComparison.OrdinalIgnoreCase) || + context.Request.PathBase.Add(context.Request.Path).StartsWithSegments(prefix, StringComparison.OrdinalIgnoreCase)) { return true; } diff --git a/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs b/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs index 0826f7a3ae..09f1001104 100644 --- a/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs +++ b/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc.Razor; @@ -50,7 +51,7 @@ public class AbpOpenIddictAspNetCoreModule : AbpModule options.RemoveClientIdClaim(); }); - // Commit tokens/authorizations/sessions written during sign-in before the response is flushed. + // Complete data written while processing OpenIddict requests before the response starts. // Derived from the configured OpenIddict server endpoint paths (including the device endpoint). context.Services.AddOptions() .Configure>((uowOptions, serverOptions) => @@ -65,6 +66,8 @@ public class AbpOpenIddictAspNetCoreModule : AbpModule }); } + private static readonly Uri RootUri = new Uri("http://localhost/"); + private static IEnumerable GetServerEndpointPaths(OpenIddictServerOptions serverOptions) { var endpoints = serverOptions.TokenEndpointUris @@ -77,10 +80,11 @@ public class AbpOpenIddictAspNetCoreModule : AbpModule foreach (var uri in endpoints) { - var path = uri.IsAbsoluteUri ? uri.AbsolutePath : uri.OriginalString; - if (!string.IsNullOrWhiteSpace(path)) + // Resolve relative endpoint URIs (e.g. "connect/token" or "./connect/token") to an absolute path. + var path = (uri.IsAbsoluteUri ? uri : new Uri(RootUri, uri)).AbsolutePath; + if (!string.IsNullOrWhiteSpace(path) && path != "/") { - yield return "/" + path.TrimStart('/'); + yield return path; } } } diff --git a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs index cf11cffdf3..483e64694b 100644 --- a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs +++ b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs @@ -34,7 +34,7 @@ public class OpenIddictTokenEndpoint_Integration_Tests : AbpWebApplicationFactor [Fact] public async Task Token_Row_Is_Committed_Before_The_Connect_Token_Response_Is_Sent() { - // The OpenIddict module opts "/connect" in by default. + // The OpenIddict module opts its endpoint paths (including "/connect/token") in by default. var response = await RequestTokenAsync(); response.StatusCode.ShouldBe(HttpStatusCode.OK); From f5c55db852bbcb2d8e8c433626a7ee6a37f9581d Mon Sep 17 00:00:00 2001 From: maliming Date: Fri, 21 Aug 2026 13:11:31 +0800 Subject: [PATCH 06/11] Add a path base matching test and document opt-in path matching --- .../AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs | 8 +++++--- .../Abp/AspNetCore/Mvc/AbpAspNetCoreMvcTestModule.cs | 3 +++ .../AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs | 10 ++++++++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs index 3784cc9005..34ba5f29c6 100644 --- a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs +++ b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs @@ -31,9 +31,11 @@ public class AbpAspNetCoreUnitOfWorkOptions /// /// Absolute request path prefixes (matched by segment) that opt-in to /// even when it is globally disabled (for example - /// "/connect" matches "/connect/token" but not "/connections"). A trailing slash is normalized; blank, - /// non-absolute, and root ("/") entries are ignored - use - /// to enable it for every request handled by the middleware. + /// "/connect" matches "/connect/token" but not "/connections"). Each prefix is matched against both + /// the request path and the path base + request path, so an endpoint configured with the path base + /// still matches. A trailing slash is normalized; blank, non-absolute, and root ("/") entries are + /// ignored - use to enable it for every request + /// handled by the middleware. /// public List CompleteUnitOfWorkOnResponseStartingUrls { get; } = new List(); } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcTestModule.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcTestModule.cs index 4339104b30..02663eae90 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcTestModule.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcTestModule.cs @@ -164,6 +164,9 @@ public class AbpAspNetCoreMvcTestModule : AbpModule app.UseStaticFiles(); app.UseAbpRequestLocalization(); app.UseAbpSecurityHeaders(); + // Moves the "/pathbase-test" prefix into Request.PathBase so a unit of work opt-in test can + // exercise path base matching; a no-op for every other request. + app.UsePathBase("/pathbase-test"); app.UseRouting(); app.UseAuthentication(); app.UseAuthorization(); diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs index 4e241cccf0..26f81ef3a0 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs @@ -139,4 +139,14 @@ public class UnitOfWorkMiddleware_Tests : AspNetCoreMvcTestBase var result = await GetResponseAsStringAsync("/api/unitofwork-test/CommitBeforeResponseFlush"); result.ShouldBe("first:not-completed"); } + + [Fact] + public async Task Opt_In_Url_Including_The_Path_Base_Matches() + { + // The request path is "/api/..." (the path base is stripped), so this only matches via path base + path. + Options.CompleteUnitOfWorkOnResponseStartingUrls.Add("/pathbase-test/api/unitofwork-test/CommitBeforeResponseFlush"); + + var result = await GetResponseAsStringAsync("/pathbase-test/api/unitofwork-test/CommitBeforeResponseFlush"); + result.ShouldBe("first:completed"); + } } From c0d9631016bceae5891413f4e8d06bf0d06e0f42 Mon Sep 17 00:00:00 2001 From: maliming Date: Fri, 21 Aug 2026 13:52:53 +0800 Subject: [PATCH 07/11] Test that response-start completion rolls back a failed request --- .../UnitOfWorkMiddleware_Relational_Tests.cs | 21 +++++++++++++++++++ .../AspNetCore/Uow/UowVisibilityController.cs | 17 +++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UnitOfWorkMiddleware_Relational_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UnitOfWorkMiddleware_Relational_Tests.cs index 00d4cdd2ec..72924981b5 100644 --- a/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UnitOfWorkMiddleware_Relational_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UnitOfWorkMiddleware_Relational_Tests.cs @@ -117,4 +117,25 @@ public class UnitOfWorkMiddleware_Relational_Tests : AbpWebApplicationFactoryInt surfaced.ShouldNotBeNull(); (await CountAsync(name)).ShouldBe(0); } + + [Fact] + public async Task Result_Serialization_Failure_Rolls_Back_And_Does_Not_Commit_On_The_Error_Response() + { + EnableCompleteOnResponseStarting(); + var name = Guid.NewGuid().ToString("N"); + + try + { + var response = await Client.GetAsync("/api/uow-visibility/insert-then-throw-in-serialization?name=" + name); + await response.Content.ReadAsStringAsync(); + } + catch (Exception) + { + } + + // The action saved the row, then serializing the result failed before the response started. The error + // response is written by the upstream exception middleware after the request unit of work is disposed, + // so response-start completion must not commit the failed request. + (await CountAsync(name)).ShouldBe(0); + } } diff --git a/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UowVisibilityController.cs b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UowVisibilityController.cs index 83d30d8410..ad0af449a2 100644 --- a/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UowVisibilityController.cs +++ b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UowVisibilityController.cs @@ -104,4 +104,21 @@ public class UowVisibilityController : AbpController await Response.WriteAsync("inserted"); await Response.Body.FlushAsync(); } + + // The action succeeds (so the action filter saves changes), then serializing the object result throws + // before the response starts. The upstream exception middleware writes the error response after the + // request unit of work is disposed, so response-start completion must not commit the failed request. + [HttpGet] + [Route("insert-then-throw-in-serialization")] + [UnitOfWork(isTransactional: true)] + public async Task InsertThenThrowInSerialization(string name) + { + await _repository.InsertAsync(new UowVisibilityTestEntity(Guid.NewGuid(), name)); + return Ok(new ThrowingOnSerializeDto()); + } + + public class ThrowingOnSerializeDto + { + public string Value => throw new AbpException("boom while serializing the object result"); + } } From e35825dc8aadf6ea780fdb1f4f98965a018ea9f0 Mon Sep 17 00:00:00 2001 From: maliming Date: Fri, 21 Aug 2026 14:33:37 +0800 Subject: [PATCH 08/11] Simplify OpenIddict endpoint path normalization --- .../Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs b/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs index 09f1001104..edaa453b2e 100644 --- a/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs +++ b/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs @@ -66,8 +66,6 @@ public class AbpOpenIddictAspNetCoreModule : AbpModule }); } - private static readonly Uri RootUri = new Uri("http://localhost/"); - private static IEnumerable GetServerEndpointPaths(OpenIddictServerOptions serverOptions) { var endpoints = serverOptions.TokenEndpointUris @@ -80,9 +78,9 @@ public class AbpOpenIddictAspNetCoreModule : AbpModule foreach (var uri in endpoints) { - // Resolve relative endpoint URIs (e.g. "connect/token" or "./connect/token") to an absolute path. - var path = (uri.IsAbsoluteUri ? uri : new Uri(RootUri, uri)).AbsolutePath; - if (!string.IsNullOrWhiteSpace(path) && path != "/") + // Normalize the (usually relative) endpoint URI to an absolute path, e.g. "connect/token" -> "/connect/token". + var path = uri.IsAbsoluteUri ? uri.AbsolutePath : "/" + uri.OriginalString.RemovePreFix("./").TrimStart('/'); + if (path.Length > 1) { yield return path; } From ab1daeeacd18b5892adf39b219fd58059733ecbf Mon Sep 17 00:00:00 2001 From: maliming Date: Fri, 21 Aug 2026 14:59:03 +0800 Subject: [PATCH 09/11] Match opt-in URLs the same way as IgnoredUrls --- .../Uow/AbpAspNetCoreUnitOfWorkOptions.cs | 11 ++--- .../AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs | 32 ++------------ .../Mvc/AbpAspNetCoreMvcTestModule.cs | 3 -- .../Mvc/Uow/UnitOfWorkMiddleware_Tests.cs | 30 ------------- .../AbpOpenIddictAspNetCoreModule.cs | 43 +++---------------- ...enIddictTokenEndpoint_Integration_Tests.cs | 2 +- 6 files changed, 14 insertions(+), 107 deletions(-) diff --git a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs index 34ba5f29c6..891b6cacf1 100644 --- a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs +++ b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs @@ -29,13 +29,10 @@ public class AbpAspNetCoreUnitOfWorkOptions public bool CompleteUnitOfWorkOnResponseStarting { get; set; } = false; /// - /// Absolute request path prefixes (matched by segment) that opt-in to - /// even when it is globally disabled (for example - /// "/connect" matches "/connect/token" but not "/connections"). Each prefix is matched against both - /// the request path and the path base + request path, so an endpoint configured with the path base - /// still matches. A trailing slash is normalized; blank, non-absolute, and root ("/") entries are - /// ignored - use to enable it for every request - /// handled by the middleware. + /// Request path prefixes that opt-in to even when + /// it is globally disabled. A request whose path starts with one of these values (for example + /// "/connect") is included, matched like . Use + /// to enable it for every request handled by the middleware. /// public List CompleteUnitOfWorkOnResponseStartingUrls { get; } = new List(); } diff --git a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs index 211a5be2cf..61ac3cc5fa 100644 --- a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs +++ b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs @@ -70,35 +70,9 @@ public class AbpUnitOfWorkMiddleware : AbpMiddlewareBase, ITransientDependency private bool ShouldCompleteOnResponseStarting(HttpContext context) { - if (_options.CompleteUnitOfWorkOnResponseStarting) - { - return true; - } - - foreach (var url in _options.CompleteUnitOfWorkOnResponseStartingUrls) - { - if (string.IsNullOrWhiteSpace(url)) - { - continue; - } - - // Normalize a trailing slash ("/connect/" behaves like "/connect") and ignore non-absolute entries. - var prefix = url.TrimEnd('/'); - if (!prefix.StartsWith("/", StringComparison.Ordinal)) - { - continue; - } - - // Match both the request path and the path base + path, so an absolute endpoint that includes - // the path base still matches when the path base is stripped from Request.Path. - if (context.Request.Path.StartsWithSegments(prefix, StringComparison.OrdinalIgnoreCase) || - context.Request.PathBase.Add(context.Request.Path).StartsWithSegments(prefix, StringComparison.OrdinalIgnoreCase)) - { - return true; - } - } - - return false; + return _options.CompleteUnitOfWorkOnResponseStarting || + (context.Request.Path.Value != null && + _options.CompleteUnitOfWorkOnResponseStartingUrls.Any(x => context.Request.Path.Value.StartsWith(x, StringComparison.OrdinalIgnoreCase))); } protected async override Task ShouldSkipAsync(HttpContext context, RequestDelegate next) diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcTestModule.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcTestModule.cs index 02663eae90..4339104b30 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcTestModule.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcTestModule.cs @@ -164,9 +164,6 @@ public class AbpAspNetCoreMvcTestModule : AbpModule app.UseStaticFiles(); app.UseAbpRequestLocalization(); app.UseAbpSecurityHeaders(); - // Moves the "/pathbase-test" prefix into Request.PathBase so a unit of work opt-in test can - // exercise path base matching; a no-op for every other request. - app.UsePathBase("/pathbase-test"); app.UseRouting(); app.UseAuthentication(); app.UseAuthorization(); diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs index 26f81ef3a0..283a7ad699 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs @@ -119,34 +119,4 @@ public class UnitOfWorkMiddleware_Tests : AspNetCoreMvcTestBase var result = await GetResponseAsStringAsync("/api/unitofwork-test/CommitBeforeResponseFlush"); result.ShouldBe("first:completed"); } - - [Fact] - public async Task Opt_In_Url_With_A_Non_Segment_Prefix_Should_Not_Match() - { - Options.CompleteUnitOfWorkOnResponseStartingUrls.Add("/api/unitofwork-test/Commit"); - - var result = await GetResponseAsStringAsync("/api/unitofwork-test/CommitBeforeResponseFlush"); - result.ShouldBe("first:not-completed"); - } - - [Fact] - public async Task Blank_Or_Malformed_Opt_In_Urls_Are_Ignored() - { - Options.CompleteUnitOfWorkOnResponseStartingUrls.Add(""); - Options.CompleteUnitOfWorkOnResponseStartingUrls.Add(" "); - Options.CompleteUnitOfWorkOnResponseStartingUrls.Add("api/no-leading-slash"); - - var result = await GetResponseAsStringAsync("/api/unitofwork-test/CommitBeforeResponseFlush"); - result.ShouldBe("first:not-completed"); - } - - [Fact] - public async Task Opt_In_Url_Including_The_Path_Base_Matches() - { - // The request path is "/api/..." (the path base is stripped), so this only matches via path base + path. - Options.CompleteUnitOfWorkOnResponseStartingUrls.Add("/pathbase-test/api/unitofwork-test/CommitBeforeResponseFlush"); - - var result = await GetResponseAsStringAsync("/pathbase-test/api/unitofwork-test/CommitBeforeResponseFlush"); - result.ShouldBe("first:completed"); - } } diff --git a/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs b/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs index edaa453b2e..ea03453c52 100644 --- a/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs +++ b/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs @@ -1,10 +1,7 @@ -using System; -using System.Collections.Generic; -using System.Linq; +using System.Collections.Generic; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc.Razor; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; using OpenIddict.Abstractions; using OpenIddict.Server; using Volo.Abp.AspNetCore.MultiTenancy; @@ -51,40 +48,12 @@ public class AbpOpenIddictAspNetCoreModule : AbpModule options.RemoveClientIdClaim(); }); - // Complete data written while processing OpenIddict requests before the response starts. - // Derived from the configured OpenIddict server endpoint paths (including the device endpoint). - context.Services.AddOptions() - .Configure>((uowOptions, serverOptions) => - { - foreach (var path in GetServerEndpointPaths(serverOptions.Value)) - { - if (!uowOptions.CompleteUnitOfWorkOnResponseStartingUrls.Contains(path)) - { - uowOptions.CompleteUnitOfWorkOnResponseStartingUrls.Add(path); - } - } - }); - } - - private static IEnumerable GetServerEndpointPaths(OpenIddictServerOptions serverOptions) - { - var endpoints = serverOptions.TokenEndpointUris - .Concat(serverOptions.AuthorizationEndpointUris) - .Concat(serverOptions.DeviceAuthorizationEndpointUris) - .Concat(serverOptions.PushedAuthorizationEndpointUris) - .Concat(serverOptions.EndSessionEndpointUris) - .Concat(serverOptions.RevocationEndpointUris) - .Concat(serverOptions.EndUserVerificationEndpointUris); - - foreach (var uri in endpoints) + // Complete tokens/authorizations/sessions written while processing OpenIddict requests before the response starts. + Configure(options => { - // Normalize the (usually relative) endpoint URI to an absolute path, e.g. "connect/token" -> "/connect/token". - var path = uri.IsAbsoluteUri ? uri.AbsolutePath : "/" + uri.OriginalString.RemovePreFix("./").TrimStart('/'); - if (path.Length > 1) - { - yield return path; - } - } + options.CompleteUnitOfWorkOnResponseStartingUrls.AddIfNotContains("/connect"); + options.CompleteUnitOfWorkOnResponseStartingUrls.AddIfNotContains("/device"); + }); } private void AddOpenIddictServer(IServiceCollection services) diff --git a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs index 483e64694b..cf11cffdf3 100644 --- a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs +++ b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs @@ -34,7 +34,7 @@ public class OpenIddictTokenEndpoint_Integration_Tests : AbpWebApplicationFactor [Fact] public async Task Token_Row_Is_Committed_Before_The_Connect_Token_Response_Is_Sent() { - // The OpenIddict module opts its endpoint paths (including "/connect/token") in by default. + // The OpenIddict module opts "/connect" in by default. var response = await RequestTokenAsync(); response.StatusCode.ShouldBe(HttpStatusCode.OK); From d79fa71b4141b8d282cd1a9395b44af5b428a477 Mon Sep 17 00:00:00 2001 From: maliming Date: Fri, 21 Aug 2026 15:39:57 +0800 Subject: [PATCH 10/11] Derive the OpenIddict opt-in URLs from the configured server endpoints --- .../UnitOfWorkMiddleware_Relational_Tests.cs | 14 +++++--- .../AbpOpenIddictAspNetCoreModule.cs | 36 ++++++++++++++++--- ...enIddictTokenEndpoint_Integration_Tests.cs | 13 ++++++- .../OpenIddictTokenIntegrationTestModule.cs | 7 ++++ 4 files changed, 59 insertions(+), 11 deletions(-) diff --git a/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UnitOfWorkMiddleware_Relational_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UnitOfWorkMiddleware_Relational_Tests.cs index 72924981b5..2d20c60005 100644 --- a/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UnitOfWorkMiddleware_Relational_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Uow.Tests/Volo/Abp/AspNetCore/Uow/UnitOfWorkMiddleware_Relational_Tests.cs @@ -124,18 +124,22 @@ public class UnitOfWorkMiddleware_Relational_Tests : AbpWebApplicationFactoryInt EnableCompleteOnResponseStarting(); var name = Guid.NewGuid().ToString("N"); + HttpResponseMessage response = null; + Exception surfaced = null; try { - var response = await Client.GetAsync("/api/uow-visibility/insert-then-throw-in-serialization?name=" + name); + response = await Client.GetAsync("/api/uow-visibility/insert-then-throw-in-serialization?name=" + name); await response.Content.ReadAsStringAsync(); } - catch (Exception) + catch (Exception ex) { + surfaced = ex; } - // The action saved the row, then serializing the result failed before the response started. The error - // response is written by the upstream exception middleware after the request unit of work is disposed, - // so response-start completion must not commit the failed request. + // The action ran and saved the row, then serializing the result failed. The request must therefore + // fail with a server error (not a 404 or a success), and the error response, written by the upstream + // exception middleware after the request unit of work is disposed, must not commit the failed request. + (surfaced != null || (response != null && (int)response.StatusCode >= 500)).ShouldBeTrue(); (await CountAsync(name)).ShouldBe(0); } } diff --git a/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs b/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs index ea03453c52..7e025a80b8 100644 --- a/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs +++ b/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs @@ -1,7 +1,9 @@ using System.Collections.Generic; +using System.Linq; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc.Razor; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using OpenIddict.Abstractions; using OpenIddict.Server; using Volo.Abp.AspNetCore.MultiTenancy; @@ -48,12 +50,36 @@ public class AbpOpenIddictAspNetCoreModule : AbpModule options.RemoveClientIdClaim(); }); - // Complete tokens/authorizations/sessions written while processing OpenIddict requests before the response starts. - Configure(options => + // Complete data written while processing OpenIddict requests before the response starts. + // Derived from the configured server endpoint paths so remapped endpoints are covered too. + context.Services.AddOptions() + .Configure>((uowOptions, serverOptions) => + { + foreach (var path in GetServerEndpointPaths(serverOptions.Value)) + { + uowOptions.CompleteUnitOfWorkOnResponseStartingUrls.AddIfNotContains(path); + } + }); + } + + private static IEnumerable GetServerEndpointPaths(OpenIddictServerOptions serverOptions) + { + var endpoints = serverOptions.TokenEndpointUris + .Concat(serverOptions.AuthorizationEndpointUris) + .Concat(serverOptions.DeviceAuthorizationEndpointUris) + .Concat(serverOptions.PushedAuthorizationEndpointUris) + .Concat(serverOptions.EndSessionEndpointUris) + .Concat(serverOptions.RevocationEndpointUris) + .Concat(serverOptions.EndUserVerificationEndpointUris); + + foreach (var uri in endpoints) { - options.CompleteUnitOfWorkOnResponseStartingUrls.AddIfNotContains("/connect"); - options.CompleteUnitOfWorkOnResponseStartingUrls.AddIfNotContains("/device"); - }); + var path = uri.IsAbsoluteUri ? uri.AbsolutePath : "/" + uri.OriginalString.TrimStart('/'); + if (path.Length > 1) + { + yield return path; + } + } } private void AddOpenIddictServer(IServiceCollection services) diff --git a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs index cf11cffdf3..3f8fd1a855 100644 --- a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs +++ b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenEndpoint_Integration_Tests.cs @@ -34,7 +34,7 @@ public class OpenIddictTokenEndpoint_Integration_Tests : AbpWebApplicationFactor [Fact] public async Task Token_Row_Is_Committed_Before_The_Connect_Token_Response_Is_Sent() { - // The OpenIddict module opts "/connect" in by default. + // The OpenIddict module opts its configured endpoint paths (including "/connect/token") in by default. var response = await RequestTokenAsync(); response.StatusCode.ShouldBe(HttpStatusCode.OK); @@ -55,4 +55,15 @@ public class OpenIddictTokenEndpoint_Integration_Tests : AbpWebApplicationFactor response.StatusCode.ShouldBe(HttpStatusCode.OK); TokenCountAtResponseStart.ShouldBe(0); } + + [Fact] + public void The_Configured_OpenIddict_Endpoint_Paths_Are_Opted_In() + { + // The opt-in list is derived from the configured server endpoints, so a non-"/connect" endpoint + // like "/device" is covered, and the custom "/my-custom/token" endpoint the test host registered + // is followed too - a hardcoded "/connect" prefix would miss both. + Options.CompleteUnitOfWorkOnResponseStartingUrls.ShouldContain("/connect/token"); + Options.CompleteUnitOfWorkOnResponseStartingUrls.ShouldContain("/device"); + Options.CompleteUnitOfWorkOnResponseStartingUrls.ShouldContain("/my-custom/token"); + } } diff --git a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenIntegrationTestModule.cs b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenIntegrationTestModule.cs index 1319045ca6..29e9c5a180 100644 --- a/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenIntegrationTestModule.cs +++ b/modules/openiddict/test/Volo.Abp.OpenIddict.AspNetCore.Tests/Volo/Abp/OpenIddict/Integration/OpenIddictTokenIntegrationTestModule.cs @@ -60,6 +60,13 @@ public class OpenIddictTokenIntegrationTestModule : AbpModule { context.Services.AddSingleton(); + // A remapped token endpoint, so the tests can prove the opt-in list is derived from the configured + // server endpoints (custom endpoints are followed) rather than a hardcoded "/connect" prefix. + Configure(options => + { + options.TokenEndpointUris.Add(new Uri("my-custom/token", UriKind.Relative)); + }); + // The OpenIddict controllers (including the token endpoint) live in a referenced assembly. context.Services.GetSingletonInstance() .ApplicationParts.AddIfNotContains(typeof(AbpOpenIddictAspNetCoreModule).Assembly); From ccd673cf201af0fa22b09670340f5bee6d8cf909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Halil=20=C4=B0brahim=20Kalkan?= Date: Sun, 23 Aug 2026 14:30:53 +0300 Subject: [PATCH 11/11] Guard UoW completion during response start This change hardens `AbpUnitOfWorkMiddleware` to avoid invalid second completion attempts when the response starts during end-of-pipeline UoW completion, and to skip early completion while a shared child UoW scope is still active. It adds active child-scope tracking in `UnitOfWorkExtensions`/`ChildUnitOfWork`, updates option docs to clarify behavior, and adds MVC tests plus controller/event-handler scenarios covering child-scope response flush and event-driven response writes during completion. --- .../Uow/AbpAspNetCoreUnitOfWorkOptions.cs | 5 +-- .../AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs | 18 +++++++--- .../Volo/Abp/Uow/ChildUnitOfWork.cs | 8 +++++ .../Volo/Abp/Uow/UnitOfWorkExtensions.cs | 27 +++++++++++++++ .../Mvc/Uow/ResponseWritingTestEvent.cs | 34 +++++++++++++++++++ .../Mvc/Uow/UnitOfWorkMiddleware_Tests.cs | 25 ++++++++++++++ .../Mvc/Uow/UnitOfWorkTestController.cs | 32 +++++++++++++++++ 7 files changed, 142 insertions(+), 7 deletions(-) create mode 100644 framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/ResponseWritingTestEvent.cs diff --git a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs index 891b6cacf1..a38d09f1fb 100644 --- a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs +++ b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpAspNetCoreUnitOfWorkOptions.cs @@ -22,8 +22,9 @@ public class AbpAspNetCoreUnitOfWorkOptions /// committed data (commit and network response are not atomic); database access after the response /// starts is outside the request unit of work (unsuitable for streaming responses); unit of work /// events and completed handlers run before the first response byte (adding to its latency); a - /// nested (requiresNew) unit of work that is current when the response starts is left to its owner - /// and the request unit of work then completes at the end of the pipeline as usual. + /// nested (requiresNew) unit of work that is current when the response starts, and an active child + /// unit of work scope (begun without requiresNew), are left to their owners and the request unit of + /// work then completes at the end of the pipeline as usual. /// /// public bool CompleteUnitOfWorkOnResponseStarting { get; set; } = false; diff --git a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs index 61ac3cc5fa..64c507d260 100644 --- a/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs +++ b/framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs @@ -37,17 +37,24 @@ public class AbpUnitOfWorkMiddleware : AbpMiddlewareBase, ITransientDependency using (var uow = _unitOfWorkManager.Reserve(UnitOfWork.UnitOfWorkReservationName)) { - var completionAttemptedOnResponseStarting = false; + var completionStarted = false; if (!context.Response.HasStarted && ShouldCompleteOnResponseStarting(context)) { context.Response.OnStarting(async () => { - // A nested (requiresNew) unit of work that is current is left to its owner. - if (_unitOfWorkManager.Current == uow) + // Skip if the completion has already been started at the end of the pipeline; + // the response is then being started from inside that completion (e.g. by an + // event handler writing to the response), so completing again would fail. + // A nested (requiresNew) unit of work that is current and an active child + // unit of work scope are left to their owners; the request unit of work then + // completes at the end of the pipeline as usual. + if (!completionStarted && + _unitOfWorkManager.Current == uow && + !uow.HasActiveChildUnitOfWorks()) { // Set before completing so a post-commit failure isn't masked by the completion below. - completionAttemptedOnResponseStarting = true; + completionStarted = true; await uow.CompleteAsync(_cancellationTokenProvider.Token); } }); @@ -55,8 +62,9 @@ public class AbpUnitOfWorkMiddleware : AbpMiddlewareBase, ITransientDependency await next(context); - if (!completionAttemptedOnResponseStarting) + if (!completionStarted) { + completionStarted = true; await uow.CompleteAsync(_cancellationTokenProvider.Token); } } diff --git a/framework/src/Volo.Abp.Uow/Volo/Abp/Uow/ChildUnitOfWork.cs b/framework/src/Volo.Abp.Uow/Volo/Abp/Uow/ChildUnitOfWork.cs index 5f400abcfd..14625aaf4a 100644 --- a/framework/src/Volo.Abp.Uow/Volo/Abp/Uow/ChildUnitOfWork.cs +++ b/framework/src/Volo.Abp.Uow/Volo/Abp/Uow/ChildUnitOfWork.cs @@ -30,12 +30,14 @@ internal class ChildUnitOfWork : IUnitOfWork public Dictionary Items => _parent.Items; private readonly IUnitOfWork _parent; + private bool _isDisposed; public ChildUnitOfWork([NotNull] IUnitOfWork parent) { Check.NotNull(parent, nameof(parent)); _parent = parent; + _parent.IncrementActiveChildUnitOfWorkCount(); _parent.Failed += (sender, args) => { Failed.InvokeSafely(sender!, args); }; _parent.Disposed += (sender, args) => { Disposed.InvokeSafely(sender!, args); }; @@ -122,7 +124,13 @@ internal class ChildUnitOfWork : IUnitOfWork public void Dispose() { + if (_isDisposed) + { + return; + } + _isDisposed = true; + _parent.DecrementActiveChildUnitOfWorkCount(); } public override string ToString() diff --git a/framework/src/Volo.Abp.Uow/Volo/Abp/Uow/UnitOfWorkExtensions.cs b/framework/src/Volo.Abp.Uow/Volo/Abp/Uow/UnitOfWorkExtensions.cs index a732e5ad21..a037023472 100644 --- a/framework/src/Volo.Abp.Uow/Volo/Abp/Uow/UnitOfWorkExtensions.cs +++ b/framework/src/Volo.Abp.Uow/Volo/Abp/Uow/UnitOfWorkExtensions.cs @@ -7,6 +7,8 @@ namespace Volo.Abp.Uow; public static class UnitOfWorkExtensions { + private const string ActiveChildUnitOfWorkCountItemKey = "_AbpActiveChildUnitOfWorkCount"; + public static bool IsReservedFor([NotNull] this IUnitOfWork unitOfWork, string reservationName) { Check.NotNull(unitOfWork, nameof(unitOfWork)); @@ -14,6 +16,31 @@ public static class UnitOfWorkExtensions return unitOfWork.IsReserved && unitOfWork.ReservationName == reservationName; } + /// + /// Checks if there is an active (not yet disposed) child unit of work scope over the given + /// unit of work, i.e. a scope created by without + /// requiresNew while this unit of work was current. Such a scope shares this unit of work, + /// so it should not be completed while the scope is still active. + /// + public static bool HasActiveChildUnitOfWorks([NotNull] this IUnitOfWork unitOfWork) + { + Check.NotNull(unitOfWork, nameof(unitOfWork)); + + return unitOfWork.Items.GetOrDefault(ActiveChildUnitOfWorkCountItemKey) is int count && count > 0; + } + + internal static void IncrementActiveChildUnitOfWorkCount(this IUnitOfWork unitOfWork) + { + var count = unitOfWork.Items.GetOrDefault(ActiveChildUnitOfWorkCountItemKey) as int? ?? 0; + unitOfWork.Items[ActiveChildUnitOfWorkCountItemKey] = count + 1; + } + + internal static void DecrementActiveChildUnitOfWorkCount(this IUnitOfWork unitOfWork) + { + var count = unitOfWork.Items.GetOrDefault(ActiveChildUnitOfWorkCountItemKey) as int? ?? 0; + unitOfWork.Items[ActiveChildUnitOfWorkCountItemKey] = Math.Max(0, count - 1); + } + public static void AddItem([NotNull] this IUnitOfWork unitOfWork, string key, TValue value) where TValue : class { diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/ResponseWritingTestEvent.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/ResponseWritingTestEvent.cs new file mode 100644 index 0000000000..989ec5b294 --- /dev/null +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/ResponseWritingTestEvent.cs @@ -0,0 +1,34 @@ +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Volo.Abp.DependencyInjection; +using Volo.Abp.EventBus; + +namespace Volo.Abp.AspNetCore.Mvc.Uow; + +public class ResponseWritingTestEvent +{ +} + +/// +/// Writes to the HTTP response from a local event handler. When the event is published inside the +/// request unit of work, this runs during the unit of work completion at the end of the pipeline +/// and starts the response from inside that completion. +/// +public class ResponseWritingTestEventHandler : ILocalEventHandler, ITransientDependency +{ + private readonly IHttpContextAccessor _httpContextAccessor; + + public ResponseWritingTestEventHandler(IHttpContextAccessor httpContextAccessor) + { + _httpContextAccessor = httpContextAccessor; + } + + public async Task HandleEventAsync(ResponseWritingTestEvent eventData) + { + var response = _httpContextAccessor.HttpContext?.Response; + if (response != null) + { + await response.WriteAsync("event-written"); + } + } +} diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs index 283a7ad699..50d058903e 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Tests.cs @@ -119,4 +119,29 @@ public class UnitOfWorkMiddleware_Tests : AspNetCoreMvcTestBase var result = await GetResponseAsStringAsync("/api/unitofwork-test/CommitBeforeResponseFlush"); result.ShouldBe("first:completed"); } + + [Fact] + public async Task Response_Flush_Inside_A_Child_Uow_Scope_Should_Not_Complete_The_Request_Uow() + { + Options.CompleteUnitOfWorkOnResponseStarting = true; + + // A child scope (Begin without requiresNew) shares the request unit of work, so completing + // it on response start would commit under the still-active scope; it is left to the end of + // the pipeline instead, like a nested (requiresNew) unit of work. + var body = await GetResponseAsStringAsync("/api/unitofwork-test/ChildUowDuringResponseFlush"); + body.ShouldBe("first:request-not-completed"); + } + + [Fact] + public async Task An_Event_Handler_Starting_The_Response_During_The_End_Of_Pipeline_Completion_Should_Not_Fail() + { + Options.CompleteUnitOfWorkOnResponseStarting = true; + + // The response does not start during the pipeline here, so the middleware completes the + // unit of work at its end; the event handler then starts the response from inside that + // completion. The OnStarting callback must not attempt a second completion (which would + // throw "Completion has already been requested for this unit of work"). + var body = await GetResponseAsStringAsync("/api/unitofwork-test/PublishEventThatWritesResponseOnCompletion"); + body.ShouldBe("event-written"); + } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs index 9185b41e03..77cf3f07b4 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Mvc; using Shouldly; using Volo.Abp; using Volo.Abp.Domain.Repositories; +using Volo.Abp.EventBus.Local; using Volo.Abp.MemoryDb; using Volo.Abp.TestApp.MemoryDb; using Volo.Abp.TestApp.Domain; @@ -186,4 +187,35 @@ public class UnitOfWorkTestController : AbpController // The middleware must still try to complete it at the end of the pipeline (original behavior). await CurrentUnitOfWork.CompleteAsync(); } + + [HttpGet] + [Route("ChildUowDuringResponseFlush")] + public async Task ChildUowDuringResponseFlush() + { + var requestUow = CurrentUnitOfWork!; + + using (UnitOfWorkManager.Begin()) + { + await Response.WriteAsync("first"); + await Response.Body.FlushAsync(); + + // The request unit of work must not be completed on response start while a child + // unit of work scope (begun without requiresNew) is still active over it. + await Response.WriteAsync(requestUow.IsCompleted ? ":request-completed" : ":request-not-completed"); + } + } + + [HttpGet] + [Route("PublishEventThatWritesResponseOnCompletion")] + public async Task PublishEventThatWritesResponseOnCompletion() + { + // Published inside the request unit of work, so the handler runs while the middleware is + // completing it at the end of the pipeline. The handler writes to the response, which starts + // it mid-completion; the middleware's OnStarting callback must not try to complete again. + await LazyServiceProvider.LazyGetRequiredService() + .PublishAsync(new ResponseWritingTestEvent()); + + // Ok() sets 200 without writing the body, so the response does not start inside the pipeline. + return Ok(); + } }