From badb9b96d4ff5e366a1f952927bd6f057363bf14 Mon Sep 17 00:00:00 2001 From: maliming Date: Thu, 20 Oct 2022 17:20:19 +0800 Subject: [PATCH 1/2] `Rollback` if `SaveChanges` failed. --- .../Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs index 5c955ca005..8f6cc2abf8 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs @@ -98,7 +98,15 @@ public class AbpUowActionFilter : IAsyncActionFilter, ITransientDependency var currentUow = unitOfWorkManager.Current; if (currentUow != null) { - await currentUow.SaveChangesAsync(context.HttpContext.RequestAborted); + try + { + await currentUow.SaveChangesAsync(context.HttpContext.RequestAborted); + } + catch (Exception e) + { + await currentUow.RollbackAsync(context.HttpContext.RequestAborted); + throw; + } } } From 59c6a94ee98f4a602075549d088137090386f9c3 Mon Sep 17 00:00:00 2001 From: maliming Date: Thu, 20 Oct 2022 18:00:37 +0800 Subject: [PATCH 2/2] Add unit tests. --- .../AspNetCore/Mvc/Uow/AbpUowPageFilter.cs | 10 +++++- .../ExceptionHandingUnitOfWork.cs | 32 +++++++++++++++++++ .../ExceptionTestController.cs | 10 +++++- .../ExceptionTestController_Tests.cs | 24 ++++++++++++++ .../ExceptionTestPage.cshtml.cs | 5 +++ .../ExceptionTestPage_Tests.cs | 25 +++++++++++++++ ...WorkMiddleware_Exception_Rollback_Tests.cs | 8 +++++ ...WorkPageFilter_Exception_Rollback_Tests.cs | 8 +++++ 8 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionHandingUnitOfWork.cs diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowPageFilter.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowPageFilter.cs index af966fedce..57e1df8558 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowPageFilter.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowPageFilter.cs @@ -104,7 +104,15 @@ public class AbpUowPageFilter : IAsyncPageFilter, ITransientDependency var currentUow = unitOfWorkManager.Current; if (currentUow != null) { - await currentUow.SaveChangesAsync(context.HttpContext.RequestAborted); + try + { + await currentUow.SaveChangesAsync(context.HttpContext.RequestAborted); + } + catch (Exception e) + { + await currentUow.RollbackAsync(context.HttpContext.RequestAborted); + throw; + } } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionHandingUnitOfWork.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionHandingUnitOfWork.cs new file mode 100644 index 0000000000..94cef2e240 --- /dev/null +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionHandingUnitOfWork.cs @@ -0,0 +1,32 @@ +using System; +using System.Data; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Volo.Abp.Data; +using Volo.Abp.Uow; +using Volo.Abp.Users; + +namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling; + +public class ExceptionHandingUnitOfWork : UnitOfWork +{ + public ExceptionHandingUnitOfWork( + IServiceProvider serviceProvider, + IUnitOfWorkEventPublisher unitOfWorkEventPublisher, + IOptions options) + : base(serviceProvider, unitOfWorkEventPublisher, options) + { + + } + public async override Task SaveChangesAsync(CancellationToken cancellationToken = default) + { + if (ServiceProvider.GetRequiredService().Id == Guid.Empty) + { + throw new AbpDbConcurrencyException(); + } + + await base.SaveChangesAsync(cancellationToken); + } +} diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestController.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestController.cs index 0b5224ce8c..07daeb6d03 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestController.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestController.cs @@ -1,4 +1,5 @@ -using Microsoft.AspNetCore.Mvc; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc; using Volo.Abp.Authorization; namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling; @@ -26,4 +27,11 @@ public class ExceptionTestController : AbpController { throw new AbpAuthorizationException("This is a sample exception!"); } + + [HttpGet] + [Route("ExceptionOnUowSaveChange")] + public Task ExceptionOnUowSaveChangeAsync() + { + return Task.FromResult("OK"); + } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestController_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestController_Tests.cs index 8190547bb1..2f10f4b027 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestController_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestController_Tests.cs @@ -3,12 +3,14 @@ using System.Net; using System.Security.Claims; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; using NSubstitute; using Shouldly; using Volo.Abp.ExceptionHandling; using Volo.Abp.Http; using Volo.Abp.Security.Claims; +using Volo.Abp.Uow; using Xunit; namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling; @@ -31,6 +33,8 @@ public class ExceptionTestController_Tests : AspNetCoreMvcTestBase _fakeExceptionSubscriber = Substitute.For(); services.AddSingleton(_fakeExceptionSubscriber); + + services.Replace(ServiceDescriptor.Transient()); } [Fact] @@ -93,4 +97,24 @@ public class ExceptionTestController_Tests : AspNetCoreMvcTestBase .HandleAsync(Arg.Any()); #pragma warning restore 4014 } + + [Fact] + public async Task Should_Handle_Exception_On_Uow_SaveChangeAsync() + { + FakeRequiredService.Claims.AddRange(new[] + { + new Claim(AbpClaimTypes.UserId, Guid.Empty.ToString()) + }); + + var result = await GetResponseAsObjectAsync("/api/exception-test/ExceptionOnUowSaveChange", HttpStatusCode.Conflict); + result.Error.ShouldNotBeNull(); + result.Error.Message.ShouldBe("The data you have submitted has already changed by another user/client. Please discard the changes you've done and try from the beginning."); + + #pragma warning disable 4014 + _fakeExceptionSubscriber + .Received() + .HandleAsync(Arg.Any()); + #pragma warning restore 4014 + + } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage.cshtml.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage.cshtml.cs index 087b75c3e0..a4da13dc6b 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage.cshtml.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage.cshtml.cs @@ -36,4 +36,9 @@ public class ExceptionTestPage : AbpPageModel { throw new AbpAuthorizationException("This is a sample exception!"); } + + public Task OnGetExceptionOnUowSaveChangeAsync() + { + return Task.FromResult(new JsonResult("OK")); + } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage_Tests.cs index 316339239d..3290b37c95 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage_Tests.cs @@ -3,12 +3,14 @@ using System.Net; using System.Security.Claims; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; using NSubstitute; using Shouldly; using Volo.Abp.ExceptionHandling; using Volo.Abp.Http; using Volo.Abp.Security.Claims; +using Volo.Abp.Uow; using Xunit; namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling; @@ -31,6 +33,8 @@ public class ExceptionTestPage_Tests : AspNetCoreMvcTestBase _fakeExceptionSubscriber = Substitute.For(); services.AddSingleton(_fakeExceptionSubscriber); + + services.Replace(ServiceDescriptor.Transient()); } [Fact] @@ -140,4 +144,25 @@ public class ExceptionTestPage_Tests : AspNetCoreMvcTestBase .HandleAsync(Arg.Any()); #pragma warning restore 4014 } + + + [Fact] + public async Task Should_Handle_Exception_On_Uow_SaveChangeAsync() + { + _fakeRequiredService.Claims.AddRange(new[] + { + new Claim(AbpClaimTypes.UserId, Guid.Empty.ToString()) + }); + + var result = await GetResponseAsObjectAsync("/api/exception-test/ExceptionOnUowSaveChange", HttpStatusCode.Conflict); + result.Error.ShouldNotBeNull(); + result.Error.Message.ShouldBe("The data you have submitted has already changed by another user/client. Please discard the changes you've done and try from the beginning."); + +#pragma warning disable 4014 + _fakeExceptionSubscriber + .Received() + .HandleAsync(Arg.Any()); +#pragma warning restore 4014 + + } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs index 5f525c93df..068accad6a 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs @@ -2,15 +2,23 @@ using System.Net; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Hosting; using Shouldly; using Volo.Abp.Http; using Volo.Abp.Json; +using Volo.Abp.Uow; using Xunit; namespace Volo.Abp.AspNetCore.Mvc.Uow; public class UnitOfWorkMiddleware_Exception_Rollback_Tests : AspNetCoreMvcTestBase { + protected override void ConfigureServices(HostBuilderContext context, IServiceCollection services) + { + services.Replace(ServiceDescriptor.Transient()); + } + [Fact] public async Task Should_Rollback_Transaction_For_Handled_Exceptions() { diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkPageFilter_Exception_Rollback_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkPageFilter_Exception_Rollback_Tests.cs index 4d51451fbc..f5e3bf2123 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkPageFilter_Exception_Rollback_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkPageFilter_Exception_Rollback_Tests.cs @@ -2,15 +2,23 @@ using System.Net; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Hosting; using Shouldly; using Volo.Abp.Http; using Volo.Abp.Json; +using Volo.Abp.Uow; using Xunit; namespace Volo.Abp.AspNetCore.Mvc.Uow; public class UnitOfWorkPageFilter_Exception_Rollback_Tests : AspNetCoreMvcTestBase { + protected override void ConfigureServices(HostBuilderContext context, IServiceCollection services) + { + services.Replace(ServiceDescriptor.Transient()); + } + [Fact] public async Task Should_Rollback_Transaction_For_Handled_Exceptions() {