Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: parameter list parsing #1131

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

novarx
Copy link

@novarx novarx commented Mar 6, 2024

If i.e. following value is passed to getParameterList:

// has two spaces here -----------\/
citrus:concat('{"lorem": ["ipsum",  "other"]}'),

it results in following String:

//note missing comma
{"lorem": ["ipsum""other"]}

(see tests for more examples)

assertEquals(resolveFunction("citrus:concat('Hello Yes, I like Citrus!', 'Hello Yes,we like Citrus!')", context), "Hello Yes, I like Citrus!Hello Yes,we like Citrus!");
assertEquals(resolveFunction("citrus:concat('Hello Yes, I like Citrus, and this is great!', 'Hello Yes,we like Citrus, and this is great!')", context), "Hello Yes, I like Citrus, and this is great!Hello Yes,we like Citrus, and this is great!");

// assertEquals(resolveFunction("citrus:upperCase(''Yes, I like Citrus!)", context), "''YES, I LIKE CITRUS!");
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand those three test - are they really intended like that?

Because compared i.E. to CSV (if I recall correct), if quotes are present in a string, they need to be inside quotes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, apparently unequal amount of quotes were accepted / ignored. and these few tests here do no longer pass, because of the provided fix in the parser. is that what you mean?

* @param parameterString comma separated parameter string.
* @return list of parameters.
*/
public static List<String> getParameterList(String parameterString) {
List<String> parameterList = new ArrayList<>();
return new ParameterParser(parameterString).parse();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The longer I think about that... wouldn't it be easier to just use a CSV-Parser for that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @bbortt
Your thoughts on that?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm.. I think CSV uses double quotes for concatenation. that's a bit problematic in the context of Java. other than that, I do too think that it's the same parsing logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants