From 128d74e2a0b9c396962a054abf1170a61b505dba Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 26 Apr 2016 11:17:57 -0700 Subject: [PATCH] Refactor Argument Binder This change just rearranges some code in the argument binder with a mind towards performance and clarity. We're removing a few Task's here as well in certain cases, but not yet all of them. We additionally save a dictionary in the case where you have bound properties. Hopefully these changes break the code into more discrete and sensible units without multiple levels of indirection without abstraction. - Main 'driver' code - BindModel - ActivateProperty --- .../Controllers/IControllerArgumentBinder.cs | 13 +- .../MvcCoreServiceCollectionExtensions.cs | 2 +- .../Internal/ControllerActionInvoker.cs | 13 +- .../ControllerActionInvokerProvider.cs | 4 +- .../Internal/ControllerArgumentBinder.cs | 218 ++++++++---------- .../Internal/FilterActionInvoker.cs | 12 +- .../Internal/ControllerActionInvokerTest.cs | 17 +- .../Internal/ControllerArgumentBinderTests.cs | 67 ++++-- .../ValidationIntegrationTests.cs | 6 +- 9 files changed, 178 insertions(+), 174 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Controllers/IControllerArgumentBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/Controllers/IControllerArgumentBinder.cs index 1fbb5c633a..410f325384 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Controllers/IControllerArgumentBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Controllers/IControllerArgumentBinder.cs @@ -9,15 +9,20 @@ namespace Microsoft.AspNetCore.Mvc.Controllers /// /// Provides a dictionary of action arguments. /// - public interface IControllerActionArgumentBinder + public interface IControllerArgumentBinder { /// - /// Returns a dictionary of the parameter-argument name-value pairs, + /// Asyncronously binds a dictionary of the parameter-argument name-value pairs, /// which can be used to invoke the action. Also binds properties explicitly marked properties on the /// . /// - /// The associated with the current action. + /// The associated with the current action. /// The controller object which contains the action. - Task> BindActionArgumentsAsync(ControllerContext context, object controller); + /// The arguments dictionary. + /// A which, when completed signals the completion of argument binding. + Task BindArgumentsAsync( + ControllerContext controllerContext, + object controller, + IDictionary arguments); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index 1d1a4eb3e2..d897d2953e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -172,7 +172,7 @@ namespace Microsoft.Extensions.DependencyInjection ServiceDescriptor.Transient()); // These are stateless - services.TryAddSingleton(); + services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddEnumerable( ServiceDescriptor.Singleton()); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs index 07b4d77549..316eba6dbd 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs @@ -21,14 +21,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal { private readonly ControllerActionDescriptor _descriptor; private readonly IControllerFactory _controllerFactory; - private readonly IControllerActionArgumentBinder _argumentBinder; + private readonly IControllerArgumentBinder _argumentBinder; public ControllerActionInvoker( ActionContext actionContext, ControllerActionInvokerCache controllerActionInvokerCache, IControllerFactory controllerFactory, ControllerActionDescriptor descriptor, - IControllerActionArgumentBinder argumentBinder, + IControllerArgumentBinder argumentBinder, IReadOnlyList valueProviderFactories, ILogger logger, DiagnosticSource diagnosticSource, @@ -111,9 +111,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal return actionResult; } - protected override Task> BindActionArgumentsAsync() + protected override Task BindActionArgumentsAsync(IDictionary arguments) { - return _argumentBinder.BindActionArgumentsAsync(Context, Instance); + if (arguments == null) + { + throw new ArgumentNullException(nameof(arguments)); + } + + return _argumentBinder.BindArgumentsAsync(Context, Instance, arguments); } // Marking as internal for Unit Testing purposes. diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerProvider.cs index ae102a62df..f27f5b2697 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerProvider.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal { public class ControllerActionInvokerProvider : IActionInvokerProvider { - private readonly IControllerActionArgumentBinder _argumentBinder; + private readonly IControllerArgumentBinder _argumentBinder; private readonly IControllerFactory _controllerFactory; private readonly ControllerActionInvokerCache _controllerActionInvokerCache; private readonly IReadOnlyList _valueProviderFactories; @@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal public ControllerActionInvokerProvider( IControllerFactory controllerFactory, ControllerActionInvokerCache controllerActionInvokerCache, - IControllerActionArgumentBinder argumentBinder, + IControllerArgumentBinder argumentBinder, IOptions optionsAccessor, ILoggerFactory loggerFactory, DiagnosticSource diagnosticSource) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerArgumentBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerArgumentBinder.cs index 8ccd759928..3cf2cc1f52 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerArgumentBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerArgumentBinder.cs @@ -15,10 +15,10 @@ using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Mvc.Internal { /// - /// Provides a default implementation of . + /// Provides a default implementation of . /// Uses ModelBinding to populate action parameters. /// - public class ControllerArgumentBinder : IControllerActionArgumentBinder + public class ControllerArgumentBinder : IControllerArgumentBinder { private static readonly MethodInfo CallPropertyAddRangeOpenGenericMethod = typeof(ControllerArgumentBinder).GetTypeInfo().GetDeclaredMethod( @@ -38,9 +38,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal _validator = validator; } - public Task> BindActionArgumentsAsync( + public Task BindArgumentsAsync( ControllerContext controllerContext, - object controller) + object controller, + IDictionary arguments) { if (controllerContext == null) { @@ -65,53 +66,54 @@ namespace Microsoft.AspNetCore.Mvc.Internal if (actionDescriptor.BoundProperties.Count == 0 && actionDescriptor.Parameters.Count == 0) { - return Task.FromResult>( - new Dictionary(StringComparer.Ordinal)); - } - else if (actionDescriptor.BoundProperties.Count == 0) - { - return BindActionArgumentsCoreAsync(controllerContext, actionDescriptor); - } - else - { - return BindActionArgumentsAndPropertiesCoreAsync( - controllerContext, - controller, - actionDescriptor); + return TaskCache.CompletedTask; } + + return BindArgumentsCoreAsync(controllerContext, controller, arguments); } - private async Task> BindActionArgumentsCoreAsync( - ControllerContext controllerContext, - ControllerActionDescriptor actionDescriptor) - { - var valueProvider = await CompositeValueProvider.CreateAsync(controllerContext); - - var actionArguments = await PopulateArgumentsAsync( - controllerContext, - actionDescriptor.Parameters, - valueProvider); - return actionArguments; - } - - private async Task> BindActionArgumentsAndPropertiesCoreAsync( + private async Task BindArgumentsCoreAsync( ControllerContext controllerContext, object controller, - ControllerActionDescriptor actionDescriptor) + IDictionary arguments) { var valueProvider = await CompositeValueProvider.CreateAsync(controllerContext); - var controllerProperties = await PopulateArgumentsAsync( - controllerContext, - actionDescriptor.BoundProperties, - valueProvider); - ActivateProperties(actionDescriptor, controller, controllerProperties); + var parameters = controllerContext.ActionDescriptor.Parameters; + for (var i = 0; i < parameters.Count; i++) + { + var parameter = parameters[i]; - var actionArguments = await PopulateArgumentsAsync( - controllerContext, - actionDescriptor.Parameters, - valueProvider); - return actionArguments; + var result = await BindModelAsync(parameter, controllerContext, valueProvider); + if (result != null && result.Value.IsModelSet) + { + arguments[parameter.Name] = result.Value.Model; + } + } + + var properties = controllerContext.ActionDescriptor.BoundProperties; + if (properties.Count == 0) + { + // Perf: Early exit to avoid PropertyHelper lookup in the (common) case where we have no + // bound properties. + return; + } + + var propertyHelpers = PropertyHelper.GetProperties(controller); + for (var i = 0; i < properties.Count; i++) + { + var property = properties[i]; + + var result = await BindModelAsync(property, controllerContext, valueProvider); + if (result != null && result.Value.IsModelSet) + { + var propertyHelper = FindPropertyHelper(propertyHelpers, property); + if (propertyHelper != null) + { + ActivateProperty(property, propertyHelper, controller, result.Value.Model); + } + } + } } public async Task BindModelAsync( @@ -199,6 +201,52 @@ namespace Microsoft.AspNetCore.Mvc.Internal return modelBindingResult; } + private void ActivateProperty( + ParameterDescriptor property, + PropertyHelper propertyHelper, + object controller, + object value) + { + var propertyType = propertyHelper.Property.PropertyType; + var metadata = _modelMetadataProvider.GetMetadataForType(propertyType); + + if (propertyHelper.Property.CanWrite && propertyHelper.Property.SetMethod?.IsPublic == true) + { + // Handle settable property. Do not set the property to null if the type is a non-nullable type. + if (value != null || metadata.IsReferenceOrNullableType) + { + propertyHelper.SetValue(controller, value); + } + + return; + } + + if (propertyType.IsArray) + { + // Do not attempt to copy values into an array because an array's length is immutable. This choice + // is also consistent with MutableObjectModelBinder's handling of a read-only array property. + return; + } + + var target = propertyHelper.GetValue(controller); + if (value == null || target == null) + { + // Nothing to do when source or target is null. + return; + } + + if (!metadata.IsCollectionType) + { + // Not a collection model. + return; + } + + // Handle a read-only collection property. + var propertyAddRange = CallPropertyAddRangeOpenGenericMethod.MakeGenericMethod( + metadata.ElementMetadata.ModelType); + propertyAddRange.Invoke(obj: null, parameters: new[] { target, value }); + } + // Called via reflection. private static void CallPropertyAddRange(object target, object source) { @@ -214,92 +262,18 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } - private void ActivateProperties( - ControllerActionDescriptor actionDescriptor, - object controller, - IDictionary properties) + private static PropertyHelper FindPropertyHelper(PropertyHelper[] propertyHelpers, ParameterDescriptor property) { - var propertyHelpers = PropertyHelper.GetProperties(controller); - for (var i = 0; i < actionDescriptor.BoundProperties.Count; i++) + for (var i = 0; i < propertyHelpers.Length; i++) { - var property = actionDescriptor.BoundProperties[i]; - - PropertyHelper propertyHelper = null; - for (var j = 0; j < propertyHelpers.Length; j++) + var propertyHelper = propertyHelpers[i]; + if (string.Equals(propertyHelper.Name, property.Name, StringComparison.Ordinal)) { - if (string.Equals(propertyHelpers[j].Name, property.Name, StringComparison.Ordinal)) - { - propertyHelper = propertyHelpers[j]; - break; - } - } - - object value; - if (propertyHelper == null || !properties.TryGetValue(property.Name, out value)) - { - continue; - } - - var propertyType = propertyHelper.Property.PropertyType; - var metadata = _modelMetadataProvider.GetMetadataForType(propertyType); - - if (propertyHelper.Property.CanWrite && propertyHelper.Property.SetMethod?.IsPublic == true) - { - // Handle settable property. Do not set the property to null if the type is a non-nullable type. - if (value != null || metadata.IsReferenceOrNullableType) - { - propertyHelper.SetValue(controller, value); - } - - continue; - } - - if (propertyType.IsArray) - { - // Do not attempt to copy values into an array because an array's length is immutable. This choice - // is also consistent with MutableObjectModelBinder's handling of a read-only array property. - continue; - } - - var target = propertyHelper.GetValue(controller); - if (value == null || target == null) - { - // Nothing to do when source or target is null. - continue; - } - - if (!metadata.IsCollectionType) - { - // Not a collection model. - continue; - } - - // Handle a read-only collection property. - var propertyAddRange = CallPropertyAddRangeOpenGenericMethod.MakeGenericMethod( - metadata.ElementMetadata.ModelType); - propertyAddRange.Invoke(obj: null, parameters: new[] { target, value }); - } - } - - private async Task> PopulateArgumentsAsync( - ControllerContext controllerContext, - IList parameters, - IValueProvider valueProvider) - { - var arguments = new Dictionary(StringComparer.Ordinal); - - // Perf: Avoid allocations - for (var i = 0; i < parameters.Count; i++) - { - var parameter = parameters[i]; - var modelBindingResult = await BindModelAsync(parameter, controllerContext, valueProvider); - if (modelBindingResult != null && modelBindingResult.Value.IsModelSet) - { - arguments[parameter.Name] = modelBindingResult.Value.Model; + return propertyHelper; } } - return arguments; + return null; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterActionInvoker.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterActionInvoker.cs index 6c72e4f53e..88092134bb 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterActionInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterActionInvoker.cs @@ -105,7 +105,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal protected abstract Task InvokeActionAsync(ActionExecutingContext actionExecutingContext); - protected abstract Task> BindActionArgumentsAsync(); + protected abstract Task BindActionArgumentsAsync(IDictionary arguments); public virtual async Task InvokeAsync() { @@ -456,13 +456,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal Instance = CreateInstance(); - var arguments = await BindActionArgumentsAsync(); - - _actionExecutingContext = new ActionExecutingContext( - Context, - _filters, - arguments, - Instance); + var arguments = new Dictionary(StringComparer.OrdinalIgnoreCase); + await BindActionArgumentsAsync(arguments); + _actionExecutingContext = new ActionExecutingContext(Context, _filters, arguments, Instance); await InvokeActionFilterAsync(); } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs index 6176cef786..48828ef0f1 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs @@ -2037,10 +2037,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal .Setup(fp => fp.OnProvidersExecuted(It.IsAny())) .Verifiable(); - var actionArgumentsBinder = new Mock(); - actionArgumentsBinder.Setup( - b => b.BindActionArgumentsAsync(It.IsAny(), It.IsAny())) - .Returns(Task.FromResult>(new Dictionary())); + var argumentBinder = new Mock(); + argumentBinder + .Setup(b => b.BindArgumentsAsync( + It.IsAny(), + It.IsAny(), + It.IsAny>())) + .Returns(TaskCache.CompletedTask); filterProvider .SetupGet(fp => fp.Order) @@ -2051,7 +2054,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new[] { filterProvider.Object }, new MockControllerFactory(this), actionDescriptor, - actionArgumentsBinder.Object, + argumentBinder.Object, new IValueProviderFactory[0], new NullLoggerFactory().CreateLogger(), new DiagnosticListener("Microsoft.AspNetCore"), @@ -2227,7 +2230,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal IFilterProvider[] filterProviders, MockControllerFactory controllerFactory, ControllerActionDescriptor descriptor, - IControllerActionArgumentBinder controllerActionArgumentBinder, + IControllerArgumentBinder argumentBinder, IReadOnlyList valueProviderFactories, ILogger logger, DiagnosticSource diagnosticSource, @@ -2237,7 +2240,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal CreateFilterCache(filterProviders), controllerFactory, descriptor, - controllerActionArgumentBinder, + argumentBinder, valueProviderFactories, logger, diagnosticSource, diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerArgumentBinderTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerArgumentBinderTests.cs index 2e43de0537..f0473c174a 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerArgumentBinderTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerArgumentBinderTests.cs @@ -39,12 +39,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal var argumentBinder = GetArgumentBinder(factory); var controllerContext = GetControllerContext(actionDescriptor); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, new TestController()); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert - Assert.Empty(result); + Assert.Empty(arguments); } [Fact] @@ -68,12 +70,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal var argumentBinder = GetArgumentBinder(factory); var controllerContext = GetControllerContext(actionDescriptor); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, new TestController()); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert - Assert.Empty(result); + Assert.Empty(arguments); } [Fact] @@ -106,13 +110,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal var argumentBinder = GetArgumentBinder(factory); var controllerContext = GetControllerContext(actionDescriptor); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, new TestController()); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert - Assert.Equal(1, result.Count); - Assert.Equal(value, result["foo"]); + Assert.Equal(1, arguments.Count); + Assert.Equal(value, arguments["foo"]); } [Fact] @@ -140,9 +146,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal It.IsAny())); var argumentBinder = GetArgumentBinder(factory, mockValidator.Object); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, new TestController()); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert mockValidator @@ -169,6 +177,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal }); var controllerContext = GetControllerContext(actionDescriptor); + var arguments = new Dictionary(StringComparer.Ordinal); var binder = new Mock(); binder @@ -184,10 +193,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal It.IsAny())); var factory = GetModelBinderFactory(binder.Object); + var controller = new TestController(); var argumentBinder = GetArgumentBinder(factory, mockValidator.Object); // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, new TestController()); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert mockValidator @@ -212,6 +222,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal }); var controllerContext = GetControllerContext(actionDescriptor); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); var mockValidator = new Mock(MockBehavior.Strict); mockValidator @@ -225,7 +237,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var argumentBinder = GetArgumentBinder(factory, mockValidator.Object); // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, new TestController()); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert mockValidator @@ -251,6 +263,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal }); var controllerContext = GetControllerContext(actionDescriptor); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); var binder = new Mock(); binder @@ -269,7 +283,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var argumentBinder = GetArgumentBinder(factory, mockValidator.Object); // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, new TestController()); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert mockValidator @@ -295,14 +309,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal }); var controllerContext = GetControllerContext(actionDescriptor); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); var factory = GetModelBinderFactory("Hello"); var argumentBinder = GetArgumentBinder(factory); - var controller = new TestController(); // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, controller); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert Assert.Equal("Hello", controller.StringProperty); @@ -324,15 +339,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal }); var controllerContext = GetControllerContext(actionDescriptor); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); var expected = new List { "Hello", "World", "!!" }; var factory = GetModelBinderFactory(expected); var argumentBinder = GetArgumentBinder(factory); - var controller = new TestController(); - // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, controller); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert Assert.Equal(expected, controller.CollectionProperty); @@ -356,18 +371,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal }); var controllerContext = GetControllerContext(actionDescriptor); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); var binder = new StubModelBinder(ModelBindingResult.Success(string.Empty, model: null)); var factory = GetModelBinderFactory(binder); var argumentBinder = GetArgumentBinder(factory); - var controller = new TestController(); // Some non default value. controller.NonNullableProperty = -1; // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, controller); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert Assert.Equal(-1, controller.NonNullableProperty); @@ -387,18 +403,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal }); var controllerContext = GetControllerContext(actionDescriptor); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); var binder = new StubModelBinder(ModelBindingResult.Success(key: string.Empty, model: null)); var factory = GetModelBinderFactory(binder); var argumentBinder = GetArgumentBinder(factory); - var controller = new TestController(); // Some non default value. controller.NullableProperty = -1; // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, controller); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert Assert.Null(controller.NullableProperty); @@ -463,14 +480,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal }); var controllerContext = GetControllerContext(actionDescriptor); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); var factory = GetModelBinderFactory(inputValue); var argumentBinder = GetArgumentBinder(factory); - var controller = new TestController(); // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, controller); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert Assert.Equal(expectedValue, propertyAccessor(controller)); @@ -520,6 +538,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal } var controllerContext = GetControllerContext(actionDescriptor); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); var binder = new StubModelBinder(bindingContext => { @@ -540,10 +560,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal controllerContext.ValueProviderFactories.Add(new SimpleValueProviderFactory()); var argumentBinder = GetArgumentBinder(factory); - var controller = new TestController(); // Act - var result = await argumentBinder.BindActionArgumentsAsync(controllerContext, controller); + await argumentBinder.BindArgumentsAsync(controllerContext, controller, arguments); // Assert Assert.Equal(new string[] { "goodbye" }, controller.ArrayProperty); // Skipped diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs index c63f1ade96..ea6d549273 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs @@ -99,10 +99,11 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests }, actionDescriptor: actionDescriptor); + var arguments = new Dictionary(StringComparer.Ordinal); var modelState = testContext.ModelState; // Act - var arguments = await argumentBinder.BindActionArgumentsAsync(testContext, new TestController()); + await argumentBinder.BindArgumentsAsync(testContext, new TestController(), arguments); // Assert Assert.False(modelState.IsValid); @@ -138,10 +139,11 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests }, actionDescriptor: actionDescriptor); + var arguments = new Dictionary(StringComparer.Ordinal); var modelState = testContext.ModelState; // Act - var arguments = await argumentBinder.BindActionArgumentsAsync(testContext, new TestController()); + await argumentBinder.BindArgumentsAsync(testContext, new TestController(), arguments); // Assert Assert.True(modelState.IsValid);