From c00e86e0eae60e56369aab0ecd2185aa35315f26 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Mon, 20 Jan 2025 22:27:10 +0100 Subject: [PATCH] Fix various routing and preview issues for the Delivery API in V15 (#18036) * Fix various routing and preview issues for the Delivery API in V15 * Fix breaking change in ctor * Fix ambigious constructors --------- Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --- .../Querying/QueryOptionBase.cs | 44 ++++++++-- .../Querying/Selectors/AncestorsSelector.cs | 73 ++++++++++------- .../Querying/Selectors/ChildrenSelector.cs | 29 ++++++- .../Querying/Selectors/DescendantsSelector.cs | 29 ++++++- .../Services/RequestStartItemProvider.cs | 5 +- .../DeliveryApi/ApiContentRouteBuilder.cs | 2 +- .../DeliveryApi/ApiDocumentUrlService.cs | 34 ++++++++ .../DeliveryApi/ApiPublishedContentCache.cs | 80 ++++++++----------- .../DeliveryApi/IApiDocumentUrlService.cs | 6 ++ .../UmbracoBuilder.CoreServices.cs | 1 + .../DeliveryApi/ApiDocumentUrlServiceTests.cs | 46 +++++++++++ .../DeliveryApi/PublishedContentCacheTests.cs | 67 ++++++++++------ .../Selectors/AncestorsSelectorTests.cs | 75 +++++++++++++++++ .../Selectors/ChildrenSelectorTests.cs | 56 +++++++++++++ .../Selectors/DescendantsSelectorTests.cs | 56 +++++++++++++ 15 files changed, 491 insertions(+), 112 deletions(-) create mode 100644 src/Umbraco.Core/DeliveryApi/ApiDocumentUrlService.cs create mode 100644 src/Umbraco.Core/DeliveryApi/IApiDocumentUrlService.cs create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ApiDocumentUrlServiceTests.cs create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/Selectors/AncestorsSelectorTests.cs create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/Selectors/ChildrenSelectorTests.cs create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/Selectors/DescendantsSelectorTests.cs diff --git a/src/Umbraco.Cms.Api.Delivery/Querying/QueryOptionBase.cs b/src/Umbraco.Cms.Api.Delivery/Querying/QueryOptionBase.cs index 1576d0037f..2b47ea166e 100644 --- a/src/Umbraco.Cms.Api.Delivery/Querying/QueryOptionBase.cs +++ b/src/Umbraco.Cms.Api.Delivery/Querying/QueryOptionBase.cs @@ -1,5 +1,6 @@ +using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.DeliveryApi; -using Umbraco.Cms.Core.Models.PublishedContent; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Extensions; @@ -7,16 +8,44 @@ namespace Umbraco.Cms.Api.Delivery.Querying; public abstract class QueryOptionBase { - private readonly IPublishedContentCache _publishedContentCache; private readonly IRequestRoutingService _requestRoutingService; + private readonly IRequestPreviewService _requestPreviewService; + private readonly IRequestCultureService _requestCultureService; + private readonly IApiDocumentUrlService _apiDocumentUrlService; - + [Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")] public QueryOptionBase( IPublishedContentCache publishedContentCache, IRequestRoutingService requestRoutingService) + : this( + requestRoutingService, + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + [Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")] + public QueryOptionBase( + IPublishedContentCache publishedContentCache, + IRequestRoutingService requestRoutingService, + IRequestPreviewService requestPreviewService, + IRequestCultureService requestCultureService, + IApiDocumentUrlService apiDocumentUrlService) + : this(requestRoutingService, requestPreviewService, requestCultureService, apiDocumentUrlService) + { + } + + public QueryOptionBase( + IRequestRoutingService requestRoutingService, + IRequestPreviewService requestPreviewService, + IRequestCultureService requestCultureService, + IApiDocumentUrlService apiDocumentUrlService) { - _publishedContentCache = publishedContentCache; _requestRoutingService = requestRoutingService; + _requestPreviewService = requestPreviewService; + _requestCultureService = requestCultureService; + _apiDocumentUrlService = apiDocumentUrlService; } protected Guid? GetGuidFromQuery(string queryStringValue) @@ -33,8 +62,9 @@ public abstract class QueryOptionBase // Check if the passed value is a path of a content item var contentRoute = _requestRoutingService.GetContentRoute(queryStringValue); - IPublishedContent? contentItem = _publishedContentCache.GetByRoute(contentRoute); - - return contentItem?.Key; + return _apiDocumentUrlService.GetDocumentKeyByRoute( + contentRoute, + _requestCultureService.GetRequestedCulture(), + _requestPreviewService.IsPreview()); } } diff --git a/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/AncestorsSelector.cs b/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/AncestorsSelector.cs index 3f3a18f00c..7a3a793118 100644 --- a/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/AncestorsSelector.cs +++ b/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/AncestorsSelector.cs @@ -2,44 +2,77 @@ using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Api.Delivery.Indexing.Selectors; using Umbraco.Cms.Core.DeliveryApi; using Umbraco.Cms.Core.DependencyInjection; -using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Services.Navigation; -using Umbraco.Extensions; namespace Umbraco.Cms.Api.Delivery.Querying.Selectors; public sealed class AncestorsSelector : QueryOptionBase, ISelectorHandler { - private readonly IPublishedContentCache _publishedContentCache; private readonly IDocumentNavigationQueryService _navigationQueryService; - private readonly IRequestPreviewService _requestPreviewService; private const string AncestorsSpecifier = "ancestors:"; + [Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")] public AncestorsSelector( IPublishedContentCache publishedContentCache, IRequestRoutingService requestRoutingService, IDocumentNavigationQueryService navigationQueryService, IRequestPreviewService requestPreviewService) - : base(publishedContentCache, requestRoutingService) + : this( + requestRoutingService, + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + navigationQueryService) { - _publishedContentCache = publishedContentCache; - _navigationQueryService = navigationQueryService; - _requestPreviewService = requestPreviewService; } - [Obsolete("Use the constructor that takes all parameters. Scheduled for removal in V17.")] + [Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")] public AncestorsSelector( IPublishedContentCache publishedContentCache, IRequestRoutingService requestRoutingService, IDocumentNavigationQueryService navigationQueryService) - : this(publishedContentCache, requestRoutingService, navigationQueryService, StaticServiceProvider.Instance.GetRequiredService()) + : this( + requestRoutingService, + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + navigationQueryService) { } [Obsolete("Use the constructor that takes all parameters. Scheduled for removal in V17.")] public AncestorsSelector(IPublishedContentCache publishedContentCache, IRequestRoutingService requestRoutingService) - : this(publishedContentCache, requestRoutingService, StaticServiceProvider.Instance.GetRequiredService()) + : this( + requestRoutingService, + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + public AncestorsSelector( + IRequestRoutingService requestRoutingService, + IRequestPreviewService requestPreviewService, + IRequestCultureService requestCultureService, + IApiDocumentUrlService apiDocumentUrlService, + IDocumentNavigationQueryService navigationQueryService) + : base(requestRoutingService, requestPreviewService, requestCultureService, apiDocumentUrlService) + => _navigationQueryService = navigationQueryService; + + [Obsolete("Use the constructor that takes all parameters. Scheduled for removal in V17.")] + public AncestorsSelector( + IRequestRoutingService requestRoutingService, + IPublishedContentCache publishedContentCache, + IRequestPreviewService requestPreviewService, + IRequestCultureService requestCultureService, + IApiDocumentUrlService apiDocumentUrlService, + IDocumentNavigationQueryService navigationQueryService) + : this(requestRoutingService, requestPreviewService, requestCultureService, apiDocumentUrlService, navigationQueryService) { } @@ -53,7 +86,7 @@ public sealed class AncestorsSelector : QueryOptionBase, ISelectorHandler var fieldValue = selector[AncestorsSpecifier.Length..]; Guid? id = GetGuidFromQuery(fieldValue); - if (id is null) + if (id is null || _navigationQueryService.TryGetAncestorsKeys(id.Value, out IEnumerable ancestorKeys) is false) { // Setting the Value to "" since that would yield no results. // It won't be appropriate to return null here since if we reached this, @@ -65,24 +98,10 @@ public sealed class AncestorsSelector : QueryOptionBase, ISelectorHandler }; } - IPublishedContent? contentItem = _publishedContentCache.GetById(_requestPreviewService.IsPreview(), id.Value); - - if (contentItem is null) - { - // no such content item, make sure the selector does not yield any results - return new SelectorOption - { - FieldName = AncestorsSelectorIndexer.FieldName, - Values = Array.Empty() - }; - } - - var ancestorKeys = contentItem.Ancestors(_publishedContentCache, _navigationQueryService).Select(a => a.Key.ToString("D")).ToArray(); - return new SelectorOption { FieldName = AncestorsSelectorIndexer.FieldName, - Values = ancestorKeys + Values = ancestorKeys.Select(key => key.ToString("D")).ToArray() }; } } diff --git a/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/ChildrenSelector.cs b/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/ChildrenSelector.cs index 9392ce8e02..ba986e0812 100644 --- a/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/ChildrenSelector.cs +++ b/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/ChildrenSelector.cs @@ -1,5 +1,7 @@ +using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Api.Delivery.Indexing.Selectors; using Umbraco.Cms.Core.DeliveryApi; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Extensions; @@ -9,8 +11,33 @@ public sealed class ChildrenSelector : QueryOptionBase, ISelectorHandler { private const string ChildrenSpecifier = "children:"; + [Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")] public ChildrenSelector(IPublishedContentCache publishedContentCache, IRequestRoutingService requestRoutingService) - : base(publishedContentCache, requestRoutingService) + : this( + requestRoutingService, + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + [Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")] + public ChildrenSelector( + IPublishedContentCache publishedContentCache, + IRequestRoutingService requestRoutingService, + IRequestPreviewService requestPreviewService, + IRequestCultureService requestCultureService, + IApiDocumentUrlService apiDocumentUrlService) + : this(requestRoutingService, requestPreviewService, requestCultureService, apiDocumentUrlService) + { + } + + public ChildrenSelector( + IRequestRoutingService requestRoutingService, + IRequestPreviewService requestPreviewService, + IRequestCultureService requestCultureService, + IApiDocumentUrlService apiDocumentUrlService) + : base(requestRoutingService, requestPreviewService, requestCultureService, apiDocumentUrlService) { } diff --git a/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/DescendantsSelector.cs b/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/DescendantsSelector.cs index 2a7512746e..7ce0c066c4 100644 --- a/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/DescendantsSelector.cs +++ b/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/DescendantsSelector.cs @@ -1,5 +1,7 @@ +using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Api.Delivery.Indexing.Selectors; using Umbraco.Cms.Core.DeliveryApi; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Extensions; @@ -9,8 +11,33 @@ public sealed class DescendantsSelector : QueryOptionBase, ISelectorHandler { private const string DescendantsSpecifier = "descendants:"; + [Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")] public DescendantsSelector(IPublishedContentCache publishedContentCache, IRequestRoutingService requestRoutingService) - : base(publishedContentCache, requestRoutingService) + : this( + requestRoutingService, + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + [Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")] + public DescendantsSelector( + IPublishedContentCache publishedContentCache, + IRequestRoutingService requestRoutingService, + IRequestPreviewService requestPreviewService, + IRequestCultureService requestCultureService, + IApiDocumentUrlService apiDocumentUrlService) + : this(requestRoutingService, requestPreviewService, requestCultureService, apiDocumentUrlService) + { + } + + public DescendantsSelector( + IRequestRoutingService requestRoutingService, + IRequestPreviewService requestPreviewService, + IRequestCultureService requestCultureService, + IApiDocumentUrlService apiDocumentUrlService) + : base(requestRoutingService, requestPreviewService, requestCultureService, apiDocumentUrlService) { } diff --git a/src/Umbraco.Cms.Api.Delivery/Services/RequestStartItemProvider.cs b/src/Umbraco.Cms.Api.Delivery/Services/RequestStartItemProvider.cs index e79a2cbdd7..92aff37011 100644 --- a/src/Umbraco.Cms.Api.Delivery/Services/RequestStartItemProvider.cs +++ b/src/Umbraco.Cms.Api.Delivery/Services/RequestStartItemProvider.cs @@ -49,9 +49,8 @@ internal sealed class RequestStartItemProvider : RequestHeaderHandler, IRequestS _documentNavigationQueryService.TryGetRootKeys(out IEnumerable rootKeys); IEnumerable rootContent = rootKeys - .Select(_publishedContentCache.GetById) - .WhereNotNull() - .Where(x => x.IsPublished() != _requestPreviewService.IsPreview()); + .Select(rootKey => _publishedContentCache.GetById(_requestPreviewService.IsPreview(), rootKey)) + .WhereNotNull(); _requestedStartContent = Guid.TryParse(headerValue, out Guid key) ? rootContent.FirstOrDefault(c => c.Key == key) diff --git a/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs b/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs index 221a1a1af5..c5d93d979c 100644 --- a/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs +++ b/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs @@ -95,7 +95,7 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder { // 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) + if (isPreview && _publishStatusQueryService.IsDocumentPublished(content.Key, culture ?? string.Empty) is false) { return ContentPreviewPath(content); } diff --git a/src/Umbraco.Core/DeliveryApi/ApiDocumentUrlService.cs b/src/Umbraco.Core/DeliveryApi/ApiDocumentUrlService.cs new file mode 100644 index 0000000000..f17eba49d0 --- /dev/null +++ b/src/Umbraco.Core/DeliveryApi/ApiDocumentUrlService.cs @@ -0,0 +1,34 @@ +using Umbraco.Cms.Core.Services; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Core.DeliveryApi; + +public sealed class ApiDocumentUrlService : IApiDocumentUrlService +{ + private readonly IDocumentUrlService _documentUrlService; + + public ApiDocumentUrlService(IDocumentUrlService documentUrlService) + => _documentUrlService = documentUrlService; + + public Guid? GetDocumentKeyByRoute(string route, string? culture, bool preview) + { + // Handle the nasty logic with domain document ids in front of paths. + int? documentStartNodeId = null; + if (route.StartsWith('/') is false) + { + var index = route.IndexOf('/'); + + if (index > -1 && int.TryParse(route.Substring(0, index), out var nodeId)) + { + documentStartNodeId = nodeId; + route = route.Substring(index); + } + } + + return _documentUrlService.GetDocumentKeyByRoute( + route, + culture.NullOrWhiteSpaceAsNull(), + documentStartNodeId, + preview); + } +} diff --git a/src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs b/src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs index 5ee6f8d379..e5996595fc 100644 --- a/src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs +++ b/src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs @@ -13,20 +13,43 @@ public sealed class ApiPublishedContentCache : IApiPublishedContentCache { private readonly IRequestPreviewService _requestPreviewService; private readonly IRequestCultureService _requestCultureService; - private readonly IDocumentUrlService _documentUrlService; + private readonly IApiDocumentUrlService _apiDocumentUrlService; private readonly IPublishedContentCache _publishedContentCache; private DeliveryApiSettings _deliveryApiSettings; + [Obsolete("Use the non-obsolete constructor. Will be removed in V17.")] public ApiPublishedContentCache( IRequestPreviewService requestPreviewService, IRequestCultureService requestCultureService, IOptionsMonitor deliveryApiSettings, IDocumentUrlService documentUrlService, IPublishedContentCache publishedContentCache) + : this(requestPreviewService, requestCultureService, deliveryApiSettings, StaticServiceProvider.Instance.GetRequiredService(), publishedContentCache) + { + } + + [Obsolete("Use the non-obsolete constructor. Will be removed in V17.")] + public ApiPublishedContentCache( + IRequestPreviewService requestPreviewService, + IRequestCultureService requestCultureService, + IOptionsMonitor deliveryApiSettings, + IDocumentUrlService documentUrlService, + IApiDocumentUrlService apiDocumentUrlService, + IPublishedContentCache publishedContentCache) + : this(requestPreviewService, requestCultureService, deliveryApiSettings, apiDocumentUrlService, publishedContentCache) + { + } + + public ApiPublishedContentCache( + IRequestPreviewService requestPreviewService, + IRequestCultureService requestCultureService, + IOptionsMonitor deliveryApiSettings, + IApiDocumentUrlService apiDocumentUrlService, + IPublishedContentCache publishedContentCache) { _requestPreviewService = requestPreviewService; _requestCultureService = requestCultureService; - _documentUrlService = documentUrlService; + _apiDocumentUrlService = apiDocumentUrlService; _publishedContentCache = publishedContentCache; _deliveryApiSettings = deliveryApiSettings.CurrentValue; deliveryApiSettings.OnChange(settings => _deliveryApiSettings = settings); @@ -36,25 +59,11 @@ public sealed class ApiPublishedContentCache : IApiPublishedContentCache { var isPreviewMode = _requestPreviewService.IsPreview(); - // Handle the nasty logic with domain document ids in front of paths. - int? documentStartNodeId = null; - if (route.StartsWith("/") is false) - { - var index = route.IndexOf('/'); - - if (index > -1 && int.TryParse(route.Substring(0, index), out var nodeId)) - { - documentStartNodeId = nodeId; - route = route.Substring(index); - } - } - - Guid? documentKey = _documentUrlService.GetDocumentKeyByRoute( + Guid? documentKey = _apiDocumentUrlService.GetDocumentKeyByRoute( route, _requestCultureService.GetRequestedCulture(), - documentStartNodeId, - _requestPreviewService.IsPreview() - ); + _requestPreviewService.IsPreview()); + IPublishedContent? content = documentKey.HasValue ? await _publishedContentCache.GetByIdAsync(documentKey.Value, isPreviewMode) : null; @@ -66,35 +75,11 @@ public sealed class ApiPublishedContentCache : IApiPublishedContentCache { var isPreviewMode = _requestPreviewService.IsPreview(); - - // Handle the nasty logic with domain document ids in front of paths. - int? documentStartNodeId = null; - if (route.StartsWith("/") is false) - { - var index = route.IndexOf('/'); - - if (index > -1 && int.TryParse(route.Substring(0, index), out var nodeId)) - { - documentStartNodeId = nodeId; - route = route.Substring(index); - } - } - - var requestCulture = _requestCultureService.GetRequestedCulture(); - - if (requestCulture?.Trim().Length <= 0) - { - // documentUrlService does not like empty strings - // todo: align culture null vs empty string behaviour across the codebase - requestCulture = null; - } - - Guid? documentKey = _documentUrlService.GetDocumentKeyByRoute( + Guid? documentKey = _apiDocumentUrlService.GetDocumentKeyByRoute( route, - requestCulture, - documentStartNodeId, - _requestPreviewService.IsPreview() - ); + _requestCultureService.GetRequestedCulture(), + _requestPreviewService.IsPreview()); + IPublishedContent? content = documentKey.HasValue ? _publishedContentCache.GetById(isPreviewMode, documentKey.Value) : null; @@ -113,6 +98,7 @@ public sealed class ApiPublishedContentCache : IApiPublishedContentCache IPublishedContent? content = _publishedContentCache.GetById(_requestPreviewService.IsPreview(), contentId); return ContentOrNullIfDisallowed(content); } + public async Task> GetByIdsAsync(IEnumerable contentIds) { var isPreviewMode = _requestPreviewService.IsPreview(); diff --git a/src/Umbraco.Core/DeliveryApi/IApiDocumentUrlService.cs b/src/Umbraco.Core/DeliveryApi/IApiDocumentUrlService.cs new file mode 100644 index 0000000000..7792fa8a47 --- /dev/null +++ b/src/Umbraco.Core/DeliveryApi/IApiDocumentUrlService.cs @@ -0,0 +1,6 @@ +namespace Umbraco.Cms.Core.DeliveryApi; + +public interface IApiDocumentUrlService +{ + Guid? GetDocumentKeyByRoute(string route, string? culture, bool preview); +} diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index bd101ad37b..91b23d123a 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -449,6 +449,7 @@ public static partial class UmbracoBuilderExtensions builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); + builder.Services.AddSingleton(); return builder; } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ApiDocumentUrlServiceTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ApiDocumentUrlServiceTests.cs new file mode 100644 index 0000000000..a79bf59941 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ApiDocumentUrlServiceTests.cs @@ -0,0 +1,46 @@ +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core.DeliveryApi; +using Umbraco.Cms.Core.Services; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.DeliveryApi; + +[TestFixture] +public class ApiDocumentUrlServiceTests +{ + [Test] + public void Can_Resolve_Document_Key_With_Start_Node() + { + var documentKey = Guid.NewGuid(); + var documentUrlServiceMock = new Mock(); + documentUrlServiceMock + .Setup(m => m.GetDocumentKeyByRoute( + "/some/where", + It.IsAny(), + 1234, + false)) + .Returns(documentKey); + + var apiDocumentUrlService = new ApiDocumentUrlService(documentUrlServiceMock.Object); + var result = apiDocumentUrlService.GetDocumentKeyByRoute("1234/some/where", null, false); + Assert.AreEqual(documentKey, result); + } + + [Test] + public void Can_Resolve_Document_Key_Without_Start_Node() + { + var documentKey = Guid.NewGuid(); + var documentUrlServiceMock = new Mock(); + documentUrlServiceMock + .Setup(m => m.GetDocumentKeyByRoute( + "/some/where", + It.IsAny(), + null, + false)) + .Returns(documentKey); + + var apiDocumentUrlService = new ApiDocumentUrlService(documentUrlServiceMock.Object); + var result = apiDocumentUrlService.GetDocumentKeyByRoute("/some/where", null, false); + Assert.AreEqual(documentKey, result); + } +} diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs index 1480210534..f9d4f54659 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs @@ -6,7 +6,6 @@ using Umbraco.Cms.Core.DeliveryApi; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Services; -using Umbraco.Cms.Infrastructure.HybridCache; namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.DeliveryApi; @@ -17,8 +16,9 @@ public class PublishedContentCacheTests : DeliveryApiTests private readonly Guid _contentTwoId = Guid.Parse("4EF11E1E-FB50-4627-8A86-E10ED6F4DCE4"); + private readonly Guid _contentThreeId = Guid.Parse("013387EE-57AF-4ABD-B03C-F991B0722CCA"); + private IPublishedContentCache _contentCache; - private IPublishedContentCache _contentCacheMock; private IDocumentUrlService _documentUrlService; [SetUp] @@ -34,37 +34,41 @@ public class PublishedContentCacheTests : DeliveryApiTests var contentTwoMock = new Mock(); ConfigurePublishedContentMock(contentTwoMock, _contentTwoId, "Content Two", "content-two", contentTypeTwoMock.Object, Array.Empty()); + var contentTypeThreeMock = new Mock(); + contentTypeThreeMock.SetupGet(m => m.Alias).Returns("theThirdContentType"); + var contentThreeMock = new Mock(); + ConfigurePublishedContentMock(contentThreeMock, _contentThreeId, "Content Three", "content-three", contentTypeThreeMock.Object, Array.Empty()); + var documentUrlService = new Mock(); documentUrlService - .Setup(x => x.GetDocumentKeyByRoute("content-one", It.IsAny(), It.IsAny(), It.IsAny())) + .Setup(x => x.GetDocumentKeyByRoute("/content-one", It.IsAny(), It.IsAny(), It.IsAny())) .Returns(_contentOneId); documentUrlService - .Setup(x => x.GetDocumentKeyByRoute("content-two", It.IsAny(), It.IsAny(), It.IsAny())) + .Setup(x => x.GetDocumentKeyByRoute("/content-two", It.IsAny(), It.IsAny(), It.IsAny())) .Returns(_contentTwoId); + documentUrlService + .Setup(x => x.GetDocumentKeyByRoute("/content-three", It.IsAny(), 1234, It.IsAny())) + .Returns(_contentThreeId); var contentCacheMock = new Mock(); - contentCacheMock - .Setup(m => m.GetByRoute(It.IsAny(), "content-one", null, null)) - .Returns(contentOneMock.Object); contentCacheMock .Setup(m => m.GetById(It.IsAny(), _contentOneId)) .Returns(contentOneMock.Object); - contentCacheMock - .Setup(m => m.GetByRoute(It.IsAny(), "content-two", null, null)) - .Returns(contentTwoMock.Object); contentCacheMock .Setup(m => m.GetById(It.IsAny(), _contentTwoId)) .Returns(contentTwoMock.Object); + contentCacheMock + .Setup(m => m.GetById(It.IsAny(), _contentThreeId)) + .Returns(contentThreeMock.Object); _contentCache = contentCacheMock.Object; - _contentCacheMock = contentCacheMock.Object; _documentUrlService = documentUrlService.Object; } [Test] public void PublishedContentCache_CanGetById() { - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(), _documentUrlService, _contentCacheMock); + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(), CreateApiDocumentUrlService(), _contentCache); var content = publishedContentCache.GetById(_contentOneId); Assert.IsNotNull(content); Assert.AreEqual(_contentOneId, content.Key); @@ -75,18 +79,29 @@ public class PublishedContentCacheTests : DeliveryApiTests [Test] public void PublishedContentCache_CanGetByRoute() { - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(), _documentUrlService, _contentCacheMock); - var content = publishedContentCache.GetByRoute("content-two"); + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(), CreateApiDocumentUrlService(), _contentCache); + var content = publishedContentCache.GetByRoute("/content-two"); Assert.IsNotNull(content); Assert.AreEqual(_contentTwoId, content.Key); Assert.AreEqual("content-two", content.UrlSegment); Assert.AreEqual("theOtherContentType", content.ContentType.Alias); } + [Test] + public void PublishedContentCache_CanGetByRoute_WithStartNodeIdPrefix() + { + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(), CreateApiDocumentUrlService(), _contentCache); + var content = publishedContentCache.GetByRoute("1234/content-three"); + Assert.IsNotNull(content); + Assert.AreEqual(_contentThreeId, content.Key); + Assert.AreEqual("content-three", content.UrlSegment); + Assert.AreEqual("theThirdContentType", content.ContentType.Alias); + } + [Test] public void PublishedContentCache_CanGetByIds() { - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(), _documentUrlService, _contentCacheMock); + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(), CreateApiDocumentUrlService(), _contentCache); var content = publishedContentCache.GetByIds(new[] { _contentOneId, _contentTwoId }).ToArray(); Assert.AreEqual(2, content.Length); Assert.AreEqual(_contentOneId, content.First().Key); @@ -98,7 +113,7 @@ public class PublishedContentCacheTests : DeliveryApiTests public void PublishedContentCache_GetById_SupportsDenyList(bool denied) { var denyList = denied ? new[] { "theOtherContentType" } : null; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), _documentUrlService, _contentCacheMock); + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache); var content = publishedContentCache.GetById(_contentTwoId); if (denied) @@ -116,8 +131,8 @@ public class PublishedContentCacheTests : DeliveryApiTests public void PublishedContentCache_GetByRoute_SupportsDenyList(bool denied) { var denyList = denied ? new[] { "theContentType" } : null; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), _documentUrlService, _contentCacheMock); - var content = publishedContentCache.GetByRoute("content-one"); + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache); + var content = publishedContentCache.GetByRoute("/content-one"); if (denied) { @@ -134,7 +149,7 @@ public class PublishedContentCacheTests : DeliveryApiTests public void PublishedContentCache_GetByIds_SupportsDenyList(string deniedContentType) { var denyList = new[] { deniedContentType }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), _documentUrlService, _contentCacheMock); + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache); var content = publishedContentCache.GetByIds(new[] { _contentOneId, _contentTwoId }).ToArray(); Assert.AreEqual(1, content.Length); @@ -152,7 +167,7 @@ public class PublishedContentCacheTests : DeliveryApiTests public void PublishedContentCache_GetById_CanRetrieveContentTypesOutsideTheDenyList() { var denyList = new[] { "theContentType" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), _documentUrlService, _contentCacheMock); + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache); var content = publishedContentCache.GetById(_contentTwoId); Assert.IsNotNull(content); Assert.AreEqual(_contentTwoId, content.Key); @@ -164,8 +179,8 @@ public class PublishedContentCacheTests : DeliveryApiTests public void PublishedContentCache_GetByRoute_CanRetrieveContentTypesOutsideTheDenyList() { var denyList = new[] { "theOtherContentType" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), _documentUrlService, _contentCacheMock); - var content = publishedContentCache.GetByRoute("content-one"); + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache); + var content = publishedContentCache.GetByRoute("/content-one"); Assert.IsNotNull(content); Assert.AreEqual(_contentOneId, content.Key); Assert.AreEqual("content-one", content.UrlSegment); @@ -176,7 +191,7 @@ public class PublishedContentCacheTests : DeliveryApiTests public void PublishedContentCache_GetByIds_CanDenyAllRequestedContent() { var denyList = new[] { "theContentType", "theOtherContentType" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), _documentUrlService, _contentCacheMock); + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache); var content = publishedContentCache.GetByIds(new[] { _contentOneId, _contentTwoId }).ToArray(); Assert.IsEmpty(content); } @@ -185,8 +200,8 @@ public class PublishedContentCacheTests : DeliveryApiTests public void PublishedContentCache_DenyListIsCaseInsensitive() { var denyList = new[] { "THEcontentTYPE" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), _documentUrlService, _contentCacheMock); - var content = publishedContentCache.GetByRoute("content-one"); + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateRequestCultureService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache); + var content = publishedContentCache.GetByRoute("/content-one"); Assert.IsNull(content); } @@ -211,4 +226,6 @@ public class PublishedContentCacheTests : DeliveryApiTests deliveryApiOptionsMonitorMock.SetupGet(s => s.CurrentValue).Returns(deliveryApiSettings); return deliveryApiOptionsMonitorMock.Object; } + + private IApiDocumentUrlService CreateApiDocumentUrlService() => new ApiDocumentUrlService(_documentUrlService); } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/Selectors/AncestorsSelectorTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/Selectors/AncestorsSelectorTests.cs new file mode 100644 index 0000000000..19d1359308 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/Selectors/AncestorsSelectorTests.cs @@ -0,0 +1,75 @@ +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Api.Delivery.Querying.Selectors; +using Umbraco.Cms.Core.DeliveryApi; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.Navigation; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.DeliveryApi.Selectors; + +[TestFixture] +public class AncestorsSelectorTests +{ + private readonly Guid _documentKey = Guid.NewGuid(); + private IDocumentNavigationQueryService _documentNavigationQueryService; + + [SetUp] + public void SetUp() + { + IEnumerable ancestorKeys = + [ + new("863e10d5-b0f8-421d-902d-5e4d1bd8e780"), + new("11fc9bdc-8366-4a6b-a9c2-6b8b2717c4b8") + ]; + var documentNavigationQueryServiceMock = new Mock(); + documentNavigationQueryServiceMock + .Setup(m => m.TryGetAncestorsKeys(_documentKey, out ancestorKeys)) + .Returns(true); + _documentNavigationQueryService = documentNavigationQueryServiceMock.Object; + } + + [TestCase(null)] + [TestCase(1234)] + public void Can_Build_Selector_Option_For_Path(int? documentStartNodeId) + { + var documentUrlServiceMock = new Mock(); + documentUrlServiceMock + .Setup(m => m.GetDocumentKeyByRoute( + "/some/where", + It.IsAny(), + documentStartNodeId, + false)) + .Returns(_documentKey); + + var requestRoutingServiceMock = new Mock(); + requestRoutingServiceMock.Setup(m => m.GetContentRoute("/some/where")).Returns($"{documentStartNodeId}/some/where"); + + var subject = new AncestorsSelector( + requestRoutingServiceMock.Object, + Mock.Of(), + Mock.Of(), + new ApiDocumentUrlService(documentUrlServiceMock.Object), + _documentNavigationQueryService); + + var result = subject.BuildSelectorOption("ancestors:/some/where"); + Assert.AreEqual(2, result.Values.Length); + Assert.AreEqual("863e10d5-b0f8-421d-902d-5e4d1bd8e780", result.Values[0]); + Assert.AreEqual("11fc9bdc-8366-4a6b-a9c2-6b8b2717c4b8", result.Values[1]); + } + + [Test] + public void Can_Build_Selector_Option_For_Id() + { + var subject = new AncestorsSelector( + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + _documentNavigationQueryService); + + var result = subject.BuildSelectorOption($"ancestors:{_documentKey:D}"); + Assert.AreEqual(2, result.Values.Length); + Assert.AreEqual("863e10d5-b0f8-421d-902d-5e4d1bd8e780", result.Values[0]); + Assert.AreEqual("11fc9bdc-8366-4a6b-a9c2-6b8b2717c4b8", result.Values[1]); + } +} diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/Selectors/ChildrenSelectorTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/Selectors/ChildrenSelectorTests.cs new file mode 100644 index 0000000000..33a3912de0 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/Selectors/ChildrenSelectorTests.cs @@ -0,0 +1,56 @@ +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Api.Delivery.Querying.Selectors; +using Umbraco.Cms.Core.DeliveryApi; +using Umbraco.Cms.Core.Services; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.DeliveryApi.Selectors; + +[TestFixture] +public class ChildrenSelectorTests +{ + [TestCase(null)] + [TestCase(1234)] + public void Can_Build_Selector_Option_For_Path(int? documentStartNodeId) + { + var documentKey = Guid.NewGuid(); + + var documentUrlServiceMock = new Mock(); + documentUrlServiceMock + .Setup(m => m.GetDocumentKeyByRoute( + "/some/where", + It.IsAny(), + documentStartNodeId, + false)) + .Returns(documentKey); + + var requestRoutingServiceMock = new Mock(); + requestRoutingServiceMock.Setup(m => m.GetContentRoute("/some/where")).Returns($"{documentStartNodeId}/some/where"); + + var subject = new ChildrenSelector( + requestRoutingServiceMock.Object, + Mock.Of(), + Mock.Of(), + new ApiDocumentUrlService(documentUrlServiceMock.Object)); + + var result = subject.BuildSelectorOption("children:/some/where"); + Assert.AreEqual(1, result.Values.Length); + Assert.AreEqual(documentKey.ToString("D"), result.Values[0]); + } + + [Test] + public void Can_Build_Selector_Option_For_Id() + { + var documentKey = Guid.NewGuid(); + + var subject = new ChildrenSelector( + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of()); + + var result = subject.BuildSelectorOption($"children:{documentKey:D}"); + Assert.AreEqual(1, result.Values.Length); + Assert.AreEqual(documentKey.ToString("D"), result.Values[0]); + } +} diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/Selectors/DescendantsSelectorTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/Selectors/DescendantsSelectorTests.cs new file mode 100644 index 0000000000..d322dc898a --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/Selectors/DescendantsSelectorTests.cs @@ -0,0 +1,56 @@ +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Api.Delivery.Querying.Selectors; +using Umbraco.Cms.Core.DeliveryApi; +using Umbraco.Cms.Core.Services; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.DeliveryApi.Selectors; + +[TestFixture] +public class DescendantsSelectorTests +{ + [TestCase(null)] + [TestCase(1234)] + public void Can_Build_Selector_Option_For_Path(int? documentStartNodeId) + { + var documentKey = Guid.NewGuid(); + + var documentUrlServiceMock = new Mock(); + documentUrlServiceMock + .Setup(m => m.GetDocumentKeyByRoute( + "/some/where", + It.IsAny(), + documentStartNodeId, + false)) + .Returns(documentKey); + + var requestRoutingServiceMock = new Mock(); + requestRoutingServiceMock.Setup(m => m.GetContentRoute("/some/where")).Returns($"{documentStartNodeId}/some/where"); + + var subject = new DescendantsSelector( + requestRoutingServiceMock.Object, + Mock.Of(), + Mock.Of(), + new ApiDocumentUrlService(documentUrlServiceMock.Object)); + + var result = subject.BuildSelectorOption("descendants:/some/where"); + Assert.AreEqual(1, result.Values.Length); + Assert.AreEqual(documentKey.ToString("D"), result.Values[0]); + } + + [Test] + public void Can_Build_Selector_Option_For_Id() + { + var documentKey = Guid.NewGuid(); + + var subject = new DescendantsSelector( + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of()); + + var result = subject.BuildSelectorOption($"descendants:{documentKey:D}"); + Assert.AreEqual(1, result.Values.Length); + Assert.AreEqual(documentKey.ToString("D"), result.Values[0]); + } +}