Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
16 changes: 11 additions & 5 deletions src/Umbraco.Cms.Api.Delivery/Services/RequestRedirectService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
private readonly IRedirectUrlService _redirectUrlService;
private readonly IApiPublishedContentCache _apiPublishedContentCache;
private readonly IApiContentRouteBuilder _apiContentRouteBuilder;
private readonly IDocumentUrlService _documentUrlService;
private readonly GlobalSettings _globalSettings;

public RequestRedirectService(
Expand All @@ -28,13 +29,15 @@
IRedirectUrlService redirectUrlService,
IApiPublishedContentCache apiPublishedContentCache,
IApiContentRouteBuilder apiContentRouteBuilder,
IOptions<GlobalSettings> globalSettings)
IOptions<GlobalSettings> globalSettings,
IDocumentUrlService documentUrlService)
: base(domainCache, httpContextAccessor, requestStartItemProviderAccessor)
{
_requestCultureService = requestCultureService;
_redirectUrlService = redirectUrlService;
_apiPublishedContentCache = apiPublishedContentCache;
_apiContentRouteBuilder = apiContentRouteBuilder;
_documentUrlService = documentUrlService;
_globalSettings = globalSettings.Value;
}

Expand All @@ -43,16 +46,19 @@
requestedPath = requestedPath.EnsureStartsWith("/");

IPublishedContent? startItem = GetStartItem();
var culture = _requestCultureService.GetRequestedCulture();

// must append the root content url segment if it is not hidden by config, because
// the URL tracking is based on the actual URL, including the root content url segment
if (_globalSettings.HideTopLevelNodeFromPath == false && startItem?.UrlSegment != null)
if (_globalSettings.HideTopLevelNodeFromPath == false && startItem is not null)
{
requestedPath = $"{startItem.UrlSegment.EnsureStartsWith("/")}{requestedPath}";
var startItemUrlSegment = _documentUrlService.GetUrlSegment(startItem.Key, culture ?? string.Empty, isDraft: false);
if (startItemUrlSegment is not null)
{
requestedPath = $"{startItemUrlSegment.EnsureStartsWith("/")}{requestedPath}";
}
}

Check warning on line 61 in src/Umbraco.Cms.Api.Delivery/Services/RequestRedirectService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v18/dev)

❌ Getting worse: Complex Method

GetRedirectRoute 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 61 in src/Umbraco.Cms.Api.Delivery/Services/RequestRedirectService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v18/dev)

❌ New issue: Bumpy Road Ahead

GetRedirectRoute 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.
var culture = _requestCultureService.GetRequestedCulture();

// important: redirect URLs are always tracked without trailing slashes
requestedPath = requestedPath.TrimEnd("/");
IRedirectUrl? redirectUrl = _redirectUrlService.GetMostRecentRedirectUrl(requestedPath, culture);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Umbraco.Cms.Core.DeliveryApi;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.PublishedCache;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Services.Navigation;
using Umbraco.Extensions;

Expand All @@ -14,6 +15,7 @@
private readonly IRequestPreviewService _requestPreviewService;
private readonly IDocumentNavigationQueryService _documentNavigationQueryService;
private readonly IPublishedContentCache _publishedContentCache;
private readonly IDocumentUrlService _documentUrlService;

// this provider lifetime is Scope, so we can cache this as a field
private IPublishedContent? _requestedStartContent;
Expand All @@ -23,14 +25,15 @@
IVariationContextAccessor variationContextAccessor,
IRequestPreviewService requestPreviewService,
IDocumentNavigationQueryService documentNavigationQueryService,
IPublishedContentCache publishedContentCache)
IPublishedContentCache publishedContentCache,
IDocumentUrlService documentUrlService)
: base(httpContextAccessor)
{

_variationContextAccessor = variationContextAccessor;
_requestPreviewService = requestPreviewService;
_documentNavigationQueryService = documentNavigationQueryService;
_publishedContentCache = publishedContentCache;
_documentUrlService = documentUrlService;

Check warning on line 36 in src/Umbraco.Cms.Api.Delivery/Services/RequestStartItemProvider.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v18/dev)

❌ New issue: Constructor Over-Injection

RequestStartItemProvider has 6 arguments, max arguments = 5. This constructor has too many arguments, indicating an object with low cohesion or missing function argument abstraction. Avoid adding more arguments.
}

/// <inheritdoc/>
Expand All @@ -47,14 +50,16 @@
return null;
}

var isPreview = _requestPreviewService.IsPreview();
_documentNavigationQueryService.TryGetRootKeys(out IEnumerable<Guid> rootKeys);
IEnumerable<IPublishedContent> rootContent = rootKeys
.Select(rootKey => _publishedContentCache.GetById(_requestPreviewService.IsPreview(), rootKey))
.Select(rootKey => _publishedContentCache.GetById(isPreview, rootKey))
.WhereNotNull();

var culture = _variationContextAccessor.VariationContext?.Culture ?? string.Empty;
_requestedStartContent = Guid.TryParse(headerValue, out Guid key)
? rootContent.FirstOrDefault(c => c.Key == key)
: rootContent.FirstOrDefault(c => c.UrlSegment(_variationContextAccessor).InvariantEquals(headerValue));
: rootContent.FirstOrDefault(c => _documentUrlService.GetUrlSegment(c.Key, culture, isPreview).InvariantEquals(headerValue));

return _requestedStartContent;
}
Expand Down
3 changes: 2 additions & 1 deletion src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ public ApiContentRouteBuilder(
return null;
}

var rootPath = root.UrlSegment(_variationContextAccessor, culture) ?? string.Empty;
var resolvedCulture = culture ?? _variationContextAccessor.VariationContext?.Culture ?? string.Empty;
var rootPath = _documentUrlService.GetUrlSegment(root.Key, resolvedCulture, isPreview) ?? string.Empty;

if (_globalSettings.HideTopLevelNodeFromPath == false)
{
Expand Down
53 changes: 0 additions & 53 deletions src/Umbraco.Core/Extensions/PublishedContentExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Umbraco.

Check notice on line 1 in src/Umbraco.Core/Extensions/PublishedContentExtensions.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v18/dev)

✅ No longer an issue: Low Cohesion

The number of different responsibilities in this module is no longer above the threshold

Check notice on line 1 in src/Umbraco.Core/Extensions/PublishedContentExtensions.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v18/dev)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 31.00% to 30.99%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
// See LICENSE for more details.

using System.Data;
Expand All @@ -8,7 +8,6 @@
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Membership;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.PublishedCache;
using Umbraco.Cms.Core.Routing;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Services.Navigation;
Expand Down Expand Up @@ -60,59 +59,7 @@
}

#endregion

Check notice on line 62 in src/Umbraco.Core/Extensions/PublishedContentExtensions.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v18/dev)

✅ No longer an issue: Complex Method

UrlSegment is no longer above the threshold for cyclomatic complexity
#region Url segment

/// <summary>
/// Gets the URL segment of the content item.
/// </summary>
/// <param name="content">The content item.</param>
/// <param name="variationContextAccessor"></param>
/// <param name="culture">
/// The specific culture to get the URL segment for. If null is used the current culture is used
/// (Default is null).
/// </param>
[Obsolete("Please use GetUrlSegment() on IDocumentUrlService instead. Scheduled for removal in Umbraco 18.")]
public static string? UrlSegment(this IPublishedContent content, IVariationContextAccessor? variationContextAccessor, string? culture = null)
{
ArgumentNullException.ThrowIfNull(content);

// The obsolete accessor only meaningfully applies to documents — the documented replacement is
// IDocumentUrlService, and media/members never had user-facing URL segments.
if (content.ItemType != PublishedItemType.Content)
{
return null;
}

string effectiveCulture = content.ContentType.VariesByCulture() is false
? string.Empty
: culture ?? variationContextAccessor?.VariationContext?.Culture ?? string.Empty;

// Variant content with no resolved culture has no associated segment; avoid an unnecessary
// ILanguageService.GetAsync("") lookup inside the service.
if (content.ContentType.VariesByCulture() && effectiveCulture.Length == 0)
{
return null;
}

// Use IDocumentUrlService to get the URL segment, aligning with the non-obsolete recommended approach.
// Fall back to in-memory lookup when the service isn't usable — either DI hasn't been bootstrapped
// (unit tests) or the service hasn't been initialised (integration tests not running full Umbraco
// startup). In production both hold.
IDocumentUrlService? documentUrlService = StaticServiceProvider.Instance?.GetService<IDocumentUrlService>();
if (documentUrlService is null || documentUrlService.IsInitialized is false)
{
return content.Cultures.TryGetValue(effectiveCulture, out PublishedCultureInfo? infos)
? infos.UrlSegment
: null;
}

var isDraft = content.IsDraft(effectiveCulture.Length == 0 ? null : effectiveCulture);
return documentUrlService.GetUrlSegment(content.Key, effectiveCulture, isDraft);
}

#endregion

#region IsComposedOf

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public interface IPublishedContent : IPublishedElement
/// <summary>
/// Gets the URL segment of the content item for the current culture.
/// </summary>
[Obsolete("Please use GetUrlSegment() on IDocumentUrlService instead. Scheduled for removal in Umbraco 20.")]
string? UrlSegment { get; }

/// <summary>
Expand Down
12 changes: 10 additions & 2 deletions src/Umbraco.Core/Models/PublishedContent/PublishedContentBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,16 @@ public abstract class PublishedContentBase : PublishableContentBase, IPublishedC
public virtual string Name => this.Name(_variationContextAccessor);

/// <inheritdoc />
[Obsolete("Please use GetUrlSegment() on IDocumentUrlService instead. Scheduled for removal in Umbraco 18.")]
public virtual string? UrlSegment => this.UrlSegment(_variationContextAccessor);
[Obsolete("Please use GetUrlSegment() on IDocumentUrlService instead. Scheduled for removal in Umbraco 20.")]
public virtual string? UrlSegment
{
get
{
#pragma warning disable CS0618 // Type or member is obsolete
return PublishedContentUrlSegmentResolver.Resolve(this, _variationContextAccessor);
#pragma warning restore CS0618 // Type or member is obsolete
}
}

/// <inheritdoc />
[Obsolete("Not supported for members. Scheduled for removal in Umbraco 18.")]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
using Microsoft.Extensions.DependencyInjection;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Web;

namespace Umbraco.Cms.Core.Models.PublishedContent;

/// <summary>
/// Resolves the URL segment of an <see cref="IPublishedContent"/> via <see cref="IDocumentUrlService.GetUrlSegment"/>,
/// honouring the ambient variation context and preview state.
/// </summary>
/// <remarks>
/// Helper used to satisfy the obsolete <see cref="IPublishedContent.UrlSegment"/> contract during the v18
/// deprecation period. Scheduled for removal in Umbraco 20 alongside the property itself. New code should call
/// <see cref="IDocumentUrlService.GetUrlSegment"/> directly.
/// </remarks>
[Obsolete("Helper for IPublishedContent.UrlSegment during the v18 deprecation period. Use IDocumentUrlService.GetUrlSegment() instead. Scheduled for removal in Umbraco 20.")]
public static class PublishedContentUrlSegmentResolver
{
/// <summary>
/// Resolves the URL segment for the specified content, using the supplied culture or falling back to the
/// variation context's current culture, and detecting preview state from the ambient <see cref="IUmbracoContext"/>.
/// </summary>
/// <param name="content">The content item.</param>
/// <param name="variationContextAccessor">Used to resolve the ambient culture when <paramref name="culture"/> is null.</param>
/// <param name="culture">An explicit culture, or null to use the ambient variation context.</param>
/// <returns>The URL segment, or null if no segment is available for the resolved culture.</returns>
public static string? Resolve(
IPublishedContent content,
IVariationContextAccessor? variationContextAccessor,
string? culture = null)
{
ArgumentNullException.ThrowIfNull(content);

var resolvedCulture = culture ?? variationContextAccessor?.VariationContext?.Culture ?? string.Empty;

IUmbracoContextAccessor umbracoContextAccessor = StaticServiceProvider.Instance.GetRequiredService<IUmbracoContextAccessor>();
var isDraft = umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext? umbracoContext) && umbracoContext.InPreviewMode;

IDocumentUrlService documentUrlService = StaticServiceProvider.Instance.GetRequiredService<IDocumentUrlService>();
return documentUrlService.GetUrlSegment(content.Key, resolvedCulture, isDraft);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,16 @@ protected PublishedContentWrapped(IPublishedContent content, IPublishedValueFall
=> _content = content;

/// <inheritdoc />
public virtual string? UrlSegment => _content.UrlSegment;
[Obsolete("Please use GetUrlSegment() on IDocumentUrlService instead. Scheduled for removal in Umbraco 20.")]
public virtual string? UrlSegment
{
get
{
#pragma warning disable CS0618 // Type or member is obsolete
return _content.UrlSegment;
#pragma warning restore CS0618 // Type or member is obsolete
}
}

/// <inheritdoc />
public virtual int Level => _content.Level;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ public object? this[string alias]
public IReadOnlyDictionary<string, PublishedCultureInfo> Cultures => _cultures ??= GetCultures();

/// <inheritdoc />
[Obsolete("Please use GetUrlSegment() on IDocumentUrlService instead. Scheduled for removal in Umbraco 20.")]
public string? UrlSegment { get; set; }

/// <inheritdoc />
Expand Down
1 change: 1 addition & 0 deletions src/Umbraco.Core/Security/PublishedExternalMember.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public PublishedExternalMember(ExternalMemberIdentity identity)
public string Name => _identity.Name ?? _identity.UserName;

/// <inheritdoc />
[Obsolete("This property is unused for external members and always returns null. Scheduled for removal in Umbraco 20.")]
public string? UrlSegment => null;

/// <inheritdoc />
Expand Down
12 changes: 10 additions & 2 deletions src/Umbraco.PublishedCache.HybridCache/PublishedContent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ public int Level
}

/// <inheritdoc />
[Obsolete("Please use GetUrlSegment() on IDocumentUrlService instead. Scheduled for removal in V16.")]
public virtual string? UrlSegment => this.UrlSegment(VariationContextAccessor);
[Obsolete("Please use GetUrlSegment() on IDocumentUrlService instead. Scheduled for removal in Umbraco 20.")]
public virtual string? UrlSegment
{
get
{
#pragma warning disable CS0618 // Type or member is obsolete
return PublishedContentUrlSegmentResolver.Resolve(this, VariationContextAccessor);
#pragma warning restore CS0618 // Type or member is obsolete
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public static class FriendlyPublishedContentExtensions
private static IPublishedValueFallback? _publishedValueFallback;
private static IMediaTypeService? _mediaTypeService;
private static IMemberTypeService? _memberTypeService;
private static IDocumentUrlService? _documentUrlService;

private static IVariationContextAccessor VariationContextAccessor
{
Expand Down Expand Up @@ -177,6 +178,15 @@ private static IMemberTypeService MemberTypeService
}
}

private static IDocumentUrlService DocumentUrlService
{
get
{
_documentUrlService ??= StaticServiceProvider.Instance.GetRequiredService<IDocumentUrlService>();
return _documentUrlService;
}
}

internal static void Reset()
{
_variationContextAccessor = null;
Expand All @@ -195,6 +205,7 @@ internal static void Reset()
_publishedValueFallback = null;
_mediaTypeService = null;
_memberTypeService = null;
_documentUrlService = null;
}

private static INavigationQueryService GetNavigationQueryService(IPublishedContent content)
Expand Down Expand Up @@ -253,10 +264,21 @@ public static string Name(
/// The specific culture to get the URL segment for. If null is used the current culture is used
/// (Default is null).
/// </param>
/// <remarks>
/// Convenience wrapper around <see cref="IDocumentUrlService.GetUrlSegment"/> for use in Razor templates
/// where injecting the service directly is awkward. Preview state is detected via the ambient
/// <see cref="IUmbracoContext"/>.
/// </remarks>
public static string? UrlSegment(
this IPublishedContent content,
string? culture = null)
=> content.UrlSegment(VariationContextAccessor, culture);
{
ArgumentNullException.ThrowIfNull(content);

var resolvedCulture = culture ?? VariationContextAccessor.VariationContext?.Culture ?? string.Empty;
var isDraft = UmbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext? umbracoContext) && umbracoContext.InPreviewMode;
return DocumentUrlService.GetUrlSegment(content.Key, resolvedCulture, isDraft);
}

/// <summary>
/// Gets the culture date of the content item.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using Examine.Search;
using Microsoft.AspNetCore.Html;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.PublishedCache;
using Umbraco.Cms.Core.Routing;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using NUnit.Framework;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Tests.Common.Testing;

Expand All @@ -9,6 +10,17 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.DeliveryApi.Request;
[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerFixture)]
public abstract class ApiContentRouteBuilderTestBase : ApiContentRequestTestBase
{
protected IDocumentUrlService DocumentUrlService => GetRequiredService<IDocumentUrlService>();

[SetUp]
public async Task EnsureDocumentUrlServiceInitialized()
{
if (DocumentUrlService.IsInitialized is false)
{
await DocumentUrlService.InitAsync(forceEmpty: true, CancellationToken.None);
}
}

protected IPublishedContent GetPublishedContent(Guid key)
{
var publishedContent = ClearAndEnsureUmbracoContext().Content.GetById(key);
Expand Down
Loading
Loading