This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch fix/CAMEL-24962 in repository https://gitbox.apache.org/repos/asf/camel.git
commit d15f234b6f542a553072e40526297a85a037a340 Author: Claus Ibsen <[email protected]> AuthorDate: Wed Sep 23 18:47:25 2026 +0200 CAMEL-24969: simple - fix collection function arguments, split, sort, uuid(), range and null numbers - a space after a comma no longer leaks into the argument: ${sort(${body}, true)}, mapAdd, listAdd, ... - split uses the separator as plain text, not a regular expression ('.' and '|') - ${sort(${header.list})} sorts the given list instead of the body - ${uuid()} is the default generator, and the kind is case insensitive - range allows a negative start (as Python); range, collate and random report null or invalid numbers - a body of a single comma no longer fails in the collection functions (camel-util StringHelper) Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Signed-off-by: Claus Ibsen <[email protected]> --- .../simple/CollectionExpressionBuilder.java | 16 ++++++-- .../language/simple/MiscExpressionBuilder.java | 25 +++++++++---- .../functions/CollectionFunctionFactory.java | 43 ++++++++++++++++------ .../apache/camel/language/simple/SimpleTest.java | 29 +++++++++++++++ .../functions/CollectionFunctionFactoryTest.java | 43 ++++++++++++++++++++++ .../java/org/apache/camel/util/StringHelper.java | 8 ++++ .../org/apache/camel/util/StringHelperTest.java | 6 +++ 7 files changed, 147 insertions(+), 23 deletions(-) diff --git a/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/CollectionExpressionBuilder.java b/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/CollectionExpressionBuilder.java index 54b38e923e31..5ef3370a872a 100644 --- a/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/CollectionExpressionBuilder.java +++ b/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/CollectionExpressionBuilder.java @@ -25,6 +25,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Pattern; import org.apache.camel.CamelContext; import org.apache.camel.CamelExecutionException; @@ -272,6 +273,9 @@ public final class CollectionExpressionBuilder { * Split the String values from the expression using the given separator */ public static Expression splitStringExpression(final String expression, final String separator) { + // the separator is plain text (not a regular expression), such as '.' or '|' + final Pattern pattern = Pattern.compile(Pattern.quote( + separator.replace("\\n", "\n").replace("\\r", "\r").replace("\\t", "\t"))); return new ExpressionAdapter() { private Expression exp; @@ -287,7 +291,7 @@ public final class CollectionExpressionBuilder { if (text == null) { return null; } - return text.split(separator); + return pattern.split(text); } @Override @@ -667,9 +671,13 @@ public final class CollectionExpressionBuilder { @Override public Object evaluate(Exchange exchange) { - int num1 = exp1.evaluate(exchange, Integer.class); - int num2 = exp2.evaluate(exchange, Integer.class); - if (num1 >= 0 && num1 <= num2 && num1 != num2) { + Integer num1 = exp1.evaluate(exchange, Integer.class); + Integer num2 = exp2.evaluate(exchange, Integer.class); + if (num1 == null || num2 == null) { + throw new IllegalArgumentException("range expression evaluated to null: " + min + "," + max); + } + // as Python, the range may start below zero: range(-2,2) is [-2, -1, 0, 1] + if (num1 < num2) { List<Integer> answer = new ArrayList<>(); for (int i = num1; i < num2; i++) { answer.add(i); diff --git a/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/MiscExpressionBuilder.java b/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/MiscExpressionBuilder.java index 439a8cc7cb6f..f1a7c23e0a1d 100644 --- a/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/MiscExpressionBuilder.java +++ b/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/MiscExpressionBuilder.java @@ -149,6 +149,9 @@ public final class MiscExpressionBuilder { @Override public Object evaluate(Exchange exchange) { Integer n = num.evaluate(exchange, Integer.class); + if (n == null) { + throw new IllegalArgumentException("collate number expression evaluated to null: " + group); + } Expression grouped = ExpressionBuilder.groupIteratorExpression(exp, null, Integer.toString(n), false); grouped.init(exchange.getContext()); return grouped.evaluate(exchange, Object.class); @@ -725,8 +728,12 @@ public final class MiscExpressionBuilder { @Override public Object evaluate(Exchange exchange) { - int num1 = exp1.evaluate(exchange, Integer.class); - int num2 = exp2.evaluate(exchange, Integer.class); + Integer num1 = exp1.evaluate(exchange, Integer.class); + Integer num2 = exp2.evaluate(exchange, Integer.class); + if (num1 == null || num2 == null || num2 <= num1) { + throw new IllegalArgumentException( + "random(min,max) requires max to be greater than min, was: " + num1 + "," + num2); + } Random random = new Random(); // NOSONAR return random.nextInt(num2 - num1) + num1; } @@ -813,19 +820,21 @@ public final class MiscExpressionBuilder { @Override public void init(CamelContext context) { - if ("classic".equalsIgnoreCase(generator)) { + // ${uuid()} is the same as ${uuid} + String kind = generator != null ? StringHelper.removeLeadingAndEndingQuotes(generator.trim()) : null; + if ("classic".equalsIgnoreCase(kind)) { uuid = new ClassicUuidGenerator(); - } else if ("short".equals(generator)) { + } else if ("short".equalsIgnoreCase(kind)) { uuid = new ShortUuidGenerator(); - } else if ("simple".equals(generator)) { + } else if ("simple".equalsIgnoreCase(kind)) { uuid = new SimpleUuidGenerator(); - } else if ("random".equals(generator)) { + } else if ("random".equalsIgnoreCase(kind)) { uuid = new RandomUuidGenerator(); - } else if (generator == null || "default".equals(generator)) { + } else if (kind == null || kind.isEmpty() || "default".equalsIgnoreCase(kind)) { uuid = new DefaultUuidGenerator(); } else { // lookup custom generator - uuid = CamelContextHelper.mandatoryLookup(context, generator, UuidGenerator.class); + uuid = CamelContextHelper.mandatoryLookup(context, kind, UuidGenerator.class); } } diff --git a/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/functions/CollectionFunctionFactory.java b/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/functions/CollectionFunctionFactory.java index aee3d6a1a7a9..4a7a98a78497 100644 --- a/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/functions/CollectionFunctionFactory.java +++ b/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/functions/CollectionFunctionFactory.java @@ -49,7 +49,7 @@ public final class CollectionFunctionFactory implements SimpleLanguageFunctionFa throw new SimpleParserException( "Valid syntax: ${setHeader(name,exp)} or ${setHeader(name,type,exp)} was: " + function, index); } - String[] tokens = StringQuoteHelper.splitSafeQuote(values, ',', false); + String[] tokens = splitArguments(values); if (tokens.length < 2 || tokens.length > 3) { throw new SimpleParserException( "Valid syntax: ${setHeader(name,exp)} or ${setHeader(name,type,exp)} was: " + function, index); @@ -74,7 +74,7 @@ public final class CollectionFunctionFactory implements SimpleLanguageFunctionFa throw new SimpleParserException( "Valid syntax: ${setVariable(name,exp)} or ${setVariable(name,type,exp)} was: " + function, index); } - String[] tokens = StringQuoteHelper.splitSafeQuote(values, ',', false); + String[] tokens = splitArguments(values); if (tokens.length < 2 || tokens.length > 3) { throw new SimpleParserException( "Valid syntax: ${setVariable(name,exp)} or ${setVariable(name,type,exp)} was: " + function, index); @@ -153,7 +153,7 @@ public final class CollectionFunctionFactory implements SimpleLanguageFunctionFa String exp = "${body}"; String separator = ","; if (ObjectHelper.isNotEmpty(values)) { - String[] tokens = StringQuoteHelper.splitSafeQuote(values, ',', false); + String[] tokens = splitArguments(values); if (tokens.length > 2) { throw new SimpleParserException( "Valid syntax: ${split(separator)} or ${split(exp,separator)} was: " + function, index); @@ -176,7 +176,7 @@ public final class CollectionFunctionFactory implements SimpleLanguageFunctionFa String exp = "${body}"; boolean reverse = false; if (ObjectHelper.isNotEmpty(values)) { - String[] tokens = StringQuoteHelper.splitSafeQuote(values, ',', false); + String[] tokens = splitArguments(values); if (tokens.length > 2) { throw new SimpleParserException( "Valid syntax: ${sort(reverse)} or ${sort(exp,reverse)} was: " + function, index); @@ -184,8 +184,11 @@ public final class CollectionFunctionFactory implements SimpleLanguageFunctionFa if (tokens.length == 2) { exp = tokens[0]; reverse = Boolean.parseBoolean(tokens[1]); - } else { + } else if ("true".equalsIgnoreCase(tokens[0]) || "false".equalsIgnoreCase(tokens[0])) { reverse = Boolean.parseBoolean(tokens[0]); + } else { + // ${sort(${header.list})}: the only argument is what to sort + exp = tokens[0]; } } return CollectionExpressionBuilder.sortExpression(exp, reverse); @@ -198,7 +201,7 @@ public final class CollectionFunctionFactory implements SimpleLanguageFunctionFa throw new SimpleParserException( "Valid syntax: ${forEach(exp,exp)} was: " + function, index); } - String[] tokens = StringQuoteHelper.splitSafeQuote(values, ',', false); + String[] tokens = splitArguments(values); if (tokens.length < 2) { throw new SimpleParserException( "Valid syntax: ${forEach(exp,exp)} was: " + function, index); @@ -215,7 +218,7 @@ public final class CollectionFunctionFactory implements SimpleLanguageFunctionFa throw new SimpleParserException( "Valid syntax: ${filter(exp,exp)} was: " + function, index); } - String[] tokens = StringQuoteHelper.splitSafeQuote(values, ',', false); + String[] tokens = splitArguments(values); if (tokens.length < 2) { throw new SimpleParserException( "Valid syntax: ${filter(exp,exp)} was: " + function, index); @@ -232,7 +235,7 @@ public final class CollectionFunctionFactory implements SimpleLanguageFunctionFa throw new SimpleParserException( "Valid syntax: ${listAdd(exp)} or ${listAdd(exp,exp)} was: " + function, index); } - String[] tokens = StringQuoteHelper.splitSafeQuote(values, ',', false); + String[] tokens = splitArguments(values); int skip = 0; String exp1 = "${body}"; if (tokens.length > 1) { @@ -250,7 +253,7 @@ public final class CollectionFunctionFactory implements SimpleLanguageFunctionFa throw new SimpleParserException( "Valid syntax: ${listRemove(exp)} or ${listRemove(exp,exp)} was: " + function, index); } - String[] tokens = StringQuoteHelper.splitSafeQuote(values, ',', false); + String[] tokens = splitArguments(values); int skip = 0; String exp1 = "${body}"; if (tokens.length > 1) { @@ -268,7 +271,7 @@ public final class CollectionFunctionFactory implements SimpleLanguageFunctionFa throw new SimpleParserException( "Valid syntax: ${mapAdd(key,exp)} or ${mapAdd(exp,key,exp)} was: " + function, index); } - String[] tokens = StringQuoteHelper.splitSafeQuote(values, ',', false); + String[] tokens = splitArguments(values); int skip; String exp1 = "${body}"; String key; @@ -294,7 +297,7 @@ public final class CollectionFunctionFactory implements SimpleLanguageFunctionFa throw new SimpleParserException( "Valid syntax: ${mapRemove(key)} or ${mapRemove(exp,key)} was: " + function, index); } - String[] tokens = StringQuoteHelper.splitSafeQuote(values, ',', false); + String[] tokens = splitArguments(values); if (tokens.length > 2) { throw new SimpleParserException( "Valid syntax: ${mapRemove(key)} or ${mapRemove(exp,key)} was: " + function, index); @@ -336,4 +339,22 @@ public final class CollectionFunctionFactory implements SimpleLanguageFunctionFa return null; } + + /** + * Splits the function arguments by comma. Unquoted arguments are trimmed, so ${sort(${body}, true)} works, while a + * quoted argument such as ' ' is kept as written (without its quotes). + */ + private static String[] splitArguments(String values) { + String[] tokens = StringQuoteHelper.splitSafeQuote(values, ',', false, true); + for (int i = 0; i < tokens.length; i++) { + String token = tokens[i]; + String trimmed = token.trim(); + if (!trimmed.isEmpty()) { + // a whitespace only argument such as a new line is kept as-is + token = trimmed; + } + tokens[i] = StringHelper.removeLeadingAndEndingQuotes(token); + } + return tokens; + } } diff --git a/core/camel-core/src/test/java/org/apache/camel/language/simple/SimpleTest.java b/core/camel-core/src/test/java/org/apache/camel/language/simple/SimpleTest.java index 165cc21096e4..e693c5f6719b 100644 --- a/core/camel-core/src/test/java/org/apache/camel/language/simple/SimpleTest.java +++ b/core/camel-core/src/test/java/org/apache/camel/language/simple/SimpleTest.java @@ -2211,11 +2211,40 @@ public class SimpleTest extends LanguageTestSupport { s = expression.evaluate(exchange, String.class); assertNotNull(s); + // empty parentheses and any case of the kind + expression = context.resolveLanguage("simple").createExpression("${uuid()}"); + s = expression.evaluate(exchange, String.class); + assertNotNull(s); + + expression = context.resolveLanguage("simple").createExpression("${uuid(Short)}"); + s = expression.evaluate(exchange, String.class); + assertNotNull(s); + // custom generator context.getRegistry().bind("mygen", (UuidGenerator) () -> "1234"); assertExpression("${uuid(mygen)}", "1234"); } + @Test + public void testCollectionFunctionsOnASingleComma() { + exchange.getMessage().setBody(","); + assertExpression("${isEmpty()}", true); + } + + @Test + public void testNullNumberArguments() { + Exception e = assertThrows(Exception.class, () -> evaluate("${collate(${header.none})}")); + assertTrue(e.getMessage().contains("collate number expression evaluated to null"), e.getMessage()); + e = assertThrows(Exception.class, () -> evaluate("${range(1,${header.none})}")); + assertTrue(e.getMessage().contains("range expression evaluated to null"), e.getMessage()); + e = assertThrows(Exception.class, () -> evaluate("${random(5,5)}")); + assertTrue(e.getMessage().contains("requires max to be greater than min"), e.getMessage()); + } + + private Object evaluate(String text) { + return context.resolveLanguage("simple").createExpression(text).evaluate(exchange, Object.class); + } + @Test public void testHash() throws Exception { Expression expression = context.resolveLanguage("simple").createExpression("${hash(hello)}"); diff --git a/core/camel-core/src/test/java/org/apache/camel/language/simple/functions/CollectionFunctionFactoryTest.java b/core/camel-core/src/test/java/org/apache/camel/language/simple/functions/CollectionFunctionFactoryTest.java index a1fbdb462d27..7cd75dc80ee7 100644 --- a/core/camel-core/src/test/java/org/apache/camel/language/simple/functions/CollectionFunctionFactoryTest.java +++ b/core/camel-core/src/test/java/org/apache/camel/language/simple/functions/CollectionFunctionFactoryTest.java @@ -220,4 +220,47 @@ public class CollectionFunctionFactoryTest extends AbstractSimpleFunctionFactory assertEquals("v2", result.get("k2")); } + // --- CAMEL-24969 --- + + @Test + @SuppressWarnings("unchecked") + public void testRangeNegativeStart() { + assertEquals(List.of(-2, -1, 0, 1), evaluate("range(-2,2)", List.class)); + } + + @Test + public void testSplitSeparatorIsNotARegex() { + exchange.getIn().setBody("a.b.c"); + assertEquals(List.of("a", "b", "c"), List.of(evaluate("split(${body},'.')", String[].class))); + exchange.getIn().setBody("a|b|c"); + assertEquals(List.of("a", "b", "c"), List.of(evaluate("split(${body},'|')", String[].class))); + exchange.getIn().setBody("a b c"); + assertEquals(List.of("a", "b", "c"), List.of(evaluate("split(${body}, ' ')", String[].class))); + exchange.getIn().setBody("a;b"); + assertEquals(List.of("a", "b"), List.of(evaluate("split(${body}, ;)", String[].class))); + } + + @Test + @SuppressWarnings("unchecked") + public void testSortOnlyArgumentIsWhatToSort() { + exchange.getIn().setBody(new ArrayList<>(List.of("b", "a"))); + exchange.getIn().setHeader("list", new ArrayList<>(List.of("z", "y"))); + assertEquals(List.of("y", "z"), evaluate("sort(${header.list})", List.class)); + assertEquals(List.of("b", "a"), evaluate("sort(true)", List.class)); + } + + @Test + @SuppressWarnings("unchecked") + public void testSpaceAfterComma() { + exchange.getIn().setBody(new ArrayList<>(List.of("a", "b"))); + assertEquals(List.of("b", "a"), evaluate("sort(${body}, true)", List.class)); + + exchange.getIn().setBody(new ArrayList<>(List.of("X"))); + assertEquals(List.of("X", "Y"), evaluate("listAdd(${body}, 'Y')", List.class)); + + exchange.getIn().setBody(new HashMap<>()); + Map<String, Object> map = evaluate("mapAdd(${body}, 'k', 'v')", Map.class); + assertEquals("v", map.get("k")); + } + } diff --git a/core/camel-util/src/main/java/org/apache/camel/util/StringHelper.java b/core/camel-util/src/main/java/org/apache/camel/util/StringHelper.java index 082b71c98435..7e37a000d964 100644 --- a/core/camel-util/src/main/java/org/apache/camel/util/StringHelper.java +++ b/core/camel-util/src/main/java/org/apache/camel/util/StringHelper.java @@ -414,6 +414,10 @@ public final class StringHelper { } public static Iterator<String> splitOnCharacterAsIterator(String value, char needle, int count) { + if (value.length() == 1 && value.charAt(0) == needle) { + // only the needle, such as a single comma, so there are no values + return Collections.emptyIterator(); + } // skip leading and trailing needles int end = value.length() - 1; boolean skipStart = value.charAt(0) == needle; @@ -462,6 +466,10 @@ public final class StringHelper { } public static List<String> splitOnCharacterAsList(String value, char needle, int count) { + if (value.length() == 1 && value.charAt(0) == needle) { + // only the needle, such as a single comma, so there are no values + return new ArrayList<>(); + } // skip leading and trailing needles int end = value.length() - 1; boolean skipStart = value.charAt(0) == needle; diff --git a/core/camel-util/src/test/java/org/apache/camel/util/StringHelperTest.java b/core/camel-util/src/test/java/org/apache/camel/util/StringHelperTest.java index cbde328e73c9..c6bf59aaa6a5 100644 --- a/core/camel-util/src/test/java/org/apache/camel/util/StringHelperTest.java +++ b/core/camel-util/src/test/java/org/apache/camel/util/StringHelperTest.java @@ -246,6 +246,9 @@ public class StringHelperTest { assertEquals("foo", list.get(0)); assertEquals("bar", list.get(1)); + list = splitOnCharacterAsList(",", ',', 2); + assertEquals(0, list.size()); + StringBuilder sb = new StringBuilder(); for (int i = 0; i < 100; i++) { sb.append(i); @@ -267,6 +270,9 @@ public class StringHelperTest { assertEquals("foo", it.next()); assertFalse(it.hasNext()); + it = splitOnCharacterAsIterator(",", ',', 2); + assertFalse(it.hasNext()); + it = splitOnCharacterAsIterator("foo,bar", ',', 2); assertEquals("foo", it.next()); assertEquals("bar", it.next());
