From 822d84a2b4216cf73d3a8fe19f544b46c0255731 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 22 May 2014 10:34:16 -0700 Subject: [PATCH] Use default values when binding action arguments Fixes #545 --- .../ReflectedActionInvoker.cs | 8 +- .../ReflectedActionInvokerTest.cs | 229 ++++++++++++++++-- test/WebSites/BasicWebSite/BasicWebSite.kproj | 1 + 3 files changed, 212 insertions(+), 26 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs index 085fe05499..70aee6e372 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs @@ -261,7 +261,7 @@ namespace Microsoft.AspNet.Mvc await InvokeActionMethodFilter(); } - private async Task> GetActionArguments(ModelStateDictionary modelState) + internal async Task> GetActionArguments(ModelStateDictionary modelState) { var actionBindingContext = await _bindingProvider.GetActionBindingContextAsync(_actionContext); var parameters = _descriptor.Parameters; @@ -304,8 +304,10 @@ namespace Microsoft.AspNet.Mvc HttpContext = actionBindingContext.ActionContext.HttpContext, FallbackToEmptyPrefix = true }; - await actionBindingContext.ModelBinder.BindModelAsync(modelBindingContext); - parameterValues[parameter.Name] = modelBindingContext.Model; + if (await actionBindingContext.ModelBinder.BindModelAsync(modelBindingContext)) + { + parameterValues[parameter.Name] = modelBindingContext.Model; + } } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionInvokerTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionInvokerTest.cs index 9f77e9569e..97015f36ac 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionInvokerTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionInvokerTest.cs @@ -5,8 +5,10 @@ using System; using System.Collections.Generic; using System.IO; using System.Linq; +using System.Reflection; using System.Threading.Tasks; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.AspNet.Routing; using Microsoft.AspNet.Testing; using Microsoft.Framework.DependencyInjection; @@ -367,11 +369,11 @@ namespace Microsoft.AspNet.Mvc var actionFilter = new Mock(MockBehavior.Strict); var resultFilter = new Mock(MockBehavior.Strict); - var invoker = CreateInvoker(new IFilter[] - { - exceptionFilter.Object, - authorizationFilter1.Object, - authorizationFilter2.Object, + var invoker = CreateInvoker(new IFilter[] + { + exceptionFilter.Object, + authorizationFilter1.Object, + authorizationFilter2.Object, actionFilter.Object, resultFilter.Object, }); @@ -653,7 +655,7 @@ namespace Microsoft.AspNet.Mvc // Act & Assert await ExceptionAssert.ThrowsAsync( - async () => await invoker.InvokeActionAsync(), + async () => await invoker.InvokeActionAsync(), message); } @@ -667,7 +669,7 @@ namespace Microsoft.AspNet.Mvc filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); filter .Setup(f => f.OnActionExecuted(It.IsAny())) - .Callback(c => + .Callback(c => { context = c; @@ -743,7 +745,7 @@ namespace Microsoft.AspNet.Mvc var filter1 = new Mock(MockBehavior.Strict); filter1 .Setup(f => f.OnActionExecutionAsync(It.IsAny(), It.IsAny())) - .Returns(async (c, next) => + .Returns(async (c, next) => { context = await next(); @@ -767,7 +769,7 @@ namespace Microsoft.AspNet.Mvc // Assert filter1.Verify( - f => f.OnActionExecutionAsync(It.IsAny(), It.IsAny()), + f => f.OnActionExecutionAsync(It.IsAny(), It.IsAny()), Times.Once()); filter2.Verify(f => f.OnActionExecuting(It.IsAny()), Times.Once()); @@ -857,7 +859,7 @@ namespace Microsoft.AspNet.Mvc f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny()), Times.Once()); } - + [Fact] public async Task InvokeAction_InvokesResultFilter_ShortCircuit() { @@ -870,7 +872,7 @@ namespace Microsoft.AspNet.Mvc .Setup(f => f.OnResultExecuted(It.IsAny())) .Callback(c => context = c) .Verifiable(); - + var filter2 = new Mock(MockBehavior.Strict); filter2 .Setup(f => f.OnResultExecuting(It.IsAny())) @@ -909,11 +911,11 @@ namespace Microsoft.AspNet.Mvc var filter2 = new Mock(MockBehavior.Strict); filter2 .Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())) - .Returns((c, next) => - { + .Returns((c, next) => + { // Not calling next here - c.Cancel = true; - return Task.FromResult(null); + c.Cancel = true; + return Task.FromResult(null); }) .Verifiable(); @@ -993,7 +995,7 @@ namespace Microsoft.AspNet.Mvc }) .Verifiable(); - var message = + var message = "If an IAsyncResultFilter cancels execution by setting the Cancel property of " + "ResultExecutingContext to 'true', then it cannot call the next filter by invoking " + "ResultExecutionDelegate."; @@ -1033,7 +1035,7 @@ namespace Microsoft.AspNet.Mvc // Assert result.Verify(r => r.ExecuteResultAsync(It.IsAny()), Times.Once()); - + filter.Verify(f => f.OnResultExecuting(It.IsAny()), Times.Once()); filter.Verify(f => f.OnResultExecuted(It.IsAny()), Times.Once()); } @@ -1059,8 +1061,8 @@ namespace Microsoft.AspNet.Mvc filter .Setup(f => f.OnResultExecuted(It.IsAny())) - .Callback(c => - { + .Callback(c => + { context = c; // Handle the exception @@ -1078,7 +1080,7 @@ namespace Microsoft.AspNet.Mvc Assert.Equal(exception, context.Exception); result.Verify(r => r.ExecuteResultAsync(It.IsAny()), Times.Once()); - + filter.Verify(f => f.OnResultExecuting(It.IsAny()), Times.Once()); filter.Verify(f => f.OnResultExecuted(It.IsAny()), Times.Once()); } @@ -1099,7 +1101,7 @@ namespace Microsoft.AspNet.Mvc var filter = new Mock(MockBehavior.Strict); filter .Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())) - .Returns(async (c, next) => + .Returns(async (c, next) => { c.Result = result.Object; @@ -1122,7 +1124,7 @@ namespace Microsoft.AspNet.Mvc result.Verify(r => r.ExecuteResultAsync(It.IsAny()), Times.Once()); filter.Verify( - f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny()), + f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny()), Times.Once()); } @@ -1206,7 +1208,7 @@ namespace Microsoft.AspNet.Mvc Assert.Equal(exception, context.Exception); resultFilter1.Verify( - f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny()), + f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny()), Times.Once()); resultFilter2.Verify(f => f.OnResultExecuting(It.IsAny()), Times.Once()); @@ -1319,6 +1321,168 @@ namespace Microsoft.AspNet.Mvc return invoker; } + [Fact] + public async Task GetActionArguments_DoesNotAddActionArgumentsToModelStateDictionary_IfBinderReturnsFalse() + { + // Arrange + Func method = x => 1; + var actionDescriptor = new ReflectedActionDescriptor + { + MethodInfo = method.Method, + Parameters = new List + { + new ParameterDescriptor + { + Name = "foo", + ParameterBindingInfo = new ParameterBindingInfo("foo", typeof(object)) + } + } + }; + var binder = new Mock(); + binder.Setup(b => b.BindModelAsync(It.IsAny())) + .Returns(Task.FromResult(result: false)); + var actionContext = new ActionContext(new RouteContext(Mock.Of()), + actionDescriptor); + var bindingContext = new ActionBindingContext(actionContext, + Mock.Of(), + binder.Object, + Mock.Of(), + Mock.Of(), + Enumerable.Empty()); + + var actionBindingContextProvider = new Mock(); + actionBindingContextProvider.Setup(p => p.GetActionBindingContextAsync(It.IsAny())) + .Returns(Task.FromResult(bindingContext)); + + var invoker = new ReflectedActionInvoker(actionContext, + actionDescriptor, + Mock.Of(), + actionBindingContextProvider.Object, + Mock.Of>()); + + var modelStateDictionary = new ModelStateDictionary(); + + // Act + var result = await invoker.GetActionArguments(modelStateDictionary); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task GetActionArguments_AddsActionArgumentsToModelStateDictionary_IfBinderReturnsTrue() + { + // Arrange + Func method = x => 1; + var actionDescriptor = new ReflectedActionDescriptor + { + MethodInfo = method.Method, + Parameters = new List + { + new ParameterDescriptor + { + Name = "foo", + ParameterBindingInfo = new ParameterBindingInfo("foo", typeof(object)) + } + } + }; + var value = "Hello world"; + var binder = new Mock(); + var metadataProvider = new EmptyModelMetadataProvider(); + binder.Setup(b => b.BindModelAsync(It.IsAny())) + .Callback((ModelBindingContext context) => + { + context.ModelMetadata = metadataProvider.GetMetadataForType(modelAccessor: null, + modelType: typeof(string)); + context.Model = value; + }) + .Returns(Task.FromResult(result: true)); + var actionContext = new ActionContext(new RouteContext(Mock.Of()), + actionDescriptor); + var bindingContext = new ActionBindingContext(actionContext, + Mock.Of(), + binder.Object, + Mock.Of(), + Mock.Of(), + Enumerable.Empty()); + + var actionBindingContextProvider = new Mock(); + actionBindingContextProvider.Setup(p => p.GetActionBindingContextAsync(It.IsAny())) + .Returns(Task.FromResult(bindingContext)); + + var invoker = new ReflectedActionInvoker(actionContext, + actionDescriptor, + Mock.Of(), + actionBindingContextProvider.Object, + Mock.Of>()); + + var modelStateDictionary = new ModelStateDictionary(); + + // Act + var result = await invoker.GetActionArguments(modelStateDictionary); + + // Assert + Assert.Equal(1, result.Count); + Assert.Equal(value, result["foo"]); + } + + [Fact] + public async Task Invoke_UsesDefaultValuesIfNotBound() + { + // Arrange + var actionDescriptor = new ReflectedActionDescriptor + { + MethodInfo = typeof(TestController).GetTypeInfo() + .DeclaredMethods + .First(m => m.Name.Equals("ActionMethodWithDefaultValues", StringComparison.Ordinal)), + Parameters = new List + { + new ParameterDescriptor + { + Name = "value", + ParameterBindingInfo = new ParameterBindingInfo("value", typeof(int)) + } + }, + FilterDescriptors = new List() + }; + + var binder = new Mock(); + var metadataProvider = new EmptyModelMetadataProvider(); + binder.Setup(b => b.BindModelAsync(It.IsAny())) + .Returns(Task.FromResult(result: false)); + var context = new Mock(); + context.SetupGet(c => c.Items) + .Returns(new Dictionary()); + var routeContext = new RouteContext(context.Object); + var actionContext = new ActionContext(routeContext, + actionDescriptor); + var bindingContext = new ActionBindingContext(actionContext, + Mock.Of(), + binder.Object, + Mock.Of(), + Mock.Of(), + Enumerable.Empty()); + + var actionBindingContextProvider = new Mock(); + actionBindingContextProvider.Setup(p => p.GetActionBindingContextAsync(It.IsAny())) + .Returns(Task.FromResult(bindingContext)); + var controllerFactory = new Mock(); + controllerFactory.Setup(c => c.CreateController(It.IsAny())) + .Returns(new TestController()); + + var invoker = new ReflectedActionInvoker(actionContext, + actionDescriptor, + controllerFactory.Object, + actionBindingContextProvider.Object, + Mock.Of>()); + + // Act + await invoker.InvokeActionAsync(); + + // Assert + Assert.Equal(5, context.Object.Items["Result"]); + } + public JsonResult ActionMethod() { return _result; @@ -1328,5 +1492,24 @@ namespace Microsoft.AspNet.Mvc { throw _actionException; } + + private sealed class TestController + { + public IActionResult ActionMethodWithDefaultValues(int value = 5) + { + return new TestActionResult { Value = value }; + } + } + + private sealed class TestActionResult : IActionResult + { + public int Value { get; set; } + + public Task ExecuteResultAsync(ActionContext context) + { + context.HttpContext.Items["Result"] = Value; + return Task.FromResult(0); + } + } } } diff --git a/test/WebSites/BasicWebSite/BasicWebSite.kproj b/test/WebSites/BasicWebSite/BasicWebSite.kproj index 70716c2d7e..0ce52f888a 100644 --- a/test/WebSites/BasicWebSite/BasicWebSite.kproj +++ b/test/WebSites/BasicWebSite/BasicWebSite.kproj @@ -30,6 +30,7 @@ +