Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ConcurrencyException with Sitemaps when saving two content items at once (Lombiq Technologies: GOV-33) #15777

Merged
merged 10 commits into from
Apr 17, 2024
Original file line number Diff line number Diff line change
@@ -1,36 +1,49 @@
using System;
using System.Threading.Tasks;
using OrchardCore.ContentManagement.Handlers;
using OrchardCore.Locking.Distributed;
using OrchardCore.Sitemaps.Handlers;

namespace OrchardCore.Contents.Sitemaps
{
public class ContentTypesSitemapUpdateHandler : ContentHandlerBase
{
private readonly IDistributedLock _distributedLock;
private readonly ISitemapUpdateHandler _sitemapUpdateHandler;

public ContentTypesSitemapUpdateHandler(ISitemapUpdateHandler sitemapUpdateHandler)
public ContentTypesSitemapUpdateHandler(IDistributedLock distributedLock, ISitemapUpdateHandler sitemapUpdateHandler)
{
_distributedLock = distributedLock;
_sitemapUpdateHandler = sitemapUpdateHandler;
}

public override Task PublishedAsync(PublishContentContext context)
{
var updateContext = new SitemapUpdateContext
{
UpdateObject = context.ContentItem,
};
public override Task PublishedAsync(PublishContentContext context) => UpdateSitemapDeferredAsync(context);

return _sitemapUpdateHandler.UpdateSitemapAsync(updateContext);
}
public override Task UnpublishedAsync(PublishContentContext context) => UpdateSitemapDeferredAsync(context);

public override Task UnpublishedAsync(PublishContentContext context)
// 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.
private async Task UpdateSitemapDeferredAsync(ContentContextBase context)
{
var updateContext = new SitemapUpdateContext
var timeout = TimeSpan.FromMilliseconds(20_000);
(var locker, var locked) = await _distributedLock.TryAcquireLockAsync("SITEMAPS_UPDATE_LOCK", timeout, timeout);
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved

if (!locked)
{
throw new TimeoutException($"Couldn't acquire a lock to update the sitemap within {timeout.Seconds} seconds.");
}
else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can remove the else here.

{
UpdateObject = context.ContentItem,
};
using (locker)
{
var updateContext = new SitemapUpdateContext
{
UpdateObject = context.ContentItem,
};

return _sitemapUpdateHandler.UpdateSitemapAsync(updateContext);
await _sitemapUpdateHandler.UpdateSitemapAsync(updateContext);
}
}
}
}
}