Add a switch to allow turning on ValidationVisitor shortcircuiting (#8599)

This commit is contained in:
Pranav K 2018-10-17 15:47:01 -07:00 committed by GitHub
parent 10a94aa7bb
commit 27e75e7a51
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 63 additions and 5 deletions

View File

@ -73,6 +73,7 @@ namespace Microsoft.AspNetCore.Mvc
/// <item><description><c>ApiBehaviorOptions.SuppressUseValidationProblemDetailsForInvalidModelStateResponses</c></description></item>
/// <item><description><c>MvcDataAnnotationsLocalizationOptions.AllowDataAnnotationsLocalizationForEnumDisplayAttributes</c></description></item>
/// <item><description><see cref="MvcOptions.EnableEndpointRouting" /></description></item>
/// <item><description><see cref="MvcOptions.AllowShortCircuitingValidationWhenNoValidatorsArePresent"/></description></item>
/// <item><description><see cref="MvcOptions.MaxValidationDepth" /></description></item>
/// <item><description><c>RazorPagesOptions.AllowDefaultHandlingForOptionsRequests</c></description></item>
/// <item><description><c>RazorViewEngineOptions.AllowRecompilingViewsOnFileChange</c></description></item>

View File

@ -5,7 +5,6 @@ using System.Collections.Generic;
using Microsoft.AspNetCore.Mvc.Internal;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
using Microsoft.Extensions.Options;
namespace Microsoft.AspNetCore.Mvc
{
@ -46,6 +45,7 @@ namespace Microsoft.AspNetCore.Mvc
validationState);
visitor.MaxValidationDepth = _mvcOptions.MaxValidationDepth;
visitor.AllowShortCircuitingValidationWhenNoValidatorsArePresent = _mvcOptions.AllowShortCircuitingValidationWhenNoValidatorsArePresent;
return visitor;
}

View File

@ -38,6 +38,9 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
// Matches JsonSerializerSettingsProvider.DefaultMaxDepth
values[nameof(MvcOptions.MaxValidationDepth)] = 32;
values[nameof(MvcOptions.AllowShortCircuitingValidationWhenNoValidatorsArePresent)] = true;
}
return values;

View File

@ -110,6 +110,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation
/// </summary>
public bool ValidateComplexTypesIfChildValidationFails { get; set; }
/// <summary>
/// Gets or sets a value that determines if <see cref="ValidationVisitor"/> can short circuit validation when a model
/// does not have any associated validators.
/// </summary>
public bool AllowShortCircuitingValidationWhenNoValidatorsArePresent { get; set; }
/// <summary>
/// Validates a object.
/// </summary>
@ -267,7 +273,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation
}
// If the metadata indicates that no validators exist AND the aggregate state for the key says that the model graph
// is not invalid (i.e. is one of Unvalidated, Valid, or Skipped) we can safely mark the graph as valid.
else if (metadata.HasValidators == false &&
else if (
AllowShortCircuitingValidationWhenNoValidatorsArePresent &&
metadata.HasValidators == false &&
ModelState.GetFieldValidationState(key) != ModelValidationState.Invalid)
{
// No validators will be created for this graph of objects. Mark it as valid if it wasn't previously validated.

View File

@ -32,6 +32,7 @@ namespace Microsoft.AspNetCore.Mvc
private readonly CompatibilitySwitch<bool> _suppressBindingUndefinedValueToEnumType;
private readonly CompatibilitySwitch<bool> _enableEndpointRouting;
private readonly NullableCompatibilitySwitch<int> _maxValidationDepth;
private readonly CompatibilitySwitch<bool> _allowShortCircuitingValidationWhenNoValidatorsArePresent;
private readonly ICompatibilitySwitch[] _switches;
/// <summary>
@ -58,6 +59,7 @@ namespace Microsoft.AspNetCore.Mvc
_suppressBindingUndefinedValueToEnumType = new CompatibilitySwitch<bool>(nameof(SuppressBindingUndefinedValueToEnumType));
_enableEndpointRouting = new CompatibilitySwitch<bool>(nameof(EnableEndpointRouting));
_maxValidationDepth = new NullableCompatibilitySwitch<int>(nameof(MaxValidationDepth));
_allowShortCircuitingValidationWhenNoValidatorsArePresent = new CompatibilitySwitch<bool>(nameof(AllowShortCircuitingValidationWhenNoValidatorsArePresent));
_switches = new ICompatibilitySwitch[]
{
@ -68,6 +70,7 @@ namespace Microsoft.AspNetCore.Mvc
_suppressBindingUndefinedValueToEnumType,
_enableEndpointRouting,
_maxValidationDepth,
_allowShortCircuitingValidationWhenNoValidatorsArePresent,
};
}
@ -442,6 +445,44 @@ namespace Microsoft.AspNetCore.Mvc
}
}
/// <summary>
/// Gets or sets a value that determines if <see cref="ValidationVisitor"/>
/// can short-circuit validation when a model does not have any associated validators.
/// </summary>
/// <value>
/// The default value is <see langword="true"/> if the version is
/// <see cref="CompatibilityVersion.Version_2_2"/> or later; <see langword="false"/> otherwise.
/// </value>
/// <remarks>
/// When <see cref="ModelMetadata.HasValidators"/> is <see langword="true"/>, that is, it is determined
/// that a model or any of it's properties or collection elements cannot have any validators,
/// <see cref="ValidationVisitor"/> can short-circuit validation for the model and mark the object
/// graph as valid. Setting this property to <see langword="true"/>, allows <see cref="ValidationVisitor"/> to
/// perform this optimization.
/// <para>
/// This property is associated with a compatibility switch and can provide a different behavior depending on
/// the configured compatibility version for the application. See <see cref="CompatibilityVersion"/> for
/// guidance and examples of setting the application's compatibility version.
/// </para>
/// <para>
/// Configuring the desired value of the compatibility switch by calling this property's setter will take precedence
/// over the value implied by the application's <see cref="CompatibilityVersion"/>.
/// </para>
/// <para>
/// If the application's compatibility version is set to <see cref="CompatibilityVersion.Version_2_2"/> then
/// this setting will have the value <see langword="true"/> unless explicitly configured.
/// </para>
/// <para>
/// If the application's compatibility version is set to <see cref="CompatibilityVersion.Version_2_1"/> or
/// earlier then this setting will have the value <see langword="false"/> unless explicitly configured.
/// </para>
/// </remarks>
public bool AllowShortCircuitingValidationWhenNoValidatorsArePresent
{
get => _allowShortCircuitingValidationWhenNoValidatorsArePresent.Value;
set => _allowShortCircuitingValidationWhenNoValidatorsArePresent.Value = value;
}
IEnumerator<ICompatibilitySwitch> IEnumerable<ICompatibilitySwitch>.GetEnumerator()
{
return ((IEnumerable<ICompatibilitySwitch>)_switches).GetEnumerator();

View File

@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
{
public class DefaultObjectValidatorTests
{
private readonly MvcOptions _options = new MvcOptions();
private readonly MvcOptions _options = new MvcOptions { AllowShortCircuitingValidationWhenNoValidatorsArePresent = true };
private ModelMetadataProvider MetadataProvider { get; } = TestModelMetadataProvider.CreateDefaultProvider();

View File

@ -129,7 +129,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
return new DefaultObjectValidator(
metadataProvider,
GetModelValidatorProviders(options),
options?.Value ?? new MvcOptions());
options?.Value ?? new MvcOptions { AllowShortCircuitingValidationWhenNoValidatorsArePresent = true });
}
private static IList<IModelValidatorProvider> GetModelValidatorProviders(IOptions<MvcOptions> options)
@ -197,7 +197,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
serviceCollection
.AddSingleton<ObjectPoolProvider, DefaultObjectPoolProvider>()
.AddSingleton<ILoggerFactory>(NullLoggerFactory.Instance)
.AddTransient<ILogger<DefaultAuthorizationService>, Logger<DefaultAuthorizationService>>();
.AddTransient<ILogger<DefaultAuthorizationService>, Logger<DefaultAuthorizationService>>()
.Configure<MvcOptions>(options => options.AllowShortCircuitingValidationWhenNoValidatorsArePresent = true);
if (updateOptions != null)
{

View File

@ -56,6 +56,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest
Assert.True(razorViewEngineOptions.AllowRecompilingViewsOnFileChange);
Assert.False(razorPagesOptions.AllowDefaultHandlingForOptionsRequests);
Assert.False(xmlOptions.AllowRfc7807CompliantProblemDetailsFormat);
Assert.False(mvcOptions.AllowShortCircuitingValidationWhenNoValidatorsArePresent);
}
[Fact]
@ -93,6 +94,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest
Assert.True(razorViewEngineOptions.AllowRecompilingViewsOnFileChange);
Assert.False(razorPagesOptions.AllowDefaultHandlingForOptionsRequests);
Assert.False(xmlOptions.AllowRfc7807CompliantProblemDetailsFormat);
Assert.False(mvcOptions.AllowShortCircuitingValidationWhenNoValidatorsArePresent);
}
[Fact]
@ -130,6 +132,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest
Assert.False(razorViewEngineOptions.AllowRecompilingViewsOnFileChange);
Assert.True(razorPagesOptions.AllowDefaultHandlingForOptionsRequests);
Assert.True(xmlOptions.AllowRfc7807CompliantProblemDetailsFormat);
Assert.True(mvcOptions.AllowShortCircuitingValidationWhenNoValidatorsArePresent);
}
[Fact]
@ -167,6 +170,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest
Assert.False(razorViewEngineOptions.AllowRecompilingViewsOnFileChange);
Assert.True(razorPagesOptions.AllowDefaultHandlingForOptionsRequests);
Assert.True(xmlOptions.AllowRfc7807CompliantProblemDetailsFormat);
Assert.True(mvcOptions.AllowShortCircuitingValidationWhenNoValidatorsArePresent);
}
// This just does the minimum needed to be able to resolve these options.