diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs index e4780832d7..7d28e706fd 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs @@ -98,7 +98,7 @@ public bool Validate(ModelMetadata metadata, string key, object model) protected virtual bool ValidateNode() { var state = _modelState.GetValidationState(_key); - if (state == ModelValidationState.Unvalidated) + if (state != ModelValidationState.Invalid) { var validators = _validatorCache.GetValidators(_metadata, _validatorProvider); diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs index 9cd8c5a1af..1570e2cc83 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs @@ -20,12 +20,15 @@ public static class ModelBindingTestHelper { public static OperationBindingContext GetOperationBindingContext( Action updateRequest = null, - Action updateOptions = null) + Action updateOptions = null, + ControllerActionDescriptor actionDescriptor = null) { var httpContext = GetHttpContext(updateRequest, updateOptions); var services = httpContext.RequestServices; - var actionContext = new ActionContext(httpContext, new RouteData(), new ControllerActionDescriptor()); + actionDescriptor = actionDescriptor ?? new ControllerActionDescriptor(); + + var actionContext = new ActionContext(httpContext, new RouteData(), actionDescriptor); var controllerContext = GetControllerContext( services.GetRequiredService>().Value, actionContext); diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs index 8862dacc35..c2c462db87 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs @@ -1,6 +1,7 @@ // 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.ComponentModel.DataAnnotations; using System.IO; @@ -9,6 +10,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Newtonsoft.Json.Linq; @@ -18,6 +20,99 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests { public class ValidationIntegrationTests { + private class TransferInfo + { + [Range(25, 50)] + public int AccountId { get; set; } + + public double Amount { get; set; } + } + + private class TestController { } + + public static TheoryData> MultipleActionParametersAndValidationData + { + get + { + return new TheoryData> + { + // Irrespective of the order in which the parameters are defined on the action, + // the validation on the TransferInfo's AccountId should occur. + new List() + { + new ParameterDescriptor() + { + Name = "accountId", + ParameterType = typeof(int) + }, + new ParameterDescriptor() + { + Name = "transferInfo", + ParameterType = typeof(TransferInfo), + BindingInfo = new BindingInfo() + { + BindingSource = BindingSource.Body + } + } + }, + new List() + { + new ParameterDescriptor() + { + Name = "transferInfo", + ParameterType = typeof(TransferInfo), + BindingInfo = new BindingInfo() + { + BindingSource = BindingSource.Body + } + }, + new ParameterDescriptor() + { + Name = "accountId", + ParameterType = typeof(int) + } + } + }; + } + } + + [Theory] + [MemberData(nameof(MultipleActionParametersAndValidationData))] + public async Task ValidationIsTriggered_OnFromBodyModels(List parameters) + { + // Arrange + var actionDescriptor = new ControllerActionDescriptor() + { + BoundProperties = new List(), + Parameters = parameters + }; + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + + var operationContext = ModelBindingTestHelper.GetOperationBindingContext( + request => + { + request.QueryString = new QueryString("?accountId=30"); + request.Body = new MemoryStream(Encoding.UTF8.GetBytes("{\"accountId\": 15,\"amount\": 250.0}")); + request.ContentType = "application/json"; + }, + actionDescriptor: actionDescriptor); + + var modelState = operationContext.ActionContext.ModelState; + + // Act + var arguments = await argumentBinder.BindActionArgumentsAsync( + (ControllerContext)operationContext.ActionContext, new TestController()); + + // Assert + Assert.False(modelState.IsValid); + + var entry = Assert.Single( + modelState, + e => string.Equals(e.Key, "AccountId", StringComparison.OrdinalIgnoreCase)).Value; + var error = Assert.Single(entry.Errors); + Assert.Equal("The field AccountId must be between 25 and 50.", error.ErrorMessage); + } + private class Order1 { [Required] @@ -1188,7 +1283,7 @@ public async Task FromBody_JToken_ExcludedFromValidation() var httpContext = operationContext.HttpContext; var modelState = operationContext.ActionContext.ModelState; - // We need to add another model state entry which should get marked as skipped so + // We need to add another model state entry which should get marked as skipped so // we can prove that the JObject was skipped. modelState.SetModelValue("CustomParameter.message", "Hello", "Hello"); @@ -1211,7 +1306,7 @@ public async Task FromBody_JToken_ExcludedFromValidation() // Regression test for https://github.com/aspnet/Mvc/issues/3743 // // A cancellation token that's bound with the empty prefix will end up suppressing - // the empty prefix. Since the empty prefix is a prefix of everything, this will + // the empty prefix. Since the empty prefix is a prefix of everything, this will // basically result in clearing out all model errors, which is BAD. // // The fix is to treat non-user-input as have a key of null, which means that the MSD