From e5e5e8cb11147b7417258d0883743e444f75df29 Mon Sep 17 00:00:00 2001 From: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> Date: Wed, 8 Jan 2025 09:14:14 +0100 Subject: [PATCH] V15: Warn when content is unroutable (#17837) * Add notification handler * Add IsContentPublished that bypasses caching * Add clarifying comment * Refactor, to never hit the db * Remove old comment * Don't add warnings if disabled * Dedicated configuration option to suppress unroutable content warnings --------- Co-authored-by: kjac --- .../Configuration/Models/ContentSettings.cs | 7 ++ ...rningsWhenPublishingNotificationHandler.cs | 119 ++++++++++++++++++ .../Routing/NewDefaultUrlProvider.cs | 19 ++- .../UmbracoBuilder.CoreServices.cs | 4 +- 4 files changed, 145 insertions(+), 4 deletions(-) create mode 100644 src/Umbraco.Core/Events/AddUnroutableContentWarningsWhenPublishingNotificationHandler.cs diff --git a/src/Umbraco.Core/Configuration/Models/ContentSettings.cs b/src/Umbraco.Core/Configuration/Models/ContentSettings.cs index 2d2ec674b9..28c9ad1427 100644 --- a/src/Umbraco.Core/Configuration/Models/ContentSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/ContentSettings.cs @@ -28,6 +28,7 @@ public class ContentSettings internal const bool StaticDisableUnpublishWhenReferenced = false; internal const bool StaticAllowEditInvariantFromNonDefault = false; internal const bool StaticShowDomainWarnings = true; + internal const bool StaticShowUnroutableContentWarnings = true; /// /// Gets or sets a value for the content notification settings. @@ -141,4 +142,10 @@ public class ContentSettings /// [DefaultValue(StaticShowDomainWarnings)] public bool ShowDomainWarnings { get; set; } = StaticShowDomainWarnings; + + /// + /// Gets or sets a value indicating whether to show unroutable content warnings. + /// + [DefaultValue(StaticShowUnroutableContentWarnings)] + public bool ShowUnroutableContentWarnings { get; set; } = StaticShowUnroutableContentWarnings; } diff --git a/src/Umbraco.Core/Events/AddUnroutableContentWarningsWhenPublishingNotificationHandler.cs b/src/Umbraco.Core/Events/AddUnroutableContentWarningsWhenPublishingNotificationHandler.cs new file mode 100644 index 0000000000..ad457d287c --- /dev/null +++ b/src/Umbraco.Core/Events/AddUnroutableContentWarningsWhenPublishingNotificationHandler.cs @@ -0,0 +1,119 @@ +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.PublishedContent; +using Umbraco.Cms.Core.Notifications; +using Umbraco.Cms.Core.PublishedCache; +using Umbraco.Cms.Core.Routing; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.Navigation; +using Umbraco.Cms.Core.Web; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Core.Events; + +public class AddUnroutableContentWarningsWhenPublishingNotificationHandler : INotificationAsyncHandler +{ + private readonly IPublishedRouter _publishedRouter; + private readonly IUmbracoContextAccessor _umbracoContextAccessor; + private readonly ILanguageService _languageService; + private readonly ILocalizedTextService _localizedTextService; + private readonly IContentService _contentService; + private readonly IVariationContextAccessor _variationContextAccessor; + private readonly ILoggerFactory _loggerFactory; + private readonly UriUtility _uriUtility; + private readonly IPublishedUrlProvider _publishedUrlProvider; + private readonly IPublishedContentCache _publishedContentCache; + private readonly IDocumentNavigationQueryService _navigationQueryService; + private readonly IEventMessagesFactory _eventMessagesFactory; + private readonly ContentSettings _contentSettings; + + public AddUnroutableContentWarningsWhenPublishingNotificationHandler( + IPublishedRouter publishedRouter, + IUmbracoContextAccessor umbracoContextAccessor, + ILanguageService languageService, + ILocalizedTextService localizedTextService, + IContentService contentService, + IVariationContextAccessor variationContextAccessor, + ILoggerFactory loggerFactory, + UriUtility uriUtility, + IPublishedUrlProvider publishedUrlProvider, + IPublishedContentCache publishedContentCache, + IDocumentNavigationQueryService navigationQueryService, + IEventMessagesFactory eventMessagesFactory, + IOptions contentSettings) + { + _publishedRouter = publishedRouter; + _umbracoContextAccessor = umbracoContextAccessor; + _languageService = languageService; + _localizedTextService = localizedTextService; + _contentService = contentService; + _variationContextAccessor = variationContextAccessor; + _loggerFactory = loggerFactory; + _uriUtility = uriUtility; + _publishedUrlProvider = publishedUrlProvider; + _publishedContentCache = publishedContentCache; + _navigationQueryService = navigationQueryService; + _eventMessagesFactory = eventMessagesFactory; + _contentSettings = contentSettings.Value; + } + + public async Task HandleAsync(ContentPublishedNotification notification, CancellationToken cancellationToken) + { + if (_contentSettings.ShowUnroutableContentWarnings is false) + { + return; + } + + foreach (IContent content in notification.PublishedEntities) + { + string[]? successfulCultures; + if (content.ContentType.VariesByCulture() is false) + { + // successfulCultures will be null here - change it to a wildcard and utilize this below + successfulCultures = ["*"]; + } + else + { + successfulCultures = content.PublishedCultures.ToArray(); + } + + if (successfulCultures?.Any() is not true) + { + return; + } + + if (!_umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext? umbracoContext)) + { + return; + } + + UrlInfo[] urls = (await content.GetContentUrlsAsync( + _publishedRouter, + umbracoContext, + _languageService, + _localizedTextService, + _contentService, + _variationContextAccessor, + _loggerFactory.CreateLogger(), + _uriUtility, + _publishedUrlProvider, + _publishedContentCache, + _navigationQueryService)).ToArray(); + + + EventMessages eventMessages = _eventMessagesFactory.Get(); + foreach (var culture in successfulCultures) + { + if (urls.Where(u => u.Culture == culture || culture == "*").All(u => u.IsUrl is false)) + { + eventMessages.Add(new EventMessage("Content published", "The document does not have a URL, possibly due to a naming collision with another document. More details can be found under Info.", EventMessageType.Warning)); + + // only add one warning here, even though there might actually be more + break; + } + } + } + } +} diff --git a/src/Umbraco.Core/Routing/NewDefaultUrlProvider.cs b/src/Umbraco.Core/Routing/NewDefaultUrlProvider.cs index f7eeb1a1b4..3818bc2776 100644 --- a/src/Umbraco.Core/Routing/NewDefaultUrlProvider.cs +++ b/src/Umbraco.Core/Routing/NewDefaultUrlProvider.cs @@ -145,8 +145,10 @@ public class NewDefaultUrlProvider : IUrlProvider private string GetLegacyRouteFormatById(Guid key, string? culture) { + var isDraft = _umbracoContextAccessor.GetRequiredUmbracoContext().InPreviewMode; - return _documentUrlService.GetLegacyRouteFormat(key, culture, _umbracoContextAccessor.GetRequiredUmbracoContext().InPreviewMode); + + return _documentUrlService.GetLegacyRouteFormat(key, culture, isDraft); } @@ -163,9 +165,22 @@ public class NewDefaultUrlProvider : IUrlProvider throw new ArgumentException("Current URL must be absolute.", nameof(current)); } + // This might seem to be some code duplication, as we do the same check in GetLegacyRouteFormat + // but this is strictly neccesary, as if we're coming from a published notification + // this document will still not always be in the memory cache. And thus we have to hit the DB + // We have the published content now, so we can check if the culture is published, and thus avoid the DB hit. + string route; + var isDraft = _umbracoContextAccessor.GetRequiredUmbracoContext().InPreviewMode; + if(isDraft is false && string.IsNullOrWhiteSpace(culture) is false && content.Cultures.Any() && content.IsInvariantOrHasCulture(culture) is false) + { + route = "#"; + } + else + { + route = GetLegacyRouteFormatById(content.Key, culture); + } // will not use cache if previewing - var route = GetLegacyRouteFormatById(content.Key, culture); return GetUrlFromRoute(route, content.Id, current, mode, culture); } diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index 50ae0d7deb..bd101ad37b 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -411,8 +411,8 @@ public static partial class UmbracoBuilderExtensions .AddNotificationHandler(); // Handlers for publish warnings - builder - .AddNotificationHandler(); + builder.AddNotificationHandler(); + builder.AddNotificationAsyncHandler(); // Handlers for save warnings builder