Fix ConcurrencyException with Sitemaps when saving two content items …

…at once (#15777)
OrchardCMS · Apr 17, 2024 · 9b7468b · 9b7468b
1 parent e9daf76
commit 9b7468b
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 12 deletions.
diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapUpdateHandler.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapUpdateHandler.cs
@@ -13,17 +13,11 @@ public ContentTypesSitemapUpdateHandler(ISitemapUpdateHandler sitemapUpdateHandl
             _sitemapUpdateHandler = sitemapUpdateHandler;
         }
 
-        public override Task PublishedAsync(PublishContentContext context)
-        {
-            var updateContext = new SitemapUpdateContext
-            {
-                UpdateObject = context.ContentItem,
-            };
+        public override Task PublishedAsync(PublishContentContext context) => UpdateSitemapAsync(context);
 
-            return _sitemapUpdateHandler.UpdateSitemapAsync(updateContext);
-        }
+        public override Task UnpublishedAsync(PublishContentContext context) => UpdateSitemapAsync(context);
 
-        public override Task UnpublishedAsync(PublishContentContext context)
+        private Task UpdateSitemapAsync(ContentContextBase context)
         {
             var updateContext = new SitemapUpdateContext
             {

diff --git a/src/OrchardCore.Modules/OrchardCore.Sitemaps/Handlers/DefaultSitemapUpdateHandler.cs b/src/OrchardCore.Modules/OrchardCore.Sitemaps/Handlers/DefaultSitemapUpdateHandler.cs
@@ -1,22 +1,42 @@
+using System;
 using System.Collections.Generic;
 using System.Threading.Tasks;
+using OrchardCore.Locking.Distributed;
 
 namespace OrchardCore.Sitemaps.Handlers
 {
     public class DefaultSitemapUpdateHandler : ISitemapUpdateHandler
     {
         private readonly IEnumerable<ISitemapTypeUpdateHandler> _sitemapTypeUpdateHandlers;
+        private readonly IDistributedLock _distributedLock;
 
-        public DefaultSitemapUpdateHandler(IEnumerable<ISitemapTypeUpdateHandler> sitemapTypeUpdateHandlers)
+        public DefaultSitemapUpdateHandler(
+            IEnumerable<ISitemapTypeUpdateHandler> sitemapTypeUpdateHandlers,
+            IDistributedLock distributedLock)
         {
             _sitemapTypeUpdateHandlers = sitemapTypeUpdateHandlers;
+            _distributedLock = distributedLock;
         }
 
         public async Task UpdateSitemapAsync(SitemapUpdateContext context)
         {
-            foreach (var sitemapTypeUpdateHandler in _sitemapTypeUpdateHandlers)
+            // Doing the update in a synchronized way makes sure that two simultaneous content item updates don't cause
+            // a ConcurrencyException due to the same sitemap document being updated.
+
+            var timeout = TimeSpan.FromMilliseconds(20_000);
+            (var locker, var locked) = await _distributedLock.TryAcquireLockAsync("SITEMAPS_UPDATE_LOCK", timeout, timeout);
+
+            if (!locked)
+            {
+                throw new TimeoutException($"Couldn't acquire a lock to update the sitemap within {timeout.Seconds} seconds.");
+            }
+
+            using (locker)
             {
-                await sitemapTypeUpdateHandler.UpdateSitemapAsync(context);
+                foreach (var sitemapTypeUpdateHandler in _sitemapTypeUpdateHandlers)
+                {
+                    await sitemapTypeUpdateHandler.UpdateSitemapAsync(context);
+                }
             }
         }
     }