From 911dfc57b0a40b8feb415e9afcc90a9845d0ba5b Mon Sep 17 00:00:00 2001
From: "N. Taylor Mullen" <nimullen@microsoft.com>
Date: Wed, 21 Oct 2015 16:20:05 -0700
Subject: [PATCH] Transition `SelectTagHelper` and `OptionTagHelper` to use
 `context.Items`.

- Added functional tests to validate data created from a `SelectTagHelper` does not impact following `<select>` tags.
- Also moved the new `SelectTagHelper` communication flow into `TagHelper.Init`.

#3347
---
 .../OptionTagHelper.cs                        |  8 +-
 .../RenderAtEndOfFormTagHelper.cs             |  5 ++
 .../SelectTagHelper.cs                        | 80 +++++++++----------
 ...ite.HtmlGeneration_Home.Order.Encoded.html |  3 +
 ...tionWebSite.HtmlGeneration_Home.Order.html |  3 +
 ...on_Home.OrderUsingHtmlHelpers.Encoded.html |  3 +
 ...Generation_Home.OrderUsingHtmlHelpers.html |  3 +
 .../OptionTagHelperTest.cs                    | 10 +--
 .../SelectTagHelperTest.cs                    | 31 ++++---
 .../Views/HtmlGeneration_Home/Order.cshtml    |  3 +
 .../OrderUsingHtmlHelpers.cshtml              |  3 +
 11 files changed, 86 insertions(+), 66 deletions(-)

diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/OptionTagHelper.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/OptionTagHelper.cs
index 97d8b627d6..b897eb580e 100644
--- a/src/Microsoft.AspNet.Mvc.TagHelpers/OptionTagHelper.cs
+++ b/src/Microsoft.AspNet.Mvc.TagHelpers/OptionTagHelper.cs
@@ -54,8 +54,8 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
 
         /// <inheritdoc />
         /// <remarks>
-        /// Does nothing unless <see cref="FormContext.FormData"/> contains a
-        /// <see cref="SelectTagHelper.SelectedValuesFormDataKey"/> entry and that entry is a non-empty
+        /// Does nothing unless <see cref="TagHelperContext.Items"/> contains a
+        /// <see cref="SelectTagHelper"/> <see cref="Type"/> entry and that entry is a non-empty
         /// <see cref="ICollection{string}"/> instance. Also does nothing if the associated &lt;option&gt; is already
         /// selected.
         /// </remarks>
@@ -82,9 +82,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             {
                 // Is this <option/> element a child of a <select/> element the SelectTagHelper targeted?
                 object formDataEntry;
-                ViewContext.FormContext.FormData.TryGetValue(
-                    SelectTagHelper.SelectedValuesFormDataKey,
-                    out formDataEntry);
+                context.Items.TryGetValue(typeof(SelectTagHelper), out formDataEntry);
 
                 // ... And did the SelectTagHelper determine any selected values?
                 var selectedValues = formDataEntry as ICollection<string>;
diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/RenderAtEndOfFormTagHelper.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/RenderAtEndOfFormTagHelper.cs
index 1716b83c87..07697deb1d 100644
--- a/src/Microsoft.AspNet.Mvc.TagHelpers/RenderAtEndOfFormTagHelper.cs
+++ b/src/Microsoft.AspNet.Mvc.TagHelpers/RenderAtEndOfFormTagHelper.cs
@@ -27,6 +27,11 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
         /// <inheritdoc />
         public override void Init(TagHelperContext context)
         {
+            if (context == null)
+            {
+                throw new ArgumentNullException(nameof(context));
+            }
+
             // Push the new FormContext.
             ViewContext.FormContext = new FormContext
             {
diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/SelectTagHelper.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/SelectTagHelper.cs
index 334747188a..19ec7e6d0d 100644
--- a/src/Microsoft.AspNet.Mvc.TagHelpers/SelectTagHelper.cs
+++ b/src/Microsoft.AspNet.Mvc.TagHelpers/SelectTagHelper.cs
@@ -21,16 +21,8 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
     {
         private const string ForAttributeName = "asp-for";
         private const string ItemsAttributeName = "asp-items";
-
-        /// <summary>
-        /// Key used for selected values in <see cref="FormContext.FormData"/>.
-        /// </summary>
-        /// <remarks>
-        /// Value for this dictionary entry will either be <c>null</c> (indicating no <see cref="SelectTagHelper"/> has
-        /// executed within this &lt;form/&gt;) or an <see cref="ICollection{string}"/> instance. Elements of the
-        /// collection are based on current <see cref="ViewDataDictionary.Model"/>.
-        /// </remarks>
-        public static readonly string SelectedValuesFormDataKey = nameof(SelectTagHelper) + "-SelectedValues";
+        private bool _allowMultiple;
+        private IReadOnlyCollection<string> _currentValues;
 
         /// <summary>
         /// Creates a new <see cref="SelectTagHelper"/>.
@@ -69,6 +61,42 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
         [HtmlAttributeName(ItemsAttributeName)]
         public IEnumerable<SelectListItem> Items { get; set; }
 
+        /// <inheritdoc />
+        public override void Init(TagHelperContext context)
+        {
+            if (context == null)
+            {
+                throw new ArgumentNullException(nameof(context));
+            }
+
+            // Note null or empty For.Name is allowed because TemplateInfo.HtmlFieldPrefix may be sufficient.
+            // IHtmlGenerator will enforce name requirements.
+            if (For.Metadata == null)
+            {
+                throw new InvalidOperationException(Resources.FormatTagHelpers_NoProvidedMetadata(
+                    "<select>",
+                    ForAttributeName,
+                    nameof(IModelMetadataProvider),
+                    For.Name));
+            }
+
+            // Base allowMultiple on the instance or declared type of the expression to avoid a
+            // "SelectExpressionNotEnumerable" InvalidOperationException during generation.
+            // Metadata.IsEnumerableType is similar but does not take runtime type into account.
+            var realModelType = For.ModelExplorer.ModelType;
+            _allowMultiple = typeof(string) != realModelType &&
+                typeof(IEnumerable).GetTypeInfo().IsAssignableFrom(realModelType.GetTypeInfo());
+            _currentValues = Generator.GetCurrentValues(
+                ViewContext,
+                For.ModelExplorer,
+                expression: For.Name,
+                allowMultiple: _allowMultiple);
+
+            // Whether or not (not being highly unlikely) we generate anything, could update contained <option/>
+            // elements. Provide selected values for <option/> tag helpers.
+            context.Items[typeof(SelectTagHelper)] = _currentValues;
+        }
+
         /// <inheritdoc />
         /// <remarks>Does nothing if <see cref="For"/> is <c>null</c>.</remarks>
         /// <exception cref="InvalidOperationException">
@@ -86,41 +114,17 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
                 throw new ArgumentNullException(nameof(output));
             }
 
-            // Note null or empty For.Name is allowed because TemplateInfo.HtmlFieldPrefix may be sufficient.
-            // IHtmlGenerator will enforce name requirements.
-            var metadata = For.Metadata;
-            if (metadata == null)
-            {
-                throw new InvalidOperationException(Resources.FormatTagHelpers_NoProvidedMetadata(
-                    "<select>",
-                    ForAttributeName,
-                    nameof(IModelMetadataProvider),
-                    For.Name));
-            }
-
-            // Base allowMultiple on the instance or declared type of the expression to avoid a
-            // "SelectExpressionNotEnumerable" InvalidOperationException during generation.
-            // Metadata.IsEnumerableType is similar but does not take runtime type into account.
-            var realModelType = For.ModelExplorer.ModelType;
-            var allowMultiple = typeof(string) != realModelType &&
-                typeof(IEnumerable).GetTypeInfo().IsAssignableFrom(realModelType.GetTypeInfo());
-
             // Ensure GenerateSelect() _never_ looks anything up in ViewData.
             var items = Items ?? Enumerable.Empty<SelectListItem>();
 
-            var currentValues = Generator.GetCurrentValues(
-                ViewContext,
-                For.ModelExplorer,
-                expression: For.Name,
-                allowMultiple: allowMultiple);
             var tagBuilder = Generator.GenerateSelect(
                 ViewContext,
                 For.ModelExplorer,
                 optionLabel: null,
                 expression: For.Name,
                 selectList: items,
-                currentValues: currentValues,
-                allowMultiple: allowMultiple,
+                currentValues: _currentValues,
+                allowMultiple: _allowMultiple,
                 htmlAttributes: null);
 
             if (tagBuilder != null)
@@ -128,10 +132,6 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
                 output.MergeAttributes(tagBuilder);
                 output.PostContent.Append(tagBuilder.InnerHtml);
             }
-
-            // Whether or not (not being highly unlikely) we generate anything, could update contained <option/>
-            // elements. Provide selected values for <option/> tag helpers. They'll run next.
-            ViewContext.FormContext.FormData[SelectedValuesFormDataKey] = currentValues;
         }
     }
 }
\ No newline at end of file
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.Encoded.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.Encoded.html
index 6f3ae7f3aa..16d8ba1fbf 100644
--- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.Encoded.html
+++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.Encoded.html
@@ -41,6 +41,9 @@
                 <option value="HtmlEncode[[Credit]]">Credit</option>
                 <option value="HtmlEncode[[Check]]" selected="HtmlEncode[[selected]]">Check</option>
             </select>
+            <select>
+                <option value="HtmlEncode[[Check]]">Check</option>
+            </select>
         </div>
         <div>
             <label class="order" for="HtmlEncode[[Customer_Number]]">HtmlEncode[[Number]]</label>
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.html
index acd557197b..0ef8e279cd 100644
--- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.html
+++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.html
@@ -41,6 +41,9 @@
                 <option value="Credit">Credit</option>
                 <option value="Check" selected="selected">Check</option>
             </select>
+            <select>
+                <option value="Check">Check</option>
+            </select>
         </div>
         <div>
             <label class="order" for="Customer_Number">Number</label>
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.Encoded.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.Encoded.html
index ed4c20ce9b..8169d7284b 100644
--- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.Encoded.html
+++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.Encoded.html
@@ -40,6 +40,9 @@
             <select id="HtmlEncode[[PaymentMethod]]" multiple="HtmlEncode[[multiple]]" name="HtmlEncode[[PaymentMethod]]"><option value="HtmlEncode[[Credit]]">HtmlEncode[[Credit]]</option>
 <option selected="HtmlEncode[[selected]]" value="HtmlEncode[[Check]]">HtmlEncode[[Check]]</option>
 </select>
+            <select>
+                <option value="Check">Check</option>
+            </select>
         </div>
         <div>
             <label class="HtmlEncode[[order]]" for="HtmlEncode[[Customer_Number]]">HtmlEncode[[Number]]</label>
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.html
index 3b0fd90fe1..9c64ee6c06 100644
--- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.html
+++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.html
@@ -40,6 +40,9 @@
             <select id="PaymentMethod" multiple="multiple" name="PaymentMethod"><option value="Credit">Credit</option>
 <option selected="selected" value="Check">Check</option>
 </select>
+            <select>
+                <option value="Check">Check</option>
+            </select>
         </div>
         <div>
             <label class="order" for="Customer_Number">Number</label>
diff --git a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/OptionTagHelperTest.cs b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/OptionTagHelperTest.cs
index c8fe519dc5..b4829d14c0 100644
--- a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/OptionTagHelperTest.cs
+++ b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/OptionTagHelperTest.cs
@@ -12,7 +12,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
 {
     public class OptionTagHelperTest
     {
-        // Original content, selected attribute, value attribute, selected values (to place in FormContext.FormData)
+        // Original content, selected attribute, value attribute, selected values (to place in TagHelperContext.Items)
         // and expected tag helper output.
         public static TheoryData<string, string, string, IEnumerable<string>, TagHelperOutput> GeneratesExpectedDataSet
         {
@@ -346,7 +346,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             }
         }
 
-        // Original content, selected attribute, value attribute, selected values (to place in FormContext.FormData)
+        // Original content, selected attribute, value attribute, selected values (to place in TagHelperContext.Items)
         // and expected output (concatenation of TagHelperOutput generations). Excludes non-null selected attribute,
         // null selected values, and empty selected values cases.
         public static IEnumerable<object[]> DoesNotUseGeneratorDataSet
@@ -358,7 +358,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             }
         }
 
-        // Original content, selected attribute, value attribute, selected values (to place in FormContext.FormData)
+        // Original content, selected attribute, value attribute, selected values (to place in TagHelperContext.Items)
         // and expected output (concatenation of TagHelperOutput generations). Excludes non-null selected attribute
         // cases.
         public static IEnumerable<object[]> DoesNotUseViewContextDataSet
@@ -420,7 +420,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
                 model: null,
                 htmlGenerator: htmlGenerator,
                 metadataProvider: metadataProvider);
-            viewContext.FormContext.FormData[SelectTagHelper.SelectedValuesFormDataKey] = selectedValues;
+            tagHelperContext.Items[typeof(SelectTagHelper)] = selectedValues;
             var tagHelper = new OptionTagHelper(htmlGenerator)
             {
                 Value = value,
@@ -491,7 +491,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
                 model: null,
                 htmlGenerator: htmlGenerator,
                 metadataProvider: metadataProvider);
-            viewContext.FormContext.FormData[SelectTagHelper.SelectedValuesFormDataKey] = selectedValues;
+            tagHelperContext.Items[typeof(SelectTagHelper)] = selectedValues;
             var tagHelper = new OptionTagHelper(htmlGenerator)
             {
                 Value = value,
diff --git a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/SelectTagHelperTest.cs b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/SelectTagHelperTest.cs
index 03b8c9e15d..3716847e44 100644
--- a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/SelectTagHelperTest.cs
+++ b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/SelectTagHelperTest.cs
@@ -235,6 +235,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             };
 
             // Act
+            tagHelper.Init(tagHelperContext);
             await tagHelper.ProcessAsync(tagHelperContext, output);
 
             // Assert
@@ -245,10 +246,9 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             Assert.Equal(expectedPostContent, output.PostContent.GetContent());
             Assert.Equal(expectedTagName, output.TagName);
 
-            Assert.NotNull(viewContext.FormContext?.FormData);
             Assert.Single(
-                viewContext.FormContext.FormData,
-                entry => entry.Key == SelectTagHelper.SelectedValuesFormDataKey);
+                tagHelperContext.Items,
+                entry => (Type)entry.Key == typeof(SelectTagHelper));
         }
 
         [Theory]
@@ -333,6 +333,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             };
 
             // Act
+            tagHelper.Init(tagHelperContext);
             await tagHelper.ProcessAsync(tagHelperContext, output);
 
             // Assert
@@ -343,10 +344,9 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             Assert.Equal(expectedPostContent, HtmlContentUtilities.HtmlContentToString(output.PostContent));
             Assert.Equal(expectedTagName, output.TagName);
 
-            Assert.NotNull(viewContext.FormContext?.FormData);
             Assert.Single(
-                viewContext.FormContext.FormData,
-                entry => entry.Key == SelectTagHelper.SelectedValuesFormDataKey);
+                tagHelperContext.Items,
+                entry => (Type)entry.Key == typeof(SelectTagHelper));
 
             Assert.Equal(savedDisabled, items.Select(item => item.Disabled));
             Assert.Equal(savedGroup, items.Select(item => item.Group));
@@ -429,7 +429,6 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             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(htmlGenerator)
             {
                 For = modelExpression,
@@ -438,6 +437,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             };
 
             // Act
+            tagHelper.Init(tagHelperContext);
             await tagHelper.ProcessAsync(tagHelperContext, output);
 
             // Assert
@@ -448,10 +448,9 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             Assert.Equal(expectedPostContent, HtmlContentUtilities.HtmlContentToString(output.PostContent));
             Assert.Equal(expectedTagName, output.TagName);
 
-            Assert.NotNull(viewContext.FormContext?.FormData);
             Assert.Single(
-                viewContext.FormContext.FormData,
-                entry => entry.Key == SelectTagHelper.SelectedValuesFormDataKey);
+                tagHelperContext.Items,
+                entry => (Type)entry.Key == typeof(SelectTagHelper));
 
             Assert.Equal(savedDisabled, items.Select(item => item.Disabled));
             Assert.Equal(savedGroup, items.Select(item => item.Group));
@@ -536,15 +535,15 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             };
 
             // Act
+            tagHelper.Init(tagHelperContext);
             await tagHelper.ProcessAsync(tagHelperContext, output);
 
             // Assert
             htmlGenerator.Verify();
 
-            Assert.NotNull(viewContext.FormContext?.FormData);
             var keyValuePair = Assert.Single(
-                viewContext.FormContext.FormData,
-                entry => entry.Key == SelectTagHelper.SelectedValuesFormDataKey);
+                tagHelperContext.Items,
+                entry => (Type)entry.Key == typeof(SelectTagHelper));
             Assert.Same(currentValues, keyValuePair.Value);
         }
 
@@ -610,15 +609,15 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             };
 
             // Act
+            tagHelper.Init(tagHelperContext);
             await tagHelper.ProcessAsync(tagHelperContext, output);
 
             // Assert
             htmlGenerator.Verify();
 
-            Assert.NotNull(viewContext.FormContext?.FormData);
             var keyValuePair = Assert.Single(
-                viewContext.FormContext.FormData,
-                entry => entry.Key == SelectTagHelper.SelectedValuesFormDataKey);
+                tagHelperContext.Items,
+                entry => (Type)entry.Key == typeof(SelectTagHelper));
             Assert.Same(currentValues, keyValuePair.Value);
         }
 
diff --git a/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/Order.cshtml b/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/Order.cshtml
index 22a634d701..aa636d6e04 100644
--- a/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/Order.cshtml
+++ b/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/Order.cshtml
@@ -52,6 +52,9 @@
                 <option value="Credit">Credit</option>
                 <option value="Check">Check</option>
             </select>
+            <select>
+                <option value="Check">Check</option>
+            </select>
         </div>
         <div>
             <label asp-for="Customer.Number" class="order"></label>
diff --git a/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/OrderUsingHtmlHelpers.cshtml b/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/OrderUsingHtmlHelpers.cshtml
index 2ced6a94a9..5cde504726 100644
--- a/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/OrderUsingHtmlHelpers.cshtml
+++ b/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/OrderUsingHtmlHelpers.cshtml
@@ -43,6 +43,9 @@ Html.BeginForm(actionName: "Submit", controllerName: "HtmlGeneration_Order"))
         <div>
             @Html.LabelFor(m => m.PaymentMethod, htmlAttributes: new { @class = "order" })
             @Html.ListBoxFor(m => m.PaymentMethod, selectList: new SelectList(new[] { new { value = "Credit" }, new { value = "Check" } }, dataValueField: "value", dataTextField: "value"))
+            <select>
+                <option value="Check">Check</option>
+            </select>
         </div>
         <div>
             @Html.LabelFor(m => m.Customer.Number, htmlAttributes: new { @class = "order" })