diff --git a/src/Microsoft.AspNet.Mvc.Core/Rendering/Expressions/ViewDataEvaluator.cs b/src/Microsoft.AspNet.Mvc.Core/Rendering/Expressions/ViewDataEvaluator.cs index 58bd040e02..05741b7884 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Rendering/Expressions/ViewDataEvaluator.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Rendering/Expressions/ViewDataEvaluator.cs @@ -1,16 +1,23 @@ // 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.Reflection; +using Microsoft.AspNet.Mvc.Core; using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Mvc.Rendering.Expressions { public static class ViewDataEvaluator { - public static ViewDataInfo Eval([NotNull] ViewDataDictionary viewData, [NotNull] string expression) + public static ViewDataInfo Eval([NotNull] ViewDataDictionary viewData, string expression) { + if (string.IsNullOrEmpty(expression)) + { + throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, nameof(expression)); + } + // Given an expression "one.two.three.four" we look up the following (pseudocode): // this["one.two.three.four"] // this["one.two.three"]["four"] @@ -24,8 +31,13 @@ namespace Microsoft.AspNet.Mvc.Rendering.Expressions return EvalComplexExpression(viewData, expression); } - public static ViewDataInfo Eval(object indexableObject, [NotNull] string expression) + public static ViewDataInfo Eval(object indexableObject, string expression) { + if (string.IsNullOrEmpty(expression)) + { + throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, nameof(expression)); + } + // Run through same cases as other Eval() overload but allow a null container. return (indexableObject == null) ? null : EvalComplexExpression(indexableObject, expression); } diff --git a/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/DefaultHtmlGenerator.cs b/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/DefaultHtmlGenerator.cs index 7ff872d02a..3af1d558b7 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/DefaultHtmlGenerator.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/DefaultHtmlGenerator.cs @@ -409,9 +409,9 @@ namespace Microsoft.AspNet.Mvc.Rendering { if (string.IsNullOrEmpty(expression)) { - // Avoid ViewData.Eval() throwing an ArgumentException with a different parameter name. Note this - // is an extreme case since users must pass a non-null selectList to use CheckBox() or ListBox() - // in a template, where a null or empty name has meaning. + // Do not call ViewData.Eval(); that would return ViewData.Model, which is not correct here. + // Note this case has a simple workaround: users must pass a non-null selectList to use + // DropDownList() or ListBox() in a template, where a null or empty name has meaning. throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, nameof(expression)); } @@ -422,16 +422,21 @@ namespace Microsoft.AspNet.Mvc.Rendering var type = allowMultiple ? typeof(string[]) : typeof(string); var defaultValue = GetModelStateValue(viewContext, fullName, type); - // If we haven't already used ViewData to get the entire list of items then we need to - // use the ViewData-supplied value before using the parameter-supplied value. - if (defaultValue == null && !string.IsNullOrEmpty(expression)) + // If ModelState did not contain a current value, fall back to ViewData- or ModelExplorer-supplied value. + if (defaultValue == null) { - if (!usedViewData) + if (modelExplorer == null) { - defaultValue = viewContext.ViewData.Eval(expression); + // Html.DropDownList() and Html.ListBox() helper case. + // Cannot use ViewData if it contains the select list. + if (!usedViewData) + { + defaultValue = viewContext.ViewData.Eval(expression); + } } - else if (modelExplorer != null) + else { + // " + + Environment.NewLine + + "" + Environment.NewLine + + "" + Environment.NewLine + + "" + Environment.NewLine + + ""; + + // Act + var html = helper.DropDownListFor( + value => value.Property1[2], + selectList, + optionLabel: null, + htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + } + + [Fact] + public void DropDownListFor_WithUnrelatedExpression_GeneratesExpectedValue() + { + // Arrange + var unrelated = "2"; + var helper = DefaultTemplatesUtilities.GetHtmlHelper(); + var selectList = SomeDisabledOneSelectedSelectList; + var savedSelected = selectList.Select(item => item.Selected).ToList(); + var expectedHtml = + ""; + + // Act + var html = helper.DropDownListFor(value => unrelated, selectList, optionLabel: null, htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + } + [Theory] [MemberData(nameof(ListBoxDataSet))] public void ListBox_WithNullModel_GeneratesExpectedValue_DoesNotChangeSelectList( @@ -503,6 +623,30 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); } + [Fact] + public void ListBoxFor_WithUnreleatedExpression_GeneratesExpectedValue() + { + // Arrange + var unrelated = new[] { "2" }; + var helper = DefaultTemplatesUtilities.GetHtmlHelper(); + var selectList = SomeDisabledOneSelectedSelectList; + var savedSelected = selectList.Select(item => item.Selected).ToList(); + var expectedHtml = + ""; + + // Act + var html = helper.ListBoxFor(value => unrelated, selectList, htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + } + [Theory] [MemberData(nameof(ListBoxDataSet))] public void ListBoxFor_WithMultipleModelValues_GeneratesExpectedValue( @@ -524,6 +668,196 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); } + [Theory] + [MemberData(nameof(DropDownListDataSet))] + public void DropDownListInTemplate_WithNullModel_GeneratesExpectedValue_DoesNotChangeSelectList( + IEnumerable selectList, + string expectedHtml, + string ignoredHtml) + { + // Arrange + var helper = DefaultTemplatesUtilities.GetHtmlHelper(model: null); + helper.ViewData.TemplateInfo.HtmlFieldPrefix = "Property1"; + var savedDisabled = selectList.Select(item => item.Disabled).ToList(); + var savedGroup = selectList.Select(item => item.Group).ToList(); + var savedSelected = selectList.Select(item => item.Selected).ToList(); + var savedText = selectList.Select(item => item.Text).ToList(); + var savedValue = selectList.Select(item => item.Value).ToList(); + + // Act + var html = helper.DropDownList( + expression: string.Empty, + selectList: selectList, + optionLabel: null, + htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedDisabled, selectList.Select(item => item.Disabled)); + Assert.Equal(savedGroup, selectList.Select(item => item.Group)); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + Assert.Equal(savedText, selectList.Select(item => item.Text)); + Assert.Equal(savedValue, selectList.Select(item => item.Value)); + } + + [Theory] + [MemberData(nameof(DropDownListDataSet))] + public void DropDownListInTemplate_WithModelValue_GeneratesExpectedValue( + IEnumerable selectList, + string ignoredHtml, + string expectedHtml) + { + // Arrange + var helper = DefaultTemplatesUtilities.GetHtmlHelper("2"); + helper.ViewData.TemplateInfo.HtmlFieldPrefix = "Property1"; + var savedSelected = selectList.Select(item => item.Selected).ToList(); + + // Act + var html = helper.DropDownList( + expression: string.Empty, + selectList: selectList, + optionLabel: null, + htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + } + + [Theory] + [MemberData(nameof(DropDownListDataSet))] + public void DropDownListForInTemplate_WithNullModel_GeneratesExpectedValue( + IEnumerable selectList, + string expectedHtml, + string ignoredHtml) + { + // Arrange + var helper = DefaultTemplatesUtilities.GetHtmlHelper(model: null); + helper.ViewData.TemplateInfo.HtmlFieldPrefix = "Property1"; + var savedSelected = selectList.Select(item => item.Selected).ToList(); + + // Act + var html = helper.DropDownListFor(value => value, selectList, optionLabel: null, htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + } + + [Theory] + [MemberData(nameof(DropDownListDataSet))] + public void DropDownListForInTemplate_WithModelValue_GeneratesExpectedValue( + IEnumerable selectList, + string ignoredHtml, + string expectedHtml) + { + // Arrange + var helper = DefaultTemplatesUtilities.GetHtmlHelper("2"); + helper.ViewData.TemplateInfo.HtmlFieldPrefix = "Property1"; + var savedSelected = selectList.Select(item => item.Selected).ToList(); + + // Act + var html = helper.DropDownListFor(value => value, selectList, optionLabel: null, htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + } + + [Theory] + [MemberData(nameof(ListBoxDataSet))] + public void ListBoxInTemplate_WithNullModel_GeneratesExpectedValue_DoesNotChangeSelectList( + IEnumerable selectList, + string expectedHtml, + string ignoredHtml1, + string ignoredHtml2) + { + // Arrange + var helper = DefaultTemplatesUtilities.GetHtmlHelper>(model: null); + helper.ViewData.TemplateInfo.HtmlFieldPrefix = "Property1"; + var savedDisabled = selectList.Select(item => item.Disabled).ToList(); + var savedGroup = selectList.Select(item => item.Group).ToList(); + var savedSelected = selectList.Select(item => item.Selected).ToList(); + var savedText = selectList.Select(item => item.Text).ToList(); + var savedValue = selectList.Select(item => item.Value).ToList(); + + // Act + var html = helper.ListBox(expression: string.Empty, selectList: selectList, htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedDisabled, selectList.Select(item => item.Disabled)); + Assert.Equal(savedGroup, selectList.Select(item => item.Group)); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + Assert.Equal(savedText, selectList.Select(item => item.Text)); + Assert.Equal(savedValue, selectList.Select(item => item.Value)); + } + + [Theory] + [MemberData(nameof(ListBoxDataSet))] + public void ListBoxInTemplate_WithModelValue_GeneratesExpectedValue( + IEnumerable selectList, + string ignoredHtml1, + string expectedHtml, + string ignoredHtml2) + { + // Arrange + var model = new List { "2" }; + var helper = DefaultTemplatesUtilities.GetHtmlHelper(model); + helper.ViewData.TemplateInfo.HtmlFieldPrefix = "Property1"; + var savedSelected = selectList.Select(item => item.Selected).ToList(); + + // Act + var html = helper.ListBox(expression: string.Empty, selectList: selectList, htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + } + + [Theory] + [MemberData(nameof(ListBoxDataSet))] + public void ListBoxForInTemplate_WithNullModel_GeneratesExpectedValue( + IEnumerable selectList, + string expectedHtml, + string ignoredHtml1, + string ignoredHtml2) + { + // Arrange + var helper = DefaultTemplatesUtilities.GetHtmlHelper>(model: null); + helper.ViewData.TemplateInfo.HtmlFieldPrefix = "Property1"; + var savedSelected = selectList.Select(item => item.Selected).ToList(); + + // Act + var html = helper.ListBoxFor(value => value, selectList, htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + } + + [Theory] + [MemberData(nameof(ListBoxDataSet))] + public void ListBoxForInTemplate_WithModelValue_GeneratesExpectedValue( + IEnumerable selectList, + string ignoredHtml1, + string expectedHtml, + string ignoredHtml2) + { + // Arrange + var model = new List { "2" }; + var helper = DefaultTemplatesUtilities.GetHtmlHelper(model); + helper.ViewData.TemplateInfo.HtmlFieldPrefix = "Property1"; + var savedSelected = selectList.Select(item => item.Selected).ToList(); + + // Act + var html = helper.ListBoxFor(value => value, selectList, htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + } + private class ModelContainingList { public List Property1 { get; } = new List(); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ViewDataDictionaryTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ViewDataDictionaryTest.cs index 4163db3331..69d15046e2 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ViewDataDictionaryTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ViewDataDictionaryTest.cs @@ -6,7 +6,6 @@ using System.Collections; using System.Collections.Generic; using System.Linq; using Microsoft.AspNet.Mvc.ModelBinding; -using Microsoft.AspNet.Testing; using Microsoft.Framework.Internal; using Moq; using Xunit; @@ -605,13 +604,18 @@ namespace Microsoft.AspNet.Mvc.Core [Theory] [InlineData(null)] [InlineData("")] - public void EvalThrowsIfExpressionIsNullOrEmpty(string expression) + public void Eval_ReturnsModel_IfExpressionIsNullOrEmpty(string expression) { // Arrange var viewData = new ViewDataDictionary(new EmptyModelMetadataProvider()); + var model = new object(); + viewData = new ViewDataDictionary(viewData, model); - // Act & Assert - ExceptionAssert.ThrowsArgumentNullOrEmpty(() => viewData.Eval(expression), "expression"); + // Act + var result = viewData.Eval(expression); + + // Assert + Assert.Same(model, result); } [Fact] diff --git a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/SelectTagHelperTest.cs b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/SelectTagHelperTest.cs index da4943031a..cb9d408ccd 100644 --- a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/SelectTagHelperTest.cs +++ b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/SelectTagHelperTest.cs @@ -60,6 +60,10 @@ namespace Microsoft.AspNet.Mvc.TagHelpers { { null, typeof(Model), () => null, new NameAndId("Text", "Text"), noneSelected }, + // Imitate a temporary variable set from somewhere else in the view model. + { null, typeof(Model), () => modelWithText.NestedModel.Text, + new NameAndId("item.Text", "item_Text"), innerSelected }, + { modelWithNull, typeof(Model), () => modelWithNull.Text, new NameAndId("Text", "Text"), noneSelected }, { modelWithText, typeof(Model), () => modelWithText.Text, @@ -75,14 +79,10 @@ namespace Microsoft.AspNet.Mvc.TagHelpers { models, typeof(NestedModel), () => models[0].NestedModel.Text, new NameAndId("[0].NestedModel.Text", "z0__NestedModel_Text"), noneSelected }, - // TODO: https://github.com/aspnet/Mvc/issues/1468 - // Skip last two test cases because DefaultHtmlGenerator evaluates expression name against - // ViewData, not using ModelMetadata.Model. ViewData.Eval() handles simple property paths and some - // dictionary lookups, but not indexing into an array or list. See #1468... - ////{ models, typeof(Model), () => models[1].Text, - //// new NameAndId("[1].Text", "z1__Text"), outerSelected }, - ////{ models, typeof(NestedModel), () => models[1].NestedModel.Text, - //// new NameAndId("[1].NestedModel.Text", "z1__NestedModel_Text"), innerSelected }, + { models, typeof(Model), () => models[1].Text, + new NameAndId("[1].Text", "z1__Text"), outerSelected }, + { models, typeof(NestedModel), () => models[1].NestedModel.Text, + new NameAndId("[1].NestedModel.Text", "z1__NestedModel_Text"), innerSelected }, }; } } @@ -344,6 +344,106 @@ namespace Microsoft.AspNet.Mvc.TagHelpers Assert.Equal(savedValue, items.Select(item => item.Value)); } + [Theory] + [MemberData(nameof(GeneratesExpectedDataSet))] + public async Task ProcessAsyncInTemplate_WithItems_GeneratesExpectedOutput_DoesNotChangeSelectList( + object model, + Type containerType, + Func modelAccessor, + NameAndId nameAndId, + string expectedOptions) + { + // Arrange + var originalAttributes = new Dictionary + { + { "class", "form-control" }, + }; + var originalPostContent = "original content"; + + var expectedAttributes = new Dictionary(originalAttributes) + { + { "id", nameAndId.Id }, + { "name", nameAndId.Name }, + { "valid", "from validation attributes" }, + }; + var expectedPreContent = "original pre-content"; + var expectedContent = "original content"; + var expectedPostContent = originalPostContent + expectedOptions; + var expectedTagName = "select"; + + var metadataProvider = new DataAnnotationsModelMetadataProvider(); + + var containerMetadata = metadataProvider.GetMetadataForType(containerType); + var containerExplorer = metadataProvider.GetModelExplorerForType(containerType, model); + + var propertyMetadata = metadataProvider.GetMetadataForProperty(containerType, "Text"); + var modelExplorer = containerExplorer.GetExplorerForExpression(propertyMetadata, modelAccessor()); + + var modelExpression = new ModelExpression(name: string.Empty, modelExplorer: modelExplorer); + + var tagHelperContext = new TagHelperContext( + allAttributes: new Dictionary(), + items: new Dictionary(), + uniqueId: "test", + getChildContentAsync: () => Task.FromResult("Something")); + var output = new TagHelperOutput(expectedTagName, originalAttributes) + { + PreContent = expectedPreContent, + Content = expectedContent, + PostContent = originalPostContent, + SelfClosing = true, + }; + + var htmlGenerator = new TestableHtmlGenerator(metadataProvider) + { + ValidationAttributes = + { + { "valid", "from validation attributes" }, + } + }; + var viewContext = TestableHtmlGenerator.GetViewContext(model, htmlGenerator, metadataProvider); + viewContext.ViewData.TemplateInfo.HtmlFieldPrefix = nameAndId.Name; + + var items = new SelectList(new[] { "", "outer text", "inner text", "other text" }); + var savedDisabled = items.Select(item => item.Disabled).ToList(); + var savedGroup = items.Select(item => item.Group).ToList(); + var savedSelected = items.Select(item => item.Selected).ToList(); + var savedText = items.Select(item => item.Text).ToList(); + var savedValue = items.Select(item => item.Value).ToList(); + var tagHelper = new SelectTagHelper + { + For = modelExpression, + Generator = htmlGenerator, + Items = items, + ViewContext = viewContext, + }; + + // Act + await tagHelper.ProcessAsync(tagHelperContext, output); + + // Assert + Assert.True(output.SelfClosing); + Assert.Equal(expectedAttributes, output.Attributes); + Assert.Equal(expectedPreContent, output.PreContent); + Assert.Equal(expectedContent, output.Content); + Assert.Equal(expectedPostContent, output.PostContent); + Assert.Equal(expectedTagName, output.TagName); + + Assert.NotNull(viewContext.FormContext?.FormData); + var keyValuePair = Assert.Single( + viewContext.FormContext.FormData, + entry => entry.Key == SelectTagHelper.SelectedValuesFormDataKey); + Assert.NotNull(keyValuePair.Value); + var selectedValues = Assert.IsAssignableFrom>(keyValuePair.Value); + Assert.InRange(selectedValues.Count, 0, 1); + + Assert.Equal(savedDisabled, items.Select(item => item.Disabled)); + Assert.Equal(savedGroup, items.Select(item => item.Group)); + Assert.Equal(savedSelected, items.Select(item => item.Selected)); + Assert.Equal(savedText, items.Select(item => item.Text)); + Assert.Equal(savedValue, items.Select(item => item.Value)); + } + [Theory] [MemberData(nameof(ItemsAndMultipleDataSet))] public async Task ProcessAsync_CallsGeneratorWithExpectedValues_ItemsAndAttribute(