Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 32 additions & 8 deletions src/Umbraco.Core/Routing/PublishedUrlInfoProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
private readonly ILocalizedTextService _localizedTextService;
private readonly ILogger<PublishedUrlInfoProvider> _logger;
private readonly UriUtility _uriUtility;
private readonly IVariationContextAccessor _variationContextAccessor;

/// <summary>
/// Initializes a new instance of the <see cref="PublishedUrlInfoProvider" /> class.
Expand All @@ -43,7 +42,7 @@
ILocalizedTextService localizedTextService,
ILogger<PublishedUrlInfoProvider> logger,
UriUtility uriUtility,
IVariationContextAccessor variationContextAccessor)
IVariationContextAccessor variationContextAccessor) // TODO (V18): Remove this unused parameter.
{
_publishedUrlProvider = publishedUrlProvider;
_languageService = languageService;
Expand All @@ -52,44 +51,69 @@
_localizedTextService = localizedTextService;
_logger = logger;
_uriUtility = uriUtility;
_variationContextAccessor = variationContextAccessor;
}

/// <inheritdoc />
public async Task<ISet<UrlInfo>> GetAllAsync(IContent content)
{
HashSet<UrlInfo> urlInfos = [];
var isInvariant = !content.ContentType.VariesByCulture();

// For invariant content, only return the URL for the default language.
// Invariant content doesn't vary by culture, so it only has one URL.
IEnumerable<string> cultures = content.ContentType.VariesByCulture()
? await _languageService.GetAllIsoCodesAsync()
: [(await _languageService.GetDefaultIsoCodeAsync())];
// For all content, iterate all cultures.
// For invariant content, cultures without a matching domain will return "#"
// and be silently skipped, naturally filtering to only relevant URLs.
IEnumerable<string> cultures = await _languageService.GetAllIsoCodesAsync();

// First we get the urls of all cultures, using the published router, meaning we respect any extensions.
foreach (var culture in cultures)
{
var url = _publishedUrlProvider.GetUrl(content.Key, culture: culture);

// Handle "could not get URL"
if (url is "#" or "#ex")
{
// For invariant content, a missing URL just means there's no domain
// for this culture — not a problem worth reporting.
if (isInvariant)
{
continue;
}

urlInfos.Add(UrlInfo.AsMessage(_localizedTextService.Localize("content", "getUrlException"), UrlProviderAlias, culture));
continue;
}

// Check for collision
Attempt<UrlInfo?> hasCollision = await VerifyCollisionAsync(content, url, culture);

if (hasCollision is { Success: true, Result: not null })
{
urlInfos.Add(hasCollision.Result);
continue;
}

urlInfos.Add(UrlInfo.AsUrl(url, UrlProviderAlias, culture));
}

// For invariant content, only show URLs for cultures that have a domain.
// When domains exist, the default culture may produce a fallback URL using the
// request's base URL (CleanedUmbracoUrl) — this is not a real routable URL and
// should be removed. When no domains exist, de-duplicate by URL string since
// multiple cultures resolve to the same path.
if (isInvariant)
{
Uri cleanedUrl = _umbracoContextAccessor.GetRequiredUmbracoContext().CleanedUmbracoUrl;
var hasDomainUrls = urlInfos.Any(u => u.Url is { IsAbsoluteUri: true } && !u.Url.Host.Equals(cleanedUrl.Host, StringComparison.OrdinalIgnoreCase));

if (hasDomainUrls)
{
urlInfos.RemoveWhere(u => u.Url is not null && (!u.Url.IsAbsoluteUri || u.Url.Host.Equals(cleanedUrl.Host, StringComparison.OrdinalIgnoreCase)));
}

var seenUrls = new HashSet<string>();
urlInfos.RemoveWhere(u => u.Url is not null && !seenUrls.Add(u.Url.ToString()));
}

Check warning on line 116 in src/Umbraco.Core/Routing/PublishedUrlInfoProvider.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ Getting worse: Complex Method

GetAllAsync increases in cyclomatic complexity from 9 to 16, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

Check warning on line 116 in src/Umbraco.Core/Routing/PublishedUrlInfoProvider.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Bumpy Road Ahead

GetAllAsync has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is 2 blocks per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
// If the content is trashed, we can't get the other URLs, as we have no parent structure to navigate through.
if (content.Trashed)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
using Microsoft.Extensions.DependencyInjection;
using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.ContentEditing;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Tests.Common.Builders;
using Umbraco.Cms.Tests.Common.Builders.Extensions;
using Umbraco.Cms.Tests.Integration.Attributes;

namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services;

internal sealed class PublishedUrlInfoProviderTests : PublishedUrlInfoProviderTestsBase
{
private ILanguageService LanguageService => GetRequiredService<ILanguageService>();

private IDomainService DomainService => GetRequiredService<IDomainService>();

public static void ConfigureHideTopLevelNodeFalse(IUmbracoBuilder builder)
=> builder.Services.Configure<GlobalSettings>(x => x.HideTopLevelNodeFromPath = false);

[Test]
public async Task Invariant_content_returns_only_default_language_url()
public async Task Invariant_Content_Without_Domain_Returns_Only_Default_Language_Url()
{
// Arrange: Add a second language (Danish) alongside the default English
var danishLanguage = new LanguageBuilder()
Expand All @@ -36,7 +45,71 @@ public async Task Invariant_content_returns_only_default_language_url()
}

[Test]
public async Task Two_items_in_level_1_with_same_name_will_have_conflicting_routes()
public async Task Invariant_Content_Under_Non_Default_Language_Domain_Returns_Only_Domain_Url()
{
// Arrange: Add a second language (Danish) alongside the default English
var danishLanguage = new LanguageBuilder()
.WithCultureInfo("da-DK")
.WithCultureName("Danish")
.Build();
await LanguageService.CreateAsync(danishLanguage, Constants.Security.SuperUserKey);

// Publish the branch (invariant content from base class)
ContentService.PublishBranch(Textpage, PublishBranchFilter.IncludeUnpublished, ["*"]);

// Assign a domain with the non-default culture (da-DK) to the root node
var updateDomainResult = await DomainService.UpdateDomainsAsync(
Textpage.Key,
new DomainsUpdateModel
{
Domains = [new DomainModel { DomainName = "test.dk", IsoCode = "da-DK" }],
});
Assert.IsTrue(updateDomainResult.Success, "Domain assignment should succeed");

// Act: Get all URLs for a child of the root with the da-DK domain
var urls = await PublishedUrlInfoProvider.GetAllAsync(Subpage);

// Assert: Should contain only the da-DK domain URL, not the default culture fallback
Assert.AreEqual(1, urls.Count, "Should return exactly one URL (the domain-based URL)");
Assert.IsNotNull(urls.First().Url);
Assert.AreEqual("da-DK", urls.First().Culture, "The URL should be for the domain culture (da-DK)");
Assert.That(urls.First().Url!.Host, Is.EqualTo("test.dk"), "The URL should use the assigned domain");
}

[Test]
[ConfigureBuilder(ActionName = nameof(ConfigureHideTopLevelNodeFalse))]
public async Task Two_Items_In_Level_1_With_Same_Name_Will_Not_Have_Conflicting_Routes_When_HideTopLevel_False()
{
// Create a second root
var secondRoot = ContentBuilder.CreateSimpleContent(ContentType, "Second Root", null);
var contentSchedule = ContentScheduleCollection.CreateWithEntry(DateTime.UtcNow.AddMinutes(-5), null);
ContentService.Save(secondRoot, -1, contentSchedule);

// Create a child of second root
var childOfSecondRoot = ContentBuilder.CreateSimpleContent(ContentType, Subpage.Name, secondRoot);
childOfSecondRoot.Key = new Guid("FF6654FB-BC68-4A65-8C6C-135567F50BD6");
ContentService.Save(childOfSecondRoot, -1, contentSchedule);

// Publish both the main root and the second root with descendants
ContentService.PublishBranch(Textpage, PublishBranchFilter.IncludeUnpublished, ["*"]);
ContentService.PublishBranch(secondRoot, PublishBranchFilter.IncludeUnpublished, ["*"]);

var subPageUrls = await PublishedUrlInfoProvider.GetAllAsync(Subpage);
var childOfSecondRootUrls = await PublishedUrlInfoProvider.GetAllAsync(childOfSecondRoot);

Assert.AreEqual(1, subPageUrls.Count);
Assert.IsNotNull(subPageUrls.First().Url);
Assert.AreEqual("/textpage/text-page-1/", subPageUrls.First().Url!.ToString());
Assert.AreEqual(Constants.UrlProviders.Content, subPageUrls.First().Provider);

Assert.AreEqual(1, childOfSecondRootUrls.Count);
Assert.IsNotNull(childOfSecondRootUrls.First().Url);
Assert.AreEqual("/second-root/text-page-1/", childOfSecondRootUrls.First().Url!.ToString());
Assert.AreEqual(Constants.UrlProviders.Content, childOfSecondRootUrls.First().Provider);
}

[Test]
public async Task Two_Items_In_Level_1_With_Same_Name_Will_Have_Conflicting_Routes()
{
// Create a second root
var secondRoot = ContentBuilder.CreateSimpleContent(ContentType, "Second Root", null);
Expand Down

This file was deleted.

Loading