This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new 1dd20e756040 CAMEL-25044: camel-core - ExpressionBuilder and
PredicateBuilder: fix bugs found in a deep review (#26924)
1dd20e756040 is described below
commit 1dd20e7560400ac6a9da2011bbb2161b4dded31e
Author: Claus Ibsen <[email protected]>
AuthorDate: Mon Sep 28 09:27:24 2026 +0200
CAMEL-25044: camel-core - ExpressionBuilder and PredicateBuilder: fix bugs
found in a deep review (#26924)
- < is false when both sides are null (as > is)
- PredicateBuilder.language no longer evaluates on a static exchange shared
by all threads
- a missing variable as the source of a language fails with
NoSuchVariableException (as header: does)
- in(...) with a null value matches a missing value (convertToExpression
returned the expression)
- ${join} keeps the separators of leading empty elements
- headerExpression/variableExpression with an array type resolve the type
- a null constant in an optimized concat adds nothing instead of "null"
- languageExpression inits its input expression
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Signed-off-by: Claus Ibsen <[email protected]>
---
.../builder/ExpressionBuilderEdgeCasesTest.java | 161 +++++++++++++++++++++
.../camel/support/builder/ExpressionBuilder.java | 93 +++++++++---
.../camel/support/builder/PredicateBuilder.java | 9 +-
.../ROOT/pages/camel-4x-upgrade-guide-4_23.adoc | 10 ++
4 files changed, 247 insertions(+), 26 deletions(-)
diff --git
a/core/camel-core/src/test/java/org/apache/camel/builder/ExpressionBuilderEdgeCasesTest.java
b/core/camel-core/src/test/java/org/apache/camel/builder/ExpressionBuilderEdgeCasesTest.java
new file mode 100644
index 000000000000..fbe4eff59341
--- /dev/null
+++
b/core/camel-core/src/test/java/org/apache/camel/builder/ExpressionBuilderEdgeCasesTest.java
@@ -0,0 +1,161 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.builder;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.camel.ContextTestSupport;
+import org.apache.camel.Exchange;
+import org.apache.camel.Expression;
+import org.apache.camel.NoSuchVariableException;
+import org.apache.camel.Predicate;
+import org.apache.camel.support.DefaultExchange;
+import org.apache.camel.support.builder.ExpressionBuilder;
+import org.apache.camel.support.builder.PredicateBuilder;
+import org.apache.camel.support.builder.ValueBuilder;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+public class ExpressionBuilderEdgeCasesTest extends ContextTestSupport {
+
+ @Override
+ public boolean isUseRouteBuilder() {
+ return false;
+ }
+
+ private Object evaluate(Expression expression, Exchange exchange) {
+ expression.init(context);
+ return expression.evaluate(exchange, Object.class);
+ }
+
+ private boolean matches(Predicate predicate, Exchange exchange) {
+ predicate.init(context);
+ return predicate.matches(exchange);
+ }
+
+ @Test
+ public void testLessThanWithBothNull() {
+ Exchange exchange = new DefaultExchange(context);
+ Predicate lessThan =
PredicateBuilder.isLessThan(ExpressionBuilder.headerExpression("a"),
+ ExpressionBuilder.headerExpression("b"));
+ assertThat(matches(lessThan, exchange)).isFalse();
+
assertThat(matches(context.resolveLanguage("simple").createPredicate("${header.a}
< ${header.b}"), exchange))
+ .isFalse();
+ // null is equal to null
+
assertThat(matches(context.resolveLanguage("simple").createPredicate("${header.a}
<= ${header.b}"), exchange))
+ .isTrue();
+ }
+
+ @Test
+ public void testLanguagePredicateWithConcurrentExchanges() throws
Exception {
+ Predicate predicate =
PredicateBuilder.language(ExpressionBuilder.headerExpression("value"), "simple",
+ "${body} == 'A'");
+ predicate.init(context);
+
+ List<String> wrong = new CopyOnWriteArrayList<>();
+ ExecutorService pool = Executors.newFixedThreadPool(8);
+ try {
+ for (int i = 0; i < 5000; i++) {
+ String value = i % 2 == 0 ? "A" : "B";
+ pool.submit(() -> {
+ Exchange exchange = new DefaultExchange(context);
+ exchange.getMessage().setHeader("value", value);
+ if (predicate.matches(exchange) != "A".equals(value)) {
+ wrong.add(value);
+ }
+ });
+ }
+ } finally {
+ pool.shutdown();
+ }
+ assertThat(pool.awaitTermination(30, TimeUnit.SECONDS)).isTrue();
+ assertThat(wrong).isEmpty();
+ }
+
+ @Test
+ public void testMissingVariableSourceIsMandatory() {
+ Exchange exchange = new DefaultExchange(context);
+ Expression source =
ExpressionBuilder.singleInputExpression("variable:missing");
+ assertThatThrownBy(() -> evaluate(source, exchange))
+ .hasCauseInstanceOf(NoSuchVariableException.class);
+ }
+
+ @Test
+ public void testInWithNullValue() {
+ Exchange exchange = new DefaultExchange(context);
+ Predicate in = new
ValueBuilder(ExpressionBuilder.headerExpression("foo")).in("a", null);
+ assertThat(matches(in, exchange)).isTrue();
+
+ exchange.getMessage().setHeader("foo", "b");
+ assertThat(matches(in, exchange)).isFalse();
+ }
+
+ @Test
+ public void testJoinKeepsEmptyElements() {
+ Exchange exchange = new DefaultExchange(context);
+ exchange.getMessage().setBody(Arrays.asList("", "", "c"));
+
assertThat(evaluate(context.resolveLanguage("simple").createExpression("${join(',')}"),
exchange))
+ .isEqualTo(",,c");
+ }
+
+ @Test
+ public void testHeaderAndVariableAsArrayType() {
+ Exchange exchange = new DefaultExchange(context);
+ exchange.getMessage().setHeader("h", new byte[] { 1, 2 });
+ exchange.setVariable("v", new byte[] { 3 });
+ assertThat(evaluate(ExpressionBuilder.headerExpression("h",
byte[].class), exchange))
+ .isEqualTo(new byte[] { 1, 2 });
+ assertThat(evaluate(ExpressionBuilder.variableExpression("v",
byte[].class), exchange))
+ .isEqualTo(new byte[] { 3 });
+ exchange.getMessage().setHeader("ints", new int[] { 4, 5 });
+ exchange.setVariable("matrix", new byte[][] { { 6 } });
+ exchange.getMessage().setHeader("nested", new Nested[] { new Nested()
});
+ assertThat(evaluate(ExpressionBuilder.headerExpression("ints",
int[].class), exchange))
+ .isEqualTo(new int[] { 4, 5 });
+ assertThat(evaluate(ExpressionBuilder.variableExpression("matrix",
byte[][].class), exchange))
+ .isEqualTo(new byte[][] { { 6 } });
+ assertThat(evaluate(ExpressionBuilder.headerExpression("nested",
Nested[].class), exchange))
+ .isInstanceOf(Nested[].class);
+ }
+
+ @Test
+ public void testConcatWithNullConstant() {
+ Exchange exchange = new DefaultExchange(context);
+ Expression concat = ExpressionBuilder.concatExpression(
+ List.of(ExpressionBuilder.constantExpression(null),
ExpressionBuilder.constantExpression("x")));
+ assertThat(evaluate(concat, exchange)).isEqualTo("x");
+ }
+
+ @Test
+ public void testLanguageExpressionInitsItsInput() {
+ Exchange exchange = new DefaultExchange(context);
+ exchange.getMessage().setHeader("foo", "Hello");
+ Expression expression = ExpressionBuilder.languageExpression(
+ ExpressionBuilder.simpleExpression("${header.foo} World"),
"simple", "${body}", String.class);
+ assertThat(evaluate(expression, exchange)).isEqualTo("Hello World");
+ }
+
+ public static class Nested {
+ }
+}
diff --git
a/core/camel-support/src/main/java/org/apache/camel/support/builder/ExpressionBuilder.java
b/core/camel-support/src/main/java/org/apache/camel/support/builder/ExpressionBuilder.java
index ab024a2afd9b..63f94bc9435e 100644
---
a/core/camel-support/src/main/java/org/apache/camel/support/builder/ExpressionBuilder.java
+++
b/core/camel-support/src/main/java/org/apache/camel/support/builder/ExpressionBuilder.java
@@ -185,7 +185,26 @@ public class ExpressionBuilder {
* @return an expression object which will return the header
value
*/
public static <T> Expression headerExpression(final String headerName,
final Class<T> type) {
- return headerExpression(simpleExpression(headerName),
constantExpression(type.getName()));
+ // the type is already known, so use it as-is instead of resolving it
again by name (which cannot load
+ // every array or nested class type)
+ final Expression name = simpleExpression(headerName);
+ return new ExpressionAdapter() {
+ @Override
+ public Object evaluate(Exchange exchange) {
+ return headerAs(exchange, name.evaluate(exchange,
String.class), type);
+ }
+
+ @Override
+ public void init(CamelContext context) {
+ super.init(context);
+ name.init(context);
+ }
+
+ @Override
+ public String toString() {
+ return "headerAs(" + name + ", " + type.getName() + ")";
+ }
+ };
}
/**
@@ -223,13 +242,7 @@ public class ExpressionBuilder {
} catch (ClassNotFoundException e) {
throw
CamelExecutionException.wrapCamelExecutionException(exchange, e);
}
- String text = headerName.evaluate(exchange, String.class);
- Object header = exchange.getIn().getHeader(text, type);
- if (header == null) {
- // fall back on a property
- header = exchange.getProperty(text, type);
- }
- return header;
+ return headerAs(exchange, headerName.evaluate(exchange,
String.class), type);
}
@Override
@@ -247,6 +260,15 @@ public class ExpressionBuilder {
};
}
+ private static Object headerAs(Exchange exchange, String name, Class<?>
type) {
+ Object header = exchange.getIn().getHeader(name, type);
+ if (header == null) {
+ // fall back on a property
+ header = exchange.getProperty(name, type);
+ }
+ return header;
+ }
+
/**
* Returns an expression for the variable with the given name
*
@@ -318,7 +340,26 @@ public class ExpressionBuilder {
* @return an expression object which will return the
variable value
*/
public static <T> Expression variableExpression(final String variableName,
final Class<T> type) {
- return variableExpression(simpleExpression(variableName),
constantExpression(type.getName()));
+ // the type is already known, so use it as-is instead of resolving it
again by name (which cannot load
+ // every array or nested class type)
+ final Expression name = simpleExpression(variableName);
+ return new ExpressionAdapter() {
+ @Override
+ public Object evaluate(Exchange exchange) {
+ return variableAs(exchange, name.evaluate(exchange,
String.class), type);
+ }
+
+ @Override
+ public void init(CamelContext context) {
+ super.init(context);
+ name.init(context);
+ }
+
+ @Override
+ public String toString() {
+ return "variableAs(" + name + ", " + type.getName() + ")";
+ }
+ };
}
/**
@@ -342,7 +383,6 @@ public class ExpressionBuilder {
public static Expression variableExpression(final Expression variableName,
final Expression typeName) {
return new ExpressionAdapter() {
private ClassResolver classResolver;
- private TypeConverter converter;
@Override
public Object evaluate(Exchange exchange) {
@@ -353,12 +393,7 @@ public class ExpressionBuilder {
} catch (ClassNotFoundException e) {
throw
CamelExecutionException.wrapCamelExecutionException(exchange, e);
}
- String key = variableName.evaluate(exchange, String.class);
- Object value = ExchangeHelper.getVariable(exchange, key);
- if (value != null) {
- value = converter.convertTo(type, value);
- }
- return value;
+ return variableAs(exchange, variableName.evaluate(exchange,
String.class), type);
}
@Override
@@ -367,7 +402,6 @@ public class ExpressionBuilder {
variableName.init(context);
typeName.init(context);
classResolver = context.getClassResolver();
- converter = context.getTypeConverter();
}
@Override
@@ -377,6 +411,14 @@ public class ExpressionBuilder {
};
}
+ private static Object variableAs(Exchange exchange, String name, Class<?>
type) {
+ Object value = ExchangeHelper.getVariable(exchange, name);
+ if (value != null) {
+ value = exchange.getContext().getTypeConverter().convertTo(type,
value);
+ }
+ return value;
+ }
+
/**
* Returns an expression for the inbound message headers
*
@@ -1116,6 +1158,7 @@ public class ExpressionBuilder {
@Override
public void init(CamelContext context) {
super.init(context);
+ expression.init(context);
Language lan = context.resolveLanguage(language);
if (lan != null) {
pred = lan.createPredicate(value);
@@ -1397,7 +1440,7 @@ public class ExpressionBuilder {
if (source.startsWith("variable:")) {
source = source.substring(9);
}
- exp = variableExpression(source);
+ exp = variableExpression(source, true);
}
return exp;
}
@@ -1694,7 +1737,7 @@ public class ExpressionBuilder {
if (type != null) {
return expression.evaluate(exchange, type);
} else {
- return expression;
+ return expression.evaluate(exchange, Object.class);
}
}
@@ -1722,7 +1765,7 @@ public class ExpressionBuilder {
if (result != null) {
return expression.evaluate(exchange, result.getClass());
} else {
- return expression;
+ return expression.evaluate(exchange, Object.class);
}
}
@@ -1936,14 +1979,17 @@ public class ExpressionBuilder {
"expression: " + expression + " evaluated on " +
exchange + " must return an java.util.Iterator");
StringBuilder sb = new StringBuilder(128);
+ boolean first = true;
while (it.hasNext()) {
Object o = it.next();
if (o != null) {
String s = converter.tryConvertTo(String.class,
exchange, o);
if (s != null) {
- if (!sb.isEmpty()) {
+ // an empty element is still an element that is
separated from the others
+ if (!first) {
sb.append(separator);
}
+ first = false;
if (prefix != null) {
sb.append(prefix);
}
@@ -2281,7 +2327,10 @@ public class ExpressionBuilder {
expression.init(context);
if (expression instanceof ConstantExpressionAdapter
constantExpressionAdapter) {
Object value =
constantExpressionAdapter.getValue();
- preprocessedExpression.add(String.valueOf(value));
+ // a null constant adds nothing (as when it is
evaluated)
+ if (value != null) {
+
preprocessedExpression.add(String.valueOf(value));
+ }
} else {
preprocessedExpression.add(expression);
constantsOnly = false;
diff --git
a/core/camel-support/src/main/java/org/apache/camel/support/builder/PredicateBuilder.java
b/core/camel-support/src/main/java/org/apache/camel/support/builder/PredicateBuilder.java
index cac09ae1dea3..398abdde9119 100644
---
a/core/camel-support/src/main/java/org/apache/camel/support/builder/PredicateBuilder.java
+++
b/core/camel-support/src/main/java/org/apache/camel/support/builder/PredicateBuilder.java
@@ -26,7 +26,7 @@ import org.apache.camel.Exchange;
import org.apache.camel.Expression;
import org.apache.camel.Predicate;
import org.apache.camel.spi.Language;
-import org.apache.camel.support.ExchangeHelper;
+import org.apache.camel.support.DefaultExchange;
import org.apache.camel.support.ExpressionToPredicateAdapter;
import org.apache.camel.support.LanguageHelper;
import org.apache.camel.support.ObjectHelper;
@@ -309,8 +309,8 @@ public class PredicateBuilder {
protected boolean matches(Exchange exchange, Object leftValue,
Object rightValue) {
if (leftValue == null && rightValue == null) {
- // they are equal
- return true;
+ // they are equal, so one is not less than the other
+ return false;
} else if (leftValue == null || rightValue == null) {
// only one of them is null so they are not equal
return false;
@@ -643,7 +643,8 @@ public class PredicateBuilder {
public boolean matches(Exchange exchange) {
Object value = expression.evaluate(exchange, Object.class);
if (value != null) {
- Exchange dummy =
ExchangeHelper.getDummy(exchange.getContext());
+ // a new exchange for each evaluation, as the predicate is
used by concurrent exchanges
+ Exchange dummy = new DefaultExchange(exchange);
dummy.getMessage().setBody(value);
return pred.matches(dummy);
}
diff --git
a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
index b805c32b9c50..c2e8e94988d0 100644
--- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
+++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
@@ -186,6 +186,16 @@ Only options whose value is a placeholder are affected,
and only when running wi
Camel Quarkus. If a component must not be stopped on reload, configure it
programmatically rather than with a
placeholder based property.
+=== Languages - less than with two null values, and a source from a variable
+
+- The `<` operator (in the Simple language and the Java DSL `isLessThan`) is
now `false` when both sides are `null`,
+ as the `>` operator already was. Prior to Camel 4.23, `${header.a} <
${header.b}` was `true` when neither header
+ existed. The `<=` and `>=` operators are unchanged and are `true` in that
case.
+- A language that reads its input from a `source` of `variable:name` now fails
with `NoSuchVariableException` when the
+ variable does not exist, as it does with `NoSuchHeaderException` for
`header:name`. From Camel 4.4 to 4.22 a missing
+ variable was used as a `null` input. This also applies to a `source` without
a prefix, as a plain name refers to a
+ variable (`source="myVar"`), and to the `source` option of the `xslt`,
`xslt-saxon` and `xquery` endpoints.
+
=== Apache Avro trusted packages
Camel now uses Apache Avro 1.12.2. Avro validates classes resolved from schemas
