Cache walk of the culture tree when building resource name list:

- #15
2015-05-20 14:41:36 -07:00 · 2015-05-20 14:41:36 -07:00 · 9384848cc7
--- a/Localization.sln
+++ b/Localization.sln
@ -24,6 +24,10 @@ Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "CultureInfoGenerator", "src
 EndProject
 Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.Framework.Globalization.CultureInfoCache", "src\Microsoft.Framework.Globalization.CultureInfoCache\Microsoft.Framework.Globalization.CultureInfoCache.xproj", "{F3988D3A-A4C8-4FD7-BAFE-13E0D0A1659A}"
 EndProject
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{B723DB83-A670-4BCB-95FB-195361331AD2}"
+EndProject
+Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.Framework.Localization.Test", "test\Microsoft.Framework.Localization.Test\Microsoft.Framework.Localization.Test.xproj", "{287AD58D-DF34-4F16-8616-FD78FA1CADF9}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@ -54,6 +58,10 @@ Global
 		{F3988D3A-A4C8-4FD7-BAFE-13E0D0A1659A}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{F3988D3A-A4C8-4FD7-BAFE-13E0D0A1659A}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{F3988D3A-A4C8-4FD7-BAFE-13E0D0A1659A}.Release|Any CPU.Build.0 = Release|Any CPU
+		{287AD58D-DF34-4F16-8616-FD78FA1CADF9}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{287AD58D-DF34-4F16-8616-FD78FA1CADF9}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{287AD58D-DF34-4F16-8616-FD78FA1CADF9}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{287AD58D-DF34-4F16-8616-FD78FA1CADF9}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@ -65,5 +73,6 @@ Global
 		{55D9501F-15B9-4339-A0AB-6082850E5FCE} = {79878809-8D1C-4BD4-BA99-F1F13FF96FD8}
 		{BD22AE1C-6631-4DA6-874D-0DC0F803CEAB} = {FB313677-BAB3-4E49-8CDB-4FA4A9564767}
 		{F3988D3A-A4C8-4FD7-BAFE-13E0D0A1659A} = {FB313677-BAB3-4E49-8CDB-4FA4A9564767}
+		{287AD58D-DF34-4F16-8616-FD78FA1CADF9} = {B723DB83-A670-4BCB-95FB-195361331AD2}
 	EndGlobalSection
 EndGlobal
--- a/NuGet.Config
+++ b/NuGet.Config
@ -2,6 +2,7 @@
 <configuration>
  <packageSources>
    <add key="AspNetVNext" value="https://www.myget.org/F/aspnetvnext/api/v2" />
+    <add key="xunit" value="https://www.myget.org/F/xunit/api/v2" />
    <add key="NuGet" value="https://nuget.org/api/v2/" />
  </packageSources>
 </configuration>
--- a/src/Microsoft.Framework.Localization/Properties/AssemblyInfo.cs
+++ b/src/Microsoft.Framework.Localization/Properties/AssemblyInfo.cs
@ -0,0 +1,6 @@
+// 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.Runtime.CompilerServices;
+
+[assembly: InternalsVisibleTo("Microsoft.Framework.Localization.Test")]
--- a/src/Microsoft.Framework.Localization/ResourceManagerStringLocalizer.cs
+++ b/src/Microsoft.Framework.Localization/ResourceManagerStringLocalizer.cs
@ -18,9 +18,12 @@ namespace Microsoft.Framework.Localization
    /// </summary>
    public class ResourceManagerStringLocalizer : IStringLocalizer
    {
-        private readonly ConcurrentDictionary<MissingManifestCacheKey, object> _missingManifestCache =
-            new ConcurrentDictionary<MissingManifestCacheKey, object>();
+        private readonly ConcurrentDictionary<string, object> _missingManifestCache =
+            new ConcurrentDictionary<string, object>();

+        private static readonly ConcurrentDictionary<string, IList<string>> _resourceNamesCache =
+            new ConcurrentDictionary<string, IList<string>>();
+        
        /// <summary>
        /// Creates a new <see cref="ResourceManagerStringLocalizer"/>.
        /// </summary>
@ -96,9 +99,10 @@ namespace Microsoft.Framework.Localization
        /// <param name="name">The name of the string resource.</param>
        /// <param name="culture">The <see cref="CultureInfo"/> to get the string for.</param>
        /// <returns>The resource string, or <c>null</c> if none was found.</returns>
-        protected string GetStringSafely([NotNull] string name, [NotNull] CultureInfo culture)
+        protected string GetStringSafely([NotNull] string name, CultureInfo culture)
        {
-            var cacheKey = new MissingManifestCacheKey(name, culture ?? CultureInfo.CurrentUICulture);
+            var cacheKey = $"name={name}&culture={(culture ?? CultureInfo.CurrentUICulture).Name}";
+
            if (_missingManifestCache.ContainsKey(cacheKey))
            {
                return null;
@ -136,7 +140,6 @@ namespace Microsoft.Framework.Localization
        /// <returns>The <see cref="IEnumerator{LocalizedString}"/>.</returns>
        protected IEnumerator<LocalizedString> GetEnumerator([NotNull] CultureInfo culture)
        {
-            // TODO: I'm sure something here should be cached, probably the whole result
            var resourceNames = GetResourceNamesFromCultureHierarchy(culture);

            foreach (var name in resourceNames)
@ -146,6 +149,12 @@ namespace Microsoft.Framework.Localization
            }
        }

+        // Internal to allow testing
+        internal static void ClearResourceNamesCache()
+        {
+            _resourceNamesCache.Clear();
+        }
+
        private IEnumerable<string> GetResourceNamesFromCultureHierarchy(CultureInfo startingCulture)
        {
            var currentCulture = startingCulture;
@ -155,20 +164,10 @@ namespace Microsoft.Framework.Localization
            {
                try
                {
-                    var resourceStreamName = ResourceBaseName;
-                    if (!string.IsNullOrEmpty(currentCulture.Name))
+                    var cultureResourceNames = GetResourceNamesForCulture(currentCulture);
+                    foreach (var resourceName in cultureResourceNames)
                    {
-                        resourceStreamName += "." + currentCulture.Name;
-                    }
-                    resourceStreamName += ".resources";
-                    using (var cultureResourceStream = ResourceAssembly.GetManifestResourceStream(resourceStreamName))
-                    using (var resources = new ResourceReader(cultureResourceStream))
-                    {
-                        foreach (DictionaryEntry entry in resources)
-                        {
-                            var resourceName = (string)entry.Key;
-                            resourceNames.Add(resourceName);
-                        }
+                        resourceNames.Add(resourceName);
                    }
                }
                catch (MissingManifestResourceException) { }
@ -185,43 +184,34 @@ namespace Microsoft.Framework.Localization
            return resourceNames;
        }

-        private class MissingManifestCacheKey : IEquatable<MissingManifestCacheKey>
+        private IList<string> GetResourceNamesForCulture(CultureInfo culture)
        {
-            private readonly int _hashCode;
-
-            public MissingManifestCacheKey(string name, CultureInfo culture)
+            var resourceStreamName = ResourceBaseName;
+            if (!string.IsNullOrEmpty(culture.Name))
            {
-                Name = name;
-                CultureInfo = culture;
-                _hashCode = new { Name, CultureInfo }.GetHashCode();
+                resourceStreamName += "." + culture.Name;
            }
+            resourceStreamName += ".resources";

-            public string Name { get; }
+            var cacheKey = $"assembly={ResourceAssembly.FullName};resourceStreamName={resourceStreamName}";

-            public CultureInfo CultureInfo { get; }
-
-            public bool Equals(MissingManifestCacheKey other)
+            var cultureResourceNames = _resourceNamesCache.GetOrAdd(cacheKey, key =>
            {
-                return string.Equals(Name, other.Name, StringComparison.Ordinal)
-                    && CultureInfo == other.CultureInfo;
-            }
-
-            public override bool Equals(object obj)
-            {
-                var other = obj as MissingManifestCacheKey;
-
-                if (other != null)
+                var names = new List<string>();
+                using (var cultureResourceStream = ResourceAssembly.GetManifestResourceStream(key))
+                using (var resources = new ResourceReader(cultureResourceStream))
                {
-                    return Equals(other);
+                    foreach (DictionaryEntry entry in resources)
+                    {
+                        var resourceName = (string)entry.Key;
+                        names.Add(resourceName);
+                    }
                }

-                return base.Equals(obj);
-            }
+                return names;
+            });

-            public override int GetHashCode()
-            {
-                return _hashCode;
-            }
+            return cultureResourceNames;
        }
    }
 }
--- a/test/Microsoft.Framework.Localization.Test/Microsoft.Framework.Localization.Test.xproj
+++ b/test/Microsoft.Framework.Localization.Test/Microsoft.Framework.Localization.Test.xproj
@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <PropertyGroup>
+    <VisualStudioVersion Condition="'$(VisualStudioVersion)' == ''">14.0</VisualStudioVersion>
+    <VSToolsPath Condition="'$(VSToolsPath)' == ''">$(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion)</VSToolsPath>
+  </PropertyGroup>
+  <Import Project="$(VSToolsPath)\DNX\Microsoft.DNX.Props" Condition="'$(VSToolsPath)' != ''" />
+  <PropertyGroup Label="Globals">
+    <ProjectGuid>287ad58d-df34-4f16-8616-fd78fa1cadf9</ProjectGuid>
+    <RootNamespace>Microsoft.Framework.Localization.Test</RootNamespace>
+    <BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)'=='' ">..\..\artifacts\obj\$(MSBuildProjectName)</BaseIntermediateOutputPath>
+    <OutputPath Condition="'$(OutputPath)'=='' ">..\..\artifacts\bin\$(MSBuildProjectName)\</OutputPath>
+  </PropertyGroup>
+  <PropertyGroup>
+    <SchemaVersion>2.0</SchemaVersion>
+  </PropertyGroup>
+  <ItemGroup>
+    <Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />
+  </ItemGroup>
+  <Import Project="$(VSToolsPath)\DNX\Microsoft.DNX.targets" Condition="'$(VSToolsPath)' != ''" />
+</Project>
--- a/test/Microsoft.Framework.Localization.Test/ResourceManagerStringLocalizerTest.cs
+++ b/test/Microsoft.Framework.Localization.Test/ResourceManagerStringLocalizerTest.cs
@ -0,0 +1,139 @@
+// 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.Globalization;
+using System.IO;
+using System.Linq;
+using System.Reflection;
+using System.Resources;
+using Moq;
+using Xunit;
+
+namespace Microsoft.Framework.Localization.Test
+{
+    public class ResourceManagerStringLocalizerTest
+    {
+        [Fact]
+        public void EnumeratorCachesCultureWalkForSameAssembly()
+        {
+            // Arrange
+            ResourceManagerStringLocalizer.ClearResourceNamesCache();
+            var resourceManager = new Mock<ResourceManager>();
+            var resourceAssembly = new Mock<TestAssembly1>();
+            resourceAssembly.Setup(rm => rm.GetManifestResourceStream(It.IsAny<string>()))
+                .Returns(() => MakeResourceStream());
+            var baseName = "test";
+            var localizer1 = new ResourceManagerStringLocalizer(
+                resourceManager.Object,
+                resourceAssembly.Object,
+                baseName);
+            var localizer2 = new ResourceManagerStringLocalizer(
+                resourceManager.Object,
+                resourceAssembly.Object,
+                baseName);
+
+            // Act
+            for (int i = 0; i < 5; i++)
+            {
+                localizer1.ToList();
+                localizer2.ToList();
+            }
+
+            // Assert
+            var expectedCallCount = GetCultureInfoDepth(CultureInfo.CurrentUICulture);
+            resourceAssembly.Verify(
+                rm => rm.GetManifestResourceStream(It.IsAny<string>()),
+                Times.Exactly(expectedCallCount));
+        }
+
+        [Fact]
+        public void EnumeratorCacheIsScopedByAssembly()
+        {
+            // Arrange
+            ResourceManagerStringLocalizer.ClearResourceNamesCache();
+            var resourceManager = new Mock<ResourceManager>();
+            var resourceAssembly1 = new Mock<TestAssembly1>();
+            resourceAssembly1.CallBase = true;
+            var resourceAssembly2 = new Mock<TestAssembly2>();
+            resourceAssembly2.CallBase = true;
+            resourceAssembly1.Setup(rm => rm.GetManifestResourceStream(It.IsAny<string>()))
+                .Returns(() => MakeResourceStream());
+            resourceAssembly2.Setup(rm => rm.GetManifestResourceStream(It.IsAny<string>()))
+                .Returns(() => MakeResourceStream());
+            var baseName = "test";
+            var localizer1 = new ResourceManagerStringLocalizer(
+                resourceManager.Object,
+                resourceAssembly1.Object,
+                baseName);
+            var localizer2 = new ResourceManagerStringLocalizer(
+                resourceManager.Object,
+                resourceAssembly2.Object,
+                baseName);
+
+            // Act
+            localizer1.ToList();
+            localizer2.ToList();
+
+            // Assert
+            var expectedCallCount = GetCultureInfoDepth(CultureInfo.CurrentUICulture);
+            resourceAssembly1.Verify(
+                rm => rm.GetManifestResourceStream(It.IsAny<string>()),
+                Times.Exactly(expectedCallCount));
+            resourceAssembly2.Verify(
+                rm => rm.GetManifestResourceStream(It.IsAny<string>()),
+                Times.Exactly(expectedCallCount));
+        }
+
+        private static Stream MakeResourceStream()
+        {
+            var stream = new MemoryStream();
+            var resourceWriter = new ResourceWriter(stream);            
+            resourceWriter.AddResource("TestName", "value");
+            resourceWriter.Generate();
+            stream.Position = 0;
+            return stream;
+        }
+
+        private static int GetCultureInfoDepth(CultureInfo culture)
+        {
+            var result = 0;
+            var currentCulture = culture;
+
+            while (true)
+            {
+                result++;
+
+                if (currentCulture == currentCulture.Parent)
+                {
+                    break;
+                }
+
+                currentCulture = currentCulture.Parent;
+            }
+
+            return result;
+        }
+
+        public class TestAssembly1 : Assembly
+        {
+            public override string FullName
+            {
+                get
+                {
+                    return nameof(TestAssembly1);
+                }
+            }
+        }
+
+        public class TestAssembly2 : Assembly
+        {
+            public override string FullName
+            {
+                get
+                {
+                    return nameof(TestAssembly2);
+                }
+            }
+        }
+    }
+}
--- a/test/Microsoft.Framework.Localization.Test/project.json
+++ b/test/Microsoft.Framework.Localization.Test/project.json
@ -0,0 +1,16 @@
+{
+    "dependencies": {
+        "Moq": "4.2.1502.911",
+        "xunit": "2.1.0-*",
+        "xunit.runner.dnx": "2.1.0-*",
+        "Microsoft.Framework.Localization": "1.0.0-*"
+    },
+
+    "commands": {
+        "test": "xunit.runner.dnx"
+    },
+
+    "frameworks": {
+        "dnx451": { }
+    }
+}