Merge pull request #104 from RWS/feature/CRQ-33855

CRQ-33855 : Fix threading issue within localization resolver
RWS · May 22, 2023 · 23578bf · 23578bf
2 parents c902a50 + 568ee77
commit 23578bf
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 16 deletions.
diff --git a/Sdl.Web.Common/CacheRegions.cs b/Sdl.Web.Common/CacheRegions.cs
@@ -32,6 +32,7 @@ public static class CacheRegions
         public const string LinkResolving = "LinkResolving";
         public const string BrokerQuery = "BrokerQuery";
         public const string RenderedOutput = "RenderedOutput";
+        public const string LocalizationResolving = "LocalizationResolving";
 
         /// <summary>
         /// Returns true if view model caching is enabled in the web applications configuration.

diff --git a/Sdl.Web.Tridion/Resolvers/Localization/GraphQLLocalizationResolver.cs b/Sdl.Web.Tridion/Resolvers/Localization/GraphQLLocalizationResolver.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Concurrent;
 using Sdl.Tridion.Api.Client.ContentModel;
 using Sdl.Web.Common;
 using Sdl.Web.Common.Configuration;
@@ -9,6 +10,8 @@ namespace Sdl.Web.Tridion
 {
     public class GraphQLLocalizationResolver : LocalizationResolver
     {
+        private static readonly ConcurrentDictionary<string, object> KeyLocks = new ConcurrentDictionary<string, object>();
+
         /// <summary>
         /// Resolves a matching <see cref="ILocalization"/> for a given URL.
         /// </summary>
@@ -19,6 +22,7 @@ public override Localization ResolveLocalization(Uri url)
         {
             using (new Tracer(url))
             {
+                Log.Trace($"Resolving localization for url: '{url}'");
                 string urlLeftPart = url.GetLeftPart(UriPartial.Path);
 
                 // TODO PERF: to optimize caching, we could only take the first part of the URL path (e.g. one or two levels)
@@ -30,25 +34,38 @@ public override Localization ResolveLocalization(Uri url)
                     urlLeftPart = urlLeftPart.Substring(0, espaceIndex);
                 }
 
-                // NOTE: we're not using UrlToLocalizationMapping here, because we may match too eagerly on a base URL when there is a matching mapping with a more specific URL.
-                PublicationMapping mapping = null;
-                try
-                {
-                    mapping = ApiClientFactory.Instance.CreateClient().GetPublicationMapping(ContentNamespace.Sites, urlLeftPart);
-                }
-                catch
-                {
-                    Log.Error($"Failed to get publication mapping for url {urlLeftPart}");
-                }
-
-                if (mapping == null || mapping.Port != url.Port.ToString()) // See CRQ-1195
+                Localization result = GetCachedLocalization(urlLeftPart);
+                if (result != null)
                 {
-                    throw new DxaUnknownLocalizationException($"No matching Localization found for URL '{urlLeftPart}'");
+                    return result;
                 }
 
-                Localization result;
-                lock (KnownLocalizations)
+                lock (KeyLocks.GetOrAdd(urlLeftPart, new object()))
                 {
+                    result = GetCachedLocalization(urlLeftPart);
+                    if (result != null)
+                    {
+                        RemoveLock(urlLeftPart);
+                        return result;
+                    }
+
+                    // NOTE: we're not using UrlToLocalizationMapping here, because we may match too eagerly on a base URL when there is a matching mapping with a more specific URL.
+                    PublicationMapping mapping = null;
+                    try
+                    {
+                        mapping = ApiClientFactory.Instance.CreateClient().GetPublicationMapping(ContentNamespace.Sites, urlLeftPart);
+                    }
+                    catch
+                    {
+                        Log.Error($"Failed to get publication mapping for url {urlLeftPart}");
+                    }
+
+                    if (mapping == null || mapping.Port != url.Port.ToString()) // See CRQ-1195
+                    {
+                        RemoveLock(urlLeftPart);
+                        throw new DxaUnknownLocalizationException($"No matching Localization found for URL '{urlLeftPart}'");
+                    }
+
                     string localizationId = mapping.PublicationId.ToString();
                     if (!KnownLocalizations.TryGetValue(localizationId, out result))
                     {
@@ -65,11 +82,40 @@ public override Localization ResolveLocalization(Uri url)
                         // a partially created localization.
                         result.Path = mapping.Path;
                     }
+
+
+                    result.EnsureInitialized();
+
+                    Log.Trace($"Localization for url '{url}' initialized and reports to be for Publication Id: {result.PublicationId()}, Path: {result.Path}");
+                    CacheLocalization(urlLeftPart, result);                    
+                    RemoveLock(urlLeftPart);
+                    return result;
                 }
+            }
+        }
 
-                result.EnsureInitialized();
+        protected void CacheLocalization(String urlPart, Localization localization)
+        {
+            Log.Trace($"Attempting to cache localization for url part: '{urlPart}'");
+            SiteConfiguration.CacheProvider.Store($"{urlPart}", CacheRegions.LocalizationResolving, localization);
+        }
+
+        protected Localization GetCachedLocalization(String urlPart)
+        {
+            Localization result;
+            if (SiteConfiguration.CacheProvider.TryGet($"{urlPart}", CacheRegions.LocalizationResolving, out result))
+            {
+                Log.Trace($"Found cached localization for url part: '{urlPart}' with Publication Id: {result.PublicationId()}, Path: {result.Path}");
                 return result;
             }
+            Log.Trace($"No cached localization found for url part: '{urlPart}'");
+            return null;
+        }
+
+        protected static void RemoveLock(String urlPart)
+        {
+            object tempKeyLock;
+            KeyLocks.TryRemove(urlPart, out tempKeyLock);
         }
 
         public override Localization GetLocalization(string localizationId)

diff --git a/Site/Web.config b/Site/Web.config
@@ -79,6 +79,7 @@
         <add name="LinkResolving" cacheName="regularCache" />
         <add name="PublicationMapping" cacheName="regularCache" />
         <add name="BrokerQuery" cacheName="regularCache" />
+        <add name="LocalizationResolving" cacheName="regularCache" />
         <!-- View Rendering output -->
         <add name="RenderedOutput" cacheName="longLivedCache" />
       </regions>