diff --git a/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs b/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs index 7fe6aec063..e045ec9d75 100644 --- a/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs +++ b/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs @@ -2,6 +2,7 @@ using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Models.DeliveryApi; using Umbraco.Cms.Core.Models.PublishedContent; +using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Routing; using Umbraco.Extensions; @@ -12,11 +13,17 @@ public class ApiContentRouteBuilder : IApiContentRouteBuilder private readonly IPublishedUrlProvider _publishedUrlProvider; private readonly GlobalSettings _globalSettings; private readonly IVariationContextAccessor _variationContextAccessor; + private readonly IPublishedSnapshotAccessor _publishedSnapshotAccessor; - public ApiContentRouteBuilder(IPublishedUrlProvider publishedUrlProvider, IOptions globalSettings, IVariationContextAccessor variationContextAccessor) + public ApiContentRouteBuilder( + IPublishedUrlProvider publishedUrlProvider, + IOptions globalSettings, + IVariationContextAccessor variationContextAccessor, + IPublishedSnapshotAccessor publishedSnapshotAccessor) { _publishedUrlProvider = publishedUrlProvider; _variationContextAccessor = variationContextAccessor; + _publishedSnapshotAccessor = publishedSnapshotAccessor; _globalSettings = globalSettings.Value; } @@ -30,8 +37,17 @@ public class ApiContentRouteBuilder : IApiContentRouteBuilder IPublishedContent root = content.Root(); var rootPath = root.UrlSegment(_variationContextAccessor, culture) ?? string.Empty; - var contentPath = _publishedUrlProvider.GetUrl(content, UrlMode.Relative, culture).EnsureStartsWith("/"); + 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 + // the URL provider resolves the content URL as empty string or "#". since the Delivery API handles routing explicitly, + // we can perform fallback to the content route. + if (contentPath.IsNullOrWhiteSpace() || "#".Equals(contentPath)) + { + contentPath = _publishedSnapshotAccessor.GetRequiredPublishedSnapshot().Content?.GetRouteById(content.Id, culture) ?? contentPath; + } + + contentPath = contentPath.EnsureStartsWith("/"); if (_globalSettings.HideTopLevelNodeFromPath == false) { contentPath = contentPath.TrimStart(rootPath.EnsureStartsWith("/")).EnsureStartsWith("/"); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs index bd1dfdbcf7..75f960bb0f 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs @@ -32,7 +32,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()); + var routeBuilder = new ApiContentRouteBuilder(publishedUrlProvider.Object, CreateGlobalSettings(), Mock.Of(), Mock.Of()); 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 29447f2e82..f769913b23 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()), + new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of(), Mock.Of()), 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 043c137a10..202026ed30 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs @@ -19,11 +19,7 @@ public class ContentRouteBuilderTests : DeliveryApiTests var rootKey = Guid.NewGuid(); var root = SetupInvariantPublishedContent("The Root", rootKey); - // yes... actually testing the mock setup here. but it's important! - var publishedUrlProvider = SetupPublishedUrlProvider(hideTopLevelNodeFromPath); - Assert.AreEqual(hideTopLevelNodeFromPath ? "/" : "/the-root", publishedUrlProvider.GetUrl(root)); - - var builder = new ApiContentRouteBuilder(publishedUrlProvider, CreateGlobalSettings(hideTopLevelNodeFromPath), Mock.Of()); + var builder = CreateApiContentRouteBuilder(hideTopLevelNodeFromPath); var result = builder.Build(root); Assert.AreEqual("/", result.Path); Assert.AreEqual(rootKey, result.StartItem.Id); @@ -40,11 +36,7 @@ public class ContentRouteBuilderTests : DeliveryApiTests var childKey = Guid.NewGuid(); var child = SetupInvariantPublishedContent("The Child", childKey, root); - // yes... actually testing the mock setup here. but it's important! - var publishedUrlProvider = SetupPublishedUrlProvider(hideTopLevelNodeFromPath); - Assert.AreEqual(hideTopLevelNodeFromPath ? "/the-child" : "/the-root/the-child", publishedUrlProvider.GetUrl(child)); - - var builder = new ApiContentRouteBuilder(publishedUrlProvider, CreateGlobalSettings(hideTopLevelNodeFromPath), Mock.Of()); + var builder = CreateApiContentRouteBuilder(hideTopLevelNodeFromPath); var result = builder.Build(child); Assert.AreEqual("/the-child", result.Path); Assert.AreEqual(rootKey, result.StartItem.Id); @@ -64,11 +56,7 @@ public class ContentRouteBuilderTests : DeliveryApiTests var grandchildKey = Guid.NewGuid(); var grandchild = SetupInvariantPublishedContent("The Grandchild", grandchildKey, child); - // yes... actually testing the mock setup here. but it's important! - var publishedUrlProvider = SetupPublishedUrlProvider(hideTopLevelNodeFromPath); - Assert.AreEqual(hideTopLevelNodeFromPath ? "/the-child/the-grandchild" : "/the-root/the-child/the-grandchild", publishedUrlProvider.GetUrl(grandchild)); - - var builder = new ApiContentRouteBuilder(publishedUrlProvider, CreateGlobalSettings(hideTopLevelNodeFromPath), Mock.Of()); + var builder = CreateApiContentRouteBuilder(hideTopLevelNodeFromPath); var result = builder.Build(grandchild); Assert.AreEqual("/the-child/the-grandchild", result.Path); Assert.AreEqual(rootKey, result.StartItem.Id); @@ -84,9 +72,7 @@ public class ContentRouteBuilderTests : DeliveryApiTests var childKey = Guid.NewGuid(); var child = SetupVariantPublishedContent("The Child", childKey, root); - var publishedUrlProvider = SetupPublishedUrlProvider(false); - - var builder = new ApiContentRouteBuilder(publishedUrlProvider, CreateGlobalSettings(false), Mock.Of()); + var builder = CreateApiContentRouteBuilder(false); var result = builder.Build(child, "en-us"); Assert.AreEqual("/the-child-en-us", result.Path); Assert.AreEqual(rootKey, result.StartItem.Id); @@ -107,9 +93,7 @@ public class ContentRouteBuilderTests : DeliveryApiTests var childKey = Guid.NewGuid(); var child = SetupInvariantPublishedContent("The Child", childKey, root); - var publishedUrlProvider = SetupPublishedUrlProvider(false); - - var builder = new ApiContentRouteBuilder(publishedUrlProvider, CreateGlobalSettings(false), Mock.Of()); + var builder = CreateApiContentRouteBuilder(false); var result = builder.Build(child, "en-us"); Assert.AreEqual("/the-child", result.Path); Assert.AreEqual(rootKey, result.StartItem.Id); @@ -130,9 +114,7 @@ public class ContentRouteBuilderTests : DeliveryApiTests var childKey = Guid.NewGuid(); var child = SetupVariantPublishedContent("The Child", childKey, root); - var publishedUrlProvider = SetupPublishedUrlProvider(false); - - var builder = new ApiContentRouteBuilder(publishedUrlProvider, CreateGlobalSettings(false), Mock.Of()); + var builder = CreateApiContentRouteBuilder(false); var result = builder.Build(child, "en-us"); Assert.AreEqual("/the-child-en-us", result.Path); Assert.AreEqual(rootKey, result.StartItem.Id); @@ -153,10 +135,67 @@ public class ContentRouteBuilderTests : DeliveryApiTests var content = new Mock(); content.SetupGet(c => c.ItemType).Returns(itemType); - var builder = new ApiContentRouteBuilder(SetupPublishedUrlProvider(true), CreateGlobalSettings(), Mock.Of()); + var builder = CreateApiContentRouteBuilder(true); Assert.Throws(() => builder.Build(content.Object)); } + [TestCase("")] + [TestCase(" ")] + [TestCase("#")] + public void FallsBackToContentPathIfUrlProviderCannotResolveUrl(string resolvedUrl) + { + var publishedUrlProviderMock = new Mock(); + publishedUrlProviderMock + .Setup(p => p.GetUrl(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(resolvedUrl); + + var publishedContentCacheMock = new Mock(); + publishedContentCacheMock + .Setup(c => c.GetRouteById(It.IsAny(), It.IsAny())) + .Returns("/the/content/route"); + + var publishedSnapshotMock = new Mock(); + publishedSnapshotMock + .SetupGet(s => s.Content) + .Returns(publishedContentCacheMock.Object); + var publishedSnapshot = publishedSnapshotMock.Object; + + var publishedSnapshotAccessorMock = new Mock(); + publishedSnapshotAccessorMock + .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); + + Assert.AreEqual("/the/content/route", builder.Build(content).Path); + } + + [TestCase(true)] + [TestCase(false)] + public void VerifyPublishedUrlProviderSetup(bool hideTopLevelNodeFromPath) + { + var rootKey = Guid.NewGuid(); + var root = SetupInvariantPublishedContent("The Root", rootKey); + + var childKey = Guid.NewGuid(); + var child = SetupInvariantPublishedContent("The Child", childKey, root); + + var grandchildKey = Guid.NewGuid(); + var grandchild = SetupInvariantPublishedContent("The Grandchild", grandchildKey, child); + + // yes... actually testing the mock setup here. but it's important for the rest of the tests that this behave correct, so we better test it. + var publishedUrlProvider = SetupPublishedUrlProvider(hideTopLevelNodeFromPath); + Assert.AreEqual(hideTopLevelNodeFromPath ? "/" : "/the-root", publishedUrlProvider.GetUrl(root)); + Assert.AreEqual(hideTopLevelNodeFromPath ? "/the-child" : "/the-root/the-child", publishedUrlProvider.GetUrl(child)); + 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) { var publishedContentType = CreatePublishedContentType(); @@ -207,4 +246,11 @@ public class ContentRouteBuilderTests : DeliveryApiTests .Returns((IPublishedContent content, UrlMode mode, string? culture, Uri? current) => Url(content, culture)); return publishedUrlProvider.Object; } + + private ApiContentRouteBuilder CreateApiContentRouteBuilder(bool hideTopLevelNodeFromPath) + => new( + SetupPublishedUrlProvider(hideTopLevelNodeFromPath), + CreateGlobalSettings(hideTopLevelNodeFromPath), + Mock.Of(), + Mock.Of()); } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs index bb7fdb8b75..ee4635f145 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); - var routeBuilder = new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of()); + var routeBuilder = new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of(), Mock.Of()); 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 0210a2cd3a..970d3492e3 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiUrlPickerValueConverterTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiUrlPickerValueConverterTests.cs @@ -8,6 +8,7 @@ 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; @@ -259,7 +260,7 @@ public class MultiUrlPickerValueConverterTests : PropertyValueConverterTests private MultiUrlPickerValueConverter MultiUrlPickerValueConverter() { - var routeBuilder = new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of()); + var routeBuilder = new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of(), Mock.Of()); 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 dda07c3051..86aa8215db 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/OutputExpansionStrategyTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/OutputExpansionStrategyTests.cs @@ -440,5 +440,5 @@ public class OutputExpansionStrategyTests : PropertyValueConverterTests return new PublishedElementPropertyBase(elementPropertyType, parent, false, PropertyCacheLevel.None); } - private IApiContentRouteBuilder ApiContentRouteBuilder() => new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of()); + private IApiContentRouteBuilder ApiContentRouteBuilder() => new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of(), Mock.Of()); }