-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Move AppendVersion & ResourceUrl filters to OC.Resources #16973
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
using Fluid; | ||
using Fluid.Values; | ||
using Microsoft.AspNetCore.Http; | ||
using Microsoft.AspNetCore.Mvc.ViewFeatures; | ||
using OrchardCore.Liquid; | ||
|
||
namespace OrchardCore.Resources.Liquid; | ||
|
||
public class AppendVersionFilter : ILiquidFilter | ||
{ | ||
private readonly IFileVersionProvider _fileVersionProvider; | ||
private readonly IHttpContextAccessor _httpContextAccessor; | ||
|
||
public AppendVersionFilter(IFileVersionProvider fileVersionProvider, IHttpContextAccessor httpContextAccessor) | ||
{ | ||
_fileVersionProvider = fileVersionProvider; | ||
_httpContextAccessor = httpContextAccessor; | ||
} | ||
|
||
public ValueTask<FluidValue> ProcessAsync(FluidValue input, FilterArguments arguments, LiquidTemplateContext ctx) | ||
{ | ||
var url = input.ToStringValue(); | ||
|
||
var imageUrl = _fileVersionProvider.AddFileVersionToPath(_httpContextAccessor.HttpContext.Request.PathBase, url); | ||
|
||
return ValueTask.FromResult<FluidValue>(new StringValue(imageUrl ?? url)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
using Fluid; | ||
using Fluid.Values; | ||
using Microsoft.AspNetCore.Http; | ||
using Microsoft.Extensions.Options; | ||
using OrchardCore.Liquid; | ||
using OrchardCore.ResourceManagement; | ||
|
||
namespace OrchardCore.Resources.Liquid; | ||
|
||
/// <summary> | ||
/// Returns the Cdn Base Url of the specified resource path. | ||
/// </summary> | ||
public class ResourceUrlFilter : ILiquidFilter | ||
{ | ||
private readonly IHttpContextAccessor _httpContextAccessor; | ||
private readonly ResourceManagementOptions _options; | ||
|
||
public ResourceUrlFilter(IHttpContextAccessor httpContextAccessor, IOptions<ResourceManagementOptions> options) | ||
{ | ||
_httpContextAccessor = httpContextAccessor; | ||
_options = options.Value; | ||
} | ||
|
||
public ValueTask<FluidValue> ProcessAsync(FluidValue input, FilterArguments arguments, LiquidTemplateContext ctx) | ||
{ | ||
var resourcePath = input.ToStringValue(); | ||
|
||
if (resourcePath.StartsWith("~/", StringComparison.Ordinal)) | ||
{ | ||
resourcePath = _httpContextAccessor.HttpContext.Request.PathBase.Add(resourcePath[1..]).Value; | ||
} | ||
|
||
// Don't prefix cdn if the path includes a protocol, i.e. is an external url, or is in debug mode. | ||
if (!_options.DebugMode && !string.IsNullOrEmpty(_options.CdnBaseUrl) && | ||
// Don't evaluate with Uri.TryCreate as it produces incorrect results on Linux. | ||
!resourcePath.StartsWith("https://", StringComparison.OrdinalIgnoreCase) && | ||
!resourcePath.StartsWith("http://", StringComparison.OrdinalIgnoreCase) && | ||
!resourcePath.StartsWith("//", StringComparison.OrdinalIgnoreCase) && | ||
!resourcePath.StartsWith("file://", StringComparison.OrdinalIgnoreCase)) | ||
{ | ||
resourcePath = _options.CdnBaseUrl + resourcePath; | ||
} | ||
|
||
return ValueTask.FromResult<FluidValue>(new StringValue(resourcePath)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
using Microsoft.Extensions.Options; | ||
using OrchardCore.DisplayManagement.Liquid; | ||
using OrchardCore.Environment.Shell.Configuration; | ||
using OrchardCore.Liquid; | ||
using OrchardCore.Modules; | ||
using OrchardCore.ResourceManagement; | ||
using OrchardCore.Resources.Liquid; | ||
|
@@ -39,3 +40,13 @@ public override void ConfigureServices(IServiceCollection serviceCollection) | |
serviceCollection.AddScoped<IResourcesTagHelperProcessor, ResourcesTagHelperProcessor>(); | ||
} | ||
} | ||
|
||
[RequireFeatures("OrchardCore.Liquid")] | ||
public sealed class ResourcesLiquidStartup : StartupBase | ||
{ | ||
public override void ConfigureServices(IServiceCollection services) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don’t think this is the right approach. If you look at line 24, you’ll see that we’re always registering Liquid-related services without needing to explicitly check for A better solution would be to introduce a new feature, Furthermore, the current line here would be replaced with This would simplify the code and improve modularity, ensuring that Liquid services are only registered when the new feature is explicitly enabled. |
||
{ | ||
services.AddLiquidFilter<AppendVersionFilter>("append_version") | ||
.AddLiquidFilter<ResourceUrlFilter>("resource_url"); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.