Merge pull request #7680 from aspnet/release/2.1

Allow PartialTagHelper to specify a null model. Fixes #7667

src/Microsoft.AspNetCore.Mvc.TagHelpers/PartialTagHelper.cs

@@ -22,6 +22,10 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
     {
         private const string ForAttributeName = "for";
         private const string ModelAttributeName = "model";
+        private object _model;
+        private bool _hasModel;
+        private bool _hasFor;
+        private ModelExpression _for;
 
         private readonly ICompositeViewEngine _viewEngine;
         private readonly IViewBufferScope _viewBufferScope;
@@ -43,13 +47,29 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
         /// An expression to be evaluated against the current model. Cannot be used together with <see cref="Model"/>.
         /// </summary>
         [HtmlAttributeName(ForAttributeName)]
-        public ModelExpression For { get; set; }
+        public ModelExpression For
+        {
+            get => _for;
+            set
+            {
+                _for = value ?? throw new ArgumentNullException(nameof(value));
+                _hasFor = true;
+            }
+        }
 
         /// <summary>
         /// The model to pass into the partial view. Cannot be used together with <see cref="For"/>.
         /// </summary>
         [HtmlAttributeName(ModelAttributeName)]
-        public object Model { get; set; }
+        public object Model
+        {
+            get => _model;
+            set
+            {
+                _model = value;
+                _hasModel = true;
+            }
+        }
 
         /// <summary>
         /// A <see cref="ViewDataDictionary"/> to pass into the partial view.
@@ -88,7 +108,12 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
         // Internal for testing
         internal object ResolveModel()
         {
-            if (Model != null & For != null)
+            // 1. Disallow specifying values for both Model and For
+            // 2. If a Model was assigned, use it even if it's null.
+            // 3. For cannot have a null value. Use it if it was assigned to.
+            // 4. Fall back to using the Model property on ViewContext.ViewData if none of the above conditions are met.
+
+            if (_hasFor && _hasModel)
             {
                 throw new InvalidOperationException(
                     Resources.FormatPartialTagHelper_InvalidModelAttributes(
@@ -97,17 +122,17 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
                         ModelAttributeName));
             }
 
-            if (Model != null)
+            if (_hasModel)
             {
                 return Model;
             }
 
-            if (For != null)
+            if (_hasFor)
             {
                 return For.Model;
             }
 
-            // Model and For are null, fallback to the ViewContext's ViewData model.
+            // A value for Model or For was not specified, fallback to the ViewContext's ViewData model.
             return ViewContext.ViewData.Model;
         }
 

test/Microsoft.AspNetCore.Mvc.FunctionalTests/HtmlGenerationTest.cs

@@ -9,6 +9,8 @@ using System.Net.Http.Headers;
 using System.Reflection;
 using System.Text;
 using System.Threading.Tasks;
+using AngleSharp.Dom;
+using AngleSharp.Dom.Html;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Mvc.FunctionalTests
@@ -554,6 +556,46 @@ Products: Music Systems, Televisions (3)";
             Assert.Contains("Hello, Private World!", response);
         }
 
+        [Fact]
+        public async Task PartialTagHelper_AllowsPassingModelValue()
+        {
+            // Arrange
+            var url = "/HtmlGeneration_Home/StatusMessage";
+
+            // Act
+            var document = await Client.GetHtmlDocumentAsync(url);
+
+            // Assert
+            var banner = QuerySelector(document, ".banner");
+            Assert.Equal("Some status message", banner.TextContent);
+        }
+
+        [Fact]
+        public async Task PartialTagHelper_AllowsPassingNullModelValue()
+        {
+            // Regression test for https://github.com/aspnet/Mvc/issues/7667.
+            // Arrange
+            var url = "/HtmlGeneration_Home/NullStatusMessage";
+
+            // Act
+            var document = await Client.GetHtmlDocumentAsync(url);
+
+            // Assert
+            var banner = QuerySelector(document, ".banner");
+            Assert.Empty(banner.TextContent);
+        }
+
+        private static IElement QuerySelector(IHtmlDocument document, string selector)
+        {
+            var element = document.QuerySelector(selector);
+            if (element == null)
+            {
+                throw new ArgumentException($"Document does not contain element that matches the selector {selector}: " + Environment.NewLine + document.DocumentElement.OuterHtml);
+            }
+
+            return element;
+        }
+
         private static HttpRequestMessage RequestWithLocale(string url, string locale)
         {
             var request = new HttpRequestMessage(HttpMethod.Get, url);

test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/PartialTagHelperTest.cs

@@ -40,6 +40,28 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
             Assert.Same(expectedModel, model);
         }
 
+        [Fact]
+        public void ResolveModel_ReturnsModelWhenNullValueIsProvided()
+        {
+            // Regression test for https://github.com/aspnet/Mvc/issues/7667.
+            // Arrange
+            var viewData = new ViewDataDictionary(new EmptyModelMetadataProvider(), new ModelStateDictionary())
+            {
+                Model = new object(),
+            };
+            var tagHelper = new PartialTagHelper(Mock.Of<ICompositeViewEngine>(), Mock.Of<IViewBufferScope>())
+            {
+                Model = null,
+                ViewData = viewData,
+            };
+
+            // Act
+            var model = tagHelper.ResolveModel();
+
+            // Assert
+            Assert.Null(model);
+        }
+
         [Fact]
         public void ResolveModel_ReturnsForModelWhenProvided()
         {
@@ -67,7 +89,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
         }
 
         [Fact]
-        public void ResolveModel_ReturnsViewContextsViewDataModelWhenModelAndForAreNull()
+        public void ResolveModel_ReturnsViewContextsViewDataModelWhenModelAndForAreNotSet()
         {
             // Arrange
             var expectedModel = new object();
@@ -107,6 +129,28 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
             Assert.Equal(expectedMessage, exception.Message);
         }
 
+        [Fact]
+        public void ResolveModel_ThrowsWhenNullModelAndForProvided()
+        {
+            // Arrange
+            var modelMetadataProvider = new TestModelMetadataProvider();
+            var containerModel = new TestModel();
+            var containerModelExplorer = modelMetadataProvider.GetModelExplorerForType(
+                typeof(TestModel),
+                containerModel);
+            var propertyModelExplorer = containerModelExplorer.GetExplorerForProperty(nameof(TestModel.Property));
+            var tagHelper = new PartialTagHelper(Mock.Of<ICompositeViewEngine>(), Mock.Of<IViewBufferScope>())
+            {
+                Model = null,
+                For = new ModelExpression("Property", propertyModelExplorer),
+            };
+            var expectedMessage = Resources.FormatPartialTagHelper_InvalidModelAttributes(typeof(PartialTagHelper).FullName, "for", "model");
+
+            // Act & Assert
+            var exception = Assert.Throws<InvalidOperationException>(() => tagHelper.ResolveModel());
+            Assert.Equal(expectedMessage, exception.Message);
+        }
+
         [Fact]
         public async Task ProcessAsync_RendersPartialView_IfGetViewReturnsView()
         {

test/WebSites/HtmlGenerationWebSite/Controllers/HtmlGeneration_HomeController.cs

@@ -239,5 +239,9 @@ namespace HtmlGenerationWebSite.Controllers
         }
 
         public IActionResult PartialTagHelperWithoutModel() => View();
+
+        public IActionResult StatusMessage() => View(new StatusMessageModel { Message = "Some status message"});
+
+        public IActionResult NullStatusMessage() => View("StatusMessage", new StatusMessageModel());
     }
 }

test/WebSites/HtmlGenerationWebSite/Models/StatusMessageModel.cs

@@ -0,0 +1,10 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+namespace HtmlGenerationWebSite.Models
+{
+    public class StatusMessageModel
+    {
+        public string Message { get; set; }
+    }
+}

test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/StatusMessage.cshtml

@@ -0,0 +1,5 @@
+@using HtmlGenerationWebSite.Models
+@model StatusMessageModel
+@addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers
+
+<partial name="_StatusMessagePartial" model="Model.Message" />

test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/_StatusMessagePartial.cshtml

@@ -0,0 +1,2 @@
+@model string
+<div class="banner">@Model</div>