[Fixes #5175] Async resource filters' short circuited result getting …

…executed more than once.
aspnet · Aug 23, 2016 · db7f0bc · db7f0bc
1 parent c942eab
commit db7f0bc
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 8 deletions.
diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs
@@ -357,7 +357,7 @@ private Task Next(ref State next, ref Scope scope, ref object state, ref bool is
                             goto case State.ExceptionBegin;
                         }
                     }
-                    
+
                 case State.ResourceAsyncBegin:
                     {
                         Debug.Assert(state != null);
@@ -392,13 +392,14 @@ private Task Next(ref State next, ref Scope scope, ref object state, ref bool is
                                 Canceled = true,
                                 Result = _resourceExecutingContext.Result,
                             };
-                        }
 
-                        _diagnosticSource.AfterOnResourceExecution(_resourceExecutedContext, filter);
+                            _diagnosticSource.AfterOnResourceExecution(_resourceExecutedContext, filter);
 
-                        if (_resourceExecutingContext.Result != null)
-                        {
-                            goto case State.ResourceShortCircuit;
+                            // A filter could complete a Task without setting a result
+                            if (_resourceExecutingContext.Result != null)
+                            {
+                                goto case State.ResourceShortCircuit;
+                            }
                         }
 
                         goto case State.ResourceEnd;
@@ -1222,7 +1223,7 @@ private async Task InvokeActionMethodAsync()
                     arguments,
                     controller);
                 logger.ActionMethodExecuting(controllerContext, orderedArguments);
-                
+
                 var returnType = executor.MethodReturnType;
                 if (returnType == typeof(void))
                 {

diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs
@@ -1321,6 +1321,28 @@ public async Task InvokeAction_InvokesResourceFilter()
 
         }
 
+        [Fact]
+        public async Task InvokeAction_ShortcircuitResult_GetsExecuted_OnlyOnce()
+        {
+            // Arrange
+            var result = new ShorcircuitedActionResult();
+            var filter1 = new TestResourceFilter(TestResourceFilterAction.Passthrough);
+            var filter2 = new TestResourceFilter(TestResourceFilterAction.Passthrough);
+            var filter3 = new TestResourceFilter(TestResourceFilterAction.ShortCircuit);
+            filter3.ActionResultToUse = result;
+            var filter4 = new TestResourceFilter(TestResourceFilterAction.ThrowException); // this should not run.
+            var invoker = CreateInvoker(new[] { filter1, filter2, filter3, filter4 });
+
+            // Act
+            await invoker.InvokeAsync();
+
+            // Assert
+            Assert.Equal(1, result.Count); // the result's execute async should be called only once
+            var resourceExecutedContext = filter1.ResourceExecutedContext;
+            Assert.Same(result, resourceExecutedContext.Result);
+            Assert.True(resourceExecutedContext.Canceled);
+        }
+
         [Fact]
         public async Task InvokeAction_InvokesAsyncResourceFilter_WithActionResult_FromAction()
         {
@@ -2751,7 +2773,7 @@ public async Task Invoke_WritesDiagnostic_ActionInvoked()
             Assert.NotNull(listener.AfterAction?.ActionDescriptor);
             Assert.NotNull(listener.AfterAction?.HttpContext);
         }
-        
+
         public async Task InvokeAction_ExceptionBubbling_AsyncActionFilter_To_ResourceFilter()
         {
             // Arrange
@@ -3310,5 +3332,60 @@ public Task BindArgumentsAsync(
                 return TaskCache.CompletedTask;
             }
         }
+
+        private class ShorcircuitedActionResult : IActionResult
+        {
+            public int Count { get; private set; }
+
+            public Task ExecuteResultAsync(ActionContext context)
+            {
+                Count++;
+                return Task.FromResult(true);
+            }
+        }
+
+        private enum TestResourceFilterAction
+        {
+            ShortCircuit,
+            Passthrough,
+            ThrowException
+        }
+
+        private class TestResourceFilter : IAsyncResourceFilter
+        {
+            private readonly TestResourceFilterAction _action;
+
+            public TestResourceFilter(TestResourceFilterAction action)
+            {
+                _action = action;
+            }
+
+            public ShorcircuitedActionResult ActionResultToUse { get; set; }
+
+            public ResourceExecutedContext ResourceExecutedContext { get; private set; }
+
+            public async Task OnResourceExecutionAsync(ResourceExecutingContext context, ResourceExecutionDelegate next)
+            {
+                if (_action == TestResourceFilterAction.Passthrough)
+                {
+                    ResourceExecutedContext = await next();
+                }
+                else if (_action == TestResourceFilterAction.ThrowException)
+                {
+                    throw new InvalidOperationException($"Error from {nameof(TestResourceFilter)}");
+                }
+                else
+                {
+                    if (ActionResultToUse == null)
+                    {
+                        context.Result = new ShorcircuitedActionResult();
+                    }
+                    else
+                    {
+                        context.Result = ActionResultToUse;
+                    }
+                }
+            }
+        }
     }
 }