Skip to content

Cleaned up constructors, regions and variables in NewDefaultUrlProvider #19403

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 1 commit into from
May 26, 2025
Merged
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
87 changes: 61 additions & 26 deletions src/Umbraco.Core/Routing/NewDefaultUrlProvider.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Globalization;

Check notice on line 1 in src/Umbraco.Core/Routing/NewDefaultUrlProvider.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ Getting better: Overall Code Complexity

The mean cyclomatic complexity decreases from 5.63 to 5.11, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
Expand All @@ -15,54 +15,102 @@
namespace Umbraco.Cms.Core.Routing;

/// <summary>
/// Provides urls.
/// Provides URLs.
/// </summary>
public class NewDefaultUrlProvider : IUrlProvider
{
private readonly ILocalizationService _localizationService;
private readonly IPublishedContentCache _publishedContentCache;
private readonly IDomainCache _domainCache;
private readonly IIdKeyMap _idKeyMap;
private readonly IDocumentUrlService _documentUrlService;
private readonly IDocumentNavigationQueryService _navigationQueryService;
private readonly IPublishedContentStatusFilteringService _publishedContentStatusFilteringService;
private readonly ILocalizedTextService? _localizedTextService;
private readonly ILogger<DefaultUrlProvider> _logger;
private readonly ISiteDomainMapper _siteDomainMapper;
private readonly IUmbracoContextAccessor _umbracoContextAccessor;
private readonly UriUtility _uriUtility;
private RequestHandlerSettings _requestSettings;
private readonly ILanguageService _languageService;

// TODO (V17): When removing the obsolete constructors, remove the unused localizationService parameter from the constructor (we can't do it now
// because it is used in the obsolete constructors and leads to an ambigious constructor error).
// See also if we can make GetUrlFromRoute asynchronous and avoid the GetAwaiter().GetResult() in when using ILanguageService.

/// <summary>
/// Initializes a new instance of the <see cref="NewDefaultUrlProvider"/> class.
/// </summary>
public NewDefaultUrlProvider(
IOptionsMonitor<RequestHandlerSettings> requestSettings,
ILogger<DefaultUrlProvider> logger,
ISiteDomainMapper siteDomainMapper,
IUmbracoContextAccessor umbracoContextAccessor,
UriUtility uriUtility,
#pragma warning disable CS0618 // Type or member is obsolete
#pragma warning disable IDE0060 // Remove unused parameter
ILocalizationService localizationService,
#pragma warning restore IDE0060 // Remove unused parameter
#pragma warning restore CS0618 // Type or member is obsolete
IPublishedContentCache publishedContentCache,
IDomainCache domainCache,
IIdKeyMap idKeyMap,
IDocumentUrlService documentUrlService,
IDocumentNavigationQueryService navigationQueryService,
IPublishedContentStatusFilteringService publishedContentStatusFilteringService)
IPublishedContentStatusFilteringService publishedContentStatusFilteringService,
ILanguageService languageService)
{
_requestSettings = requestSettings.CurrentValue;
_logger = logger;
_siteDomainMapper = siteDomainMapper;
_umbracoContextAccessor = umbracoContextAccessor;
_uriUtility = uriUtility;
_localizationService = localizationService;
_publishedContentCache = publishedContentCache;
_domainCache = domainCache;
_idKeyMap = idKeyMap;
_documentUrlService = documentUrlService;
_navigationQueryService = navigationQueryService;
_publishedContentStatusFilteringService = publishedContentStatusFilteringService;
_languageService = languageService;

requestSettings.OnChange(x => _requestSettings = x);
}

/// <summary>
/// Initializes a new instance of the <see cref="NewDefaultUrlProvider"/> class.
/// </summary>
[Obsolete("Use the non-obsolete constructor. Scheduled for removal in V17.")]
public NewDefaultUrlProvider(
IOptionsMonitor<RequestHandlerSettings> requestSettings,
ILogger<DefaultUrlProvider> logger,
ISiteDomainMapper siteDomainMapper,
IUmbracoContextAccessor umbracoContextAccessor,
UriUtility uriUtility,
ILocalizationService localizationService,
IPublishedContentCache publishedContentCache,
IDomainCache domainCache,
IIdKeyMap idKeyMap,
IDocumentUrlService documentUrlService,
IDocumentNavigationQueryService navigationQueryService,
IPublishedContentStatusFilteringService publishedContentStatusFilteringService)
: this(
requestSettings,
logger,
siteDomainMapper,
umbracoContextAccessor,
uriUtility,
localizationService,
publishedContentCache,
domainCache,
idKeyMap,
documentUrlService,
navigationQueryService,
publishedContentStatusFilteringService,
StaticServiceProvider.Instance.GetRequiredService<ILanguageService>())
{
}

Check warning on line 109 in src/Umbraco.Core/Routing/NewDefaultUrlProvider.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Code Duplication

The module contains 2 functions with similar structure: NewDefaultUrlProvider,NewDefaultUrlProvider. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

/// <summary>
/// Initializes a new instance of the <see cref="NewDefaultUrlProvider"/> class.
/// </summary>
[Obsolete("Use the non-obsolete constructor. Scheduled for removal in V17.")]
public NewDefaultUrlProvider(
IOptionsMonitor<RequestHandlerSettings> requestSettings,
Expand Down Expand Up @@ -92,8 +140,6 @@
{
}

#region GetOtherUrls

/// <summary>
/// Gets the other URLs of a published content.
/// </summary>
Expand All @@ -108,14 +154,14 @@
/// </remarks>
public virtual IEnumerable<UrlInfo> GetOtherUrls(int id, Uri current)
{
var keyAttempt = _idKeyMap.GetKeyForId(id, UmbracoObjectTypes.Document);
Attempt<Guid> keyAttempt = _idKeyMap.GetKeyForId(id, UmbracoObjectTypes.Document);

if (keyAttempt.Success is false)
{
yield break;
}

var key = keyAttempt.Result;
Guid key = keyAttempt.Result;

IPublishedContent? node = _publishedContentCache.GetById(key);
if (node == null)
Expand Down Expand Up @@ -156,7 +202,7 @@

// need to strip off the leading ID for the route if it exists (occurs if the route is for a node with a domain assigned)
var pos = route.IndexOf('/', StringComparison.Ordinal);
var path = pos == 0 ? route : route.Substring(pos);
var path = pos == 0 ? route : route[pos..];

var uri = new Uri(CombinePaths(d.Uri.GetLeftPart(UriPartial.Path), path));
uri = _uriUtility.UriFromUmbraco(uri, _requestSettings);
Expand All @@ -178,17 +224,9 @@
private string GetLegacyRouteFormatById(Guid key, string? culture)
{
var isDraft = _umbracoContextAccessor.GetRequiredUmbracoContext().InPreviewMode;


return _documentUrlService.GetLegacyRouteFormat(key, culture, isDraft);


}

#endregion

#region GetUrl

/// <inheritdoc />
public virtual UrlInfo? GetUrl(IPublishedContent content, UrlMode mode, string? culture, Uri current)
{
Expand Down Expand Up @@ -217,6 +255,9 @@
return GetUrlFromRoute(route, content.Id, current, mode, culture);
}

/// <summary>
/// Gets the URL from the provided route.
/// </summary>
internal UrlInfo? GetUrlFromRoute(
string? route,
int id,
Expand All @@ -226,7 +267,7 @@
{
if (string.IsNullOrWhiteSpace(route) || route.Equals("#"))
{
if (_logger.IsEnabled(Microsoft.Extensions.Logging.LogLevel.Debug))
if (_logger.IsEnabled(LogLevel.Debug))
{
_logger.LogDebug(
"Couldn't find any page with nodeId={NodeId}. This is most likely caused by the page not being published.",
Expand All @@ -248,7 +289,7 @@
current,
culture);

var defaultCulture = _localizationService.GetDefaultLanguageIsoCode();
var defaultCulture = _languageService.GetDefaultIsoCodeAsync().GetAwaiter().GetResult();
if (domainUri is not null ||
string.IsNullOrEmpty(culture) ||
culture.Equals(defaultCulture, StringComparison.InvariantCultureIgnoreCase))
Expand All @@ -260,10 +301,6 @@
return null;
}

#endregion

#region Utilities

private Uri AssembleUrl(DomainAndUri? domainUri, string path, Uri current, UrlMode mode)
{
Uri uri;
Expand Down Expand Up @@ -331,6 +368,4 @@
var path = path1.TrimEnd(Constants.CharArrays.ForwardSlash) + path2;
return path == "/" ? path : path.TrimEnd(Constants.CharArrays.ForwardSlash);
}

#endregion
}