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

Introduce HttpRequest & HttpContext Fluid values #16979

Merged
merged 7 commits into from
Nov 9, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
Expand Up @@ -3,6 +3,7 @@ namespace OrchardCore.Liquid;
/// <summary>
/// This is a placeholder class that allows modules to extend the `HttpContext` property in the current Liquid scope.
/// </summary>
[Obsolete("This class is obsolete and will be removed in a future version.", error: true)]
public class LiquidHttpContextAccessor
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ namespace OrchardCore.Liquid;
/// <summary>
/// This is a placeholder class that allows modules to extend the `HttpRequest` property in the current Liquid scope.
/// </summary>
[Obsolete("This class is obsolete and will be removed in a future version.", error: true)]
public class LiquidRequestAccessor
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,45 +44,10 @@

options.Scope.SetValue("Environment", new HostingEnvironmentValue(_hostEnvironment));

options.Scope.SetValue("Request", new ObjectValue(new LiquidRequestAccessor()));
options.MemberAccessStrategy.Register<LiquidRequestAccessor, FluidValue>((obj, name, ctx) =>
{
var request = ((LiquidTemplateContext)ctx).Services.GetRequiredService<IHttpContextAccessor>().HttpContext?.Request;
if (request != null)
{
return name switch
{
nameof(HttpRequest.QueryString) => new StringValue(request.QueryString.Value),
nameof(HttpRequest.ContentType) => new StringValue(request.ContentType),
nameof(HttpRequest.ContentLength) => NumberValue.Create(request.ContentLength ?? 0),
nameof(HttpRequest.Cookies) => new ObjectValue(new CookieCollectionWrapper(request.Cookies)),
nameof(HttpRequest.Headers) => new ObjectValue(new HeaderDictionaryWrapper(request.Headers)),
nameof(HttpRequest.Query) => new ObjectValue(new QueryCollection(request.Query.ToDictionary(kv => kv.Key, kv => kv.Value))),
nameof(HttpRequest.Form) => request.HasFormContentType ? (FluidValue)new ObjectValue(request.Form) : NilValue.Instance,
nameof(HttpRequest.Protocol) => new StringValue(request.Protocol),
nameof(HttpRequest.Path) => new StringValue(request.Path.Value),
nameof(HttpRequest.PathBase) => new StringValue(request.PathBase.Value),
nameof(HttpRequest.Host) => new StringValue(request.Host.Value),
nameof(HttpRequest.IsHttps) => BooleanValue.Create(request.IsHttps),
nameof(HttpRequest.Scheme) => new StringValue(request.Scheme),
nameof(HttpRequest.Method) => new StringValue(request.Method),
nameof(HttpRequest.RouteValues) => new ObjectValue(new RouteValueDictionaryWrapper(request.RouteValues)),

// Provides correct escaping to reconstruct a request or redirect URI.
"UriHost" => new StringValue(request.Host.ToUriComponent(), encode: false),
"UriPath" => new StringValue(request.Path.ToUriComponent(), encode: false),
"UriPathBase" => new StringValue(request.PathBase.ToUriComponent(), encode: false),
"UriQueryString" => new StringValue(request.QueryString.ToUriComponent(), encode: false),

_ => NilValue.Instance
};
}

return NilValue.Instance;
});
options.Scope.SetValue("Request", new HttpRequestValue());

options.Scope.SetValue("HttpContext", new ObjectValue(new LiquidHttpContextAccessor()));

Check failure on line 49 in src/OrchardCore/OrchardCore.DisplayManagement.Liquid/TemplateOptionsConfigurations.cs

View workflow job for this annotation

GitHub Actions / Build & Test (ubuntu-latest)

'LiquidHttpContextAccessor' is obsolete: 'This class is obsolete and will be removed in a future version.'

Check failure on line 49 in src/OrchardCore/OrchardCore.DisplayManagement.Liquid/TemplateOptionsConfigurations.cs

View workflow job for this annotation

GitHub Actions / Build & Test (ubuntu-latest)

'LiquidHttpContextAccessor' is obsolete: 'This class is obsolete and will be removed in a future version.'

Check failure on line 49 in src/OrchardCore/OrchardCore.DisplayManagement.Liquid/TemplateOptionsConfigurations.cs

View workflow job for this annotation

GitHub Actions / Build & Test (windows-latest)

'LiquidHttpContextAccessor' is obsolete: 'This class is obsolete and will be removed in a future version.'

Check failure on line 49 in src/OrchardCore/OrchardCore.DisplayManagement.Liquid/TemplateOptionsConfigurations.cs

View workflow job for this annotation

GitHub Actions / Build & Test (windows-latest)

'LiquidHttpContextAccessor' is obsolete: 'This class is obsolete and will be removed in a future version.'
options.MemberAccessStrategy.Register<LiquidHttpContextAccessor, FluidValue>((obj, name, ctx) =>

Check failure on line 50 in src/OrchardCore/OrchardCore.DisplayManagement.Liquid/TemplateOptionsConfigurations.cs

View workflow job for this annotation

GitHub Actions / Build & Test (ubuntu-latest)

'LiquidHttpContextAccessor' is obsolete: 'This class is obsolete and will be removed in a future version.'

Check failure on line 50 in src/OrchardCore/OrchardCore.DisplayManagement.Liquid/TemplateOptionsConfigurations.cs

View workflow job for this annotation

GitHub Actions / Build & Test (ubuntu-latest)

'LiquidHttpContextAccessor' is obsolete: 'This class is obsolete and will be removed in a future version.'

Check failure on line 50 in src/OrchardCore/OrchardCore.DisplayManagement.Liquid/TemplateOptionsConfigurations.cs

View workflow job for this annotation

GitHub Actions / Build & Test (windows-latest)

'LiquidHttpContextAccessor' is obsolete: 'This class is obsolete and will be removed in a future version.'

Check failure on line 50 in src/OrchardCore/OrchardCore.DisplayManagement.Liquid/TemplateOptionsConfigurations.cs

View workflow job for this annotation

GitHub Actions / Build & Test (windows-latest)

'LiquidHttpContextAccessor' is obsolete: 'This class is obsolete and will be removed in a future version.'
{
var httpContext = ((LiquidTemplateContext)ctx).Services.GetRequiredService<IHttpContextAccessor>().HttpContext;
if (httpContext != null)
Expand Down Expand Up @@ -113,44 +78,4 @@
options.MemberAccessStrategy.Register<HeaderDictionaryWrapper, string[]>((headers, name) => headers.HeaderDictionary[name].ToArray());
options.MemberAccessStrategy.Register<RouteValueDictionaryWrapper, object>((headers, name) => headers.RouteValueDictionary[name]);
}

private sealed class CookieCollectionWrapper
{
public readonly IRequestCookieCollection RequestCookieCollection;

public CookieCollectionWrapper(IRequestCookieCollection requestCookieCollection)
{
RequestCookieCollection = requestCookieCollection;
}
}

private sealed class HeaderDictionaryWrapper
{
public readonly IHeaderDictionary HeaderDictionary;

public HeaderDictionaryWrapper(IHeaderDictionary headerDictionary)
{
HeaderDictionary = headerDictionary;
}
}

private sealed class HttpContextItemsWrapper
{
public readonly IDictionary<object, object> Items;

public HttpContextItemsWrapper(IDictionary<object, object> items)
{
Items = items;
}
}

private sealed class RouteValueDictionaryWrapper
{
public readonly IReadOnlyDictionary<string, object> RouteValueDictionary;

public RouteValueDictionaryWrapper(IReadOnlyDictionary<string, object> routeValueDictionary)
{
RouteValueDictionary = routeValueDictionary;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using Microsoft.AspNetCore.Http;

namespace OrchardCore.DisplayManagement.Liquid.Values;

internal sealed class CookieCollectionWrapper
{
public readonly IRequestCookieCollection RequestCookieCollection;

public CookieCollectionWrapper(IRequestCookieCollection requestCookieCollection)
{
RequestCookieCollection = requestCookieCollection;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using Microsoft.AspNetCore.Http;

namespace OrchardCore.DisplayManagement.Liquid.Values;

internal sealed class HeaderDictionaryWrapper
{
public readonly IHeaderDictionary HeaderDictionary;

public HeaderDictionaryWrapper(IHeaderDictionary headerDictionary)
{
HeaderDictionary = headerDictionary;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
namespace OrchardCore.DisplayManagement.Liquid.Values;

internal sealed class HttpContextItemsWrapper
{
public readonly IDictionary<object, object> Items;

public HttpContextItemsWrapper(IDictionary<object, object> items)
{
Items = items;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
using System.Globalization;
using System.Text.Encodings.Web;
using Fluid;
using Fluid.Values;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;
using OrchardCore.Liquid;

namespace OrchardCore.DisplayManagement.Liquid.Values;

internal sealed partial class HttpContextValue : FluidValue
hishamco marked this conversation as resolved.
Show resolved Hide resolved
hishamco marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly HttpContext _context;

public override FluidValues Type => FluidValues.Object;

/// <summary>
/// Creates a new instance of a <see cref="HttpContextValue"/> for the specified HTTP context.
/// </summary>
public HttpContextValue(HttpContext context = null)
{
ArgumentNullException.ThrowIfNull(context);

_context = context;
}

public override bool Equals(FluidValue other)
{
if (other is null)
{
return false;
}

return ToObjectValue() == other.ToObjectValue();
}

public override bool ToBooleanValue() => true;

public override decimal ToNumberValue() => 0;

public override object ToObjectValue() => _context;

public override string ToStringValue() => _context?.ToString();

#pragma warning disable CS0672 // Member overrides obsolete member
public override void WriteTo(TextWriter writer, TextEncoder encoder, CultureInfo cultureInfo)
#pragma warning restore CS0672 // Member overrides obsolete member
=> writer.Write(_context?.ToString());

public async override ValueTask WriteToAsync(TextWriter writer, TextEncoder encoder, CultureInfo cultureInfo)
=> await writer.WriteAsync(_context?.ToString());

public override ValueTask<FluidValue> GetValueAsync(string name, TemplateContext context)
{
var httpContext = _context ?? GetHttpContext(context).ToObjectValue() as HttpContext;
Copy link
Member

Choose a reason for hiding this comment

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

You can't cache the _context, it's specific to each request. Get a new value of the context every time this method is called. And don't wrap it in an ObjectValue

Copy link
Member

Choose a reason for hiding this comment

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

If you want to cache the HttpContext in case the fluid value is used multiple times (not common I'd say, and for what gain). You could use the TemplateContext object which has a dictionary. And use a private constant to put it there. But again might be more work that benefits so I wouldn't advise that.

Copy link
Member Author

Choose a reason for hiding this comment

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

You can't cache the _context, it's specific to each request. Get a new value of the context every time this method is called. And don't wrap it in an ObjectValue

The question again, how ToXXXValue() will be evaluated or should we ignore it for this particular object?

Copy link
Member

Choose a reason for hiding this comment

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

I fixed it. Now I understand your question. And now you have your answer.

Copy link
Member

Choose a reason for hiding this comment

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

Technically we have to decide what to render when we do {{ Request }}. Here it's returning "Request", but we could decide to return the url (even if it's more costly) or something that is valuable. I am fine if you think "" is actually better in that case, or "(Request)" if that helps users understand that it's an object.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm fine with your changes, I was struggling to display something useful if we render {{ Request }} but what you did is much simpler and doesn't require construction injection


return name switch
{
nameof(HttpContext.Items) => new ObjectValue(new HttpContextItemsWrapper(httpContext.Items)),
Copy link
Member Author

Choose a reason for hiding this comment

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

We could add Request, User .. etc and deprecate the Request object in a major release

Copy link
Member Author

Choose a reason for hiding this comment

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

What about this one @sebastienros?

_ => NilValue.Instance
};
}

private static ObjectValue GetHttpContext(TemplateContext context)
{
var ctx = context as LiquidTemplateContext
?? throw new InvalidOperationException($"An implementation of '{nameof(LiquidTemplateContext)}' is required");

var httpContextAccessor = ctx.Services.GetRequiredService<IHttpContextAccessor>();

return new ObjectValue(httpContextAccessor.HttpContext);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
using System.Globalization;
using System.Text.Encodings.Web;
using Fluid;
using Fluid.Values;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;
using OrchardCore.Liquid;

namespace OrchardCore.DisplayManagement.Liquid.Values;

internal sealed partial class HttpRequestValue : FluidValue
hishamco marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly HttpRequest _request;

public override FluidValues Type => FluidValues.Object;

/// <summary>
/// Creates a new instance of a <see cref="HttpRequestValue"/> for the specified HTTP request.
/// </summary>
public HttpRequestValue(HttpRequest request = null)
{
ArgumentNullException.ThrowIfNull(request);

_request = request;
}

public override bool Equals(FluidValue other)
{
if (other is null)
{
return false;
}

return ToStringValue() == other.ToStringValue();
}

public override bool ToBooleanValue() => true;

public override decimal ToNumberValue() => 0;

public override object ToObjectValue() => _request;

public override string ToStringValue() => _request?.Path;

#pragma warning disable CS0672 // Member overrides obsolete member
public override void WriteTo(TextWriter writer, TextEncoder encoder, CultureInfo cultureInfo)
#pragma warning restore CS0672 // Member overrides obsolete member
=> writer.Write(_request?.Path);

public async override ValueTask WriteToAsync(TextWriter writer, TextEncoder encoder, CultureInfo cultureInfo)
=> await writer.WriteAsync(_request?.Path);

public override ValueTask<FluidValue> GetValueAsync(string name, TemplateContext context)
{
var request = _request ?? GetHttpRequest(context).ToObjectValue() as HttpRequest;
hishamco marked this conversation as resolved.
Show resolved Hide resolved

return name switch
{
nameof(HttpRequest.QueryString) => new StringValue(request.QueryString.Value),
nameof(HttpRequest.ContentType) => new StringValue(request.ContentType),
nameof(HttpRequest.ContentLength) => NumberValue.Create(request.ContentLength ?? 0),
nameof(HttpRequest.Cookies) => new ObjectValue(new CookieCollectionWrapper(request.Cookies)),
nameof(HttpRequest.Headers) => new ObjectValue(new HeaderDictionaryWrapper(request.Headers)),
nameof(HttpRequest.Query) => new ObjectValue(new QueryCollection(request.Query.ToDictionary(kv => kv.Key, kv => kv.Value))),
nameof(HttpRequest.Form) => request.HasFormContentType ? (FluidValue)new ObjectValue(request.Form) : NilValue.Instance,
nameof(HttpRequest.Protocol) => new StringValue(request.Protocol),
nameof(HttpRequest.Path) => new StringValue(request.Path.Value),
nameof(HttpRequest.PathBase) => new StringValue(request.PathBase.Value),
nameof(HttpRequest.Host) => new StringValue(request.Host.Value),
nameof(HttpRequest.IsHttps) => BooleanValue.Create(request.IsHttps),
nameof(HttpRequest.Scheme) => new StringValue(request.Scheme),
nameof(HttpRequest.Method) => new StringValue(request.Method),
nameof(HttpRequest.RouteValues) => new ObjectValue(new RouteValueDictionaryWrapper(request.RouteValues)),

// Provides correct escaping to reconstruct a request or redirect URI.
"UriHost" => new StringValue(request.Host.ToUriComponent(), encode: false),
"UriPath" => new StringValue(request.Path.ToUriComponent(), encode: false),
"UriPathBase" => new StringValue(request.PathBase.ToUriComponent(), encode: false),
"UriQueryString" => new StringValue(request.QueryString.ToUriComponent(), encode: false),
_ => ValueTask.FromResult<FluidValue>(NilValue.Instance)
};
}

private static ObjectValue GetHttpRequest(TemplateContext context)
{
var ctx = context as LiquidTemplateContext
?? throw new InvalidOperationException($"An implementation of '{nameof(LiquidTemplateContext)}' is required");

var httpContext = ctx.Services.GetRequiredService<IHttpContextAccessor>().HttpContext;

return new ObjectValue(httpContext.Request);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
namespace OrchardCore.DisplayManagement.Liquid.Values;

internal sealed class RouteValueDictionaryWrapper
{
public readonly IReadOnlyDictionary<string, object> RouteValueDictionary;

public RouteValueDictionaryWrapper(IReadOnlyDictionary<string, object> routeValueDictionary)
{
RouteValueDictionary = routeValueDictionary;
}
}
Loading