diff --git a/src/Microsoft.AspNet.Mvc.Razor/DefaultViewLocationCache.cs b/src/Microsoft.AspNet.Mvc.Razor/DefaultViewLocationCache.cs index dae4a3ff08..05179bff3f 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/DefaultViewLocationCache.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/DefaultViewLocationCache.cs @@ -3,8 +3,7 @@ using System; using System.Collections.Concurrent; -using System.Linq; -using System.Text; +using System.Collections.Generic; using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Mvc.Razor @@ -14,23 +13,22 @@ namespace Microsoft.AspNet.Mvc.Razor /// public class DefaultViewLocationCache : IViewLocationCache { - private const char CacheKeySeparator = ':'; - // A mapping of keys generated from ViewLocationExpanderContext to view locations. - private readonly ConcurrentDictionary _cache; + private readonly ConcurrentDictionary _cache; /// /// Initializes a new instance of . /// public DefaultViewLocationCache() { - _cache = new ConcurrentDictionary(StringComparer.Ordinal); + _cache = new ConcurrentDictionary( + ViewLocationCacheKeyComparer.Instance); } /// public ViewLocationCacheResult Get([NotNull] ViewLocationExpanderContext context) { - var cacheKey = GenerateKey(context); + var cacheKey = GenerateKey(context, copyViewExpanderValues: false); ViewLocationCacheResult result; if (_cache.TryGetValue(cacheKey, out result)) { @@ -45,46 +43,124 @@ namespace Microsoft.AspNet.Mvc.Razor [NotNull] ViewLocationExpanderContext context, [NotNull] ViewLocationCacheResult value) { - var cacheKey = GenerateKey(context); + var cacheKey = GenerateKey(context, copyViewExpanderValues: true); _cache.TryAdd(cacheKey, value); } - internal static string GenerateKey(ViewLocationExpanderContext context) + // Internal for unit testing + internal static ViewLocationCacheKey GenerateKey( + ViewLocationExpanderContext context, + bool copyViewExpanderValues) { - var keyBuilder = new StringBuilder(); - var routeValues = context.ActionContext.RouteData.Values; var controller = RazorViewEngine.GetNormalizedRouteValue( context.ActionContext, RazorViewEngine.ControllerKey); - // format is "{viewName}:{isPartial}:{controllerName}:{areaName}:" - keyBuilder.Append(context.ViewName) - .Append(CacheKeySeparator) - .Append(context.IsPartial ? 1 : 0) - .Append(CacheKeySeparator) - .Append(controller); + var area = RazorViewEngine.GetNormalizedRouteValue( + context.ActionContext, + RazorViewEngine.AreaKey); - var area = RazorViewEngine.GetNormalizedRouteValue(context.ActionContext, RazorViewEngine.AreaKey); - if (!string.IsNullOrEmpty(area)) + + var values = context.Values; + if (values != null && copyViewExpanderValues) { - keyBuilder.Append(CacheKeySeparator) - .Append(area); + // When performing a Get, avoid creating a copy of the values dictionary + values = new Dictionary(values, StringComparer.Ordinal); } - if (context.Values != null) + return new ViewLocationCacheKey( + context.ViewName, + controller, + area, + context.IsPartial, + values); + } + + // Internal for unit testing + internal class ViewLocationCacheKeyComparer : IEqualityComparer + { + public static readonly ViewLocationCacheKeyComparer Instance = new ViewLocationCacheKeyComparer(); + + public bool Equals(ViewLocationCacheKey x, ViewLocationCacheKey y) { - var valuesDictionary = context.Values; - foreach (var item in valuesDictionary.OrderBy(k => k.Key, StringComparer.Ordinal)) + if (x.IsPartial != y.IsPartial || + !string.Equals(x.ViewName, y.ViewName, StringComparison.Ordinal) || + !string.Equals(x.ControllerName, y.ControllerName, StringComparison.Ordinal) || + !string.Equals(x.AreaName, y.AreaName, StringComparison.Ordinal)) { - keyBuilder.Append(CacheKeySeparator) - .Append(item.Key) - .Append(CacheKeySeparator) - .Append(item.Value); + return false; } + + if (ReferenceEquals(x.Values, y.Values)) + { + return true; + } + + if (x.Values == null || y.Values == null || (x.Values.Count != y.Values.Count)) + { + return false; + } + + foreach (var item in x.Values) + { + string yValue; + if (!y.Values.TryGetValue(item.Key, out yValue) || + !string.Equals(item.Value, yValue, StringComparison.Ordinal)) + { + return false; + } + } + + return true; } - var cacheKey = keyBuilder.ToString(); - return cacheKey; + public int GetHashCode(ViewLocationCacheKey key) + { + var hashCodeCombiner = HashCodeCombiner.Start(); + hashCodeCombiner.Add(key.IsPartial ? 1 : 0); + hashCodeCombiner.Add(key.ViewName, StringComparer.Ordinal); + hashCodeCombiner.Add(key.ControllerName, StringComparer.Ordinal); + hashCodeCombiner.Add(key.AreaName, StringComparer.Ordinal); + + if (key.Values != null) + { + foreach (var item in key.Values) + { + hashCodeCombiner.Add(item.Key, StringComparer.Ordinal); + hashCodeCombiner.Add(item.Value, StringComparer.Ordinal); + } + } + + return hashCodeCombiner; + } + } + + // Internal for unit testing + internal struct ViewLocationCacheKey + { + public ViewLocationCacheKey( + string viewName, + string controllerName, + string areaName, + bool isPartial, + IDictionary values) + { + ViewName = viewName; + ControllerName = controllerName; + AreaName = areaName; + IsPartial = isPartial; + Values = values; + } + + public string ViewName { get; } + + public string ControllerName { get; } + + public string AreaName { get; } + + public bool IsPartial { get; } + + public IDictionary Values { get; } } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Razor.Test/DefaultViewLocationCacheTest.cs b/test/Microsoft.AspNet.Mvc.Razor.Test/DefaultViewLocationCacheTest.cs index 1d2797e8e5..83e2416a66 100644 --- a/test/Microsoft.AspNet.Mvc.Razor.Test/DefaultViewLocationCacheTest.cs +++ b/test/Microsoft.AspNet.Mvc.Razor.Test/DefaultViewLocationCacheTest.cs @@ -43,9 +43,12 @@ namespace Microsoft.AspNet.Mvc.Razor } } + private static DefaultViewLocationCache.ViewLocationCacheKeyComparer CacheKeyComparer => + DefaultViewLocationCache.ViewLocationCacheKeyComparer.Instance; + [Theory] [MemberData(nameof(CacheEntryData))] - public void Get_GeneratesCacheKeyIfItemDoesNotExist(ViewLocationExpanderContext context) + public void Get_ReturnsNoneResultIfItemDoesNotExist(ViewLocationExpanderContext context) { // Arrange var cache = new DefaultViewLocationCache(); @@ -85,129 +88,390 @@ namespace Microsoft.AspNet.Mvc.Razor Assert.Equal(value, result); } - public static IEnumerable CacheKeyData + [Theory] + [InlineData("View1", "View2")] + [InlineData("View1", "view1")] + public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfViewNamesAreDifferent( + string viewName1, + string viewName2) { - get - { - yield return new object[] - { - new ViewLocationExpanderContext(GetActionContext(), "test", isPartial: false), - "test:0:mycontroller" - }; + // Arrange + var actionContext = GetActionContext(); + var viewLocationExpanderContext1 = new ViewLocationExpanderContext( + actionContext, + viewName1, + isPartial: true); + var viewLocationExpanderContext2 = new ViewLocationExpanderContext( + actionContext, + viewName2, + isPartial: true); - yield return new object[] - { - new ViewLocationExpanderContext(GetActionContext(), "test", isPartial: true), - "test:1:mycontroller" - }; + // Act + var key1 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext1, + copyViewExpanderValues: false); - var areaActionContext = GetActionContext("controller2", "myarea"); - yield return new object[] - { - new ViewLocationExpanderContext(areaActionContext, "test2", isPartial: false), - "test2:0:controller2:myarea" - }; - yield return new object[] - { - new ViewLocationExpanderContext(areaActionContext, "test2", isPartial: true), - "test2:1:controller2:myarea" - }; + var key2 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext2, + copyViewExpanderValues: false); - var actionContext = GetActionContext("controller3", "area3"); - var values = new Dictionary(StringComparer.Ordinal) - { - { "culture", "fr" }, - { "theme", "sleek" } - }; - var expanderContext = new ViewLocationExpanderContext(actionContext, "test3", isPartial: false) - { - Values = values - }; + var result = CacheKeyComparer.Equals(key1, key2); + var hash1 = CacheKeyComparer.GetHashCode(key1); + var hash2 = CacheKeyComparer.GetHashCode(key2); - yield return new object[] - { - expanderContext, - "test3:0:controller3:area3:culture:fr:theme:sleek" - }; - - expanderContext = new ViewLocationExpanderContext(actionContext, "test3", isPartial: true) - { - Values = values - }; - yield return new object[] - { - expanderContext, - "test3:1:controller3:area3:culture:fr:theme:sleek" - }; - - yield return new object[] - { - new ViewLocationExpanderContext( - GetActionContextWithActionDescriptor( - new Dictionary() - { - {"controller", "MyController" }, - }, - new Dictionary() - { - {"controller", "mycontroller" }, - }, - isAttributeRouted: true), - "test", - isPartial: false), - "test:0:mycontroller" - }; - - yield return new object[] - { - new ViewLocationExpanderContext( - GetActionContextWithActionDescriptor( - new Dictionary() - { - {"controller", "MyController" }, - }, - new Dictionary() - { - {"controller", "mycontroller" }, - }, - isAttributeRouted: true), - "test", - isPartial: false), - "test:0:mycontroller" - }; - - yield return new object[] - { - new ViewLocationExpanderContext( - GetActionContextWithActionDescriptor( - new Dictionary() - { - {"controller", "mycontroller" }, - }, - new Dictionary() - { - }, - isAttributeRouted: true), - "test", - isPartial: false), - "test:0:mycontroller" - }; - } + // Assert + Assert.False(result); + Assert.NotEqual(hash1, hash2); } [Theory] - [MemberData(nameof(CacheKeyData))] - public void CacheKeyIsComputedBasedOnValuesInExpander(ViewLocationExpanderContext context, string expected) + [InlineData(false, true)] + [InlineData(true, false)] + public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfIsPartialAreDifferent( + bool isPartial1, + bool isPartial2) { + // Arrange + var actionContext = GetActionContext(); + var viewLocationExpanderContext1 = new ViewLocationExpanderContext( + actionContext, + "View1", + isPartial1); + var viewLocationExpanderContext2 = new ViewLocationExpanderContext( + actionContext, + "View1", + isPartial2); + // Act - var result = DefaultViewLocationCache.GenerateKey(context); + var key1 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext1, + copyViewExpanderValues: false); + + var key2 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext2, + copyViewExpanderValues: false); + + var result = CacheKeyComparer.Equals(key1, key2); + var hash1 = CacheKeyComparer.GetHashCode(key1); + var hash2 = CacheKeyComparer.GetHashCode(key2); // Assert - Assert.Equal(expected, result); + Assert.False(result); + Assert.NotEqual(hash1, hash2); } - public static ActionContext GetActionContext(string controller = "mycontroller", - string area = null) + [Theory] + [InlineData("Controller1", "Controller2")] + [InlineData("controller1", "Controller1")] + public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfIsControllerNamesAreDifferent( + string controller1, + string controller2) + { + // Arrange + var viewLocationExpanderContext1 = new ViewLocationExpanderContext( + GetActionContext(controller1), + "View1", + isPartial: false); + var viewLocationExpanderContext2 = new ViewLocationExpanderContext( + GetActionContext(controller2), + "View1", + isPartial: false); + + // Act + var key1 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext1, + copyViewExpanderValues: false); + + var key2 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext2, + copyViewExpanderValues: false); + + var result = CacheKeyComparer.Equals(key1, key2); + var hash1 = CacheKeyComparer.GetHashCode(key1); + var hash2 = CacheKeyComparer.GetHashCode(key2); + + // Assert + Assert.False(result); + Assert.NotEqual(hash1, hash2); + } + + [Theory] + [InlineData("area1", null)] + [InlineData("Area1", "Area2")] + [InlineData("area1", "aRea1")] + public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfIsAreaNamesAreDifferent( + string area1, + string area2) + { + // Arrange + var viewLocationExpanderContext1 = new ViewLocationExpanderContext( + GetActionContext("Controller1", area1), + "View1", + isPartial: false); + var viewLocationExpanderContext2 = new ViewLocationExpanderContext( + GetActionContext("Controller1", area2), + "View1", + isPartial: false); + + // Act + var key1 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext1, + copyViewExpanderValues: false); + + var key2 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext2, + copyViewExpanderValues: false); + + var result = CacheKeyComparer.Equals(key1, key2); + var hash1 = CacheKeyComparer.GetHashCode(key1); + var hash2 = CacheKeyComparer.GetHashCode(key2); + + // Assert + Assert.False(result); + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public void ViewLocationCacheKeyComparer_EqualsReturnsTrueIfControllerAreaAndViewNamesAreIdentical() + { + // Arrange + var viewLocationExpanderContext1 = new ViewLocationExpanderContext( + GetActionContext("Controller1", "Area1"), + "View1", + isPartial: false); + var viewLocationExpanderContext2 = new ViewLocationExpanderContext( + GetActionContext("Controller1", "Area1"), + "View1", + isPartial: false); + + // Act + var key1 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext1, + copyViewExpanderValues: false); + + var key2 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext2, + copyViewExpanderValues: false); + + var result = CacheKeyComparer.Equals(key1, key2); + var hash1 = CacheKeyComparer.GetHashCode(key1); + var hash2 = CacheKeyComparer.GetHashCode(key2); + + // Assert + Assert.True(result); + Assert.Equal(hash1, hash2); + } + + [Fact] + public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfViewLocationExpanderIsNullForEitherKey() + { + // Arrange + var viewLocationExpanderContext1 = new ViewLocationExpanderContext( + GetActionContext("Controller1", "Area1"), + "View1", + isPartial: false); + viewLocationExpanderContext1.Values = new Dictionary + { + { "somekey", "somevalue" } + }; + + var viewLocationExpanderContext2 = new ViewLocationExpanderContext( + GetActionContext("Controller1", "Area1"), + "View1", + isPartial: false); + + // Act + var key1 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext1, + copyViewExpanderValues: false); + + var key2 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext2, + copyViewExpanderValues: false); + + var result = CacheKeyComparer.Equals(key1, key2); + var hash1 = CacheKeyComparer.GetHashCode(key1); + var hash2 = CacheKeyComparer.GetHashCode(key2); + + // Assert + Assert.False(result); + Assert.NotEqual(hash1, hash2); + } + + [Fact] + public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfExpanderValueCountIsDifferent() + { + // Arrange + var viewLocationExpanderContext1 = new ViewLocationExpanderContext( + GetActionContext("Controller1", "Area1"), + "View1", + isPartial: false); + viewLocationExpanderContext1.Values = new Dictionary + { + { "somekey", "somevalue" } + }; + + var viewLocationExpanderContext2 = new ViewLocationExpanderContext( + GetActionContext("Controller1", "Area1"), + "View1", + isPartial: false); + viewLocationExpanderContext2.Values = new Dictionary + { + { "somekey", "somevalue" }, + { "somekey2", "somevalue2" }, + }; + + // Act + var key1 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext1, + copyViewExpanderValues: false); + + var key2 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext2, + copyViewExpanderValues: false); + + var result = CacheKeyComparer.Equals(key1, key2); + var hash1 = CacheKeyComparer.GetHashCode(key1); + var hash2 = CacheKeyComparer.GetHashCode(key2); + + // Assert + Assert.False(result); + Assert.NotEqual(hash1, hash2); + } + + [Theory] + [InlineData("key1", "key2")] + [InlineData("Key1", "key1")] + public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfKeysAreDifferent( + string keyName1, + string keyName2) + { + // Arrange + var viewLocationExpanderContext1 = new ViewLocationExpanderContext( + GetActionContext("Controller1", "Area1"), + "View1", + isPartial: false); + viewLocationExpanderContext1.Values = new Dictionary + { + { keyName1, "somevalue" } + }; + + var viewLocationExpanderContext2 = new ViewLocationExpanderContext( + GetActionContext("Controller1", "Area1"), + "View1", + isPartial: false); + viewLocationExpanderContext2.Values = new Dictionary + { + { keyName2, "somevalue" }, + }; + + // Act + var key1 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext1, + copyViewExpanderValues: false); + + var key2 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext2, + copyViewExpanderValues: false); + + var result = CacheKeyComparer.Equals(key1, key2); + var hash1 = CacheKeyComparer.GetHashCode(key1); + var hash2 = CacheKeyComparer.GetHashCode(key2); + + // Assert + Assert.False(result); + Assert.NotEqual(hash1, hash2); + } + + [Theory] + [InlineData("value1", null)] + [InlineData("value1", "value2")] + [InlineData("value1", "Value1")] + public void ViewLocationCacheKeyComparer_EqualsReturnsFalseIfValuesAreDifferent( + string value1, + string value2) + { + // Arrange + var viewLocationExpanderContext1 = new ViewLocationExpanderContext( + GetActionContext("Controller1", "Area1"), + "View1", + isPartial: false); + viewLocationExpanderContext1.Values = new Dictionary + { + { "somekey", value1 } + }; + + var viewLocationExpanderContext2 = new ViewLocationExpanderContext( + GetActionContext("Controller1", "Area1"), + "View1", + isPartial: false); + viewLocationExpanderContext2.Values = new Dictionary + { + { "somekey", value2 }, + }; + + // Act + var key1 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext1, + copyViewExpanderValues: false); + + var key2 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext2, + copyViewExpanderValues: false); + + var result = CacheKeyComparer.Equals(key1, key2); + var hash1 = CacheKeyComparer.GetHashCode(key1); + var hash2 = CacheKeyComparer.GetHashCode(key2); + + // Assert + Assert.False(result); + Assert.NotEqual(hash1, hash2); + } + + public void ViewLocationCacheKeyComparer_EqualsReturnsTrueIfValuesAreSame() + { + // Arrange + var viewLocationExpanderContext1 = new ViewLocationExpanderContext( + GetActionContext("Controller1", "Area1"), + "View1", + isPartial: false); + viewLocationExpanderContext1.Values = new Dictionary + { + { "somekey1", "value1" }, + { "somekey2", "value2" }, + }; + + var viewLocationExpanderContext2 = new ViewLocationExpanderContext( + GetActionContext("Controller1", "Area1"), + "View1", + isPartial: false); + viewLocationExpanderContext2.Values = new Dictionary + { + { "somekey2", "value2" }, + { "somekey1", "value1" }, + }; + + // Act + var key1 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext1, + copyViewExpanderValues: false); + + var key2 = DefaultViewLocationCache.GenerateKey( + viewLocationExpanderContext2, + copyViewExpanderValues: false); + + var result = CacheKeyComparer.Equals(key1, key2); + var hash1 = CacheKeyComparer.GetHashCode(key1); + var hash2 = CacheKeyComparer.GetHashCode(key2); + + // Assert + Assert.True(result); + Assert.Equal(hash1, hash2); + } + + public static ActionContext GetActionContext( + string controller = "mycontroller", + string area = null) { var routeData = new RouteData(); routeData.Values["controller"] = controller;