diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingResult.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingResult.cs index 82fea79acf..2233828c52 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingResult.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingResult.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Diagnostics; using System.Threading.Tasks; using Microsoft.Framework.Internal; diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BinderTypeBasedModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BinderTypeBasedModelBinder.cs index 295d45b40f..8c38c798f0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BinderTypeBasedModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BinderTypeBasedModelBinder.cs @@ -21,15 +21,24 @@ namespace Microsoft.AspNet.Mvc.ModelBinding private readonly ConcurrentDictionary _typeActivatorCache = new ConcurrentDictionary(); - public async Task BindModelAsync(ModelBindingContext bindingContext) + public Task BindModelAsync(ModelBindingContext bindingContext) { + // This method is optimized to use cached tasks when possible and avoid allocating + // using Task.FromResult. If you need to make changes of this nature, profile + // allocations afterwards and look for Task. + if (bindingContext.BinderType == null) { // Return NoResult so that we are able to continue with the default set of model binders, // if there is no specific model binder provided. - return ModelBindingResult.NoResult; + return ModelBindingResult.NoResultAsync; } + return BindModelCoreAsync(bindingContext); + } + + private async Task BindModelCoreAsync(ModelBindingContext bindingContext) + { var requestServices = bindingContext.OperationBindingContext.HttpContext.RequestServices; var createFactory = _typeActivatorCache.GetOrAdd(bindingContext.BinderType, _createFactory); var instance = createFactory(requestServices, arguments: null); diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BindingSourceModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BindingSourceModelBinder.cs deleted file mode 100644 index bc2b464ef2..0000000000 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BindingSourceModelBinder.cs +++ /dev/null @@ -1,91 +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. - -using System; -using System.Threading.Tasks; -using Microsoft.AspNet.Mvc.Core; -using Microsoft.Framework.Internal; - -namespace Microsoft.AspNet.Mvc.ModelBinding -{ - /// - /// An which provides data from a specific . - /// - /// - /// - /// A is an base-implementation which - /// can provide data for all parameters and model properties which specify the corresponding - /// . - /// - /// - /// is greedy, meaning that a given instance expects to handle all - /// parameters and properties annotated with the corresponding and - /// will short-circuit the model binding process to prevent other binders from running. - /// of must be set to true. - /// - /// - public abstract class BindingSourceModelBinder : IModelBinder - { - /// - /// Creates a new . - /// - /// - /// The . Must be a single-source (non-composite) with - /// equal to true. - /// - protected BindingSourceModelBinder([NotNull] BindingSource bindingSource) - { - // This class implements a pattern that's only useful for greedy model binders. If you need - // to implement something non-greedy then don't use the base class. - if (!bindingSource.IsGreedy) - { - var message = Resources.FormatBindingSource_MustBeGreedy( - bindingSource.DisplayName, - nameof(BindingSourceModelBinder)); - throw new ArgumentException(message, nameof(bindingSource)); - } - - BindingSource = bindingSource; - } - - /// - /// Gets the corresponding . - /// - protected BindingSource BindingSource { get; } - - /// - /// Binds the model. Called when the model's supported binding-source matches . - /// - /// The . - /// - /// A which will complete when model binding has completed. - /// - /// - /// Other model binders will never run if this method is called. Return null to skip other model binders - /// but allow higher-level handling e.g. falling back to empty prefix. - /// - protected abstract Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext); - - /// - public async Task BindModelAsync(ModelBindingContext context) - { - var allowedBindingSource = context.BindingSource; - if (allowedBindingSource == null || !allowedBindingSource.CanAcceptDataFrom(BindingSource)) - { - // Binding Sources are opt-in. This model either didn't specify one or specified something - // incompatible so let other binders run. - return ModelBindingResult.NoResult; - } - - var result = await BindModelCoreAsync(context); - - var modelBindingResult = result != ModelBindingResult.NoResult ? - result : - ModelBindingResult.Failed(context.ModelName); - - // This model binder is the only handler for its binding source. - // Always tell the model binding system to skip other model binders i.e. return non-null. - return modelBindingResult; - } - } -} diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs index 68c9c9a065..eb96fce813 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs @@ -14,19 +14,35 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// An which binds models from the request body using an /// when a model has the binding source / /// - public class BodyModelBinder : BindingSourceModelBinder + public class BodyModelBinder : IModelBinder { - /// - /// Creates a new . - /// - public BodyModelBinder() - : base(BindingSource.Body) + /// + public Task BindModelAsync(ModelBindingContext bindingContext) { + // This method is optimized to use cached tasks when possible and avoid allocating + // using Task.FromResult. If you need to make changes of this nature, profile + // allocations afterwards and look for Task. + + var allowedBindingSource = bindingContext.BindingSource; + if (allowedBindingSource == null || + !allowedBindingSource.CanAcceptDataFrom(BindingSource.Body)) + { + // Formatters are opt-in. This model either didn't specify [FromBody] or specified something + // incompatible so let other binders run. + return ModelBindingResult.NoResultAsync; + } + + return BindModelCoreAsync(bindingContext); } - /// - protected async override Task BindModelCoreAsync( - [NotNull] ModelBindingContext bindingContext) + /// + /// Attempts to bind the model using formatters. + /// + /// The . + /// + /// A which when completed returns a . + /// + private async Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) { // For compatibility with MVC 5.0 for top level object we want to consider an empty key instead of // the parameter name/a custom name. In all other cases (like when binding body to a property) we diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs index 1348855f74..fea4920b5f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs @@ -15,22 +15,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// public Task BindModelAsync([NotNull] ModelBindingContext bindingContext) { - return Task.FromResult(BindModel(bindingContext)); - } + // This method is optimized to use cached tasks when possible and avoid allocating + // using Task.FromResult. If you need to make changes of this nature, profile + // allocations afterwards and look for Task. - private ModelBindingResult BindModel(ModelBindingContext bindingContext) - { // Check if this binder applies. if (bindingContext.ModelType != typeof(byte[])) { - return ModelBindingResult.NoResult; + return ModelBindingResult.NoResultAsync; } // Check for missing data case 1: There was no element containing this data. var valueProviderResult = bindingContext.ValueProvider.GetValue(bindingContext.ModelName); if (valueProviderResult == ValueProviderResult.None) { - return ModelBindingResult.Failed(bindingContext.ModelName); + return ModelBindingResult.FailedAsync(bindingContext.ModelName); } bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult); @@ -39,7 +38,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var value = valueProviderResult.FirstValue; if (string.IsNullOrEmpty(value)) { - return ModelBindingResult.Failed(bindingContext.ModelName); + return ModelBindingResult.FailedAsync(bindingContext.ModelName); } try @@ -50,7 +49,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding bindingContext.ModelMetadata, model); - return ModelBindingResult.Success(bindingContext.ModelName, model, validationNode); + return ModelBindingResult.SuccessAsync(bindingContext.ModelName, model, validationNode); } catch (Exception ex) { @@ -59,7 +58,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Matched the type (byte[]) only this binder supports. As in missing data cases, always tell the model // binding system to skip other model binders i.e. return non-null. - return ModelBindingResult.Failed(bindingContext.ModelName); + return ModelBindingResult.FailedAsync(bindingContext.ModelName); } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs index 97d7ccc934..ac073085d4 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs @@ -18,13 +18,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public class FormCollectionModelBinder : IModelBinder { /// - public async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) + public Task BindModelAsync([NotNull] ModelBindingContext bindingContext) { + // This method is optimized to use cached tasks when possible and avoid allocating + // using Task.FromResult. If you need to make changes of this nature, profile + // allocations afterwards and look for Task. + if (bindingContext.ModelType != typeof(IFormCollection)) { - return ModelBindingResult.NoResult; + return ModelBindingResult.NoResultAsync; } + return BindModelCoreAsync(bindingContext); + } + + private async Task BindModelCoreAsync(ModelBindingContext bindingContext) + { object model; var request = bindingContext.OperationBindingContext.HttpContext.Request; if (request.HasFormContentType) diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs index 914678f710..cb320f918f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; #if DNXCORE50 using System.Reflection; @@ -20,8 +21,23 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public class FormFileModelBinder : IModelBinder { /// - public async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) + public Task BindModelAsync([NotNull] ModelBindingContext bindingContext) { + // This method is optimized to use cached tasks when possible and avoid allocating + // using Task.FromResult. If you need to make changes of this nature, profile + // allocations afterwards and look for Task. + + if (bindingContext.ModelType != typeof(IFormFile) && + !typeof(IEnumerable).IsAssignableFrom(bindingContext.ModelType)) + { + return ModelBindingResult.NoResultAsync; + } + + return BindModelCoreAsync(bindingContext); + } + + private async Task BindModelCoreAsync(ModelBindingContext bindingContext) + { object value; if (bindingContext.ModelType == typeof(IFormFile)) { @@ -36,6 +52,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding else { // This binder does not support the requested type. + Debug.Fail("We shouldn't be called without a matching type."); return ModelBindingResult.NoResult; } diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs index 36cc3097a1..cfad6553aa 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs @@ -14,33 +14,44 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public class GenericModelBinder : IModelBinder { - public async Task BindModelAsync(ModelBindingContext bindingContext) + public Task BindModelAsync(ModelBindingContext bindingContext) { + // This method is optimized to use cached tasks when possible and avoid allocating + // using Task.FromResult. If you need to make changes of this nature, profile + // allocations afterwards and look for Task. + var binderType = ResolveBinderType(bindingContext); - if (binderType != null) + if (binderType == null) { - var binder = (IModelBinder)Activator.CreateInstance(binderType); - - var collectionBinder = binder as ICollectionModelBinder; - if (collectionBinder != null && - bindingContext.Model == null && - !collectionBinder.CanCreateInstance(bindingContext.ModelType)) - { - // Able to resolve a binder type but need a new model instance and that binder cannot create it. - return ModelBindingResult.NoResult; - } - - var result = await binder.BindModelAsync(bindingContext); - var modelBindingResult = result != ModelBindingResult.NoResult ? - result : - ModelBindingResult.Failed(bindingContext.ModelName); - - // Were able to resolve a binder type. - // Always tell the model binding system to skip other model binders. - return modelBindingResult; + return ModelBindingResult.NoResultAsync; } - return ModelBindingResult.NoResult; + var binder = (IModelBinder)Activator.CreateInstance(binderType); + + var collectionBinder = binder as ICollectionModelBinder; + if (collectionBinder != null && + bindingContext.Model == null && + !collectionBinder.CanCreateInstance(bindingContext.ModelType)) + { + // Able to resolve a binder type but need a new model instance and that binder cannot create it. + return ModelBindingResult.NoResultAsync; + } + + return BindModelCoreAsync(bindingContext, binder); + } + + private async Task BindModelCoreAsync(ModelBindingContext bindingContext, IModelBinder binder) + { + Debug.Assert(binder != null); + + var result = await binder.BindModelAsync(bindingContext); + var modelBindingResult = result != ModelBindingResult.NoResult ? + result : + ModelBindingResult.Failed(bindingContext.ModelName); + + // Were able to resolve a binder type. + // Always tell the model binding system to skip other model binders. + return modelBindingResult; } private static Type ResolveBinderType(ModelBindingContext context) diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs index ec7d2deb64..5397332f27 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs @@ -7,7 +7,6 @@ using System.Reflection; #endif using System.Threading.Tasks; using Microsoft.AspNet.Http; -using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Mvc.ModelBinding { @@ -15,19 +14,24 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// An which binds models from the request headers when a model /// has the binding source / /// - public class HeaderModelBinder : BindingSourceModelBinder + public class HeaderModelBinder : IModelBinder { - /// - /// Creates a new . - /// - public HeaderModelBinder() - : base(BindingSource.Header) - { - } - /// - protected override Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) + public Task BindModelAsync(ModelBindingContext bindingContext) { + // This method is optimized to use cached tasks when possible and avoid allocating + // using Task.FromResult. If you need to make changes of this nature, profile + // allocations afterwards and look for Task. + + var allowedBindingSource = bindingContext.BindingSource; + if (allowedBindingSource == null || + !allowedBindingSource.CanAcceptDataFrom(BindingSource.Header)) + { + // Headers are opt-in. This model either didn't specify [FromHeader] or specified something + // incompatible so let other binders run. + return ModelBindingResult.NoResultAsync; + } + var request = bindingContext.OperationBindingContext.HttpContext.Request; var modelMetadata = bindingContext.ModelMetadata; diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs index a662a7eb6e..f6f0ea5e68 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs @@ -21,12 +21,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding typeof(MutableObjectModelBinder).GetTypeInfo().GetDeclaredMethod(nameof(CallPropertyAddRange)); /// - public virtual async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) + public Task BindModelAsync([NotNull] ModelBindingContext bindingContext) { ModelBindingHelper.ValidateBindingContext(bindingContext); if (!CanBindType(bindingContext.ModelMetadata)) { - return ModelBindingResult.NoResult; + return ModelBindingResult.NoResultAsync; } var mutableObjectBinderContext = new MutableObjectBinderContext() @@ -37,9 +37,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding if (!(CanCreateModel(mutableObjectBinderContext))) { - return ModelBindingResult.NoResult; + return ModelBindingResult.NoResultAsync; } + return BindModelCoreAsync(bindingContext, mutableObjectBinderContext); + } + + private async Task BindModelCoreAsync( + ModelBindingContext bindingContext, + MutableObjectBinderContext mutableObjectBinderContext) + { // Create model first (if necessary) to avoid reporting errors about properties when activation fails. var model = GetModel(bindingContext); diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ServicesModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ServicesModelBinder.cs index f0071b876d..c28cab3442 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ServicesModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ServicesModelBinder.cs @@ -3,7 +3,6 @@ using System.Threading.Tasks; using Microsoft.Framework.DependencyInjection; -using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Mvc.ModelBinding { @@ -11,19 +10,24 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// An which binds models from the request services when a model /// has the binding source / /// - public class ServicesModelBinder : BindingSourceModelBinder + public class ServicesModelBinder : IModelBinder { - /// - /// Creates a new . - /// - public ServicesModelBinder() - : base(BindingSource.Services) - { - } - /// - protected override Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) + public Task BindModelAsync(ModelBindingContext bindingContext) { + // This method is optimized to use cached tasks when possible and avoid allocating + // using Task.FromResult. If you need to make changes of this nature, profile + // allocations afterwards and look for Task. + + var allowedBindingSource = bindingContext.BindingSource; + if (allowedBindingSource == null || + !allowedBindingSource.CanAcceptDataFrom(BindingSource.Services)) + { + // Services are opt-in. This model either didn't specify [FromService] or specified something + // incompatible so let other binders run. + return ModelBindingResult.NoResultAsync; + } + var requestServices = bindingContext.OperationBindingContext.HttpContext.RequestServices; var model = requestServices.GetRequiredService(bindingContext.ModelType); var validationNode = diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/SimpleTypeModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/SimpleTypeModelBinder.cs index 89f1fd09de..ae4756ebd5 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/SimpleTypeModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/SimpleTypeModelBinder.cs @@ -11,22 +11,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public Task BindModelAsync(ModelBindingContext bindingContext) { - return Task.FromResult(BindModel(bindingContext)); - } + // This method is optimized to use cached tasks when possible and avoid allocating + // using Task.FromResult. If you need to make changes of this nature, profile + // allocations afterwards and look for Task. - public ModelBindingResult BindModel(ModelBindingContext bindingContext) - { if (bindingContext.ModelMetadata.IsComplexType) { // this type cannot be converted - return ModelBindingResult.NoResult; + return ModelBindingResult.NoResultAsync; } var valueProviderResult = bindingContext.ValueProvider.GetValue(bindingContext.ModelName); if (valueProviderResult == ValueProviderResult.None) { // no entry - return ModelBindingResult.NoResult; + return ModelBindingResult.NoResultAsync; } bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult); @@ -54,16 +53,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding bindingContext.ModelName, Resources.FormatCommon_ValueNotValidForProperty(model)); - return ModelBindingResult.Failed(bindingContext.ModelName); + return ModelBindingResult.FailedAsync(bindingContext.ModelName); } else { var validationNode = new ModelValidationNode( bindingContext.ModelName, - bindingContext.ModelMetadata, + bindingContext.ModelMetadata, model); - return ModelBindingResult.Success(bindingContext.ModelName, model, validationNode); + return ModelBindingResult.SuccessAsync(bindingContext.ModelName, model, validationNode); } } catch (Exception ex) @@ -72,7 +71,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Were able to find a converter for the type but conversion failed. // Tell the model binding system to skip other model binders. - return ModelBindingResult.Failed(bindingContext.ModelName); + return ModelBindingResult.FailedAsync(bindingContext.ModelName); } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs index 99077dab81..1049f779ee 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs @@ -954,22 +954,6 @@ namespace Microsoft.AspNet.Mvc.Core return string.Format(CultureInfo.CurrentCulture, GetString("BindingSource_MustBeFromRequest"), p0, p1); } - /// - /// The provided binding source '{0}' is not a greedy data source. '{1}' only supports greedy data sources. - /// - internal static string BindingSource_MustBeGreedy - { - get { return GetString("BindingSource_MustBeGreedy"); } - } - - /// - /// The provided binding source '{0}' is not a greedy data source. '{1}' only supports greedy data sources. - /// - internal static string FormatBindingSource_MustBeGreedy(object p0, object p1) - { - return string.Format(CultureInfo.CurrentCulture, GetString("BindingSource_MustBeGreedy"), p0, p1); - } - /// /// The property {0}.{1} could not be found. /// diff --git a/src/Microsoft.AspNet.Mvc.Core/Resources.resx b/src/Microsoft.AspNet.Mvc.Core/Resources.resx index 58b8251c8c..b153558e3f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Core/Resources.resx @@ -303,9 +303,6 @@ The provided binding source '{0}' is not a request-based binding source. '{1}' requires that the source must represent data from an HTTP request. - - The provided binding source '{0}' is not a greedy data source. '{1}' only supports greedy data sources. - The property {0}.{1} could not be found. diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BindingSourceModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BindingSourceModelBinderTest.cs deleted file mode 100644 index d32c559dc6..0000000000 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BindingSourceModelBinderTest.cs +++ /dev/null @@ -1,136 +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. - -using System; -using System.Threading.Tasks; -using Microsoft.Framework.Internal; -using Xunit; - -namespace Microsoft.AspNet.Mvc.ModelBinding -{ - public class BindingSourceModelBinderTest - { - [Fact] - public void BindingSourceModelBinder_ThrowsOnNonGreedySource() - { - // Arrange - var expected = - "The provided binding source 'Test Source' is not a greedy data source. " + - "'BindingSourceModelBinder' only supports greedy data sources." + Environment.NewLine + - "Parameter name: bindingSource"; - - var bindingSource = new BindingSource( - "Test", - displayName: "Test Source", - isGreedy: false, - isFromRequest: true); - - // Act & Assert - var exception = Assert.Throws( - () => new TestableBindingSourceModelBinder(bindingSource, isModelSet: false)); - Assert.Equal(expected, exception.Message); - } - - [Fact] - public async Task BindingSourceModelBinder_ReturnsNull_WithNoSource() - { - // Arrange - var context = new ModelBindingContext() - { - ModelMetadata = new EmptyModelMetadataProvider().GetMetadataForType(typeof(string)), - ModelName = "model", - }; - - var binder = new TestableBindingSourceModelBinder(BindingSource.Body, isModelSet: false); - - // Act - var result = await binder.BindModelAsync(context); - - // Assert - Assert.Equal(ModelBindingResult.NoResult, result); - Assert.False(binder.WasBindModelCoreCalled); - } - - [Fact] - public async Task BindingSourceModelBinder_ReturnsNull_NonMatchingSource() - { - // Arrange - var provider = new TestModelMetadataProvider(); - provider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Query); - - var context = new ModelBindingContext() - { - ModelMetadata = new EmptyModelMetadataProvider().GetMetadataForType(typeof(string)), - ModelName = "model", - }; - - var binder = new TestableBindingSourceModelBinder(BindingSource.Body, isModelSet: false); - - // Act - var result = await binder.BindModelAsync(context); - - // Assert - Assert.Equal(ModelBindingResult.NoResult, result); - Assert.False(binder.WasBindModelCoreCalled); - } - - [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task BindingSourceModelBinder_ReturnsNonEmptyResult_MatchingSource(bool isModelSet) - { - // Arrange - var provider = new TestModelMetadataProvider(); - provider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); - var modelMetadata = provider.GetMetadataForType(typeof(string)); - var context = new ModelBindingContext() - { - BinderModelName = modelMetadata.BinderModelName, - BindingSource = modelMetadata.BindingSource, - ModelMetadata = modelMetadata, - ModelName = "model", - }; - - var binder = new TestableBindingSourceModelBinder(BindingSource.Body, isModelSet); - - // Act - var result = await binder.BindModelAsync(context); - - // Assert - Assert.NotEqual(ModelBindingResult.NoResult, result); - Assert.Equal(isModelSet, result.IsModelSet); - Assert.Null(result.Model); - Assert.True(binder.WasBindModelCoreCalled); - } - - private class TestableBindingSourceModelBinder : BindingSourceModelBinder - { - bool _isModelSet; - - public TestableBindingSourceModelBinder(BindingSource source, bool isModelSet) - : base(source) - { - _isModelSet = isModelSet; - } - - public bool WasBindModelCoreCalled { get; private set; } - - protected override Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) - { - WasBindModelCoreCalled = true; - - if (_isModelSet) - { - return ModelBindingResult.SuccessAsync( - bindingContext.ModelName, - model: null, - validationNode: null); - } - else - { - return ModelBindingResult.FailedAsync(bindingContext.ModelName); - } - } - } - } -} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/HeaderModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/HeaderModelBinderTests.cs index fad2242c68..71c9fc0b9a 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/HeaderModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/HeaderModelBinderTests.cs @@ -72,6 +72,50 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.Equal(headerValue, result.Model); } + [Fact] + public async Task HeaderBinder_ReturnsNoResult_ForNullBindingSource() + { + // Arrange + var type = typeof(string); + var header = "User-Agent"; + var headerValue = "UnitTest"; + + var binder = new HeaderModelBinder(); + var modelBindingContext = GetBindingContext(type); + modelBindingContext.BindingSource = null; + + modelBindingContext.FieldName = header; + modelBindingContext.OperationBindingContext.HttpContext.Request.Headers.Add(header, new[] { headerValue }); + + // Act + var result = await binder.BindModelAsync(modelBindingContext); + + // Assert + Assert.Equal(ModelBindingResult.NoResult, result); + } + + [Fact] + public async Task HeaderBinder_ReturnsNoResult_ForNonHeaderBindingSource() + { + // Arrange + var type = typeof(string); + var header = "User-Agent"; + var headerValue = "UnitTest"; + + var binder = new HeaderModelBinder(); + var modelBindingContext = GetBindingContext(type); + modelBindingContext.BindingSource = BindingSource.Body; + + modelBindingContext.FieldName = header; + modelBindingContext.OperationBindingContext.HttpContext.Request.Headers.Add(header, new[] { headerValue }); + + // Act + var result = await binder.BindModelAsync(modelBindingContext); + + // Assert + Assert.Equal(ModelBindingResult.NoResult, result); + } + private static ModelBindingContext GetBindingContext(Type modelType) { var metadataProvider = new TestModelMetadataProvider(); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ServiceModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ServiceModelBinderTest.cs new file mode 100644 index 0000000000..5ffb12904a --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ServiceModelBinderTest.cs @@ -0,0 +1,111 @@ +// 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.AspNet.Http.Internal; +using Microsoft.Framework.DependencyInjection; +using Xunit; + +namespace Microsoft.AspNet.Mvc.ModelBinding +{ + public class ServiceModelBinderTest + { + [Fact] + public async Task ServiceModelBinder_BindsService() + { + // Arrange + var type = typeof(IService); + + var binder = new ServicesModelBinder(); + var modelBindingContext = GetBindingContext(type); + + // Act + var result = await binder.BindModelAsync(modelBindingContext); + + // Assert + Assert.NotEqual(ModelBindingResult.NoResult, result); + Assert.True(result.IsModelSet); + Assert.NotNull(result.Model); + Assert.Equal("modelName", result.Key); + + Assert.NotNull(result.ValidationNode); + Assert.Equal("modelName", result.ValidationNode.Key); + Assert.True(result.ValidationNode.SuppressValidation); + } + + [Fact] + public async Task ServiceModelBinder_ReturnsNoResult_ForNullBindingSource() + { + // Arrange + var type = typeof(IService); + + var binder = new ServicesModelBinder(); + var modelBindingContext = GetBindingContext(type); + modelBindingContext.BindingSource = null; + + // Act + var result = await binder.BindModelAsync(modelBindingContext); + + // Assert + Assert.Equal(ModelBindingResult.NoResult, result); + } + + [Fact] + public async Task ServiceModelBinder_ReturnsNoResult_ForNonServiceBindingSource() + { + // Arrange + var type = typeof(IService); + + var binder = new ServicesModelBinder(); + var modelBindingContext = GetBindingContext(type); + modelBindingContext.BindingSource = BindingSource.Body; + + // Act + var result = await binder.BindModelAsync(modelBindingContext); + + // Assert + Assert.Equal(ModelBindingResult.NoResult, result); + } + + private static ModelBindingContext GetBindingContext(Type modelType) + { + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider.ForType(modelType).BindingDetails(d => d.BindingSource = BindingSource.Services); + var modelMetadata = metadataProvider.GetMetadataForType(modelType); + + + var services = new ServiceCollection(); + services.AddInstance(new Service()); + + var bindingContext = new ModelBindingContext + { + ModelMetadata = modelMetadata, + ModelName = "modelName", + FieldName = "modelName", + ModelState = new ModelStateDictionary(), + OperationBindingContext = new OperationBindingContext + { + ModelBinder = new HeaderModelBinder(), + MetadataProvider = metadataProvider, + HttpContext = new DefaultHttpContext() + { + RequestServices = services.BuildServiceProvider(), + }, + }, + BinderModelName = modelMetadata.BinderModelName, + BindingSource = modelMetadata.BindingSource, + }; + + return bindingContext; + } + + private interface IService + { + } + + private class Service : IService + { + } + } +} diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs index b84bf5684e..fd0b570525 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs @@ -148,15 +148,19 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests } } - private class AddressBinder : BindingSourceModelBinder + private class AddressBinder : IModelBinder { - public AddressBinder() - : base(BindAddressAttribute.Source) + public Task BindModelAsync(ModelBindingContext bindingContext) { - } + var allowedBindingSource = bindingContext.BindingSource; + if (allowedBindingSource == null || + !allowedBindingSource.CanAcceptDataFrom(BindAddressAttribute.Source)) + { + // Binding Sources are opt-in. This model either didn't specify one or specified something + // incompatible so let other binders run. + return ModelBindingResult.NoResultAsync; + } - protected override Task BindModelCoreAsync(ModelBindingContext bindingContext) - { return ModelBindingResult.SuccessAsync(bindingContext.ModelName, new Address(), validationNode: null); } } diff --git a/test/WebSites/ModelBindingWebSite/TestBindingSourceModelBinder.cs b/test/WebSites/ModelBindingWebSite/TestBindingSourceModelBinder.cs index 67c080ddef..10f9bb4cab 100644 --- a/test/WebSites/ModelBindingWebSite/TestBindingSourceModelBinder.cs +++ b/test/WebSites/ModelBindingWebSite/TestBindingSourceModelBinder.cs @@ -10,15 +10,17 @@ using Microsoft.AspNet.Mvc.ModelBinding.Metadata; namespace ModelBindingWebSite { - public class TestBindingSourceModelBinder : BindingSourceModelBinder + public class TestBindingSourceModelBinder : IModelBinder { - public TestBindingSourceModelBinder() - : base(FromTestAttribute.TestBindingSource) + public Task BindModelAsync(ModelBindingContext bindingContext) { - } + var allowedBindingSource = bindingContext.BindingSource; + if (allowedBindingSource == null || + !allowedBindingSource.CanAcceptDataFrom(FromTestAttribute.TestBindingSource)) + { + return ModelBindingResult.NoResultAsync; + } - protected override Task BindModelCoreAsync(ModelBindingContext bindingContext) - { var attributes = ((DefaultModelMetadata)bindingContext.ModelMetadata).Attributes; var metadata = attributes.Attributes.OfType().First(); var model = metadata.Value;