huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix #7503 change to model name for IValidableObject 

This change undoes a breaking change introduced by the 2.1 model
validation changes. Now an implementation of IValidableObject on a
top-level model will be called correctly with the 'empty' prefix instead
of the parameter name.

When fixing this we undid a workaround for another issue.

When validating a parameter that didn't bind we didn't correctly compute
the model name for 'fallback to empty prefix' cases.

(cherry picked from commit 7a1096a72b)
This commit is contained in:
Ryan Nowak 2018-03-29 12:51:25 -07:00 committed by Ryan Nowak
parent 264f9c871e
commit f20bf9ea02
4 changed files with 147 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs
View File

@ -249,6 +249,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
EnforceBindRequiredAndValidate(
baseObjectValidator,
actionContext,
parameter,
metadata,
modelBindingContext,
modelBindingResult);
@ -276,6 +277,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
private void EnforceBindRequiredAndValidate(
ObjectModelValidator baseObjectValidator,
ActionContext actionContext,
ParameterDescriptor parameter,
ModelMetadata metadata,
ModelBindingContext modelBindingContext,
ModelBindingResult modelBindingResult)
@ -287,7 +289,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
var message = metadata.ModelBindingMessageProvider.MissingBindRequiredValueAccessor(modelName);
actionContext.ModelState.TryAddModelError(modelName, message);
}
else if (modelBindingResult.IsModelSet || metadata.IsRequired)
else if (modelBindingResult.IsModelSet)
{
// Enforce any other validation rules
baseObjectValidator.Validate(
@ -297,6 +299,35 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
modelBindingResult.Model,
metadata);
}
else if (metadata.IsRequired)
{
// We need to special case the model name for cases where a 'fallback' to empty
// prefix occurred but binding wasn't successful. For these cases there will be no
// entry in validation state to match and determine the correct key.
//
// See https://github.com/aspnet/Mvc/issues/7503
//
// This is to avoid adding validation errors for an 'empty' prefix when a simple
// type fails to bind. The fix for #7503 uncovered this issue, and was likely the
// original problem being worked around that regressed #7503.
string modelName = modelBindingContext.ModelName;
if (string.IsNullOrEmpty(modelBindingContext.ModelName) &&
(parameter.BindingInfo?.BinderModelName ?? metadata.BinderModelName) == null)
{
// If we get here then this is a fallback case. The model name wasn't explicitly set
// and we ended up with an empty prefix.
modelName = modelBindingContext.FieldName;
}
// Run validation, we expect this to validate [Required].
baseObjectValidator.Validate(
actionContext,
modelBindingContext.ValidationState,
modelName,
modelBindingResult.Model,
metadata);
}
}
}
}

9
src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs
View File

@ -146,13 +146,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation
var result = results[i];
var key = ModelNames.CreatePropertyModelName(Key, result.MemberName);
// If this is a top-level parameter/property, the key would be empty.
// So, use the name of the top-level property/property.
if (string.IsNullOrEmpty(key) && Metadata.Name != null)
{
key = Metadata.Name;
}
// It's OK for key to be the empty string here. This can happen when a top
// level object implements IValidatableObject.
ModelState.TryAddModelError(key, result.Message);
}
}

2
test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs
View File

@ -381,7 +381,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
ModelMetadata metadata)
{
// Arrange
var expectedKey = metadata.Name ?? string.Empty;
var expectedKey = string.Empty;
var expectedFieldName = metadata.Name ?? nameof(Person);
var actionContext = GetControllerContext();

115
test/Microsoft.AspNetCore.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs
View File

@ -578,7 +578,10 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
[InlineData("displayNameStringLengthParam", "abc", true)]
[InlineData("displayNameStringLengthParam", "abcTooLong", false, "My Display Name")]
public async Task ActionParameter_EnforcesDataAnnotationsAttributes(
string paramName, string input, bool isValid, string displayName = null)
string paramName,
string input,
bool isValid,
string displayName = null)
{
// Arrange
var parameterBinder = ModelBindingTestHelper.GetParameterBinder();
@ -614,11 +617,114 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
Assert.Equal(isValid, modelState.IsValid);
if (!isValid)
{
var message = modelState[paramName].Errors.Single().ErrorMessage;
var entry = modelState[paramName];
Assert.NotNull(entry);
var message = entry.Errors.Single().ErrorMessage;
Assert.Contains(displayName ?? parameter.Name, message);
}
}
[Fact]
public async Task ActionParameter_CanRunIValidatableObject_EmptyPrefix()
{
// Arrange
var parameterBinder = ModelBindingTestHelper.GetParameterBinder();
var parameterInfo = BindingAndValidationController.ValidatableObjectParameterInfo;
var parameter = new ParameterDescriptor()
{
Name = parameterInfo.Name,
ParameterType = parameterInfo.ParameterType,
};
var testContext = ModelBindingTestHelper.GetTestContext(request =>
{
request.QueryString = QueryString.Create(nameof(ModelWithIValidatableObject.FirstName), "Billy");
});
var modelState = testContext.ModelState;
var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
var modelMetadata = modelMetadataProvider
.GetMetadataForParameter(parameterInfo);
// Act
var modelBindingResult = await parameterBinder.BindModelAsync(
parameter,
testContext,
modelMetadataProvider,
modelMetadata);
// Assert
Assert.True(modelBindingResult.IsModelSet, "model is set");
Assert.False(modelState.IsValid, "model is valid");
var entry = modelState[string.Empty];
Assert.NotNull(entry);
var message = entry.Errors.Single().ErrorMessage;
Assert.Equal("Not valid.", message);
entry = modelState[nameof(ModelWithIValidatableObject.FirstName)];
Assert.NotNull(entry);
message = entry.Errors.Single().ErrorMessage;
Assert.Equal("FirstName Not valid.", message);
}
[Fact]
public async Task ActionParameter_CanRunIValidatableObject_WithPrefix()
{
// Arrange
var parameterBinder = ModelBindingTestHelper.GetParameterBinder();
var parameterInfo = BindingAndValidationController.ValidatableObjectParameterInfo;
var parameter = new ParameterDescriptor()
{
Name = parameterInfo.Name,
ParameterType = parameterInfo.ParameterType,
};
var testContext = ModelBindingTestHelper.GetTestContext(request =>
{
var key = ModelNames.CreatePropertyModelName(parameter.Name, nameof(ModelWithIValidatableObject.FirstName));
request.QueryString = QueryString.Create(key, "Billy");
});
var modelState = testContext.ModelState;
var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
var modelMetadata = modelMetadataProvider
.GetMetadataForParameter(parameterInfo);
// Act
var modelBindingResult = await parameterBinder.BindModelAsync(
parameter,
testContext,
modelMetadataProvider,
modelMetadata);
// Assert
Assert.True(modelBindingResult.IsModelSet, "model is set");
Assert.False(modelState.IsValid, "model is valid");
var entry = modelState[parameter.Name];
Assert.NotNull(entry);
var message = entry.Errors.Single().ErrorMessage;
Assert.Equal("Not valid.", message);
entry = modelState[ModelNames.CreatePropertyModelName(parameter.Name, nameof(ModelWithIValidatableObject.FirstName))];
Assert.NotNull(entry);
message = entry.Errors.Single().ErrorMessage;
Assert.Equal("FirstName Not valid.", message);
}
private class ModelWithIValidatableObject : IValidatableObject
{
public string FirstName { get; set; }
public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
{
yield return new ValidationResult("Not valid.");
yield return new ValidationResult("FirstName Not valid.", new string[] { nameof(FirstName) });
}
}
private struct PointStruct
{
public PointStruct(double x, double y)
@ -669,7 +775,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
[BindNever] int bindNeverParam,
[BindRequired] int bindRequiredParam,
[Required, StringLength(3)] string requiredAndStringLengthParam,
[Display(Name = "My Display Name"), StringLength(3)] string displayNameStringLengthParam)
[Display(Name = "My Display Name"), StringLength(3)] string displayNameStringLengthParam,
ModelWithIValidatableObject validatableObject)
{
}
@ -682,6 +789,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
public static ParameterInfo BindRequiredParamInfo
=> MyActionMethodInfo.GetParameters()[1];
public static ParameterInfo ValidatableObjectParameterInfo => MyActionMethodInfo.GetParameters()[4];
public static ParameterInfo GetParameterInfo(string parameterName)
{
return MyActionMethodInfo