Merge pull request #3974 from abpframework/maliming/page-void-task

Razor page's exception filter ignores void and task return types.

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ActionResultHelper.cs

@@ -8,7 +8,7 @@ namespace Volo.Abp.AspNetCore.Mvc
 {
     public static class ActionResultHelper
     {
-        public static List<Type> ObjectResultTypes { get; } 
+        public static List<Type> ObjectResultTypes { get; }
 
         static ActionResultHelper()
         {
@@ -20,10 +20,15 @@ namespace Volo.Abp.AspNetCore.Mvc
             };
         }
 
-        public static bool IsObjectResult(Type returnType)
+        public static bool IsObjectResult(Type returnType, params Type[] excludeTypes)
         {
             returnType = AsyncHelper.UnwrapTask(returnType);
 
+            if (!excludeTypes.IsNullOrEmpty() && excludeTypes.Any(t => t.IsAssignableFrom(returnType)))
+            {
+                return false;
+            }
+
             if (!typeof(IActionResult).IsAssignableFrom(returnType))
             {
                 return true;
@@ -32,4 +37,4 @@ namespace Volo.Abp.AspNetCore.Mvc
             return ObjectResultTypes.Any(t => t.IsAssignableFrom(returnType));
         }
     }
-}
+}

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/AbpExceptionPageFilter.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
@@ -25,7 +25,7 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
 
         public AbpExceptionPageFilter(
             IExceptionToErrorInfoConverter errorInfoConverter,
-            IHttpExceptionStatusCodeFinder statusCodeFinder, 
+            IHttpExceptionStatusCodeFinder statusCodeFinder,
             IJsonSerializer jsonSerializer)
         {
             _errorInfoConverter = errorInfoConverter;
@@ -34,13 +34,12 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
 
             Logger = NullLogger<AbpExceptionPageFilter>.Instance;
         }
-        
 
         public Task OnPageHandlerSelectionAsync(PageHandlerSelectedContext context)
         {
             return Task.CompletedTask;
         }
-        
+
         public async Task OnPageHandlerExecutionAsync(PageHandlerExecutingContext context, PageHandlerExecutionDelegate next)
         {
             if (context.HandlerMethod == null || !ShouldHandleException(context))
@@ -54,20 +53,20 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
             {
                 return;;
             }
-            
+
             await HandleAndWrapException(pageHandlerExecutedContext);
         }
-        
+
         protected virtual bool ShouldHandleException(PageHandlerExecutingContext context)
         {
             //TODO: Create DontWrap attribute to control wrapping..?
 
             if (context.ActionDescriptor.IsPageAction() &&
-                ActionResultHelper.IsObjectResult(context.HandlerMethod.MethodInfo.ReturnType))
+                ActionResultHelper.IsObjectResult(context.HandlerMethod.MethodInfo.ReturnType, typeof(void)))
             {
                 return true;
             }
-            
+
             if (context.HttpContext.Request.CanAccept(MimeTypes.Application.Json))
             {
                 return true;
@@ -107,6 +106,5 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
 
             context.Exception = null; //Handled!
         }
-        
     }
-}
+}

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Uow/AbpUowPageFilter.cs

@@ -38,7 +38,7 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow
 
             context.HttpContext.Items["_AbpActionInfo"] = new AbpActionInfoInHttpContext
             {
-                IsObjectResult = ActionResultHelper.IsObjectResult(context.HandlerMethod.MethodInfo.ReturnType)
+                IsObjectResult = ActionResultHelper.IsObjectResult(context.HandlerMethod.MethodInfo.ReturnType, typeof(void))
             };
 
             if (unitOfWorkAttr?.IsDisabled == true)
@@ -71,7 +71,7 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow
                 }
             }
         }
-        
+
         private AbpUnitOfWorkOptions CreateOptions(PageHandlerExecutingContext context, UnitOfWorkAttribute unitOfWorkAttribute)
         {
             var options = new AbpUnitOfWorkOptions();
@@ -102,4 +102,4 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow
             return result.Exception == null || result.ExceptionHandled;
         }
     }
-}
+}

framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage.cshtml.cs

@@ -1,18 +1,35 @@
-using Microsoft.AspNetCore.Mvc;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Mvc;
 using Volo.Abp.AspNetCore.Mvc.UI.RazorPages;
 
 namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
 {
     public class ExceptionTestPage : AbpPageModel
     {
-        public void OnGetUserFriendlyException1()
+        public void OnGetUserFriendlyException_void()
         {
             throw new UserFriendlyException("This is a sample exception!");
         }
-        
-        public IActionResult OnGetUserFriendlyException2()
+
+        public Task OnGetUserFriendlyException_Task()
+        {
+            throw new UserFriendlyException("This is a sample exception!");
+        }
+
+        public IActionResult OnGetUserFriendlyException_ActionResult()
+        {
+            throw new UserFriendlyException("This is a sample exception!");
+        }
+
+        public JsonResult OnGetUserFriendlyException_JsonResult()
         {
             throw new UserFriendlyException("This is a sample exception!");
         }
+
+        public Task<JsonResult> OnGetUserFriendlyException_Task_JsonResult()
+        {
+            throw new UserFriendlyException("This is a sample exception!");
+        }
+
     }
-}
+}

framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ExceptionHandling/ExceptionTestPage_Tests.cs

@@ -1,4 +1,4 @@
-using System.Net;
+using System.Net;
 using System.Threading.Tasks;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
@@ -24,15 +24,33 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
         }
 
         [Fact]
-        public async Task Should_Return_RemoteServiceErrorResponse_For_UserFriendlyException_For_Void_Return_Value()
+        public async Task Should_Not_Handle_Exceptions_For_Void_Return_Values()
         {
-            var result = await GetResponseAsObjectAsync<RemoteServiceErrorResponse>("/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException1", HttpStatusCode.Forbidden);
-            result.Error.ShouldNotBeNull();
-            result.Error.Message.ShouldBe("This is a sample exception!");
+            await Assert.ThrowsAsync<UserFriendlyException>(
+                async () => await GetResponseAsStringAsync(
+                    "/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException_Void"
+                )
+            );
 
 #pragma warning disable 4014
             _fakeExceptionSubscriber
-                .Received()
+                .DidNotReceive()
+                .HandleAsync(Arg.Any<ExceptionNotificationContext>());
+#pragma warning restore 4014
+        }
+
+        [Fact]
+        public async Task Should_Not_Handle_Exceptions_For_Task_Return_Values()
+        {
+            await Assert.ThrowsAsync<UserFriendlyException>(
+                async () => await GetResponseAsStringAsync(
+                    "/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException_Task"
+                )
+            );
+
+#pragma warning disable 4014
+            _fakeExceptionSubscriber
+                .DidNotReceive()
                 .HandleAsync(Arg.Any<ExceptionNotificationContext>());
 #pragma warning restore 4014
         }
@@ -41,8 +59,8 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
         public async Task Should_Not_Handle_Exceptions_For_ActionResult_Return_Values()
         {
             await Assert.ThrowsAsync<UserFriendlyException>(
-                async () => await GetResponseAsObjectAsync<RemoteServiceErrorResponse>(
-                    "/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException2"
+                async () => await GetResponseAsStringAsync(
+                    "/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException_ActionResult"
                 )
             );
 
@@ -50,7 +68,35 @@ namespace Volo.Abp.AspNetCore.Mvc.ExceptionHandling
             _fakeExceptionSubscriber
                 .DidNotReceive()
                 .HandleAsync(Arg.Any<ExceptionNotificationContext>());
+#pragma warning restore 4014
+        }
+
+        [Fact]
+        public async Task Should_Return_RemoteServiceErrorResponse_For_UserFriendlyException_For_Object_Return_Value()
+        {
+            var result = await GetResponseAsObjectAsync<RemoteServiceErrorResponse>("/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException_JsonResult", HttpStatusCode.Forbidden);
+            result.Error.ShouldNotBeNull();
+            result.Error.Message.ShouldBe("This is a sample exception!");
+
+#pragma warning disable 4014
+            _fakeExceptionSubscriber
+                .Received()
+                .HandleAsync(Arg.Any<ExceptionNotificationContext>());
+#pragma warning restore 4014
+        }
+
+        [Fact]
+        public async Task Should_Return_RemoteServiceErrorResponse_For_UserFriendlyException_For_Task_Object_Return_Value()
+        {
+            var result = await GetResponseAsObjectAsync<RemoteServiceErrorResponse>("/ExceptionHandling/ExceptionTestPage?handler=UserFriendlyException_Task_JsonResult", HttpStatusCode.Forbidden);
+            result.Error.ShouldNotBeNull();
+            result.Error.Message.ShouldBe("This is a sample exception!");
+
+#pragma warning disable 4014
+            _fakeExceptionSubscriber
+                .Received()
+                .HandleAsync(Arg.Any<ExceptionNotificationContext>());
 #pragma warning restore 4014
         }
     }
-}
+}

framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Features/FeatureTestPage.cshtml.cs

@@ -14,9 +14,9 @@ namespace Volo.Abp.AspNetCore.Mvc.Features
         }
 
         [RequiresFeature("NotAllowedFeature")]
-        public void OnGetNotAllowedFeature()
+        public ObjectResult OnGetNotAllowedFeature()
         {
-            
+            return new ObjectResult(42);
         }
 
         public ObjectResult OnGetNoFeature()
@@ -24,4 +24,4 @@ namespace Volo.Abp.AspNetCore.Mvc.Features
             return new ObjectResult(42);
         }
     }
-}
+}

framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Uow/UnitOfWorkTestPage.cshtml.cs

@@ -32,23 +32,23 @@ namespace Volo.Abp.AspNetCore.Mvc.Uow
         }
 
         [UnitOfWork(isTransactional: true)]
-        public void OnGetHandledException()
+        public ObjectResult OnGetHandledException()
         {
             CurrentUnitOfWork.ShouldNotBeNull();
             CurrentUnitOfWork.Options.IsTransactional.ShouldBeTrue();
 
             throw new UserFriendlyException("This is a sample exception!");
         }
-        
+
         public ObjectResult OnGetExceptionOnComplete()
         {
             CurrentUnitOfWork.ShouldNotBeNull();
             CurrentUnitOfWork.Options.IsTransactional.ShouldBeFalse();
 
             _testUnitOfWorkConfig.ThrowExceptionOnComplete = true;
-            
+
             //Prevent rendering of pages.
             return new ObjectResult("");
         }
     }
-}
+}