-
Notifications
You must be signed in to change notification settings - Fork 5.1k
CAMEL-22884: Validate method attribute placement in when clause #21969
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -449,6 +450,54 @@ public void testParseError() throws Exception { | |
| assertTrue(e.getMessage().startsWith("Unexpected attribute '{}ref'")); | ||
| } | ||
|
|
||
| @Test | ||
| public void testMethodAfterStepsInWhenClauseShouldFail() throws Exception { | ||
| String routesXml = """ | ||
| <routes xmlns="http://camel.apache.org/schema/xml-io"> | ||
| <route> | ||
| <from uri="direct:start"/> | ||
| <choice> | ||
| <when> | ||
| <simple>${header.foo} == 'bar'</simple> | ||
| <log message="Condition met"/> | ||
| <method ref="someBean" method="processFooBar"/> | ||
| </when> | ||
| <otherwise> | ||
| <to uri="mock:other"/> | ||
| </otherwise> | ||
| </choice> | ||
| </route> | ||
| </routes> | ||
| """; | ||
| Exception e = assertThrows(Exception.class, () -> { | ||
| new ModelParser(new StringReader(routesXml)).parseRoutesDefinition(); | ||
| }); | ||
| assertThat(e).hasStackTraceContaining("already has a predicate"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testMethodAsPredicateInWhenClauseShouldWork() throws Exception { | ||
| String routesXml = """ | ||
| <routes xmlns="http://camel.apache.org/schema/xml-io"> | ||
| <route> | ||
| <from uri="direct:start"/> | ||
| <choice> | ||
| <when> | ||
| <method ref="someBean" method="isFoo"/> | ||
| <to uri="mock:foo"/> | ||
| </when> | ||
| <otherwise> | ||
| <to uri="mock:other"/> | ||
| </otherwise> | ||
| </choice> | ||
| </route> | ||
| </routes> | ||
| """; | ||
| RoutesDefinition routes = new ModelParser(new StringReader(routesXml)).parseRoutesDefinition().orElse(null); | ||
| assertNotNull(routes); | ||
| assertEquals(1, routes.getRoutes().size()); | ||
|
Comment on lines
+497
to
+498
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. using AssertJ assertions will allow to write it in a single assertion and have a more precise error message out of the box
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done — replaced with
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this comment was for another piece of code |
||
| } | ||
|
|
||
| private Path getResourceFolder() { | ||
| final URL resource = getClass().getClassLoader().getResource("barInterceptorRoute.xml"); | ||
| assert resource != null : "Cannot find barInterceptorRoute.xml"; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be good to add a here as the exception message tells
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea — adding a
<bean id="someBean" type="..."/>would make the test XML more realistic, even though this is purely a parser-level test (no runtime resolution). I'll update both test methods.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, on second thought —
<bean>declarations live at the<camel>root level, not inside<routes>. This test usesparseRoutesDefinition()which expects<routes>as root. Restructuring to use a<camel>wrapper would require a different parse method and adds complexity for a purely parser-level test where bean refs aren't resolved. Happy to change it if you still think it's worth it, but I'd lean toward keeping it simple since the test is only validating XML structure parsing, not runtime behavior.