From 5d1bc14daa89e6a12c59c11c9ced2ce6ced67101 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 13 Mar 2026 08:36:13 +0100 Subject: [PATCH 1/2] CAMEL-22884: Validate method attribute placement in when clause Co-Authored-By: Claude Opus 4.6 --- .../model/BasicOutputExpressionNode.java | 28 +++++++++++ .../apache/camel/xml/in/ModelParserTest.java | 46 +++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/core/camel-core-model/src/main/java/org/apache/camel/model/BasicOutputExpressionNode.java b/core/camel-core-model/src/main/java/org/apache/camel/model/BasicOutputExpressionNode.java index 2ab7ecb72732c..469905347bd0e 100644 --- a/core/camel-core-model/src/main/java/org/apache/camel/model/BasicOutputExpressionNode.java +++ b/core/camel-core-model/src/main/java/org/apache/camel/model/BasicOutputExpressionNode.java @@ -75,4 +75,32 @@ public void setOutputs(List> outputs) { public void addOutput(ProcessorDefinition output) { this.outputs.add(output); } + + @Override + public void setExpression(ExpressionDefinition expression) { + // Detect when an expression element (e.g. ) appears after processing steps + // inside a when/filter clause in XML/YAML DSL. This is almost certainly a user mistake + // where they intended to use (processor) instead of (expression/predicate). + // We skip this check when the existing expression wraps an ExpressionClause (Java DSL), + // because preCreateProcessor() legitimately re-sets the expression after resolving it. + if (expression != null && getExpression() != null && !outputs.isEmpty()) { + ExpressionDefinition existing = getExpression(); + boolean isExpressionClause + = existing.getExpressionValue() instanceof org.apache.camel.builder.ExpressionClause + || existing.getPredicate() instanceof org.apache.camel.builder.ExpressionClause; + if (!isExpressionClause) { + String lang = expression.getLanguage() != null + ? expression.getLanguage() : expression.getClass().getSimpleName(); + throw new IllegalArgumentException( + "The " + getShortName() + " already has a predicate (" + existing + + ") and " + outputs.size() + " output(s). " + + "The expression '" + lang + + "' is being parsed as an expression/predicate but appears after processing steps. " + + "If you intended to call a bean method as a processing step, use instead of . " + + "An expression element must be the first child of <" + getShortName() + + ">."); + } + } + super.setExpression(expression); + } } diff --git a/core/camel-xml-io/src/test/java/org/apache/camel/xml/in/ModelParserTest.java b/core/camel-xml-io/src/test/java/org/apache/camel/xml/in/ModelParserTest.java index 150e89434e8d2..679edbff0924c 100644 --- a/core/camel-xml-io/src/test/java/org/apache/camel/xml/in/ModelParserTest.java +++ b/core/camel-xml-io/src/test/java/org/apache/camel/xml/in/ModelParserTest.java @@ -449,6 +449,52 @@ public void testParseError() throws Exception { assertTrue(e.getMessage().startsWith("Unexpected attribute '{}ref'")); } + @Test + public void testMethodAfterStepsInWhenClauseShouldFail() throws Exception { + String routesXml = "" + + " " + + " " + + " " + + " " + + " ${header.foo} == 'bar'" + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + ""; + Exception e = assertThrows(Exception.class, () -> { + new ModelParser(new StringReader(routesXml)).parseRoutesDefinition(); + }); + assertTrue(e.getMessage().contains("already has a predicate") + || e.getCause().getMessage().contains("already has a predicate"), + "Error message should indicate that the when clause already has a predicate: " + e.getMessage()); + } + + @Test + public void testMethodAsPredicateInWhenClauseShouldWork() throws Exception { + String routesXml = "" + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + ""; + RoutesDefinition routes = new ModelParser(new StringReader(routesXml)).parseRoutesDefinition().orElse(null); + assertNotNull(routes); + assertEquals(1, routes.getRoutes().size()); + } + private Path getResourceFolder() { final URL resource = getClass().getClassLoader().getResource("barInterceptorRoute.xml"); assert resource != null : "Cannot find barInterceptorRoute.xml"; From 577b5cb231590fd89c738967bbe19e0b364e4dc9 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 13 Mar 2026 11:15:33 +0100 Subject: [PATCH 2/2] CAMEL-22884: Address review comments - use text blocks and AssertJ --- .../apache/camel/xml/in/ModelParserTest.java | 67 ++++++++++--------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/core/camel-xml-io/src/test/java/org/apache/camel/xml/in/ModelParserTest.java b/core/camel-xml-io/src/test/java/org/apache/camel/xml/in/ModelParserTest.java index 679edbff0924c..882ec709678be 100644 --- a/core/camel-xml-io/src/test/java/org/apache/camel/xml/in/ModelParserTest.java +++ b/core/camel-xml-io/src/test/java/org/apache/camel/xml/in/ModelParserTest.java @@ -57,6 +57,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -451,45 +452,47 @@ public void testParseError() throws Exception { @Test public void testMethodAfterStepsInWhenClauseShouldFail() throws Exception { - String routesXml = "" - + " " - + " " - + " " - + " " - + " ${header.foo} == 'bar'" - + " " - + " " - + " " - + " " - + " " - + " " - + " " - + " " - + ""; + String routesXml = """ + + + + + + ${header.foo} == 'bar' + + + + + + + + + + """; Exception e = assertThrows(Exception.class, () -> { new ModelParser(new StringReader(routesXml)).parseRoutesDefinition(); }); - assertTrue(e.getMessage().contains("already has a predicate") - || e.getCause().getMessage().contains("already has a predicate"), - "Error message should indicate that the when clause already has a predicate: " + e.getMessage()); + assertThat(e).hasStackTraceContaining("already has a predicate"); } @Test public void testMethodAsPredicateInWhenClauseShouldWork() throws Exception { - String routesXml = "" - + " " - + " " - + " " - + " " - + " " - + " " - + " " - + " " - + " " - + " " - + " " - + " " - + ""; + String routesXml = """ + + + + + + + + + + + + + + + """; RoutesDefinition routes = new ModelParser(new StringReader(routesXml)).parseRoutesDefinition().orElse(null); assertNotNull(routes); assertEquals(1, routes.getRoutes().size());