Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Cache the results of ModelValidatorProviders across model binding and…

… value validation

ModelValidatorProviders can be slow since they may reflect over types to decide what validators to provide. So it is important that the results are cached so that runtime performance isn't affected by slow validator providers on every request. This changeset introduces an internal cache on the configuration properties used to cache the validators that will be provided for a given type or property. This cache only applies to the use of the extension method actionContext.GetValidators(metadata) and its callers.
  • Loading branch information...
commit ca103b45d3d0487355df2f4ccf942de5b4f0d6be 1 parent 4c1ee37
youssefm authored
View
13 src/System.Web.Http/Controllers/HttpActionContextExtensions.cs
@@ -58,8 +58,17 @@ public static IEnumerable<ModelValidator> GetValidators(this HttpActionContext a
throw Error.ArgumentNull("actionContext");
}
- IEnumerable<ModelValidatorProvider> validatorProviders = GetValidatorProviders(actionContext);
- return validatorProviders.SelectMany(provider => provider.GetValidators(metadata, validatorProviders));
+ HttpConfiguration configuration = actionContext.ControllerContext.Configuration;
+ if (configuration.Services.IsSingleService(typeof(ModelValidatorCache)))
+ {
+ ModelValidatorCache validatorCache = configuration.Services.GetModelValidatorCache();
+ return validatorCache.GetValidators(metadata);
+ }
+ else
+ {
+ // slow path: there is no validator cache on the configuration
+ return metadata.GetValidators(actionContext.GetValidatorProviders());
+ }
}
public static bool TryBindStrongModel<TModel>(this HttpActionContext actionContext, ModelBindingContext parentBindingContext, string propertyName, ModelMetadataProvider metadataProvider, out TModel model)
View
31 src/System.Web.Http/HttpConfiguration.cs
@@ -5,6 +5,7 @@
using System.Collections.ObjectModel;
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics.Contracts;
+using System.Linq;
using System.Net.Http;
using System.Net.Http.Formatting;
using System.Web.Http.Controllers;
@@ -32,6 +33,7 @@ public class HttpConfiguration : IDisposable
private IDependencyResolver _dependencyResolver = EmptyResolver.Instance;
private Action<HttpConfiguration> _initializer = DefaultInitializer;
+ private List<IDisposable> _resourcesToDispose = new List<IDisposable>();
private bool _disposed;
/// <summary>
@@ -60,6 +62,7 @@ public HttpConfiguration(HttpRouteCollection routes)
ParameterBindingRules = DefaultActionValueBinder.GetDefaultParameterBinders();
}
+ [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "We're registering the ValidationCache to be disposed by the HttpConfiguration.")]
private HttpConfiguration(HttpConfiguration configuration, HttpControllerSettings settings)
{
_routes = configuration.Routes;
@@ -77,6 +80,15 @@ private HttpConfiguration(HttpConfiguration configuration, HttpControllerSetting
// Use the original configuration's initializer so that its Initialize()
// will perform the same logic on this clone as on the original.
Initializer = configuration.Initializer;
+
+ // create a new validator cache if the validator providers have changed
+ if (settings.IsServiceCollectionInitialized &&
+ !settings.Services.GetModelValidatorProviders().SequenceEqual(configuration.Services.GetModelValidatorProviders()))
+ {
+ ModelValidatorCache validatorCache = new ModelValidatorCache(new Lazy<IEnumerable<ModelValidatorProvider>>(() => Services.GetModelValidatorProviders()));
+ RegisterForDispose(validatorCache);
+ settings.Services.Replace(typeof(ModelValidatorCache), validatorCache);
+ }
}
/// <summary>
@@ -280,6 +292,20 @@ internal bool ShouldIncludeErrorDetail(HttpRequestMessage request)
}
}
+ /// <summary>
+ /// Adds the given <paramref name="resource"/> to a list of resources that will be disposed once the configuration is disposed.
+ /// </summary>
+ /// <param name="resource">The resource to dispose. Can be <c>null</c>.</param>
+ internal void RegisterForDispose(IDisposable resource)
+ {
+ if (resource == null)
+ {
+ return;
+ }
+
+ _resourcesToDispose.Add(resource);
+ }
+
public void Dispose()
{
Dispose(true);
@@ -296,6 +322,11 @@ protected virtual void Dispose(bool disposing)
_routes.Dispose();
Services.Dispose();
DependencyResolver.Dispose();
+
+ foreach (IDisposable resource in _resourcesToDispose)
+ {
+ resource.Dispose();
+ }
}
}
}
View
6 src/System.Web.Http/Services/DefaultServices.cs
@@ -52,6 +52,7 @@ namespace System.Web.Http.Services
/// <item><see cref="IStructuredQueryBuilder"/></item>
/// <item><see cref="ModelBinderProvider"/></item>
/// <item><see cref="ModelMetadataProvider"/></item>
+ /// <item><see cref="ModelValidatorCache"/></item>
/// <item><see cref="ModelValidatorProvider"/></item>
/// <item><see cref="ValueProviderFactory"/></item>
/// </list>
@@ -95,6 +96,7 @@ protected DefaultServices()
}
[SuppressMessage("Microsoft.Maintainability", "CA1506:AvoidExcessiveClassCoupling", Justification = "Class needs references to large number of types.")]
+ [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "We're registering the ValidationCache to be disposed by the HttpConfiguration.")]
public DefaultServices(HttpConfiguration configuration)
{
if (configuration == null)
@@ -145,6 +147,10 @@ public DefaultServices(HttpConfiguration configuration)
SetMultiple<ValueProviderFactory>(new QueryStringValueProviderFactory(),
new RouteDataValueProviderFactory());
+ ModelValidatorCache validatorCache = new ModelValidatorCache(new Lazy<IEnumerable<ModelValidatorProvider>>(() => this.GetModelValidatorProviders()));
+ configuration.RegisterForDispose(validatorCache);
+ SetSingle<ModelValidatorCache>(validatorCache);
+
_serviceTypesSingle = new HashSet<Type>(_defaultServicesSingle.Keys);
_serviceTypesMulti = new HashSet<Type>(_defaultServicesMulti.Keys);
View
5 src/System.Web.Http/ServicesExtensions.cs
@@ -52,6 +52,11 @@ public static IEnumerable<ModelValidatorProvider> GetModelValidatorProviders(thi
return services.GetServices<ModelValidatorProvider>();
}
+ internal static ModelValidatorCache GetModelValidatorCache(this ServicesContainer services)
+ {
+ return services.GetService<ModelValidatorCache>();
+ }
+
public static IContentNegotiator GetContentNegotiator(this ServicesContainer services)
{
return services.GetService<IContentNegotiator>();
View
1  src/System.Web.Http/System.Web.Http.csproj
@@ -99,6 +99,7 @@
<Compile Include="Controllers\IActionResultConverter.cs" />
<Compile Include="Controllers\HttpActionContext.cs" />
<Compile Include="Controllers\IControllerConfiguration.cs" />
+ <Compile Include="Validation\ModelValidatorCache.cs" />
<Compile Include="Controllers\ResponseMessageResultConverter.cs" />
<Compile Include="Controllers\ValueResultConverter.cs" />
<Compile Include="Controllers\VoidResultConverter.cs" />
View
28 src/System.Web.Http/Validation/DefaultBodyModelValidator.cs
@@ -18,9 +18,6 @@ namespace System.Web.Http.Validation
/// </summary>
public class DefaultBodyModelValidator : IBodyModelValidator
{
- // Keyed on (Type, propertyName) tuple
- private ConcurrentDictionary<Tuple<Type, string>, IEnumerable<ModelValidator>> _validatorCache = new ConcurrentDictionary<Tuple<Type, string>, IEnumerable<ModelValidator>>();
-
private interface IKeyBuilder
{
string AppendTo(string prefix);
@@ -69,7 +66,7 @@ public bool Validate(object model, Type type, ModelMetadataProvider metadataProv
ValidationContext validationContext = new ValidationContext()
{
MetadataProvider = metadataProvider,
- ValidatorProviders = validatorProviders,
+ ActionContext = actionContext,
ModelState = actionContext.ModelState,
Visited = new HashSet<object>(),
KeyBuilders = new Stack<IKeyBuilder>(),
@@ -158,11 +155,11 @@ private bool ValidateElements(IEnumerable model, ValidationContext validationCon
// Validates a single node (not including children)
// Returns true if validation passes successfully
- private bool ShallowValidate(ModelMetadata metadata, ValidationContext validationContext, object container)
+ private static bool ShallowValidate(ModelMetadata metadata, ValidationContext validationContext, object container)
{
bool isValid = true;
string key = null;
- foreach (ModelValidator validator in GetValidators(validationContext.ValidatorProviders, metadata))
+ foreach (ModelValidator validator in validationContext.ActionContext.GetValidators(metadata))
{
foreach (ModelValidationResult error in validator.Validate(metadata, container))
{
@@ -188,23 +185,6 @@ private bool ShallowValidate(ModelMetadata metadata, ValidationContext validatio
return isValid;
}
- private IEnumerable<ModelValidator> GetValidators(IEnumerable<ModelValidatorProvider> validatorProviders, ModelMetadata metadata)
- {
- // If metadata is for a property then containerType != null && propertyName != null
- // If metadata is for a type then containerType == null && propertyName == null, so we have to use modelType for the cache key.
- Type typeForCache = metadata.ContainerType ?? metadata.ModelType;
- Tuple<Type, string> cacheKey = Tuple.Create(typeForCache, metadata.PropertyName);
-
- // This retrieval is implemented as a TryGetValue/TryAdd instead of a GetOrAdd to avoid the performance cost of creating delegates
- IEnumerable<ModelValidator> validators;
- if (!_validatorCache.TryGetValue(cacheKey, out validators))
- {
- validators = metadata.GetValidators(validatorProviders).ToArray();
- _validatorCache.TryAdd(cacheKey, validators);
- }
- return validators;
- }
-
private static Type GetElementType(Type type)
{
Contract.Assert(typeof(IEnumerable).IsAssignableFrom(type));
@@ -247,7 +227,7 @@ public string AppendTo(string prefix)
private class ValidationContext
{
public ModelMetadataProvider MetadataProvider { get; set; }
- public IEnumerable<ModelValidatorProvider> ValidatorProviders { get; set; }
+ public HttpActionContext ActionContext { get; set; }
public ModelStateDictionary ModelState { get; set; }
public HashSet<object> Visited { get; set; }
public Stack<IKeyBuilder> KeyBuilders { get; set; }
View
12 src/System.Web.Http/Validation/ModelValidationNode.cs
@@ -12,10 +12,7 @@ namespace System.Web.Http.Validation
{
public sealed class ModelValidationNode
{
- // This is a cache of the validators.
- // Use an array instead of IEnumerable to ensure that we've actually computed the result.
- // Array also reduces the memory footprint of the cache compared to other collections.
- private ModelValidator[] _validators;
+ private IEnumerable<ModelValidator> _validators;
public ModelValidationNode(ModelMetadata modelMetadata, string modelStateKey)
: this(modelMetadata, modelStateKey, null)
@@ -169,7 +166,7 @@ private void ValidateProperties(HttpActionContext actionContext)
if (modelState.IsValidField(propertyKeyRoot))
{
- foreach (ModelValidator propertyValidator in propertyMetadata.GetValidators(actionContext.GetValidatorProviders()))
+ foreach (ModelValidator propertyValidator in actionContext.GetValidators(propertyMetadata))
{
foreach (ModelValidationResult propertyResult in propertyValidator.Validate(propertyMetadata, model))
{
@@ -199,10 +196,7 @@ private void ValidateThis(HttpActionContext actionContext, ModelValidationNode p
return;
}
- if (_validators == null)
- {
- Interlocked.Exchange(ref _validators, ModelMetadata.GetValidators(actionContext.GetValidatorProviders()).ToArray());
- }
+ _validators = actionContext.GetValidators(ModelMetadata);
object container = TryConvertContainerToMetadataType(parentNode);
foreach (ModelValidator validator in _validators)
View
75 src/System.Web.Http/Validation/ModelValidatorCache.cs
@@ -0,0 +1,75 @@
+// Copyright (c) Microsoft Corporation. All rights reserved. See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading;
+using System.Web.Http.Metadata;
+
+namespace System.Web.Http.Validation
+{
+ /// <summary>
+ /// Defines a cache for <see cref="ModelValidator"/>s. This cache is keyed on the type or property that the metadata is associated with.
+ /// </summary>
+ internal class ModelValidatorCache : IDisposable
+ {
+ private ReaderWriterLockSlim _cacheLock = new ReaderWriterLockSlim();
+ private Dictionary<Tuple<Type, string>, ModelValidator[]> _validatorCache = new Dictionary<Tuple<Type, string>, ModelValidator[]>();
+ private Lazy<IEnumerable<ModelValidatorProvider>> _validatorProviders;
+
+ public ModelValidatorCache(Lazy<IEnumerable<ModelValidatorProvider>> validatorProviders)
+ {
+ _validatorProviders = validatorProviders;
+ }
+
+ public IEnumerable<ModelValidator> GetValidators(ModelMetadata metadata)
+ {
+ // If metadata is for a property then containerType != null && propertyName != null
+ // If metadata is for a type then containerType == null && propertyName == null, so we have to use modelType for the cache key.
+ Type typeForCache = metadata.ContainerType ?? metadata.ModelType;
+ Tuple<Type, string> cacheKey = Tuple.Create(typeForCache, metadata.PropertyName);
+
+ ModelValidator[] validators;
+ if (!TryGetValue(cacheKey, out validators))
+ {
+ _cacheLock.EnterWriteLock();
+ try
+ {
+ // Check the cache again in case the value was computed and added to the cache while we were waiting on the write lock
+ if (!_validatorCache.TryGetValue(cacheKey, out validators))
+ {
+ // Compute validators
+ validators = metadata.GetValidators(_validatorProviders.Value).ToArray();
+ _validatorCache.Add(cacheKey, validators);
+ }
+ }
+ finally
+ {
+ _cacheLock.ExitWriteLock();
+ }
+ }
+ return validators;
+ }
+
+ internal bool TryGetValue(Tuple<Type, string> cacheKey, out ModelValidator[] validators)
+ {
+ if (_cacheLock.TryEnterReadLock(0))
+ {
+ try
+ {
+ return _validatorCache.TryGetValue(cacheKey, out validators);
+ }
+ finally
+ {
+ _cacheLock.ExitReadLock();
+ }
+ }
+ validators = null;
+ return false;
+ }
+
+ void IDisposable.Dispose()
+ {
+ _cacheLock.Dispose();
+ }
+ }
+}
View
1  test/System.Web.Http.Test/Services/DefaultServicesTests.cs
@@ -59,6 +59,7 @@ public void Constructor_DefaultServicesInContainer()
Assert.IsType<DefaultHttpControllerTypeResolver>(defaultServices.GetService(typeof(IHttpControllerTypeResolver)));
Assert.IsType<TraceManager>(defaultServices.GetService(typeof(ITraceManager)));
Assert.IsType<DataAnnotationsModelMetadataProvider>(defaultServices.GetService(typeof(ModelMetadataProvider)));
+ Assert.IsType<ModelValidatorCache>(defaultServices.GetService(typeof(ModelValidatorCache)));
object[] filterProviders = defaultServices.GetServices(typeof(IFilterProvider)).ToArray();
Assert.Equal(2, filterProviders.Length);
Please sign in to comment.
Something went wrong with that request. Please try again.