diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ActionConstraints/ActionConstraintItem.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ActionConstraints/ActionConstraintItem.cs index 3a9ed36166..e0734da78c 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/ActionConstraints/ActionConstraintItem.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/ActionConstraints/ActionConstraintItem.cs @@ -34,5 +34,10 @@ namespace Microsoft.AspNet.Mvc.ActionConstraints /// The instance. /// public IActionConstraintMetadata Metadata { get; private set; } + + /// + /// Gets or sets a value indicating whether or not can be reused across requests. + /// + public bool IsReusable { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ActionConstraints/IActionConstraintFactory.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ActionConstraints/IActionConstraintFactory.cs index 5f09a7fb98..94dacdca1e 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/ActionConstraints/IActionConstraintFactory.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/ActionConstraints/IActionConstraintFactory.cs @@ -17,6 +17,12 @@ namespace Microsoft.AspNet.Mvc.ActionConstraints /// public interface IActionConstraintFactory : IActionConstraintMetadata { + /// + /// Gets a value that indicates if the result of + /// can be reused across requests. + /// + bool IsReusable { get; } + /// /// Creates a new . /// diff --git a/src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index 85a2295958..348287f102 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -98,6 +98,7 @@ namespace Microsoft.Extensions.DependencyInjection // Action Selection // services.TryAddSingleton(); + services.TryAddSingleton(); // Performs caching services.TryAddSingleton(); diff --git a/src/Microsoft.AspNet.Mvc.Core/Internal/ActionConstraintCache.cs b/src/Microsoft.AspNet.Mvc.Core/Internal/ActionConstraintCache.cs new file mode 100644 index 0000000000..53b190abd3 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Internal/ActionConstraintCache.cs @@ -0,0 +1,200 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using Microsoft.AspNet.Http; +using Microsoft.AspNet.Mvc.Abstractions; +using Microsoft.AspNet.Mvc.ActionConstraints; +using Microsoft.AspNet.Mvc.Infrastructure; + +namespace Microsoft.AspNet.Mvc.Internal +{ + public class ActionConstraintCache + { + private readonly IActionDescriptorCollectionProvider _collectionProvider; + private readonly IActionConstraintProvider[] _actionConstraintProviders; + + private volatile InnerCache _currentCache; + + public ActionConstraintCache( + IActionDescriptorCollectionProvider collectionProvider, + IEnumerable actionConstraintProviders) + { + _collectionProvider = collectionProvider; + _actionConstraintProviders = actionConstraintProviders.OrderBy(item => item.Order).ToArray(); + } + + private InnerCache CurrentCache + { + get + { + var current = _currentCache; + var actionDescriptors = _collectionProvider.ActionDescriptors; + + if (current == null || current.Version != actionDescriptors.Version) + { + current = new InnerCache(actionDescriptors.Version); + _currentCache = current; + } + + return current; + } + } + + public IReadOnlyList GetActionConstraints(HttpContext httpContext, ActionDescriptor action) + { + var cache = CurrentCache; + + CacheEntry entry; + if (cache.Entries.TryGetValue(action, out entry)) + { + return GetActionConstraintsFromEntry(entry, httpContext, action); + } + + if (action.ActionConstraints == null || action.ActionConstraints.Count == 0) + { + return null; + } + + var items = new List(action.ActionConstraints.Count); + for (var i = 0; i < action.ActionConstraints.Count; i++) + { + items.Add(new ActionConstraintItem(action.ActionConstraints[i])); + } + + ExecuteProviders(httpContext, action, items); + + var actionConstraints = ExtractActionConstraints(items); + + var allActionConstraintsCached = true; + for (var i = 0; i < items.Count; i++) + { + var item = items[i]; + if (!item.IsReusable) + { + item.Constraint = null; + allActionConstraintsCached = false; + } + } + + if (allActionConstraintsCached) + { + entry = new CacheEntry(actionConstraints); + } + else + { + entry = new CacheEntry(items); + } + + cache.Entries.TryAdd(action, entry); + return actionConstraints; + } + + private IReadOnlyList GetActionConstraintsFromEntry(CacheEntry entry, HttpContext httpContext, ActionDescriptor action) + { + Debug.Assert(entry.ActionConstraints != null || entry.Items != null); + + if (entry.ActionConstraints != null) + { + return entry.ActionConstraints; + } + + var items = new List(entry.Items.Count); + for (var i = 0; i < entry.Items.Count; i++) + { + var item = entry.Items[i]; + if (item.IsReusable) + { + items.Add(item); + } + else + { + items.Add(new ActionConstraintItem(item.Metadata)); + } + } + + ExecuteProviders(httpContext, action, items); + + return ExtractActionConstraints(items); + } + + private void ExecuteProviders(HttpContext httpContext, ActionDescriptor action, List items) + { + var context = new ActionConstraintProviderContext(httpContext, action, items); + + for (var i = 0; i < _actionConstraintProviders.Length; i++) + { + _actionConstraintProviders[i].OnProvidersExecuting(context); + } + + for (var i = _actionConstraintProviders.Length - 1; i >= 0; i--) + { + _actionConstraintProviders[i].OnProvidersExecuted(context); + } + } + + private IReadOnlyList ExtractActionConstraints(List items) + { + var count = 0; + for (var i = 0; i < items.Count; i++) + { + if (items[i].Constraint != null) + { + count++; + } + } + + if (count == 0) + { + return null; + } + + var actionConstraints = new IActionConstraint[count]; + for (int i = 0, j = 0; i < items.Count; i++) + { + var actionConstraint = items[i].Constraint; + if (actionConstraint != null) + { + actionConstraints[j++] = actionConstraint; + } + } + + return actionConstraints; + } + + private class InnerCache + { + public InnerCache(int version) + { + Version = version; + } + + public ConcurrentDictionary Entries { get; } = + new ConcurrentDictionary(); + + public int Version { get; } + } + + private struct CacheEntry + { + public CacheEntry(IReadOnlyList actionConstraints) + { + ActionConstraints = actionConstraints; + Items = null; + } + + public CacheEntry(List items) + { + Items = items; + ActionConstraints = null; + } + + public IReadOnlyList ActionConstraints { get; } + + public List Items { get; } + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/Internal/ActionSelector.cs b/src/Microsoft.AspNet.Mvc.Core/Internal/ActionSelector.cs index 5ed74b345e..e0cc8c8bb3 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Internal/ActionSelector.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Internal/ActionSelector.cs @@ -4,13 +4,11 @@ using System; using System.Collections.Generic; using System.Linq; -using Microsoft.AspNet.Http; using Microsoft.AspNet.Mvc.Abstractions; using Microsoft.AspNet.Mvc.ActionConstraints; using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Mvc.Infrastructure; using Microsoft.AspNet.Mvc.Logging; -using Microsoft.AspNet.Mvc.Routing; using Microsoft.AspNet.Routing; using Microsoft.Extensions.Logging; @@ -22,7 +20,7 @@ namespace Microsoft.AspNet.Mvc.Internal public class ActionSelector : IActionSelector { private readonly IActionSelectorDecisionTreeProvider _decisionTreeProvider; - private readonly IActionConstraintProvider[] _actionConstraintProviders; + private readonly ActionConstraintCache _actionConstraintCache; private ILogger _logger; /// @@ -33,12 +31,12 @@ namespace Microsoft.AspNet.Mvc.Internal /// The . public ActionSelector( IActionSelectorDecisionTreeProvider decisionTreeProvider, - IEnumerable actionConstraintProviders, + ActionConstraintCache actionConstraintCache, ILoggerFactory loggerFactory) { _decisionTreeProvider = decisionTreeProvider; - _actionConstraintProviders = actionConstraintProviders.OrderBy(item => item.Order).ToArray(); _logger = loggerFactory.CreateLogger(); + _actionConstraintCache = actionConstraintCache; } /// @@ -58,7 +56,7 @@ namespace Microsoft.AspNet.Mvc.Internal for (var i = 0; i < matchingRouteConstraints.Count; i++) { var action = matchingRouteConstraints[i]; - var constraints = GetConstraints(context.HttpContext, action); + var constraints = _actionConstraintCache.GetActionConstraints(context.HttpContext, action); candidates.Add(new ActionSelectorCandidate(action, constraints)); } @@ -217,56 +215,5 @@ namespace Microsoft.AspNet.Mvc.Internal return EvaluateActionConstraints(context, actionsWithoutConstraint, order); } } - - private IReadOnlyList GetConstraints(HttpContext httpContext, ActionDescriptor action) - { - if (action.ActionConstraints == null || action.ActionConstraints.Count == 0) - { - return null; - } - - var items = new List(action.ActionConstraints.Count); - for (var i = 0; i < action.ActionConstraints.Count; i++) - { - items.Add(new ActionConstraintItem(action.ActionConstraints[i])); - } - - var context = new ActionConstraintProviderContext(httpContext, action, items); - for (var i = 0; i < _actionConstraintProviders.Length; i++) - { - _actionConstraintProviders[i].OnProvidersExecuting(context); - } - - for (var i = _actionConstraintProviders.Length - 1; i >= 0; i--) - { - _actionConstraintProviders[i].OnProvidersExecuted(context); - } - - var count = 0; - for (var i = 0; i < context.Results.Count; i++) - { - if (context.Results[i].Constraint != null) - { - count++; - } - } - - if (count == 0) - { - return null; - } - - var results = new IActionConstraint[count]; - for (int i = 0, j = 0; i < context.Results.Count; i++) - { - var constraint = context.Results[i].Constraint; - if (constraint != null) - { - results[j++] = constraint; - } - } - - return results; - } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Internal/DefaultActionConstraintProvider.cs b/src/Microsoft.AspNet.Mvc.Core/Internal/DefaultActionConstraintProvider.cs index 12dec79de2..fb30732363 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Internal/DefaultActionConstraintProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Internal/DefaultActionConstraintProvider.cs @@ -53,6 +53,7 @@ namespace Microsoft.AspNet.Mvc.Internal if (constraint != null) { item.Constraint = constraint; + item.IsReusable = true; return; } @@ -60,6 +61,7 @@ namespace Microsoft.AspNet.Mvc.Internal if (factory != null) { item.Constraint = factory.CreateInstance(services); + item.IsReusable = factory.IsReusable; return; } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Internal/FilterCache.cs b/src/Microsoft.AspNet.Mvc.Core/Internal/FilterCache.cs index e3076c99af..c2d5afa706 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Internal/FilterCache.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Internal/FilterCache.cs @@ -5,7 +5,6 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.Linq; -using System.Threading; using Microsoft.AspNet.Mvc.Abstractions; using Microsoft.AspNet.Mvc.Filters; using Microsoft.AspNet.Mvc.Infrastructure; @@ -100,8 +99,8 @@ namespace Microsoft.AspNet.Mvc.Internal return entry.Filters; } - var items = new List(entry.Items.Length); - for (var i = 0; i < entry.Items.Length; i++) + var items = new List(entry.Items.Count); + for (var i = 0; i < entry.Items.Count; i++) { var item = entry.Items[i]; if (item.IsReusable) @@ -188,26 +187,13 @@ namespace Microsoft.AspNet.Mvc.Internal public CacheEntry(List items) { - Items = new FilterItem[items.Count]; - for (var i = 0; i < Items.Length; i++) - { - var item = items[i]; - if (item.IsReusable) - { - Items[i] = item; - } - else - { - Items[i] = new FilterItem(item.Descriptor); - } - } - + Items = items; Filters = null; } public IFilterMetadata[] Filters { get; } - public FilterItem[] Items { get; } + public List Items { get; } } } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs index b86c55f4f4..71674f8638 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs @@ -530,7 +530,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure var defaultActionSelector = new ActionSelector( decisionTreeProvider, - actionConstraintProviders, + GetActionConstraintCache(actionConstraintProviders), NullLoggerFactory.Instance); return defaultActionSelector.Select(context); @@ -614,7 +614,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure return new ActionSelector( decisionTreeProvider, - actionConstraintProviders, + GetActionConstraintCache(actionConstraintProviders), loggerFactory); } @@ -680,6 +680,13 @@ namespace Microsoft.AspNet.Mvc.Infrastructure return actionDescriptor; } + private static ActionConstraintCache GetActionConstraintCache(IActionConstraintProvider[] actionConstraintProviders = null) + { + var services = new ServiceCollection().BuildServiceProvider(); + var descriptorProvider = new ActionDescriptorCollectionProvider(services); + return new ActionConstraintCache(descriptorProvider, actionConstraintProviders.AsEnumerable() ?? new List()); + } + private class BooleanConstraint : IActionConstraint { public bool Pass { get; set; } @@ -696,6 +703,8 @@ namespace Microsoft.AspNet.Mvc.Infrastructure { public IActionConstraint Constraint { get; set; } + public bool IsReusable => true; + public IActionConstraint CreateInstance(IServiceProvider services) { return Constraint; diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Internal/ActionConstraintCacheTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Internal/ActionConstraintCacheTest.cs new file mode 100644 index 0000000000..74d90cc047 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Internal/ActionConstraintCacheTest.cs @@ -0,0 +1,165 @@ +// Copyright (c) .NET Foundation. 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.Http.Internal; +using Microsoft.AspNet.Mvc.ActionConstraints; +using Microsoft.AspNet.Mvc.Controllers; +using Microsoft.AspNet.Mvc.Infrastructure; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Microsoft.AspNet.Mvc.Internal +{ + public class ActionConstraintCacheTest + { + [Fact] + public void GetActionConstraints_CachesAllActionConstraints() + { + // Arrange + var services = CreateServices(); + var cache = CreateCache(new DefaultActionConstraintProvider()); + + var action = new ControllerActionDescriptor() + { + ActionConstraints = new[] + { + new TestActionConstraint(), + new TestActionConstraint() + }, + }; + var context = new DefaultHttpContext(); + + // Act - 1 + var actionConstraints1 = cache.GetActionConstraints(context, action); + + // Assert - 1 + Assert.Collection( + actionConstraints1, + a => Assert.Same(action.ActionConstraints[0], a), // Copied by provider + a => Assert.Same(action.ActionConstraints[1], a)); // Copied by provider + + // Act - 2 + var actionConstraints2 = cache.GetActionConstraints(context, action); + + Assert.Same(actionConstraints1, actionConstraints2); + + Assert.Collection( + actionConstraints2, + a => Assert.Same(actionConstraints1[0], a), // Cached + a => Assert.Same(actionConstraints1[1], a)); // Cached + } + + [Fact] + public void GetActionConstraints_CachesActionConstraintFromFactory() + { + // Arrange + var services = CreateServices(); + var cache = CreateCache(new DefaultActionConstraintProvider()); + + var action = new ControllerActionDescriptor() + { + ActionConstraints = new[] + { + new TestActionConstraintFactory() { IsReusable = true }, + new TestActionConstraint() as IActionConstraintMetadata + }, + }; + var context = new DefaultHttpContext(); + + // Act - 1 + var actionConstraints1 = cache.GetActionConstraints(context, action); + + // Assert - 1 + Assert.Collection( + actionConstraints1, + a => Assert.NotSame(action.ActionConstraints[0], a), // Created by factory + a => Assert.Same(action.ActionConstraints[1], a)); // Copied by provider + + // Act - 2 + var actionConstraints2 = cache.GetActionConstraints(context, action); + + Assert.Same(actionConstraints1, actionConstraints2); + + Assert.Collection( + actionConstraints2, + a => Assert.Same(actionConstraints1[0], a), // Cached + a => Assert.Same(actionConstraints1[1], a)); // Cached + } + + [Fact] + public void GetActionConstraints_DoesNotCacheActionConstraintsWithIsReusableFalse() + { + // Arrange + var services = CreateServices(); + var cache = CreateCache(new DefaultActionConstraintProvider()); + + var action = new ControllerActionDescriptor() + { + ActionConstraints = new[] + { + new TestActionConstraintFactory() { IsReusable = false }, + new TestActionConstraint() as IActionConstraintMetadata + }, + }; + var context = new DefaultHttpContext(); + + // Act - 1 + var actionConstraints1 = cache.GetActionConstraints(context, action); + + // Assert - 1 + Assert.Collection( + actionConstraints1, + a => Assert.NotSame(action.ActionConstraints[0], a), // Created by factory + a => Assert.Same(action.ActionConstraints[1], a)); // Copied by provider + + // Act - 2 + var actionConstraints2 = cache.GetActionConstraints(context, action); + + Assert.NotSame(actionConstraints1, actionConstraints2); + + Assert.Collection( + actionConstraints2, + a => Assert.NotSame(actionConstraints1[0], a), // Created by factory (again) + a => Assert.Same(actionConstraints1[1], a)); // Cached + } + + private class TestActionConstraint : IActionConstraint + { + public int Order + { + get + { + throw new NotImplementedException(); + } + } + + public bool Accept(ActionConstraintContext context) + { + throw new NotImplementedException(); + } + } + + private class TestActionConstraintFactory : IActionConstraintFactory + { + public bool IsReusable { get; set; } + + public IActionConstraint CreateInstance(IServiceProvider serviceProvider) + { + return new TestActionConstraint(); + } + } + + private static IServiceProvider CreateServices() + { + return new ServiceCollection().BuildServiceProvider(); + } + + private static ActionConstraintCache CreateCache(params IActionConstraintProvider[] providers) + { + var services = CreateServices(); + var descriptorProvider = new ActionDescriptorCollectionProvider(services); + return new ActionConstraintCache(descriptorProvider, providers); + } + } +} diff --git a/test/WebSites/BasicWebSite/RequestScopedActionConstraint.cs b/test/WebSites/BasicWebSite/RequestScopedActionConstraint.cs index 38d8de08e3..d602be6ad0 100644 --- a/test/WebSites/BasicWebSite/RequestScopedActionConstraint.cs +++ b/test/WebSites/BasicWebSite/RequestScopedActionConstraint.cs @@ -17,6 +17,8 @@ namespace BasicWebSite private readonly ConcurrentDictionary _constraintCache = new ConcurrentDictionary(); + public bool IsReusable => false; + public RequestScopedActionConstraintAttribute(string requestId) { _requestId = requestId; diff --git a/test/WebSites/VersioningWebSite/VersionAttribute.cs b/test/WebSites/VersioningWebSite/VersionAttribute.cs index adc85c7fa6..84744ff3ca 100644 --- a/test/WebSites/VersioningWebSite/VersionAttribute.cs +++ b/test/WebSites/VersioningWebSite/VersionAttribute.cs @@ -30,6 +30,8 @@ namespace VersioningWebSite set { _order = value; } } + public bool IsReusable => true; + public IActionConstraint CreateInstance(IServiceProvider services) { return new VersionRangeValidator(_minVersion, _maxVersion) { Order = _order ?? 0 }; diff --git a/test/WebSites/VersioningWebSite/VersionRoute.cs b/test/WebSites/VersioningWebSite/VersionRoute.cs index 3db6690c5e..d7c431013f 100644 --- a/test/WebSites/VersioningWebSite/VersionRoute.cs +++ b/test/WebSites/VersioningWebSite/VersionRoute.cs @@ -26,6 +26,8 @@ namespace VersioningWebSite // We filter out (5), (5], [5) manually after we do the parsing. private static readonly Regex _versionParser = new Regex(@"^(?[\(\[])?(?\d+(-\d+)?)(?[\)\]])?$"); + public bool IsReusable => true; + public VersionRoute(string template) : base(template) {