From 0549769fd0e467d6f6c96ef59b32c7700461336f Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Thu, 12 Feb 2015 21:42:41 -0800 Subject: [PATCH] Set `ModelMetadata.TemplateHint` based on data annotations - add `CachedDataAnnotationsMetadataAttributes.UIHint` - set `ModelMetadata.TemplateHint` using `UIHintAttribute` or `HiddenInputAttribute` - add doc comments for `TemplateHint`-related properties and methods - add unit tests and use these attributes in functional tests nits: - cache and seal `CachedModelMetadata.IsCollectionType` - correct doc comments for `ModelMetadata.RealModelType` - add doc comments for `IsCollectionType`-related properties and methods - add doc comments for `IsComplexType`-related properties and methods - move `CachedModelMetadata.IsComplexType` right below `IsCollectionType` - same for related fields and methods --- ...CachedDataAnnotationsMetadataAttributes.cs | 11 +- .../CachedDataAnnotationsModelMetadata.cs | 29 ++++- .../Metadata/CachedModelMetadata.cs | 104 ++++++++++++++---- .../Metadata/ModelMetadata.cs | 23 +++- ...edDataAnnotationsMetadataAttributesTest.cs | 3 + ...ataAnnotationsModelMetadataProviderTest.cs | 34 +++++- .../CachedDataAnnotationsModelMetadataTest.cs | 32 +++++- .../MvcTagHelpersWebSite/Models/Person.cs | 3 + .../MvcTagHelper_Home/EditWarehouse.cshtml | 2 +- ...er.cshtml => GenderUsingTagHelpers.cshtml} | 0 10 files changed, 210 insertions(+), 31 deletions(-) rename test/WebSites/MvcTagHelpersWebSite/Views/MvcTagHelper_Home/EditorTemplates/{Gender.cshtml => GenderUsingTagHelpers.cshtml} (100%) diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedDataAnnotationsMetadataAttributes.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedDataAnnotationsMetadataAttributes.cs index cf7cdbc92f..d463ef502f 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedDataAnnotationsMetadataAttributes.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedDataAnnotationsMetadataAttributes.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Open Technologies, Inc. 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; using System.ComponentModel.DataAnnotations; using System.Linq; @@ -20,6 +19,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding HiddenInput = attributes.OfType().FirstOrDefault(); Required = attributes.OfType().FirstOrDefault(); ScaffoldColumn = attributes.OfType().FirstOrDefault(); + UIHint = attributes.OfType().FirstOrDefault(); + BinderMetadata = attributes.OfType().FirstOrDefault(); PropertyBindingPredicateProviders = attributes.OfType(); BinderModelNameProvider = attributes.OfType().FirstOrDefault(); @@ -82,7 +83,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public HiddenInputAttribute HiddenInput { get; protected set; } /// - /// Gets (or sets in subclasses) found in + /// Gets (or sets in subclasses) found in /// collection passed to the /// constructor, if any. /// @@ -91,5 +92,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public RequiredAttribute Required { get; protected set; } public ScaffoldColumnAttribute ScaffoldColumn { get; protected set; } + + /// + /// Gets (or sets in subclasses) found in collection passed to the + /// constructor, if any. + /// + public UIHintAttribute UIHint { get; protected set; } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedDataAnnotationsModelMetadata.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedDataAnnotationsModelMetadata.cs index 3448715089..3220d35f60 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedDataAnnotationsModelMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedDataAnnotationsModelMetadata.cs @@ -9,8 +9,8 @@ using System.Reflection; namespace Microsoft.AspNet.Mvc.ModelBinding { - // Class does not override ComputeIsComplexType() because value calculated in ModelMetadata's base implementation - // is correct. + // Class does not override ComputeIsCollectionType() or ComputeIsComplexType() because values calculated in + // ModelMetadata's base implementation are correct. No data annotations override those calculations. public class CachedDataAnnotationsModelMetadata : CachedModelMetadata { private static readonly string HtmlName = DataType.Html.ToString(); @@ -333,6 +333,31 @@ namespace Microsoft.AspNet.Mvc.ModelBinding : base.ComputeShowForEdit(); } + /// + /// Calculate the value based on presence of a + /// and its value or presence of a + /// when no exists. + /// + /// + /// Calculated value. if a + /// exists. "HiddenInput" if a exists + /// and no exists. null otherwise. + /// + protected override string ComputeTemplateHint() + { + if (PrototypeCache.UIHint != null) + { + return PrototypeCache.UIHint.UIHint; + } + + if (PrototypeCache.HiddenInput != null) + { + return "HiddenInput"; + } + + return base.ComputeTemplateHint(); + } + private static void ValidateDisplayColumnAttribute(DisplayColumnAttribute displayColumnAttribute, PropertyInfo displayColumnProperty, Type modelType) { diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedModelMetadata.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedModelMetadata.cs index fb41d9df14..3e5e6e094a 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedModelMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedModelMetadata.cs @@ -25,13 +25,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding private bool _hasNonDefaultEditFormat; private bool _hideSurroundingHtml; private bool _htmlEncode; - private bool _isReadOnly; + private bool _isCollectionType; private bool _isComplexType; + private bool _isReadOnly; private bool _isRequired; private string _nullDisplayText; private int _order; private bool _showForDisplay; private bool _showForEdit; + private string _templateHint; private IBinderMetadata _binderMetadata; private string _binderModelName; private IPropertyBindingPredicateProvider _propertyBindingPredicateProvider; @@ -46,13 +48,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding private bool _hasNonDefaultEditFormatComputed; private bool _hideSurroundingHtmlComputed; private bool _htmlEncodeComputed; - private bool _isReadOnlyComputed; + private bool _isCollectionTypeComputed; private bool _isComplexTypeComputed; + private bool _isReadOnlyComputed; private bool _isRequiredComputed; private bool _nullDisplayTextComputed; private bool _orderComputed; private bool _showForDisplayComputed; private bool _showForEditComputed; + private bool _templateHintComputed; private bool _isBinderMetadataComputed; private bool _isBinderModelNameComputed; private bool _isBinderTypeComputed; @@ -320,6 +324,35 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } } + /// + public sealed override bool IsCollectionType + { + get + { + if (!_isCollectionTypeComputed) + { + _isCollectionType = ComputeIsCollectionType(); + _isCollectionTypeComputed = true; + } + + return _isCollectionType; + } + } + + /// + public sealed override bool IsComplexType + { + get + { + if (!_isComplexTypeComputed) + { + _isComplexType = ComputeIsComplexType(); + _isComplexTypeComputed = true; + } + return _isComplexType; + } + } + /// public sealed override bool IsReadOnly { @@ -358,20 +391,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } } - /// - public sealed override bool IsComplexType - { - get - { - if (!_isComplexTypeComputed) - { - _isComplexType = ComputeIsComplexType(); - _isComplexTypeComputed = true; - } - return _isComplexType; - } - } - /// public sealed override string NullDisplayText { @@ -495,6 +514,27 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } } + /// + public sealed override string TemplateHint + { + get + { + if (!_templateHintComputed) + { + _templateHint = ComputeTemplateHint(); + _templateHintComputed = true; + } + + return _templateHint; + } + + set + { + _templateHint = value; + _templateHintComputed = true; + } + } + /// public sealed override Type BinderType { @@ -605,6 +645,24 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return base.HtmlEncode; } + /// + /// Calculate the value. + /// + /// Calculated value. + protected virtual bool ComputeIsCollectionType() + { + return base.IsCollectionType; + } + + /// + /// Calculate the value. + /// + /// Calculated value. + protected virtual bool ComputeIsComplexType() + { + return base.IsComplexType; + } + protected virtual bool ComputeIsReadOnly() { return base.IsReadOnly; @@ -615,11 +673,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return base.IsRequired; } - protected virtual bool ComputeIsComplexType() - { - return base.IsComplexType; - } - /// /// Calculate the value. /// @@ -647,5 +700,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { return base.ShowForEdit; } + + /// + /// Calculate the value. + /// + /// Calculated value. + protected virtual string ComputeTemplateHint() + { + return base.TemplateHint; + } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelMetadata.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelMetadata.cs index 1391077ec7..80dfae98bf 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelMetadata.cs @@ -142,11 +142,25 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// public virtual bool HideSurroundingHtml { get; set; } + /// + /// Gets a value indicating whether the is a collection . + /// + /// + /// true if the is not and is assignable to + /// ; false otherwise. + /// public virtual bool IsCollectionType { get { return TypeHelper.IsCollectionType(ModelType); } } + /// + /// Gets a value indicating whether the is a complex type. + /// + /// + /// false if the has a direct conversion to ; true + /// otherwise. + /// public virtual bool IsComplexType { get { return !TypeHelper.HasStringConverter(ModelType); } @@ -238,8 +252,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding protected IModelMetadataProvider Provider { get; set; } /// - /// Gets T if is ; - /// otherwise. + /// Gets runtime of if is non-null and + /// is not ; otherwise. /// public Type RealModelType { @@ -294,6 +308,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding set { _showForEdit = value; } } + /// + /// Gets or sets a hint that suggests what template to use for this model. Overrides + /// in that context but, unlike , this value is not used elsewhere. + /// + /// null unless set manually or through additional metadata e.g. attributes. public virtual string TemplateHint { get; set; } internal EfficientTypePropertyKey CacheKey diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsMetadataAttributesTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsMetadataAttributesTest.cs index fda9e0d1f0..87853f7324 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsMetadataAttributesTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsMetadataAttributesTest.cs @@ -32,6 +32,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Assert.Null(cache.HiddenInput); Assert.Null(cache.Required); Assert.Null(cache.ScaffoldColumn); + Assert.Null(cache.UIHint); Assert.Null(cache.BinderMetadata); Assert.Null(cache.BinderModelNameProvider); Assert.Empty(cache.PropertyBindingPredicateProviders); @@ -51,6 +52,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { new EditableAttribute(allowEdit: false), cache => cache.Editable }, { new HiddenInputAttribute(), cache => cache.HiddenInput }, { new RequiredAttribute(), cache => cache.Required }, + { new ScaffoldColumnAttribute(scaffold: true), cache => cache.ScaffoldColumn }, + { new UIHintAttribute("hintHint"), cache => cache.UIHint }, { new TestBinderMetadata(), cache => cache.BinderMetadata }, { new TestModelNameProvider(), cache => cache.BinderModelNameProvider }, }; diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsModelMetadataProviderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsModelMetadataProviderTest.cs index e3a0b9c799..cd7a4e53b5 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsModelMetadataProviderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsModelMetadataProviderTest.cs @@ -180,7 +180,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } [Fact] - public void HiddenInputWorksOnProperty() + public void HiddenInputWorksOnProperty_ForHideSurroundingHtml() { // Arrange var provider = new DataAnnotationsModelMetadataProvider(); @@ -195,7 +195,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } [Fact] - public void HiddenInputWorksOnPropertyType() + public void HiddenInputWorksOnPropertyType_ForHideSurroundingHtml() { // Arrange var provider = new DataAnnotationsModelMetadataProvider(); @@ -209,6 +209,36 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Assert.True(result); } + [Fact] + public void HiddenInputWorksOnProperty_ForTemplateHint() + { + // Arrange + var provider = new DataAnnotationsModelMetadataProvider(); + var metadata = provider.GetMetadataForType(modelAccessor: null, modelType: typeof(ClassWithHiddenProperties)); + var property = metadata.Properties["DirectlyHidden"]; + + // Act + var result = property.TemplateHint; + + // Assert + Assert.Equal("HiddenInput", result); + } + + [Fact] + public void HiddenInputWorksOnPropertyType_ForTemplateHint() + { + // Arrange + var provider = new DataAnnotationsModelMetadataProvider(); + var metadata = provider.GetMetadataForType(modelAccessor: null, modelType: typeof(ClassWithHiddenProperties)); + var property = metadata.Properties["OfHiddenType"]; + + // Act + var result = property.TemplateHint; + + // Assert + Assert.Equal("HiddenInput", result); + } + [Fact] public void GetMetadataForProperty_WithNoBinderMetadata_GetsItFromType() { diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsModelMetadataTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsModelMetadataTest.cs index cd069136e7..d9711d7159 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsModelMetadataTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsModelMetadataTest.cs @@ -109,7 +109,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding new DisplayFormatAttribute { NullDisplayText = "value" }, metadata => metadata.NullDisplayText }, { - new TestModelNameProvider() { Name = "value" }, metadata => metadata.BinderModelName + new TestModelNameProvider { Name = "value" }, metadata => metadata.BinderModelName + }, + { + new UIHintAttribute("value"), metadata => metadata.TemplateHint }, }; } @@ -208,6 +211,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding metadata => metadata.HideSurroundingHtml, false }, + { + new HiddenInputAttribute(), + metadata => string.Equals("HiddenInput", metadata.TemplateHint, StringComparison.Ordinal), + true + }, { new RequiredAttribute(), metadata => metadata.IsRequired, @@ -466,6 +474,28 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Assert.Null(metadata.EditFormatString); } + [Fact] + public void TemplateHint_AttributesHaveExpectedPrecedence() + { + // Arrange + var expected = "this is a hint"; + var hidden = new HiddenInputAttribute(); + var uiHint = new UIHintAttribute(expected); + var provider = new DataAnnotationsModelMetadataProvider(); + var metadata = new CachedDataAnnotationsModelMetadata( + provider, + containerType: null, + modelType: typeof(object), + propertyName: null, + attributes: new Attribute[] { hidden, uiHint }); + + // Act + var result = metadata.TemplateHint; + + // Assert + Assert.Equal(expected, result); + } + [Fact] public void Constructor_FindsBinderTypeProviders_Null() { diff --git a/test/WebSites/MvcTagHelpersWebSite/Models/Person.cs b/test/WebSites/MvcTagHelpersWebSite/Models/Person.cs index 1718fe9913..e26d94600d 100644 --- a/test/WebSites/MvcTagHelpersWebSite/Models/Person.cs +++ b/test/WebSites/MvcTagHelpersWebSite/Models/Person.cs @@ -2,11 +2,13 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.ComponentModel.DataAnnotations; +using Microsoft.AspNet.Mvc; namespace MvcTagHelpersWebSite.Models { public class Person { + [HiddenInput(DisplayValue = false)] [Range(1, 100)] public int Number { @@ -28,6 +30,7 @@ namespace MvcTagHelpersWebSite.Models } [EnumDataType(typeof(Gender))] + [UIHint("GenderUsingTagHelpers")] public Gender Gender { get; diff --git a/test/WebSites/MvcTagHelpersWebSite/Views/MvcTagHelper_Home/EditWarehouse.cshtml b/test/WebSites/MvcTagHelpersWebSite/Views/MvcTagHelper_Home/EditWarehouse.cshtml index 465776386e..d8c13e1181 100644 --- a/test/WebSites/MvcTagHelpersWebSite/Views/MvcTagHelper_Home/EditWarehouse.cshtml +++ b/test/WebSites/MvcTagHelpersWebSite/Views/MvcTagHelper_Home/EditWarehouse.cshtml @@ -42,7 +42,7 @@ } @Html.DropDownListFor(m => m.Employee.OfficeNumber, offices) - @Html.HiddenFor(m => m.Employee.Number) + @Html.EditorFor(m => m.Employee.Number) @Html.ValidationSummary() diff --git a/test/WebSites/MvcTagHelpersWebSite/Views/MvcTagHelper_Home/EditorTemplates/Gender.cshtml b/test/WebSites/MvcTagHelpersWebSite/Views/MvcTagHelper_Home/EditorTemplates/GenderUsingTagHelpers.cshtml similarity index 100% rename from test/WebSites/MvcTagHelpersWebSite/Views/MvcTagHelper_Home/EditorTemplates/Gender.cshtml rename to test/WebSites/MvcTagHelpersWebSite/Views/MvcTagHelper_Home/EditorTemplates/GenderUsingTagHelpers.cshtml