Simplify using StringValues in Liquid templates
#18399
Conversation
gvkries
changed the title
StringValues in Luquid templatesStringValues in Liquid templates
src/OrchardCore/OrchardCore.DisplayManagement.Liquid/TemplateOptionsConfigurations.cs
Show resolved
Hide resolved
src/OrchardCore/OrchardCore.DisplayManagement.Liquid/TemplateOptionsConfigurations.cs
Show resolved
Hide resolved
src/OrchardCore/OrchardCore.DisplayManagement.Liquid/Values/StringValuesValue.cs
Show resolved
Hide resolved
Co-authored-by: Zoltán Lehóczky <[email protected]>
Co-authored-by: Zoltán Lehóczky <[email protected]>
src/OrchardCore/OrchardCore.DisplayManagement.Liquid/Values/StringValuesValue.cs
Show resolved
Hide resolved
src/OrchardCore/OrchardCore.DisplayManagement.Liquid/Values/StringValuesValue.cs
Show resolved
Hide resolved
| } | ||
|
|
||
| [Obsolete("WriteTo is obsolete, prefer the WriteToAsync method.")] | ||
| public override void WriteTo(TextWriter writer, TextEncoder encoder, CultureInfo cultureInfo) |
There was a problem hiding this comment.
Why not you call the async version instead of duplicating the code
There was a problem hiding this comment.
Using the sync APIs (which are designed to be used in a synchronous way) are safer than doing sync-over-async.
There was a problem hiding this comment.
AFAIK, we planned in Fluid to deprecate this API, so it doesn't make sense to use the Sync version, and later we will be forced to use the Async version
There was a problem hiding this comment.
This method here is necessary for backward compatibility.
There was a problem hiding this comment.
I think it will be removed in the next major release, 3.0.0
There was a problem hiding this comment.
Please have a look at the recently added Fluid Values here. AFAIK, we use the async version
There was a problem hiding this comment.
As @Piedone already pointed out, I’d prefer not to rely on sync-over-async in this case. Unlike the other FluidValues you mentioned, this one makes multiple calls to the writer, which introduces additional complexity. Therefore, as long as the method exists, I believe it should be implemented correctly, even if it's marked as obsolete.
This pull request adds support for handling
StringValuesin Liquid templates within OrchardCore, allowing for more natural manipulation and comparison of HTTP query, header, and form values that may be multi-valued. It introduces a newStringValuesValueFluid value type, updates the template options configuration to use it, and adds comprehensive unit tests to ensure correct behavior and compatibility with Liquid operations.Core functionality:
StringValuesValueclass implementingFluidValue, enabling proper handling ofStringValues(multi-valued strings) in Liquid templates, including comparison, indexing, iteration, and property access likesize,first, andlast.TemplateOptionsConfigurations.csto wrapStringValuesinStringValuesValueand to register member access strategies forQueryCollectionandHeaderDictionaryWrapperusingStringValues, ensuring these HTTP-related values are exposed correctly to Liquid templates. [1] [2] [3]Testing and validation:
LiquidTests.csto verifyStringValuesValuebehaviors, including string comparison, array operations (iteration, indexing), property access, containment checks, boolean conversion, and handling of empty values. [1] [2] [3] [4]Fixes #1924