Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Variable placeholders are not resolved after update to 3.14.0 #3804

Open
dpozinen opened this issue Oct 10, 2023 · 2 comments
Open

Variable placeholders are not resolved after update to 3.14.0 #3804

dpozinen opened this issue Oct 10, 2023 · 2 comments
Labels
type: bug bugs/ bug fixes

Comments

@dpozinen
Copy link

dpozinen commented Oct 10, 2023

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.

@dpozinen dpozinen added the type: bug bugs/ bug fixes label Oct 10, 2023
@Young-Zen
Copy link
Contributor

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

@dpozinen
Copy link
Author

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug bugs/ bug fixes
Projects
None yet
Development

No branches or pull requests

2 participants