#2267 - Moving responsibility for setting ModelState.MaxAllowedErrors into a more appropriate location.

This commit is contained in:
sornaks 2015-04-09 09:57:30 -07:00
parent 8bae02928d
commit 064c01cf2b
8 changed files with 64 additions and 94 deletions

View File

@ -30,7 +30,8 @@ namespace Microsoft.AspNet.Mvc.Core
[NotNull] IReadOnlyList<IModelValidatorProvider> modelValidatorProviders, [NotNull] IReadOnlyList<IModelValidatorProvider> modelValidatorProviders,
[NotNull] IReadOnlyList<IValueProviderFactory> valueProviderFactories, [NotNull] IReadOnlyList<IValueProviderFactory> valueProviderFactories,
[NotNull] IScopedInstance<ActionBindingContext> actionBindingContextAccessor, [NotNull] IScopedInstance<ActionBindingContext> actionBindingContextAccessor,
[NotNull] ITempDataDictionary tempData) [NotNull] ITempDataDictionary tempData,
int maxModelValidationErrors)
: base( : base(
actionContext, actionContext,
filterProviders, filterProviders,
@ -39,7 +40,8 @@ namespace Microsoft.AspNet.Mvc.Core
modelBinders, modelBinders,
modelValidatorProviders, modelValidatorProviders,
valueProviderFactories, valueProviderFactories,
actionBindingContextAccessor) actionBindingContextAccessor,
maxModelValidationErrors)
{ {
_descriptor = descriptor; _descriptor = descriptor;
_controllerFactory = controllerFactory; _controllerFactory = controllerFactory;

View File

@ -22,6 +22,7 @@ namespace Microsoft.AspNet.Mvc.Core
private readonly IReadOnlyList<IValueProviderFactory> _valueProviderFactories; private readonly IReadOnlyList<IValueProviderFactory> _valueProviderFactories;
private readonly IScopedInstance<ActionBindingContext> _actionBindingContextAccessor; private readonly IScopedInstance<ActionBindingContext> _actionBindingContextAccessor;
private readonly ITempDataDictionary _tempData; private readonly ITempDataDictionary _tempData;
private readonly int _maxModelValidationErrors;
public ControllerActionInvokerProvider( public ControllerActionInvokerProvider(
IControllerFactory controllerFactory, IControllerFactory controllerFactory,
@ -41,6 +42,7 @@ namespace Microsoft.AspNet.Mvc.Core
_valueProviderFactories = optionsAccessor.Options.ValueProviderFactories.ToArray(); _valueProviderFactories = optionsAccessor.Options.ValueProviderFactories.ToArray();
_actionBindingContextAccessor = actionBindingContextAccessor; _actionBindingContextAccessor = actionBindingContextAccessor;
_tempData = tempData; _tempData = tempData;
_maxModelValidationErrors = optionsAccessor.Options.MaxModelValidationErrors;
} }
public int Order public int Order
@ -67,7 +69,8 @@ namespace Microsoft.AspNet.Mvc.Core
_modelValidatorProviders, _modelValidatorProviders,
_valueProviderFactories, _valueProviderFactories,
_actionBindingContextAccessor, _actionBindingContextAccessor,
_tempData); _tempData,
_maxModelValidationErrors);
} }
} }

View File

@ -20,16 +20,13 @@ namespace Microsoft.AspNet.Mvc
public class DefaultControllerActionArgumentBinder : IControllerActionArgumentBinder public class DefaultControllerActionArgumentBinder : IControllerActionArgumentBinder
{ {
private readonly IModelMetadataProvider _modelMetadataProvider; private readonly IModelMetadataProvider _modelMetadataProvider;
private readonly MvcOptions _options;
private readonly IObjectModelValidator _validator; private readonly IObjectModelValidator _validator;
public DefaultControllerActionArgumentBinder( public DefaultControllerActionArgumentBinder(
IModelMetadataProvider modelMetadataProvider, IModelMetadataProvider modelMetadataProvider,
IObjectModelValidator validator, IObjectModelValidator validator)
IOptions<MvcOptions> optionsAccessor)
{ {
_modelMetadataProvider = modelMetadataProvider; _modelMetadataProvider = modelMetadataProvider;
_options = optionsAccessor.Options;
_validator = validator; _validator = validator;
} }
@ -89,7 +86,6 @@ namespace Microsoft.AspNet.Mvc
IDictionary<string, object> arguments, IDictionary<string, object> arguments,
IEnumerable<ParameterDescriptor> parameterMetadata) IEnumerable<ParameterDescriptor> parameterMetadata)
{ {
modelState.MaxAllowedErrors = _options.MaxModelValidationErrors;
foreach (var parameter in parameterMetadata) foreach (var parameter in parameterMetadata)
{ {
var metadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterType); var metadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterType);

View File

@ -21,8 +21,8 @@ namespace Microsoft.AspNet.Mvc.Core
private readonly IReadOnlyList<IOutputFormatter> _outputFormatters; private readonly IReadOnlyList<IOutputFormatter> _outputFormatters;
private readonly IReadOnlyList<IModelValidatorProvider> _modelValidatorProviders; private readonly IReadOnlyList<IModelValidatorProvider> _modelValidatorProviders;
private readonly IReadOnlyList<IValueProviderFactory> _valueProviderFactories; private readonly IReadOnlyList<IValueProviderFactory> _valueProviderFactories;
private readonly IScopedInstance<ActionBindingContext> _actionBindingContextAccessor; private readonly IScopedInstance<ActionBindingContext> _actionBindingContextAccessor;
private readonly int _maxModelValidationErrors;
private IFilter[] _filters; private IFilter[] _filters;
private FilterCursor _cursor; private FilterCursor _cursor;
@ -48,7 +48,8 @@ namespace Microsoft.AspNet.Mvc.Core
[NotNull] IReadOnlyList<IModelBinder> modelBinders, [NotNull] IReadOnlyList<IModelBinder> modelBinders,
[NotNull] IReadOnlyList<IModelValidatorProvider> modelValidatorProviders, [NotNull] IReadOnlyList<IModelValidatorProvider> modelValidatorProviders,
[NotNull] IReadOnlyList<IValueProviderFactory> valueProviderFactories, [NotNull] IReadOnlyList<IValueProviderFactory> valueProviderFactories,
[NotNull] IScopedInstance<ActionBindingContext> actionBindingContextAccessor) [NotNull] IScopedInstance<ActionBindingContext> actionBindingContextAccessor,
int maxModelValidationErrors)
{ {
ActionContext = actionContext; ActionContext = actionContext;
@ -59,6 +60,7 @@ namespace Microsoft.AspNet.Mvc.Core
_modelValidatorProviders = modelValidatorProviders; _modelValidatorProviders = modelValidatorProviders;
_valueProviderFactories = valueProviderFactories; _valueProviderFactories = valueProviderFactories;
_actionBindingContextAccessor = actionBindingContextAccessor; _actionBindingContextAccessor = actionBindingContextAccessor;
_maxModelValidationErrors = maxModelValidationErrors;
} }
protected ActionContext ActionContext { get; private set; } protected ActionContext ActionContext { get; private set; }
@ -101,6 +103,8 @@ namespace Microsoft.AspNet.Mvc.Core
_filters = GetFilters(); _filters = GetFilters();
_cursor = new FilterCursor(_filters); _cursor = new FilterCursor(_filters);
ActionContext.ModelState.MaxAllowedErrors = _maxModelValidationErrors;
await InvokeAllAuthorizationFiltersAsync(); await InvokeAllAuthorizationFiltersAsync();
// If Authorization Filters return a result, it's a short circuit because // If Authorization Filters return a result, it's a short circuit because

View File

@ -93,9 +93,6 @@ namespace Microsoft.AspNet.Mvc
var actionContext = new ActionContext(context.HttpContext, context.RouteData, actionDescriptor); var actionContext = new ActionContext(context.HttpContext, context.RouteData, actionDescriptor);
var optionsAccessor = services.GetRequiredService<IOptions<MvcOptions>>();
actionContext.ModelState.MaxAllowedErrors = optionsAccessor.Options.MaxModelValidationErrors;
var contextAccessor = services.GetRequiredService<IScopedInstance<ActionContext>>(); var contextAccessor = services.GetRequiredService<IScopedInstance<ActionContext>>();
contextAccessor.Value = actionContext; contextAccessor.Value = actionContext;
var invokerFactory = services.GetRequiredService<IActionInvokerFactory>(); var invokerFactory = services.GetRequiredService<IActionInvokerFactory>();

View File

@ -13,6 +13,7 @@ using Microsoft.AspNet.Mvc.ModelBinding;
using Microsoft.AspNet.Mvc.ModelBinding.Validation; using Microsoft.AspNet.Mvc.ModelBinding.Validation;
using Microsoft.AspNet.Routing; using Microsoft.AspNet.Routing;
using Microsoft.AspNet.Testing; using Microsoft.AspNet.Testing;
using Microsoft.Framework.Internal;
using Microsoft.Framework.Logging; using Microsoft.Framework.Logging;
using Microsoft.Framework.Logging.Testing; using Microsoft.Framework.Logging.Testing;
using Microsoft.Framework.OptionsModel; using Microsoft.Framework.OptionsModel;
@ -1959,18 +1960,37 @@ namespace Microsoft.AspNet.Mvc
Assert.Same(input, contentResult.Value); Assert.Same(input, contentResult.Value);
} }
[Fact]
public async Task MaxAllowedErrorsIsSet_BeforeCallingAuthorizationFilter()
{
// Arrange
var expected = 147;
var filter = new MockAuthorizationFilter(expected);
var invoker = CreateInvoker(
filter,
actionThrows: false,
tempData: null,
maxAllowedErrorsInModelState: expected);
// Act & Assert
// The authorization filter asserts if MaxAllowedErrors was set to the right value.
await invoker.InvokeAsync();
}
private TestControllerActionInvoker CreateInvoker( private TestControllerActionInvoker CreateInvoker(
IFilter filter, IFilter filter,
bool actionThrows = false, bool actionThrows = false,
ITempDataDictionary tempData = null) ITempDataDictionary tempData = null,
int maxAllowedErrorsInModelState = 200)
{ {
return CreateInvoker(new[] { filter }, actionThrows, tempData); return CreateInvoker(new[] { filter }, actionThrows, tempData, maxAllowedErrorsInModelState);
} }
private TestControllerActionInvoker CreateInvoker( private TestControllerActionInvoker CreateInvoker(
IFilter[] filters, IFilter[] filters,
bool actionThrows = false, bool actionThrows = false,
ITempDataDictionary tempData = null) ITempDataDictionary tempData = null,
int maxAllowedErrorsInModelState = 200)
{ {
var actionDescriptor = new ControllerActionDescriptor() var actionDescriptor = new ControllerActionDescriptor()
{ {
@ -2044,7 +2064,8 @@ namespace Microsoft.AspNet.Mvc
new IModelValidatorProvider[0], new IModelValidatorProvider[0],
new IValueProviderFactory[0], new IValueProviderFactory[0],
new MockScopedInstance<ActionBindingContext>(), new MockScopedInstance<ActionBindingContext>(),
tempData); tempData,
maxAllowedErrorsInModelState);
return invoker; return invoker;
} }
@ -2100,13 +2121,13 @@ namespace Microsoft.AspNet.Mvc
new IOutputFormatter[0], new IOutputFormatter[0],
new DefaultControllerActionArgumentBinder( new DefaultControllerActionArgumentBinder(
metadataProvider, metadataProvider,
new DefaultObjectValidator(new IExcludeTypeValidationFilter[0], metadataProvider), new DefaultObjectValidator(new IExcludeTypeValidationFilter[0], metadataProvider)),
new MockMvcOptionsAccessor()),
new IModelBinder[] { binder.Object }, new IModelBinder[] { binder.Object },
new IModelValidatorProvider[0], new IModelValidatorProvider[0],
new IValueProviderFactory[0], new IValueProviderFactory[0],
new MockScopedInstance<ActionBindingContext>(), new MockScopedInstance<ActionBindingContext>(),
Mock.Of<ITempDataDictionary>()); Mock.Of<ITempDataDictionary>(),
200);
// Act // Act
await invoker.InvokeAsync(); await invoker.InvokeAsync();
@ -2205,7 +2226,8 @@ namespace Microsoft.AspNet.Mvc
IReadOnlyList<IModelValidatorProvider> modelValidatorProviders, IReadOnlyList<IModelValidatorProvider> modelValidatorProviders,
IReadOnlyList<IValueProviderFactory> valueProviderFactories, IReadOnlyList<IValueProviderFactory> valueProviderFactories,
IScopedInstance<ActionBindingContext> actionBindingContext, IScopedInstance<ActionBindingContext> actionBindingContext,
ITempDataDictionary tempData) ITempDataDictionary tempData,
int maxAllowedErrorsInModelState)
: base( : base(
actionContext, actionContext,
filterProvider, filterProvider,
@ -2218,7 +2240,8 @@ namespace Microsoft.AspNet.Mvc
modelValidatorProviders, modelValidatorProviders,
valueProviderFactories, valueProviderFactories,
actionBindingContext, actionBindingContext,
tempData) tempData,
maxAllowedErrorsInModelState)
{ {
ControllerFactory = controllerFactory; ControllerFactory = controllerFactory;
} }
@ -2233,5 +2256,21 @@ namespace Microsoft.AspNet.Mvc
ControllerFactory.Verify(); ControllerFactory.Verify();
} }
} }
private class MockAuthorizationFilter : IAuthorizationFilter
{
int _expectedMaxAllowedErrors;
public MockAuthorizationFilter(int maxAllowedErrors)
{
_expectedMaxAllowedErrors = maxAllowedErrors;
}
public void OnAuthorization([NotNull]AuthorizationContext context)
{
Assert.NotNull(context.ModelState.MaxAllowedErrors);
Assert.Equal(_expectedMaxAllowedErrors, context.ModelState.MaxAllowedErrors);
}
}
} }
} }

View File

@ -67,42 +67,6 @@ namespace Microsoft.AspNet.Mvc
Assert.Equal(expectedMessage, sink.Writes[0].State?.ToString()); Assert.Equal(expectedMessage, sink.Writes[0].State?.ToString());
} }
[Fact]
public async Task RouteAsync_SetsMaxErrorCountOnModelStateDictionary()
{
// Arrange
var expected = 199;
var optionsAccessor = new Mock<IOptions<MvcOptions>>();
var options = new MvcOptions
{
MaxModelValidationErrors = expected
};
optionsAccessor.SetupGet(o => o.Options)
.Returns(options);
var invoked = false;
var mockInvokerFactory = new Mock<IActionInvokerFactory>();
mockInvokerFactory.Setup(f => f.CreateInvoker(It.IsAny<ActionContext>()))
.Callback<ActionContext>(c =>
{
Assert.Equal(expected, c.ModelState.MaxAllowedErrors);
invoked = true;
})
.Returns(Mock.Of<IActionInvoker>());
var context = CreateRouteContext(
invokerFactory: mockInvokerFactory.Object,
optionsAccessor: optionsAccessor.Object);
var handler = new MvcRouteHandler();
// Act
await handler.RouteAsync(context);
// Assert
Assert.True(invoked);
}
[Fact] [Fact]
public async Task RouteAsync_CreatesNewRouteData() public async Task RouteAsync_CreatesNewRouteData()
{ {

View File

@ -245,37 +245,6 @@ namespace Microsoft.AspNet.Mvc.Core.Test
mockValidatorProvider.Verify(o => o.Validate(It.IsAny<ModelValidationContext>()), Times.Never()); mockValidatorProvider.Verify(o => o.Validate(It.IsAny<ModelValidationContext>()), Times.Never());
} }
[Fact]
public async Task GetActionArgumentsAsync_SetsMaxModelErrors()
{
// Arrange
Func<object, int> method = foo => 1;
var actionDescriptor = GetActionDescriptor();
actionDescriptor.Parameters.Add(
new ParameterDescriptor
{
Name = "foo",
ParameterType = typeof(object),
BindingInfo = new BindingInfo(),
});
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] [Fact]
public async Task GetActionArgumentsAsync_CallsValidator_ForControllerProperties_IfModelBinderSucceeds() public async Task GetActionArgumentsAsync_CallsValidator_ForControllerProperties_IfModelBinderSucceeds()
{ {
@ -409,9 +378,6 @@ namespace Microsoft.AspNet.Mvc.Core.Test
private static DefaultControllerActionArgumentBinder GetArgumentBinder(IObjectModelValidator validator = null) private static DefaultControllerActionArgumentBinder GetArgumentBinder(IObjectModelValidator validator = null)
{ {
var options = new MockMvcOptionsAccessor();
options.Options.MaxModelValidationErrors = 5;
if (validator == null) if (validator == null)
{ {
var mockValidator = new Mock<IObjectModelValidator>(MockBehavior.Strict); var mockValidator = new Mock<IObjectModelValidator>(MockBehavior.Strict);
@ -421,8 +387,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test
return new DefaultControllerActionArgumentBinder( return new DefaultControllerActionArgumentBinder(
TestModelMetadataProvider.CreateDefaultProvider(), TestModelMetadataProvider.CreateDefaultProvider(),
validator, validator);
options);
} }
private class TestController private class TestController