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
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
41 changes: 0 additions & 41 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)

✅ Getting better: Low Cohesion

The number of different responsibilities decreases from 5 to 4, threshold = 3. Cohesion is calculated using the LCOM4 metric. Low cohesion means that the module/class has multiple unrelated responsibilities, doing too many things and breaking the Single Responsibility Principle.

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: Code Duplication

reduced similar code in: Name,UrlSegment. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

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.24% to 31.22%, 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 Down Expand Up @@ -61,47 +61,6 @@

#endregion

#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)
{
if (content == null)
{
throw new ArgumentNullException(nameof(content));
}

// invariant has invariant value (whatever the requested culture)
if (!content.ContentType.VariesByCulture())
{
return content.Cultures.TryGetValue(string.Empty, out PublishedCultureInfo? invariantInfos)
? invariantInfos.UrlSegment
: null;
}

// handle context culture for variant
if (culture == null)
{
culture = variationContextAccessor?.VariationContext?.Culture ?? string.Empty;
}

// get
return culture != string.Empty && content.Cultures.TryGetValue(culture, out PublishedCultureInfo? infos)
? infos.UrlSegment
: null;
}

#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 19.")]
Comment thread
AndyButland marked this conversation as resolved.
Outdated
Comment thread
AndyButland marked this conversation as resolved.
Outdated
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 19.")]
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,44 @@
using Umbraco.Extensions;

namespace Umbraco.Cms.Core.Models.PublishedContent;

/// <summary>
/// Resolves the URL segment of an <see cref="IPublishedContent"/> from its <see cref="IPublishedContent.Cultures"/>
/// dictionary, honouring culture variance and the ambient variation context.
/// </summary>
/// <remarks>
/// Internal helper used to satisfy the obsolete <see cref="IPublishedContent.UrlSegment"/> contract during the v18
/// deprecation period. Scheduled for removal in Umbraco 19 alongside the property itself. New code should use
/// <c>IDocumentUrlService.GetUrlSegment()</c>.
/// </remarks>
[Obsolete("Helper for IPublishedContent.UrlSegment during the v18 deprecation period. Use IDocumentUrlService.GetUrlSegment() instead. Scheduled for removal in Umbraco 19.")]
Comment thread
AndyButland marked this conversation as resolved.
Outdated
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 for variant content.
/// </summary>
/// <param name="content">The content item.</param>
/// <param name="variationContextAccessor">Used to resolve the ambient culture for variant content 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);

if (content.ContentType.VariesByCulture() is false)
{
return content.Cultures.TryGetValue(string.Empty, out PublishedCultureInfo? invariantInfos)
? invariantInfos.UrlSegment
: null;
}

culture ??= variationContextAccessor?.VariationContext?.Culture ?? string.Empty;
return culture != string.Empty && content.Cultures.TryGetValue(culture, out PublishedCultureInfo? infos)
? infos.UrlSegment
: null;
}
}
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 19.")]
Comment thread
AndyButland marked this conversation as resolved.
Outdated
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 19.")]
Comment thread
AndyButland marked this conversation as resolved.
Outdated
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("Please use GetUrlSegment() on IDocumentUrlService instead. Scheduled for removal in Umbraco 19.")]
Comment thread
AndyButland marked this conversation as resolved.
Outdated
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 19.")]
Comment thread
AndyButland marked this conversation as resolved.
Outdated
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 @@ -264,10 +264,13 @@ 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>
[Obsolete("Please use GetUrlSegment() on IDocumentUrlService instead. Scheduled for removal in Umbraco 19.")]
Comment thread
AndyButland marked this conversation as resolved.
Outdated
public static string? UrlSegment(
this IPublishedContent content,
string? culture = null)
=> content.UrlSegment(VariationContextAccessor, culture);
#pragma warning disable CS0618 // Type or member is obsolete
=> PublishedContentUrlSegmentResolver.Resolve(content, VariationContextAccessor, culture);
#pragma warning restore CS0618 // Type or member is obsolete

/// <summary>
/// Gets the culture date of the content item.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ public void ContentBuilder_MapsContentDataAndPropertiesCorrectly()
var key = Guid.NewGuid();
var urlSegment = "url-segment";
var name = "The page";
ConfigurePublishedContentMock(content, key, name, urlSegment, contentType.Object, new[] { prop1, prop2 });
ConfigurePublishedContentMock(content, key, name, contentType.Object, new[] { prop1, prop2 });
content.SetupGet(c => c.CreateDate).Returns(new DateTime(2023, 06, 01));
content.SetupGet(c => c.UpdateDate).Returns(new DateTime(2023, 07, 12));

var apiContentRouteProvider = new Mock<IApiContentPathProvider>();
apiContentRouteProvider
.Setup(p => p.GetContentPath(It.IsAny<IPublishedContent>(), It.IsAny<string?>()))
.Returns((IPublishedContent c, string? culture) => $"url:{c.UrlSegment}");
.Returns((IPublishedContent c, string? culture) => $"url:{urlSegment}");

var navigationQueryServiceMock = new Mock<IDocumentNavigationQueryService>();
IEnumerable<Guid> ancestorsKeys = [];
Expand Down Expand Up @@ -76,7 +76,7 @@ public void ContentBuilder_MapsContentDatesCorrectlyForCultureVariance(string cu
var key = Guid.NewGuid();
var urlSegment = "url-segment";
var name = "The page";
ConfigurePublishedContentMock(content, key, name, urlSegment, contentType.Object, []);
ConfigurePublishedContentMock(content, key, name, contentType.Object, []);
content.SetupGet(c => c.CreateDate).Returns(new DateTime(2023, 07, 02));
content
.SetupGet(c => c.Cultures)
Expand All @@ -89,7 +89,7 @@ public void ContentBuilder_MapsContentDatesCorrectlyForCultureVariance(string cu
var routeBuilder = new Mock<IApiContentRouteBuilder>();
routeBuilder
.Setup(r => r.Build(content.Object, It.IsAny<string?>()))
.Returns(new ApiContentRoute(content.Object.UrlSegment!, new ApiContentStartItem(Guid.NewGuid(), "/")));
.Returns(new ApiContentRoute(urlSegment, new ApiContentStartItem(Guid.NewGuid(), "/")));

var variationContextAccessor = new TestVariationContextAccessor { VariationContext = new VariationContext(culture) };

Expand All @@ -109,15 +109,15 @@ public void ContentBuilder_CanCustomizeContentNameInDeliveryApiOutput()
var contentType = new Mock<IPublishedContentType>();
contentType.SetupGet(c => c.Alias).Returns("thePageType");

ConfigurePublishedContentMock(content, Guid.NewGuid(), "The page", "the-page", contentType.Object, Array.Empty<PublishedPropertyBase>());
ConfigurePublishedContentMock(content, Guid.NewGuid(), "The page", contentType.Object, Array.Empty<PublishedPropertyBase>());

var customNameProvider = new Mock<IApiContentNameProvider>();
customNameProvider.Setup(n => n.GetName(content.Object)).Returns($"Custom name for: {content.Object.Name}");

var routeBuilder = new Mock<IApiContentRouteBuilder>();
routeBuilder
.Setup(r => r.Build(content.Object, It.IsAny<string?>()))
.Returns(new ApiContentRoute(content.Object.UrlSegment!, new ApiContentStartItem(Guid.NewGuid(), "/")));
.Returns(new ApiContentRoute("the-page", new ApiContentStartItem(Guid.NewGuid(), "/")));

var builder = new ApiContentBuilder(customNameProvider.Object, routeBuilder.Object, CreateOutputExpansionStrategyAccessor(), CreateVariationContextAccessor());
var result = builder.Build(content.Object);
Expand All @@ -134,7 +134,7 @@ public void ContentBuilder_ReturnsNullForUnRoutableContent()
var contentType = new Mock<IPublishedContentType>();
contentType.SetupGet(c => c.Alias).Returns("thePageType");

ConfigurePublishedContentMock(content, Guid.NewGuid(), "The page", "the-page", contentType.Object, Array.Empty<PublishedPropertyBase>());
ConfigurePublishedContentMock(content, Guid.NewGuid(), "The page", contentType.Object, Array.Empty<PublishedPropertyBase>());

var routeBuilder = new Mock<IApiContentRouteBuilder>();
routeBuilder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ public void ContentPickerValueConverter_RendersContentProperties()
var key = Guid.NewGuid();
var urlSegment = "page-url-segment";
var name = "The page";
ConfigurePublishedContentMock(content, key, name, urlSegment, PublishedContentType, new[] { prop1, prop2 });
ConfigurePublishedContentMock(content, key, name, PublishedContentType, new[] { prop1, prop2 });

PublishedUrlProviderMock
.Setup(p => p.GetUrl(content.Object, It.IsAny<UrlMode>(), It.IsAny<string?>(), It.IsAny<Uri?>()))
.Returns(content.Object.UrlSegment);
.Returns(urlSegment);
PublishedContentCacheMock
.Setup(pcc => pcc.GetById(false, key))
.Returns(content.Object);
Expand Down
Loading
Loading