Rewrite MutableObjectModelBinder

A rewrite focused on simplifying extensibility points and reducing
allocations.
This commit is contained in:
Ryan Nowak 2016-02-05 12:14:16 -08:00
parent fa9f13db2d
commit 698502df8c
5 changed files with 553 additions and 1243 deletions

View File

@ -71,7 +71,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal
// PropertyBindingPredicateProvider
var predicateProviders = context.Attributes.OfType<IPropertyBindingPredicateProvider>().ToArray();
if (predicateProviders.Length > 0)
if (predicateProviders.Length == 0)
{
context.BindingMetadata.PropertyBindingPredicateProvider = null;
}
else if (predicateProviders.Length == 1)
{
context.BindingMetadata.PropertyBindingPredicateProvider = predicateProviders[0];
}
else
{
context.BindingMetadata.PropertyBindingPredicateProvider = new CompositePredicateProvider(
predicateProviders);

View File

@ -2,8 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc.Internal;
@ -15,9 +13,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
/// </summary>
public class MutableObjectModelBinder : IModelBinder
{
private static readonly MethodInfo CallPropertyAddRangeOpenGenericMethod =
typeof(MutableObjectModelBinder).GetTypeInfo().GetDeclaredMethod(nameof(CallPropertyAddRange));
/// <inheritdoc />
public Task BindModelAsync(ModelBindingContext bindingContext)
{
@ -32,55 +27,119 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
return TaskCache.CompletedTask;
}
var mutableObjectBinderContext = new MutableObjectBinderContext()
{
ModelBindingContext = bindingContext,
PropertyMetadata = GetMetadataForProperties(bindingContext).ToArray(),
};
if (!(CanCreateModel(mutableObjectBinderContext)))
if (!CanCreateModel(bindingContext))
{
return TaskCache.CompletedTask;
}
return BindModelCoreAsync(bindingContext, mutableObjectBinderContext);
// Perf: separated to avoid allocating a state machine when we don't
// need to go async.
return BindModelCoreAsync(bindingContext);
}
private async Task BindModelCoreAsync(
ModelBindingContext bindingContext,
MutableObjectBinderContext mutableObjectBinderContext)
private async Task BindModelCoreAsync(ModelBindingContext bindingContext)
{
// Create model first (if necessary) to avoid reporting errors about properties when activation fails.
var model = GetModel(bindingContext);
if (bindingContext.Model == null)
{
bindingContext.Model = CreateModel(bindingContext);
}
var results = await BindPropertiesAsync(bindingContext, mutableObjectBinderContext.PropertyMetadata);
foreach (var property in bindingContext.ModelMetadata.Properties)
{
if (!CanBindProperty(bindingContext, property))
{
continue;
}
// Post-processing e.g. property setters and hooking up validation.
bindingContext.Model = model;
ProcessResults(bindingContext, results);
// Pass complex (including collection) values down so that binding system does not unnecessarily
// recreate instances or overwrite inner properties that are not bound. No need for this with simple
// values because they will be overwritten if binding succeeds. Arrays are never reused because they
// cannot be resized.
object propertyModel = null;
if (property.PropertyGetter != null &&
property.IsComplexType &&
!property.ModelType.IsArray)
{
propertyModel = property.PropertyGetter(bindingContext.Model);
}
bindingContext.Result = ModelBindingResult.Success(bindingContext.ModelName, model);
var fieldName = property.BinderModelName ?? property.PropertyName;
var modelName = ModelNames.CreatePropertyModelName(bindingContext.ModelName, fieldName);
ModelBindingResult result;
using (bindingContext.EnterNestedScope(
modelMetadata: property,
fieldName: fieldName,
modelName: modelName,
model: propertyModel))
{
await BindProperty(bindingContext);
result = bindingContext.Result ?? ModelBindingResult.Failed(modelName);
}
if (result.IsModelSet)
{
SetProperty(bindingContext, property, result);
}
else if (property.IsBindingRequired)
{
var message = property.ModelBindingMessageProvider.MissingBindRequiredValueAccessor(fieldName);
bindingContext.ModelState.TryAddModelError(modelName, message);
}
}
bindingContext.Result = ModelBindingResult.Success(bindingContext.ModelName, bindingContext.Model);
}
/// <summary>
/// Gets an indication whether a property with the given <paramref name="propertyMetadata"/> can be updated.
/// Gets a value indicating whether or not the model property identified by <paramref name="propertyMetadata"/>
/// can be bound.
/// </summary>
/// <param name="propertyMetadata"><see cref="ModelMetadata"/> for the property of interest.</param>
/// <returns><c>true</c> if the property can be updated; <c>false</c> otherwise.</returns>
/// <remarks>Should return <c>true</c> only for properties <see cref="SetProperty"/> can update.</remarks>
protected virtual bool CanUpdateProperty(ModelMetadata propertyMetadata)
/// <param name="bindingContext">The <see cref="ModelBindingContext"/> for the container model.</param>
/// <param name="propertyMetadata">The <see cref="ModelMetadata"/> for the model property.</param>
/// <returns><c>true</c> if the model property can be bound, otherwise <c>false</c>.</returns>
protected virtual bool CanBindProperty(ModelBindingContext bindingContext, ModelMetadata propertyMetadata)
{
if (propertyMetadata == null)
var modelMetadataPredicate = bindingContext.ModelMetadata.PropertyBindingPredicateProvider?.PropertyFilter;
if (modelMetadataPredicate?.Invoke(bindingContext, propertyMetadata.PropertyName) == false)
{
throw new ArgumentNullException(nameof(propertyMetadata));
return false;
}
return CanUpdatePropertyInternal(propertyMetadata);
if (bindingContext.PropertyFilter?.Invoke(bindingContext, propertyMetadata.PropertyName) == false)
{
return false;
}
if (!propertyMetadata.IsBindingAllowed)
{
return false;
}
if (!CanUpdatePropertyInternal(propertyMetadata))
{
return false;
}
return true;
}
internal bool CanCreateModel(MutableObjectBinderContext context)
/// <summary>
/// Attempts to bind a property of the model.
/// </summary>
/// <param name="bindingContext">The <see cref="ModelBindingContext"/> for the model property.</param>
/// <returns>
/// A <see cref="Task"/> that when completed will set <see cref="ModelBindingContext.Result"/> to the
/// result of model binding.
/// </returns>
protected virtual Task BindProperty(ModelBindingContext bindingContext)
{
return bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(bindingContext);
}
internal bool CanCreateModel(ModelBindingContext bindingContext)
{
var bindingContext = context.ModelBindingContext;
var isTopLevelObject = bindingContext.IsTopLevelObject;
// If we get here the model is a complex object which was not directly bound by any previous model binder,
@ -88,7 +147,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
// recursion.
//
// First, we want to make sure this object is allowed to come from a value provider source as this binder
// will always include value provider data. For instance if the model is marked with [FromBody], then we
// will only include value provider data. For instance if the model is marked with [FromBody], then we
// can just skip it. A greedy source cannot be a value provider.
//
// If the model isn't marked with ANY binding source, then we assume it's OK also.
@ -108,14 +167,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
return true;
}
// 2. If it is top level object and there are no properties to bind
if (isTopLevelObject && context.PropertyMetadata != null && context.PropertyMetadata.Count == 0)
{
return true;
}
// 3. Any of the model properties can be bound using a value provider.
if (CanValueBindAnyModelProperties(context))
// 2. Any of the model properties can be bound using a value provider.
if (CanValueBindAnyModelProperties(bindingContext))
{
return true;
}
@ -123,11 +176,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
return false;
}
private bool CanValueBindAnyModelProperties(MutableObjectBinderContext context)
private bool CanValueBindAnyModelProperties(ModelBindingContext bindingContext)
{
// If there are no properties on the model, there is nothing to bind. We are here means this is not a top
// level object. So we return false.
if (context.PropertyMetadata == null || context.PropertyMetadata.Count == 0)
if (bindingContext.ModelMetadata.Properties.Count == 0)
{
return false;
}
@ -137,7 +190,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
//
// However, because a property might specify a custom binding source ([FromForm]), it's not correct
// for us to just try bindingContext.ValueProvider.ContainsPrefixAsync(bindingContext.ModelName),
// because that may include ALL value providers - that would lead us to mistakenly create the model
// because that may include other value providers - that would lead us to mistakenly create the model
// when the data is coming from a source we should use (ex: value found in query string, but the
// model has [FromForm]).
//
@ -157,9 +210,17 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
// create the model and try to bind it. OR if ALL properties of the model have a greedy source,
// then we go ahead and create it.
//
var hasBindableProperty = false;
var isAnyPropertyEnabledForValueProviderBasedBinding = false;
foreach (var propertyMetadata in context.PropertyMetadata)
foreach (var propertyMetadata in bindingContext.ModelMetadata.Properties)
{
if (!CanBindProperty(bindingContext, propertyMetadata))
{
continue;
}
hasBindableProperty = true;
// This check will skip properties which are marked explicitly using a non value binder.
var bindingSource = propertyMetadata.BindingSource;
if (bindingSource == null || !bindingSource.IsGreedy)
@ -168,17 +229,17 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
var fieldName = propertyMetadata.BinderModelName ?? propertyMetadata.PropertyName;
var modelName = ModelNames.CreatePropertyModelName(
context.ModelBindingContext.ModelName,
bindingContext.ModelName,
fieldName);
using (context.ModelBindingContext.EnterNestedScope(
using (bindingContext.EnterNestedScope(
modelMetadata: propertyMetadata,
fieldName: fieldName,
modelName: modelName,
model: null))
{
// If any property can return a true value.
if (CanBindValue(context.ModelBindingContext))
if (CanBindValue(bindingContext))
{
return true;
}
@ -186,12 +247,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
}
}
if (!isAnyPropertyEnabledForValueProviderBasedBinding)
if (hasBindableProperty && !isAnyPropertyEnabledForValueProviderBasedBinding)
{
// Either there are no properties or all the properties are marked as
// a non value provider based marker.
// This would be the case when the model has all its properties annotated with
// a IBinderMetadata. We want to be able to create such a model.
// All the properties are marked with a non value provider based marker like [FromHeader] or
// [FromBody].
return true;
}
@ -205,8 +264,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
var bindingSource = bindingContext.BindingSource;
if (bindingSource != null && !bindingSource.IsGreedy)
{
var rootValueProvider =
bindingContext.OperationBindingContext.ValueProvider as IBindingSourceValueProvider;
var rootValueProvider = bindingContext.OperationBindingContext.ValueProvider as IBindingSourceValueProvider;
if (rootValueProvider != null)
{
valueProvider = rootValueProvider.Filter(bindingSource);
@ -242,6 +300,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
return true;
}
// Internal for tests
internal static bool CanUpdatePropertyInternal(ModelMetadata propertyMetadata)
{
return !propertyMetadata.IsReadOnly || CanUpdateReadOnlyProperty(propertyMetadata.ModelType);
@ -273,52 +332,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
return true;
}
// Returned dictionary contains entries corresponding to properties against which binding was attempted. If
// binding failed, the entry's value will have IsModelSet == false. Binding is attempted for all elements of
// propertyMetadatas.
private async Task<IDictionary<ModelMetadata, ModelBindingResult>> BindPropertiesAsync(
ModelBindingContext bindingContext,
IEnumerable<ModelMetadata> propertyMetadatas)
{
var results = new Dictionary<ModelMetadata, ModelBindingResult>();
foreach (var propertyMetadata in propertyMetadatas)
{
// ModelBindingContext.Model property values may be non-null when invoked via TryUpdateModel(). Pass
// complex (including collection) values down so that binding system does not unnecessarily recreate
// instances or overwrite inner properties that are not bound. No need for this with simple values
// because they will be overwritten if binding succeeds. Arrays are never reused because they cannot
// be resized.
object model = null;
if (propertyMetadata.PropertyGetter != null &&
propertyMetadata.IsComplexType &&
!propertyMetadata.ModelType.IsArray)
{
model = propertyMetadata.PropertyGetter(bindingContext.Model);
}
var fieldName = propertyMetadata.BinderModelName ?? propertyMetadata.PropertyName;
var modelName = ModelNames.CreatePropertyModelName(bindingContext.ModelName, fieldName);
using (bindingContext.EnterNestedScope(
modelMetadata: propertyMetadata,
fieldName: fieldName,
modelName: modelName,
model: model))
{
await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(bindingContext);
var result = bindingContext.Result;
if (result == null)
{
// Could not bind. Let ProcessResult() know explicitly.
result = ModelBindingResult.Failed(bindingContext.ModelName);
}
results[propertyMetadata] = result.Value;
}
}
return results;
}
/// <summary>
/// Creates suitable <see cref="object"/> for given <paramref name="bindingContext"/>.
@ -337,137 +350,14 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
return Activator.CreateInstance(bindingContext.ModelType);
}
/// <summary>
/// Get <see cref="ModelBindingContext.Model"/> if that property is not <c>null</c>. Otherwise activate a
/// new instance of <see cref="ModelBindingContext.ModelType"/>.
/// </summary>
/// <param name="bindingContext">The <see cref="ModelBindingContext"/>.</param>
protected virtual object GetModel(ModelBindingContext bindingContext)
{
if (bindingContext == null)
{
throw new ArgumentNullException(nameof(bindingContext));
}
if (bindingContext.Model != null)
{
return bindingContext.Model;
}
return CreateModel(bindingContext);
}
/// <summary>
/// Gets the collection of <see cref="ModelMetadata"/> for properties this binder should update.
/// </summary>
/// <param name="bindingContext">The <see cref="ModelBindingContext"/>.</param>
/// <returns>Collection of <see cref="ModelMetadata"/> for properties this binder should update.</returns>
protected virtual IEnumerable<ModelMetadata> GetMetadataForProperties(
ModelBindingContext bindingContext)
{
if (bindingContext == null)
{
throw new ArgumentNullException(nameof(bindingContext));
}
var validationInfo = GetPropertyValidationInfo(bindingContext);
var newPropertyFilter = GetPropertyFilter();
return bindingContext.ModelMetadata.Properties
.Where(propertyMetadata =>
newPropertyFilter(bindingContext, propertyMetadata.PropertyName) &&
(validationInfo.RequiredProperties.Contains(propertyMetadata.PropertyName) ||
!validationInfo.SkipProperties.Contains(propertyMetadata.PropertyName)) &&
CanUpdateProperty(propertyMetadata));
}
private static Func<ModelBindingContext, string, bool> GetPropertyFilter()
{
return (ModelBindingContext context, string propertyName) =>
{
var modelMetadataPredicate = context.ModelMetadata.PropertyBindingPredicateProvider?.PropertyFilter;
return
(context.PropertyFilter == null || context.PropertyFilter(context, propertyName)) &&
(modelMetadataPredicate == null || modelMetadataPredicate(context, propertyName));
};
}
internal static PropertyValidationInfo GetPropertyValidationInfo(ModelBindingContext bindingContext)
{
var validationInfo = new PropertyValidationInfo();
foreach (var propertyMetadata in bindingContext.ModelMetadata.Properties)
{
var propertyName = propertyMetadata.PropertyName;
if (!propertyMetadata.IsBindingAllowed)
{
// Nothing to do here if binding is not allowed.
validationInfo.SkipProperties.Add(propertyName);
continue;
}
if (propertyMetadata.IsBindingRequired)
{
validationInfo.RequiredProperties.Add(propertyName);
}
}
return validationInfo;
}
// Internal for testing.
internal void ProcessResults(
ModelBindingContext bindingContext,
IDictionary<ModelMetadata, ModelBindingResult> results)
{
var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider;
var metadata = metadataProvider.GetMetadataForType(bindingContext.ModelType);
var validationInfo = GetPropertyValidationInfo(bindingContext);
// Eliminate provided properties from RequiredProperties; leaving just *missing* required properties.
var boundProperties = results.Where(p => p.Value.IsModelSet).Select(p => p.Key.PropertyName);
validationInfo.RequiredProperties.ExceptWith(boundProperties);
foreach (var missingRequiredProperty in validationInfo.RequiredProperties)
{
var propertyMetadata = metadata.Properties[missingRequiredProperty];
var propertyName = propertyMetadata.BinderModelName ?? missingRequiredProperty;
var modelStateKey = ModelNames.CreatePropertyModelName(bindingContext.ModelName, propertyName);
bindingContext.ModelState.TryAddModelError(
modelStateKey,
bindingContext.ModelMetadata.ModelBindingMessageProvider.MissingBindRequiredValueAccessor(
propertyName));
}
// For each property that BindPropertiesAsync() attempted to bind, call the setter, recording
// exceptions as necessary.
foreach (var entry in results)
{
if (entry.Value != null)
{
var result = entry.Value;
var propertyMetadata = entry.Key;
SetProperty(bindingContext, metadata, propertyMetadata, result);
}
}
}
/// <summary>
/// Updates a property in the current <see cref="ModelBindingContext.Model"/>.
/// </summary>
/// <param name="bindingContext">The <see cref="ModelBindingContext"/>.</param>
/// <param name="metadata">
/// The <see cref="ModelMetadata"/> for the model containing property to set.
/// </param>
/// <param name="propertyMetadata">The <see cref="ModelMetadata"/> for the property to set.</param>
/// <param name="result">The <see cref="ModelBindingResult"/> for the property's new value.</param>
/// <remarks>Should succeed in all cases that <see cref="CanUpdateProperty"/> returns <c>true</c>.</remarks>
protected virtual void SetProperty(
ModelBindingContext bindingContext,
ModelMetadata metadata,
ModelMetadata propertyMetadata,
ModelBindingResult result)
{
@ -476,17 +366,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
throw new ArgumentNullException(nameof(bindingContext));
}
if (metadata == null)
{
throw new ArgumentNullException(nameof(metadata));
}
if (propertyMetadata == null)
{
throw new ArgumentNullException(nameof(propertyMetadata));
}
if (!result.IsModelSet)
{
// If we don't have a value, don't set it on the model and trounce a pre-initialized value.
@ -495,8 +379,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
if (propertyMetadata.IsReadOnly)
{
// Try to handle as a collection if property exists but is not settable.
AddToProperty(bindingContext, metadata, propertyMetadata, result);
// The property should have already been set when we called BindPropertyAsync, so there's
// nothing to do here.
return;
}
@ -511,62 +395,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
}
}
private void AddToProperty(
ModelBindingContext bindingContext,
ModelMetadata modelMetadata,
ModelMetadata propertyMetadata,
ModelBindingResult result)
{
var target = propertyMetadata.PropertyGetter(bindingContext.Model);
var source = result.Model;
if (target == null || source == null)
{
// Cannot copy to or from a null collection.
return;
}
if (target == source)
{
// Added to the target collection in BindPropertiesAsync().
return;
}
// Determine T if this is an ICollection<T> property. No need for a T[] case because CanUpdateProperty()
// ensures property is either settable or not an array. Underlying assumption is that CanUpdateProperty()
// and SetProperty() are overridden together.
if (!propertyMetadata.IsCollectionType)
{
// Not a collection model.
return;
}
var propertyAddRange = CallPropertyAddRangeOpenGenericMethod.MakeGenericMethod(
propertyMetadata.ElementMetadata.ModelType);
try
{
propertyAddRange.Invoke(obj: null, parameters: new[] { target, source });
}
catch (Exception exception)
{
AddModelError(exception, bindingContext, result);
}
}
// Called via reflection.
private static void CallPropertyAddRange<TElement>(object target, object source)
{
var targetCollection = (ICollection<TElement>)target;
var sourceCollection = source as IEnumerable<TElement>;
if (sourceCollection != null && !targetCollection.IsReadOnly)
{
targetCollection.Clear();
foreach (var item in sourceCollection)
{
targetCollection.Add(item);
}
}
}
private static void AddModelError(
Exception exception,
ModelBindingContext bindingContext,
@ -587,18 +415,5 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
modelState.AddModelError(modelStateKey, exception, bindingContext.ModelMetadata);
}
}
internal sealed class PropertyValidationInfo
{
public PropertyValidationInfo()
{
RequiredProperties = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
SkipProperties = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
}
public HashSet<string> RequiredProperties { get; private set; }
public HashSet<string> SkipProperties { get; private set; }
}
}
}

View File

@ -1,14 +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.Collections.Generic;
namespace Microsoft.AspNetCore.Mvc.ModelBinding
{
public class MutableObjectBinderContext
{
public ModelBindingContext ModelBindingContext { get; set; }
public IReadOnlyList<ModelMetadata> PropertyMetadata { get; set; }
}
}

View File

@ -1400,7 +1400,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
// Arrange
var metadataProvider = new TestModelMetadataProvider();
metadataProvider
.ForType(typeof(Order10))
.ForProperty(typeof(Order10), nameof(Order10.Customer))
.BindingDetails((Action<ModelBinding.Metadata.BindingMetadata>)(binding =>
{
// A real details provider could customize message based on BindingMetadataProviderContext.