Add `IsReferenceOrNullableType` and `UnderlyingOrModelType` to `ModelMetadata`

- #2992
- use new properties to replace common helper methods
- still a few `Nullable.GetUnderlyingType()` calls
  - creating `ModelMetadata` or sites lacking `ModelMetadata` access e.g. `ModelBindingHelper.ConvertTo()`
This commit is contained in:
Doug Bunting 2015-08-22 14:02:24 -07:00
parent 0beb39ec1c
commit bf7e0f141e
11 changed files with 124 additions and 96 deletions

View File

@ -5,9 +5,7 @@ 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;
@ -139,8 +137,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
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>.
/// Gets the ordered display names and values of all <see cref="Enum"/> values in
/// <see cref="UnderlyingOrModelType"/>.
/// </summary>
/// <value>
/// An <see cref="IEnumerable{KeyValuePair{string, string}}"/> of mappings between <see cref="Enum"/> field names
@ -149,8 +147,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
public abstract IEnumerable<KeyValuePair<string, string>> EnumDisplayNamesAndValues { get; }
/// <summary>
/// Gets the names and values of all <see cref="Enum"/> values in <see cref="ModelType"/> or
/// <c>Nullable.GetUnderlyingType(ModelType)</c>.
/// Gets the names and values of all <see cref="Enum"/> values in <see cref="UnderlyingOrModelType"/>.
/// </summary>
/// <value>
/// An <see cref="IReadOnlyDictionary{string, string}"/> of mappings between <see cref="Enum"/> field names
@ -204,23 +201,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
public abstract bool IsBindingRequired { get; }
/// <summary>
/// Gets a value indicating whether <see cref="ModelType"/> or <c>Nullable.GetUnderlyingType(ModelType)</c> is
/// for an <see cref="Enum"/>.
/// Gets a value indicating whether <see cref="UnderlyingOrModelType"/> is for an <see cref="Enum"/>.
/// </summary>
/// <value>
/// <c>true</c> if <c>type.IsEnum</c> (<c>type.GetTypeInfo().IsEnum</c> for DNX Core 5.0) is <c>true</c> for
/// <see cref="ModelType"/> or <c>Nullable.GetUnderlyingType(ModelType)</c>; <c>false</c> otherwise.
/// <see cref="UnderlyingOrModelType"/>; <c>false</c> otherwise.
/// </value>
public abstract bool IsEnum { get; }
/// <summary>
/// Gets a value indicating whether <see cref="ModelType"/> or <c>Nullable.GetUnderlyingType(ModelType)</c> is
/// for an <see cref="Enum"/> with an associated <see cref="FlagsAttribute"/>.
/// Gets a value indicating whether <see cref="UnderlyingOrModelType"/> is for an <see cref="Enum"/> with an
/// associated <see cref="FlagsAttribute"/>.
/// </summary>
/// <value>
/// <c>true</c> if <see cref="IsEnum"/> is <c>true</c> and <see cref="ModelType"/> or
/// <c>Nullable.GetUnderlyingType(ModelType)</c> has an associated <see cref="FlagsAttribute"/>; <c>false</c>
/// otherwise.
/// <c>true</c> if <see cref="IsEnum"/> is <c>true</c> and <see cref="UnderlyingOrModelType"/> has an
/// associated <see cref="FlagsAttribute"/>; <c>false</c> otherwise.
/// </value>
public abstract bool IsFlagsEnum { get; }
@ -346,6 +341,32 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
}
}
/// <summary>
/// Gets a value indicating whether or not <see cref="ModelType"/> allows <c>null</c> values.
/// </summary>
public bool IsReferenceOrNullableType
{
get
{
return !ModelType.GetTypeInfo().IsValueType || IsNullableValueType;
}
}
/// <summary>
/// Gets the underlying type argument if <see cref="ModelType"/> inherits from <see cref="Nullable{T}"/>.
/// Otherwise gets <see cref="ModelType"/>.
/// </summary>
/// <remarks>
/// Identical to <see cref="ModelType"/> unless <see cref="IsNullableValueType"/> is <c>true</c>.
/// </remarks>
public Type UnderlyingOrModelType
{
get
{
return Nullable.GetUnderlyingType(ModelType) ?? ModelType;
}
}
/// <summary>
/// Gets a property getter delegate to get the property value from a model object.
/// </summary>

View File

@ -127,8 +127,11 @@ namespace Microsoft.AspNet.Mvc
var source = property.Value;
if (propertyHelper.Property.CanWrite && propertyHelper.Property.SetMethod?.IsPublic == true)
{
// Handle settable property. Do not set the property if the type is a non-nullable type.
if (source != null || AllowsNullValue(propertyType))
// Handle settable property.
var metadata = _modelMetadataProvider.GetMetadataForType(propertyType);
// Do not set the property to null if the type is a non-nullable type.
if (source != null || metadata.IsReferenceOrNullableType)
{
propertyHelper.SetValue(controller, source);
}
@ -218,10 +221,5 @@ namespace Microsoft.AspNet.Mvc
ValueProvider = bindingContext.ValueProvider,
};
}
private static bool AllowsNullValue([NotNull] Type type)
{
return !type.GetTypeInfo().IsValueType || Nullable.GetUnderlyingType(type) != null;
}
}
}

View File

@ -7,7 +7,9 @@ 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
@ -407,8 +409,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
}
else
{
// Default to IsRequired = true for value types.
_isRequired = !AllowsNullValue(ModelType);
// Default to IsRequired = true for non-Nullable<T> value types.
_isRequired = !IsReferenceOrNullableType;
}
}
@ -526,10 +528,5 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
return _details.PropertySetter;
}
}
private static bool AllowsNullValue([NotNull] Type type)
{
return !type.GetTypeInfo().IsValueType || Nullable.GetUnderlyingType(type) != null;
}
}
}

View File

@ -58,14 +58,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
/// <summary>
/// Gets the ordered display names and values of all <see cref="System.Enum"/> values in
/// <see cref="ModelMetadata.ModelType"/> or <c>Nullable.GetUnderlyingType(ModelType)</c>. See
/// <see cref="ModelMetadata.UnderlyingOrModelType"/>. See
/// <see cref="ModelMetadata.EnumDisplayNamesAndValues"/>.
/// </summary>
public IEnumerable<KeyValuePair<string, string>> EnumDisplayNamesAndValues { get; set; }
/// <summary>
/// Gets the names and values of all <see cref="System.Enum"/> values in <see cref="ModelMetadata.ModelType"/>
/// or <c>Nullable.GetUnderlyingType(ModelType)</c>. See <see cref="ModelMetadata.EnumNamesAndValues"/>.
/// Gets the names and values of all <see cref="System.Enum"/> values in
/// <see cref="ModelMetadata.UnderlyingOrModelType"/>. See <see cref="ModelMetadata.EnumNamesAndValues"/>.
/// </summary>
// This could be implemented in DefaultModelMetadata. But value should be cached.
public IReadOnlyDictionary<string, string> EnumNamesAndValues { get; set; }
@ -89,17 +89,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
public bool HtmlEncode { get; set; } = true;
/// <summary>
/// Gets a value indicating whether <see cref="ModelMetadata.ModelType"/> or
/// <c>Nullable.GetUnderlyingType(ModelType)</c> is for an <see cref="System.Enum"/>. See
/// <see cref="ModelMetadata.IsEnum"/>.
/// Gets a value indicating whether <see cref="ModelMetadata.UnderlyingOrModelType"/> is for an
/// <see cref="System.Enum"/>. See <see cref="ModelMetadata.IsEnum"/>.
/// </summary>
// This could be implemented in DefaultModelMetadata. But value is needed in the details provider.
public bool IsEnum { get; set; }
/// <summary>
/// Gets a value indicating whether <see cref="ModelMetadata.ModelType"/> or
/// <c>Nullable.GetUnderlyingType(ModelType)</c> is for an <see cref="System.Enum"/> with an associated
/// <see cref="System.FlagsAttribute"/>. See <see cref="ModelMetadata.IsFlagsEnum"/>.
/// Gets a value indicating whether <see cref="ModelMetadata.UnderlyingOrModelType"/> is for an
/// <see cref="System.Enum"/> with an associated <see cref="System.FlagsAttribute"/>. See
/// <see cref="ModelMetadata.IsFlagsEnum"/>.
/// </summary>
// This could be implemented in DefaultModelMetadata. But value is needed in the details provider.
public bool IsFlagsEnum { get; set; }

View File

@ -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.Reflection;
using System.Threading.Tasks;
using Microsoft.AspNet.Mvc.Core;
@ -51,7 +50,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
// When converting newModel a null value may indicate a failed conversion for an otherwise required
// model (can't set a ValueType to null). This detects if a null model value is acceptable given the
// current bindingContext. If not, an error is logged.
if (model == null && !AllowsNullValue(bindingContext.ModelType))
if (model == null && !bindingContext.ModelMetadata.IsReferenceOrNullableType)
{
bindingContext.ModelState.TryAddModelError(
bindingContext.ModelName,
@ -83,10 +82,5 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
key: bindingContext.ModelName,
isModelSet: false);
}
private static bool AllowsNullValue(Type type)
{
return !type.GetTypeInfo().IsValueType || Nullable.GetUnderlyingType(type) != null;
}
}
}

View File

@ -1,9 +1,6 @@
// 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.Collections.Generic;
namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
{
/// <summary>
@ -15,8 +12,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
/// <inheritdoc />
public void GetValidators(ClientValidatorProviderContext context)
{
var type = context.ModelMetadata.ModelType;
var typeToValidate = Nullable.GetUnderlyingType(type) ?? type;
var typeToValidate = context.ModelMetadata.UnderlyingOrModelType;
// Check only the numeric types for which we set type='text'.
if (typeToValidate == typeof(float) ||

View File

@ -386,11 +386,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
var fieldType = modelExplorer.ModelType;
if (typeof(bool?) != fieldType)
{
var underlyingType = Nullable.GetUnderlyingType(fieldType);
if (underlyingType != null)
{
fieldType = underlyingType;
}
fieldType = modelExplorer.Metadata.UnderlyingOrModelType;
}
foreach (string typeName in TemplateRenderer.GetTypeNames(modelExplorer.Metadata, fieldType))

View File

@ -8,7 +8,6 @@ using System.Diagnostics;
using System.Globalization;
using System.Linq;
using System.Reflection;
using System.Text;
using Microsoft.AspNet.Antiforgery;
using Microsoft.AspNet.Html.Abstractions;
using Microsoft.AspNet.Mvc.ModelBinding;
@ -838,7 +837,7 @@ namespace Microsoft.AspNet.Mvc.Rendering
// 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;
var innerType = metadata.UnderlyingOrModelType;
// Convert raw value collection to strings.
var currentValues = new HashSet<string>(StringComparer.OrdinalIgnoreCase);

View File

@ -146,10 +146,8 @@ namespace Microsoft.AspNet.Mvc.Rendering.Internal
// We don't want to search for Nullable<T>, we want to search for T (which should handle both T and
// Nullable<T>).
var modelType = _viewData.ModelExplorer.ModelType;
var fieldType = Nullable.GetUnderlyingType(modelType) ?? modelType;
foreach (var typeName in GetTypeNames(_viewData.ModelExplorer.Metadata, fieldType))
var fieldType = metadata.UnderlyingOrModelType;
foreach (var typeName in GetTypeNames(metadata, fieldType))
{
yield return typeName;
}

View File

@ -5,7 +5,9 @@ using System;
using System.Collections;
using System.Collections.Generic;
using System.Globalization;
#if DNXCORE50
using System.Reflection;
#endif
using Microsoft.AspNet.Mvc.ModelBinding;
using Microsoft.AspNet.Mvc.Rendering.Expressions;
using Microsoft.AspNet.Mvc.ViewFeatures;
@ -186,8 +188,7 @@ namespace Microsoft.AspNet.Mvc
{
// This is the core constructor called when Model is known.
var modelType = GetModelType(model);
var metadataModelType =
Nullable.GetUnderlyingType(source.ModelMetadata.ModelType) ?? source.ModelMetadata.ModelType;
var metadataModelType = source.ModelMetadata.UnderlyingOrModelType;
if (modelType == metadataModelType && model == source.ModelExplorer.Model)
{
// Preserve any customizations made to source.ModelExplorer.ModelMetadata if the Type
@ -260,7 +261,7 @@ namespace Microsoft.AspNet.Mvc
public TemplateInfo TemplateInfo { get; }
#region IDictionary properties
#region IDictionary properties
// Do not just pass through to _data: Indexer should not throw a KeyNotFoundException.
public object this[string index]
{
@ -295,7 +296,7 @@ namespace Microsoft.AspNet.Mvc
{
get { return _data.Values; }
}
#endregion
#endregion
// for unit testing
internal IDictionary<string, object> Data
@ -383,13 +384,13 @@ namespace Microsoft.AspNet.Mvc
EnsureCompatible(value);
// Reset or override ModelMetadata based on runtime value type. Fall back to declared type if value is
// null. When called from the Model setter, ModelMetadata will (temporarily) be null. When called from
// a constructor, current ModelMetadata may already be set to preserve customizations made in parent scope.
// null. When called from a constructor, current ModelExplorer may already be set to preserve
// customizations made in parent scope. But ModelExplorer is never null after instance is initialized.
var modelType = GetModelType(value);
Type metadataModelType = null;
if (ModelExplorer != null)
{
metadataModelType = Nullable.GetUnderlyingType(ModelMetadata.ModelType) ?? ModelMetadata.ModelType;
metadataModelType = ModelMetadata.UnderlyingOrModelType;
}
if (metadataModelType != modelType)
@ -413,7 +414,7 @@ namespace Microsoft.AspNet.Mvc
{
// IsCompatibleObject verifies if the value is either an instance of _declaredModelType or (if value is
// null) that _declaredModelType is a nullable type.
var castWillSucceed = IsCompatibleWith(_declaredModelType, value);
var castWillSucceed = IsCompatibleWithDeclaredType(value);
if (!castWillSucceed)
{
string message;
@ -435,19 +436,20 @@ namespace Microsoft.AspNet.Mvc
return (value == null) ? _declaredModelType : value.GetType();
}
private static bool IsCompatibleWith([NotNull] Type type, object value)
private bool IsCompatibleWithDeclaredType(object value)
{
if (value == null)
{
return !type.GetTypeInfo().IsValueType || Nullable.GetUnderlyingType(type) != null;
// In this case ModelMetadata.ModelType matches _declaredModelType.
return ModelMetadata.IsReferenceOrNullableType;
}
else
{
return type.GetTypeInfo().IsAssignableFrom(value.GetType().GetTypeInfo());
return _declaredModelType.IsAssignableFrom(value.GetType());
}
}
#region IDictionary methods
#region IDictionary methods
public void Add([NotNull] string key, object value)
{
_data.Add(key, value);
@ -502,6 +504,6 @@ namespace Microsoft.AspNet.Mvc
{
return _data.GetEnumerator();
}
#endregion
#endregion
}
}

View File

@ -21,7 +21,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
[InlineData(typeof(string))]
[InlineData(typeof(Nullable<int>))]
[InlineData(typeof(int))]
public void IsComplexTypeTestsReturnsFalseForSimpleTypes(Type type)
public void IsComplexType_ReturnsFalseForSimpleTypes(Type type)
{
// Arrange
var provider = new EmptyModelMetadataProvider();
@ -38,7 +38,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
[InlineData(typeof(IDisposable))]
[InlineData(typeof(IsComplexTypeModel))]
[InlineData(typeof(Nullable<IsComplexTypeModel>))]
public void IsComplexTypeTestsReturnsTrueForComplexTypes(Type type)
public void IsComplexType_ReturnsTrueForComplexTypes(Type type)
{
// Arrange
var provider = new EmptyModelMetadataProvider();
@ -50,12 +50,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
Assert.True(modelMetadata.IsComplexType);
}
// IsCollectionType
private class NonCollectionType
{
}
private class DerivedList : List<int>
{
}
[Theory]
[InlineData(typeof(object))]
[InlineData(typeof(int))]
[InlineData(typeof(NonCollectionType))]
[InlineData(typeof(string))]
public void IsCollectionType_NonCollectionTypes(Type type)
public void IsCollectionType_ReturnsFalseForNonCollectionTypes(Type type)
{
// Arrange
var provider = new EmptyModelMetadataProvider();
@ -75,7 +84,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
[InlineData(typeof(IEnumerable<string>))]
[InlineData(typeof(Collection<int>))]
[InlineData(typeof(Dictionary<object, object>))]
public void IsCollectionType_CollectionTypes(Type type)
public void IsCollectionType_ReturnsTrueForCollectionTypes(Type type)
{
// Arrange
var provider = new EmptyModelMetadataProvider();
@ -87,14 +96,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
Assert.True(modelMetadata.IsCollectionType);
}
private class NonCollectionType
{
}
private class DerivedList : List<int>
{
}
// IsNullableValueType
[Theory]
@ -102,7 +103,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
[InlineData(typeof(IDisposable), false)]
[InlineData(typeof(Nullable<int>), true)]
[InlineData(typeof(int), false)]
public void IsNullableValueTypeTests(Type modelType, bool expected)
[InlineData(typeof(DerivedList), false)]
[InlineData(typeof(IsComplexTypeModel), false)]
[InlineData(typeof(Nullable<IsComplexTypeModel>), true)]
public void IsNullableValueType_ReturnsExpectedValue(Type modelType, bool expected)
{
// Arrange
var modelMetadata = new TestModelMetadata(modelType);
@ -111,18 +115,42 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
Assert.Equal(expected, modelMetadata.IsNullableValueType);
}
private class Class1
// IsReferenceOrNullableType
[Theory]
[InlineData(typeof(string), true)]
[InlineData(typeof(IDisposable), true)]
[InlineData(typeof(Nullable<int>), true)]
[InlineData(typeof(int), false)]
[InlineData(typeof(DerivedList), true)]
[InlineData(typeof(IsComplexTypeModel), false)]
[InlineData(typeof(Nullable<IsComplexTypeModel>), true)]
public void IsReferenceOrNullableType_ReturnsExpectedValue(Type modelType, bool expected)
{
public string Prop1 { get; set; }
public override string ToString()
{
return "Class1";
}
// Arrange
var modelMetadata = new TestModelMetadata(modelType);
// Act & Assert
Assert.Equal(expected, modelMetadata.IsReferenceOrNullableType);
}
private class Class2
// UnderlyingOrModelType
[Theory]
[InlineData(typeof(string), typeof(string))]
[InlineData(typeof(IDisposable), typeof(IDisposable))]
[InlineData(typeof(Nullable<int>), typeof(int))]
[InlineData(typeof(int), typeof(int))]
[InlineData(typeof(DerivedList), typeof(DerivedList))]
[InlineData(typeof(IsComplexTypeModel), typeof(IsComplexTypeModel))]
[InlineData(typeof(Nullable<IsComplexTypeModel>), typeof(IsComplexTypeModel))]
public void UnderlyingOrModelType_ReturnsExpectedValue(Type modelType, Type expected)
{
public int Prop2 { get; set; }
// Arrange
var modelMetadata = new TestModelMetadata(modelType);
// Act & Assert
Assert.Equal(expected, modelMetadata.UnderlyingOrModelType);
}
// GetDisplayName()
@ -143,7 +171,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
}
[Fact]
public void ReturnsPropertyNameWhenSetAndDisplayNameIsNull()
public void GetDisplayName_ReturnsPropertyName_WhenSetAndDisplayNameIsNull()
{
// Arrange
var provider = new EmptyModelMetadataProvider();
@ -157,7 +185,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
}
[Fact]
public void ReturnsTypeNameWhenPropertyNameAndDisplayNameAreNull()
public void GetDisplayName_ReturnsTypeName_WhenPropertyNameAndDisplayNameAreNull()
{
// Arrange
var provider = new EmptyModelMetadataProvider();