From 6a3f864b73da65614b582228a80c60eecb1bd7b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Halil=20=C4=B0brahim=20Kalkan?= Date: Tue, 26 Sep 2017 17:39:09 +0300 Subject: [PATCH] Improved unit of work. --- .../ExceptionHandling/AbpExceptionFilter.cs | 2 +- .../Mvc/Uow/AbpUnitOfWorkMiddleware.cs | 14 ++++- .../AspNetCore/Mvc/Uow/AbpUowActionFilter.cs | 36 ++++++++--- src/Volo.Abp/Volo/Abp/Uow/UnitOfWork.cs | 20 +++---- .../Volo/Abp/Uow/UnitOfWorkAttribute.cs | 29 ++++++++- .../Volo/Abp/Uow/UnitOfWorkInterceptor.cs | 9 ++- .../App/UnitOfWorkTestController.cs | 30 ---------- .../ExceptionTestController.cs | 3 +- .../Abp/AspNetCore/Mvc/Uow/TestUnitOfWork.cs | 42 +++++++++++++ .../Mvc/Uow/TestUnitOfWorkConfig.cs | 11 ++++ ...WorkMiddleware_Exception_Rollback_Tests.cs | 27 +++++++++ .../Mvc/Uow/UnitOfWorkTestController.cs | 59 +++++++++++++++++++ .../Validation}/ValidationTestController.cs | 3 +- .../MemoryDb/TestAppMemoryDbContext.cs | 3 +- 14 files changed, 228 insertions(+), 60 deletions(-) delete mode 100644 test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/UnitOfWorkTestController.cs rename test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/{App => Mvc/ExceptionHandling}/ExceptionTestController.cs (89%) create mode 100644 test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWork.cs create mode 100644 test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWorkConfig.cs create mode 100644 test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs create mode 100644 test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs rename test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/{App => Mvc/Validation}/ValidationTestController.cs (93%) diff --git a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionFilter.cs b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionFilter.cs index 9498365932..99e449442f 100644 --- a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionFilter.cs +++ b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionFilter.cs @@ -62,7 +62,7 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling context.Exception = null; //Handled! } - private int GetStatusCode(ExceptionContext context) + private static int GetStatusCode(ExceptionContext context) { if (context.Exception is AbpAuthorizationException) { diff --git a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUnitOfWorkMiddleware.cs b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUnitOfWorkMiddleware.cs index dffa833f2b..df1c09f490 100644 --- a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUnitOfWorkMiddleware.cs +++ b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUnitOfWorkMiddleware.cs @@ -1,4 +1,5 @@ -using System.Threading.Tasks; +using System; +using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Volo.Abp.Uow; @@ -18,7 +19,16 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow using (var uow = unitOfWorkManager.Reserve(AbpUowActionFilter.UnitOfWorkReservationName)) { await _next(httpContext); - await uow.CompleteAsync(httpContext.RequestAborted); + + try + { + await uow.CompleteAsync(httpContext.RequestAborted); + } + catch (Exception e) + { + + throw; + } } } } diff --git a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs index 7a98a7729e..b57cf58c2a 100644 --- a/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs +++ b/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowActionFilter.cs @@ -42,31 +42,53 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow if (_unitOfWorkManager.TryBeginReserved(UnitOfWorkReservationName, options)) { - await next(); + var result = await next(); + if (!Succeed(result)) + { + await RollbackAsync(context); + } + return; } using (var uow = _unitOfWorkManager.Begin(options)) { var result = await next(); - if (result.Exception == null || result.ExceptionHandled) + if (Succeed(result)) { await uow.CompleteAsync(context.HttpContext.RequestAborted); } } } - private UnitOfWorkOptions CreateOptions(ActionExecutingContext context, UnitOfWorkAttribute unitOfWorkAttr) + private UnitOfWorkOptions CreateOptions(ActionExecutingContext context, UnitOfWorkAttribute unitOfWorkAttribute) { var options = new UnitOfWorkOptions(); - unitOfWorkAttr?.SetOptions(options); + unitOfWorkAttribute?.SetOptions(options); - options.IsTransactional = _defaultOptions.CalculateIsTransactional( - autoValue: !string.Equals(context.HttpContext.Request.Method, HttpMethod.Get.Method, StringComparison.OrdinalIgnoreCase) - ); + if (unitOfWorkAttribute?.IsTransactional == null) + { + options.IsTransactional = _defaultOptions.CalculateIsTransactional( + autoValue: !string.Equals(context.HttpContext.Request.Method, HttpMethod.Get.Method, StringComparison.OrdinalIgnoreCase) + ); + } return options; } + + private async Task RollbackAsync(ActionExecutingContext context) + { + var currentUow = _unitOfWorkManager.Current; + if (currentUow != null) + { + await currentUow.RollbackAsync(context.HttpContext.RequestAborted); + } + } + + private static bool Succeed(ActionExecutedContext result) + { + return result.Exception == null || result.ExceptionHandled; + } } } diff --git a/src/Volo.Abp/Volo/Abp/Uow/UnitOfWork.cs b/src/Volo.Abp/Volo/Abp/Uow/UnitOfWork.cs index aac08f3fa2..4a329d7d3a 100644 --- a/src/Volo.Abp/Volo/Abp/Uow/UnitOfWork.cs +++ b/src/Volo.Abp/Volo/Abp/Uow/UnitOfWork.cs @@ -43,7 +43,7 @@ namespace Volo.Abp.Uow _transactionApis = new Dictionary(); } - public void Initialize(UnitOfWorkOptions options) + public virtual void Initialize(UnitOfWorkOptions options) { Check.NotNull(options, nameof(options)); @@ -56,7 +56,7 @@ namespace Volo.Abp.Uow IsReserved = false; } - public void Reserve(string reservationName) + public virtual void Reserve(string reservationName) { Check.NotNull(reservationName, nameof(reservationName)); @@ -64,12 +64,12 @@ namespace Volo.Abp.Uow IsReserved = true; } - public void SetOuter(IUnitOfWork outer) + public virtual void SetOuter(IUnitOfWork outer) { Outer = outer; } - public void SaveChanges() + public virtual void SaveChanges() { foreach (var databaseApi in _databaseApis.Values) { @@ -77,7 +77,7 @@ namespace Volo.Abp.Uow } } - public async Task SaveChangesAsync(CancellationToken cancellationToken = default(CancellationToken)) + public virtual async Task SaveChangesAsync(CancellationToken cancellationToken = default(CancellationToken)) { foreach (var databaseApi in _databaseApis.Values) { @@ -88,7 +88,7 @@ namespace Volo.Abp.Uow } } - public void Complete() + public virtual void Complete() { if (_isRolledback) { @@ -110,7 +110,7 @@ namespace Volo.Abp.Uow } } - public async Task CompleteAsync(CancellationToken cancellationToken = default(CancellationToken)) + public virtual async Task CompleteAsync(CancellationToken cancellationToken = default(CancellationToken)) { if (_isRolledback) { @@ -132,7 +132,7 @@ namespace Volo.Abp.Uow } } - public void Rollback() + public virtual void Rollback() { if (_isRolledback) { @@ -152,7 +152,7 @@ namespace Volo.Abp.Uow } } - public async Task RollbackAsync(CancellationToken cancellationToken = default(CancellationToken)) + public virtual async Task RollbackAsync(CancellationToken cancellationToken = default(CancellationToken)) { if (_isRolledback) { @@ -247,7 +247,7 @@ namespace Volo.Abp.Uow Disposed.InvokeSafely(this, new UnitOfWorkEventArgs(this)); } - public void Dispose() + public virtual void Dispose() { if (_isDisposed) { diff --git a/src/Volo.Abp/Volo/Abp/Uow/UnitOfWorkAttribute.cs b/src/Volo.Abp/Volo/Abp/Uow/UnitOfWorkAttribute.cs index 351eefd859..6bf6d992a1 100644 --- a/src/Volo.Abp/Volo/Abp/Uow/UnitOfWorkAttribute.cs +++ b/src/Volo.Abp/Volo/Abp/Uow/UnitOfWorkAttribute.cs @@ -1,5 +1,7 @@ using System; using System.Data; +using System.Threading; +using JetBrains.Annotations; namespace Volo.Abp.Uow { @@ -37,7 +39,32 @@ namespace Volo.Abp.Uow /// public bool IsDisabled { get; set; } - public virtual void SetOptions(UnitOfWorkOptions options) + public UnitOfWorkAttribute() + { + + } + + public UnitOfWorkAttribute(bool isTransactional) + { + IsTransactional = isTransactional; + } + + public UnitOfWorkAttribute(bool isTransactional, IsolationLevel isolationLevel) + { + IsTransactional = isTransactional; + IsolationLevel = isolationLevel; + } + + public UnitOfWorkAttribute(bool isTransactional, IsolationLevel isolationLevel, TimeSpan timeout) + { + IsTransactional = isTransactional; + IsolationLevel = isolationLevel; + Timeout = timeout; + } + + //TODO: More constructors! + + internal virtual void SetOptions(UnitOfWorkOptions options) { if (IsTransactional.HasValue) { diff --git a/src/Volo.Abp/Volo/Abp/Uow/UnitOfWorkInterceptor.cs b/src/Volo.Abp/Volo/Abp/Uow/UnitOfWorkInterceptor.cs index b95173edde..383d31916a 100644 --- a/src/Volo.Abp/Volo/Abp/Uow/UnitOfWorkInterceptor.cs +++ b/src/Volo.Abp/Volo/Abp/Uow/UnitOfWorkInterceptor.cs @@ -55,9 +55,12 @@ namespace Volo.Abp.Uow unitOfWorkAttribute?.SetOptions(options); - options.IsTransactional = _defaultOptions.CalculateIsTransactional( - autoValue: !invocation.Method.Name.StartsWith("Get", StringComparison.InvariantCultureIgnoreCase) - ); + if (unitOfWorkAttribute?.IsTransactional == null) + { + options.IsTransactional = _defaultOptions.CalculateIsTransactional( + autoValue: !invocation.Method.Name.StartsWith("Get", StringComparison.InvariantCultureIgnoreCase) + ); + } return options; } diff --git a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/UnitOfWorkTestController.cs b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/UnitOfWorkTestController.cs deleted file mode 100644 index 4a465e16db..0000000000 --- a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/UnitOfWorkTestController.cs +++ /dev/null @@ -1,30 +0,0 @@ -using Microsoft.AspNetCore.Mvc; -using Shouldly; -using Volo.Abp.AspNetCore.Mvc; - -namespace Volo.Abp.AspNetCore.App -{ - [Route("api/unitofwork-test")] - public class UnitOfWorkTestController : AbpController - { - [HttpGet] - [Route("ActionRequiresUow")] - public ActionResult ActionRequiresUow() - { - CurrentUnitOfWork.ShouldNotBeNull(); - CurrentUnitOfWork.Options.IsTransactional.ShouldBeFalse(); - - return Content("OK"); - } - - [HttpPost] - [Route("ActionRequiresUowPost")] - public ActionResult ActionRequiresUowPost() - { - CurrentUnitOfWork.ShouldNotBeNull(); - CurrentUnitOfWork.Options.IsTransactional.ShouldBeTrue(); - - return Content("OK"); - } - } -} diff --git a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/ExceptionTestController.cs b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestController.cs similarity index 89% rename from test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/ExceptionTestController.cs rename to test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestController.cs index dfe2f939d5..d9b1b5d314 100644 --- a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/ExceptionTestController.cs +++ b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestController.cs @@ -1,8 +1,7 @@ using Microsoft.AspNetCore.Mvc; -using Volo.Abp.AspNetCore.Mvc; using Volo.Abp.Ui; -namespace Volo.Abp.AspNetCore.App +namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling { [Route("api/exception-test")] public class ExceptionTestController : AbpController diff --git a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWork.cs b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWork.cs new file mode 100644 index 0000000000..ed5f2d59c8 --- /dev/null +++ b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWork.cs @@ -0,0 +1,42 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Options; +using Volo.Abp.DependencyInjection; +using Volo.Abp.Ui; +using Volo.Abp.Uow; + +namespace Volo.Abp.AspNetCore.Mvc.Uow +{ + [Dependency(ReplaceServices = true)] + public class TestUnitOfWork : UnitOfWork + { + private readonly TestUnitOfWorkConfig _config; + + public TestUnitOfWork(IServiceProvider serviceProvider, IOptions options, TestUnitOfWorkConfig config) + : base(serviceProvider, options) + { + _config = config; + } + + public override void Complete() + { + ThrowExceptionIfRequested(); + base.Complete(); + } + + public override Task CompleteAsync(CancellationToken cancellationToken = default(CancellationToken)) + { + ThrowExceptionIfRequested(); + return base.CompleteAsync(cancellationToken); + } + + private void ThrowExceptionIfRequested() + { + if (_config.ThrowExceptionOnComplete) + { + throw new UserFriendlyException(TestUnitOfWorkConfig.ExceptionOnCompleteMessage); + } + } + } +} diff --git a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWorkConfig.cs b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWorkConfig.cs new file mode 100644 index 0000000000..d9a5835675 --- /dev/null +++ b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/TestUnitOfWorkConfig.cs @@ -0,0 +1,11 @@ +using Volo.Abp.DependencyInjection; + +namespace Volo.Abp.AspNetCore.Mvc.Uow +{ + public class TestUnitOfWorkConfig : ISingletonDependency + { + public const string ExceptionOnCompleteMessage = "TestUnitOfWork configured for exception"; + + public bool ThrowExceptionOnComplete { get; set; } + } +} \ No newline at end of file diff --git a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs new file mode 100644 index 0000000000..1616efd77c --- /dev/null +++ b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkMiddleware_Exception_Rollback_Tests.cs @@ -0,0 +1,27 @@ +using System.Net; +using System.Threading.Tasks; +using Shouldly; +using Volo.Abp.Http; +using Xunit; + +namespace Volo.Abp.AspNetCore.Mvc.Uow +{ + public class UnitOfWorkMiddleware_Exception_Rollback_Tests : AspNetCoreMvcTestBase + { + [Fact] + public async Task Should_Rollback_Transaction_For_Handled_Exceptions() + { + var result = await GetResponseAsObjectAsync("/api/unitofwork-test/HandledException", HttpStatusCode.InternalServerError); + result.Error.ShouldNotBeNull(); + result.Error.Message.ShouldBe("This is a sample exception!"); + } + + [Fact] + public async Task Should_Gracefully_Handle_Exceptions_On_Complete() + { + var result = await GetResponseAsObjectAsync("/api/unitofwork-test/ExceptionOnComplete", HttpStatusCode.InternalServerError); + result.Error.ShouldNotBeNull(); + result.Error.Message.ShouldBe(TestUnitOfWorkConfig.ExceptionOnCompleteMessage); + } + } +} \ No newline at end of file diff --git a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs new file mode 100644 index 0000000000..e7c4da74a5 --- /dev/null +++ b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestController.cs @@ -0,0 +1,59 @@ +using Microsoft.AspNetCore.Mvc; +using Shouldly; +using Volo.Abp.Ui; +using Volo.Abp.Uow; + +namespace Volo.Abp.AspNetCore.Mvc.Uow +{ + [Route("api/unitofwork-test")] + public class UnitOfWorkTestController : AbpController + { + private readonly TestUnitOfWorkConfig _testUnitOfWorkConfig; + + public UnitOfWorkTestController(TestUnitOfWorkConfig testUnitOfWorkConfig) + { + _testUnitOfWorkConfig = testUnitOfWorkConfig; + } + + [HttpGet] + [Route("ActionRequiresUow")] + public ActionResult ActionRequiresUow() + { + CurrentUnitOfWork.ShouldNotBeNull(); + CurrentUnitOfWork.Options.IsTransactional.ShouldBeFalse(); + + return Content("OK"); + } + + [HttpPost] + [Route("ActionRequiresUowPost")] + public ActionResult ActionRequiresUowPost() + { + CurrentUnitOfWork.ShouldNotBeNull(); + CurrentUnitOfWork.Options.IsTransactional.ShouldBeTrue(); + + return Content("OK"); + } + + [HttpGet] + [Route("HandledException")] + [UnitOfWork(isTransactional: true)] + public void HandledException() + { + CurrentUnitOfWork.ShouldNotBeNull(); + CurrentUnitOfWork.Options.IsTransactional.ShouldBeTrue(); + + throw new UserFriendlyException("This is a sample exception!"); + } + + [HttpGet] + [Route("ExceptionOnComplete")] + public void ExceptionOnComplete() + { + CurrentUnitOfWork.ShouldNotBeNull(); + CurrentUnitOfWork.Options.IsTransactional.ShouldBeFalse(); + + _testUnitOfWorkConfig.ThrowExceptionOnComplete = true; + } + } +} diff --git a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/ValidationTestController.cs b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Validation/ValidationTestController.cs similarity index 93% rename from test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/ValidationTestController.cs rename to test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Validation/ValidationTestController.cs index 39107af34f..b9d01a86c4 100644 --- a/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/App/ValidationTestController.cs +++ b/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Validation/ValidationTestController.cs @@ -2,9 +2,8 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc; using Shouldly; -using Volo.Abp.AspNetCore.Mvc; -namespace Volo.Abp.AspNetCore.App +namespace Volo.Abp.AspNetCore.Mvc.Validation { [Route("api/validation-test")] public class ValidationTestController : AbpController diff --git a/test/Volo.Abp.MemoryDb.Tests/Volo/Abp/TestApp/MemoryDb/TestAppMemoryDbContext.cs b/test/Volo.Abp.MemoryDb.Tests/Volo/Abp/TestApp/MemoryDb/TestAppMemoryDbContext.cs index 74c08bcb5c..902c013ed8 100644 --- a/test/Volo.Abp.MemoryDb.Tests/Volo/Abp/TestApp/MemoryDb/TestAppMemoryDbContext.cs +++ b/test/Volo.Abp.MemoryDb.Tests/Volo/Abp/TestApp/MemoryDb/TestAppMemoryDbContext.cs @@ -7,8 +7,7 @@ namespace Volo.Abp.TestApp.MemoryDb { public class TestAppMemoryDbContext : MemoryDbContext { - private static readonly Type[] EntityTypeList = new Type[] - { + private static readonly Type[] EntityTypeList = { typeof(Person) };