CompositeModelBinder should only validate the object graph at the top

level.
DataAnnotationsModelValidator should prefer the container to Metadata.Model

Fixes #247
This commit is contained in:
Pranav K 2014-04-25 12:55:03 -07:00
parent 18d3395a5e
commit 904c91d2b9
3 changed files with 91 additions and 20 deletions

View File

@ -15,7 +15,6 @@
// See the Apache 2 License for the specific language governing // See the Apache 2 License for the specific language governing
// permissions and limitations under the License. // permissions and limitations under the License.
using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq; using System.Linq;
using System.Threading.Tasks; using System.Threading.Tasks;
@ -44,7 +43,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
private IModelBinder[] Binders { get; set; } private IModelBinder[] Binders { get; set; }
public virtual async Task<bool> BindModelAsync(ModelBindingContext bindingContext) public virtual async Task<bool> BindModelAsync([NotNull] ModelBindingContext bindingContext)
{ {
var newBindingContext = CreateNewBindingContext(bindingContext, var newBindingContext = CreateNewBindingContext(bindingContext,
bindingContext.ModelName, bindingContext.ModelName,
@ -66,22 +65,30 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
return false; // something went wrong return false; // something went wrong
} }
// run validation and return the model // Only perform validation at the root of the object graph. ValidationNode will recursively walk the graph.
// If we fell back to an empty prefix above and are dealing with simple types, // Ignore ComplexModelDto since it essentially wraps the primary object.
// propagate the non-blank model name through for user clarity in validation errors. if (newBindingContext.ModelMetadata.ContainerType == null &&
// Complex types will reveal their individual properties as model names and do not require this. newBindingContext.ModelMetadata.ModelType != typeof(ComplexModelDto))
if (!newBindingContext.ModelMetadata.IsComplexType && String.IsNullOrEmpty(newBindingContext.ModelName))
{ {
newBindingContext.ValidationNode = new ModelValidationNode(newBindingContext.ModelMetadata, bindingContext.ModelName); // run validation and return the model
// If we fell back to an empty prefix above and are dealing with simple types,
// propagate the non-blank model name through for user clarity in validation errors.
// Complex types will reveal their individual properties as model names and do not require this.
if (!newBindingContext.ModelMetadata.IsComplexType && string.IsNullOrEmpty(newBindingContext.ModelName))
{
newBindingContext.ValidationNode = new ModelValidationNode(newBindingContext.ModelMetadata,
bindingContext.ModelName);
}
var validationContext = new ModelValidationContext(bindingContext.MetadataProvider,
bindingContext.ValidatorProviders,
bindingContext.ModelState,
bindingContext.ModelMetadata,
containerMetadata: null);
newBindingContext.ValidationNode.Validate(validationContext, parentNode: null);
} }
var validationContext = new ModelValidationContext(bindingContext.MetadataProvider,
bindingContext.ValidatorProviders,
bindingContext.ModelState,
bindingContext.ModelMetadata,
containerMetadata: null);
newBindingContext.ValidationNode.Validate(validationContext, parentNode: null);
bindingContext.Model = newBindingContext.Model; bindingContext.Model = newBindingContext.Model;
return true; return true;
} }

View File

@ -40,8 +40,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
{ {
var metadata = validationContext.ModelMetadata; var metadata = validationContext.ModelMetadata;
var memberName = metadata.PropertyName ?? metadata.ModelType.Name; var memberName = metadata.PropertyName ?? metadata.ModelType.Name;
var instance = metadata.Model ?? validationContext.ContainerMetadata.Model; var containerMetadata = validationContext.ContainerMetadata;
var context = new ValidationContext(instance) var container = containerMetadata != null ? containerMetadata.Model : null;
var context = new ValidationContext(container ?? metadata.Model)
{ {
DisplayName = metadata.GetDisplayName(), DisplayName = metadata.GetDisplayName(),
MemberName = memberName MemberName = memberName

View File

@ -18,6 +18,7 @@
#if NET45 #if NET45
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Linq; using System.Linq;
using System.Threading.Tasks; using System.Threading.Tasks;
using Microsoft.Framework.DependencyInjection; using Microsoft.Framework.DependencyInjection;
@ -234,10 +235,56 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
Assert.Equal("name", model.Friends[1].LastName); Assert.Equal("name", model.Friends[1].LastName);
} }
[Fact]
public async Task BindModel_WithDefaultValidators_ValidatesSubProperties()
{
// Arrange
var validatorProvider = new DataAnnotationsModelValidatorProvider();
var binder = CreateBinderWithDefaults();
var valueProvider = new SimpleHttpValueProvider
{
{ "user.password", "password-val" },
{ "user.confirmpassword", "not-password-val" },
};
var bindingContext = CreateBindingContext(binder, valueProvider, typeof(User), new[] { validatorProvider });
bindingContext.ModelName = "user";
// Act
var isBound = await binder.BindModelAsync(bindingContext);
// Assert
var error = Assert.Single(bindingContext.ModelState["user.confirmpassword"].Errors);
Assert.Equal("'ConfirmPassword' and 'Password' do not match.", error.ErrorMessage);
}
[Fact]
public async Task BindModel_WithDefaultValidators_ValidatesInstance()
{
// Arrange
var validatorProvider = new DataAnnotationsModelValidatorProvider();
var binder = CreateBinderWithDefaults();
var valueProvider = new SimpleHttpValueProvider
{
{ "user.password", "password" },
{ "user.confirmpassword", "password" },
};
var bindingContext = CreateBindingContext(binder, valueProvider, typeof(User), new[] { validatorProvider });
bindingContext.ModelName = "user";
// Act
var isBound = await binder.BindModelAsync(bindingContext);
// Assert
var error = Assert.Single(bindingContext.ModelState["user"].Errors);
Assert.Equal("Password does not meet complexity requirements.", error.ErrorMessage);
}
private static ModelBindingContext CreateBindingContext(IModelBinder binder, private static ModelBindingContext CreateBindingContext(IModelBinder binder,
IValueProvider valueProvider, IValueProvider valueProvider,
Type type) Type type,
IEnumerable<IModelValidatorProvider> validatorProviders = null)
{ {
validatorProviders = validatorProviders ?? Enumerable.Empty<IModelValidatorProvider>();
var metadataProvider = new DataAnnotationsModelMetadataProvider(); var metadataProvider = new DataAnnotationsModelMetadataProvider();
var bindingContext = new ModelBindingContext var bindingContext = new ModelBindingContext
{ {
@ -247,7 +294,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
ModelMetadata = metadataProvider.GetMetadataForType(null, type), ModelMetadata = metadataProvider.GetMetadataForType(null, type),
ModelState = new ModelStateDictionary(), ModelState = new ModelStateDictionary(),
ValueProvider = valueProvider, ValueProvider = valueProvider,
ValidatorProviders = Enumerable.Empty<IModelValidatorProvider>() ValidatorProviders = validatorProviders
}; };
return bindingContext; return bindingContext;
} }
@ -287,6 +334,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
public List<Person> Friends { get; set; } public List<Person> Friends { get; set; }
} }
private class User : IValidatableObject
{
public string Password { get; set; }
[Compare("Password")]
public string ConfirmPassword { get; set; }
public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
{
if (Password == "password")
{
yield return new ValidationResult("Password does not meet complexity requirements.");
}
}
}
} }
} }
#endif #endif