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

V13: Eaglery route domains for virtual page controllers #16635

Merged
merged 3 commits into from
Jun 20, 2024
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
39 changes: 37 additions & 2 deletions src/Umbraco.Web.Website/Routing/EagerMatcherPolicy.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Reflection;

Check notice on line 1 in src/Umbraco.Web.Website/Routing/EagerMatcherPolicy.cs

View check run for this annotation

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

✅ Getting better: Overall Code Complexity

The mean cyclomatic complexity decreases from 5.57 to 5.38, 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.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Controllers;
using Microsoft.AspNetCore.Routing;
Expand All @@ -8,7 +8,9 @@
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Routing;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Web.Common.Controllers;
using Umbraco.Cms.Web.Common.Routing;
using Umbraco.Cms.Web.Website.Controllers;
using Umbraco.Extensions;

Expand Down Expand Up @@ -37,6 +39,8 @@
private readonly IRuntimeState _runtimeState;
private readonly EndpointDataSource _endpointDataSource;
private readonly UmbracoRequestPaths _umbracoRequestPaths;
private readonly IUmbracoContextAccessor _umbracoContextAccessor;
private readonly IPublishedRouter _publishedRouter;
private GlobalSettings _globalSettings;
private readonly Lazy<Endpoint> _installEndpoint;
private readonly Lazy<Endpoint> _renderEndpoint;
Expand All @@ -45,11 +49,15 @@
IRuntimeState runtimeState,
EndpointDataSource endpointDataSource,
UmbracoRequestPaths umbracoRequestPaths,
IOptionsMonitor<GlobalSettings> globalSettings)
IOptionsMonitor<GlobalSettings> globalSettings,
IUmbracoContextAccessor umbracoContextAccessor,
IPublishedRouter publishedRouter)
{
_runtimeState = runtimeState;
_endpointDataSource = endpointDataSource;
_umbracoRequestPaths = umbracoRequestPaths;
_umbracoContextAccessor = umbracoContextAccessor;
_publishedRouter = publishedRouter;

Check warning on line 60 in src/Umbraco.Web.Website/Routing/EagerMatcherPolicy.cs

View check run for this annotation

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

❌ New issue: Constructor Over-Injection

EagerMatcherPolicy has 6 arguments, threshold = 5. This constructor has too many arguments, indicating an object with low cohesion or missing function argument abstraction. Avoid adding more arguments.
_globalSettings = globalSettings.CurrentValue;
globalSettings.OnChange(settings => _globalSettings = settings);
_installEndpoint = new Lazy<Endpoint>(GetInstallEndpoint);
Expand Down Expand Up @@ -112,11 +120,22 @@
ControllerActionDescriptor? controllerDescriptor = routeEndpoint.Metadata.GetMetadata<ControllerActionDescriptor>();
TypeInfo? controllerTypeInfo = controllerDescriptor?.ControllerTypeInfo;
if (controllerTypeInfo is not null &&
(controllerTypeInfo.IsType<RenderController>() || controllerTypeInfo.IsType<SurfaceController>()))
(controllerTypeInfo.IsType<RenderController>()
|| controllerTypeInfo.IsType<SurfaceController>()))
{
return;
}

// If it's an UmbracoPageController we need to do some domain routing.
// We need to do this in oder to handle cultures for our Dictionary.
// This is because UmbracoPublishedContentCultureProvider is ued to set the Thread.CurrentThread.CurrentUICulture
// The CultureProvider is run before the actual routing, this means that our UmbracoVirtualPageFilterAttribute is hit AFTER the culture is set.
// Meaning we have to route the domain part already now, this is not pretty, but it beats having to look for content we know doesn't exist.
if (controllerTypeInfo is not null && controllerTypeInfo.IsType<UmbracoPageController>())
{
await RouteVirtualRequestAsync(httpContext);
}

Check notice on line 138 in src/Umbraco.Web.Website/Routing/EagerMatcherPolicy.cs

View check run for this annotation

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

ℹ Getting worse: Complex Method

ApplyAsync increases in cyclomatic complexity from 22 to 24, 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.
if (routeEndpoint.Order < lowestOrder)
{
// We have to ensure that the route is valid for the current request method.
Expand Down Expand Up @@ -153,6 +172,22 @@
}
}

private async Task RouteVirtualRequestAsync(HttpContext context)
{
if (_umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext? umbracoContext) is false)
{
return;
}

IPublishedRequestBuilder requestBuilder =
await _publishedRouter.CreateRequestAsync(umbracoContext.CleanedUmbracoUrl);
_publishedRouter.RouteDomain(requestBuilder);
// This is just a temporary RouteValues object just for culture which will be overwritten later
// so we can just use a dummy action descriptor.
var umbracoRouteValues = new UmbracoRouteValues(requestBuilder.Build(), new ControllerActionDescriptor());
context.Features.Set(umbracoRouteValues);
}

/// <summary>
/// Replaces the first endpoint candidate with the specified endpoint, invalidating all other candidates,
/// guaranteeing that the specified endpoint will be hit.
Expand Down
Loading