From adeb1ba194ba24663bb41c954c0acbe12c494ced Mon Sep 17 00:00:00 2001 From: Harsh Gupta Date: Mon, 2 Mar 2015 18:12:16 -0800 Subject: [PATCH] Adding support for model binding specifically marked controller properties. --- .../ActionDescriptor.cs | 5 + .../ApplicationModels/ControllerModel.cs | 5 + .../DefaultControllerModelBuilder.cs | 34 +- .../ApplicationModels/PropertyModel.cs | 69 ++++ src/Microsoft.AspNet.Mvc.Core/Controller.cs | 7 +- .../ControllerActionDescriptorBuilder.cs | 28 +- .../ControllerActionInvoker.cs | 4 +- .../DefaultControllerActionArgumentBinder.cs | 74 +++- .../DefaultControllerFactory.cs | 8 +- .../DefaultApiDescriptionProvider.cs | 18 + .../FilterActionInvoker.cs | 4 +- .../IControllerActionArgumentBinder.cs | 9 +- .../Logging/ActionDescriptorValues.cs | 7 + .../Logging/ControllerModelValues.cs | 6 + .../Logging/PropertyModelValues.cs | 39 ++ .../BindingInfo.cs | 7 +- .../Metadata/ModelAttributes.cs | 16 + .../ModelBindingContext.cs | 10 +- .../Validation/DefaultObjectValidator.cs | 46 ++- .../Validation/ModelValidationContext.cs | 26 +- .../ApiController.cs | 7 +- ...erConventionsApplicationModelConvention.cs | 5 +- .../OverloadActionConstraint.cs | 4 +- .../ApplicationModel/ControllerModelTest.cs | 7 +- .../DefaultControllerModelBuilderTest.cs | 30 ++ .../ApplicationModel/PropertyModelTest.cs | 64 ++++ .../ControllerActionDescriptorBuilderTest.cs | 43 +++ ...ControllerActionDescriptorProviderTests.cs | 12 +- .../ControllerActionInvokerTest.cs | 2 + .../DefaultControllerFactoryTest.cs | 52 --- .../DefaultApiDescriptionProviderTest.cs | 91 ++++- .../ControllerActionArgumentBinderTests.cs | 356 ++++++++++-------- .../ModelBindingTest.cs | 84 +++++ .../DataAnnotationsModelValidatorTest.cs | 7 +- .../Validation/DefaultObjectValidatorTests.cs | 1 + .../Controllers/ParameterModelController.cs | 1 + .../ValidateBodyParameterAttribute.cs | 2 +- .../FromBodyControllerPropertyController.cs | 30 ++ .../MultiplePropertiesFromBodyController.cs | 22 ++ .../Controllers/ValidationController.cs | 8 + .../ModelBindingWebSite/ITestService.cs | 5 + .../ModelBindingWebSite/TestService.cs | 5 + 42 files changed, 969 insertions(+), 291 deletions(-) create mode 100644 src/Microsoft.AspNet.Mvc.Core/ApplicationModels/PropertyModel.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/Logging/PropertyModelValues.cs create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/PropertyModelTest.cs create mode 100644 test/WebSites/ModelBindingWebSite/Controllers/FromBodyControllerPropertyController.cs create mode 100644 test/WebSites/ModelBindingWebSite/Controllers/MultiplePropertiesFromBodyController.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionDescriptor.cs b/src/Microsoft.AspNet.Mvc.Core/ActionDescriptor.cs index 5a90af5a35..1e9a3ae008 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionDescriptor.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionDescriptor.cs @@ -30,6 +30,11 @@ namespace Microsoft.AspNet.Mvc public IList Parameters { get; set; } + /// + /// The set of properties which are model bound. + /// + public IList BoundProperties { get; set; } + public IList FilterDescriptors { get; set; } /// diff --git a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ControllerModel.cs b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ControllerModel.cs index c0950a29fc..b79f00e707 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ControllerModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/ControllerModel.cs @@ -26,6 +26,7 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels Filters = new List(); RouteConstraints = new List(); Properties = new Dictionary(); + ControllerProperties = new List(); } public ControllerModel([NotNull] ControllerModel other) @@ -48,6 +49,8 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels ApiExplorer = new ApiExplorerModel(other.ApiExplorer); AttributeRoutes = new List( other.AttributeRoutes.Select(a => new AttributeRouteModel(a))); + ControllerProperties = + new List(other.ControllerProperties.Select(p => new PropertyModel(p))); } public IList ActionConstraints { get; private set; } @@ -76,6 +79,8 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels public TypeInfo ControllerType { get; private set; } + public IList ControllerProperties { get; } + public IList Filters { get; private set; } public IList RouteConstraints { get; private set; } diff --git a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultControllerModelBuilder.cs b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultControllerModelBuilder.cs index eb9ed9883c..7d183a5722 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultControllerModelBuilder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultControllerModelBuilder.cs @@ -10,6 +10,7 @@ using Microsoft.AspNet.Cors; using Microsoft.AspNet.Cors.Core; using Microsoft.AspNet.Mvc.Description; using Microsoft.AspNet.Mvc.Filters; +using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.AspNet.Mvc.Routing; using Microsoft.Framework.Internal; using Microsoft.Framework.Logging; @@ -44,8 +45,9 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels public ControllerModel BuildControllerModel([NotNull] TypeInfo typeInfo) { var controllerModel = CreateControllerModel(typeInfo); + var controllerType = typeInfo.AsType(); - foreach (var methodInfo in typeInfo.AsType().GetMethods()) + foreach (var methodInfo in controllerType.GetMethods()) { var actionModels = _actionModelBuilder.BuildActionModels(typeInfo, methodInfo); if (actionModels != null) @@ -58,6 +60,17 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels } } + foreach (var propertyHelper in PropertyHelper.GetProperties(controllerType)) + { + var propertyInfo = propertyHelper.Property; + var propertyModel = CreatePropertyModel(propertyInfo); + if (propertyModel != null) + { + propertyModel.Controller = controllerModel; + controllerModel.ControllerProperties.Add(propertyModel); + } + } + return controllerModel; } @@ -134,6 +147,25 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels return controllerModel; } + /// + /// Creates a for the given . + /// + /// The . + /// A for the given . + protected virtual PropertyModel CreatePropertyModel([NotNull] PropertyInfo propertyInfo) + { + // CoreCLR returns IEnumerable from GetCustomAttributes - the OfType + // is needed to so that the result of ToArray() is object + var attributes = propertyInfo.GetCustomAttributes(inherit: true).OfType().ToArray(); + var propertyModel = new PropertyModel(propertyInfo, attributes); + var bindingInfo = BindingInfo.GetBindingInfo(attributes); + + propertyModel.BindingInfo = bindingInfo; + propertyModel.PropertyName = propertyInfo.Name; + + return propertyModel; + } + private static void AddRange(IList list, IEnumerable items) { foreach (var item in items) diff --git a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/PropertyModel.cs b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/PropertyModel.cs new file mode 100644 index 0000000000..7f383908ca --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/PropertyModel.cs @@ -0,0 +1,69 @@ +// 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.Collections.Generic; +using System.Diagnostics; +using System.Reflection; +using Microsoft.AspNet.Mvc.ModelBinding; +using Microsoft.Framework.Internal; + +namespace Microsoft.AspNet.Mvc.ApplicationModels +{ + /// + /// A type which is used to represent a property in a . + /// + [DebuggerDisplay("PropertyModel: Name={PropertyName}")] + public class PropertyModel + { + /// + /// Creates a new instance of . + /// + /// The for the underlying property. + /// Any attributes which are annotated on the property. + public PropertyModel([NotNull] PropertyInfo propertyInfo, + [NotNull] IReadOnlyList attributes) + { + PropertyInfo = propertyInfo; + + Attributes = new List(attributes); + } + + /// + /// Creats a new instance of from a given . + /// + /// The which needs to be copied. + public PropertyModel([NotNull] PropertyModel other) + { + Controller = other.Controller; + Attributes = new List(other.Attributes); + BindingInfo = other.BindingInfo; + PropertyInfo = other.PropertyInfo; + PropertyName = other.PropertyName; + } + + /// + /// Gets or sets the this is associated with. + /// + public ControllerModel Controller { get; set; } + + /// + /// Gets any attributes which are annotated on the property. + /// + public IReadOnlyList Attributes { get; } + + /// + /// Gets or sets the associated with this model. + /// + public BindingInfo BindingInfo { get; set; } + + /// + /// Gets the underlying . + /// + public PropertyInfo PropertyInfo { get; } + + /// + /// Gets or sets the name of the property represented by this model. + /// + public string PropertyName { get; set; } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Controller.cs b/src/Microsoft.AspNet.Mvc.Core/Controller.cs index 06d1c9414b..56dd9bec9d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controller.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controller.cs @@ -1293,9 +1293,10 @@ namespace Microsoft.AspNet.Mvc var validationContext = new ModelValidationContext( modelName, - BindingContext.ValidatorProvider, - ModelState, - modelExplorer); + bindingSource: null, + validatorProvider: BindingContext.ValidatorProvider, + modelState: ModelState, + modelExplorer: modelExplorer); ObjectValidator.Validate(validationContext); return ModelState.IsValid; diff --git a/src/Microsoft.AspNet.Mvc.Core/ControllerActionDescriptorBuilder.cs b/src/Microsoft.AspNet.Mvc.Core/ControllerActionDescriptorBuilder.cs index 8ae45aa5ac..68810405df 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ControllerActionDescriptorBuilder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ControllerActionDescriptorBuilder.cs @@ -11,6 +11,7 @@ using Microsoft.AspNet.Mvc.ApplicationModels; using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Mvc.Description; using Microsoft.AspNet.Mvc.Routing; +using Microsoft.AspNet.Mvc.ModelBinding; namespace Microsoft.AspNet.Mvc { @@ -42,6 +43,12 @@ namespace Microsoft.AspNet.Mvc foreach (var controller in application.Controllers) { + // Only add properties which are explictly marked to bind. + // The attribute check is required for ModelBinder attribute. + var controllerPropertyDescriptors = controller.ControllerProperties + .Where(p => p.BindingInfo != null) + .Select(CreateParameterDescriptor) + .ToList(); foreach (var action in controller.Actions) { // Controllers with multiple [Route] attributes (or user defined implementation of @@ -60,6 +67,7 @@ namespace Microsoft.AspNet.Mvc AddRouteConstraints(removalConstraints, actionDescriptor, controller, action); AddProperties(actionDescriptor, action, controller, application); + actionDescriptor.BoundProperties = controllerPropertyDescriptors; if (IsAttributeRoutedAction(actionDescriptor)) { hasAttributeRoutes = true; @@ -272,13 +280,25 @@ namespace Microsoft.AspNet.Mvc return actionDescriptor; } - private static ParameterDescriptor CreateParameterDescriptor(ParameterModel parameter) + private static ParameterDescriptor CreateParameterDescriptor(ParameterModel parameterModel) { var parameterDescriptor = new ParameterDescriptor() { - Name = parameter.ParameterName, - ParameterType = parameter.ParameterInfo.ParameterType, - BindingInfo = parameter.BindingInfo + Name = parameterModel.ParameterName, + ParameterType = parameterModel.ParameterInfo.ParameterType, + BindingInfo = parameterModel.BindingInfo + }; + + return parameterDescriptor; + } + + private static ParameterDescriptor CreateParameterDescriptor(PropertyModel propertyModel) + { + var parameterDescriptor = new ParameterDescriptor() + { + BindingInfo = propertyModel.BindingInfo, + Name = propertyModel.PropertyName, + ParameterType = propertyModel.PropertyInfo.PropertyType, }; return parameterDescriptor; diff --git a/src/Microsoft.AspNet.Mvc.Core/ControllerActionInvoker.cs b/src/Microsoft.AspNet.Mvc.Core/ControllerActionInvoker.cs index 222f8b8cee..391b8ff3c0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ControllerActionInvoker.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ControllerActionInvoker.cs @@ -80,11 +80,11 @@ namespace Microsoft.AspNet.Mvc.Core return actionResult; } - protected override Task> GetActionArgumentsAsync( + protected override Task> BindActionArgumentsAsync( ActionContext context, ActionBindingContext bindingContext) { - return _argumentBinder.GetActionArgumentsAsync(context, bindingContext); + return _argumentBinder.BindActionArgumentsAsync(context, bindingContext, Instance); } // Marking as internal for Unit Testing purposes. diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs index d9fdabe77a..fe58ab5523 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs @@ -10,6 +10,7 @@ using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.AspNet.Mvc.ModelBinding.Metadata; using Microsoft.AspNet.Mvc.ModelBinding.Validation; +using Microsoft.Framework.Internal; using Microsoft.Framework.OptionsModel; namespace Microsoft.AspNet.Mvc @@ -34,9 +35,10 @@ namespace Microsoft.AspNet.Mvc _validator = validator; } - public async Task> GetActionArgumentsAsync( + public async Task> BindActionArgumentsAsync( ActionContext actionContext, - ActionBindingContext actionBindingContext) + ActionBindingContext actionBindingContext, + object controller) { var actionDescriptor = actionContext.ActionDescriptor as ControllerActionDescriptor; if (actionDescriptor == null) @@ -47,31 +49,48 @@ namespace Microsoft.AspNet.Mvc nameof(actionContext)); } + var operationBindingContext = GetOperationBindingContext(actionContext, actionBindingContext); + var controllerProperties = new Dictionary(StringComparer.Ordinal); + await PopulateArgumentsAsync( + operationBindingContext, + actionContext.ModelState, + controllerProperties, + actionDescriptor.BoundProperties); + var controllerType = actionDescriptor.ControllerTypeInfo.AsType(); + ActivateProperties(controller, controllerType, controllerProperties); + var actionArguments = new Dictionary(StringComparer.Ordinal); await PopulateArgumentsAsync( - actionContext, - actionBindingContext, + operationBindingContext, + actionContext.ModelState, actionArguments, actionDescriptor.Parameters); return actionArguments; } + private void ActivateProperties(object controller, Type containerType, Dictionary properties) + { + var propertyHelpers = PropertyHelper.GetProperties(controller); + foreach (var property in properties) + { + var propertyHelper = propertyHelpers.First(helper => helper.Name == property.Key); + if (propertyHelper.Property == null || !propertyHelper.Property.CanWrite) + { + // nothing to do + return; + } + + var setter = PropertyHelper.MakeFastPropertySetter(propertyHelper.Property); + setter(controller, property.Value); + } + } + private async Task PopulateArgumentsAsync( - ActionContext actionContext, - ActionBindingContext bindingContext, + OperationBindingContext operationContext, + ModelStateDictionary modelState, IDictionary arguments, IEnumerable parameterMetadata) { - var operationBindingContext = new OperationBindingContext - { - ModelBinder = bindingContext.ModelBinder, - ValidatorProvider = bindingContext.ValidatorProvider, - MetadataProvider = _modelMetadataProvider, - HttpContext = actionContext.HttpContext, - ValueProvider = bindingContext.ValueProvider, - }; - - var modelState = actionContext.ModelState; modelState.MaxAllowedErrors = _options.MaxModelValidationErrors; foreach (var parameter in parameterMetadata) { @@ -82,9 +101,9 @@ namespace Microsoft.AspNet.Mvc metadata, parameter.BindingInfo, modelState, - operationBindingContext); + operationContext); - var modelBindingResult = await bindingContext.ModelBinder.BindModelAsync(modelBindingContext); + var modelBindingResult = await operationContext.ModelBinder.BindModelAsync(modelBindingContext); if (modelBindingResult != null && modelBindingResult.IsModelSet) { var modelExplorer = new ModelExplorer( @@ -95,8 +114,9 @@ namespace Microsoft.AspNet.Mvc arguments[parameter.Name] = modelBindingResult.Model; var validationContext = new ModelValidationContext( modelBindingResult.Key, - bindingContext.ValidatorProvider, - actionContext.ModelState, + modelBindingContext.BindingSource, + operationContext.ValidatorProvider, + modelState, modelExplorer); _validator.Validate(validationContext); } @@ -121,5 +141,19 @@ namespace Microsoft.AspNet.Mvc return modelBindingContext; } + + private OperationBindingContext GetOperationBindingContext( + ActionContext actionContext, + ActionBindingContext bindingContext) + { + return new OperationBindingContext + { + ModelBinder = bindingContext.ModelBinder, + ValidatorProvider = bindingContext.ValidatorProvider, + MetadataProvider = _modelMetadataProvider, + HttpContext = actionContext.HttpContext, + ValueProvider = bindingContext.ValueProvider, + }; + } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs index c21a03af7a..4da4083e58 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs @@ -149,13 +149,7 @@ namespace Microsoft.AspNet.Mvc type, typeof(ActivateAttribute), CreateActivateInfo); - var activatorsForFromServiceProperties = PropertyActivator.GetPropertiesToActivate( - type, - typeof(FromServicesAttribute), - CreateFromServicesInfo); - - return Enumerable.Concat(activatorsForActivateProperties, activatorsForFromServiceProperties) - .ToArray(); + return activatorsForActivateProperties; } private PropertyActivator CreateActivateInfo( diff --git a/src/Microsoft.AspNet.Mvc.Core/Description/DefaultApiDescriptionProvider.cs b/src/Microsoft.AspNet.Mvc.Core/Description/DefaultApiDescriptionProvider.cs index 82209af194..31139637fa 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Description/DefaultApiDescriptionProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Description/DefaultApiDescriptionProvider.cs @@ -147,6 +147,24 @@ namespace Microsoft.AspNet.Mvc.Description } } + if (context.ActionDescriptor.BoundProperties != null) + { + foreach (var actionParameter in context.ActionDescriptor.BoundProperties) + { + var visitor = new PseudoModelBindingVisitor(context, actionParameter); + var modelMetadata = context.MetadataProvider.GetMetadataForProperty( + containerType: context.ActionDescriptor.ControllerTypeInfo.AsType(), + propertyName: actionParameter.Name); + + var bindingContext = ApiParameterDescriptionContext.GetContext( + modelMetadata, + actionParameter.BindingInfo, + propertyName: actionParameter.Name); + + visitor.WalkParameter(bindingContext); + } + } + for (var i = context.Results.Count - 1; i >= 0; i--) { // Remove any 'hidden' parameters. These are things that can't come from user input, diff --git a/src/Microsoft.AspNet.Mvc.Core/FilterActionInvoker.cs b/src/Microsoft.AspNet.Mvc.Core/FilterActionInvoker.cs index ac08004838..de38bde056 100644 --- a/src/Microsoft.AspNet.Mvc.Core/FilterActionInvoker.cs +++ b/src/Microsoft.AspNet.Mvc.Core/FilterActionInvoker.cs @@ -88,7 +88,7 @@ namespace Microsoft.AspNet.Mvc.Core protected abstract Task InvokeActionAsync(ActionExecutingContext actionExecutingContext); - protected abstract Task> GetActionArgumentsAsync( + protected abstract Task> BindActionArgumentsAsync( [NotNull] ActionContext context, [NotNull] ActionBindingContext bindingContext); @@ -434,7 +434,7 @@ namespace Microsoft.AspNet.Mvc.Core Instance = CreateInstance(); - var arguments = await GetActionArgumentsAsync(ActionContext, ActionBindingContext); + var arguments = await BindActionArgumentsAsync(ActionContext, ActionBindingContext); _actionExecutingContext = new ActionExecutingContext( ActionContext, diff --git a/src/Microsoft.AspNet.Mvc.Core/IControllerActionArgumentBinder.cs b/src/Microsoft.AspNet.Mvc.Core/IControllerActionArgumentBinder.cs index b11f763932..011568f595 100644 --- a/src/Microsoft.AspNet.Mvc.Core/IControllerActionArgumentBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/IControllerActionArgumentBinder.cs @@ -14,12 +14,15 @@ namespace Microsoft.AspNet.Mvc { /// /// Returns a dictionary of representing the parameter-argument name-value pairs, - /// which can be used to invoke the action. + /// which can be used to invoke the action. Also binds properties explicitly marked properties on the + /// . /// /// The action context assoicated with the current action. /// The . - Task> GetActionArgumentsAsync( + /// The controller object which contains the action. + Task> BindActionArgumentsAsync( [NotNull] ActionContext context, - [NotNull] ActionBindingContext bindingContext); + [NotNull] ActionBindingContext bindingContext, + [NotNull] object controller); } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/ActionDescriptorValues.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/ActionDescriptorValues.cs index 9b5098f482..d02e1d55fc 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/ActionDescriptorValues.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/ActionDescriptorValues.cs @@ -21,6 +21,7 @@ namespace Microsoft.AspNet.Mvc.Logging Name = inner.Name; DisplayName = inner.DisplayName; Parameters = inner.Parameters.Select(p => new ParameterDescriptorValues(p)).ToList(); + BoundProperties = inner.BoundProperties.Select(p => new ParameterDescriptorValues(p)).ToList(); FilterDescriptors = inner.FilterDescriptors.Select(f => new FilterDescriptorValues(f)).ToList(); RouteConstraints = inner.RouteConstraints.Select(r => new RouteDataActionConstraintValues(r)).ToList(); AttributeRouteInfo = new AttributeRouteInfoValues(inner.AttributeRouteInfo); @@ -54,6 +55,12 @@ namespace Microsoft.AspNet.Mvc.Logging /// public IList Parameters { get; } + /// + /// The parameters of the action as . + /// See . + /// + public IList BoundProperties { get; } + /// /// The filters of the action as . /// See . diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/ControllerModelValues.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/ControllerModelValues.cs index 4ecee1c5f0..e71fb9039c 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/ControllerModelValues.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/ControllerModelValues.cs @@ -23,6 +23,7 @@ namespace Microsoft.AspNet.Mvc.Logging ApiExplorer = new ApiExplorerModelValues(inner.ApiExplorer); Actions = inner.Actions.Select(a => new ActionModelValues(a)).ToList(); Attributes = inner.Attributes.Select(a => a.GetType()).ToList(); + ControllerProperties = inner.ControllerProperties.Select(p => new PropertyModelValues(p)).ToList(); Filters = inner.Filters.Select(f => new FilterValues(f)).ToList(); ActionConstraints = inner.ActionConstraints?.Select(a => new ActionConstraintValues(a))?.ToList(); RouteConstraints = inner.RouteConstraints.Select( @@ -43,6 +44,11 @@ namespace Microsoft.AspNet.Mvc.Logging /// public Type ControllerType { get; } + /// + /// The of the controller. See . + /// + public IList ControllerProperties { get; } + /// /// See . /// diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/PropertyModelValues.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/PropertyModelValues.cs new file mode 100644 index 0000000000..fe5d003d3d --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/PropertyModelValues.cs @@ -0,0 +1,39 @@ +// 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 Microsoft.AspNet.Mvc.ApplicationModels; +using Microsoft.Framework.Internal; +using Microsoft.Framework.Logging; + +namespace Microsoft.AspNet.Mvc.Logging +{ + /// + /// Logging representation of a . Logged as a substructure of + /// , this contains the name, type, and + /// binder metadata of the property. + /// + public class PropertyModelValues : ReflectionBasedLogValues + { + public PropertyModelValues([NotNull] PropertyModel inner) + { + PropertyName = inner.PropertyName; + PropertyType = inner.PropertyInfo.PropertyType; + } + + /// + /// The name of the property. See . + /// + public string PropertyName { get; } + + /// + /// The of the property. + /// + public Type PropertyType { get; } + + public override string Format() + { + return LogFormatter.FormatLogValues(this); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/BindingInfo.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/BindingInfo.cs index 833388b186..75040346a8 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/BindingInfo.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/BindingInfo.cs @@ -41,10 +41,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public static BindingInfo GetBindingInfo(IEnumerable attributes) { var bindingInfo = new BindingInfo(); + var isBindingInfoPresent = false; // BinderModelName foreach (var binderModelNameAttribute in attributes.OfType()) { + isBindingInfoPresent = true; if (binderModelNameAttribute?.Name != null) { bindingInfo.BinderModelName = binderModelNameAttribute.Name; @@ -55,6 +57,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // BinderType foreach (var binderTypeAttribute in attributes.OfType()) { + isBindingInfoPresent = true; if (binderTypeAttribute.BinderType != null) { bindingInfo.BinderType = binderTypeAttribute.BinderType; @@ -65,6 +68,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // BindingSource foreach (var bindingSourceAttribute in attributes.OfType()) { + isBindingInfoPresent = true; if (bindingSourceAttribute.BindingSource != null) { bindingInfo.BindingSource = bindingSourceAttribute.BindingSource; @@ -76,11 +80,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var predicateProviders = attributes.OfType().ToArray(); if (predicateProviders.Length > 0) { + isBindingInfoPresent = true; bindingInfo.PropertyBindingPredicateProvider = new CompositePredicateProvider( predicateProviders); } - return bindingInfo; + return isBindingInfoPresent ? bindingInfo : null; } private class CompositePredicateProvider : IPropertyBindingPredicateProvider diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelAttributes.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelAttributes.cs index 25f8a64225..543c8c6dbb 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelAttributes.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelAttributes.cs @@ -15,6 +15,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// public static class ModelAttributes { + /// + /// Gets the attributes for the given . + /// + /// A for which attributes need to be resolved. + /// + /// An containing the attributes on the + /// before the attributes on the type. + public static IEnumerable GetAttributesForParameter(ParameterInfo parameter) + { + // Return the parameter attributes first. + var parameterAttributes = parameter.GetCustomAttributes(); + var typeAttributes = parameter.ParameterType.GetTypeInfo().GetCustomAttributes(); + + return parameterAttributes.Concat(typeAttributes); + } + /// /// Gets the attributes for the given . /// diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs index 42ec3527ea..3392a5795f 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs @@ -64,18 +64,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// A new instance of . public static ModelBindingContext GetModelBindingContext( [NotNull] ModelMetadata metadata, - [NotNull] BindingInfo bindingInfo, + BindingInfo bindingInfo, string modelName) { - var binderModelName = bindingInfo.BinderModelName ?? metadata.BinderModelName; + var binderModelName = bindingInfo?.BinderModelName ?? metadata.BinderModelName; var propertyPredicateProvider = - bindingInfo.PropertyBindingPredicateProvider ?? metadata.PropertyBindingPredicateProvider; + bindingInfo?.PropertyBindingPredicateProvider ?? metadata.PropertyBindingPredicateProvider; return new ModelBindingContext() { ModelMetadata = metadata, - BindingSource = bindingInfo.BindingSource ?? metadata.BindingSource, + BindingSource = bindingInfo?.BindingSource ?? metadata.BindingSource, PropertyFilter = propertyPredicateProvider?.PropertyFilter, - BinderType = bindingInfo.BinderType ?? metadata.BinderType, + BinderType = bindingInfo?.BinderType ?? metadata.BinderType, BinderModelName = binderModelName, ModelName = binderModelName ?? metadata.PropertyName ?? modelName, FallbackToEmptyPrefix = binderModelName == null, diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/DefaultObjectValidator.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/DefaultObjectValidator.cs index 952b596996..8f46da0fe5 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/DefaultObjectValidator.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/DefaultObjectValidator.cs @@ -31,7 +31,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation /// public void Validate([NotNull] ModelValidationContext modelValidationContext) { - var modelExplorer = modelValidationContext.ModelExplorer; var validationContext = new ValidationContext() { ModelValidationContext = modelValidationContext, @@ -40,23 +39,24 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation ValidateNonVisitedNodeAndChildren( modelValidationContext.RootPrefix, - modelExplorer, validationContext, validators: null); } private bool ValidateNonVisitedNodeAndChildren( string modelKey, - ModelExplorer modelExplorer, ValidationContext validationContext, IList validators) { + var modelValidationContext = validationContext.ModelValidationContext; + var modelExplorer = modelValidationContext.ModelExplorer; + // Recursion guard to avoid stack overflows RuntimeHelpers.EnsureSufficientExecutionStack(); - var modelState = validationContext.ModelValidationContext.ModelState; + var modelState = modelValidationContext.ModelState; - var bindingSource = modelExplorer.Metadata.BindingSource; + var bindingSource = modelValidationContext.BindingSource; if (bindingSource != null && !bindingSource.IsFromRequest) { // Short circuit if the metadata represents something that was not bound using request data. @@ -64,7 +64,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var validationState = modelState.GetFieldValidationState(modelKey); if (validationState == ModelValidationState.Unvalidated) { - validationContext.ModelValidationContext.ModelState.MarkFieldSkipped(modelKey); + modelValidationContext.ModelState.MarkFieldSkipped(modelKey); } // For validation purposes this model is valid. @@ -83,7 +83,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation // The validators are not null in the case of validating an array. Since the validators are // the same for all the elements of the array, we do not do GetValidators for each element, // instead we just pass them over. See ValidateElements function. - var validatorProvider = validationContext.ModelValidationContext.ValidatorProvider; + var validatorProvider = modelValidationContext.ValidatorProvider; var validatorProviderContext = new ModelValidatorProviderContext(modelExplorer.Metadata); validatorProvider.GetValidators(validatorProviderContext); @@ -160,7 +160,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation } } - private bool ValidateProperties(string currentModelKey, ModelExplorer modelExplorer, ValidationContext validationContext) + private bool ValidateProperties( + string currentModelKey, + ModelExplorer modelExplorer, + ValidationContext validationContext) { var isValid = true; @@ -168,13 +171,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation { var propertyExplorer = modelExplorer.GetExplorerForProperty(property.PropertyName); var propertyMetadata = propertyExplorer.Metadata; + var propertyValidationContext = new ValidationContext() + { + ModelValidationContext = ModelValidationContext.GetChildValidationContext( + validationContext.ModelValidationContext, + propertyExplorer), + Visited = validationContext.Visited + }; var propertyBindingName = propertyMetadata.BinderModelName ?? propertyMetadata.PropertyName; var childKey = ModelBindingHelper.CreatePropertyModelName(currentModelKey, propertyBindingName); if (!ValidateNonVisitedNodeAndChildren( childKey, - propertyExplorer, - validationContext, + propertyValidationContext, validators: null)) { isValid = false; @@ -209,7 +218,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation { var elementExplorer = new ModelExplorer(_modelMetadataProvider, elementMetadata, element); var elementKey = ModelBindingHelper.CreateIndexModelName(currentKey, index); - if (!ValidateNonVisitedNodeAndChildren(elementKey, elementExplorer, validationContext, validators)) + var elementValidationContext = new ValidationContext() + { + ModelValidationContext = ModelValidationContext.GetChildValidationContext( + validationContext.ModelValidationContext, + elementExplorer), + Visited = validationContext.Visited + }; + + if (!ValidateNonVisitedNodeAndChildren(elementKey, elementValidationContext, validators)) { isValid = false; } @@ -245,9 +262,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation // In a large array (tens of thousands or more) scenario it's very significant. if (validators == null || validators.Count > 0) { - var modelValidationContext = - new ModelValidationContext(validationContext.ModelValidationContext, modelExplorer); - + var modelValidationContext = ModelValidationContext.GetChildValidationContext( + validationContext.ModelValidationContext, + modelExplorer); + var modelValidationState = modelState.GetValidationState(modelKey); // If either the model or its properties are unvalidated, validate them now. diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationContext.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationContext.cs index b93cea0ffd..bc1726cec9 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationContext.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationContext.cs @@ -11,6 +11,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation [NotNull] ModelBindingContext bindingContext, [NotNull] ModelExplorer modelExplorer) : this(bindingContext.ModelName, + bindingContext.BindingSource, bindingContext.OperationBindingContext.ValidatorProvider, bindingContext.ModelState, modelExplorer) @@ -19,6 +20,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation public ModelValidationContext( string rootPrefix, + BindingSource bindingSource, [NotNull] IModelValidatorProvider validatorProvider, [NotNull] ModelStateDictionary modelState, [NotNull] ModelExplorer modelExplorer) @@ -27,25 +29,37 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation RootPrefix = rootPrefix; ValidatorProvider = validatorProvider; ModelExplorer = modelExplorer; + BindingSource = bindingSource; } - public ModelValidationContext( + /// + /// Constructs a new instance of the class using the + /// and . + /// + /// Existing . + /// associated with the new + /// . + /// + public static ModelValidationContext GetChildValidationContext( [NotNull] ModelValidationContext parentContext, [NotNull] ModelExplorer modelExplorer) { - ModelExplorer = modelExplorer; - ModelState = parentContext.ModelState; - RootPrefix = parentContext.RootPrefix; - ValidatorProvider = parentContext.ValidatorProvider; + return new ModelValidationContext( + parentContext.RootPrefix, + modelExplorer.Metadata.BindingSource, + parentContext.ValidatorProvider, + parentContext.ModelState, + modelExplorer); } - public ModelExplorer ModelExplorer { get; } public ModelStateDictionary ModelState { get; } public string RootPrefix { get; set; } + public BindingSource BindingSource { get; set; } + public IModelValidatorProvider ValidatorProvider { get; } } } diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs index b80688de9a..ebeb47c5bb 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs @@ -424,9 +424,10 @@ namespace System.Web.Http var modelValidationContext = new ModelValidationContext( keyPrefix, - BindingContext.ValidatorProvider, - ModelState, - modelExplorer); + bindingSource: null, + validatorProvider: BindingContext.ValidatorProvider, + modelState: ModelState, + modelExplorer: modelExplorer); ObjectValidator.Validate(modelValidationContext); } diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/Conventions/WebApiParameterConventionsApplicationModelConvention.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/Conventions/WebApiParameterConventionsApplicationModelConvention.cs index 9a1ccd8939..9b18c4c2e1 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/Conventions/WebApiParameterConventionsApplicationModelConvention.cs +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/Conventions/WebApiParameterConventionsApplicationModelConvention.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNet.Mvc.WebApiCompatShim { // Some IBindingSourceMetadata attributes like ModelBinder attribute return null // as their binding source. Special case to ensure we do not ignore them. - if (parameter.BindingInfo.BindingSource != null || + if (parameter.BindingInfo?.BindingSource != null || parameter.Attributes.OfType().Any()) { // This has a binding behavior configured, just leave it alone. @@ -30,12 +30,13 @@ namespace Microsoft.AspNet.Mvc.WebApiCompatShim else if (ValueProviderResult.CanConvertFromString(parameter.ParameterInfo.ParameterType)) { // Simple types are by-default from the URI. - + parameter.BindingInfo = parameter.BindingInfo ?? new BindingInfo(); parameter.BindingInfo.BindingSource = uriBindingSource; } else { // Complex types are by-default from the body. + parameter.BindingInfo = parameter.BindingInfo ?? new BindingInfo(); parameter.BindingInfo.BindingSource = BindingSource.Body; } diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/OverloadActionConstraint.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/OverloadActionConstraint.cs index 792f90220e..f79774ff08 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/OverloadActionConstraint.cs +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/OverloadActionConstraint.cs @@ -93,7 +93,7 @@ namespace Microsoft.AspNet.Mvc.WebApiCompatShim foreach (var parameter in candidate.Action.Parameters) { // We only consider parameters that are marked as bound from the URL. - var source = parameter.BindingInfo.BindingSource; + var source = parameter.BindingInfo?.BindingSource; if (source == null) { continue; @@ -115,7 +115,7 @@ namespace Microsoft.AspNet.Mvc.WebApiCompatShim } } - var prefix = parameter.BindingInfo.BinderModelName ?? parameter.Name; + var prefix = parameter.BindingInfo?.BinderModelName ?? parameter.Name; parameters.Add(new OverloadedParameter() { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs index 5d935ab792..ce54f3c2ba 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs @@ -58,6 +58,8 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels controller.Filters.Add(new AuthorizeFilter(new AuthorizationPolicyBuilder().Build())); controller.RouteConstraints.Add(new AreaAttribute("Admin")); controller.Properties.Add(new KeyValuePair("test key", "test value")); + controller.ControllerProperties.Add( + new PropertyModel(typeof(TestController).GetProperty("TestProperty"), new List())); // Act var controller2 = new ControllerModel(controller); @@ -67,7 +69,8 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels { if (property.Name.Equals("Actions") || property.Name.Equals("AttributeRoutes") || - property.Name.Equals("ApiExplorer")) + property.Name.Equals("ApiExplorer") || + property.Name.Equals("ControllerProperties")) { // This test excludes other ApplicationModel objects on purpose because we deep copy them. continue; @@ -110,6 +113,8 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels private class TestController { + public string TestProperty { get; set; } + public void Edit() { } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs index b853dfc628..a852f9fca9 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using Microsoft.AspNet.Authorization; using Microsoft.AspNet.Cors.Core; using Microsoft.AspNet.Mvc.Filters; +using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.Framework.Internal; using Microsoft.Framework.OptionsModel; using Moq; @@ -71,6 +72,27 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels Assert.Single(model.Filters, f => f is CorsAuthorizationFilterFactory); } + [Fact] + public void BuildControllerModel_AddsControllerProperties() + { + // Arrange + var builder = new DefaultControllerModelBuilder(new DefaultActionModelBuilder(null), + NullLoggerFactory.Instance, + null); + var typeInfo = typeof(ModelBinderController).GetTypeInfo(); + + // Act + var model = builder.BuildControllerModel(typeInfo); + + // Assert + Assert.Equal(2, model.ControllerProperties.Count); + Assert.Equal("Bound", model.ControllerProperties[0].PropertyName); + Assert.Equal(BindingSource.Query, model.ControllerProperties[0].BindingInfo.BindingSource); + Assert.NotNull(model.ControllerProperties[0].Controller); + var attribute = Assert.Single(model.ControllerProperties[0].Attributes); + Assert.IsType(attribute); + } + [Fact] public void BuildControllerModel_DisableCorsAttributeAddsDisableCorsAuthorizationFilter() { @@ -168,6 +190,14 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels { } + public class ModelBinderController + { + [FromQuery] + public string Bound { get; set; } + + public string Unbound { get; set; } + } + public class SomeFiltersController : IAsyncActionFilter, IResultFilter { public Task OnActionExecutionAsync( diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/PropertyModelTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/PropertyModelTest.cs new file mode 100644 index 0000000000..2282414c11 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/PropertyModelTest.cs @@ -0,0 +1,64 @@ +// 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.Reflection; +using Microsoft.AspNet.Mvc.ModelBinding; +using Xunit; + +namespace Microsoft.AspNet.Mvc.ApplicationModels +{ + public class PropertyModelTest + { + [Fact] + public void CopyConstructor_CopiesAllProperties() + { + // Arrange + var propertyModel = new PropertyModel(typeof(TestController).GetProperty("Property"), + new List() { new FromBodyAttribute() }); + + propertyModel.Controller = new ControllerModel(typeof(TestController).GetTypeInfo(), new List()); + propertyModel.BindingInfo = BindingInfo.GetBindingInfo(propertyModel.Attributes); + propertyModel.PropertyName = "Property"; + + // Act + var propertyModel2 = new PropertyModel(propertyModel); + + // Assert + foreach (var property in typeof(PropertyModel).GetProperties()) + { + var value1 = property.GetValue(propertyModel); + var value2 = property.GetValue(propertyModel2); + + if (typeof(IEnumerable).IsAssignableFrom(property.PropertyType)) + { + Assert.Equal((IEnumerable)value1, (IEnumerable)value2); + + // Ensure non-default value + Assert.NotEmpty((IEnumerable)value1); + } + else if (property.PropertyType.IsValueType || + Nullable.GetUnderlyingType(property.PropertyType) != null) + { + Assert.Equal(value1, value2); + + // Ensure non-default value + Assert.NotEqual(value1, Activator.CreateInstance(property.PropertyType)); + } + else + { + Assert.Same(value1, value2); + + // Ensure non-default value + Assert.NotNull(value1); + } + } + } + + private class TestController + { + public string Property { get; set; } + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorBuilderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorBuilderTest.cs index 04809fe3ea..1399b62526 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorBuilderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorBuilderTest.cs @@ -5,12 +5,50 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; using Microsoft.AspNet.Mvc.ApplicationModels; +using Microsoft.AspNet.Mvc.ModelBinding; using Xunit; namespace Microsoft.AspNet.Mvc { public class ControllerActionDescriptorBuilderTest { + [Fact] + public void Build_WithControllerPropertiesSet_AddsPropertiesWithBinderMetadataSet() + { + // Arrange + var applicationModel = new ApplicationModel(); + var controller = new ControllerModel(typeof(TestController).GetTypeInfo(), + new List() { }); + controller.ControllerProperties.Add( + new PropertyModel( + controller.ControllerType.GetProperty("BoundProperty"), + new List() { }) + { + BindingInfo = BindingInfo.GetBindingInfo(new object[] { new FromQueryAttribute() }), + PropertyName = "BoundProperty" + }); + + controller.ControllerProperties.Add( + new PropertyModel(controller.ControllerType.GetProperty("UnboundProperty"), new List() { })); + + controller.Application = applicationModel; + applicationModel.Controllers.Add(controller); + + var methodInfo = typeof(TestController).GetMethod("SomeAction"); + var actionModel = new ActionModel(methodInfo, new List() { }); + actionModel.Controller = controller; + controller.Actions.Add(actionModel); + + // Act + var descriptors = ControllerActionDescriptorBuilder.Build(applicationModel); + + // Assert + var property = Assert.Single(descriptors.Single().BoundProperties); + Assert.Equal("BoundProperty", property.Name); + Assert.Equal(typeof(string), property.ParameterType); + Assert.Equal(BindingSource.Query, property.BindingInfo.BindingSource); + } + [Fact] public void Build_WithPropertiesSet_FromApplicationModel() { @@ -88,6 +126,11 @@ namespace Microsoft.AspNet.Mvc private class TestController { + [FromQuery] + public string BoundProperty { get; set; } + + public string UnboundProperty { get; set; } + public void SomeAction() { } } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorProviderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorProviderTests.cs index 573215e5f2..51b0d3a2ae 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorProviderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorProviderTests.cs @@ -111,7 +111,7 @@ namespace Microsoft.AspNet.Mvc.Test var id = Assert.Single(main.Parameters); Assert.Equal("id", id.Name); - Assert.Null(id.BindingInfo.BindingSource); + Assert.Null(id.BindingInfo?.BindingSource); Assert.Equal(typeof(int), id.ParameterType); } @@ -130,7 +130,7 @@ namespace Microsoft.AspNet.Mvc.Test var id = Assert.Single(main.Parameters, p => p.Name == "id"); Assert.Equal("id", id.Name); - Assert.Null(id.BindingInfo.BindingSource); + Assert.Null(id.BindingInfo?.BindingSource); Assert.Equal(typeof(int), id.ParameterType); var entity = Assert.Single(main.Parameters, p => p.Name == "entity"); @@ -155,19 +155,19 @@ namespace Microsoft.AspNet.Mvc.Test var id = Assert.Single(main.Parameters, p => p.Name == "id"); Assert.Equal("id", id.Name); - Assert.Null(id.BindingInfo.BindingSource); + Assert.Null(id.BindingInfo?.BindingSource); Assert.Equal(typeof(int), id.ParameterType); var upperCaseId = Assert.Single(main.Parameters, p => p.Name == "ID"); Assert.Equal("ID", upperCaseId.Name); - Assert.Null(upperCaseId.BindingInfo.BindingSource); + Assert.Null(upperCaseId.BindingInfo?.BindingSource); Assert.Equal(typeof(int), upperCaseId.ParameterType); var pascalCaseId = Assert.Single(main.Parameters, p => p.Name == "Id"); Assert.Equal("Id", pascalCaseId.Name); - Assert.Null(id.BindingInfo.BindingSource); + Assert.Null(id.BindingInfo?.BindingSource); Assert.Equal(typeof(int), pascalCaseId.ParameterType); } @@ -209,7 +209,7 @@ namespace Microsoft.AspNet.Mvc.Test var entity = Assert.Single(notFromBody.Parameters); Assert.Equal("entity", entity.Name); - Assert.Null(entity.BindingInfo.BindingSource); + Assert.Null(entity.BindingInfo?.BindingSource); Assert.Equal(typeof(TestActionParameter), entity.ParameterType); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionInvokerTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionInvokerTest.cs index cca14ad33a..9728174cdd 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionInvokerTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionInvokerTest.cs @@ -2063,6 +2063,8 @@ namespace Microsoft.AspNet.Mvc // Arrange var actionDescriptor = new ControllerActionDescriptor { + ControllerTypeInfo = typeof(TestController).GetTypeInfo(), + BoundProperties = new List(), MethodInfo = typeof(TestController).GetTypeInfo() .DeclaredMethods .First(m => m.Name.Equals("ActionMethodWithDefaultValues", StringComparison.Ordinal)), diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerFactoryTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerFactoryTest.cs index 50b458c603..76c64c29b7 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerFactoryTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerFactoryTest.cs @@ -143,58 +143,6 @@ namespace Microsoft.AspNet.Mvc.Core Assert.Same(bindingContext, controller.BindingContext); } - [Fact] - public void CreateController_PopulatesServicesFromServiceContainer() - { - // Arrange - var actionDescriptor = new ControllerActionDescriptor - { - ControllerTypeInfo = typeof(ControllerWithActivateAndFromServices).GetTypeInfo() - }; - var services = GetServices(); - var urlHelper = services.GetRequiredService(); - - var httpContext = new DefaultHttpContext - { - RequestServices = services - }; - var context = new ActionContext(httpContext, new RouteData(), actionDescriptor); - var factory = new DefaultControllerFactory(new DefaultControllerActivator(new DefaultTypeActivatorCache())); - - // Act - var result = factory.CreateController(context); - - // Assert - var controller = Assert.IsType(result); - Assert.Same(urlHelper, controller.Helper); - } - - [Fact] - public void CreateController_PopulatesUserServicesFromServiceContainer() - { - // Arrange - var actionDescriptor = new ControllerActionDescriptor - { - ControllerTypeInfo = typeof(ControllerWithActivateAndFromServices).GetTypeInfo() - }; - var services = GetServices(); - var testService = services.GetService(); - - var httpContext = new DefaultHttpContext - { - RequestServices = services - }; - var context = new ActionContext(httpContext, new RouteData(), actionDescriptor); - var factory = new DefaultControllerFactory(new DefaultControllerActivator(new DefaultTypeActivatorCache())); - - // Act - var result = factory.CreateController(context); - - // Assert - var controller = Assert.IsType(result); - Assert.Same(testService, controller.TestService); - } - [Fact] public void CreateController_IgnoresPropertiesThatAreNotDecoratedWithActivateAttribute() { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Description/DefaultApiDescriptionProviderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Description/DefaultApiDescriptionProviderTest.cs index 0c1469438f..207b71bb17 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Description/DefaultApiDescriptionProviderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Description/DefaultApiDescriptionProviderTest.cs @@ -901,6 +901,41 @@ namespace Microsoft.AspNet.Mvc.Description Assert.Equal(typeof(int), id.Type); } + [Fact] + public void GetApiDescription_WithControllerProperties_Merges_ParameterDescription() + { + // Arrange + var action = CreateActionDescriptor("FromQueryName", typeof(TestController)); + var parameterDescriptor = action.Parameters.Single(); + + // Act + var descriptions = GetApiDescriptions(action); + + // Assert + var description = Assert.Single(descriptions); + Assert.Equal(5, description.ParameterDescriptions.Count); + + var name = Assert.Single(description.ParameterDescriptions, p => p.Name == "name"); + Assert.Same(BindingSource.Query, name.Source); + Assert.Equal(typeof(string), name.Type); + + var id = Assert.Single(description.ParameterDescriptions, p => p.Name == "Id"); + Assert.Same(BindingSource.Path, id.Source); + Assert.Equal(typeof(int), id.Type); + + var product = Assert.Single(description.ParameterDescriptions, p => p.Name == "Product"); + Assert.Same(BindingSource.Body, product.Source); + Assert.Equal(typeof(Product), product.Type); + + var userId = Assert.Single(description.ParameterDescriptions, p => p.Name == "UserId"); + Assert.Same(BindingSource.Header, userId.Source); + Assert.Equal(typeof(string), userId.Type); + + var comments = Assert.Single(description.ParameterDescriptions, p => p.Name == "Comments"); + Assert.Same(BindingSource.ModelBinding, comments.Source); + Assert.Equal(typeof(string), comments.Type); + } + private IReadOnlyList GetApiDescriptions(ActionDescriptor action) { return GetApiDescriptions(action, CreateFormatters()); @@ -950,17 +985,42 @@ namespace Microsoft.AspNet.Mvc.Description return formatters; } - private ControllerActionDescriptor CreateActionDescriptor(string methodName = null) + private ControllerActionDescriptor CreateActionDescriptor(string methodName = null, Type controllerType = null) { var action = new ControllerActionDescriptor(); action.SetProperty(new ApiDescriptionActionData()); - action.MethodInfo = GetType().GetMethod( - methodName ?? "ReturnsObject", - BindingFlags.Instance | BindingFlags.NonPublic); + if (controllerType != null) + { + action.MethodInfo = controllerType.GetMethod( + methodName ?? "ReturnsObject", + BindingFlags.Instance | BindingFlags.Public); + + action.ControllerTypeInfo = controllerType.GetTypeInfo(); + action.BoundProperties = new List(); + + foreach (var property in action.ControllerTypeInfo.GetProperties()) + { + var bindingInfo = BindingInfo.GetBindingInfo(property.GetCustomAttributes().OfType()); + if (bindingInfo != null) + { + action.BoundProperties.Add(new ParameterDescriptor() + { + BindingInfo = bindingInfo, + Name = property.Name, + ParameterType = property.PropertyType, + }); + } + } + } + else + { + action.MethodInfo = GetType().GetMethod( + methodName ?? "ReturnsObject", + BindingFlags.Instance | BindingFlags.NonPublic); + } action.Parameters = new List(); - foreach (var parameter in action.MethodInfo.GetParameters()) { action.Parameters.Add(new ParameterDescriptor() @@ -1118,6 +1178,27 @@ namespace Microsoft.AspNet.Mvc.Description { } + private class TestController + { + [FromRoute] + public int Id { get; set; } + + [FromBody] + public Product Product { get; set; } + + [FromHeader] + public string UserId { get; set; } + + [ModelBinder] + public string Comments { get; set; } + + public string NotBound { get; set; } + + public void FromQueryName([FromQuery] string name) + { + } + } + private class Product { public int ProductId { get; set; } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs index 114b58e656..439e5b21f5 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Reflection; using System.Threading.Tasks; using Microsoft.AspNet.Http.Core; using Microsoft.AspNet.Mvc.ModelBinding; @@ -54,48 +55,33 @@ namespace Microsoft.AspNet.Mvc.Core.Test public async Task GetActionArgumentsAsync_DoesNotAddActionArguments_IfBinderReturnsFalse() { // Arrange - Func method = foo => 1; - var actionDescriptor = new ControllerActionDescriptor - { - MethodInfo = method.Method, - Parameters = new List + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.Parameters.Add( + new ParameterDescriptor { - new ParameterDescriptor - { - Name = "foo", - ParameterType = typeof(object), + Name = "foo", + ParameterType = typeof(object), BindingInfo = new BindingInfo(), - } - } - }; + }); + + var actionContext = GetActionContext(actionDescriptor); var binder = new Mock(); binder .Setup(b => b.BindModelAsync(It.IsAny())) .Returns(Task.FromResult(result: null)); - - var actionContext = new ActionContext( - new DefaultHttpContext(), - new RouteData(), - actionDescriptor); - var actionBindingContext = new ActionBindingContext() { ModelBinder = binder.Object, }; var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var inputFormattersProvider = new Mock(); - inputFormattersProvider - .SetupGet(o => o.InputFormatters) - .Returns(new List()); - var invoker = new DefaultControllerActionArgumentBinder( - modelMetadataProvider, - new DefaultObjectValidator(Mock.Of(), modelMetadataProvider), - new MockMvcOptionsAccessor()); + + var argumentBinder = GetArgumentBinder(); // Act - var result = await invoker.GetActionArgumentsAsync(actionContext, actionBindingContext); + var result = await argumentBinder + .BindActionArgumentsAsync(actionContext, actionBindingContext, new TestController()); // Assert Assert.Empty(result); @@ -105,20 +91,14 @@ namespace Microsoft.AspNet.Mvc.Core.Test public async Task GetActionArgumentsAsync_DoesNotAddActionArguments_IfBinderDoesNotSetModel() { // Arrange - Func method = foo => 1; - var actionDescriptor = new ControllerActionDescriptor - { - MethodInfo = method.Method, - Parameters = new List + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.Parameters.Add( + new ParameterDescriptor { - new ParameterDescriptor - { - Name = "foo", - ParameterType = typeof(object), + Name = "foo", + ParameterType = typeof(object), BindingInfo = new BindingInfo(), - } - } - }; + }); var binder = new Mock(); binder @@ -135,19 +115,12 @@ namespace Microsoft.AspNet.Mvc.Core.Test ModelBinder = binder.Object, }; - var inputFormattersProvider = new Mock(); - inputFormattersProvider - .SetupGet(o => o.InputFormatters) - .Returns(new List()); - + var argumentBinder = GetArgumentBinder(); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var invoker = new DefaultControllerActionArgumentBinder( - modelMetadataProvider, - new DefaultObjectValidator(Mock.Of(), modelMetadataProvider), - new MockMvcOptionsAccessor()); // Act - var result = await invoker.GetActionArgumentsAsync(actionContext, actionBindingContext); + var result = await argumentBinder + .BindActionArgumentsAsync(actionContext, actionBindingContext, new TestController()); // Assert Assert.Empty(result); @@ -158,19 +131,14 @@ namespace Microsoft.AspNet.Mvc.Core.Test { // Arrange Func method = foo => 1; - var actionDescriptor = new ControllerActionDescriptor - { - MethodInfo = method.Method, - Parameters = new List + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.Parameters.Add( + new ParameterDescriptor { - new ParameterDescriptor - { - Name = "foo", - ParameterType = typeof(string), + Name = "foo", + ParameterType = typeof(string), BindingInfo = new BindingInfo(), - } - }, - }; + }); var value = "Hello world"; var metadataProvider = new EmptyModelMetadataProvider(); @@ -194,16 +162,11 @@ namespace Microsoft.AspNet.Mvc.Core.Test ModelBinder = binder.Object, }; - var mockValidatorProvider = new Mock(MockBehavior.Strict); - mockValidatorProvider.Setup(o => o.Validate(It.IsAny())); - - var invoker = new DefaultControllerActionArgumentBinder( - metadataProvider, - mockValidatorProvider.Object, - new MockMvcOptionsAccessor()); + var argumentBinder = GetArgumentBinder(); // Act - var result = await invoker.GetActionArgumentsAsync(actionContext, actionBindingContext); + var result = await argumentBinder + .BindActionArgumentsAsync(actionContext, actionBindingContext, new TestController()); // Assert Assert.Equal(1, result.Count); @@ -214,50 +177,26 @@ namespace Microsoft.AspNet.Mvc.Core.Test public async Task GetActionArgumentsAsync_CallsValidator_IfModelBinderSucceeds() { // Arrange - Func method = foo => 1; - var actionDescriptor = new ControllerActionDescriptor - { - MethodInfo = method.Method, - Parameters = new List + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.Parameters.Add( + new ParameterDescriptor { - new ParameterDescriptor - { - Name = "foo", - ParameterType = typeof(object), - BindingInfo = new BindingInfo(), - } - } - }; + Name = "foo", + ParameterType = typeof(object), + }); - var actionContext = new ActionContext( - new DefaultHttpContext(), - new RouteData(), - actionDescriptor); - - var binder = new Mock(); - binder - .Setup(b => b.BindModelAsync(It.IsAny())) - .Returns(Task.FromResult(result: new ModelBindingResult( - model: null, - key: string.Empty, - isModelSet: true))); - - var actionBindingContext = new ActionBindingContext() - { - ModelBinder = binder.Object, - }; + var actionContext = GetActionContext(actionDescriptor); + var actionBindingContext = GetActionBindingContext(); var mockValidatorProvider = new Mock(MockBehavior.Strict); mockValidatorProvider .Setup(o => o.Validate(It.IsAny())) .Verifiable(); - var invoker = new DefaultControllerActionArgumentBinder( - TestModelMetadataProvider.CreateDefaultProvider(), - mockValidatorProvider.Object, - new MockMvcOptionsAccessor()); + var argumentBinder = GetArgumentBinder(mockValidatorProvider.Object); // Act - var result = await invoker.GetActionArgumentsAsync(actionContext, actionBindingContext); + var result = await argumentBinder + .BindActionArgumentsAsync(actionContext, actionBindingContext, new TestController()); // Assert mockValidatorProvider.Verify( @@ -269,19 +208,14 @@ namespace Microsoft.AspNet.Mvc.Core.Test { // Arrange Func method = foo => 1; - var actionDescriptor = new ControllerActionDescriptor - { - MethodInfo = method.Method, - Parameters = new List + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.Parameters.Add( + new ParameterDescriptor { - new ParameterDescriptor - { - Name = "foo", - ParameterType = typeof(object), + Name = "foo", + ParameterType = typeof(object), BindingInfo = new BindingInfo(), - } - } - }; + }); var actionContext = new ActionContext( new DefaultHttpContext(), @@ -301,13 +235,11 @@ namespace Microsoft.AspNet.Mvc.Core.Test var mockValidatorProvider = new Mock(MockBehavior.Strict); mockValidatorProvider.Setup(o => o.Validate(It.IsAny())) .Verifiable(); - var invoker = new DefaultControllerActionArgumentBinder( - TestModelMetadataProvider.CreateDefaultProvider(), - mockValidatorProvider.Object, - new MockMvcOptionsAccessor()); + var argumentBinder = GetArgumentBinder(mockValidatorProvider.Object); // Act - var result = await invoker.GetActionArgumentsAsync(actionContext, actionBindingContext); + var result = await argumentBinder + .BindActionArgumentsAsync(actionContext, actionBindingContext, new TestController()); // Assert mockValidatorProvider.Verify(o => o.Validate(It.IsAny()), Times.Never()); @@ -318,55 +250,179 @@ namespace Microsoft.AspNet.Mvc.Core.Test { // Arrange Func method = foo => 1; - var actionDescriptor = new ControllerActionDescriptor - { - MethodInfo = method.Method, - Parameters = new List + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.Parameters.Add( + new ParameterDescriptor { - new ParameterDescriptor - { - Name = "foo", - ParameterType = typeof(object), + Name = "foo", + ParameterType = typeof(object), BindingInfo = new BindingInfo(), - } - } - }; - - var binder = new Mock(); - binder - .Setup(b => b.BindModelAsync(It.IsAny())) - .Returns(Task.FromResult( - result: new ModelBindingResult(model: "Hello", key: string.Empty, isModelSet: true))); + }); var actionContext = new ActionContext( new DefaultHttpContext(), new RouteData(), actionDescriptor); + var actionBindingContext = GetActionBindingContext(); + + var argumentBinder = GetArgumentBinder(); + + // Act + var result = await argumentBinder + .BindActionArgumentsAsync(actionContext, actionBindingContext, new TestController()); + + // Assert + Assert.Equal(5, actionContext.ModelState.MaxAllowedErrors); + } + + [Fact] + public async Task GetActionArgumentsAsync_CallsValidator_ForControllerProperties_IfModelBinderSucceeds() + { + // Arrange + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.BoundProperties.Add( + new ParameterDescriptor + { + Name = "ValueBinderMarkedProperty", + ParameterType = typeof(string), + }); + + var actionContext = GetActionContext(actionDescriptor); + var actionBindingContext = GetActionBindingContext(); + + var mockValidatorProvider = new Mock(MockBehavior.Strict); + mockValidatorProvider + .Setup(o => o.Validate(It.IsAny())) + .Verifiable(); + var argumentBinder = GetArgumentBinder(mockValidatorProvider.Object); + + // Act + var result = await argumentBinder + .BindActionArgumentsAsync(actionContext, actionBindingContext, new TestController()); + + // Assert + mockValidatorProvider.Verify( + o => o.Validate(It.IsAny()), Times.Once()); + } + + [Fact] + public async Task GetActionArgumentsAsync_DoesNotCallValidator_ForControllerProperties_IfModelBinderFails() + { + // Arrange + Func method = foo => 1; + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.BoundProperties.Add( + new ParameterDescriptor + { + Name = "ValueBinderMarkedProperty", + ParameterType = typeof(string), + }); + + var actionContext = new ActionContext( + new DefaultHttpContext(), + new RouteData(), + actionDescriptor); + + var binder = new Mock(); + binder + .Setup(b => b.BindModelAsync(It.IsAny())) + .Returns(Task.FromResult(null)); + var actionBindingContext = new ActionBindingContext() { ModelBinder = binder.Object, }; - var inputFormattersProvider = new Mock(); - inputFormattersProvider - .SetupGet(o => o.InputFormatters) - .Returns(new List()); - - var options = new MockMvcOptionsAccessor(); - options.Options.MaxModelValidationErrors = 5; var mockValidatorProvider = new Mock(MockBehavior.Strict); - mockValidatorProvider.Setup(o => o.Validate(It.IsAny())); - var invoker = new DefaultControllerActionArgumentBinder( - TestModelMetadataProvider.CreateDefaultProvider(), - mockValidatorProvider.Object, - options); + mockValidatorProvider.Setup(o => o.Validate(It.IsAny())) + .Verifiable(); + var argumentBinder = GetArgumentBinder(mockValidatorProvider.Object); // Act - var result = await invoker.GetActionArgumentsAsync(actionContext, actionBindingContext); + var result = await argumentBinder + .BindActionArgumentsAsync(actionContext, actionBindingContext, new TestController()); // Assert - Assert.Equal(5, actionContext.ModelState.MaxAllowedErrors); + mockValidatorProvider.Verify(o => o.Validate(It.IsAny()), Times.Never()); + } + + + [Fact] + public async Task GetActionArgumentsAsync_SetsControllerProperties() + { + // Arrange + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.BoundProperties.Add( + new ParameterDescriptor + { + Name = "ValueBinderMarkedProperty", + BindingInfo = new BindingInfo(), + ParameterType = typeof(string) + }); + + var actionContext = GetActionContext(actionDescriptor); + var actionBindingContext = GetActionBindingContext(); + var argumentBinder = GetArgumentBinder(); + var controller = new TestController(); + + // Act + var result = await argumentBinder.BindActionArgumentsAsync(actionContext, actionBindingContext, controller); + + // Assert + Assert.Equal("Hello", controller.ValueBinderMarkedProperty); + Assert.Null(controller.UnmarkedProperty); + } + + private static ActionContext GetActionContext(ActionDescriptor descriptor = null) + { + return new ActionContext( + new DefaultHttpContext(), + new RouteData(), + descriptor ?? GetActionDescriptor()); + } + + private static ActionDescriptor GetActionDescriptor() + { + Func method = foo => 1; + return new ControllerActionDescriptor + { + MethodInfo = method.Method, + ControllerTypeInfo = typeof(TestController).GetTypeInfo(), + BoundProperties = new List(), + Parameters = new List() + }; + } + + private static ActionBindingContext GetActionBindingContext() + { + var binder = new Mock(); + binder + .Setup(b => b.BindModelAsync(It.IsAny())) + .Returns(Task.FromResult( + result: new ModelBindingResult(model: "Hello", key: string.Empty, isModelSet: true))); + return new ActionBindingContext() + { + ModelBinder = binder.Object, + }; + } + + private static DefaultControllerActionArgumentBinder GetArgumentBinder(IObjectModelValidator validator = null) + { + var options = new MockMvcOptionsAccessor(); + options.Options.MaxModelValidationErrors = 5; + + if (validator == null) + { + var mockValidator = new Mock(MockBehavior.Strict); + mockValidator.Setup(o => o.Validate(It.IsAny())); + validator = mockValidator.Object; + } + + return new DefaultControllerActionArgumentBinder( + TestModelMetadataProvider.CreateDefaultProvider(), + validator, + options); } private class TestController diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs index be33f0526a..850640d046 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs @@ -25,6 +25,22 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests private readonly Action _app = new ModelBindingWebSite.Startup().Configure; private readonly Action _configureServices = new ModelBindingWebSite.Startup().ConfigureServices; + [Fact] + public async Task DoNotValidate_ParametersOrControllerProperties_IfSourceNotFromRequest() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync("http://localhost/Validation/DoNotValidateParameter"); + + // Assert + var stringValue = await response.Content.ReadAsStringAsync(); + var isModelStateValid = JsonConvert.DeserializeObject(stringValue); + Assert.True(isModelStateValid); + } + [Fact] public async Task TypeBasedExclusion_ForBodyAndNonBodyBoundModels() { @@ -181,6 +197,74 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests exception.ExceptionMessage); } + [Fact] + public async Task ControllerPropertyAndAnActionWithoutFromBody_InvokesWithoutErrors() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync("http://localhost/FromBodyControllerProperty/GetSiteUser"); + + // Assert + Assert.Equal(HttpStatusCode.NoContent, response.StatusCode); + } + + [Fact] + public async Task ControllerPropertyAndAnActionParameterWithFromBody_Throws() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync("http://localhost/FromBodyControllerProperty/AddUser"); + + // Assert + var exception = response.GetServerException(); + Assert.Equal(typeof(InvalidOperationException).FullName, exception.ExceptionType); + Assert.Equal( + "More than one parameter and/or property is bound to the HTTP request's content.", + exception.ExceptionMessage); + } + + [Fact] + public async Task ControllerPropertyAndAModelPropertyWithFromBody_Throws() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync("http://localhost/FromBodyControllerProperty/AddUser"); + + // Assert + var exception = response.GetServerException(); + Assert.Equal(typeof(InvalidOperationException).FullName, exception.ExceptionType); + Assert.Equal( + "More than one parameter and/or property is bound to the HTTP request's content.", + exception.ExceptionMessage); + } + + [Fact] + public async Task MultipleControllerPropertiesMarkedWithFromBody_Throws() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync("http://localhost/MultiplePropertiesFromBody/GetUser"); + + // Assert + var exception = response.GetServerException(); + Assert.Equal(typeof(InvalidOperationException).FullName, exception.ExceptionType); + Assert.Equal( + "More than one parameter and/or property is bound to the HTTP request's content.", + exception.ExceptionMessage); + } + [Fact] public async Task MultipleParameterAndPropertiesMarkedWithFromBody_Throws() { diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DataAnnotationsModelValidatorTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DataAnnotationsModelValidatorTest.cs index 7a594ea85e..ccdf812adc 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DataAnnotationsModelValidatorTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DataAnnotationsModelValidatorTest.cs @@ -279,7 +279,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation private static ModelValidationContext CreateValidationContext(ModelExplorer modelExplorer) { - return new ModelValidationContext(null, null, null, modelExplorer); + return new ModelValidationContext( + rootPrefix: null, + bindingSource: null, + modelState: null, + validatorProvider: null, + modelExplorer: modelExplorer); } private class DerivedRequiredAttribute : RequiredAttribute diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DefaultObjectValidatorTests.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DefaultObjectValidatorTests.cs index 9b983d589a..aea1abcdf3 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DefaultObjectValidatorTests.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DefaultObjectValidatorTests.cs @@ -509,6 +509,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation { ModelValidationContext = new ModelValidationContext( key, + null, TestModelValidatorProvider.CreateDefaultProvider(), modelStateDictionary, modelExplorer), diff --git a/test/WebSites/ApplicationModelWebSite/Controllers/ParameterModelController.cs b/test/WebSites/ApplicationModelWebSite/Controllers/ParameterModelController.cs index 305a98ffb6..eb5b8db253 100644 --- a/test/WebSites/ApplicationModelWebSite/Controllers/ParameterModelController.cs +++ b/test/WebSites/ApplicationModelWebSite/Controllers/ParameterModelController.cs @@ -23,6 +23,7 @@ namespace ApplicationModelWebSite { public void Apply(ParameterModel model) { + model.BindingInfo = model.BindingInfo ?? new BindingInfo(); model.BindingInfo.BindingSource = BindingSource.Custom; model.BindingInfo.BinderModelName = "CoolMetadata"; } diff --git a/test/WebSites/FormatterWebSite/ValidateBodyParameterAttribute.cs b/test/WebSites/FormatterWebSite/ValidateBodyParameterAttribute.cs index 85ac86d2e0..5310ca9e93 100644 --- a/test/WebSites/FormatterWebSite/ValidateBodyParameterAttribute.cs +++ b/test/WebSites/FormatterWebSite/ValidateBodyParameterAttribute.cs @@ -16,7 +16,7 @@ namespace FormatterWebSite var bodyParameter = context.ActionDescriptor .Parameters .FirstOrDefault(parameter => IsBodyBindingSource( - parameter.BindingInfo.BindingSource)); + parameter.BindingInfo?.BindingSource)); if (bodyParameter != null) { var parameterBindingErrors = context.ModelState[bodyParameter.Name].Errors; diff --git a/test/WebSites/ModelBindingWebSite/Controllers/FromBodyControllerPropertyController.cs b/test/WebSites/ModelBindingWebSite/Controllers/FromBodyControllerPropertyController.cs new file mode 100644 index 0000000000..b1e6eb3498 --- /dev/null +++ b/test/WebSites/ModelBindingWebSite/Controllers/FromBodyControllerPropertyController.cs @@ -0,0 +1,30 @@ +// 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 Microsoft.AspNet.Mvc; +using ModelBindingWebSite.Models; + +namespace ModelBindingWebSite.Controllers +{ + public class FromBodyControllerProperty : Controller + { + [FromBody] + public User SiteUser { get; set; } + + public User GetSiteUser(int id) + { + return SiteUser; + } + + // Will throw as Customer reads body. + public Customer GetCustomer(Customer customer) + { + return customer; + } + + // Will throw as a controller property and a parameter name are being read from body. + public void AddUser([FromBody] User user) + { + } + } +} \ No newline at end of file diff --git a/test/WebSites/ModelBindingWebSite/Controllers/MultiplePropertiesFromBodyController.cs b/test/WebSites/ModelBindingWebSite/Controllers/MultiplePropertiesFromBodyController.cs new file mode 100644 index 0000000000..9a09be02d2 --- /dev/null +++ b/test/WebSites/ModelBindingWebSite/Controllers/MultiplePropertiesFromBodyController.cs @@ -0,0 +1,22 @@ +// 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 Microsoft.AspNet.Mvc; +using ModelBindingWebSite.Models; + +namespace ModelBindingWebSite.Controllers +{ + public class MultiplePropertiesFromBodyController : Controller + { + [FromBody] + public User SiteUser { get; set; } + + [FromBody] + public Country Country { get; set; } + + public User GetUser() + { + return SiteUser; + } + } +} \ No newline at end of file diff --git a/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs b/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs index 6ef1f46c1d..4eb3915d4b 100644 --- a/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs +++ b/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs @@ -11,6 +11,9 @@ namespace ModelBindingWebSite.Controllers [Route("Validation/[Action]")] public class ValidationController : Controller { + [FromServices] + public ITestService ControllerService { get; set; } + public bool SkipValidation(Resident resident) { return ModelState.IsValid; @@ -21,6 +24,11 @@ namespace ModelBindingWebSite.Controllers return ModelState.IsValid; } + public bool DoNotValidateParameter([FromServices] ITestService service) + { + return ModelState.IsValid; + } + public IActionResult CreateRectangle([FromBody] Rectangle rectangle) { if (!ModelState.IsValid) diff --git a/test/WebSites/ModelBindingWebSite/ITestService.cs b/test/WebSites/ModelBindingWebSite/ITestService.cs index 8245aada13..0faf7e7c78 100644 --- a/test/WebSites/ModelBindingWebSite/ITestService.cs +++ b/test/WebSites/ModelBindingWebSite/ITestService.cs @@ -1,10 +1,15 @@ // 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.ComponentModel.DataAnnotations; + namespace ModelBindingWebSite { public interface ITestService { + [Required] + string NeverBound { get; set; } + bool Test(); } } diff --git a/test/WebSites/ModelBindingWebSite/TestService.cs b/test/WebSites/ModelBindingWebSite/TestService.cs index c72c62c350..4ccce576df 100644 --- a/test/WebSites/ModelBindingWebSite/TestService.cs +++ b/test/WebSites/ModelBindingWebSite/TestService.cs @@ -1,10 +1,15 @@ // 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.ComponentModel.DataAnnotations; + namespace ModelBindingWebSite { public class TestService : ITestService { + [Required] + public string NeverBound { get; set; } + public bool Test() { return true;