Skip to content

CAMEL-22884: Validate method attribute placement in when clause#21969

Open
gnodet wants to merge 2 commits intomainfrom
camel-22884-when-method-validation
Open

CAMEL-22884: Validate method attribute placement in when clause#21969
gnodet wants to merge 2 commits intomainfrom
camel-22884-when-method-validation

Conversation

@gnodet
Copy link
Contributor

@gnodet gnodet commented Mar 13, 2026

Summary

  • Adds validation in BasicOutputExpressionNode.setExpression() to detect when an expression element (e.g. <method>) appears after processing steps inside a <when> clause in XML/YAML DSL
  • Previously, a <method> element placed after <log> or other steps in a <when> clause would silently overwrite the predicate expression instead of being treated as a processor or raising an error
  • The validation correctly skips the check for Java DSL where ExpressionClause is legitimately resolved to the actual expression during preCreateProcessor()

Test plan

  • Added testMethodAfterStepsInWhenClauseShouldFail to verify that placing <method> after processing steps in a <when> clause raises an error
  • Added testMethodAsPredicateInWhenClauseShouldWork to verify that <method> as the first (and only expression) child of <when> still works correctly
  • Verified all existing Choice/When tests pass (144 tests in camel-core, 21 tests in camel-xml-io)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using build-all, build-dependents, skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@github-actions github-actions bot added the core label Mar 13, 2026
@gnodet gnodet added the enhancement New feature or request label Mar 13, 2026
@gnodet gnodet marked this pull request as ready for review March 13, 2026 08:14
<choice>
<when>
<method ref="someBean" method="isFoo"/>
<to uri="mock:foo"/>
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

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 uses parseRoutesDefinition() 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants