Add `ModelMetadata.ElementMetadata`

- #2664
- use new property to correctly determine `isTargetEnum` in `GetCurrentValues()`
 - avoid `ArgumentNullException` in all cases where raw values are `enum` but target is not
- stop skipping tests blocked by #2664, exposing a couple more #1487 issues
- use new property instead of private `GetElementType()` methods where possible
 - cleans up some duplicate code
 - also remove redundant use of `IsCollectionType` and `ElementMetadata`

nits:
- move properties above methods in `ModelMetadata`
- avoid accidentally-incorrect "Remove Unnecessary Usings"
This commit is contained in:
Doug Bunting 2015-06-10 12:02:50 -07:00
parent afd5b4f2a6
commit 3f6ab3bb03
10 changed files with 224 additions and 110 deletions

View File

@ -5,7 +5,9 @@ using System;
using System.Collections;
using System.Collections.Generic;
using System.ComponentModel;
#if DNXCORE50
using System.Reflection;
#endif
using Microsoft.AspNet.Mvc.ModelBinding.Metadata;
using Microsoft.Framework.Internal;
@ -79,7 +81,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
public abstract string BinderModelName { get; }
/// <summary>
/// Gets the <see cref="Type"/> of an <see cref="IModelBinder"/> of a model if specified explicitly using
/// Gets the <see cref="Type"/> of an <see cref="IModelBinder"/> of a model if specified explicitly using
/// <see cref="IBinderTypeProviderMetadata"/>.
/// </summary>
public abstract Type BinderType { get; }
@ -124,6 +126,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
/// </summary>
public abstract string EditFormatString { get; }
/// <summary>
/// Gets the <see cref="ModelMetadata"/> for elements of <see cref="ModelType"/> if that <see cref="Type"/>
/// implements <see cref="IEnumerable"/>.
/// </summary>
/// <value>
/// <see cref="ModelMetadata"/> for <c>T</c> if <see cref="ModelType"/> implements
/// <see cref="IEnumerable{T}"/>. <see cref="ModelMetadata"/> for <c>object</c> if <see cref="ModelType"/>
/// implements <see cref="IEnumerable"/> but not <see cref="IEnumerable{T}"/>. <c>null</c> otherwise i.e. when
/// <see cref="IsCollectionType"/> is <c>false</c>.
/// </value>
public abstract ModelMetadata ElementMetadata { get; }
/// <summary>
/// Gets the ordered display names and values of all <see cref="Enum"/> values in <see cref="ModelType"/> or
/// <c>Nullable.GetUnderlyingType(ModelType)</c>.
@ -329,6 +343,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
}
}
/// <summary>
/// Gets a property getter delegate to get the property value from a model object.
/// </summary>
public abstract Func<object, object> PropertyGetter { get; }
/// <summary>
/// Gets a property setter delegate to set the property value on a model object.
/// </summary>
public abstract Action<object, object> PropertySetter { get; }
/// <summary>
/// Gets a display name for the model.
/// </summary>
@ -341,15 +365,5 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
{
return DisplayName ?? PropertyName ?? ModelType.Name;
}
/// <summary>
/// Gets or sets a property getter delegate to get the property value from a model object.
/// </summary>
public abstract Func<object, object> PropertyGetter { get; }
/// <summary>
/// Gets or sets a property setter delegate to set the property value on a model object.
/// </summary>
public abstract Action<object, object> PropertySetter { get; }
}
}

View File

@ -2,9 +2,14 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics;
using System.Linq;
#if DNXCORE50
using System.Reflection;
#endif
using Microsoft.Framework.Internal;
namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
@ -19,6 +24,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
private readonly DefaultMetadataDetails _details;
private ReadOnlyDictionary<object, object> _additionalValues;
private ModelMetadata _elementMetadata;
private bool _haveCalculatedElementMetadata;
private bool? _isReadOnly;
private bool? _isRequired;
private ModelPropertyCollection _properties;
@ -220,6 +227,49 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
}
}
/// <inheritdoc />
public override ModelMetadata ElementMetadata
{
get
{
if (!_haveCalculatedElementMetadata)
{
_haveCalculatedElementMetadata = true;
if (!IsCollectionType)
{
// Short-circuit checks below. If not IsCollectionType, ElementMetadata is null.
// For example, as in IsCollectionType, do not consider strings collections.
return null;
}
Type elementType = null;
if (ModelType.IsArray)
{
elementType = ModelType.GetElementType();
}
else
{
elementType = ClosedGenericMatcher.ExtractGenericInterface(ModelType, typeof(IEnumerable<>))
?.GenericTypeArguments[0];
if (elementType == null && typeof(IEnumerable).IsAssignableFrom(ModelType))
{
// ModelType implements IEnumerable but not IEnumerable<T>.
elementType = typeof(object);
}
}
Debug.Assert(
elementType != null,
$"Unable to find element type for '{ ModelType.FullName }' though IsCollectionType is true.");
// Success
_elementMetadata = _provider.GetMetadataForType(elementType);
}
return _elementMetadata;
}
}
/// <inheritdoc />
public override IEnumerable<KeyValuePair<string, string>> EnumDisplayNamesAndValues
{

View File

@ -169,11 +169,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
ExpandValidationNode(validationContext, modelExplorer);
IList<IModelValidator> validators = null;
if (modelExplorer.Metadata.IsCollectionType && modelExplorer.Model != null)
var elementMetadata = modelExplorer.Metadata.ElementMetadata;
if (elementMetadata != null)
{
var enumerableModel = (IEnumerable)modelExplorer.Model;
var elementType = GetElementType(enumerableModel.GetType());
var elementMetadata = _modelMetadataProvider.GetMetadataForType(elementType);
validators = GetValidators(validationContext.ModelValidationContext.ValidatorProvider, elementMetadata);
}
@ -283,7 +281,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
return;
}
if (!modelExplorer.Metadata.IsCollectionType)
var elementMetadata = modelExplorer.Metadata.ElementMetadata;
if (elementMetadata == null)
{
foreach (var property in validationNode.ModelMetadata.Properties)
{
@ -300,8 +299,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
else
{
var enumerableModel = (IEnumerable)modelExplorer.Model;
var elementType = GetElementType(enumerableModel.GetType());
var elementMetadata = _modelMetadataProvider.GetMetadataForType(elementType);
// An integer index is incorrect in scenarios where there is a custom index provided by the user.
// However those scenarios are supported by createing a ModelValidationNode with the right keys.
@ -321,26 +318,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
}
}
private static Type GetElementType(Type type)
{
Debug.Assert(typeof(IEnumerable).GetTypeInfo().IsAssignableFrom(type.GetTypeInfo()));
if (type.IsArray)
{
return type.GetElementType();
}
foreach (var implementedInterface in type.GetInterfaces())
{
if (implementedInterface.GetTypeInfo().IsGenericType &&
implementedInterface.GetGenericTypeDefinition() == typeof(IEnumerable<>))
{
return implementedInterface.GetGenericArguments()[0];
}
}
return typeof(object);
}
private class ValidationContext
{
public ModelValidationContext ModelValidationContext { get; set; }

View File

@ -369,7 +369,7 @@ namespace Microsoft.AspNet.Mvc
/// <param name="expressions">Expressions identifying the properties to allow for binding.</param>
/// <returns>An expression which can be used with <see cref="IPropertyBindingPredicateProvider"/>.</returns>
public static Expression<Func<ModelBindingContext, string, bool>> GetIncludePredicateExpression<TModel>(
string prefix,
string prefix,
Expression<Func<TModel, object>>[] expressions)
{
if (expressions.Length == 0)
@ -417,15 +417,15 @@ namespace Microsoft.AspNet.Mvc
{
// If modelkey is empty, we need to iterate through properties (obtained from ModelMetadata) and
// clear validation state for all entries in ModelStateDictionary that start with each property name.
// If modelkey is non-empty, clear validation state for all entries in ModelStateDictionary
// If modelkey is non-empty, clear validation state for all entries in ModelStateDictionary
// that start with modelKey
if (string.IsNullOrEmpty(modelKey))
{
var modelMetadata = metadataProvider.GetMetadataForType(modelType);
if (modelMetadata.IsCollectionType)
var elementMetadata = modelMetadata.ElementMetadata;
if (elementMetadata != null)
{
var elementType = GetElementType(modelMetadata.ModelType);
modelMetadata = metadataProvider.GetMetadataForType(elementType);
modelMetadata = elementMetadata;
}
foreach (var property in modelMetadata.Properties)
@ -440,27 +440,6 @@ namespace Microsoft.AspNet.Mvc
}
}
private static Type GetElementType(Type type)
{
Debug.Assert(typeof(IEnumerable).GetTypeInfo().IsAssignableFrom(type.GetTypeInfo()));
if (type.IsArray)
{
return type.GetElementType();
}
foreach (var implementedInterface in type.GetInterfaces())
{
if (implementedInterface.GetTypeInfo().IsGenericType &&
implementedInterface.GetGenericTypeDefinition() == typeof(IEnumerable<>))
{
return implementedInterface.GetGenericArguments()[0];
}
}
return typeof(object);
}
internal static void ValidateBindingContext([NotNull] ModelBindingContext bindingContext)
{
if (bindingContext.ModelMetadata == null)

View File

@ -4,6 +4,7 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.Text;
using Microsoft.AspNet.Mvc.Extensions;
@ -106,40 +107,42 @@ namespace Microsoft.AspNet.Mvc.Rendering
"Collection", model.GetType().FullName, typeof(IEnumerable).FullName));
}
var typeInCollection = typeof(string);
var genericEnumerableType = ClosedGenericMatcher.ExtractGenericInterface(
collection.GetType(),
typeof(IEnumerable<>));
if (genericEnumerableType != null)
var elementMetadata = htmlHelper.ViewData.ModelMetadata.ElementMetadata;
Debug.Assert(elementMetadata != null);
var typeInCollectionIsNullableValueType = elementMetadata.IsNullableValueType;
var serviceProvider = htmlHelper.ViewContext.HttpContext.RequestServices;
var metadataProvider = serviceProvider.GetRequiredService<IModelMetadataProvider>();
// Use typeof(string) instead of typeof(object) for IEnumerable collections. Neither type is Nullable<T>.
if (elementMetadata.ModelType == typeof(object))
{
typeInCollection = genericEnumerableType.GenericTypeArguments[0];
elementMetadata = metadataProvider.GetMetadataForType(typeof(string));
}
var typeInCollectionIsNullableValueType = TypeHelper.IsNullableValueType(typeInCollection);
var oldPrefix = htmlHelper.ViewData.TemplateInfo.HtmlFieldPrefix;
try
{
htmlHelper.ViewData.TemplateInfo.HtmlFieldPrefix = string.Empty;
var fieldNameBase = oldPrefix;
var result = new StringBuilder();
var serviceProvider = htmlHelper.ViewContext.HttpContext.RequestServices;
var metadataProvider = serviceProvider.GetRequiredService<IModelMetadataProvider>();
var viewEngine = serviceProvider.GetRequiredService<ICompositeViewEngine>();
var index = 0;
foreach (var item in collection)
{
var itemType = typeInCollection;
var itemMetadata = elementMetadata;
if (item != null && !typeInCollectionIsNullableValueType)
{
itemType = item.GetType();
itemMetadata = metadataProvider.GetMetadataForType(item.GetType());
}
var modelExplorer = metadataProvider.GetModelExplorerForType(itemType, item);
var modelExplorer = new ModelExplorer(
metadataProvider,
container: htmlHelper.ViewData.ModelExplorer,
metadata: itemMetadata,
model: item);
var fieldName = string.Format(CultureInfo.InvariantCulture, "{0}[{1}]", fieldNameBase, index++);
var templateBuilder = new TemplateBuilder(

View File

@ -4,6 +4,7 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.Text;
using Microsoft.AspNet.Mvc.Extensions;
@ -67,39 +68,42 @@ namespace Microsoft.AspNet.Mvc.Rendering
"Collection", model.GetType().FullName, typeof(IEnumerable).FullName));
}
var typeInCollection = typeof(string);
var genericEnumerableType = ClosedGenericMatcher.ExtractGenericInterface(
collection.GetType(),
typeof(IEnumerable<>));
if (genericEnumerableType != null)
var elementMetadata = htmlHelper.ViewData.ModelMetadata.ElementMetadata;
Debug.Assert(elementMetadata != null);
var typeInCollectionIsNullableValueType = elementMetadata.IsNullableValueType;
var serviceProvider = htmlHelper.ViewContext.HttpContext.RequestServices;
var metadataProvider = serviceProvider.GetRequiredService<IModelMetadataProvider>();
// Use typeof(string) instead of typeof(object) for IEnumerable collections. Neither type is Nullable<T>.
if (elementMetadata.ModelType == typeof(object))
{
typeInCollection = genericEnumerableType.GenericTypeArguments[0];
elementMetadata = metadataProvider.GetMetadataForType(typeof(string));
}
var typeInCollectionIsNullableValueType = TypeHelper.IsNullableValueType(typeInCollection);
var oldPrefix = viewData.TemplateInfo.HtmlFieldPrefix;
try
{
viewData.TemplateInfo.HtmlFieldPrefix = string.Empty;
var fieldNameBase = oldPrefix;
var result = new StringBuilder();
var serviceProvider = htmlHelper.ViewContext.HttpContext.RequestServices;
var metadataProvider = serviceProvider.GetRequiredService<IModelMetadataProvider>();
var viewEngine = serviceProvider.GetRequiredService<ICompositeViewEngine>();
var index = 0;
foreach (var item in collection)
{
var itemType = typeInCollection;
var itemMetadata = elementMetadata;
if (item != null && !typeInCollectionIsNullableValueType)
{
itemType = item.GetType();
itemMetadata = metadataProvider.GetMetadataForType(item.GetType());
}
var modelExplorer = metadataProvider.GetModelExplorerForType(itemType, item);
var modelExplorer = new ModelExplorer(
metadataProvider,
container: htmlHelper.ViewData.ModelExplorer,
metadata: itemMetadata,
model: item);
var fieldName = string.Format(CultureInfo.InvariantCulture, "{0}[{1}]", fieldNameBase, index++);
var templateBuilder = new TemplateBuilder(

View File

@ -34,7 +34,7 @@ namespace Microsoft.AspNet.Mvc.Rendering
/// <summary>
/// Initializes a new instance of the <see cref="DefaultHtmlGenerator"/> class.
/// </summary>
/// <param name="antiForgery">The <see cref="AntiForgery"/> instance which is used to generate anti-forgery
/// <param name="antiForgery">The <see cref="AntiForgery"/> instance which is used to generate anti-forgery
/// tokens.</param>
/// <param name="optionsAccessor">The accessor for <see cref="MvcOptions"/>.</param>
/// <param name="metadataProvider">The <see cref="IModelMetadataProvider"/>.</param>
@ -796,11 +796,19 @@ namespace Microsoft.AspNet.Mvc.Rendering
modelExplorer = modelExplorer ??
ExpressionMetadataProvider.FromStringExpression(expression, viewContext.ViewData, _metadataProvider);
var metadata = modelExplorer.Metadata;
if (allowMultiple && metadata.IsCollectionType)
{
metadata = metadata.ElementMetadata;
}
var enumNames = modelExplorer.Metadata.EnumNamesAndValues;
var isTargetEnum = modelExplorer.Metadata.IsEnum;
var innerType =
Nullable.GetUnderlyingType(modelExplorer.Metadata.ModelType) ?? modelExplorer.Metadata.ModelType;
var enumNames = metadata.EnumNamesAndValues;
var isTargetEnum = metadata.IsEnum;
// Logic below assumes isTargetEnum and enumNames are consistent. Confirm that expectation is met.
Debug.Assert(isTargetEnum ^ enumNames == null);
var innerType = Nullable.GetUnderlyingType(metadata.ModelType) ?? metadata.ModelType;
// Convert raw value collection to strings.
var currentValues = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
@ -841,13 +849,18 @@ namespace Microsoft.AspNet.Mvc.Rendering
var integerString = enumValue.ToString("d");
currentValues.Add(integerString);
// Add all simple names for this value.
var matchingNames = enumNames
.Where(kvp => string.Equals(integerString, kvp.Value, StringComparison.Ordinal))
.Select(kvp => kvp.Key);
foreach (var name in matchingNames)
// isTargetEnum may be false when raw value has a different type than the target e.g. ViewData
// contains enum values and property has type int or string.
if (isTargetEnum)
{
currentValues.Add(name);
// Add all simple names for this value.
var matchingNames = enumNames
.Where(kvp => string.Equals(integerString, kvp.Value, StringComparison.Ordinal))
.Select(kvp => kvp.Key);
foreach (var name in matchingNames)
{
currentValues.Add(name);
}
}
}
}

View File

@ -269,6 +269,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
}
}
public override ModelMetadata ElementMetadata
{
get
{
throw new NotImplementedException();
}
}
public override IEnumerable<KeyValuePair<string, string>> EnumDisplayNamesAndValues
{
get

View File

@ -2,7 +2,9 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
#if !DNXCORE50
using Moq;
@ -52,6 +54,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
Assert.Null(metadata.DisplayFormatString);
Assert.Null(metadata.DisplayName);
Assert.Null(metadata.EditFormatString);
Assert.Null(metadata.ElementMetadata);
Assert.Null(metadata.EnumDisplayNamesAndValues);
Assert.Null(metadata.EnumNamesAndValues);
Assert.Null(metadata.NullDisplayText);
@ -106,6 +109,69 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
Assert.Equal(typeof(Exception), metadata.ContainerType);
}
[Theory]
[InlineData(typeof(object))]
[InlineData(typeof(int))]
[InlineData(typeof(NonCollectionType))]
[InlineData(typeof(string))]
public void ElementMetadata_ReturnsNull_ForNonCollections(Type modelType)
{
// Arrange
var provider = new EmptyModelMetadataProvider();
var detailsProvider = new EmptyCompositeMetadataDetailsProvider();
var key = ModelMetadataIdentity.ForType(modelType);
var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0]));
var metadata = new DefaultModelMetadata(provider, detailsProvider, cache);
// Act
var elementMetadata = metadata.ElementMetadata;
// Assert
Assert.Null(elementMetadata);
}
[Theory]
[InlineData(typeof(int[]), typeof(int))]
[InlineData(typeof(List<string>), typeof(string))]
[InlineData(typeof(DerivedList), typeof(int))]
[InlineData(typeof(IEnumerable), typeof(object))]
[InlineData(typeof(IEnumerable<string>), typeof(string))]
[InlineData(typeof(Collection<int>), typeof(int))]
[InlineData(typeof(Dictionary<object, object>), typeof(KeyValuePair<object, object>))]
[InlineData(typeof(DerivedDictionary), typeof(KeyValuePair<string, int>))]
public void ElementMetadata_ReturnsExpectedMetadata(Type modelType, Type elementType)
{
// Arrange
var provider = new EmptyModelMetadataProvider();
var detailsProvider = new EmptyCompositeMetadataDetailsProvider();
var key = ModelMetadataIdentity.ForType(modelType);
var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0]));
var metadata = new DefaultModelMetadata(provider, detailsProvider, cache);
// Act
var elementMetadata = metadata.ElementMetadata;
// Assert
Assert.NotNull(elementMetadata);
Assert.Equal(elementType, elementMetadata.ModelType);
}
private class NonCollectionType
{
}
private class DerivedList : List<int>
{
}
private class DerivedDictionary : Dictionary<string, int>
{
}
[Theory]
[InlineData(typeof(string))]
[InlineData(typeof(IDisposable))]
@ -160,7 +226,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
var expectedProperties = new DefaultModelMetadata[]
{
new DefaultModelMetadata(
provider.Object,
provider.Object,
detailsProvider,
new DefaultMetadataDetails(
ModelMetadataIdentity.ForProperty(typeof(int), "Prop1", typeof(string)),

View File

@ -874,7 +874,7 @@ namespace Microsoft.AspNet.Mvc.Rendering
Assert.Equal(expectedHtml, html.ToString());
}
[Fact(Skip = "#2664, throws ArgumentNullException")]
[Fact]
public void ListBoxNotInTemplate_GetsViewDataEntry_IfModelStateEmpty()
{
// Arrange
@ -899,7 +899,7 @@ namespace Microsoft.AspNet.Mvc.Rendering
Assert.Equal(expectedHtml, html.ToString());
}
[Fact(Skip = "#2664, throws ArgumentNullException")]
[Fact(Skip = "#1487, incorrectly matches Property1 entry (without prefix) in ViewData.")]
public void ListBoxInTemplate_GetsViewDataEntry_IfModelStateEmpty()
{
// Arrange
@ -927,7 +927,7 @@ namespace Microsoft.AspNet.Mvc.Rendering
Assert.Equal(expectedHtml, html.ToString());
}
[Fact(Skip = "#2664, throws ArgumentNullException")]
[Fact(Skip = "#1487, incorrectly matches Property1 entry (without prefix) in ViewData.")]
public void ListBoxInTemplate_GetsPropertyOfViewDataEntry_IfModelStateEmptyAndNoViewDataEntryWithPrefix()
{
// Arrange
@ -954,7 +954,7 @@ namespace Microsoft.AspNet.Mvc.Rendering
Assert.Equal(expectedHtml, html.ToString());
}
[Fact(Skip = "#2664, throws ArgumentNullException")]
[Fact]
public void ListBoxNotInTemplate_GetsPropertyOfModel_IfModelStateAndViewDataEmpty()
{
// Arrange
@ -970,7 +970,7 @@ namespace Microsoft.AspNet.Mvc.Rendering
Assert.Equal(expectedHtml, html.ToString());
}
[Fact(Skip = "#2664, throws ArgumentNullException")]
[Fact]
public void ListBoxInTemplate_GetsPropertyOfModel_IfModelStateAndViewDataEmpty()
{
// Arrange