Simplify `<select/>` tag helper `multiple` attribute handling

- #1516
- `allowMultiple == true` when model has a collection type
 - ignore any `multiple` attribute in Razor source when generating element
- simplify tests too: fewer error cases

Note Razor author could allow browser user to submit values model binding
will likely ignore: Could mix a `multiple` attribute with a non-collection
`asp-for` expression.
This commit is contained in:
Doug Bunting 2015-02-10 19:01:30 -08:00
parent dd3f67c93f
commit 7e166295ba
4 changed files with 40 additions and 177 deletions

View File

@ -122,22 +122,6 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
return string.Format(CultureInfo.CurrentCulture, GetString("SelectTagHelper_CannotDetermineContentWhenOnlyItemsSpecified"), p0, p1, p2);
}
/// <summary>
/// Cannot parse '{1}' value '{2}' for {0}. Acceptable values are '{3}', '{4}' and '{5}'.
/// </summary>
internal static string TagHelpers_InvalidValue_ThreeAcceptableValues
{
get { return GetString("TagHelpers_InvalidValue_ThreeAcceptableValues"); }
}
/// <summary>
/// Cannot parse '{1}' value '{2}' for {0}. Acceptable values are '{3}', '{4}' and '{5}'.
/// </summary>
internal static string FormatTagHelpers_InvalidValue_ThreeAcceptableValues(object p0, object p1, object p2, object p3, object p4, object p5)
{
return string.Format(CultureInfo.CurrentCulture, GetString("TagHelpers_InvalidValue_ThreeAcceptableValues"), p0, p1, p2, p3, p4, p5);
}
/// <summary>
/// The {2} was unable to provide metadata about '{1}' expression value '{3}' for {0}.
/// </summary>

View File

@ -138,9 +138,6 @@
<data name="SelectTagHelper_CannotDetermineContentWhenOnlyItemsSpecified" xml:space="preserve">
<value>Cannot determine body for {0}. '{2}' must be null if '{1}' is null.</value>
</data>
<data name="TagHelpers_InvalidValue_ThreeAcceptableValues" xml:space="preserve">
<value>Cannot parse '{1}' value '{2}' for {0}. Acceptable values are '{3}', '{4}' and '{5}'.</value>
</data>
<data name="TagHelpers_NoProvidedMetadata" xml:space="preserve">
<value>The {2} was unable to provide metadata about '{1}' expression value '{3}' for {0}.</value>
</data>

View File

@ -8,7 +8,6 @@ using System.Linq;
using Microsoft.AspNet.Mvc.ModelBinding;
using Microsoft.AspNet.Mvc.Rendering;
using Microsoft.AspNet.Razor.Runtime.TagHelpers;
using Microsoft.AspNet.Razor.TagHelpers;
namespace Microsoft.AspNet.Mvc.TagHelpers
{
@ -44,16 +43,6 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
[HtmlAttributeName(ForAttributeName)]
public ModelExpression For { get; set; }
/// <summary>
/// Specifies that multiple options can be selected at once.
/// </summary>
/// <remarks>
/// Passed through to the generated HTML if value is <c>multiple</c>. Converted to <c>multiple</c> or absent if
/// value is <c>true</c> or <c>false</c>. Other values are not acceptable. Also used to determine the correct
/// "selected" attributes for generated &lt;option&gt; elements.
/// </remarks>
public string Multiple { get; set; }
/// <summary>
/// A collection of <see cref="SelectListItem"/> objects used to populate the &lt;select&gt; element with
/// &lt;optgroup&gt; and &lt;option&gt; elements.
@ -79,12 +68,6 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
ItemsAttributeName);
throw new InvalidOperationException(message);
}
// Pass through attribute that is also a well-known HTML attribute.
if (!string.IsNullOrEmpty(Multiple))
{
output.CopyHtmlAttribute(nameof(Multiple), context);
}
}
else
{
@ -100,33 +83,12 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
For.Name));
}
bool allowMultiple;
if (string.IsNullOrEmpty(Multiple))
{
// Base allowMultiple on the instance or declared type of the expression.
var realModelType = For.Metadata.RealModelType;
allowMultiple =
// Base allowMultiple on the instance or declared type of the expression to avoid a
// "SelectExpressionNotEnumerable" InvalidOperationException during generation.
// Metadata.IsCollectionType() is similar but does not take runtime type into account.
var realModelType = For.Metadata.RealModelType;
var allowMultiple =
typeof(string) != realModelType && typeof(IEnumerable).IsAssignableFrom(realModelType);
}
else if (string.Equals(Multiple, "multiple", StringComparison.OrdinalIgnoreCase))
{
allowMultiple = true;
// Copy exact attribute name and value the user entered. Must be done prior to any copying from a
// TagBuilder. Not done in next case because "true" and "false" aren't valid for the HTML 5
// attribute.
output.CopyHtmlAttribute(nameof(Multiple), context);
}
else if (!bool.TryParse(Multiple.ToLowerInvariant(), out allowMultiple))
{
throw new InvalidOperationException(Resources.FormatTagHelpers_InvalidValue_ThreeAcceptableValues(
"<select>",
nameof(Multiple).ToLowerInvariant(),
Multiple,
bool.FalseString.ToLowerInvariant(),
bool.TrueString.ToLowerInvariant(),
nameof(Multiple).ToLowerInvariant())); // acceptable value (as well as attribute name)
}
// Ensure GenerateSelect() _never_ looks anything up in ViewData.
var items = Items ?? Enumerable.Empty<SelectListItem>();

View File

@ -87,9 +87,9 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
}
}
// Items value, Multiple value, expected items value (passed to generator), expected allowMultiple.
// Provides cross product of Items and Multiple values. These attribute values should not interact.
public static TheoryData<IEnumerable<SelectListItem>, string, IEnumerable<SelectListItem>, bool>
// Items property value, attribute name, attribute value, expected items value (passed to generator). Provides
// cross product of Items and attributes. These values should not interact.
public static TheoryData<IEnumerable<SelectListItem>, string, string, IEnumerable<SelectListItem>>
ItemsAndMultipleDataSet
{
get
@ -106,24 +106,34 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
new[] { multiItems, multiItems },
new[] { selectItems, selectItems },
};
var mutlipleData = new[]
var attributeData = new Dictionary<string, string>(StringComparer.Ordinal)
{
new Tuple<string, bool>(null, false), // allowMultiple determined by string datatype.
new Tuple<string, bool>("", false), // allowMultiple determined by string datatype.
new Tuple<string, bool>("true", true),
new Tuple<string, bool>("false", false),
new Tuple<string, bool>("multiple", true),
new Tuple<string, bool>("Multiple", true),
new Tuple<string, bool>("MULTIPLE", true),
// SelectTagHelper ignores all "multiple" attribute values.
{ "multiple", null },
{ "mUltiple", string.Empty },
{ "muLtiple", "true" },
{ "Multiple", "false" },
{ "MUltiple", "multiple" },
{ "MuLtiple", "Multiple" },
{ "mUlTiPlE", "mUlTiPlE" },
{ "mULTiPlE", "MULTIPLE" },
{ "mUlTIPlE", "Invalid" },
{ "MULTiPLE", "0" },
{ "MUlTIPLE", "1" },
{ "MULTIPLE", "__true" },
// SelectTagHelper also ignores non-"multiple" attributes.
{ "multiple_", "multiple" },
{ "not-multiple", "multiple" },
{ "__multiple", "multiple" },
};
var theoryData =
new TheoryData<IEnumerable<SelectListItem>, string, IEnumerable<SelectListItem>, bool>();
new TheoryData<IEnumerable<SelectListItem>, string, string, IEnumerable<SelectListItem>>();
foreach (var items in itemsData)
{
foreach (var multiples in mutlipleData)
foreach (var attribute in attributeData)
{
theoryData.Add(items[0], multiples.Item1, items[1], multiples.Item2);
theoryData.Add(items[0], attribute.Key, attribute.Value, items[1]);
}
}
@ -315,19 +325,22 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
[Theory]
[MemberData(nameof(ItemsAndMultipleDataSet))]
public async Task ProcessAsync_CallsGeneratorWithExpectedValues_ItemsAndMultiple(
public async Task ProcessAsync_CallsGeneratorWithExpectedValues_ItemsAndAttribute(
IEnumerable<SelectListItem> inputItems,
string multiple,
IEnumerable<SelectListItem> expectedItems,
bool expectedAllowMultiple)
string attributeName,
string attributeValue,
IEnumerable<SelectListItem> expectedItems)
{
// Arrange
var contextAttributes = new Dictionary<string, object>
{
// Attribute will be restored if value matches "multiple".
{ "multiple", multiple },
// Provided for completeness. Select tag helper does not confirm AllAttributes set is consistent.
{ attributeName, attributeValue },
};
var originalAttributes = new Dictionary<string, string>
{
{ attributeName, attributeValue },
};
var originalAttributes = new Dictionary<string, string>();
var propertyName = "Property1";
var expectedTagName = "select";
@ -356,7 +369,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
null, // optionLabel
string.Empty, // name
expectedItems,
expectedAllowMultiple,
false, // allowMultiple
null, // htmlAttributes
out selectedValues))
.Returns((TagBuilder)null)
@ -367,7 +380,6 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
For = modelExpression,
Items = inputItems,
Generator = htmlGenerator.Object,
Multiple = multiple,
ViewContext = viewContext,
};
@ -443,58 +455,6 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
Assert.Same(selectedValues, keyValuePair.Value);
}
[Theory]
[InlineData("multiple")]
[InlineData("mUlTiPlE")]
[InlineData("MULTIPLE")]
public async Task ProcessAsync_RestoresMultiple_IfForNotBound(string attributeName)
{
// Arrange
var contextAttributes = new Dictionary<string, object>
{
{ attributeName, "I'm more than one" },
};
var originalAttributes = new Dictionary<string, string>
{
{ "class", "form-control" },
{ "size", "2" },
};
var expectedAttributes = new Dictionary<string, string>(originalAttributes);
expectedAttributes[attributeName] = (string)contextAttributes[attributeName];
var expectedPreContent = "original pre-content";
var expectedContent = "original content";
var expectedPostContent = "original post-content";
var expectedTagName = "select";
var tagHelperContext = new TagHelperContext(
contextAttributes,
uniqueId: "test",
getChildContentAsync: () => Task.FromResult("Something"));
var output = new TagHelperOutput(expectedTagName, originalAttributes)
{
PreContent = expectedPreContent,
Content = expectedContent,
PostContent = expectedPostContent,
SelfClosing = true,
};
var tagHelper = new SelectTagHelper
{
Multiple = "I'm more than one",
};
// Act
await tagHelper.ProcessAsync(tagHelperContext, output);
// Assert
Assert.Equal(expectedAttributes, output.Attributes);
Assert.Equal(expectedPreContent, output.PreContent);
Assert.Equal(expectedContent, output.Content);
Assert.Equal(expectedPostContent, output.PostContent);
Assert.True(output.SelfClosing);
Assert.Equal(expectedTagName, output.TagName);
}
[Fact]
public async Task ProcessAsync_Throws_IfForNotBoundButItemsIs()
{
@ -520,46 +480,6 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
Assert.Equal(expectedMessage, exception.Message);
}
[Theory]
[InlineData("Invalid")]
[InlineData("0")]
[InlineData("1")]
[InlineData("__true")]
[InlineData("false__")]
[InlineData("__Multiple")]
[InlineData("Multiple__")]
public async Task ProcessAsync_Throws_IfMultipleInvalid(string multiple)
{
// Arrange
var contextAttributes = new Dictionary<string, object>();
var originalAttributes = new Dictionary<string, string>();
var expectedTagName = "select";
var expectedMessage = "Cannot parse 'multiple' value '" + multiple +
"' for <select>. Acceptable values are 'false', 'true' and 'multiple'.";
var tagHelperContext = new TagHelperContext(
contextAttributes,
uniqueId: "test",
getChildContentAsync: () => Task.FromResult("Something"));
var output = new TagHelperOutput(expectedTagName, originalAttributes);
var metadataProvider = new EmptyModelMetadataProvider();
string model = null;
var metadata = metadataProvider.GetMetadataForType(() => model, typeof(string));
var modelExpression = new ModelExpression("Property1", metadata);
var tagHelper = new SelectTagHelper
{
For = modelExpression,
Multiple = multiple,
};
// Act & Assert
var exception = await Assert.ThrowsAsync<InvalidOperationException>(
() => tagHelper.ProcessAsync(tagHelperContext, output));
Assert.Equal(expectedMessage, exception.Message);
}
public class NameAndId
{
public NameAndId(string name, string id)