From e5cb6f959544b95efa0616741891f0dc81e645d2 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Thu, 14 Jul 2016 16:47:39 -0700 Subject: [PATCH] Add `null` check for `ModelStateEntry.Children` - #4989 --- .../Internal/ValidationHelpers.cs | 3 +- .../TestableHtmlGenerator.cs | 17 +- .../ValidationSummaryTagHelperTest.cs | 178 +++++++++++++++++- .../HtmlHelperValidationSummaryTest.cs | 9 + 4 files changed, 197 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ValidationHelpers.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ValidationHelpers.cs index b606ec2693..25e683468f 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ValidationHelpers.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ValidationHelpers.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Diagnostics; -using System.Linq; using Microsoft.AspNetCore.Mvc.ModelBinding; namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal @@ -90,7 +89,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal ModelMetadata metadata, List orderedModelStateEntries) { - if (metadata.ElementMetadata != null) + if (metadata.ElementMetadata != null && modelStateEntry.Children != null) { foreach (var indexEntry in modelStateEntry.Children) { diff --git a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/TestableHtmlGenerator.cs b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/TestableHtmlGenerator.cs index b801f7d5fe..2cd9c0b4e3 100644 --- a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/TestableHtmlGenerator.cs +++ b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/TestableHtmlGenerator.cs @@ -65,8 +65,21 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers IHtmlGenerator htmlGenerator, IModelMetadataProvider metadataProvider) { - var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()); - var viewData = new ViewDataDictionary(metadataProvider) + return GetViewContext(model, htmlGenerator, metadataProvider, modelState: new ModelStateDictionary()); + } + + public static ViewContext GetViewContext( + object model, + IHtmlGenerator htmlGenerator, + IModelMetadataProvider metadataProvider, + ModelStateDictionary modelState) + { + var actionContext = new ActionContext( + new DefaultHttpContext(), + new RouteData(), + new ActionDescriptor(), + modelState); + var viewData = new ViewDataDictionary(metadataProvider, modelState) { Model = model, }; diff --git a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ValidationSummaryTagHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ValidationSummaryTagHelperTest.cs index cd5c74a2df..82b0ab58ec 100644 --- a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ValidationSummaryTagHelperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ValidationSummaryTagHelperTest.cs @@ -23,14 +23,153 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers { public class ValidationSummaryTagHelperTest { - [Fact] - public async Task ProcessAsync_GeneratesExpectedOutput() + public static TheoryData ProcessAsync_GeneratesExpectedOutput_WithNoErrorsData + { + get + { + var emptyModelState = new ModelStateDictionary(); + + var modelState = new ModelStateDictionary(); + SetValidModelState(modelState); + + return new TheoryData + { + emptyModelState, + modelState, + }; + } + } + + [Theory] + [MemberData(nameof(ProcessAsync_GeneratesExpectedOutput_WithNoErrorsData))] + public async Task ProcessAsync_GeneratesExpectedOutput_WithNoErrors( + ModelStateDictionary modelState) { // Arrange var expectedTagName = "not-div"; var metadataProvider = new TestModelMetadataProvider(); var htmlGenerator = new TestableHtmlGenerator(metadataProvider); + var expectedPreContent = "original pre-content"; + var expectedContent = "original content"; + var tagHelperContext = new TagHelperContext( + allAttributes: new TagHelperAttributeList( + Enumerable.Empty()), + items: new Dictionary(), + uniqueId: "test"); + var output = new TagHelperOutput( + expectedTagName, + attributes: new TagHelperAttributeList + { + { "class", "form-control" } + }, + getChildContentAsync: (useCachedResult, encoder) => + { + var tagHelperContent = new DefaultTagHelperContent(); + tagHelperContent.SetContent("Something"); + return Task.FromResult(tagHelperContent); + }); + output.PreContent.SetContent(expectedPreContent); + output.Content.SetContent(expectedContent); + output.PostContent.SetContent("Custom Content"); + + var model = new Model(); + var viewContext = TestableHtmlGenerator.GetViewContext(model, htmlGenerator, metadataProvider, modelState); + var validationSummaryTagHelper = new ValidationSummaryTagHelper(htmlGenerator) + { + ValidationSummary = ValidationSummary.All, + ViewContext = viewContext, + }; + + // Act + await validationSummaryTagHelper.ProcessAsync(tagHelperContext, output); + + // Assert + Assert.Equal(2, output.Attributes.Count); + var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("class")); + Assert.Equal("form-control validation-summary-valid", attribute.Value); + attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("data-valmsg-summary")); + Assert.Equal("true", attribute.Value); + Assert.Equal(expectedPreContent, output.PreContent.GetContent()); + Assert.Equal(expectedContent, output.Content.GetContent()); + Assert.Equal( + $"Custom Content
  • {Environment.NewLine}
", + output.PostContent.GetContent()); + Assert.Equal(expectedTagName, output.TagName); + } + + [Theory] + [InlineData(ValidationSummary.All)] + [InlineData(ValidationSummary.ModelOnly)] + public async Task ProcessAsync_GeneratesExpectedOutput_WithModelError(ValidationSummary validationSummary) + { + // Arrange + var expectedError = "I am an error."; + var expectedTagName = "not-div"; + var metadataProvider = new TestModelMetadataProvider(); + var htmlGenerator = new TestableHtmlGenerator(metadataProvider); + + var validationSummaryTagHelper = new ValidationSummaryTagHelper(htmlGenerator) + { + ValidationSummary = validationSummary, + }; + + var expectedPreContent = "original pre-content"; + var expectedContent = "original content"; + var tagHelperContext = new TagHelperContext( + allAttributes: new TagHelperAttributeList( + Enumerable.Empty()), + items: new Dictionary(), + uniqueId: "test"); + var output = new TagHelperOutput( + expectedTagName, + attributes: new TagHelperAttributeList + { + { "class", "form-control" } + }, + getChildContentAsync: (useCachedResult, encoder) => + { + var tagHelperContent = new DefaultTagHelperContent(); + tagHelperContent.SetContent("Something"); + return Task.FromResult(tagHelperContent); + }); + output.PreContent.SetContent(expectedPreContent); + output.Content.SetContent(expectedContent); + output.PostContent.SetContent("Custom Content"); + + var model = new Model(); + var viewContext = TestableHtmlGenerator.GetViewContext(model, htmlGenerator, metadataProvider); + validationSummaryTagHelper.ViewContext = viewContext; + + var modelState = viewContext.ModelState; + SetValidModelState(modelState); + modelState.AddModelError(string.Empty, expectedError); + + // Act + await validationSummaryTagHelper.ProcessAsync(tagHelperContext, output); + + // Assert + Assert.InRange(output.Attributes.Count, low: 1, high: 2); + var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("class")); + Assert.Equal("form-control validation-summary-errors", attribute.Value); + Assert.Equal(expectedPreContent, output.PreContent.GetContent()); + Assert.Equal(expectedContent, output.Content.GetContent()); + Assert.Equal( + $"Custom Content
  • {expectedError}
    • {Environment.NewLine}
", + output.PostContent.GetContent()); + Assert.Equal(expectedTagName, output.TagName); + } + + [Fact] + public async Task ProcessAsync_GeneratesExpectedOutput_WithPropertyErrors() + { + // Arrange + var expectedError0 = "I am an error."; + var expectedError2 = "I am also an error."; + var expectedTagName = "not-div"; + var metadataProvider = new TestModelMetadataProvider(); + var htmlGenerator = new TestableHtmlGenerator(metadataProvider); + var validationSummaryTagHelper = new ValidationSummaryTagHelper(htmlGenerator) { ValidationSummary = ValidationSummary.All, @@ -59,23 +198,30 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers output.Content.SetContent(expectedContent); output.PostContent.SetContent("Custom Content"); - Model model = null; + var model = new Model(); var viewContext = TestableHtmlGenerator.GetViewContext(model, htmlGenerator, metadataProvider); validationSummaryTagHelper.ViewContext = viewContext; + var modelState = viewContext.ModelState; + SetValidModelState(modelState); + modelState.AddModelError(key: $"{nameof(Model.Strings)}[0]", errorMessage: expectedError0); + modelState.AddModelError(key: $"{nameof(Model.Strings)}[2]", errorMessage: expectedError2); + // Act await validationSummaryTagHelper.ProcessAsync(tagHelperContext, output); // Assert Assert.Equal(2, output.Attributes.Count); var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("class")); - Assert.Equal("form-control validation-summary-valid", attribute.Value); + Assert.Equal("form-control validation-summary-errors", attribute.Value); attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("data-valmsg-summary")); Assert.Equal("true", attribute.Value); Assert.Equal(expectedPreContent, output.PreContent.GetContent()); Assert.Equal(expectedContent, output.Content.GetContent()); - Assert.Equal("Custom Content
  • " + Environment.NewLine + "
", - output.PostContent.GetContent()); + Assert.Equal( + $"Custom Content
  • {expectedError0}
    • {Environment.NewLine}" + + $"
  • {expectedError2}
    • {Environment.NewLine}
", + output.PostContent.GetContent()); Assert.Equal(expectedTagName, output.TagName); } @@ -337,9 +483,29 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers new HtmlHelperOptions()); } + private static void SetValidModelState(ModelStateDictionary modelState) + { + modelState.SetModelValue(key: nameof(Model.Empty), rawValue: null, attemptedValue: null); + modelState.SetModelValue(key: $"{nameof(Model.Strings)}[0]", rawValue: null, attemptedValue: null); + modelState.SetModelValue(key: $"{nameof(Model.Strings)}[1]", rawValue: null, attemptedValue: null); + modelState.SetModelValue(key: $"{nameof(Model.Strings)}[2]", rawValue: null, attemptedValue: null); + modelState.SetModelValue(key: nameof(Model.Text), rawValue: null, attemptedValue: null); + + foreach (var key in modelState.Keys) + { + modelState.MarkFieldValid(key); + } + } + private class Model { public string Text { get; set; } + + public string[] Strings { get; set; } + + // Exists to ensure #4989 does not regress. Issue specific to case where collection has a ModelStateEntry + // but no element does. + public byte[] Empty { get; set; } } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/HtmlHelperValidationSummaryTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/HtmlHelperValidationSummaryTest.cs index 80af7b56e9..e89698eb2a 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/HtmlHelperValidationSummaryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/HtmlHelperValidationSummaryTest.cs @@ -672,6 +672,8 @@ namespace Microsoft.AspNetCore.Mvc.Rendering modelState.AddModelError("Property3.Property2", "This is an error for Property3.Property2."); modelState.AddModelError("Property3.OrderedProperty3", "This is an error for Property3.OrderedProperty3."); modelState.AddModelError("Property3.OrderedProperty2", "This is an error for Property3.OrderedProperty2."); + modelState.SetModelValue("Property3.Empty", rawValue: null, attemptedValue: null); + modelState.MarkFieldValid("Property3.Empty"); var provider = new EmptyModelMetadataProvider(); var metadata = provider.GetMetadataForProperty(typeof(ValidationModel), nameof(ValidationModel.Property3)); @@ -712,6 +714,9 @@ namespace Microsoft.AspNetCore.Mvc.Rendering modelState.AddModelError("OrderedProperty1", "This is an error for OrderedProperty1."); modelState.AddModelError("OrderedProperty2", "This is yet-another error for OrderedProperty2."); + + modelState.SetModelValue("Empty", rawValue: null, attemptedValue: null); + modelState.MarkFieldValid("Empty"); } private class ValidationModel @@ -738,6 +743,10 @@ namespace Microsoft.AspNetCore.Mvc.Rendering public string OrderedProperty2 { get; set; } [Display(Order = 23)] public string OrderedProperty1 { get; set; } + + // Exists to ensure #4989 does not regress. Issue specific to case where collection has a ModelStateEntry + // but no element does. + public byte[] Empty { get; set; } } private class ModelWithCollection