From 809883f0884201aa6780f03ddb8398b5fad8db54 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Mon, 5 Jun 2023 12:00:25 +0200 Subject: [PATCH] Support entirely unpublished content in preview mode (#14307) * Support draft-only content in the Delivery API query * Allow outputting "entirely unpublished" content * Make the preview path explicit to avoid clashing endpoints * Handle trailing slash setting for preview URLs * Update src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexValueSetBuilder.cs Co-authored-by: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com> * Remove superfluous (and incorrect) unpublished route handling * Make sure preview output includes routes for unpublished cultures * Ensure that published content with unpublished ancestors are available in preview * Fix route start item when previewing published content with unpublished parent --------- Co-authored-by: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com> --- .../ByRouteContentApiController.cs | 39 +++++- .../Services/ApiContentQueryProvider.cs | 12 +- .../Services/ApiContentQueryService.cs | 8 +- src/Umbraco.Core/Constants-DeliveryApi.cs | 21 +++ .../DeliveryApi/ApiContentRouteBuilder.cs | 71 ++++++++-- .../DeliveryApi/IApiContentQueryProvider.cs | 3 +- .../DeliveryApiContentIndex.cs | 2 +- ...veryApiContentIndexHandleContentChanges.cs | 91 +++++++++---- ...ryApiContentIndexFieldDefinitionBuilder.cs | 1 + .../Examine/DeliveryApiContentIndexHelper.cs | 33 +---- .../DeliveryApiContentIndexValueSetBuilder.cs | 23 +++- .../Examine/UmbracoExamineFieldNames.cs | 5 + .../DeliveryApi/ContentBuilderTests.cs | 2 +- .../ContentPickerValueConverterTests.cs | 2 +- .../DeliveryApi/ContentRouteBuilderTests.cs | 128 +++++++++++++++--- .../DeliveryApi/DeliveryApiTests.cs | 27 ++++ .../MultiNodeTreePickerValueConverterTests.cs | 2 +- .../MultiUrlPickerValueConverterTests.cs | 3 +- .../OutputExpansionStrategyTests.cs | 2 +- 19 files changed, 373 insertions(+), 102 deletions(-) create mode 100644 src/Umbraco.Core/Constants-DeliveryApi.cs diff --git a/src/Umbraco.Cms.Api.Delivery/Controllers/ByRouteContentApiController.cs b/src/Umbraco.Cms.Api.Delivery/Controllers/ByRouteContentApiController.cs index 77bc86f807..2209c1a330 100644 --- a/src/Umbraco.Cms.Api.Delivery/Controllers/ByRouteContentApiController.cs +++ b/src/Umbraco.Cms.Api.Delivery/Controllers/ByRouteContentApiController.cs @@ -2,6 +2,7 @@ using System.Net; using Asp.Versioning; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.DeliveryApi; using Umbraco.Cms.Core.Models.DeliveryApi; using Umbraco.Cms.Core.Models.PublishedContent; @@ -15,17 +16,20 @@ public class ByRouteContentApiController : ContentApiItemControllerBase { private readonly IRequestRoutingService _requestRoutingService; private readonly IRequestRedirectService _requestRedirectService; + private readonly IRequestPreviewService _requestPreviewService; public ByRouteContentApiController( IApiPublishedContentCache apiPublishedContentCache, IApiContentResponseBuilder apiContentResponseBuilder, IPublicAccessService publicAccessService, IRequestRoutingService requestRoutingService, - IRequestRedirectService requestRedirectService) + IRequestRedirectService requestRedirectService, + IRequestPreviewService requestPreviewService) : base(apiPublishedContentCache, apiContentResponseBuilder, publicAccessService) { _requestRoutingService = requestRoutingService; _requestRedirectService = requestRedirectService; + _requestPreviewService = requestPreviewService; } /// @@ -56,9 +60,7 @@ public class ByRouteContentApiController : ContentApiItemControllerBase path = path.EnsureStartsWith("/"); - var contentRoute = _requestRoutingService.GetContentRoute(path); - - IPublishedContent? contentItem = ApiPublishedContentCache.GetByRoute(contentRoute); + IPublishedContent? contentItem = GetContent(path); if (contentItem is not null) { if (IsProtected(contentItem)) @@ -75,6 +77,35 @@ public class ByRouteContentApiController : ContentApiItemControllerBase : NotFound(); } + private IPublishedContent? GetContent(string path) + => path.StartsWith(Constants.DeliveryApi.Routing.PreviewContentPathPrefix) + ? GetPreviewContent(path) + : GetPublishedContent(path); + + private IPublishedContent? GetPublishedContent(string path) + { + var contentRoute = _requestRoutingService.GetContentRoute(path); + + IPublishedContent? contentItem = ApiPublishedContentCache.GetByRoute(contentRoute); + return contentItem; + } + + private IPublishedContent? GetPreviewContent(string path) + { + if (_requestPreviewService.IsPreview() is false) + { + return null; + } + + if (Guid.TryParse(path.AsSpan(Constants.DeliveryApi.Routing.PreviewContentPathPrefix.Length).TrimEnd("/"), out Guid contentId) is false) + { + return null; + } + + IPublishedContent? contentItem = ApiPublishedContentCache.GetById(contentId); + return contentItem; + } + private IActionResult RedirectTo(IApiContentRoute redirectRoute) { Response.Headers.Add("Location-Start-Item-Path", redirectRoute.StartItem.Path); diff --git a/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryProvider.cs b/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryProvider.cs index 37be4f16ee..45ae32b4f5 100644 --- a/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryProvider.cs +++ b/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryProvider.cs @@ -41,7 +41,7 @@ internal sealed class ApiContentQueryProvider : IApiContentQueryProvider .ToDictionary(field => field.FieldName, field => field.FieldType, StringComparer.InvariantCultureIgnoreCase); } - public PagedModel ExecuteQuery(SelectorOption selectorOption, IList filterOptions, IList sortOptions, string culture, int skip, int take) + public PagedModel ExecuteQuery(SelectorOption selectorOption, IList filterOptions, IList sortOptions, string culture, bool preview, int skip, int take) { if (!_examineManager.TryGetIndex(Constants.UmbracoIndexes.DeliveryApiContentIndexName, out IIndex? index)) { @@ -49,7 +49,7 @@ internal sealed class ApiContentQueryProvider : IApiContentQueryProvider return new PagedModel(); } - IBooleanOperation queryOperation = BuildSelectorOperation(selectorOption, index, culture); + IBooleanOperation queryOperation = BuildSelectorOperation(selectorOption, index, culture, preview); ApplyFiltering(filterOptions, queryOperation); ApplySorting(sortOptions, queryOperation); @@ -77,7 +77,7 @@ internal sealed class ApiContentQueryProvider : IApiContentQueryProvider FieldName = UmbracoExamineFieldNames.CategoryFieldName, Values = new[] { "content" } }; - private IBooleanOperation BuildSelectorOperation(SelectorOption selectorOption, IIndex index, string culture) + private IBooleanOperation BuildSelectorOperation(SelectorOption selectorOption, IIndex index, string culture, bool preview) { // Needed for enabling leading wildcards searches BaseLuceneSearcher searcher = index.Searcher as BaseLuceneSearcher ?? throw new InvalidOperationException($"Index searcher must be of type {nameof(BaseLuceneSearcher)}."); @@ -95,6 +95,12 @@ internal sealed class ApiContentQueryProvider : IApiContentQueryProvider // Item culture must be either the requested culture or "none" selectorOperation.And().GroupedOr(new[] { UmbracoExamineFieldNames.DeliveryApiContentIndex.Culture }, culture.ToLowerInvariant().IfNullOrWhiteSpace(_fallbackGuidValue), "none"); + // when not fetching for preview, make sure the "published" field is "y" + if (preview is false) + { + selectorOperation.And().Field(UmbracoExamineFieldNames.DeliveryApiContentIndex.Published, "y"); + } + return selectorOperation; } diff --git a/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryService.cs b/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryService.cs index 11b456865f..dc03a79e19 100644 --- a/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryService.cs +++ b/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryService.cs @@ -16,6 +16,7 @@ internal sealed class ApiContentQueryService : IApiContentQueryService private readonly SortHandlerCollection _sortHandlers; private readonly IVariationContextAccessor _variationContextAccessor; private readonly IApiContentQueryProvider _apiContentQueryProvider; + private readonly IRequestPreviewService _requestPreviewService; public ApiContentQueryService( IRequestStartItemProviderAccessor requestStartItemProviderAccessor, @@ -23,7 +24,8 @@ internal sealed class ApiContentQueryService : IApiContentQueryService FilterHandlerCollection filterHandlers, SortHandlerCollection sortHandlers, IVariationContextAccessor variationContextAccessor, - IApiContentQueryProvider apiContentQueryProvider) + IApiContentQueryProvider apiContentQueryProvider, + IRequestPreviewService requestPreviewService) { _requestStartItemProviderAccessor = requestStartItemProviderAccessor; _selectorHandlers = selectorHandlers; @@ -31,6 +33,7 @@ internal sealed class ApiContentQueryService : IApiContentQueryService _sortHandlers = sortHandlers; _variationContextAccessor = variationContextAccessor; _apiContentQueryProvider = apiContentQueryProvider; + _requestPreviewService = requestPreviewService; } /// @@ -72,8 +75,9 @@ internal sealed class ApiContentQueryService : IApiContentQueryService } var culture = _variationContextAccessor.VariationContext?.Culture ?? string.Empty; + var isPreview = _requestPreviewService.IsPreview(); - PagedModel result = _apiContentQueryProvider.ExecuteQuery(selectorOption, filterOptions, sortOptions, culture, skip, take); + PagedModel result = _apiContentQueryProvider.ExecuteQuery(selectorOption, filterOptions, sortOptions, culture, isPreview, skip, take); return Attempt.SucceedWithStatus(ApiContentQueryOperationStatus.Success, result); } diff --git a/src/Umbraco.Core/Constants-DeliveryApi.cs b/src/Umbraco.Core/Constants-DeliveryApi.cs new file mode 100644 index 0000000000..e2f23e414c --- /dev/null +++ b/src/Umbraco.Core/Constants-DeliveryApi.cs @@ -0,0 +1,21 @@ +namespace Umbraco.Cms.Core; + +public static partial class Constants +{ + /// + /// Defines constants for the Delivery API. + /// + public static class DeliveryApi + { + /// + /// Constants for Delivery API routing purposes. + /// + public static class Routing + { + /// + /// Path prefix for unpublished content requested in a preview context. + /// + public const string PreviewContentPathPrefix = "preview-"; + } + } +} diff --git a/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs b/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs index 80552b6488..0ae310d585 100644 --- a/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs +++ b/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs @@ -14,17 +14,24 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder private readonly GlobalSettings _globalSettings; private readonly IVariationContextAccessor _variationContextAccessor; private readonly IPublishedSnapshotAccessor _publishedSnapshotAccessor; + private readonly IRequestPreviewService _requestPreviewService; + private RequestHandlerSettings _requestSettings; public ApiContentRouteBuilder( IPublishedUrlProvider publishedUrlProvider, IOptions globalSettings, IVariationContextAccessor variationContextAccessor, - IPublishedSnapshotAccessor publishedSnapshotAccessor) + IPublishedSnapshotAccessor publishedSnapshotAccessor, + IRequestPreviewService requestPreviewService, + IOptionsMonitor requestSettings) { _publishedUrlProvider = publishedUrlProvider; _variationContextAccessor = variationContextAccessor; _publishedSnapshotAccessor = publishedSnapshotAccessor; + _requestPreviewService = requestPreviewService; _globalSettings = globalSettings.Value; + _requestSettings = requestSettings.CurrentValue; + requestSettings.OnChange(settings => _requestSettings = settings); } public IApiContentRoute? Build(IPublishedContent content, string? culture = null) @@ -34,9 +41,37 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder throw new ArgumentException("Content locations can only be built from Content items.", nameof(content)); } - IPublishedContent root = content.Root(); + var isPreview = _requestPreviewService.IsPreview(); + + var contentPath = GetContentPath(content, culture, isPreview); + if (contentPath == null) + { + return null; + } + + contentPath = contentPath.EnsureStartsWith("/"); + + IPublishedContent root = GetRoot(content, isPreview); var rootPath = root.UrlSegment(_variationContextAccessor, culture) ?? string.Empty; + if (_globalSettings.HideTopLevelNodeFromPath == false) + { + contentPath = contentPath.TrimStart(rootPath.EnsureStartsWith("/")).EnsureStartsWith("/"); + } + + return new ApiContentRoute(contentPath, new ApiContentStartItem(root.Key, rootPath)); + } + + private string? GetContentPath(IPublishedContent content, string? culture, bool isPreview) + { + // entirely unpublished content does not resolve any route, but we need one i.e. for preview to work, + // so we'll use the content key as path. + if (isPreview && content.IsPublished(culture) is false) + { + return ContentPreviewPath(content); + } + + // grab the content path from the URL provider var contentPath = _publishedUrlProvider.GetUrl(content, UrlMode.Relative, culture); // in some scenarios the published content is actually routable, but due to the built-in handling of i.e. lacking culture setup @@ -48,19 +83,35 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder } // if the content path has still not been resolved as a valid path, the content is un-routable in this culture + // - unless we are routing for preview if (IsInvalidContentPath(contentPath)) { - return null; + return isPreview + ? ContentPreviewPath(content) + : null; } - contentPath = contentPath.EnsureStartsWith("/"); - if (_globalSettings.HideTopLevelNodeFromPath == false) - { - contentPath = contentPath.TrimStart(rootPath.EnsureStartsWith("/")).EnsureStartsWith("/"); - } - - return new ApiContentRoute(contentPath, new ApiContentStartItem(root.Key, rootPath)); + return contentPath; } + private string ContentPreviewPath(IPublishedContent content) => $"{Constants.DeliveryApi.Routing.PreviewContentPathPrefix}{content.Key:D}{(_requestSettings.AddTrailingSlash ? "/" : string.Empty)}"; + private static bool IsInvalidContentPath(string path) => path.IsNullOrWhiteSpace() || "#".Equals(path); + + private IPublishedContent GetRoot(IPublishedContent content, bool isPreview) + { + if (isPreview is false) + { + return content.Root(); + } + + // 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 _publishedSnapshotAccessor + .GetRequiredPublishedSnapshot() + .Content? + .GetAtRoot(true) + .FirstOrDefault(root => root.IsAncestorOrSelf(content)) + ?? content.Root(); + } } diff --git a/src/Umbraco.Core/DeliveryApi/IApiContentQueryProvider.cs b/src/Umbraco.Core/DeliveryApi/IApiContentQueryProvider.cs index 94aa42820c..feb3a3a74b 100644 --- a/src/Umbraco.Core/DeliveryApi/IApiContentQueryProvider.cs +++ b/src/Umbraco.Core/DeliveryApi/IApiContentQueryProvider.cs @@ -14,10 +14,11 @@ public interface IApiContentQueryProvider /// The filter options of the search criteria. /// The sorting options of the search criteria. /// The requested culture. + /// Whether or not to search for preview content. /// Number of search results to skip (for pagination). /// Number of search results to retrieve (for pagination). /// A paged model containing the resulting IDs and the total number of results that matching the search criteria. - PagedModel ExecuteQuery(SelectorOption selectorOption, IList filterOptions, IList sortOptions, string culture, int skip, int take); + PagedModel ExecuteQuery(SelectorOption selectorOption, IList filterOptions, IList sortOptions, string culture, bool preview, int skip, int take); /// /// Returns a selector option that can be applied to fetch "all content" (i.e. if a selector option is not present when performing a search). diff --git a/src/Umbraco.Examine.Lucene/DeliveryApiContentIndex.cs b/src/Umbraco.Examine.Lucene/DeliveryApiContentIndex.cs index 0967022d77..c70c9d4da0 100644 --- a/src/Umbraco.Examine.Lucene/DeliveryApiContentIndex.cs +++ b/src/Umbraco.Examine.Lucene/DeliveryApiContentIndex.cs @@ -21,7 +21,7 @@ public class DeliveryApiContentIndex : UmbracoExamineIndex IRuntimeState runtimeState) : base(loggerFactory, name, indexOptions, hostingEnvironment, runtimeState) { - PublishedValuesOnly = true; + PublishedValuesOnly = false; EnableDefaultEventHandler = false; _logger = loggerFactory.CreateLogger(); diff --git a/src/Umbraco.Infrastructure/Examine/Deferred/DeliveryApiContentIndexHandleContentChanges.cs b/src/Umbraco.Infrastructure/Examine/Deferred/DeliveryApiContentIndexHandleContentChanges.cs index c93f42b6e8..66859edd7d 100644 --- a/src/Umbraco.Infrastructure/Examine/Deferred/DeliveryApiContentIndexHandleContentChanges.cs +++ b/src/Umbraco.Infrastructure/Examine/Deferred/DeliveryApiContentIndexHandleContentChanges.cs @@ -71,56 +71,57 @@ internal sealed class DeliveryApiContentIndexHandleContentChanges : DeliveryApiC private void Reindex(IContent content, IIndex index) { // get the currently indexed cultures for the content - var existingIndexCultures = index + CulturePublishStatus[] existingCultures = index .Searcher .CreateQuery() .Field(UmbracoExamineFieldNames.DeliveryApiContentIndex.Id, content.Id.ToString()) - .SelectField(UmbracoExamineFieldNames.DeliveryApiContentIndex.Culture) + .SelectFields(new HashSet + { + UmbracoExamineFieldNames.DeliveryApiContentIndex.Culture, + UmbracoExamineFieldNames.DeliveryApiContentIndex.Published + }) .Execute() - .SelectMany(f => f.GetValues(UmbracoExamineFieldNames.DeliveryApiContentIndex.Culture)) + .Select(f => new CulturePublishStatus + { + Culture = f.GetValues(UmbracoExamineFieldNames.DeliveryApiContentIndex.Culture).Single(), + Published = f.GetValues(UmbracoExamineFieldNames.DeliveryApiContentIndex.Published).Single() + }) .ToArray(); // index the content - var indexedCultures = UpdateIndex(content, index); + CulturePublishStatus[] indexedCultures = UpdateIndex(content, index); if (indexedCultures.Any() is false) { - // we likely got here because unpublishing triggered a "refresh branch" notification, now we + // we likely got here because a removal triggered a "refresh branch" notification, now we // need to delete every last culture of this content and all descendants RemoveFromIndex(content.Id, index); return; } - // if any of the content cultures did not exist in the index before, nor will any of its published descendants - // in those cultures be at this point, so make sure those are added as well - if (indexedCultures.Except(existingIndexCultures).Any()) + // if the published state changed of any culture, chances are there are similar changes ot the content descendants + // that need to be reflected in the index, so we'll reindex all descendants + var changedCulturePublishStatus = indexedCultures.Intersect(existingCultures).Count() != existingCultures.Length; + if (changedCulturePublishStatus) { ReindexDescendants(content, index); } - - // ensure that any unpublished cultures are removed from the index - var unpublishedCultures = existingIndexCultures.Except(indexedCultures).ToArray(); - if (unpublishedCultures.Any() is false) - { - return; - } - - var idsToDelete = unpublishedCultures - .Select(culture => DeliveryApiContentIndexUtilites.IndexId(content, culture)).ToArray(); - RemoveFromIndex(idsToDelete, index); } - private string[] UpdateIndex(IContent content, IIndex index) + private CulturePublishStatus[] UpdateIndex(IContent content, IIndex index) { ValueSet[] valueSets = _deliveryApiContentIndexValueSetBuilder.GetValueSets(content).ToArray(); if (valueSets.Any() is false) { - return Array.Empty(); + return Array.Empty(); } index.IndexItems(valueSets); return valueSets - .SelectMany(v => v.GetValues("culture").Select(c => c.ToString())) - .WhereNotNull() + .Select(v => new CulturePublishStatus + { + Culture = v.GetValue(UmbracoExamineFieldNames.DeliveryApiContentIndex.Culture).ToString()!, + Published = v.GetValue(UmbracoExamineFieldNames.DeliveryApiContentIndex.Published).ToString()! + }) .ToArray(); } @@ -134,4 +135,48 @@ internal sealed class DeliveryApiContentIndexHandleContentChanges : DeliveryApiC UpdateIndex(descendant, index); } }); + + private class CulturePublishStatus : IEquatable + { + public required string Culture { get; set; } + + public required string Published { get; set; } + + public bool Equals(CulturePublishStatus? other) + { + if (ReferenceEquals(null, other)) + { + return false; + } + + if (ReferenceEquals(this, other)) + { + return true; + } + + return Culture == other.Culture && Published == other.Published; + } + + public override bool Equals(object? obj) + { + if (ReferenceEquals(null, obj)) + { + return false; + } + + if (ReferenceEquals(this, obj)) + { + return true; + } + + if (obj.GetType() != this.GetType()) + { + return false; + } + + return Equals((CulturePublishStatus)obj); + } + + public override int GetHashCode() => HashCode.Combine(Culture, Published); + } } diff --git a/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexFieldDefinitionBuilder.cs b/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexFieldDefinitionBuilder.cs index a0bbecbd74..f7d4024c57 100644 --- a/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexFieldDefinitionBuilder.cs +++ b/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexFieldDefinitionBuilder.cs @@ -35,6 +35,7 @@ internal sealed class DeliveryApiContentIndexFieldDefinitionBuilder : IDeliveryA fieldDefinitions.Add(new(UmbracoExamineFieldNames.DeliveryApiContentIndex.Id, FieldDefinitionTypes.Raw)); fieldDefinitions.Add(new(UmbracoExamineFieldNames.DeliveryApiContentIndex.ContentTypeId, FieldDefinitionTypes.Raw)); fieldDefinitions.Add(new(UmbracoExamineFieldNames.DeliveryApiContentIndex.Culture, FieldDefinitionTypes.Raw)); + fieldDefinitions.Add(new(UmbracoExamineFieldNames.DeliveryApiContentIndex.Published, FieldDefinitionTypes.Raw)); fieldDefinitions.Add(new(UmbracoExamineFieldNames.IndexPathFieldName, FieldDefinitionTypes.Raw)); fieldDefinitions.Add(new(UmbracoExamineFieldNames.NodeNameFieldName, FieldDefinitionTypes.Raw)); } diff --git a/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexHelper.cs b/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexHelper.cs index 34897bd7af..744502b3de 100644 --- a/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexHelper.cs +++ b/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexHelper.cs @@ -29,38 +29,17 @@ internal sealed class DeliveryApiContentIndexHelper : IDeliveryApiContentIndexHe { const int pageSize = 10000; var pageIndex = 0; - var publishedContentIds = new HashSet { rootContentId }; IContent[] descendants; - IQuery publishedQuery = _umbracoDatabaseFactory.SqlContext.Query().Where(x => x.Published && x.Trashed == false); + IQuery query = _umbracoDatabaseFactory.SqlContext.Query().Where(content => content.Trashed == false); do { - descendants = _contentService.GetPagedDescendants(rootContentId, pageIndex, pageSize, out _, publishedQuery, Ordering.By("Path")).ToArray(); + descendants = _contentService + .GetPagedDescendants(rootContentId, pageIndex, pageSize, out _, query, Ordering.By("Path")) + .Where(descendant => _deliveryApiSettings.IsAllowedContentType(descendant.ContentType.Alias)) + .ToArray(); - // there are a few rules we need to abide to when populating the index: - // - children of unpublished content can still be published; we need to filter them out, as they're not supposed to go into the index. - // - content of disallowed content types are not allowed in the index, but their children are - // as we're querying published content and ordering by path, we can construct a list of "allowed" published content IDs like this. - var allowedDescendants = new List(); - foreach (IContent descendant in descendants) - { - if (_deliveryApiSettings.IsDisallowedContentType(descendant.ContentType.Alias)) - { - // the content type is disallowed; make sure we consider all its children as candidates for the index anyway - publishedContentIds.Add(descendant.Id); - continue; - } - - // content at root level is by definition published, because we only fetch published content in the query above. - // content not at root level should be included only if their parents are included (unbroken chain of published content) - if (descendant.Level == 1 || publishedContentIds.Contains(descendant.ParentId)) - { - publishedContentIds.Add(descendant.Id); - allowedDescendants.Add(descendant); - } - } - - actionToPerform(allowedDescendants.ToArray()); + actionToPerform(descendants.ToArray()); pageIndex++; } diff --git a/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexValueSetBuilder.cs b/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexValueSetBuilder.cs index 20942336ab..4d95dc1cde 100644 --- a/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexValueSetBuilder.cs +++ b/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexValueSetBuilder.cs @@ -37,11 +37,13 @@ internal sealed class DeliveryApiContentIndexValueSetBuilder : IDeliveryApiConte { foreach (IContent content in contents.Where(CanIndex)) { - var cultures = IndexableCultures(content); + var publishedCultures = PublishedCultures(content); + var availableCultures = AvailableCultures(content); - foreach (var culture in cultures) + foreach (var culture in availableCultures) { var indexCulture = culture ?? "none"; + var isPublished = publishedCultures.Contains(culture); // required index values go here var indexValues = new Dictionary>(StringComparer.InvariantCultureIgnoreCase) @@ -49,8 +51,9 @@ internal sealed class DeliveryApiContentIndexValueSetBuilder : IDeliveryApiConte [UmbracoExamineFieldNames.DeliveryApiContentIndex.Id] = new object[] { content.Id.ToString() }, // required for correct publishing handling and also needed for backoffice index browsing [UmbracoExamineFieldNames.DeliveryApiContentIndex.ContentTypeId] = new object[] { content.ContentTypeId.ToString() }, // required for correct content type change handling [UmbracoExamineFieldNames.DeliveryApiContentIndex.Culture] = new object[] { indexCulture }, // required for culture variant querying + [UmbracoExamineFieldNames.DeliveryApiContentIndex.Published] = new object[] { isPublished ? "y" : "n" }, // required for querying draft content [UmbracoExamineFieldNames.IndexPathFieldName] = new object[] { content.Path }, // required for unpublishing/deletion handling - [UmbracoExamineFieldNames.NodeNameFieldName] = new object[] { content.GetPublishName(culture) ?? string.Empty }, // primarily needed for backoffice index browsing + [UmbracoExamineFieldNames.NodeNameFieldName] = new object[] { content.GetPublishName(culture) ?? content.GetCultureName(culture) ?? string.Empty }, // primarily needed for backoffice index browsing }; AddContentIndexHandlerFields(content, culture, indexValues); @@ -60,8 +63,18 @@ internal sealed class DeliveryApiContentIndexValueSetBuilder : IDeliveryApiConte } } - private string?[] IndexableCultures(IContent content) + private string?[] AvailableCultures(IContent content) + => content.ContentType.VariesByCulture() + ? content.AvailableCultures.ToArray() + : new string?[] { null }; + + private string?[] PublishedCultures(IContent content) { + if (content.Published == false) + { + return Array.Empty(); + } + var variesByCulture = content.ContentType.VariesByCulture(); // if the content varies by culture, the indexable cultures are the published @@ -116,7 +129,7 @@ internal sealed class DeliveryApiContentIndexValueSetBuilder : IDeliveryApiConte private bool CanIndex(IContent content) { // is the content in a state that is allowed in the index? - if (content.Published is false || content.Trashed) + if (content.Trashed) { return false; } diff --git a/src/Umbraco.Infrastructure/Examine/UmbracoExamineFieldNames.cs b/src/Umbraco.Infrastructure/Examine/UmbracoExamineFieldNames.cs index 12b2eb2207..5376e91897 100644 --- a/src/Umbraco.Infrastructure/Examine/UmbracoExamineFieldNames.cs +++ b/src/Umbraco.Infrastructure/Examine/UmbracoExamineFieldNames.cs @@ -45,5 +45,10 @@ public static class UmbracoExamineFieldNames /// The content culture /// public const string Culture = "culture"; + + /// + /// Whether or not the content exists in a published state + /// + public const string Published = "published"; } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs index f214c05446..026ab42c0c 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs @@ -34,7 +34,7 @@ public class ContentBuilderTests : DeliveryApiTests .Setup(p => p.GetUrl(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns((IPublishedContent content, UrlMode mode, string? culture, Uri? current) => $"url:{content.UrlSegment}"); - var routeBuilder = new ApiContentRouteBuilder(publishedUrlProvider.Object, CreateGlobalSettings(), Mock.Of(), Mock.Of()); + var routeBuilder = CreateContentRouteBuilder(publishedUrlProvider.Object, CreateGlobalSettings()); var builder = new ApiContentBuilder(new ApiContentNameProvider(), routeBuilder, CreateOutputExpansionStrategyAccessor()); var result = builder.Build(content.Object); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentPickerValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentPickerValueConverterTests.cs index 6a48e6ebda..61e55b76db 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentPickerValueConverterTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentPickerValueConverterTests.cs @@ -18,7 +18,7 @@ public class ContentPickerValueConverterTests : PropertyValueConverterTests PublishedSnapshotAccessor, new ApiContentBuilder( nameProvider ?? new ApiContentNameProvider(), - new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of(), Mock.Of()), + CreateContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings()), CreateOutputExpansionStrategyAccessor())); [Test] diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs index 90774c5e25..d60097bf69 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs @@ -1,5 +1,8 @@ -using Moq; +using Microsoft.Extensions.Options; +using Moq; using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.DeliveryApi; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.DeliveryApi; @@ -188,18 +191,81 @@ public class ContentRouteBuilderTests : DeliveryApiTests Assert.AreEqual(hideTopLevelNodeFromPath ? "/the-child/the-grandchild" : "/the-root/the-child/the-grandchild", publishedUrlProvider.GetUrl(grandchild)); } - private IPublishedContent SetupInvariantPublishedContent(string name, Guid key, IPublishedContent? parent = null) + [TestCase(true)] + [TestCase(false)] + public void CanRouteUnpublishedChild(bool hideTopLevelNodeFromPath) + { + var rootKey = Guid.NewGuid(); + var root = SetupInvariantPublishedContent("The Root", rootKey); + + var childKey = Guid.NewGuid(); + var child = SetupInvariantPublishedContent("The Child", childKey, root, false); + + var builder = CreateApiContentRouteBuilder(hideTopLevelNodeFromPath, isPreview: true); + var result = builder.Build(child); + Assert.IsNotNull(result); + Assert.AreEqual($"/{Constants.DeliveryApi.Routing.PreviewContentPathPrefix}{childKey:D}", result.Path); + Assert.AreEqual(rootKey, result.StartItem.Id); + Assert.AreEqual("the-root", result.StartItem.Path); + } + + [TestCase(true)] + [TestCase(false)] + public void UnpublishedChildRouteRespectsTrailingSlashSettings(bool addTrailingSlash) + { + var rootKey = Guid.NewGuid(); + var root = SetupInvariantPublishedContent("The Root", rootKey); + + var childKey = Guid.NewGuid(); + var child = SetupInvariantPublishedContent("The Child", childKey, root, false); + + var builder = CreateApiContentRouteBuilder(true, addTrailingSlash, isPreview: true); + var result = builder.Build(child); + Assert.IsNotNull(result); + Assert.AreEqual(addTrailingSlash, result.Path.EndsWith("/")); + } + + [TestCase(true)] + [TestCase(false)] + public void CanRoutePublishedChildOfUnpublishedParentInPreview(bool isPreview) + { + var rootKey = Guid.NewGuid(); + var root = SetupInvariantPublishedContent("The Root", rootKey, published: false); + + var childKey = Guid.NewGuid(); + var child = SetupInvariantPublishedContent("The Child", childKey, root); + + var requestPreviewServiceMock = new Mock(); + requestPreviewServiceMock.Setup(m => m.IsPreview()).Returns(isPreview); + + var builder = CreateApiContentRouteBuilder(true, isPreview: isPreview); + var result = builder.Build(child); + + if (isPreview) + { + Assert.IsNotNull(result); + Assert.AreEqual($"/{Constants.DeliveryApi.Routing.PreviewContentPathPrefix}{childKey:D}", result.Path); + Assert.AreEqual(rootKey, result.StartItem.Id); + Assert.AreEqual("the-root", result.StartItem.Path); + } + else + { + Assert.IsNull(result); + } + } + + private IPublishedContent SetupInvariantPublishedContent(string name, Guid key, IPublishedContent? parent = null, bool published = true) { var publishedContentType = CreatePublishedContentType(); - var content = CreatePublishedContentMock(publishedContentType.Object, name, key, parent); + var content = CreatePublishedContentMock(publishedContentType.Object, name, key, parent, published); return content.Object; } - private IPublishedContent SetupVariantPublishedContent(string name, Guid key, IPublishedContent? parent = null) + private IPublishedContent SetupVariantPublishedContent(string name, Guid key, IPublishedContent? parent = null, bool published = true) { var publishedContentType = CreatePublishedContentType(); publishedContentType.SetupGet(m => m.Variations).Returns(ContentVariation.Culture); - var content = CreatePublishedContentMock(publishedContentType.Object, name, key, parent); + var content = CreatePublishedContentMock(publishedContentType.Object, name, key, parent, published); var cultures = new[] { "en-us", "da-dk" }; content .SetupGet(m => m.Cultures) @@ -209,10 +275,11 @@ public class ContentRouteBuilderTests : DeliveryApiTests return content.Object; } - private Mock CreatePublishedContentMock(IPublishedContentType publishedContentType, string name, Guid key, IPublishedContent? parent) + private Mock CreatePublishedContentMock(IPublishedContentType publishedContentType, string name, Guid key, IPublishedContent? parent, bool published) { var content = new Mock(); ConfigurePublishedContentMock(content, key, name, DefaultUrlSegment(name), publishedContentType, Array.Empty()); + content.Setup(c => c.IsPublished(It.IsAny())).Returns(published); content.SetupGet(c => c.Parent).Returns(parent); content.SetupGet(c => c.Level).Returns((parent?.Level ?? 0) + 1); return content; @@ -230,7 +297,11 @@ public class ContentRouteBuilderTests : DeliveryApiTests { var variantContextAccessor = Mock.Of(); string Url(IPublishedContent content, string? culture) - => string.Join("/", content.AncestorsOrSelf().Reverse().Skip(hideTopLevelNodeFromPath ? 1 : 0).Select(c => c.UrlSegment(variantContextAccessor, culture))).EnsureStartsWith("/"); + { + return content.AncestorsOrSelf().All(c => c.IsPublished(culture)) + ? string.Join("/", content.AncestorsOrSelf().Reverse().Skip(hideTopLevelNodeFromPath ? 1 : 0).Select(c => c.UrlSegment(variantContextAccessor, culture))).EnsureStartsWith("/") + : "#"; + } var publishedUrlProvider = new Mock(); publishedUrlProvider @@ -239,12 +310,24 @@ public class ContentRouteBuilderTests : DeliveryApiTests return publishedUrlProvider.Object; } - private ApiContentRouteBuilder CreateApiContentRouteBuilder(bool hideTopLevelNodeFromPath) - => new( + private ApiContentRouteBuilder CreateApiContentRouteBuilder(bool hideTopLevelNodeFromPath, bool addTrailingSlash = false, bool isPreview = false, IPublishedSnapshotAccessor? publishedSnapshotAccessor = null) + { + var requestHandlerSettings = new RequestHandlerSettings { AddTrailingSlash = addTrailingSlash }; + var requestHandlerSettingsMonitorMock = new Mock>(); + requestHandlerSettingsMonitorMock.Setup(m => m.CurrentValue).Returns(requestHandlerSettings); + + var requestPreviewServiceMock = new Mock(); + requestPreviewServiceMock.Setup(m => m.IsPreview()).Returns(isPreview); + + publishedSnapshotAccessor ??= CreatePublishedSnapshotAccessorForRoute("#"); + + return CreateContentRouteBuilder( SetupPublishedUrlProvider(hideTopLevelNodeFromPath), CreateGlobalSettings(hideTopLevelNodeFromPath), - Mock.Of(), - Mock.Of()); + requestHandlerSettingsMonitor: requestHandlerSettingsMonitorMock.Object, + requestPreviewService: requestPreviewServiceMock.Object, + publishedSnapshotAccessor: publishedSnapshotAccessor); + } private IApiContentRoute? GetUnRoutableRoute(string publishedUrl, string routeById) { @@ -253,6 +336,19 @@ public class ContentRouteBuilderTests : DeliveryApiTests .Setup(p => p.GetUrl(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(publishedUrl); + var publishedSnapshotAccessor = CreatePublishedSnapshotAccessorForRoute(routeById); + var content = SetupVariantPublishedContent("The Content", Guid.NewGuid()); + + var builder = CreateContentRouteBuilder( + publishedUrlProviderMock.Object, + CreateGlobalSettings(), + publishedSnapshotAccessor: publishedSnapshotAccessor); + + return builder.Build(content); + } + + private IPublishedSnapshotAccessor CreatePublishedSnapshotAccessorForRoute(string routeById) + { var publishedContentCacheMock = new Mock(); publishedContentCacheMock .Setup(c => c.GetRouteById(It.IsAny(), It.IsAny())) @@ -269,14 +365,6 @@ public class ContentRouteBuilderTests : DeliveryApiTests .Setup(a => a.TryGetPublishedSnapshot(out publishedSnapshot)) .Returns(true); - var content = SetupVariantPublishedContent("The Content", Guid.NewGuid()); - - var builder = new ApiContentRouteBuilder( - publishedUrlProviderMock.Object, - CreateGlobalSettings(), - Mock.Of(), - publishedSnapshotAccessorMock.Object); - - return builder.Build(content); + return publishedSnapshotAccessorMock.Object; } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/DeliveryApiTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/DeliveryApiTests.cs index 3e8fae0de0..3a8adb6a1e 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/DeliveryApiTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/DeliveryApiTests.cs @@ -8,6 +8,8 @@ using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.PropertyEditors.DeliveryApi; +using Umbraco.Cms.Core.PublishedCache; +using Umbraco.Cms.Core.Routing; using Umbraco.Extensions; namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.DeliveryApi; @@ -102,8 +104,33 @@ public class DeliveryApiTests content.SetupGet(c => c.ContentType).Returns(contentType); content.SetupGet(c => c.Properties).Returns(properties); content.SetupGet(c => c.ItemType).Returns(contentType.ItemType); + content.Setup(c => c.IsPublished(It.IsAny())).Returns(true); } protected string DefaultUrlSegment(string name, string? culture = null) => $"{name.ToLowerInvariant().Replace(" ", "-")}{(culture.IsNullOrWhiteSpace() ? string.Empty : $"-{culture}")}"; + + protected ApiContentRouteBuilder CreateContentRouteBuilder( + IPublishedUrlProvider publishedUrlProvider, + IOptions globalSettings, + IVariationContextAccessor? variationContextAccessor = null, + IPublishedSnapshotAccessor? publishedSnapshotAccessor = null, + IRequestPreviewService? requestPreviewService = null, + IOptionsMonitor? requestHandlerSettingsMonitor = null) + { + if (requestHandlerSettingsMonitor == null) + { + var mock = new Mock>(); + mock.SetupGet(m => m.CurrentValue).Returns(new RequestHandlerSettings()); + requestHandlerSettingsMonitor = mock.Object; + } + + return new ApiContentRouteBuilder( + publishedUrlProvider, + globalSettings, + variationContextAccessor ?? Mock.Of(), + publishedSnapshotAccessor ?? Mock.Of(), + requestPreviewService ?? Mock.Of(), + requestHandlerSettingsMonitor); + } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs index 05f89173a2..b38aa33c8a 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs @@ -21,7 +21,7 @@ public class MultiNodeTreePickerValueConverterTests : PropertyValueConverterTest var contentNameProvider = new ApiContentNameProvider(); var apiUrProvider = new ApiMediaUrlProvider(PublishedUrlProvider); - routeBuilder = routeBuilder ?? new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of(), Mock.Of()); + routeBuilder = routeBuilder ?? CreateContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings()); return new MultiNodeTreePickerValueConverter( PublishedSnapshotAccessor, Mock.Of(), diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiUrlPickerValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiUrlPickerValueConverterTests.cs index ef10dd008b..818f55861d 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiUrlPickerValueConverterTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiUrlPickerValueConverterTests.cs @@ -8,7 +8,6 @@ using Umbraco.Cms.Core.Models.DeliveryApi; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.PropertyEditors.ValueConverters; -using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Serialization; using Umbraco.Cms.Core.Web; using Umbraco.Cms.Infrastructure.Serialization; @@ -260,7 +259,7 @@ public class MultiUrlPickerValueConverterTests : PropertyValueConverterTests private MultiUrlPickerValueConverter MultiUrlPickerValueConverter() { - var routeBuilder = new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of(), Mock.Of()); + var routeBuilder = CreateContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings()); return new MultiUrlPickerValueConverter( PublishedSnapshotAccessor, Mock.Of(), diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/OutputExpansionStrategyTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/OutputExpansionStrategyTests.cs index e6f81c5c36..1e12eef746 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/OutputExpansionStrategyTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/OutputExpansionStrategyTests.cs @@ -591,5 +591,5 @@ public class OutputExpansionStrategyTests : PropertyValueConverterTests return new PublishedElementPropertyBase(elementPropertyType, parent, false, PropertyCacheLevel.None); } - private IApiContentRouteBuilder ApiContentRouteBuilder() => new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of(), Mock.Of()); + private IApiContentRouteBuilder ApiContentRouteBuilder() => CreateContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings()); }