This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch fix/CAMEL-25044 in repository https://gitbox.apache.org/repos/asf/camel.git
commit 5663885f3e46fa9876ff082d24234321d9a0fc6d Author: Claus Ibsen <[email protected]> AuthorDate: Sun Sep 27 11:31:36 2026 +0200 CAMEL-25044: camel-core - ExpressionBuilder and PredicateBuilder: fix bugs found in a deep review - < 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 | 149 +++++++++++++++++++++ .../camel/support/builder/ExpressionBuilder.java | 26 +++- .../camel/support/builder/PredicateBuilder.java | 9 +- .../ROOT/pages/camel-4x-upgrade-guide-4_23.adoc | 9 ++ 4 files changed, 182 insertions(+), 11 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..abcddae44a35 --- /dev/null +++ b/core/camel-core/src/test/java/org/apache/camel/builder/ExpressionBuilderEdgeCasesTest.java @@ -0,0 +1,149 @@ +/* + * 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 }); + } + + @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"); + } +} 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 49d63c3005b3..8d3ae4d26f89 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 @@ -175,6 +175,11 @@ public class ExpressionBuilder { }; } + private static String typeName(Class<?> type) { + // the class resolver loads an array type by its canonical name (byte[]) and not by its binary name ([B) + return type.isArray() ? type.getCanonicalName() : type.getName(); + } + /** * Returns an expression for the header value with the given name converted to the given type * <p/> @@ -185,7 +190,7 @@ 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())); + return headerExpression(simpleExpression(headerName), constantExpression(typeName(type))); } /** @@ -318,7 +323,7 @@ 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())); + return variableExpression(simpleExpression(variableName), constantExpression(typeName(type))); } /** @@ -1116,6 +1121,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 +1403,7 @@ public class ExpressionBuilder { if (source.startsWith("variable:")) { source = source.substring(9); } - exp = variableExpression(source); + exp = variableExpression(source, true); } return exp; } @@ -1694,7 +1700,7 @@ public class ExpressionBuilder { if (type != null) { return expression.evaluate(exchange, type); } else { - return expression; + return expression.evaluate(exchange, Object.class); } } @@ -1722,7 +1728,7 @@ public class ExpressionBuilder { if (result != null) { return expression.evaluate(exchange, result.getClass()); } else { - return expression; + return expression.evaluate(exchange, Object.class); } } @@ -1929,14 +1935,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); } @@ -2274,7 +2283,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 5f750d4fb38e..9d8f896c2317 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 @@ -173,6 +173,15 @@ 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. + === Apache Avro trusted packages Camel now uses Apache Avro 1.12.2. Avro validates classes resolved from schemas
