Netflix/conductor

Variable placeholders are not resolved after update to 3.14.0

dpozinen opened this issue · 2 comments

After #3764 if the checked string ends with a new line + space the variables are not resolved. See test below:

    @Test
    public void testNestedPathExpressions() throws Exception {
        var js = "calculateDelay('${workflow.variables.delayStartTs}');\n ";
        var ctx = JsonPath.parse("""
            { "workflow": { "variables": { "delayStartTs": "I should be the value" } } }""");

        var replaceVariables = ParametersUtils.class.getDeclaredMethod("replaceVariables", String.class, DocumentContext.class, String.class);
        replaceVariables.setAccessible(true);

        String replaced = (String) replaceVariables.invoke(new ParametersUtils(new ObjectMapper()), js, ctx, "123");

        assertThat(replaced).contains("calculateDelay('I should be the value');");
    }

It might be a bit weird that the string in the workflow is generated that way, but it was actually done automatically when resolving a multiline string

"""
calculateDelay('${delay_wait_task.output.updatedDelayEndTs}');
"""

Regardless, while minor, it's still a bug and i expect lots of people use multiline string for js scripts.

another failed test case where the matcher doesn't match

private static final String js = """
    function f() { var baseCallbackUrl = '${workflow.input.conductor_actions_callback_url}'; }
    f();""";

We have also identified another js file that doesn't have a new line + space at the end, but still the regex matcher does not match any variables in it.

This is because by default, the regular expression . matches any character except the line terminator.

the fix provided in #3810 seems to be working well in our test cases