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

Move AppendVersion & ResourceUrl filters to OC.Resources #16973

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
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) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
!resourcePath.StartsWith("//", StringComparison.OrdinalIgnoreCase) &&
!resourcePath.StartsWith("//", StringComparison.Ordinal) &&

!resourcePath.StartsWith("file://", StringComparison.OrdinalIgnoreCase))
{
resourcePath = _options.CdnBaseUrl + resourcePath;
}

return ValueTask.FromResult<FluidValue>(new StringValue(resourcePath));
}
}
11 changes: 11 additions & 0 deletions src/OrchardCore.Modules/OrchardCore.Resources/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The 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 OrchardCore.Liquid. This is because Liquid support is automatically included when .AddOrchardCms() is called in the web project. In other words, the Liquid services are available even if OrchardCore.Liquid is not explicitly enabled.

A better solution would be to introduce a new feature, OrchardCore.Liquid.Core, which would register the necessary Liquid services. This feature would be enabled through dependencies and could be used in the current context. We can then move all Liquid-related configuration into a new startup class dedicated to this feature. This approach would allow us to cleanly encapsulate the registration of Liquid services, as shown here: Liquid Service Registration.

Furthermore, the current line here would be replaced with .AddGlobalFeatures("OrchardCore.Liquid.Core").

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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

namespace OrchardCore.DisplayManagement.Liquid.Filters;

[Obsolete("This filter is obsolete. Use OrchardCore.Resources.Liquid.AppendVersionFilter instead.")]
public class AppendVersionFilter : ILiquidFilter
{
private readonly IFileVersionProvider _fileVersionProvider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace OrchardCore.DisplayManagement.Liquid.Filters;
/// <summary>
/// Returns the Cdn Base Url of the specified resource path.
/// </summary>
[Obsolete("This filter is obsolete. Use OrchardCore.Resources.Liquid.ResourceUrlFilter instead.")]
public class ResourceUrlFilter : ILiquidFilter
{
private readonly IHttpContextAccessor _httpContextAccessor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,8 @@ public static OrchardCoreBuilder AddLiquidViews(this OrchardCoreBuilder builder)

services.AddSingleton<IConfigureOptions<TemplateOptions>, TemplateOptionsConfigurations>();


#pragma warning disable CS0618 // Type or member is obsolete
services.AddLiquidFilter<AppendVersionFilter>("append_version")
.AddLiquidFilter<ResourceUrlFilter>("resource_url")
.AddLiquidFilter<SanitizeHtmlFilter>("sanitize_html")
services.AddLiquidFilter<SanitizeHtmlFilter>("sanitize_html")

// Deprecated, remove in a future version.
.AddLiquidFilter<SupportedCulturesFilter>("supported_cultures");
Expand Down