From ae4cafc002c8475d116b3c5c0be75fe9deb2cb11 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Sun, 1 Mar 2015 21:06:47 -0800 Subject: [PATCH] Correct evaluation of expression result in `GenerateSelect()` - #1468 - Always use `ModelExplorer` in `, Html.DropDownListFor() and Html.ListBoxFor() helper case. Do not use ViewData. defaultValue = modelExplorer.Model; } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/HtmlHelper.cs b/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/HtmlHelper.cs index f4921be5ab..557d95f5c0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/HtmlHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/HtmlHelper.cs @@ -971,16 +971,8 @@ namespace Microsoft.AspNet.Mvc.Rendering } else if (useViewData) { - if (string.IsNullOrEmpty(expression)) - { - // case 2(a): format the value from ViewData for the current model - resolvedValue = FormatValue(ViewData.Model, format); - } - else - { - // case 2(b): format the value from ViewData - resolvedValue = DefaultHtmlGenerator.EvalString(ViewContext, expression, format); - } + // case 2: format the value from ViewData + resolvedValue = DefaultHtmlGenerator.EvalString(ViewContext, expression, format); } else { diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs b/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs index fb63b11b2d..105b672f6d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs @@ -333,7 +333,8 @@ namespace Microsoft.AspNet.Mvc { if (string.IsNullOrEmpty(expression)) { - throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, "expression"); + // Null or empty expression name means current model. + return new ViewDataInfo(container: null, value: Model); } return ViewDataEvaluator.Eval(this, expression); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Rendering/HtmlHelperSelectTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Rendering/HtmlHelperSelectTest.cs index ff75ae4d19..525a290db0 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Rendering/HtmlHelperSelectTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Rendering/HtmlHelperSelectTest.cs @@ -325,6 +325,26 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.Equal(savedValue, selectList.Select(item => item.Value)); } + [Theory] + [MemberData(nameof(DropDownListDataSet))] + public void DropDownList_WithNullSelectList_GeneratesExpectedValue( + IEnumerable selectList, + string expectedHtml, + string ignoredHtml) + { + // Arrange + var helper = DefaultTemplatesUtilities.GetHtmlHelper(); + helper.ViewData["Property1"] = selectList; + var savedSelected = selectList.Select(item => item.Selected).ToList(); + + // Act + var html = helper.DropDownList("Property1", selectList: null, optionLabel: null, htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + } + [Theory] [MemberData(nameof(DropDownListDataSet))] public void DropDownList_WithModelValue_GeneratesExpectedValue( @@ -368,6 +388,30 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); } + [Theory] + [MemberData(nameof(DropDownListDataSet))] + public void DropDownListFor_WithNullModelAndNullSelectList_GeneratesExpectedValue( + IEnumerable selectList, + string expectedHtml, + string ignoredHtml) + { + // Arrange + var helper = DefaultTemplatesUtilities.GetHtmlHelper(); + helper.ViewData["Property1"] = selectList; + var savedSelected = selectList.Select(item => item.Selected).ToList(); + + // Act + var html = helper.DropDownListFor( + value => value.Property1, + selectList: null, + optionLabel: null, + htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + } + [Theory] [MemberData(nameof(DropDownListDataSet))] public void DropDownListFor_WithModelValue_GeneratesExpectedValue( @@ -392,6 +436,82 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); } + [Theory] + [MemberData(nameof(DropDownListDataSet))] + public void DropDownListFor_WithModelValueAndNullSelectList_GeneratesExpectedValue( + IEnumerable selectList, + string ignoredHtml, + string expectedHtml) + { + // Arrange + var model = new DefaultTemplatesUtilities.ObjectTemplateModel { Property1 = "2" }; + var helper = DefaultTemplatesUtilities.GetHtmlHelper(model); + helper.ViewData["Property1"] = selectList; + var savedSelected = selectList.Select(item => item.Selected).ToList(); + + // Act + var html = helper.DropDownListFor( + value => value.Property1, + selectList: null, + optionLabel: null, + htmlAttributes: null); + + // Assert + Assert.Equal(expectedHtml, html.ToString()); + Assert.Equal(savedSelected, selectList.Select(item => item.Selected)); + } + + [Fact] + public void DropDownListFor_WithIndexerExpression_GeneratesExpectedValue() + { + // Arrange + var model = new ModelContainingList { Property1 = { "0", "1", "2" } }; + var helper = DefaultTemplatesUtilities.GetHtmlHelper(model); + var selectList = SomeDisabledOneSelectedSelectList; + var savedSelected = selectList.Select(item => item.Selected).ToList(); + var expectedHtml = + ""; + + // 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(