Merge pull request #17779 from umbraco/v15/hotfix/fix-friendly-content-extension-performance

* Refactor .Children to use PublishStatusQuery

* Fix descendants

* Fix ancestors

* Fix siblings

* Handle empty string in published status service

* Fix unit test

* Fixes issue found in tests

---------

Co-authored-by: Bjarke Berg <[email protected]>

Author: bergmania

src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs

@@ -1,5 +1,7 @@
-using Microsoft.Extensions.Options;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Options;
 using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Models.DeliveryApi;
 using Umbraco.Cms.Core.Models.PublishedContent;
 using Umbraco.Cms.Core.PublishedCache;
@@ -16,6 +18,7 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder
     private readonly IRequestPreviewService _requestPreviewService;
     private readonly IPublishedContentCache _contentCache;
     private readonly IDocumentNavigationQueryService _navigationQueryService;
+    private readonly IPublishStatusQueryService _publishStatusQueryService;
     private RequestHandlerSettings _requestSettings;
 
     public ApiContentRouteBuilder(
@@ -25,18 +28,41 @@ public ApiContentRouteBuilder(
         IRequestPreviewService requestPreviewService,
         IOptionsMonitor<RequestHandlerSettings> requestSettings,
         IPublishedContentCache contentCache,
-        IDocumentNavigationQueryService navigationQueryService)
+        IDocumentNavigationQueryService navigationQueryService,
+        IPublishStatusQueryService publishStatusQueryService)
     {
         _apiContentPathProvider = apiContentPathProvider;
         _variationContextAccessor = variationContextAccessor;
         _requestPreviewService = requestPreviewService;
         _contentCache = contentCache;
         _navigationQueryService = navigationQueryService;
+        _publishStatusQueryService = publishStatusQueryService;
         _globalSettings = globalSettings.Value;
         _requestSettings = requestSettings.CurrentValue;
         requestSettings.OnChange(settings => _requestSettings = settings);
     }
 
+    [Obsolete("Use constructor that takes an IPublishStatusQueryService instead, scheduled for removal in v17")]
+    public ApiContentRouteBuilder(
+        IApiContentPathProvider apiContentPathProvider,
+        IOptions<GlobalSettings> globalSettings,
+        IVariationContextAccessor variationContextAccessor,
+        IRequestPreviewService requestPreviewService,
+        IOptionsMonitor<RequestHandlerSettings> requestSettings,
+        IPublishedContentCache contentCache,
+        IDocumentNavigationQueryService navigationQueryService)
+        : this(
+        apiContentPathProvider,
+        globalSettings,
+        variationContextAccessor,
+        requestPreviewService,
+        requestSettings,
+        contentCache,
+        navigationQueryService,
+        StaticServiceProvider.Instance.GetRequiredService<IPublishStatusQueryService>())
+    {
+    }
+
     public IApiContentRoute? Build(IPublishedContent content, string? culture = null)
     {
         if (content.ItemType != PublishedItemType.Content)
@@ -105,7 +131,7 @@ private IPublishedContent GetRoot(IPublishedContent content, bool isPreview)
     {
         if (isPreview is false)
         {
-            return content.Root(_contentCache, _navigationQueryService);
+            return content.Root(_variationContextAccessor, _contentCache, _navigationQueryService, _publishStatusQueryService);
         }
 
         _navigationQueryService.TryGetRootKeys(out IEnumerable<Guid> rootKeys);
@@ -114,6 +140,6 @@ private IPublishedContent GetRoot(IPublishedContent content, bool isPreview)
         // in very edge case scenarios during preview, content.Root() does not map to the root.
         // we'll code our way around it for the time being.
         return rootContent.FirstOrDefault(root => root.IsAncestorOrSelf(content))
-               ?? content.Root(_contentCache, _navigationQueryService);
+               ?? content.Root(_variationContextAccessor, _contentCache, _navigationQueryService, _publishStatusQueryService);
     }
 }

src/Umbraco.Core/Extensions/PublishedContentExtensions.cs

@@ -1,4 +1,6 @@
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Models.PublishedContent;
 using Umbraco.Cms.Core.PublishedCache;
 using Umbraco.Cms.Core.Services.Navigation;
@@ -24,6 +26,7 @@ public class ContentFinderByUrlAlias : IContentFinder
     private readonly IUmbracoContextAccessor _umbracoContextAccessor;
     private readonly IPublishedContentCache _contentCache;
     private readonly IDocumentNavigationQueryService _documentNavigationQueryService;
+    private readonly IPublishStatusQueryService _publishStatusQueryService;
     private readonly IVariationContextAccessor _variationContextAccessor;
 
     /// <summary>
@@ -35,16 +38,37 @@ public ContentFinderByUrlAlias(
         IVariationContextAccessor variationContextAccessor,
         IUmbracoContextAccessor umbracoContextAccessor,
         IPublishedContentCache contentCache,
-        IDocumentNavigationQueryService documentNavigationQueryService)
+        IDocumentNavigationQueryService documentNavigationQueryService,
+        IPublishStatusQueryService publishStatusQueryService)
     {
         _publishedValueFallback = publishedValueFallback;
         _variationContextAccessor = variationContextAccessor;
         _umbracoContextAccessor = umbracoContextAccessor;
         _contentCache = contentCache;
         _documentNavigationQueryService = documentNavigationQueryService;
+        _publishStatusQueryService = publishStatusQueryService;
         _logger = logger;
     }
 
+    [Obsolete("Please use constructor that takes an IPublishStatusQueryService instead. Scheduled removal in v17")]
+    public ContentFinderByUrlAlias(
+        ILogger<ContentFinderByUrlAlias> logger,
+        IPublishedValueFallback publishedValueFallback,
+        IVariationContextAccessor variationContextAccessor,
+        IUmbracoContextAccessor umbracoContextAccessor,
+        IPublishedContentCache contentCache,
+        IDocumentNavigationQueryService documentNavigationQueryService)
+    : this(
+        logger,
+        publishedValueFallback,
+        variationContextAccessor,
+        umbracoContextAccessor,
+        contentCache,
+        documentNavigationQueryService,
+        StaticServiceProvider.Instance.GetRequiredService<IPublishStatusQueryService>())
+    {
+    }
+
     /// <summary>
     ///     Tries to find and assign an Umbraco document to a <c>PublishedRequest</c>.
     /// </summary>
@@ -145,14 +169,14 @@ bool IsMatch(IPublishedContent c, string a1, string a2)
         if (rootNodeId > 0)
         {
             IPublishedContent? rootNode = cache?.GetById(rootNodeId);
-            return rootNode?.Descendants(_variationContextAccessor, _contentCache, _documentNavigationQueryService).FirstOrDefault(x => IsMatch(x, test1, test2));
+            return rootNode?.Descendants(_variationContextAccessor, _contentCache, _documentNavigationQueryService, _publishStatusQueryService).FirstOrDefault(x => IsMatch(x, test1, test2));
         }
 
         if (cache is not null)
         {
             foreach (IPublishedContent rootContent in cache.GetAtRoot())
             {
-                IPublishedContent? c = rootContent.DescendantsOrSelf(_variationContextAccessor, _contentCache, _documentNavigationQueryService)
+                IPublishedContent? c = rootContent.DescendantsOrSelf(_variationContextAccessor, _contentCache, _documentNavigationQueryService, _publishStatusQueryService)
                     .FirstOrDefault(x => IsMatch(x, test1, test2));
                 if (c != null)
                 {

src/Umbraco.Core/Routing/ContentFinderByUrlAlias.cs

@@ -27,24 +27,48 @@ public class UrlProvider : IPublishedUrlProvider
         /// <param name="variationContextAccessor">The current variation accessor.</param>
         /// <param name="contentCache">The content cache.</param>
         /// <param name="navigationQueryService">The query service for the in-memory navigation structure.</param>
+        /// <param name="publishStatusQueryService">The publish status query service, to query if a given content is published in a given culture.</param>
         public UrlProvider(
             IUmbracoContextAccessor umbracoContextAccessor,
             IOptions<WebRoutingSettings> routingSettings,
             UrlProviderCollection urlProviders,
             MediaUrlProviderCollection mediaUrlProviders,
             IVariationContextAccessor variationContextAccessor,
             IPublishedContentCache contentCache,
-            IDocumentNavigationQueryService navigationQueryService)
+            IDocumentNavigationQueryService navigationQueryService,
+            IPublishStatusQueryService publishStatusQueryService)
         {
             _umbracoContextAccessor = umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor));
             _urlProviders = urlProviders;
             _mediaUrlProviders = mediaUrlProviders;
             _variationContextAccessor = variationContextAccessor ?? throw new ArgumentNullException(nameof(variationContextAccessor));
             _contentCache = contentCache;
             _navigationQueryService = navigationQueryService;
+            _publishStatusQueryService = publishStatusQueryService;
             Mode = routingSettings.Value.UrlProviderMode;
         }
 
+        [Obsolete("Use the constructor that takes all parameters. Scheduled for removal in V17.")]
+        public UrlProvider(
+            IUmbracoContextAccessor umbracoContextAccessor,
+            IOptions<WebRoutingSettings> routingSettings,
+            UrlProviderCollection urlProviders,
+            MediaUrlProviderCollection mediaUrlProviders,
+            IVariationContextAccessor variationContextAccessor,
+            IPublishedContentCache contentCache,
+            IDocumentNavigationQueryService navigationQueryService)
+            : this(
+                umbracoContextAccessor,
+                routingSettings,
+                urlProviders,
+                mediaUrlProviders,
+                variationContextAccessor,
+                contentCache,
+                navigationQueryService,
+                StaticServiceProvider.Instance.GetRequiredService<IPublishStatusQueryService>())
+        {
+        }
+
         [Obsolete("Use the constructor that takes all parameters. Scheduled for removal in V17.")]
         public UrlProvider(
             IUmbracoContextAccessor umbracoContextAccessor,
@@ -59,7 +83,8 @@ public UrlProvider(
                 mediaUrlProviders,
                 variationContextAccessor,
                 StaticServiceProvider.Instance.GetRequiredService<IPublishedContentCache>(),
-                StaticServiceProvider.Instance.GetRequiredService<IDocumentNavigationQueryService>())
+                StaticServiceProvider.Instance.GetRequiredService<IDocumentNavigationQueryService>(),
+                StaticServiceProvider.Instance.GetRequiredService<IPublishStatusQueryService>())
         {
         }
 
@@ -69,6 +94,7 @@ public UrlProvider(
         private readonly IVariationContextAccessor _variationContextAccessor;
         private readonly IPublishedContentCache _contentCache;
         private readonly IDocumentNavigationQueryService _navigationQueryService;
+        private readonly IPublishStatusQueryService _publishStatusQueryService;
 
         /// <summary>
         /// Gets or sets the provider URL mode.
@@ -147,7 +173,7 @@ public string GetUrl(IPublishedContent? content, UrlMode mode = UrlMode.Default,
             // be nice with tests, assume things can be null, ultimately fall back to invariant
             // (but only for variant content of course)
             // We need to check all ancestors because urls are variant even for invariant content, if an ancestor is variant.
-            if (culture == null && content.AncestorsOrSelf(_contentCache, _navigationQueryService).Any(x => x.ContentType.VariesByCulture()))
+            if (culture == null && content.AncestorsOrSelf(_variationContextAccessor, _contentCache, _navigationQueryService, _publishStatusQueryService).Any(x => x.ContentType.VariesByCulture()))
             {
                 culture = _variationContextAccessor?.VariationContext?.Culture ?? string.Empty;
             }

src/Umbraco.Core/Routing/UrlProvider.cs

@@ -1,5 +1,7 @@
 using System.Collections.Concurrent;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Persistence.Repositories;
 using Umbraco.Cms.Core.Scoping;
 
@@ -10,15 +12,31 @@ public class PublishStatusService : IPublishStatusManagementService, IPublishSta
     private readonly ILogger<PublishStatusService> _logger;
     private readonly IPublishStatusRepository _publishStatusRepository;
     private readonly ICoreScopeProvider _coreScopeProvider;
+    private readonly ILanguageService _languageService;
     private readonly IDictionary<Guid, ISet<string>> _publishedCultures = new Dictionary<Guid, ISet<string>>();
+
+    private string? DefaultCulture { get; set; }
+
+    [Obsolete("Use non-obsolete constructor. This will be removed in Umbraco 17.")]
     public PublishStatusService(
         ILogger<PublishStatusService> logger,
         IPublishStatusRepository publishStatusRepository,
         ICoreScopeProvider coreScopeProvider)
+        : this(logger, publishStatusRepository, coreScopeProvider, StaticServiceProvider.Instance.GetRequiredService<ILanguageService>())
+    {
+
+    }
+
+    public PublishStatusService(
+        ILogger<PublishStatusService> logger,
+        IPublishStatusRepository publishStatusRepository,
+        ICoreScopeProvider coreScopeProvider,
+        ILanguageService languageService)
     {
         _logger = logger;
         _publishStatusRepository = publishStatusRepository;
         _coreScopeProvider = coreScopeProvider;
+        _languageService = languageService;
     }
 
     public async Task InitializeAsync(CancellationToken cancellationToken)
@@ -39,11 +57,16 @@ public async Task InitializeAsync(CancellationToken cancellationToken)
             }
         }
 
-
+        DefaultCulture = await _languageService.GetDefaultIsoCodeAsync();
     }
 
     public bool IsDocumentPublished(Guid documentKey, string culture)
     {
+        if (string.IsNullOrEmpty(culture) && DefaultCulture is not null)
+        {
+            culture = DefaultCulture;
+        }
+
         if (_publishedCultures.TryGetValue(documentKey, out ISet<string>? publishedCultures))
         {
             return publishedCultures.Contains(culture, StringComparer.InvariantCultureIgnoreCase);