From a627930b581048f6ec3484b3d5aa895597ee8f9a Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Mon, 9 Dec 2024 11:07:32 +0100 Subject: [PATCH 1/2] Warn about un-routable content at publish time (#17705) (cherry picked from commit 2d9cfc880bed5398ccce374295f0f6e93d76949d) --- .../EmbeddedResources/Lang/da.xml | 1 + .../EmbeddedResources/Lang/en.xml | 1 + .../EmbeddedResources/Lang/en_us.xml | 1 + .../Controllers/ContentController.cs | 82 +++++++++++++++++-- .../Controllers/ContentControllerTests.cs | 40 +++++++-- 5 files changed, 110 insertions(+), 15 deletions(-) diff --git a/src/Umbraco.Core/EmbeddedResources/Lang/da.xml b/src/Umbraco.Core/EmbeddedResources/Lang/da.xml index 15cbe5f30a..73ef388cb1 100644 --- a/src/Umbraco.Core/EmbeddedResources/Lang/da.xml +++ b/src/Umbraco.Core/EmbeddedResources/Lang/da.xml @@ -1469,6 +1469,7 @@ Mange hilsner fra Umbraco robotten Der er ikke noget domæne konfigureret for %0%, kontakt vensligst en administrator, se loggen for mere information + Dokumentet har ikke nogen URL, muligvis grundet en kollision med et andet dokuments navn. Flere detaljer kan ses under Info. Dit systems information er blevet kopieret til udklipsholderen Kunne desværre ikke kopiere dit systems information til udklipsholderen Webhook gemt diff --git a/src/Umbraco.Core/EmbeddedResources/Lang/en.xml b/src/Umbraco.Core/EmbeddedResources/Lang/en.xml index 4995967993..2b8a4a4c2f 100644 --- a/src/Umbraco.Core/EmbeddedResources/Lang/en.xml +++ b/src/Umbraco.Core/EmbeddedResources/Lang/en.xml @@ -1700,6 +1700,7 @@ To manage your website, simply open the Umbraco backoffice and start adding cont There is no domain configured for %0%, please contact an administrator, see log for more information + The document does not have a URL, possibly due to a naming collision with another document. More details can be found under Info. Your system information has successfully been copied to the clipboard Could not copy your system information to the clipboard Webhook saved diff --git a/src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml b/src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml index 39b20009f4..472aa58c0f 100644 --- a/src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml +++ b/src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml @@ -1732,6 +1732,7 @@ To manage your website, simply open the Umbraco backoffice and start adding cont There is no domain configured for %0%, please contact an administrator, see log for more information + The document does not have a URL, possibly due to a naming collision with another document. More details can be found under Info. An error occurred while enabling version cleanup for %0% An error occurred while disabling version cleanup for %0% Your system information has successfully been copied to the clipboard diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs index 62cbae3012..c676cee2ca 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs @@ -1134,8 +1134,12 @@ public class ContentController : ContentControllerBase { PublishResult publishStatus = PublishInternal(contentItem, defaultCulture, cultureForInvariantErrors, out wasCancelled, out var successfulCultures); // Add warnings if domains are not set up correctly - AddDomainWarnings(publishStatus.Content, successfulCultures, globalNotifications); + var addedDomainWarnings = AddDomainWarnings(publishStatus.Content, successfulCultures, globalNotifications, defaultCulture); AddPublishStatusNotifications(new[] { publishStatus }, globalNotifications, notifications, successfulCultures); + if (addedDomainWarnings is false) + { + AddPublishRoutableErrorNotifications(new[] { publishStatus }, globalNotifications, successfulCultures); + } } break; case ContentSaveAction.PublishWithDescendants: @@ -1151,8 +1155,12 @@ public class ContentController : ContentControllerBase } var publishStatus = PublishBranchInternal(contentItem, false, cultureForInvariantErrors, out wasCancelled, out var successfulCultures).ToList(); - AddDomainWarnings(publishStatus, successfulCultures, globalNotifications); + var addedDomainWarnings = AddDomainWarnings(publishStatus, successfulCultures, globalNotifications, defaultCulture); AddPublishStatusNotifications(publishStatus, globalNotifications, notifications, successfulCultures); + if (addedDomainWarnings is false) + { + AddPublishRoutableErrorNotifications(publishStatus, globalNotifications, successfulCultures); + } } break; case ContentSaveAction.PublishWithDescendantsForce: @@ -1235,6 +1243,48 @@ public class ContentController : ContentControllerBase } } + private void AddPublishRoutableErrorNotifications( + IReadOnlyCollection publishStatus, + SimpleNotificationModel globalNotifications, + string[]? successfulCultures) + { + IContent? content = publishStatus.FirstOrDefault()?.Content; + if (content is null) + { + return; + } + + if (content.ContentType.VariesByCulture() is false) + { + // successfulCultures will be null here - change it to a wildcard and utilize this below + successfulCultures = ["*"]; + } + + if (successfulCultures?.Any() is not true) + { + return; + } + + ContentItemDisplay? contentItemDisplay = _umbracoMapper.Map(publishStatus.FirstOrDefault()?.Content); + if (contentItemDisplay?.Urls is null) + { + return; + } + + foreach (var culture in successfulCultures) + { + if (contentItemDisplay.Urls.Where(u => u.Culture == culture || culture == "*").All(u => u.IsUrl is false)) + { + globalNotifications.AddWarningNotification( + _localizedTextService.Localize("auditTrails", "publish"), + _localizedTextService.Localize("speechBubbles", "publishWithNoUrl")); + + // only add one warning here, even though there might actually be more + break; + } + } + } + /// /// Validates critical data for persistence and updates the ModelState and result accordingly /// @@ -1770,12 +1820,15 @@ public class ContentController : ContentControllerBase } } - private void AddDomainWarnings(IEnumerable publishResults, string[]? culturesPublished, SimpleNotificationModel globalNotifications) + private bool AddDomainWarnings(IEnumerable publishResults, string[]? culturesPublished, SimpleNotificationModel globalNotifications, string? defaultCulture) { + var addedDomainWarnings = false; foreach (PublishResult publishResult in publishResults) { - AddDomainWarnings(publishResult.Content, culturesPublished, globalNotifications); + addedDomainWarnings &= AddDomainWarnings(publishResult.Content, culturesPublished, globalNotifications, defaultCulture); } + + return addedDomainWarnings; } /// @@ -1788,24 +1841,25 @@ public class ContentController : ContentControllerBase /// /// /// - internal void AddDomainWarnings(IContent? persistedContent, string[]? culturesPublished, SimpleNotificationModel globalNotifications) + /// + internal bool AddDomainWarnings(IContent? persistedContent, string[]? culturesPublished, SimpleNotificationModel globalNotifications, string? defaultCulture) { if (_contentSettings.ShowDomainWarnings is false) { - return; + return false; } // Don't try to verify if no cultures were published if (culturesPublished is null) { - return; + return false; } var publishedCultures = GetPublishedCulturesFromAncestors(persistedContent).ToList(); // If only a single culture is published we shouldn't have any routing issues if (publishedCultures.Count < 2) { - return; + return false; } // If more than a single culture is published we need to verify that there's a domain registered for each published culture @@ -1827,6 +1881,12 @@ public class ContentController : ContentControllerBase // No domains at all, add a warning, to add domains. if (assignedDomains is null || assignedDomains.Count == 0) { + // If only the default culture was published we shouldn't have any routing issues + if (culturesPublished.Length == 1 && culturesPublished[0].InvariantEquals(defaultCulture)) + { + return false; + } + globalNotifications.AddWarningNotification( _localizedTextService.Localize("auditTrails", "publish"), _localizedTextService.Localize("speechBubbles", "publishWithNoDomains")); @@ -1835,14 +1895,16 @@ public class ContentController : ContentControllerBase "The root node {RootNodeName} was published with multiple cultures, but no domains are configured, this will cause routing and caching issues, please register domains for: {Cultures}", persistedContent?.Name, string.Join(", ", publishedCultures)); - return; + return true; } // If there is some domains, verify that there's a domain for each of the published cultures + var addedDomainWarnings = false; foreach (var culture in culturesPublished .Where(culture => assignedDomains.Any(x => x.LanguageIsoCode?.Equals(culture, StringComparison.OrdinalIgnoreCase) ?? false) is false)) { + addedDomainWarnings = true; globalNotifications.AddWarningNotification( _localizedTextService.Localize("auditTrails", "publish"), _localizedTextService.Localize("speechBubbles", "publishWithMissingDomain", new[] { culture })); @@ -1852,6 +1914,8 @@ public class ContentController : ContentControllerBase persistedContent?.Name, culture); } + + return addedDomainWarnings; } /// diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerTests.cs index b7b9b85c94..bd0f9e33d0 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerTests.cs @@ -56,7 +56,7 @@ public class ContentControllerTests var notifications = new SimpleNotificationModel(); var contentController = CreateContentController(domainServiceMock.Object); - contentController.AddDomainWarnings(rootNode, culturesPublished, notifications); + contentController.AddDomainWarnings(rootNode, culturesPublished, notifications, "en-us"); Assert.IsEmpty(notifications.Notifications); } @@ -82,7 +82,7 @@ public class ContentControllerTests var notifications = new SimpleNotificationModel(); var contentController = CreateContentController(domainServiceMock.Object); - contentController.AddDomainWarnings(rootNode, culturesPublished, notifications); + contentController.AddDomainWarnings(rootNode, culturesPublished, notifications, "en-us"); Assert.IsEmpty(notifications.Notifications); } @@ -111,7 +111,7 @@ public class ContentControllerTests var notifications = new SimpleNotificationModel(); var contentController = CreateContentController(domainServiceMock.Object); - contentController.AddDomainWarnings(rootNode, culturesPublished, notifications); + contentController.AddDomainWarnings(rootNode, culturesPublished, notifications, "en-us"); Assert.AreEqual(1, notifications.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning)); } @@ -139,10 +139,38 @@ public class ContentControllerTests var notifications = new SimpleNotificationModel(); var contentController = CreateContentController(domainServiceMock.Object); - contentController.AddDomainWarnings(rootNode, culturesPublished, notifications); + contentController.AddDomainWarnings(rootNode, culturesPublished, notifications, "en-us"); Assert.AreEqual(3, notifications.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning)); } + [Test] + public void Does_Not_Warn_When_Only_Publishing_The_Default_Culture_With_No_Domains_Assigned() + { + var domainServiceMock = new Mock(); + domainServiceMock.Setup(x => x.GetAssignedDomains(It.IsAny(), It.IsAny())) + .Returns(Enumerable.Empty()); + + var rootNode = new ContentBuilder() + .WithContentType(CreateContentType()) + .WithId(1060) + .AddContentCultureInfosCollection() + .AddCultureInfos() + .WithCultureIso("da-dk") + .Done() + .AddCultureInfos() + .WithCultureIso("en-us") + .Done() + .Done() + .Build(); + + var culturesPublished = new[] { "en-us" }; + var notifications = new SimpleNotificationModel(); + + var contentController = CreateContentController(domainServiceMock.Object); + contentController.AddDomainWarnings(rootNode, culturesPublished, notifications, "en-us"); + Assert.IsEmpty(notifications.Notifications); + } + [Test] public void Ancestor_Domains_Counts() { @@ -189,7 +217,7 @@ public class ContentControllerTests var contentController = CreateContentController(domainServiceMock.Object); var notifications = new SimpleNotificationModel(); - contentController.AddDomainWarnings(level3Node, culturesPublished, notifications); + contentController.AddDomainWarnings(level3Node, culturesPublished, notifications, "en-us"); // We expect one error because all domains except "de-de" is registered somewhere in the ancestor path Assert.AreEqual(1, notifications.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning)); @@ -225,7 +253,7 @@ public class ContentControllerTests var notifications = new SimpleNotificationModel(); var contentController = CreateContentController(domainServiceMock.Object); - contentController.AddDomainWarnings(rootNode, culturesPublished, notifications); + contentController.AddDomainWarnings(rootNode, culturesPublished, notifications, "en-us"); // We only get two errors, one for each culture being published, so no errors from previously published cultures. Assert.AreEqual(2, notifications.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning)); From 16749a724dcd7469d960c9761f8a0cb3de580f3e Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Mon, 9 Dec 2024 11:36:48 +0100 Subject: [PATCH 2/2] Add (un)publishing details to TreeChange notifications (#17757) (cherry picked from commit 404a62aa0b35d671da4d0cdaaec5bf481d81892d) --- .../Cache/DistributedCacheExtensions.cs | 4 +- .../Implement/ContentCacheRefresher.cs | 4 + .../ContentTreeChangeNotification.cs | 10 ++ .../Services/Changes/TreeChange.cs | 12 +++ src/Umbraco.Core/Services/ContentService.cs | 34 ++++-- .../ContentServiceNotificationTests.cs | 101 +++++++++++++++++- .../Umbraco.Core/Cache/RefresherTests.cs | 58 ++++++++-- 7 files changed, 205 insertions(+), 18 deletions(-) diff --git a/src/Umbraco.Core/Cache/DistributedCacheExtensions.cs b/src/Umbraco.Core/Cache/DistributedCacheExtensions.cs index 438c66a2c1..f1a925da75 100644 --- a/src/Umbraco.Core/Cache/DistributedCacheExtensions.cs +++ b/src/Umbraco.Core/Cache/DistributedCacheExtensions.cs @@ -142,7 +142,9 @@ public static class DistributedCacheExtensions Id = x.Item.Id, Key = x.Item.Key, ChangeTypes = x.ChangeTypes, - Blueprint = x.Item.Blueprint + Blueprint = x.Item.Blueprint, + PublishedCultures = x.PublishedCultures?.ToArray(), + UnpublishedCultures = x.UnpublishedCultures?.ToArray() }); dc.RefreshByPayload(ContentCacheRefresher.UniqueId, payloads); diff --git a/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs b/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs index 779b22fe68..c99987e9fc 100644 --- a/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs +++ b/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs @@ -182,6 +182,10 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase : base(new TreeChange(target, changeTypes), messages) { } + + public ContentTreeChangeNotification( + IContent target, + TreeChangeTypes changeTypes, + IEnumerable? publishedCultures, + IEnumerable? unpublishedCultures, + EventMessages messages) + : base(new TreeChange(target, changeTypes, publishedCultures, unpublishedCultures), messages) + { + } } diff --git a/src/Umbraco.Core/Services/Changes/TreeChange.cs b/src/Umbraco.Core/Services/Changes/TreeChange.cs index bb722dce24..70adf1e005 100644 --- a/src/Umbraco.Core/Services/Changes/TreeChange.cs +++ b/src/Umbraco.Core/Services/Changes/TreeChange.cs @@ -8,10 +8,22 @@ public class TreeChange ChangeTypes = changeTypes; } + public TreeChange(TItem changedItem, TreeChangeTypes changeTypes, IEnumerable? publishedCultures, IEnumerable? unpublishedCultures) + { + Item = changedItem; + ChangeTypes = changeTypes; + PublishedCultures = publishedCultures; + UnpublishedCultures = unpublishedCultures; + } + public TItem Item { get; } public TreeChangeTypes ChangeTypes { get; } + public IEnumerable? PublishedCultures { get; } + + public IEnumerable? UnpublishedCultures { get; } + public EventArgs ToEventArgs() => new EventArgs(this); public class EventArgs : System.EventArgs diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 59656de6b5..5ff81a2165 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -1595,7 +1595,12 @@ public class ContentService : RepositoryService, IContentService // events and audit scope.Notifications.Publish( new ContentUnpublishedNotification(content, eventMessages).WithState(notificationState)); - scope.Notifications.Publish(new ContentTreeChangeNotification(content, TreeChangeTypes.RefreshBranch, eventMessages)); + scope.Notifications.Publish(new ContentTreeChangeNotification( + content, + TreeChangeTypes.RefreshBranch, + variesByCulture ? culturesPublishing.IsCollectionEmpty() ? null : culturesPublishing : null, + variesByCulture ? culturesUnpublishing.IsCollectionEmpty() ? null : culturesUnpublishing : ["*"], + eventMessages)); if (culturesUnpublishing != null) { @@ -1654,7 +1659,12 @@ public class ContentService : RepositoryService, IContentService if (!branchOne) { scope.Notifications.Publish( - new ContentTreeChangeNotification(content, changeType, eventMessages)); + new ContentTreeChangeNotification( + content, + changeType, + variesByCulture ? culturesPublishing.IsCollectionEmpty() ? null : culturesPublishing : ["*"], + variesByCulture ? culturesUnpublishing.IsCollectionEmpty() ? null : culturesUnpublishing : null, + eventMessages)); scope.Notifications.Publish( new ContentPublishedNotification(content, eventMessages).WithState(notificationState)); } @@ -2118,7 +2128,8 @@ public class ContentService : RepositoryService, IContentService } // deal with the branch root - if it fails, abort - PublishResult? result = SaveAndPublishBranchItem(scope, document, shouldPublish, publishCultures, true, publishedDocuments, eventMessages, userId, allLangs, out IDictionary notificationState); + HashSet? culturesToPublish = shouldPublish(document); + PublishResult? result = SaveAndPublishBranchItem(scope, document, culturesToPublish, publishCultures, true, publishedDocuments, eventMessages, userId, allLangs, out IDictionary notificationState); if (result != null) { results.Add(result); @@ -2128,6 +2139,8 @@ public class ContentService : RepositoryService, IContentService } } + HashSet culturesPublished = culturesToPublish ?? []; + // deal with descendants // if one fails, abort its branch var exclude = new HashSet(); @@ -2153,12 +2166,14 @@ public class ContentService : RepositoryService, IContentService } // no need to check path here, parent has to be published here - result = SaveAndPublishBranchItem(scope, d, shouldPublish, publishCultures, false, publishedDocuments, eventMessages, userId, allLangs, out _); + culturesToPublish = shouldPublish(d); + result = SaveAndPublishBranchItem(scope, d, culturesToPublish, publishCultures, false, publishedDocuments, eventMessages, userId, allLangs, out _); if (result != null) { results.Add(result); if (result.Success) { + culturesPublished.UnionWith(culturesToPublish ?? []); continue; } } @@ -2175,8 +2190,14 @@ public class ContentService : RepositoryService, IContentService // trigger events for the entire branch // (SaveAndPublishBranchOne does *not* do it) + var variesByCulture = document.ContentType.VariesByCulture(); scope.Notifications.Publish( - new ContentTreeChangeNotification(document, TreeChangeTypes.RefreshBranch, eventMessages)); + new ContentTreeChangeNotification( + document, + TreeChangeTypes.RefreshBranch, + variesByCulture ? culturesPublished.IsCollectionEmpty() ? null : culturesPublished : ["*"], + null, + eventMessages)); scope.Notifications.Publish(new ContentPublishedNotification(publishedDocuments, eventMessages).WithState(notificationState)); scope.Complete(); @@ -2191,7 +2212,7 @@ public class ContentService : RepositoryService, IContentService private PublishResult? SaveAndPublishBranchItem( ICoreScope scope, IContent document, - Func?> shouldPublish, + HashSet? culturesToPublish, Func, IReadOnlyCollection, bool> publishCultures, bool isRoot, @@ -2202,7 +2223,6 @@ public class ContentService : RepositoryService, IContentService out IDictionary notificationState) { notificationState = new Dictionary(); - HashSet? culturesToPublish = shouldPublish(document); // null = do not include if (culturesToPublish == null) diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceNotificationTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceNotificationTests.cs index f7fbe57185..9f6ca6cee9 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceNotificationTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceNotificationTests.cs @@ -55,7 +55,8 @@ public class ContentServiceNotificationTests : UmbracoIntegrationTest .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() - .AddNotificationHandler(); + .AddNotificationHandler() + .AddNotificationHandler(); private void CreateTestData() { @@ -177,6 +178,67 @@ public class ContentServiceNotificationTests : UmbracoIntegrationTest } } + [Test] + public void Publishing_Invariant() + { + IContent document = new Content("content", -1, _contentType); + + var treeChangeWasCalled = false; + + ContentNotificationHandler.TreeChange += notification => + { + var change = notification.Changes.FirstOrDefault(); + var publishedCultures = change?.PublishedCultures?.ToArray(); + Assert.IsNotNull(publishedCultures); + Assert.AreEqual(1, publishedCultures.Length); + Assert.IsTrue(publishedCultures.InvariantContains("*")); + Assert.IsNull(change.UnpublishedCultures); + + treeChangeWasCalled = true; + }; + + try + { + ContentService.SaveAndPublish(document); + Assert.IsTrue(treeChangeWasCalled); + } + finally + { + ContentNotificationHandler.TreeChange = null; + } + } + + [Test] + public void Unpublishing_Invariant() + { + IContent document = new Content("content", -1, _contentType); + ContentService.SaveAndPublish(document); + + var treeChangeWasCalled = false; + + ContentNotificationHandler.TreeChange += notification => + { + var change = notification.Changes.FirstOrDefault(); + Assert.IsNull(change?.PublishedCultures); + var unpublishedCultures = change?.UnpublishedCultures?.ToArray(); + Assert.IsNotNull(unpublishedCultures); + Assert.AreEqual(1, unpublishedCultures.Length); + Assert.IsTrue(unpublishedCultures.InvariantContains("*")); + + treeChangeWasCalled = true; + }; + + try + { + ContentService.Unpublish(document); + Assert.IsTrue(treeChangeWasCalled); + } + finally + { + ContentNotificationHandler.TreeChange = null; + } + } + [Test] public void Publishing_Culture() { @@ -203,6 +265,7 @@ public class ContentServiceNotificationTests : UmbracoIntegrationTest var publishingWasCalled = false; var publishedWasCalled = false; + var treeChangeWasCalled = false; ContentNotificationHandler.PublishingContent += notification => { @@ -228,16 +291,30 @@ public class ContentServiceNotificationTests : UmbracoIntegrationTest publishedWasCalled = true; }; + ContentNotificationHandler.TreeChange += notification => + { + var change = notification.Changes.FirstOrDefault(); + var publishedCultures = change?.PublishedCultures?.ToArray(); + Assert.IsNotNull(publishedCultures); + Assert.AreEqual(1, publishedCultures.Length); + Assert.IsTrue(publishedCultures.InvariantContains("fr-FR")); + Assert.IsNull(change.UnpublishedCultures); + + treeChangeWasCalled = true; + }; + try { ContentService.SaveAndPublish(document, "fr-FR"); Assert.IsTrue(publishingWasCalled); Assert.IsTrue(publishedWasCalled); + Assert.IsTrue(treeChangeWasCalled); } finally { ContentNotificationHandler.PublishingContent = null; ContentNotificationHandler.PublishedContent = null; + ContentNotificationHandler.TreeChange = null; } document = ContentService.GetById(document.Id); @@ -366,6 +443,7 @@ public class ContentServiceNotificationTests : UmbracoIntegrationTest var publishingWasCalled = false; var publishedWasCalled = false; + var treeChangeWasCalled = false; // TODO: revisit this - it was migrated when removing static events, but the expected result seems illogic - why does this test bind to Published and not Unpublished? @@ -399,16 +477,30 @@ public class ContentServiceNotificationTests : UmbracoIntegrationTest publishedWasCalled = true; }; + ContentNotificationHandler.TreeChange += notification => + { + var change = notification.Changes.FirstOrDefault(); + var unpublishedCultures = change?.UnpublishedCultures?.ToArray(); + Assert.IsNotNull(unpublishedCultures); + Assert.AreEqual(1, unpublishedCultures.Length); + Assert.IsTrue(unpublishedCultures.InvariantContains("fr-FR")); + Assert.IsNull(change.PublishedCultures); + + treeChangeWasCalled = true; + }; + try { ContentService.CommitDocumentChanges(document); Assert.IsTrue(publishingWasCalled); Assert.IsTrue(publishedWasCalled); + Assert.IsTrue(treeChangeWasCalled); } finally { ContentNotificationHandler.PublishingContent = null; ContentNotificationHandler.PublishedContent = null; + ContentNotificationHandler.TreeChange = null; } document = ContentService.GetById(document.Id); @@ -423,7 +515,8 @@ public class ContentServiceNotificationTests : UmbracoIntegrationTest INotificationHandler, INotificationHandler, INotificationHandler, - INotificationHandler + INotificationHandler, + INotificationHandler { public static Action SavingContent { get; set; } @@ -437,6 +530,8 @@ public class ContentServiceNotificationTests : UmbracoIntegrationTest public static Action UnpublishedContent { get; set; } + public static Action TreeChange { get; set; } + public void Handle(ContentPublishedNotification notification) => PublishedContent?.Invoke(notification); public void Handle(ContentPublishingNotification notification) => PublishingContent?.Invoke(notification); @@ -447,5 +542,7 @@ public class ContentServiceNotificationTests : UmbracoIntegrationTest public void Handle(ContentUnpublishedNotification notification) => UnpublishedContent?.Invoke(notification); public void Handle(ContentUnpublishingNotification notification) => UnpublishingContent?.Invoke(notification); + + public void Handle(ContentTreeChangeNotification notification) => TreeChange?.Invoke(notification); } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/RefresherTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/RefresherTests.cs index e509742cb9..93e59c9719 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/RefresherTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/RefresherTests.cs @@ -27,26 +27,68 @@ public class RefresherTests Assert.AreEqual(source[0].ChangeTypes, payload[0].ChangeTypes); } - [Test] - public void ContentCacheRefresherCanDeserializeJsonPayload() + [TestCase(TreeChangeTypes.None, false)] + [TestCase(TreeChangeTypes.RefreshAll, true)] + [TestCase(TreeChangeTypes.RefreshBranch, false)] + [TestCase(TreeChangeTypes.Remove, true)] + [TestCase(TreeChangeTypes.RefreshNode, false)] + public void ContentCacheRefresherCanDeserializeJsonPayload(TreeChangeTypes changeTypes, bool blueprint) { + var key = Guid.NewGuid(); ContentCacheRefresher.JsonPayload[] source = { new ContentCacheRefresher.JsonPayload() { Id = 1234, - Key = Guid.NewGuid(), - ChangeTypes = TreeChangeTypes.None + Key = key, + ChangeTypes = changeTypes, + Blueprint = blueprint } }; var json = JsonConvert.SerializeObject(source); var payload = JsonConvert.DeserializeObject(json); - Assert.AreEqual(source[0].Id, payload[0].Id); - Assert.AreEqual(source[0].Key, payload[0].Key); - Assert.AreEqual(source[0].ChangeTypes, payload[0].ChangeTypes); - Assert.AreEqual(source[0].Blueprint, payload[0].Blueprint); + Assert.AreEqual(1234, payload[0].Id); + Assert.AreEqual(key, payload[0].Key); + Assert.AreEqual(changeTypes, payload[0].ChangeTypes); + Assert.AreEqual(blueprint, payload[0].Blueprint); + Assert.IsNull(payload[0].PublishedCultures); + Assert.IsNull(payload[0].UnpublishedCultures); + } + + [Test] + public void ContentCacheRefresherCanDeserializeJsonPayloadWithCultures() + { + var key = Guid.NewGuid(); + ContentCacheRefresher.JsonPayload[] source = + { + new ContentCacheRefresher.JsonPayload() + { + Id = 1234, + Key = key, + PublishedCultures = ["en-US", "da-DK"], + UnpublishedCultures = ["de-DE"] + } + }; + + var json = JsonConvert.SerializeObject(source); + var payload = JsonConvert.DeserializeObject(json); + + Assert.IsNotNull(payload[0].PublishedCultures); + Assert.Multiple(() => + { + Assert.AreEqual(2, payload[0].PublishedCultures.Length); + Assert.AreEqual("en-US", payload[0].PublishedCultures.First()); + Assert.AreEqual("da-DK", payload[0].PublishedCultures.Last()); + }); + + Assert.IsNotNull(payload[0].UnpublishedCultures); + Assert.Multiple(() => + { + Assert.AreEqual(1, payload[0].UnpublishedCultures.Length); + Assert.AreEqual("de-DE", payload[0].UnpublishedCultures.First()); + }); } [Test]