Only export content/element items once per save-and-publish - #1018
Merged
Conversation
Umbraco 18.1 raises the saved notification for a save-and-publish as well as the published one (umbraco/Umbraco-CMS#23523). In v18 that change sits in PublishableContentServiceBase, so it applies to Documents and Elements alike. ContentHandler and ElementHandler both listen for the saved *and* the published/unpublished notifications, on the assumption that publishing never raises a save. That assumption no longer holds: a single save-and-publish serialized and wrote the item twice, and a save that un-published a culture wrote it three times. Every notification raised by one Umbraco operation carries the same notification state instance (they are all raised WithState(notificationState)), so ProcessItem now claims each item in that shared state and skips it if an earlier notification in the same operation already exported it. The first notification wins. Umbraco persists the item - including its publish state - before raising any of them, so the export reflects the finished operation whichever notification triggers it. The item is claimed before the export is attempted, so a failed export is not retried, and re-reported, by the next notification. SavedNotification handling for these two handlers is now routed through ProcessItem rather than the inline copy of the export loop in SyncHandlerRoot, which bypassed the de-duplication. Publishing and un-publishing from the tree still raise only the publish notification, and are unaffected. MediaHandler does not derive from PublishableContentHandlerBase and has no publish notifications, so it is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
uSync exported a content or Library (Element) item twice for a single save-and-publish, and three times for a save that un-published a culture. This fixes it so each item is exported once per Umbraco operation.
Why
Umbraco 18.1 raises
ContentSavedNotificationfor a save-and-publish as well asContentPublishedNotification(umbraco/Umbraco-CMS#23523, PR #23528).Worth noting for review: the upstream PR was raised against
release/17.6and patchesContentService.CommitDocumentChangesInternal, so the diff makes it look Document-only. In v18 that method lives inPublishableContentServiceBase, shared by Documents and Elements — so onrelease/18.1the extra notification fires for both, andElementHandlerhad the same problem asContentHandler.ContentHandlerandElementHandlereach listen for the saved and the published/unpublished notifications. The<remarks>on those handlers explained why — "some publication events do not fire the save notification" — and that premise is what broke.How
Every notification raised by one Umbraco operation carries the same notification state instance —
PublishableContentServiceBaseraises them all with.WithState(notificationState), sourced fromsavingNotification.State. SoProcessItemnow claims each item in that shared state (aHashSet<Guid>under a newuSync.EventExportedItemsKey) and skips it if an earlier notification in the same operation already exported it.Three decisions worth a look:
SaveContent(content)runs before the Saved notification is raised, and the publish values were applied before that — the item is already in its final published state, on the same instance both notifications carry.notification.Messages.DictionaryandSyncScopedNotificationPublishercan dispatch on a queued background thread whenBackgroundNotificationsis on.SavedNotificationhandling for these two handlers is now routed throughProcessItemvia an override, rather than the inline copy of the export loop inSyncHandlerRoot— that copy bypassed the de-duplication. It's untouched for every other handler.MediaHandlerderives fromContentHandlerBase, notPublishableContentHandlerBase, and has no publish notifications, so it's unaffected.Behaviour
Testing
New
uSync.Tests/SyncHandlers/ExportDeduplicationTests.cscovers the save-and-publish pair, the save+unpublish pair, publish-with-no-save, per-item claiming across a branch publish, re-claiming in a later operation, and that uSync's own notification state keys survive.Full solution builds clean; all 190 tests pass (6 new).
Not included
Both came out of the same review of the Umbraco 18.1 changes, but are behaviour changes rather than bug fixes, so they're deliberately left out:
ContentSavedNotification.SavedCultures/ContentPublishedNotification.PublishedCultures(#23313) would let uSync export only the affected cultures, and skip no-op saves.PublishableContentBaseSerializerstill does save-then-publish as two operations rather than the new single-scopeIPublishableContentService.SaveAndPublish(#23277).🤖 Generated with Claude Code