Introduce HttpRequest & HttpContext Fluid values #16979
Conversation
|
|
||
| return name switch | ||
| { | ||
| nameof(HttpContext.Items) => new ObjectValue(new HttpContextItemsWrapper(httpContext.Items)), |
There was a problem hiding this comment.
We could add Request, User .. etc and deprecate the Request object in a major release
There was a problem hiding this comment.
What about this one @sebastienros?
src/OrchardCore/OrchardCore.DisplayManagement.Liquid/Values/HttpRequestValue.cs
Outdated
Show resolved
Hide resolved
src/OrchardCore/OrchardCore.DisplayManagement.Liquid/Values/HttpContextValue.cs
Outdated
Show resolved
Hide resolved
|
|
||
| public override ValueTask<FluidValue> GetValueAsync(string name, TemplateContext context) | ||
| { | ||
| var httpContext = _context ?? GetHttpContext(context).ToObjectValue() as HttpContext; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
I fixed it. Now I understand your question. And now you have your answer.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
src/OrchardCore/OrchardCore.DisplayManagement.Liquid/Values/HttpContextValue.cs
Outdated
Show resolved
Hide resolved
src/OrchardCore/OrchardCore.DisplayManagement.Liquid/Values/HttpRequestValue.cs
Outdated
Show resolved
Hide resolved
Last two Liquid accessor