Skip to content

Look-up redirect in content finder for multi-lingual sites using path and legacy route prefixed with the integer ID of the node with domains defined #18763

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 7, 2025
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Umbraco.Cms.Api.Management.ViewModels;
using Umbraco.Cms.Api.Management.ViewModels;
using Umbraco.Cms.Api.Management.ViewModels.RedirectUrlManagement;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Routing;
Expand All @@ -22,6 +22,12 @@ public RedirectUrlResponseModel Create(IRedirectUrl source)

var originalUrl = _publishedUrlProvider.GetUrlFromRoute(source.ContentId, source.Url, source.Culture);

// Even if the URL could not be extracted from the route, if we have a path as a the route for the original URL, we should display it.
if (originalUrl == "#" && source.Url.StartsWith('/'))
{
originalUrl = source.Url;
}

return new RedirectUrlResponseModel
{
OriginalUrl = originalUrl,
Expand Down
31 changes: 24 additions & 7 deletions src/Umbraco.Core/Routing/ContentFinderByRedirectUrl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,38 @@
return false;
}

var route = frequest.Domain != null
? frequest.Domain.ContentId +
DomainUtilities.PathRelativeToDomain(frequest.Domain.Uri, frequest.AbsolutePathDecoded)
: frequest.AbsolutePathDecoded;

var route = frequest.AbsolutePathDecoded;
IRedirectUrl? redirectUrl = await _redirectUrlService.GetMostRecentRedirectUrlAsync(route, frequest.Culture);

if (redirectUrl == null)
if (redirectUrl is null)
{
if (_logger.IsEnabled(LogLevel.Debug))
{
_logger.LogDebug("No match for route: {Route}", route);
}

return false;
// Routes under domains can be stored with the integer ID of the content where the domains were defined as the first part of the route,
// so if we haven't found a redirect, try using that format too.
// See: https://github.com/umbraco/Umbraco-CMS/pull/18160 and https://github.com/umbraco/Umbraco-CMS/pull/18763
if (frequest.Domain is not null)
{
route = frequest.Domain.ContentId + DomainUtilities.PathRelativeToDomain(frequest.Domain.Uri, frequest.AbsolutePathDecoded);
redirectUrl = await _redirectUrlService.GetMostRecentRedirectUrlAsync(route, frequest.Culture);

if (redirectUrl is null)
{
if (_logger.IsEnabled(LogLevel.Debug))
{
_logger.LogDebug("No match for route with domain: {Route}", route);
}

return false;
}
}
else
{
return false;
}

Check warning on line 87 in src/Umbraco.Core/Routing/ContentFinderByRedirectUrl.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v15/dev)

❌ Getting worse: Complex Method

TryFindContent increases in cyclomatic complexity from 10 to 12, 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 87 in src/Umbraco.Core/Routing/ContentFinderByRedirectUrl.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v15/dev)

❌ New issue: Deep, Nested Complexity

TryFindContent has a nested complexity depth of 4, threshold = 4. This function contains deeply nested logic such as if statements and/or loops. The deeper the nesting, the lower the code health.
}

IPublishedContent? content = umbracoContext.Content?.GetById(redirectUrl.ContentId);
Expand Down
3 changes: 2 additions & 1 deletion src/Umbraco.Core/Routing/NewDefaultUrlProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,8 @@ private string GetLegacyRouteFormatById(Guid key, string? culture)
culture);

var defaultCulture = _localizationService.GetDefaultLanguageIsoCode();
if (domainUri is not null || string.IsNullOrEmpty(culture) ||
if (domainUri is not null ||
string.IsNullOrEmpty(culture) ||
culture.Equals(defaultCulture, StringComparison.InvariantCultureIgnoreCase))
{
var url = AssembleUrl(domainUri, path, current, mode).ToString();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
using System.Net;
using Microsoft.Extensions.Logging.Abstractions;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.Routing;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Web;

namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Routing;

[TestFixture]
public class ContentFinderByRedirectUrlTests
{
private const int DomainContentId = 1233;
private const int ContentId = 1234;

[Test]
public async Task Can_Find_Invariant_Content()
{
const string OldPath = "/old-page-path";
const string NewPath = "/new-page-path";

var mockRedirectUrlService = CreateMockRedirectUrlService(OldPath);

var mockContent = CreateMockPublishedContent();

var mockUmbracoContextAccessor = CreateMockUmbracoContextAccessor(mockContent);

var mockPublishedUrlProvider = CreateMockPublishedUrlProvider(NewPath);

var sut = CreateContentFinder(mockRedirectUrlService, mockUmbracoContextAccessor, mockPublishedUrlProvider);

var publishedRequestBuilder = CreatePublishedRequestBuilder(OldPath);

var result = await sut.TryFindContent(publishedRequestBuilder);

AssertRedirectResult(publishedRequestBuilder, result);
}

Check warning on line 40 in tests/Umbraco.Tests.UnitTests/Umbraco.Core/Routing/ContentFinderByRedirectUrlTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v15/dev)

❌ New issue: Code Duplication

The module contains 3 functions with similar structure: Can_Find_Invariant_Content,Can_Find_Variant_Content_With_Domain_Node_Id_Prefixed_Path,Can_Find_Variant_Content_With_Path_Root. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

[Test]
public async Task Can_Find_Variant_Content_With_Path_Root()
{
const string OldPath = "/en/old-page-path";
const string NewPath = "/en/new-page-path";

var mockRedirectUrlService = CreateMockRedirectUrlService(OldPath);

var mockContent = CreateMockPublishedContent();

var mockUmbracoContextAccessor = CreateMockUmbracoContextAccessor(mockContent);

var mockPublishedUrlProvider = CreateMockPublishedUrlProvider(NewPath);

var sut = CreateContentFinder(mockRedirectUrlService, mockUmbracoContextAccessor, mockPublishedUrlProvider);

var publishedRequestBuilder = CreatePublishedRequestBuilder(OldPath, withDomain: true);

var result = await sut.TryFindContent(publishedRequestBuilder);

AssertRedirectResult(publishedRequestBuilder, result);
}

[Test]
public async Task Can_Find_Variant_Content_With_Domain_Node_Id_Prefixed_Path()
{
const string OldPath = "/en/old-page-path";
var domainPrefixedOldPath = $"{DomainContentId}/old-page-path";
const string NewPath = "/en/new-page-path";

var mockRedirectUrlService = CreateMockRedirectUrlService(domainPrefixedOldPath);

var mockContent = CreateMockPublishedContent();

var mockUmbracoContextAccessor = CreateMockUmbracoContextAccessor(mockContent);

var mockPublishedUrlProvider = CreateMockPublishedUrlProvider(NewPath);

var sut = CreateContentFinder(mockRedirectUrlService, mockUmbracoContextAccessor, mockPublishedUrlProvider);

var publishedRequestBuilder = CreatePublishedRequestBuilder(OldPath, withDomain: true);

var result = await sut.TryFindContent(publishedRequestBuilder);

AssertRedirectResult(publishedRequestBuilder, result);
}

private static Mock<IRedirectUrlService> CreateMockRedirectUrlService(string oldPath)
{
var mockRedirectUrlService = new Mock<IRedirectUrlService>();
mockRedirectUrlService
.Setup(x => x.GetMostRecentRedirectUrlAsync(It.Is<string>(y => y == oldPath), It.IsAny<string>()))
.ReturnsAsync(new RedirectUrl
{
ContentId = ContentId,
});
return mockRedirectUrlService;
}

private static Mock<IPublishedUrlProvider> CreateMockPublishedUrlProvider(string newPath)
{
var mockPublishedUrlProvider = new Mock<IPublishedUrlProvider>();
mockPublishedUrlProvider
.Setup(x => x.GetUrl(It.Is<IPublishedContent>(y => y.Id == ContentId), It.IsAny<UrlMode>(), It.IsAny<string?>(), It.IsAny<Uri?>()))
.Returns(newPath);
return mockPublishedUrlProvider;
}

private static Mock<IPublishedContent> CreateMockPublishedContent()
{
var mockContent = new Mock<IPublishedContent>();
mockContent
.SetupGet(x => x.Id)
.Returns(ContentId);
mockContent
.SetupGet(x => x.ContentType.ItemType)
.Returns(PublishedItemType.Content);
return mockContent;
}

private static Mock<IUmbracoContextAccessor> CreateMockUmbracoContextAccessor(Mock<IPublishedContent> mockContent)
{
var mockUmbracoContext = new Mock<IUmbracoContext>();
mockUmbracoContext
.Setup(x => x.Content.GetById(It.Is<int>(y => y == ContentId)))
.Returns(mockContent.Object);
var mockUmbracoContextAccessor = new Mock<IUmbracoContextAccessor>();
var umbracoContext = mockUmbracoContext.Object;
mockUmbracoContextAccessor
.Setup(x => x.TryGetUmbracoContext(out umbracoContext))
.Returns(true);
return mockUmbracoContextAccessor;
}

private static ContentFinderByRedirectUrl CreateContentFinder(
Mock<IRedirectUrlService> mockRedirectUrlService,
Mock<IUmbracoContextAccessor> mockUmbracoContextAccessor,
Mock<IPublishedUrlProvider> mockPublishedUrlProvider)
=> new ContentFinderByRedirectUrl(
mockRedirectUrlService.Object,
new NullLogger<ContentFinderByRedirectUrl>(),
mockPublishedUrlProvider.Object,
mockUmbracoContextAccessor.Object);

private static PublishedRequestBuilder CreatePublishedRequestBuilder(string path, bool withDomain = false)
{
var publishedRequestBuilder = new PublishedRequestBuilder(new Uri($"https://example.com{path}"), Mock.Of<IFileService>());
if (withDomain)
{
publishedRequestBuilder.SetDomain(new DomainAndUri(new Domain(1, "/en", DomainContentId, "en-US", false, 0), new Uri($"https://example.com{path}")));
}

return publishedRequestBuilder;
}

private static void AssertRedirectResult(PublishedRequestBuilder publishedRequestBuilder, bool result)
{
Assert.AreEqual(true, result);
Assert.AreEqual(HttpStatusCode.Moved, (HttpStatusCode)publishedRequestBuilder.ResponseStatusCode);
}
}