Skip to content
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

Support segments from querystring in preview #17819

Merged
merged 2 commits into from
Dec 17, 2024
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
1 change: 1 addition & 0 deletions src/Umbraco.Core/Routing/ContentFinderByIdPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public Task<bool> TryFindContent(IPublishedRequestBuilder frequest)
}

ResolveAndSetCultureOnRequest(frequest);
ResolveAndSetSegmentOnRequest(frequest);

frequest.SetPublishedContent(node);
if (_logger.IsEnabled(LogLevel.Debug))
Expand Down
12 changes: 12 additions & 0 deletions src/Umbraco.Core/Routing/ContentFinderByIdentifierPathBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ protected void ResolveAndSetCultureOnRequest(IPublishedRequestBuilder frequest)
}
}

protected void ResolveAndSetSegmentOnRequest(IPublishedRequestBuilder frequest)
{
var segmentFromQuerystring = _requestAccessor.GetQueryStringValue("segment");

// Check if we have a segment in the query string
if (!string.IsNullOrEmpty(segmentFromQuerystring))
{
// We're assuming it will match a segment
frequest.SetSegment(segmentFromQuerystring);
}
}

protected Task<bool> LogAndReturnFailure()
{
if (_logger.IsEnabled(LogLevel.Debug))
Expand Down
1 change: 1 addition & 0 deletions src/Umbraco.Core/Routing/ContentFinderByKeyPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public Task<bool> TryFindContent(IPublishedRequestBuilder frequest)
}

ResolveAndSetCultureOnRequest(frequest);
ResolveAndSetSegmentOnRequest(frequest);

frequest.SetPublishedContent(node);
if (_logger.IsEnabled(LogLevel.Debug))
Expand Down
5 changes: 5 additions & 0 deletions src/Umbraco.Core/Routing/IPublishedRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ public interface IPublishedRequest
/// </remarks>
string? Culture { get; }

/// <summary>
/// Gets the content request's segment (if any).
/// </summary>
string? Segment => null;

/// <summary>
/// Gets the url to redirect to, when the content request triggers a redirect.
/// </summary>
Expand Down
10 changes: 10 additions & 0 deletions src/Umbraco.Core/Routing/IPublishedRequestBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ public interface IPublishedRequestBuilder
/// </summary>
string? Culture { get; }

/// <summary>
/// Gets the segment assigned (if any)
/// </summary>
string? Segment => null;

/// <summary>
/// Gets a value indicating whether the current published content has been obtained
/// from the initial published content following internal redirections exclusively.
Expand Down Expand Up @@ -72,6 +77,11 @@ public interface IPublishedRequestBuilder
/// </summary>
IPublishedRequestBuilder SetCulture(string? culture);

/// <summary>
/// Sets the segment for the request
/// </summary>
IPublishedRequestBuilder SetSegment(string? segment) => this;

/// <summary>
/// Sets the found <see cref="IPublishedContent" /> for the request
/// </summary>
Expand Down
38 changes: 38 additions & 0 deletions src/Umbraco.Core/Routing/PublishedRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,39 @@

public class PublishedRequest : IPublishedRequest
{
[Obsolete("Please use the constructor that accepts a segment. Will be removed in V16.")]
public PublishedRequest(
Uri uri,
string absolutePathDecoded,
IPublishedContent? publishedContent,
bool isInternalRedirect,
ITemplate? template,
DomainAndUri? domain,
string? culture,
string? redirectUrl,
int? responseStatusCode,
IReadOnlyList<string>? cacheExtensions,
IReadOnlyDictionary<string, string>? headers,
bool setNoCacheHeader,
bool ignorePublishedContentCollisions)
: this(
uri,
absolutePathDecoded,
publishedContent,
isInternalRedirect,
template,
domain,
culture,
segment: null,
redirectUrl,
responseStatusCode,
cacheExtensions,
headers,
setNoCacheHeader,
ignorePublishedContentCollisions)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="PublishedRequest" /> class.
/// </summary>
Expand All @@ -16,20 +49,22 @@
ITemplate? template,
DomainAndUri? domain,
string? culture,
string? segment,
string? redirectUrl,
int? responseStatusCode,
IReadOnlyList<string>? cacheExtensions,
IReadOnlyDictionary<string, string>? headers,
bool setNoCacheHeader,
bool ignorePublishedContentCollisions)
{
Uri = uri ?? throw new ArgumentNullException(nameof(uri));
AbsolutePathDecoded = absolutePathDecoded ?? throw new ArgumentNullException(nameof(absolutePathDecoded));
PublishedContent = publishedContent;
IsInternalRedirect = isInternalRedirect;
Template = template;
Domain = domain;
Culture = culture;
Segment = segment;

Check notice on line 67 in src/Umbraco.Core/Routing/PublishedRequest.cs

View check run for this annotation

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

ℹ New issue: Constructor Over-Injection

PublishedRequest has 14 arguments, threshold = 5. This constructor has too many arguments, indicating an object with low cohesion or missing function argument abstraction. Avoid adding more arguments.
RedirectUrl = redirectUrl;
ResponseStatusCode = responseStatusCode;
CacheExtensions = cacheExtensions;
Expand Down Expand Up @@ -62,6 +97,9 @@
/// <inheritdoc />
public string? Culture { get; }

/// <inheritdoc />
public string? Segment { get; }

/// <inheritdoc />
public string? RedirectUrl { get; }

Expand Down
11 changes: 11 additions & 0 deletions src/Umbraco.Core/Routing/PublishedRequestBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ public PublishedRequestBuilder(Uri uri, IFileService fileService)
/// <inheritdoc />
public string? Culture { get; private set; }

/// <inheritdoc />
public string? Segment { get; private set; }

/// <inheritdoc />
public ITemplate? Template { get; private set; }

Expand Down Expand Up @@ -69,6 +72,7 @@ private set
Template,
Domain,
Culture,
Segment,
_redirectUrl,
_responseStatus.HasValue ? (int?)_responseStatus : null,
_cacheExtensions,
Expand Down Expand Up @@ -97,6 +101,13 @@ public IPublishedRequestBuilder SetCulture(string? culture)
return this;
}

/// <inheritdoc />
public IPublishedRequestBuilder SetSegment(string? segment)
{
Segment = segment;
return this;
}

/// <inheritdoc />
public IPublishedRequestBuilder SetDomain(DomainAndUri domain)
{
Expand Down
17 changes: 9 additions & 8 deletions src/Umbraco.Core/Routing/PublishedRouter.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/PublishedRouter.cs

View check run for this annotation

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

ℹ Getting worse: Overall Code Complexity

The mean cyclomatic complexity increases from 6.19 to 6.24, 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.Logging;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Configuration.Models;
Expand Down Expand Up @@ -136,6 +136,7 @@
builder.SetDomain(request.Domain);
}
builder.SetCulture(request.Culture);
builder.SetSegment(request.Segment);

// set to the new content (or null if specified)
builder.SetPublishedContent(publishedContent);
Expand Down Expand Up @@ -181,7 +182,7 @@
}

// set the culture -- again, 'cos it might have changed in the event handler
SetVariationContext(result.Culture);
SetVariationContext(result.Culture, result.Segment);

return result;
}
Expand All @@ -205,15 +206,15 @@
return request.Build();
}

private void SetVariationContext(string? culture)
private void SetVariationContext(string? culture, string? segment)
{
VariationContext? variationContext = _variationContextAccessor.VariationContext;
if (variationContext != null && variationContext.Culture == culture)
if (variationContext != null && variationContext.Culture == culture && variationContext.Segment == segment)

Check warning on line 212 in src/Umbraco.Core/Routing/PublishedRouter.cs

View check run for this annotation

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

❌ New issue: Complex Conditional

SetVariationContext has 1 complex conditionals with 2 branches, threshold = 2. A complex conditional is an expression inside a branch (e.g. if, for, while) which consists of multiple, logical operators such as AND/OR. The more logical operators in an expression, the more severe the code smell.
{
return;
}

_variationContextAccessor.VariationContext = new VariationContext(culture);
_variationContextAccessor.VariationContext = new VariationContext(culture, segment);
}

private async Task RouteRequestInternalAsync(IPublishedRequestBuilder builder, bool skipContentFinders = false)
Expand All @@ -226,7 +227,7 @@
}

// set the culture
SetVariationContext(builder.Culture);
SetVariationContext(builder.Culture, builder.Segment);

var foundContentByFinders = false;

Expand Down Expand Up @@ -261,7 +262,7 @@
HandleWildcardDomains(builder);

// set the culture -- again, 'cos it might have changed due to a finder or wildcard domain
SetVariationContext(builder.Culture);
SetVariationContext(builder.Culture, builder.Segment);
}

// trigger the routing request (used to be called Prepared) event - at that point it is still possible to change about anything
Expand All @@ -278,15 +279,15 @@
{
var found = FindAndSetDomain(request);
HandleWildcardDomains(request);
SetVariationContext(request.Culture);
SetVariationContext(request.Culture, request.Segment);
return found;
}

/// <inheritdoc />
public bool UpdateVariationContext(Uri uri)
{
DomainAndUri? domain = FindDomain(uri, out _);
SetVariationContext(domain?.Culture);
SetVariationContext(domain?.Culture, null);
return domain?.Culture is not null;
}

Expand Down
5 changes: 3 additions & 2 deletions src/Umbraco.Web.Common/Controllers/RenderController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,11 @@ public override async Task OnActionExecutionAsync(ActionExecutingContext context
if (_logger.IsEnabled(Microsoft.Extensions.Logging.LogLevel.Debug))
{
_logger.LogDebug(
"Response status: Content={Content}, StatusCode={ResponseStatusCode}, Culture={Culture}",
"Response status: Content={Content}, StatusCode={ResponseStatusCode}, Culture={Culture}, Segment={Segment}",
pcr.PublishedContent?.Id ?? -1,
pcr.ResponseStatusCode,
pcr.Culture);
pcr.Culture,
pcr.Segment);
}

UmbracoRouteResult routeStatus = pcr.GetRouteResult();
Expand Down
1 change: 1 addition & 0 deletions src/Umbraco.Web.Common/Templates/TemplateRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ public async Task RenderAsync(int pageId, int? altTemplateId, StringWriter write
else
{
requestBuilder.SetCulture(umbracoContext.PublishedRequest.Culture);
requestBuilder.SetSegment(umbracoContext.PublishedRequest.Segment);
}

// set the doc that was found by id
Expand Down
Loading