Fix unit of work middleware edge cases and document its options

docs/en/framework/architecture/domain-driven-design/unit-of-work.md

@@ -361,6 +361,30 @@ app.UseUnitOfWork();
 app.UseConfiguredEndpoints();
 ````
 
+By default, the middleware completes the unit of work at the end of the request pipeline. If the response has already started, a failure (like a transaction commit or a unit of work event handler failure) can not change the status code, so the client may still get a success response.
+
+`UseUnitOfWork` also adds the exception handling middleware. If you don't use `UseUnitOfWork`, add `UseAbpExceptionHandling` to the same place:
+
+````csharp
+app.UseAbpExceptionHandling();
+app.UseConfiguredEndpoints();
+````
+
+#### AbpAspNetCoreUnitOfWorkOptions
+
+`AbpAspNetCoreUnitOfWorkOptions` is used to configure the middleware:
+
+````csharp
+Configure<AbpAspNetCoreUnitOfWorkOptions>(options =>
+{
+    options.IgnoredUrls.Add("/api/my-streaming-endpoint");
+});
+````
+
+* `IgnoredUrls` (`List<string>`): The middleware is disabled for the requests whose path starts with one of these values. It is also disabled for the [SignalR](../../real-time/signalr.md) hub requests.
+* `CompleteUnitOfWorkOnResponseStarting` (`bool`, default: `false`): Completes the unit of work just before the response starts, so a failure can return an error status code (with an empty response body). The changes are committed at that point, so a later failure can not roll them back. It is not suitable for streaming responses.
+* `CompleteUnitOfWorkOnResponseStartingUrls` (`List<string>`): Completes the unit of work just before the response starts for the requests whose path starts with one of these values, even if `CompleteUnitOfWorkOnResponseStarting` is `false`. The [OpenIddict module](../../../modules/openiddict.md) adds some of its server endpoints, like the token endpoint, to this list.
+
 ## See Also
 
 * [Video tutorial Code](https://abp.io/video-courses/essentials/unit-of-work-code)

framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Uow/AbpUnitOfWorkMiddleware.cs

@@ -3,6 +3,7 @@
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Components.Endpoints;
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.SignalR;
 using Microsoft.Extensions.Options;
 using Volo.Abp.AspNetCore.Middleware;
 using Volo.Abp.DependencyInjection;
@@ -94,6 +95,13 @@ protected async override Task<bool> ShouldSkipAsync(HttpContext context, Request
             return true;
         }
 
+        // A hub connection request lives as long as the connection. A unit of work reserved for it would be
+        // shared by all the hub method calls of the connection and can not be completed when the client disconnects.
+        if (context.GetEndpoint()?.Metadata?.GetMetadata<HubMetadata>() != null)
+        {
+            return true;
+        }
+
         return await base.ShouldSkipAsync(context, next);
     }
 }

framework/src/Volo.Abp.Uow/Volo/Abp/Uow/UnitOfWorkManager.cs

@@ -71,7 +71,8 @@ public bool TryBeginReserved(string reservationName, AbpUnitOfWorkOptions option
         var uow = _ambientUnitOfWork.UnitOfWork;
 
         //Find reserved unit of work starting from current and going to outers
-        while (uow != null && !uow.IsReservedFor(reservationName))
+        //A reservation that has ended can still be the ambient one for the code that outlives it
+        while (uow != null && (!uow.IsReservedFor(reservationName) || uow.IsCompleted || uow.IsDisposed))
         {
             uow = uow.Outer;
         }

framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs

@@ -3,7 +3,9 @@
 using System.Threading.Tasks;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Extensions.Options;
 using Shouldly;
+using Volo.Abp.AspNetCore.Uow;
 using Volo.Abp.Http;
 using Volo.Abp.Json;
 using Volo.Abp.Uow;
@@ -40,4 +42,18 @@ public async Task Should_Gracefully_Handle_Exceptions_On_Complete()
         result.Error.ShouldNotBeNull();
         result.Error.Message.ShouldBe(TestUnitOfWorkConfig.ExceptionOnCompleteMessage);
     }
+
+    [Fact]
+    public async Task Should_Return_The_Error_When_Completing_Fails_After_An_Object_Result_Without_The_Middleware()
+    {
+        const string url = "/api/unitofwork-test/ObjectResultExceptionOnComplete";
+
+        // The request runs as if the pipeline only had app.UseAbpExceptionHandling().
+        ServiceProvider.GetRequiredService<IOptions<AbpAspNetCoreUnitOfWorkOptions>>().Value.IgnoredUrls.Add(url);
+
+        var result = await GetResponseAsObjectAsync<RemoteServiceErrorResponse>(url, HttpStatusCode.Forbidden);
+
+        result.Error.ShouldNotBeNull();
+        result.Error.Message.ShouldBe(TestUnitOfWorkConfig.ExceptionOnCompleteMessage);
+    }
 }

framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs

@@ -74,6 +74,15 @@ public void ExceptionOnComplete()
         _testUnitOfWorkConfig.ThrowExceptionOnComplete = true;
     }
 
+    [HttpGet]
+    [Route("ObjectResultExceptionOnComplete")]
+    public object ObjectResultExceptionOnComplete()
+    {
+        _testUnitOfWorkConfig.ThrowExceptionOnComplete = true;
+
+        return new { Value = 42 };
+    }
+
     [HttpGet]
     [Route("CommitBeforeResponseFlush")]
     public async Task CommitBeforeResponseFlush()

framework/test/Volo.Abp.AspNetCore.SignalR.Tests/Volo/Abp/AspNetCore/SignalR/AbpAspNetCoreSignalRTestModule.cs

@@ -1,8 +1,11 @@
 using System;
 using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.DependencyInjection;
+using Volo.Abp.AspNetCore.SignalR.SampleHubs;
 using Volo.Abp.AspNetCore.TestBase;
 using Volo.Abp.Autofac;
 using Volo.Abp.Modularity;
+using Volo.Abp.Uow;
 
 namespace Volo.Abp.AspNetCore.SignalR;
 
@@ -13,18 +16,31 @@ namespace Volo.Abp.AspNetCore.SignalR;
     )]
 public class AbpAspNetCoreSignalRTestModule : AbpModule
 {
+    public const string UnitOfWorkReservedHeaderName = "X-Unit-Of-Work-Reserved";
+
     public static Exception UseConfiguredEndpointsException { get; set; }
 
     public override void OnApplicationInitialization(ApplicationInitializationContext context)
     {
         var app = context.GetApplicationBuilder();
 
         app.UseRouting();
+        app.UseUnitOfWork();
+        app.Use(async (httpContext, next) =>
+        {
+            httpContext.Response.Headers[UnitOfWorkReservedHeaderName] =
+                (httpContext.RequestServices.GetRequiredService<IAmbientUnitOfWork>().UnitOfWork?.IsReserved == true).ToString();
+
+            await next();
+        });
 
         UseConfiguredEndpointsException = null;
         try
         {
-            app.UseConfiguredEndpoints();
+            app.UseConfiguredEndpoints(endpoints =>
+            {
+                endpoints.MapHub<DisableAutoHubMapHub>("/manually-mapped-hub");
+            });
         }
         catch (Exception e)
         {

framework/test/Volo.Abp.AspNetCore.SignalR.Tests/Volo/Abp/AspNetCore/SignalR/AbpSignalRUnitOfWork_Tests.cs

@@ -0,0 +1,38 @@
+using System.Linq;
+using System.Net;
+using System.Net.Http;
+using System.Threading.Tasks;
+using Shouldly;
+using Xunit;
+
+namespace Volo.Abp.AspNetCore.SignalR;
+
+public class AbpSignalRUnitOfWork_Tests : AbpAspNetCoreTestBase<Program>
+{
+    [Theory]
+    [InlineData("/signalr-hubs/regular-abp/negotiate?negotiateVersion=1")]
+    [InlineData("/custom-route-hub/negotiate?negotiateVersion=1")]
+    [InlineData("/manually-mapped-hub/negotiate?negotiateVersion=1")]
+    public async Task Should_Not_Reserve_A_Unit_Of_Work_For_The_Hub_Requests(string url)
+    {
+        var response = await Client.PostAsync(url, null);
+
+        response.StatusCode.ShouldBe(HttpStatusCode.OK);
+        IsUnitOfWorkReserved(response).ShouldBeFalse();
+    }
+
+    [Theory]
+    [InlineData("/not-a-hub")]
+    [InlineData("/custom-route-hub-export")]
+    public async Task Should_Reserve_A_Unit_Of_Work_For_The_Other_Requests(string url)
+    {
+        var response = await Client.GetAsync(url);
+
+        IsUnitOfWorkReserved(response).ShouldBeTrue();
+    }
+
+    private static bool IsUnitOfWorkReserved(HttpResponseMessage response)
+    {
+        return bool.Parse(response.Headers.GetValues(AbpAspNetCoreSignalRTestModule.UnitOfWorkReservedHeaderName).Single());
+    }
+}

framework/test/Volo.Abp.AspNetCore.SignalR.Tests/Volo/Abp/AspNetCore/SignalR/SampleHubs/CustomRouteHub.cs

@@ -0,0 +1,6 @@
+namespace Volo.Abp.AspNetCore.SignalR.SampleHubs;
+
+[HubRoute("/custom-route-hub")]
+public class CustomRouteHub : AbpHub
+{
+}

framework/test/Volo.Abp.Uow.Tests/Volo/Abp/Uow/UnitOfWork_Ambient_Scope_Tests.cs

@@ -71,4 +71,49 @@ public async Task UnitOfWorkManager_Reservation_Test()
 
         _unitOfWorkManager.Current.ShouldBeNull();
     }
+
+    [Theory]
+    [InlineData(true, false)]
+    [InlineData(true, true)]
+    [InlineData(false, true)]
+    public async Task Should_Not_Begin_A_Reserved_Unit_Of_Work_That_Is_Not_Alive_Anymore(bool complete, bool dispose)
+    {
+        var reservationEnded = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+
+        using (var uow1 = _unitOfWorkManager.Reserve("Reservation1"))
+        {
+            // Captures the reserved unit of work as its ambient one and keeps running after the reservation ends,
+            // like the work that is started by a request and outlives it.
+            var outlivingWork = Task.Run(async () =>
+            {
+                await reservationEnded.Task;
+
+                var isBegun = _unitOfWorkManager.TryBeginReserved("Reservation1", new AbpUnitOfWorkOptions());
+
+                using (var uow2 = _unitOfWorkManager.Begin())
+                {
+                    _unitOfWorkManager.Current.ShouldNotBeNull();
+                    _unitOfWorkManager.Current.Id.ShouldNotBe(uow1.Id);
+
+                    await uow2.CompleteAsync();
+                }
+
+                return isBegun;
+            });
+
+            if (complete)
+            {
+                await uow1.CompleteAsync();
+            }
+
+            if (dispose)
+            {
+                uow1.Dispose();
+            }
+
+            reservationEnded.SetResult();
+
+            (await outlivingWork).ShouldBeFalse();
+        }
+    }
 }