From 4d63ffa879370f6945d011e0d1e2a5f55bbac119 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 26 Apr 2016 09:13:35 -0700 Subject: [PATCH] Make ValueProvider creation lazy We want this change to avoid MVC eagerly reading the form. This is good for general perf and also for scenarios where you want read the body yourself (large file uploads). We DO have scenarios where you want to configure the value providers per-request or also to change the limits on the value providers (form) so it's worth keeping these around on the context. --- .../ControllerBase.cs | 26 +++++---- .../ControllerContext.cs | 14 ++--- .../Internal/ControllerActionInvoker.cs | 1 - .../Internal/ControllerArgumentBinder.cs | 55 +++++++++++++++++-- .../Internal/FilterActionInvoker.cs | 24 +++----- .../ModelBinding/CompositeValueProvider.cs | 28 ++++++++++ .../ICompositeValueProviderFactory.cs | 12 ---- .../ControllerBaseTest.cs | 3 +- .../Internal/ControllerArgumentBinderTests.cs | 4 +- .../ModelBindingTestHelper.cs | 25 +-------- .../TryUpdateModelIntegrationTest.cs | 53 +++++++++--------- .../SimpleValueProvider.cs | 0 .../SimpleValueProviderFactory.cs | 36 ++++++++++++ .../ControllerTest.cs | 3 +- 14 files changed, 177 insertions(+), 107 deletions(-) delete mode 100644 src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ICompositeValueProviderFactory.cs rename test/{Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding => Microsoft.AspNetCore.Mvc.TestCommon}/SimpleValueProvider.cs (100%) create mode 100644 test/Microsoft.AspNetCore.Mvc.TestCommon/SimpleValueProviderFactory.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs b/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs index f387c03757..670d5631f9 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs @@ -1064,7 +1064,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// A that on completion returns true if the update is successful. [NonAction] - public virtual Task TryUpdateModelAsync( + public virtual async Task TryUpdateModelAsync( TModel model, string prefix) where TModel : class @@ -1079,7 +1079,8 @@ namespace Microsoft.AspNetCore.Mvc throw new ArgumentNullException(nameof(prefix)); } - return TryUpdateModelAsync(model, prefix, new CompositeValueProvider(ControllerContext.ValueProviders)); + var valueProvider = await CompositeValueProvider.CreateAsync(ControllerContext); + return await TryUpdateModelAsync(model, prefix, valueProvider); } /// @@ -1136,7 +1137,7 @@ namespace Microsoft.AspNetCore.Mvc /// which need to be included for the current model. /// A that on completion returns true if the update is successful. [NonAction] - public Task TryUpdateModelAsync( + public async Task TryUpdateModelAsync( TModel model, string prefix, params Expression>[] includeExpressions) @@ -1152,13 +1153,14 @@ namespace Microsoft.AspNetCore.Mvc throw new ArgumentNullException(nameof(includeExpressions)); } - return ModelBindingHelper.TryUpdateModelAsync( + var valueProvider = await CompositeValueProvider.CreateAsync(ControllerContext); + return await ModelBindingHelper.TryUpdateModelAsync( model, prefix, ControllerContext, MetadataProvider, ModelBinderFactory, - new CompositeValueProvider(ControllerContext.ValueProviders), + valueProvider, ObjectValidator, includeExpressions); } @@ -1174,7 +1176,7 @@ namespace Microsoft.AspNetCore.Mvc /// A predicate which can be used to filter properties at runtime. /// A that on completion returns true if the update is successful. [NonAction] - public Task TryUpdateModelAsync( + public async Task TryUpdateModelAsync( TModel model, string prefix, Func propertyFilter) @@ -1190,13 +1192,14 @@ namespace Microsoft.AspNetCore.Mvc throw new ArgumentNullException(nameof(propertyFilter)); } - return ModelBindingHelper.TryUpdateModelAsync( + var valueProvider = await CompositeValueProvider.CreateAsync(ControllerContext); + return await ModelBindingHelper.TryUpdateModelAsync( model, prefix, ControllerContext, MetadataProvider, ModelBinderFactory, - new CompositeValueProvider(ControllerContext.ValueProviders), + valueProvider, ObjectValidator, propertyFilter); } @@ -1302,7 +1305,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// A that on completion returns true if the update is successful. [NonAction] - public virtual Task TryUpdateModelAsync( + public virtual async Task TryUpdateModelAsync( object model, Type modelType, string prefix) @@ -1317,14 +1320,15 @@ namespace Microsoft.AspNetCore.Mvc throw new ArgumentNullException(nameof(modelType)); } - return ModelBindingHelper.TryUpdateModelAsync( + var valueProvider = await CompositeValueProvider.CreateAsync(ControllerContext); + return await ModelBindingHelper.TryUpdateModelAsync( model, modelType, prefix, ControllerContext, MetadataProvider, ModelBinderFactory, - new CompositeValueProvider(ControllerContext.ValueProviders), + valueProvider, ObjectValidator); } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ControllerContext.cs b/src/Microsoft.AspNetCore.Mvc.Core/ControllerContext.cs index 690a029f67..fe9286e1a0 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ControllerContext.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ControllerContext.cs @@ -14,7 +14,7 @@ namespace Microsoft.AspNetCore.Mvc /// public class ControllerContext : ActionContext { - private IList _valueProviders; + private IList _valueProviderFactories; /// /// Creates a new . @@ -52,18 +52,18 @@ namespace Microsoft.AspNetCore.Mvc } /// - /// Gets or sets the list of instances for the current request. + /// Gets or sets the list of instances for the current request. /// - public virtual IList ValueProviders + public virtual IList ValueProviderFactories { get { - if (_valueProviders == null) + if (_valueProviderFactories == null) { - _valueProviders = new List(); + _valueProviderFactories = new List(); } - return _valueProviders; + return _valueProviderFactories; } set { @@ -72,7 +72,7 @@ namespace Microsoft.AspNetCore.Mvc throw new ArgumentNullException(nameof(value)); } - _valueProviders = value; + _valueProviderFactories = value; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs index 094769b01c..07b4d77549 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs @@ -11,7 +11,6 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Filters; -using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerArgumentBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerArgumentBinder.cs index 72341226ff..8ccd759928 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerArgumentBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerArgumentBinder.cs @@ -70,7 +70,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } else if (actionDescriptor.BoundProperties.Count == 0) { - return PopulateArgumentsAsync(controllerContext, actionDescriptor.Parameters); + return BindActionArgumentsCoreAsync(controllerContext, actionDescriptor); } else { @@ -81,19 +81,36 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } + 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( ControllerContext controllerContext, object controller, ControllerActionDescriptor actionDescriptor) { + var valueProvider = await CompositeValueProvider.CreateAsync(controllerContext); + var controllerProperties = await PopulateArgumentsAsync( controllerContext, - actionDescriptor.BoundProperties); + actionDescriptor.BoundProperties, + valueProvider); ActivateProperties(actionDescriptor, controller, controllerProperties); var actionArguments = await PopulateArgumentsAsync( controllerContext, - actionDescriptor.Parameters); + actionDescriptor.Parameters, + valueProvider); return actionArguments; } @@ -111,10 +128,35 @@ namespace Microsoft.AspNetCore.Mvc.Internal throw new ArgumentNullException(nameof(controllerContext)); } + var valueProvider = await CompositeValueProvider.CreateAsync(controllerContext); + + return await BindModelAsync(parameter, controllerContext, valueProvider); + } + + public async Task BindModelAsync( + ParameterDescriptor parameter, + ControllerContext controllerContext, + IValueProvider valueProvider) + { + if (parameter == null) + { + throw new ArgumentNullException(nameof(parameter)); + } + + if (controllerContext == null) + { + throw new ArgumentNullException(nameof(controllerContext)); + } + + if (valueProvider == null) + { + throw new ArgumentNullException(nameof(valueProvider)); + } + var metadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterType); var modelBindingContext = DefaultModelBindingContext.CreateBindingContext( controllerContext, - new CompositeValueProvider(controllerContext.ValueProviders), + valueProvider, metadata, parameter.BindingInfo, parameter.Name); @@ -241,7 +283,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal private async Task> PopulateArgumentsAsync( ControllerContext controllerContext, - IList parameters) + IList parameters, + IValueProvider valueProvider) { var arguments = new Dictionary(StringComparer.Ordinal); @@ -249,7 +292,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal for (var i = 0; i < parameters.Count; i++) { var parameter = parameters[i]; - var modelBindingResult = await BindModelAsync(parameter, controllerContext); + var modelBindingResult = await BindModelAsync(parameter, controllerContext, valueProvider); if (modelBindingResult != null && modelBindingResult.Value.IsModelSet) { arguments[parameter.Name] = modelBindingResult.Value.Model; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterActionInvoker.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterActionInvoker.cs index 9bc2a01c7f..6c72e4f53e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterActionInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterActionInvoker.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal public abstract class FilterActionInvoker : IActionInvoker { private readonly ControllerActionInvokerCache _controllerActionInvokerCache; - private readonly IReadOnlyList _valueProviderFactories; + private readonly DiagnosticSource _diagnosticSource; private readonly int _maxModelValidationErrors; @@ -71,13 +71,17 @@ namespace Microsoft.AspNetCore.Mvc.Internal throw new ArgumentNullException(nameof(diagnosticSource)); } - Context = new ControllerContext(actionContext); _controllerActionInvokerCache = controllerActionInvokerCache; - _valueProviderFactories = valueProviderFactories; Logger = logger; _diagnosticSource = diagnosticSource; _maxModelValidationErrors = maxModelValidationErrors; + + Context = new ControllerContext(actionContext); + Context.ModelState.MaxAllowedErrors = _maxModelValidationErrors; + + // PERF: These are rarely going to be changed, so let's go copy-on-write. + Context.ValueProviderFactories = new CopyOnWriteList(valueProviderFactories); } protected ControllerContext Context { get; } @@ -110,8 +114,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal _controllerActionMethodExecutor = controllerActionInvokerState.ActionMethodExecutor; _cursor = new FilterCursor(_filters); - Context.ModelState.MaxAllowedErrors = _maxModelValidationErrors; - await InvokeAllAuthorizationFiltersAsync(); // If Authorization Filters return a result, it's a short circuit because @@ -301,18 +303,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal } else { - // We've reached the end of resource filters, so move to setting up state to invoke model - // binding. - var valueProviders = new List(); - var factoryContext = new ValueProviderFactoryContext(Context); - - for (var i = 0; i < _valueProviderFactories.Count; i++) - { - var factory = _valueProviderFactories[i]; - await factory.CreateValueProviderAsync(factoryContext); - } - Context.ValueProviders = factoryContext.ValueProviders; - // >> ExceptionFilters >> Model Binding >> ActionFilters >> Action await InvokeAllExceptionFiltersAsync(); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/CompositeValueProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/CompositeValueProvider.cs index be03408732..88a66204fa 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/CompositeValueProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/CompositeValueProvider.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Linq; +using System.Threading.Tasks; namespace Microsoft.AspNetCore.Mvc.ModelBinding { @@ -34,6 +35,33 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { } + /// + /// Asynchronously creates a using the provided + /// . + /// + /// The associated with the current request. + /// + /// A which, when completed, asynchronously returns a + /// . + /// + public static async Task CreateAsync(ControllerContext controllerContext) + { + if (controllerContext == null) + { + throw new ArgumentNullException(nameof(controllerContext)); + } + + var factories = controllerContext.ValueProviderFactories; + var valueProviderFactoryContext = new ValueProviderFactoryContext(controllerContext); + for (var i = 0; i < factories.Count; i++) + { + var factory = factories[i]; + await factory.CreateValueProviderAsync(valueProviderFactoryContext); + } + + return new CompositeValueProvider(valueProviderFactoryContext.ValueProviders); + } + /// public virtual bool ContainsPrefix(string prefix) { diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ICompositeValueProviderFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ICompositeValueProviderFactory.cs deleted file mode 100644 index 979d2c5422..0000000000 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ICompositeValueProviderFactory.cs +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Microsoft.AspNetCore.Mvc.ModelBinding -{ - /// - /// Represents an aggregate of . - /// - public interface ICompositeValueProviderFactory : IValueProviderFactory - { - } -} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ControllerBaseTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ControllerBaseTest.cs index 750a4dd109..b494e1be75 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ControllerBaseTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ControllerBaseTest.cs @@ -1488,10 +1488,11 @@ namespace Microsoft.AspNetCore.Mvc.Core.Test stringLocalizerFactory: null), }; + valueProvider = valueProvider ?? new SimpleValueProvider(); var controllerContext = new ControllerContext() { HttpContext = httpContext, - ValueProviders = new[] { valueProvider, }, + ValueProviderFactories = new[] { new SimpleValueProviderFactory(valueProvider), }, }; var binderFactory = new Mock(); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerArgumentBinderTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerArgumentBinderTests.cs index 3e45954f4f..2e43de0537 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerArgumentBinderTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerArgumentBinderTests.cs @@ -537,7 +537,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal }); var factory = GetModelBinderFactory(binder); - controllerContext.ValueProviders.Add(new SimpleValueProvider()); + controllerContext.ValueProviderFactories.Add(new SimpleValueProviderFactory()); var argumentBinder = GetArgumentBinder(factory); var controller = new TestController(); @@ -563,7 +563,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal RouteData = new RouteData(), }; - context.ValueProviders.Add(new SimpleValueProvider()); + context.ValueProviderFactories.Add(new SimpleValueProviderFactory()); return context; } diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs index 5de1052a2c..eaf0763e53 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs @@ -26,6 +26,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests { var httpContext = GetHttpContext(updateRequest, updateOptions); var services = httpContext.RequestServices; + var options = services.GetRequiredService>(); var context = new ModelBindingTestContext() { @@ -33,17 +34,9 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests HttpContext = httpContext, MetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(), RouteData = new RouteData(), + ValueProviderFactories = new List(options.Value.ValueProviderFactories), }; - var options = services.GetRequiredService>(); - var valueProviderFactoryContext = new ValueProviderFactoryContext(context); - foreach (var factory in options.Value.ValueProviderFactories) - { - factory.CreateValueProviderAsync(valueProviderFactoryContext).GetAwaiter().GetResult(); - } - - context.ValueProviders = valueProviderFactoryContext.ValueProviders; - return context; } @@ -129,19 +122,5 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests return serviceCollection.BuildServiceProvider(); } - - private static ControllerContext GetControllerContext(MvcOptions options, ActionContext context) - { - var valueProviderFactoryContext = new ValueProviderFactoryContext(context); - foreach (var factory in options.ValueProviderFactories) - { - factory.CreateValueProviderAsync(valueProviderFactoryContext).GetAwaiter().GetResult(); - } - - return new ControllerContext(context) - { - ValueProviders = valueProviderFactoryContext.ValueProviders - }; - } } } diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs index 759807990b..e6326dfacf 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs @@ -41,7 +41,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var oldModel = model; // Act - var result = await TryUpdateModel(model, string.Empty, testContext); + var result = await TryUpdateModelAsync(model, string.Empty, testContext); // Assert Assert.True(result); @@ -76,7 +76,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var model = new Address(); // Act - var result = await TryUpdateModel(model, string.Empty, testContext); + var result = await TryUpdateModelAsync(model, string.Empty, testContext); // Assert Assert.True(result); @@ -134,7 +134,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests }; // Act - var result = await TryUpdateModel(model, string.Empty, testContext); + var result = await TryUpdateModelAsync(model, string.Empty, testContext); // Assert Assert.True(result); @@ -184,7 +184,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var oldModel = model; // Act - var result = await TryUpdateModel(model, string.Empty, testContext); + var result = await TryUpdateModelAsync(model, string.Empty, testContext); // Assert Assert.True(result); @@ -225,7 +225,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var model = new Person2(); // Act - var result = await TryUpdateModel(model, string.Empty, testContext); + var result = await TryUpdateModelAsync(model, string.Empty, testContext); // Assert Assert.True(result); @@ -265,7 +265,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var collection = model.Address; // Act - var result = await TryUpdateModel(model, string.Empty, testContext); + var result = await TryUpdateModelAsync(model, string.Empty, testContext); // Assert Assert.True(result); @@ -327,7 +327,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests }; // Act - var result = await TryUpdateModel(model, string.Empty, testContext); + var result = await TryUpdateModelAsync(model, string.Empty, testContext); // Assert Assert.True(result); @@ -367,7 +367,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var model = new Person6(); // Act - var result = await TryUpdateModel(model, string.Empty, testContext); + var result = await TryUpdateModelAsync(model, string.Empty, testContext); // Assert Assert.True(result); @@ -406,7 +406,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var model = new Person4(); // Act - var result = await TryUpdateModel(model, string.Empty, testContext); + var result = await TryUpdateModelAsync(model, string.Empty, testContext); // Assert Assert.True(result); @@ -453,7 +453,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var collection = model.Address; // Act - var result = await TryUpdateModel(model, string.Empty, testContext); + var result = await TryUpdateModelAsync(model, string.Empty, testContext); // Assert Assert.True(result); @@ -495,7 +495,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var model = new Person5(); // Act - var result = await TryUpdateModel(model, string.Empty, testContext); + var result = await TryUpdateModelAsync(model, string.Empty, testContext); // Assert Assert.True(result); @@ -530,7 +530,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var oldModel = model; // Act - var result = await TryUpdateModel(model, "prefix", testContext); + var result = await TryUpdateModelAsync(model, "prefix", testContext); // Assert Assert.True(result); @@ -565,7 +565,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var model = new Address(); // Act - var result = await TryUpdateModel(model, "prefix", testContext); + var result = await TryUpdateModelAsync(model, "prefix", testContext); // Assert Assert.True(result); @@ -616,7 +616,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests }; // Act - var result = await TryUpdateModel(model, "prefix", testContext); + var result = await TryUpdateModelAsync(model, "prefix", testContext); // Assert Assert.True(result); @@ -666,7 +666,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var oldModel = model; // Act - var result = await TryUpdateModel(model, "prefix", testContext); + var result = await TryUpdateModelAsync(model, "prefix", testContext); // Assert Assert.True(result); @@ -702,7 +702,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var model = new Person2(); // Act - var result = await TryUpdateModel(model, "prefix", testContext); + var result = await TryUpdateModelAsync(model, "prefix", testContext); // Assert Assert.True(result); @@ -742,7 +742,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var collection = model.Address; // Act - var result = await TryUpdateModel(model, "prefix", testContext); + var result = await TryUpdateModelAsync(model, "prefix", testContext); // Assert Assert.True(result); @@ -794,7 +794,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests }; // Act - var result = await TryUpdateModel(model, "prefix", testContext); + var result = await TryUpdateModelAsync(model, "prefix", testContext); // Assert Assert.True(result); @@ -829,7 +829,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var model = new Person6(); // Act - var result = await TryUpdateModel(model, "prefix", testContext); + var result = await TryUpdateModelAsync(model, "prefix", testContext); // Assert Assert.True(result); @@ -863,7 +863,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var model = new Person4(); // Act - var result = await TryUpdateModel(model, "prefix", testContext); + var result = await TryUpdateModelAsync(model, "prefix", testContext); // Assert Assert.True(result); @@ -910,7 +910,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var collection = model.Address; // Act - var result = await TryUpdateModel(model, "prefix", testContext); + var result = await TryUpdateModelAsync(model, "prefix", testContext); // Assert Assert.True(result); @@ -947,7 +947,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var model = new Person5(); // Act - var result = await TryUpdateModel(model, "prefix", testContext); + var result = await TryUpdateModelAsync(model, "prefix", testContext); // Assert Assert.True(result); @@ -979,7 +979,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests }; // Act - var result = await TryUpdateModel(model, prefix: "files", testContext: testContext); + var result = await TryUpdateModelAsync(model, prefix: "files", testContext: testContext); // Assert Assert.True(result); @@ -1085,19 +1085,20 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests } } - private Task TryUpdateModel( + private async Task TryUpdateModelAsync( object model, string prefix, ModelBindingTestContext testContext) { - return ModelBindingHelper.TryUpdateModelAsync( + var valueProvider = await CompositeValueProvider.CreateAsync(testContext); + return await ModelBindingHelper.TryUpdateModelAsync( model, model.GetType(), prefix, testContext, testContext.MetadataProvider, TestModelBinderFactory.CreateDefault(), - new CompositeValueProvider(testContext.ValueProviders), + valueProvider, ModelBindingTestHelper.GetObjectValidator(testContext.MetadataProvider)); } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/SimpleValueProvider.cs b/test/Microsoft.AspNetCore.Mvc.TestCommon/SimpleValueProvider.cs similarity index 100% rename from test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/SimpleValueProvider.cs rename to test/Microsoft.AspNetCore.Mvc.TestCommon/SimpleValueProvider.cs diff --git a/test/Microsoft.AspNetCore.Mvc.TestCommon/SimpleValueProviderFactory.cs b/test/Microsoft.AspNetCore.Mvc.TestCommon/SimpleValueProviderFactory.cs new file mode 100644 index 0000000000..443deabade --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.TestCommon/SimpleValueProviderFactory.cs @@ -0,0 +1,36 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + + +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc.Internal; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding +{ + public class SimpleValueProviderFactory : IValueProviderFactory + { + private readonly IValueProvider _valueProvider; + + public SimpleValueProviderFactory() + { + _valueProvider = new SimpleValueProvider(); + } + + public SimpleValueProviderFactory(IValueProvider valueProvider) + { + if (valueProvider == null) + { + throw new ArgumentNullException(nameof(valueProvider)); + } + + _valueProvider = valueProvider; + } + + public Task CreateValueProviderAsync(ValueProviderFactoryContext context) + { + context.ValueProviders.Add(_valueProvider); + return TaskCache.CompletedTask; + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ControllerTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ControllerTest.cs index 2259d500f6..57e31a7c5c 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ControllerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ControllerTest.cs @@ -287,10 +287,11 @@ namespace Microsoft.AspNetCore.Mvc.Test stringLocalizerFactory: null), }; + valueProvider = valueProvider ?? new SimpleValueProvider(); var controllerContext = new ControllerContext() { HttpContext = httpContext, - ValueProviders = new[] { valueProvider, }, + ValueProviderFactories = new[] { new SimpleValueProviderFactory(valueProvider), }, }; var controller = new TestableController()