Allow multiple URL segments per document (#18603)

* Code tidy - XML header comments, method ordering, warning resolution.

* Add extension method for retrieving all URL segments for a document.

* Cache and persist multiple URL segments per document.

* Allowed segment providers to terminate or allow additional segments.
Set up currently failing integration test for expected routes.

* Resolved cache issue to ensure passing new integration tests.

* Fixed failing integration test.

* Test class naming tidy up.

* Added resolution and persistance of a primary segment, to retain previous behaviour when a single segment is retrieved.

* Further integration tests.

* Resolved backward compatibility of interface.

* Supress amends made to integration tests.

* Aligned naming of integration tests.

* Removed unused using, added XML header comment.

* Throw on missing table in migration.

* Code clean-up.

* Fix multiple enumeration

* Used default on migrated column.

* Use 1 over true for default value.

* Remove unused logger

---------

Co-authored-by: mole <nikolajlauridsen@protonmail.ch>
This commit is contained in:
Andy Butland
2025-03-13 13:47:46 +01:00
committed by GitHub
parent da5669d9fc
commit e9c97f8c9b
13 changed files with 901 additions and 481 deletions

View File

@@ -1,6 +1,20 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
<Suppression>
<DiagnosticId>CP0001</DiagnosticId>
<Target>T:Umbraco.Cms.Tests.Integration.Umbraco.Core.Services.DocumentUrlServiceTest</Target>
<Left>lib/net9.0/Umbraco.Tests.Integration.dll</Left>
<Right>lib/net9.0/Umbraco.Tests.Integration.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0001</DiagnosticId>
<Target>T:Umbraco.Cms.Tests.Integration.Umbraco.Core.Services.DocumentUrlServiceTest_HideTopLevel_False</Target>
<Left>lib/net9.0/Umbraco.Tests.Integration.dll</Left>
<Right>lib/net9.0/Umbraco.Tests.Integration.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0001</DiagnosticId>
<Target>T:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.PropertyEditors.BlockEditorBackwardsCompatibilityTests</Target>

View File

@@ -3,6 +3,7 @@ using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Strings;
using Umbraco.Cms.Core.Sync;
using Umbraco.Cms.Tests.Common.Builders;
using Umbraco.Cms.Tests.Common.Testing;
@@ -13,9 +14,13 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services;
[TestFixture]
[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest, Logger = UmbracoTestOptions.Logger.Mock)]
public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
public class DocumentUrlServiceTests : UmbracoIntegrationTestWithContent
{
private const string SubSubPage2Key = "48AE405E-5142-4EBE-929F-55EB616F51F2";
private const string SubSubPage3Key = "AACF2979-3F53-4184-B071-BA34D3338497";
protected IDocumentUrlService DocumentUrlService => GetRequiredService<IDocumentUrlService>();
protected ILanguageService LanguageService => GetRequiredService<ILanguageService>();
protected override void CustomTestSetup(IUmbracoBuilder builder)
@@ -24,8 +29,60 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
builder.AddNotificationHandler<ContentTreeChangeNotification, ContentTreeChangeDistributedCacheNotificationHandler>();
builder.Services.AddNotificationAsyncHandler<UmbracoApplicationStartingNotification, DocumentUrlServiceInitializerNotificationHandler>();
builder.UrlSegmentProviders().Insert<CustomUrlSegmentProvider1>();
builder.UrlSegmentProviders().Insert<CustomUrlSegmentProvider2>();
}
private abstract class CustomUrlSegmentProviderBase
{
private readonly IUrlSegmentProvider _defaultProvider;
public CustomUrlSegmentProviderBase(IShortStringHelper stringHelper) => _defaultProvider = new DefaultUrlSegmentProvider(stringHelper);
protected string? GetUrlSegment(IContentBase content, string? culture, params Guid[] pageKeys)
{
if (pageKeys.Contains(content.Key) is false)
{
return null;
}
var segment = _defaultProvider.GetUrlSegment(content, culture);
return segment is not null ? segment + "-custom" : null;
}
}
/// <summary>
/// A test implementation of <see cref="IUrlSegmentProvider"/> that provides a custom URL segment for a specific page
/// and allows for additional providers to provide segments too.
/// </summary>
private class CustomUrlSegmentProvider1 : CustomUrlSegmentProviderBase, IUrlSegmentProvider
{
public CustomUrlSegmentProvider1(IShortStringHelper stringHelper)
: base(stringHelper)
{
}
public bool AllowAdditionalSegments => true;
public string? GetUrlSegment(IContentBase content, string? culture = null)
=> GetUrlSegment(content, culture, Guid.Parse(SubPageKey), Guid.Parse(SubSubPage3Key));
}
/// <summary>
/// A test implementation of <see cref="IUrlSegmentProvider"/> that provides a custom URL segment for a specific page
/// and terminates, not allowing additional providers to provide segments too.
/// </summary>
private class CustomUrlSegmentProvider2 : CustomUrlSegmentProviderBase, IUrlSegmentProvider
{
public CustomUrlSegmentProvider2(IShortStringHelper stringHelper)
: base(stringHelper)
{
}
public string? GetUrlSegment(IContentBase content, string? culture = null)
=> GetUrlSegment(content, culture, Guid.Parse(SubPage2Key), Guid.Parse(SubSubPage2Key));
}
public override void Setup()
{
@@ -52,7 +109,7 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
// }
[Test]
public async Task Trashed_documents_do_not_have_a_url_segment()
public async Task GetUrlSegment_For_Deleted_Document_Does_Not_Have_Url_Segment()
{
var isoCode = (await LanguageService.GetDefaultLanguageAsync()).IsoCode;
@@ -64,7 +121,7 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
//TODO test with the urlsegment property value!
[Test]
public async Task Deleted_documents_do_not_have_a_url_segment__Parent_deleted()
public async Task GetUrlSegment_For_Document_With_Parent_Deleted_Does_Not_Have_Url_Segment()
{
ContentService.PublishBranch(Textpage, PublishBranchFilter.IncludeUnpublished, ["*"]);
@@ -78,7 +135,7 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
}
[Test]
public async Task Deleted_documents_do_not_have_a_url_segment()
public async Task GetUrlSegment_For_Published_Then_Deleted_Document_Does_Not_Have_Url_Segment()
{
ContentService.PublishBranch(Textpage, PublishBranchFilter.IncludeUnpublished, ["*"]);
@@ -91,16 +148,19 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
Assert.IsNull(actual);
}
[Test]
[TestCase("/", "en-US", true, ExpectedResult = TextpageKey)]
[TestCase("/text-page-1", "en-US", true, ExpectedResult = SubPageKey)]
[TestCase("/text-page-2", "en-US", true, ExpectedResult = SubPage2Key)]
[TestCase("/text-page-1-custom", "en-US", true, ExpectedResult = SubPageKey)] // Uses the segment registered by the custom IIUrlSegmentProvider that allows for more than one segment per document.
[TestCase("/text-page-2", "en-US", true, ExpectedResult = null)]
[TestCase("/text-page-2-custom", "en-US", true, ExpectedResult = SubPage2Key)] // Uses the segment registered by the custom IIUrlSegmentProvider that does not allow for more than one segment per document.
[TestCase("/text-page-3", "en-US", true, ExpectedResult = SubPage3Key)]
[TestCase("/", "en-US", false, ExpectedResult = TextpageKey)]
[TestCase("/text-page-1", "en-US", false, ExpectedResult = SubPageKey)]
[TestCase("/text-page-2", "en-US", false, ExpectedResult = SubPage2Key)]
[TestCase("/text-page-1-custom", "en-US", false, ExpectedResult = SubPageKey)] // Uses the segment registered by the custom IIUrlSegmentProvider that allows for more than one segment per document.
[TestCase("/text-page-2", "en-US", false, ExpectedResult = null)]
[TestCase("/text-page-2-custom", "en-US", false, ExpectedResult = SubPage2Key)] // Uses the segment registered by the custom IIUrlSegmentProvider that does not allow for more than one segment per document.
[TestCase("/text-page-3", "en-US", false, ExpectedResult = SubPage3Key)]
public string? Expected_Routes(string route, string isoCode, bool loadDraft)
public string? GetDocumentKeyByRoute_Returns_Expected_Route(string route, string isoCode, bool loadDraft)
{
if (loadDraft is false)
{
@@ -111,16 +171,16 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
}
[Test]
public void No_Published_Route_when_not_published()
public void GetDocumentKeyByRoute_UnPublished_Documents_Have_No_Published_Route()
{
Assert.IsNotNull(DocumentUrlService.GetDocumentKeyByRoute("/text-page-1", "en-US", null, true));
Assert.IsNull(DocumentUrlService.GetDocumentKeyByRoute("/text-page-1", "en-US", null, false));
}
[Test]
public void Unpublished_Pages_Are_not_available()
public void GetDocumentKeyByRoute_Published_Then_Unpublished_Documents_Have_No_Published_Route()
{
//Arrange
// Arrange
ContentService.PublishBranch(Textpage, PublishBranchFilter.IncludeUnpublished, ["*"]);
Assert.Multiple(() =>
@@ -131,7 +191,7 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
Assert.IsNotNull(DocumentUrlService.GetDocumentKeyByRoute("/text-page-1", "en-US", null, false));
});
//Act
// Act
ContentService.Unpublish(Textpage );
Assert.Multiple(() =>
@@ -144,18 +204,32 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
Assert.IsNotNull(DocumentUrlService.GetDocumentKeyByRoute("/text-page-1", "en-US", null, true));
Assert.IsNull(DocumentUrlService.GetDocumentKeyByRoute("/text-page-1", "en-US", null, false));
});
}
[Test]
[TestCase("/text-page-1/sub-page-1", "en-US", true, ExpectedResult = "DF49F477-12F2-4E33-8563-91A7CC1DCDBB")]
[TestCase("/text-page-1/sub-page-1", "en-US", false, ExpectedResult = "DF49F477-12F2-4E33-8563-91A7CC1DCDBB")]
public string? Expected_Routes_with_subpages(string route, string isoCode, bool loadDraft)
public string? GetDocumentKeyByRoute_Returns_Expected_Route_For_SubPage(string route, string isoCode, bool loadDraft)
=> ExecuteSubPageTest("DF49F477-12F2-4E33-8563-91A7CC1DCDBB", "Sub Page 1", route, isoCode, loadDraft);
[TestCase("/text-page-1/sub-page-2-custom", "en-US", true, ExpectedResult = SubSubPage2Key)]
[TestCase("/text-page-1/sub-page-2-custom", "en-US", false, ExpectedResult = SubSubPage2Key)]
[TestCase("/text-page-1/sub-page-2", "en-US", true, ExpectedResult = null)]
[TestCase("/text-page-1/sub-page-2", "en-US", false, ExpectedResult = null)]
public string? GetDocumentKeyByRoute_Returns_Expected_Route_For_SubPage_With_Terminating_Custom_Url_Provider(string route, string isoCode, bool loadDraft)
=> ExecuteSubPageTest(SubSubPage2Key, "Sub Page 2", route, isoCode, loadDraft);
[TestCase("/text-page-1/sub-page-3-custom", "en-US", true, ExpectedResult = SubSubPage3Key)]
[TestCase("/text-page-1/sub-page-3-custom", "en-US", false, ExpectedResult = SubSubPage3Key)]
[TestCase("/text-page-1/sub-page-3", "en-US", true, ExpectedResult = SubSubPage3Key)]
[TestCase("/text-page-1/sub-page-3", "en-US", false, ExpectedResult = SubSubPage3Key)]
public string? GetDocumentKeyByRoute_Returns_Expected_Route_For_SubPage_With_Non_Terminating_Custom_Url_Provider(string route, string isoCode, bool loadDraft)
=> ExecuteSubPageTest(SubSubPage3Key, "Sub Page 3", route, isoCode, loadDraft);
private string? ExecuteSubPageTest(string documentKey, string documentName, string route, string isoCode, bool loadDraft)
{
// Create a subpage
var subsubpage = ContentBuilder.CreateSimpleContent(ContentType, "Sub Page 1", Subpage.Id);
subsubpage.Key = new Guid("DF49F477-12F2-4E33-8563-91A7CC1DCDBB");
var subsubpage = ContentBuilder.CreateSimpleContent(ContentType, documentName, Subpage.Id);
subsubpage.Key = Guid.Parse(documentKey);
var contentSchedule = ContentScheduleCollection.CreateWithEntry(DateTime.Now.AddMinutes(-5), null);
ContentService.Save(subsubpage, -1, contentSchedule);
@@ -164,13 +238,12 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
ContentService.PublishBranch(Textpage, PublishBranchFilter.IncludeUnpublished, ["*"]);
}
return DocumentUrlService.GetDocumentKeyByRoute(route, isoCode, null, loadDraft)?.ToString()?.ToUpper();
return DocumentUrlService.GetDocumentKeyByRoute(route, isoCode, null, loadDraft)?.ToString()?.ToUpper();
}
[Test]
[TestCase("/second-root", "en-US", true, ExpectedResult = "8E21BCD4-02CA-483D-84B0-1FC92702E198")]
[TestCase("/second-root", "en-US", false, ExpectedResult = "8E21BCD4-02CA-483D-84B0-1FC92702E198")]
public string? Second_root_cannot_hide_url(string route, string isoCode, bool loadDraft)
public string? GetDocumentKeyByRoute_Second_Root_Does_Not_Hide_Url(string route, string isoCode, bool loadDraft)
{
// Create a second root
var secondRoot = ContentBuilder.CreateSimpleContent(ContentType, "Second Root", null);
@@ -187,10 +260,9 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
return DocumentUrlService.GetDocumentKeyByRoute(route, isoCode, null, loadDraft)?.ToString()?.ToUpper();
}
[Test]
[TestCase("/child-of-second-root", "en-US", true, ExpectedResult = "FF6654FB-BC68-4A65-8C6C-135567F50BD6")]
[TestCase("/child-of-second-root", "en-US", false, ExpectedResult = "FF6654FB-BC68-4A65-8C6C-135567F50BD6")]
public string? Child_of_second_root_do_not_have_parents_url_as_prefix(string route, string isoCode, bool loadDraft)
public string? GetDocumentKeyByRoute_Child_Of_Second_Root_Does_Not_Have_Parents_Url_As_Prefix(string route, string isoCode, bool loadDraft)
{
// Create a second root
var secondRoot = ContentBuilder.CreateSimpleContent(ContentType, "Second Root", null);
@@ -205,7 +277,6 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
// Publish both the main root and the second root with descendants
if (loadDraft is false)
{
ContentService.PublishBranch(Textpage, PublishBranchFilter.IncludeUnpublished, ["*"]);
ContentService.PublishBranch(secondRoot, PublishBranchFilter.IncludeUnpublished, ["*"]);
}
@@ -213,6 +284,16 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
return DocumentUrlService.GetDocumentKeyByRoute(route, isoCode, null, loadDraft)?.ToString()?.ToUpper();
}
[TestCase(TextpageKey, "en-US", ExpectedResult = "/")]
[TestCase(SubPageKey, "en-US", ExpectedResult = "/text-page-1-custom")] // Has non-terminating custom URL segment provider.
[TestCase(SubPage2Key, "en-US", ExpectedResult = "/text-page-2-custom")] // Has terminating custom URL segment provider.
[TestCase(SubPage3Key, "en-US", ExpectedResult = "/text-page-3")]
public string? GetLegacyRouteFormat_Returns_Expected_Route(string documentKey, string culture)
{
ContentService.PublishBranch(Textpage, PublishBranchFilter.IncludeUnpublished, ["*"]);
return DocumentUrlService.GetLegacyRouteFormat(Guid.Parse(documentKey), culture, false);
}
//TODO test cases:
// - Find the root, when a domain is set

View File

@@ -2,12 +2,10 @@ using Microsoft.Extensions.DependencyInjection;
using NUnit.Framework;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Handlers;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Sync;
using Umbraco.Cms.Tests.Common.Attributes;
using Umbraco.Cms.Tests.Common.Builders;
using Umbraco.Cms.Tests.Common.Testing;
using Umbraco.Cms.Tests.Integration.Testing;
@@ -17,7 +15,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services;
[TestFixture]
[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest, Logger = UmbracoTestOptions.Logger.Console)]
public class DocumentUrlServiceTest_HideTopLevel_False : UmbracoIntegrationTestWithContent
public class DocumentUrlServiceTests_HideTopLevel_False : UmbracoIntegrationTestWithContent
{
protected IDocumentUrlService DocumentUrlService => GetRequiredService<IDocumentUrlService>();
protected ILanguageService LanguageService => GetRequiredService<ILanguageService>();
@@ -28,7 +26,6 @@ public class DocumentUrlServiceTest_HideTopLevel_False : UmbracoIntegrationTestW
builder.Services.AddUnique<IServerMessenger, ScopedRepositoryTests.LocalServerMessenger>();
builder.AddNotificationHandler<ContentTreeChangeNotification, ContentTreeChangeDistributedCacheNotificationHandler>();
}
public override void Setup()
@@ -46,7 +43,7 @@ public class DocumentUrlServiceTest_HideTopLevel_False : UmbracoIntegrationTestW
[TestCase("/textpage/text-page-1", "en-US", false, ExpectedResult = SubPageKey)]
[TestCase("/textpage/text-page-2", "en-US", false, ExpectedResult = SubPage2Key)]
[TestCase("/textpage/text-page-3", "en-US", false, ExpectedResult = SubPage3Key)]
public string? Expected_Routes(string route, string isoCode, bool loadDraft)
public string? GetDocumentKeyByRoute_Returns_Expected_Route(string route, string isoCode, bool loadDraft)
{
if (loadDraft is false)
{
@@ -60,7 +57,7 @@ public class DocumentUrlServiceTest_HideTopLevel_False : UmbracoIntegrationTestW
[Test]
[TestCase("/textpage/text-page-1/sub-page-1", "en-US", true, ExpectedResult = "DF49F477-12F2-4E33-8563-91A7CC1DCDBB")]
[TestCase("/textpage/text-page-1/sub-page-1", "en-US", false, ExpectedResult = "DF49F477-12F2-4E33-8563-91A7CC1DCDBB")]
public string? Expected_Routes_with_subpages(string route, string isoCode, bool loadDraft)
public string? GetDocumentKeyByRoute_Returns_Expected_Route_For_SubPage(string route, string isoCode, bool loadDraft)
{
// Create a subpage
var subsubpage = ContentBuilder.CreateSimpleContent(ContentType, "Sub Page 1", Subpage.Id);
@@ -79,7 +76,7 @@ public class DocumentUrlServiceTest_HideTopLevel_False : UmbracoIntegrationTestW
[Test]
[TestCase("/second-root", "en-US", true, ExpectedResult = "8E21BCD4-02CA-483D-84B0-1FC92702E198")]
[TestCase("/second-root", "en-US", false, ExpectedResult = "8E21BCD4-02CA-483D-84B0-1FC92702E198")]
public string? Second_root_cannot_hide_url(string route, string isoCode, bool loadDraft)
public string? GetDocumentKeyByRoute_Second_Root_Does_Not_Hide_Url(string route, string isoCode, bool loadDraft)
{
// Create a second root
var secondRoot = ContentBuilder.CreateSimpleContent(ContentType, "Second Root", null);
@@ -99,7 +96,7 @@ public class DocumentUrlServiceTest_HideTopLevel_False : UmbracoIntegrationTestW
[Test]
[TestCase("/second-root/child-of-second-root", "en-US", true, ExpectedResult = "FF6654FB-BC68-4A65-8C6C-135567F50BD6")]
[TestCase("/second-root/child-of-second-root", "en-US", false, ExpectedResult = "FF6654FB-BC68-4A65-8C6C-135567F50BD6")]
public string? Child_of_second_root_do_not_have_parents_url_as_prefix(string route, string isoCode, bool loadDraft)
public string? GetDocumentKeyByRoute_Child_Of_Second_Root_Does_Not_Have_Parents_Url_As_Prefix(string route, string isoCode, bool loadDraft)
{
// Create a second root
var secondRoot = ContentBuilder.CreateSimpleContent(ContentType, "Second Root", null);