Handle unroutable published content (#14166)

Co-authored-by: Elitsa <elm@umbraco.dk>
This commit is contained in:
Kenn Jacobsen
2023-04-27 13:32:15 +02:00
committed by GitHub
parent d2a8068ca6
commit d72eb23102
7 changed files with 95 additions and 32 deletions

View File

@@ -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> globalSettings, IVariationContextAccessor variationContextAccessor)
public ApiContentRouteBuilder(
IPublishedUrlProvider publishedUrlProvider,
IOptions<GlobalSettings> 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("/");

View File

@@ -32,7 +32,7 @@ public class ContentBuilderTests : DeliveryApiTests
.Setup(p => p.GetUrl(It.IsAny<IPublishedContent>(), It.IsAny<UrlMode>(), It.IsAny<string?>(), It.IsAny<Uri?>()))
.Returns((IPublishedContent content, UrlMode mode, string? culture, Uri? current) => $"url:{content.UrlSegment}");
var routeBuilder = new ApiContentRouteBuilder(publishedUrlProvider.Object, CreateGlobalSettings(), Mock.Of<IVariationContextAccessor>());
var routeBuilder = new ApiContentRouteBuilder(publishedUrlProvider.Object, CreateGlobalSettings(), Mock.Of<IVariationContextAccessor>(), Mock.Of<IPublishedSnapshotAccessor>());
var builder = new ApiContentBuilder(new ApiContentNameProvider(), routeBuilder, CreateOutputExpansionStrategyAccessor());
var result = builder.Build(content.Object);

View File

@@ -18,7 +18,7 @@ public class ContentPickerValueConverterTests : PropertyValueConverterTests
PublishedSnapshotAccessor,
new ApiContentBuilder(
nameProvider ?? new ApiContentNameProvider(),
new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of<IVariationContextAccessor>()),
new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of<IVariationContextAccessor>(), Mock.Of<IPublishedSnapshotAccessor>()),
CreateOutputExpansionStrategyAccessor()));
[Test]

View File

@@ -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<IVariationContextAccessor>());
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<IVariationContextAccessor>());
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<IVariationContextAccessor>());
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<IVariationContextAccessor>());
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<IVariationContextAccessor>());
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<IVariationContextAccessor>());
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<IPublishedContent>();
content.SetupGet(c => c.ItemType).Returns(itemType);
var builder = new ApiContentRouteBuilder(SetupPublishedUrlProvider(true), CreateGlobalSettings(), Mock.Of<IVariationContextAccessor>());
var builder = CreateApiContentRouteBuilder(true);
Assert.Throws<ArgumentException>(() => builder.Build(content.Object));
}
[TestCase("")]
[TestCase(" ")]
[TestCase("#")]
public void FallsBackToContentPathIfUrlProviderCannotResolveUrl(string resolvedUrl)
{
var publishedUrlProviderMock = new Mock<IPublishedUrlProvider>();
publishedUrlProviderMock
.Setup(p => p.GetUrl(It.IsAny<IPublishedContent>(), It.IsAny<UrlMode>(), It.IsAny<string?>(), It.IsAny<Uri?>()))
.Returns(resolvedUrl);
var publishedContentCacheMock = new Mock<IPublishedContentCache>();
publishedContentCacheMock
.Setup(c => c.GetRouteById(It.IsAny<int>(), It.IsAny<string?>()))
.Returns("/the/content/route");
var publishedSnapshotMock = new Mock<IPublishedSnapshot>();
publishedSnapshotMock
.SetupGet(s => s.Content)
.Returns(publishedContentCacheMock.Object);
var publishedSnapshot = publishedSnapshotMock.Object;
var publishedSnapshotAccessorMock = new Mock<IPublishedSnapshotAccessor>();
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<IVariationContextAccessor>(),
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<IVariationContextAccessor>(),
Mock.Of<IPublishedSnapshotAccessor>());
}

View File

@@ -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<IVariationContextAccessor>());
var routeBuilder = new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of<IVariationContextAccessor>(), Mock.Of<IPublishedSnapshotAccessor>());
return new MultiNodeTreePickerValueConverter(
PublishedSnapshotAccessor,
Mock.Of<IUmbracoContextAccessor>(),

View File

@@ -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<IVariationContextAccessor>());
var routeBuilder = new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of<IVariationContextAccessor>(), Mock.Of<IPublishedSnapshotAccessor>());
return new MultiUrlPickerValueConverter(
PublishedSnapshotAccessor,
Mock.Of<IProfilingLogger>(),

View File

@@ -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<IVariationContextAccessor>());
private IApiContentRouteBuilder ApiContentRouteBuilder() => new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of<IVariationContextAccessor>(), Mock.Of<IPublishedSnapshotAccessor>());
}