diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelMetadata.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelMetadata.cs index 8e77c345fc..6eeb98510b 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelMetadata.cs @@ -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; } /// - /// Gets the ordered display names and values of all values in or - /// Nullable.GetUnderlyingType(ModelType). + /// Gets the ordered display names and values of all values in + /// . /// /// /// An of mappings between field names @@ -149,8 +147,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public abstract IEnumerable> EnumDisplayNamesAndValues { get; } /// - /// Gets the names and values of all values in or - /// Nullable.GetUnderlyingType(ModelType). + /// Gets the names and values of all values in . /// /// /// An of mappings between field names @@ -204,23 +201,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public abstract bool IsBindingRequired { get; } /// - /// Gets a value indicating whether or Nullable.GetUnderlyingType(ModelType) is - /// for an . + /// Gets a value indicating whether is for an . /// /// /// true if type.IsEnum (type.GetTypeInfo().IsEnum for DNX Core 5.0) is true for - /// or Nullable.GetUnderlyingType(ModelType); false otherwise. + /// ; false otherwise. /// public abstract bool IsEnum { get; } /// - /// Gets a value indicating whether or Nullable.GetUnderlyingType(ModelType) is - /// for an with an associated . + /// Gets a value indicating whether is for an with an + /// associated . /// /// - /// true if is true and or - /// Nullable.GetUnderlyingType(ModelType) has an associated ; false - /// otherwise. + /// true if is true and has an + /// associated ; false otherwise. /// public abstract bool IsFlagsEnum { get; } @@ -346,6 +341,32 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } } + /// + /// Gets a value indicating whether or not allows null values. + /// + public bool IsReferenceOrNullableType + { + get + { + return !ModelType.GetTypeInfo().IsValueType || IsNullableValueType; + } + } + + /// + /// Gets the underlying type argument if inherits from . + /// Otherwise gets . + /// + /// + /// Identical to unless is true. + /// + public Type UnderlyingOrModelType + { + get + { + return Nullable.GetUnderlyingType(ModelType) ?? ModelType; + } + } + /// /// Gets a property getter delegate to get the property value from a model object. /// diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs index daeab334ec..12b96429fe 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs @@ -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; - } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs index 28d3e1a695..9d9723315c 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs @@ -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 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; - } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/DisplayMetadata.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/DisplayMetadata.cs index d6ecfd9251..707bb0d07c 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/DisplayMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/DisplayMetadata.cs @@ -58,14 +58,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata /// /// Gets the ordered display names and values of all values in - /// or Nullable.GetUnderlyingType(ModelType). See + /// . See /// . /// public IEnumerable> EnumDisplayNamesAndValues { get; set; } /// - /// Gets the names and values of all values in - /// or Nullable.GetUnderlyingType(ModelType). See . + /// Gets the names and values of all values in + /// . See . /// // This could be implemented in DefaultModelMetadata. But value should be cached. public IReadOnlyDictionary EnumNamesAndValues { get; set; } @@ -89,17 +89,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata public bool HtmlEncode { get; set; } = true; /// - /// Gets a value indicating whether or - /// Nullable.GetUnderlyingType(ModelType) is for an . See - /// . + /// Gets a value indicating whether is for an + /// . See . /// // This could be implemented in DefaultModelMetadata. But value is needed in the details provider. public bool IsEnum { get; set; } /// - /// Gets a value indicating whether or - /// Nullable.GetUnderlyingType(ModelType) is for an with an associated - /// . See . + /// Gets a value indicating whether is for an + /// with an associated . See + /// . /// // This could be implemented in DefaultModelMetadata. But value is needed in the details provider. public bool IsFlagsEnum { get; set; } diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/SimpleTypeModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/SimpleTypeModelBinder.cs index db83cb5cb8..6a96ec335c 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/SimpleTypeModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/SimpleTypeModelBinder.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.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; - } } } diff --git a/src/Microsoft.AspNet.Mvc.DataAnnotations/NumericClientModelValidatorProvider.cs b/src/Microsoft.AspNet.Mvc.DataAnnotations/NumericClientModelValidatorProvider.cs index 5863911357..fb485aaef0 100644 --- a/src/Microsoft.AspNet.Mvc.DataAnnotations/NumericClientModelValidatorProvider.cs +++ b/src/Microsoft.AspNet.Mvc.DataAnnotations/NumericClientModelValidatorProvider.cs @@ -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 { /// @@ -15,8 +12,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation /// 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) || diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/InputTagHelper.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/InputTagHelper.cs index 71a20900ef..1e4ab385ff 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/InputTagHelper.cs +++ b/src/Microsoft.AspNet.Mvc.TagHelpers/InputTagHelper.cs @@ -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)) diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/Html/DefaultHtmlGenerator.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/Html/DefaultHtmlGenerator.cs index be877b4951..5eaca77bec 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/Html/DefaultHtmlGenerator.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/Html/DefaultHtmlGenerator.cs @@ -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(StringComparer.OrdinalIgnoreCase); diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/Internal/TemplateRenderer.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/Internal/TemplateRenderer.cs index 0c792268d6..428a820e5f 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/Internal/TemplateRenderer.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/Internal/TemplateRenderer.cs @@ -146,10 +146,8 @@ namespace Microsoft.AspNet.Mvc.Rendering.Internal // We don't want to search for Nullable, we want to search for T (which should handle both T and // Nullable). - 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; } diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewDataDictionary.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewDataDictionary.cs index e1346f0662..1a90f6701a 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewDataDictionary.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewDataDictionary.cs @@ -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 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 } } diff --git a/test/Microsoft.AspNet.Mvc.Abstractions.Test/ModelBinding/ModelMetadataTest.cs b/test/Microsoft.AspNet.Mvc.Abstractions.Test/ModelBinding/ModelMetadataTest.cs index 7e13d12f4b..4f028d9e6a 100644 --- a/test/Microsoft.AspNet.Mvc.Abstractions.Test/ModelBinding/ModelMetadataTest.cs +++ b/test/Microsoft.AspNet.Mvc.Abstractions.Test/ModelBinding/ModelMetadataTest.cs @@ -21,7 +21,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding [InlineData(typeof(string))] [InlineData(typeof(Nullable))] [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))] - 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 + { + } + [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))] [InlineData(typeof(Collection))] [InlineData(typeof(Dictionary))] - 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 - { - } - // IsNullableValueType [Theory] @@ -102,7 +103,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding [InlineData(typeof(IDisposable), false)] [InlineData(typeof(Nullable), true)] [InlineData(typeof(int), false)] - public void IsNullableValueTypeTests(Type modelType, bool expected) + [InlineData(typeof(DerivedList), false)] + [InlineData(typeof(IsComplexTypeModel), false)] + [InlineData(typeof(Nullable), 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), true)] + [InlineData(typeof(int), false)] + [InlineData(typeof(DerivedList), true)] + [InlineData(typeof(IsComplexTypeModel), false)] + [InlineData(typeof(Nullable), 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), typeof(int))] + [InlineData(typeof(int), typeof(int))] + [InlineData(typeof(DerivedList), typeof(DerivedList))] + [InlineData(typeof(IsComplexTypeModel), typeof(IsComplexTypeModel))] + [InlineData(typeof(Nullable), 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();