Liquid filters and comparison stopped working after upgrade to 1.9

[SzymonSel]

Describe the bug

Another breaking change is in our Liquid views. What stopped working:

{% assign my_variable_1 = "some_stuff" %}
{% assign my_variable_2 = "some_stuff" %}
{% if my_variable_1 == my_variable_2 %}
   {{ "Never here" }}
{% endif %}

{% assign formated_datetime = Model.ContentItem.CreatedAtUtc | date: "%y-%M-%d" %} this return and empty string

UNLESS a do some hacking and use the raw filter like so:

{% assign my_variable_1 = "some_stuff" | raw %}
{% assign my_variable_2 = "some_stuff" | raw %}
{% if my_variable_1 == my_variable_2 %}
   {{ "Success" }}
{% endif %}

{% assign formated_datetime = Model.ContentItem.CreatedAtUtc | raw | date: "%y-%M-%d" %}

[MikeAlhayek]

@sebastienros thoughts?

[MikeAlhayek]

@SzymonSel
In 980f322 we change the Liquid options to allow functions new FluidParserOptions() { AllowFunctions = true } I am wondering if that caused the issue.

Can you try using 1.9.0-preview-18130 which was the last package before that change

[sebastienros]

@MikeAlhayek I tried the fluid tests with this option and it worked. Will have to debug that in OC.

[sebastienros]

Works for me in main, @SzymonSel we need a more detailed repro

[SzymonSel]

Got a another one when when trying to devide integer by decimal. Not very forgiving...

I create a more detailed description shortly.

InvalidCastException: Unable to cast object of type 'System.Text.Json.Nodes.JsonValuePrimitive`1[System.Text.Json.JsonElement]' to type 'System.IConvertible'.

System.Convert.ToDecimal(object value)

    Stack Query Cookies Headers Routing 

    InvalidCastException: Unable to cast object of type 'System.Text.Json.Nodes.JsonValuePrimitive`1[System.Text.Json.JsonElement]' to type 'System.IConvertible'.
        System.Convert.ToDecimal(object value)
        Fluid.Values.ObjectValueBase.ToNumberValue()
        Fluid.Filters.NumberFilters.DividedBy(FluidValue input, FilterArguments arguments, TemplateContext context)

[MikeAlhayek]

@sebastienros I am facing the same problem with OC preview "before merging STJ"

I added a workflow that would create a content type

{
   "RecordKeeper": {
      "Type": {
         "Text": "{{ Request.Form.Scenarios | escape }}"
      },
      "CaseNumber": {
         "Text":"{{ Request.Form.CaseNumber | escape }}"
      },
      "VerifiedEmail": {
         "Value": {% if Request.Form.VerifiedEmail == "yes" %}true{% else %}false{% endif %}
      },
      "PreviouslyUpdatedCustomer": {
         "Value": {% if Request.Form.PreviouslyUpdatedCustomer == "yes" %}true{% else %}false{% endif %}
      },
      "CustomerInteractedInEmailMarketing": {
         "Value": {% if Request.Form.CustomerInteractedInEmailMarketing == "yes" %}true{% else %}false{% endif %}
      },
      "HasAtLeastOnePurchase": {
         "Value": {% if Request.Form.HasAtLeastOnePurchase == "yes" %}true{% else %}false{% endif %}
      },
      "IsLessThan40": {
         "Value": {% if Request.Form.IsLessThan40 == "yes" %}true{% else %}false{% endif %}
      },
      "IsLessThan100": {
         "Value": {% if Request.Form.IsLessThan100 == "yes" %}true{% else %}false{% endif %}
      },
      "IsGreetingCard": {
         "Value": {% if Request.Form.IsGreetingCard == "yes" %}true{% else %}false{% endif %}
      },
      "IsBroken": {
         "Value": {% if Request.Form.IsBroken == "yes" %}true{% else %}false{% endif %}
      },
      "Result": {
         "Text": "{{ Workflow.Properties["actionResult"] | escape }}"
      }
   }
}

Assigning the Text works. But {% if Request.Form.IsBroken == "yes" %}true{% else %}false{% endif %} always returns false

[sebastienros]

3 different issues to me though. The first one I proved it was working, I will need a better repro.

The second one is a JSON conversion issue in Fluid. Could use a repro, but maybe I can try to guess how this would happen. Probably a JsonValue number is used as the input.

The 3rd one looks like a property/class no registered (allowed) in the templates.

[MikeAlhayek]

I have to look more. not sure. The following test works

{
    [Fact]
    public async Task RenderString_WhenComparingString_ReturnedCorrectValue()
    {
        var context = new SiteContext();
        await context.InitializeAsync();
        await context.UsingTenantScopeAsync(async scope =>
        {
            var liquidTemplateManager = scope.ServiceProvider.GetRequiredService<ILiquidTemplateManager>();
            var result = await liquidTemplateManager.RenderStringAsync("{% if IsLessThan40 == \"yes\" %}true{% else %}false{% endif %}",
                NullEncoder.Default,
                new Dictionary<string, FluidValue>() { ["IsLessThan40"] = new StringValue("yes") }
                );

            Assert.Equal("true", result);
        });
    }

[MikeAlhayek]

@sebastienros

The 3rd one looks like a property/class no registered (allowed) in the templates.

By this one are you referring to the issue I posted here #15625 (comment) ?

That data is coming from a form and is in Request.Form so there is no class to register. In fact, I logged the data from the workflow just to confirm the expected data.

I added this to the Log Task just before creating the content item.

{{ Request.Form | json | append: 'CustomerInteractedInEmailMarketing >>' | append: Request.Form.CustomerInteractedInEmailMarketing }}

As you can see in the logs I am getting the expected value of yes.

So the Liquid {{ Request.Form.CustomerInteractedInEmailMarketing }} prints the correct yes value. But evaluation {% if Request.Form.CustomerInteractedInEmailMarketing == "yes" %}true{% else %}false{% endif %} always returns false.

[MikeAlhayek]

Okay. I think I found my issue {{ Request.Form.CustomerInteractedInEmailMarketing }} prints the first value in the collection from the request. However, the Request.Form.CustomerInteractedInEmailMarketing == "yes" would return false because the value is not a string but a collection of strings.

So changing

{% if Request.Form["CustomerInteractedInEmailMarketing"] == "yes" %}true{% else %}false{% endif %}

to

{% if Request.Form["CustomerInteractedInEmailMarketing"] contains "yes" %}true{% else %}false{% endif %}

fixes the problem for me. @sebastienros in this the expected results? printing {{ Request.Form.CustomerInteractedInEmailMarketing }} as the first value seems misleading here.

[sebastienros]

InvalidCastException: Unable to cast object of type 'System.Text.Json.Nodes.JsonValuePrimitive`1[System.Text.Json.JsonElement]' to type 'System.IConvertible'.
        System.Convert.ToDecimal(object value)
        Fluid.Values.ObjectValueBase.ToNumberValue()
        Fluid.Filters.NumberFilters.DividedBy(FluidValue input, FilterArguments arguments, TemplateContext context)

I need to check in Fluid how these STJ nodes get converted to FluidValue. It's possible that Newtonsoft was implementing IConvertible and not STJ.

[sebastienros]

For @MikeAlhayek 's issue we need to check that Request.Form["CustomerInteractedInEmailMarketing"] was already displaying the string, and also behave as an array for tests. Probably because it's using a StringValues internally.

[SzymonSel]

So I've tested this on a vanilla OC, latest nightly build.
I created a simple content type with just one field. And I created a template like so:

Now after a small change in the template:

[sebastienros]

| raw is rendering the value, hence converting to a string, so the comparison will work.
The fact that Value == "string" where Value is a DateTime is a good question though. Are you saying it's working on 1.8.3 but not main?

I would need to check with the Liquid language if that's expected. In a way we could assume that comparing a string to a datetime should try to compare the datetime values by parsing the string.

[SzymonSel]

Correct, it worked until the switch from Newtonsoft.Json to System.Text.Json

Oh, I don't get decived by the DateTimeField, the same goes for other fields and even checking the ContentType when comparing to string.

[sebastienros]

It's highly probable that this PR will fix it.

Can you or @gvkries try it?

[SzymonSel]

Sure, I'll give it go! Thanks!

[SzymonSel]

Sadly, no. I've additionally tested this with a simple TextField and a List

[gvkries]

This looks like a different problem than addressed in #15816. In

OrchardCore/src/OrchardCore.Modules/OrchardCore.Liquid/Startup.cs

Line 59 in d815446

JsonDynamicObject o => new ObjectValue((JsonObject)o),

we always pass the original JsonObject to fluid, so the dynamic wrappers are not used in liquid as far as I see.

[sebastienros]

I will debug that then

[gvkries]

I've created #16053 as a starting point for a solution. But I'm not sure about the date and time values, or how they should be supported in Liquid.
I've also added some rudimentary unit tests, but I'm also not sure where to put them. Are there already tests as I haven't found any?