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 9fac889dd2f CAMEL-21958: camel-core: Java DSL. Fix endChoice() to reuse end() and scope to current/nearest choice. (#17761) 9fac889dd2f is described below commit 9fac889dd2f21597a3f918275e0abc3eca977447 Author: Claus Ibsen <claus.ib...@gmail.com> AuthorDate: Wed Apr 16 07:20:21 2025 +0200 CAMEL-21958: camel-core: Java DSL. Fix endChoice() to reuse end() and scope to current/nearest choice. (#17761) * CAMEL-21958: camel-core: Java DSL. Fix endChoice() to reuse end() and scope to current/nearest choice. --- .../org/apache/camel/model/ChoiceDefinition.java | 5 ++ .../apache/camel/model/ProcessorDefinition.java | 24 +++---- .../camel/processor/NestedChoiceIssueTest.java | 10 ++- .../processor/NestedChoiceOtherwiseIssueTest.java | 82 ++++++++++++++++++++++ .../processor/TripleNestedChoiceIssueTest.java | 10 ++- .../async/AsyncNestedTripleChoiceIssueTest.java | 4 +- .../ROOT/pages/camel-4x-upgrade-guide-4_12.adoc | 52 ++++++++++++++ 7 files changed, 170 insertions(+), 17 deletions(-) diff --git a/core/camel-core-model/src/main/java/org/apache/camel/model/ChoiceDefinition.java b/core/camel-core-model/src/main/java/org/apache/camel/model/ChoiceDefinition.java index 4d771dda24e..64e2dd5abc7 100644 --- a/core/camel-core-model/src/main/java/org/apache/camel/model/ChoiceDefinition.java +++ b/core/camel-core-model/src/main/java/org/apache/camel/model/ChoiceDefinition.java @@ -186,6 +186,11 @@ public class ChoiceDefinition extends NoOutputDefinition<ChoiceDefinition> { * @return the builder */ public ChoiceDefinition otherwise() { + if (this.otherwise != null) { + throw new IllegalArgumentException( + "Cannot add a 2nd otherwise to this choice: " + this + + ". If you have nested choice then you may need to end().endChoice() to go back to parent choice."); + } OtherwiseDefinition answer = new OtherwiseDefinition(); addClause(answer); return this; diff --git a/core/camel-core-model/src/main/java/org/apache/camel/model/ProcessorDefinition.java b/core/camel-core-model/src/main/java/org/apache/camel/model/ProcessorDefinition.java index 8f8c7ae841f..79e652f0264 100644 --- a/core/camel-core-model/src/main/java/org/apache/camel/model/ProcessorDefinition.java +++ b/core/camel-core-model/src/main/java/org/apache/camel/model/ProcessorDefinition.java @@ -41,7 +41,6 @@ import org.apache.camel.Exchange; import org.apache.camel.ExchangePattern; import org.apache.camel.Expression; import org.apache.camel.LoggingLevel; -import org.apache.camel.NamedNode; import org.apache.camel.Predicate; import org.apache.camel.Processor; import org.apache.camel.builder.DataFormatClause; @@ -1167,25 +1166,24 @@ public abstract class ProcessorDefinition<Type extends ProcessorDefinition<Type> public ChoiceDefinition endChoice() { ProcessorDefinition<?> def = this; - // are we nested choice? - if (def.getParent() instanceof ChoiceDefinition cho) { + // are we already a choice + if (def instanceof ChoiceDefinition cho) { return cho; } + // end and find the choice + def = end(); + if (def instanceof RouteDefinition) { + // okay that was too far down so go back up + def = this; + } + // are we already a choice? if (def instanceof ChoiceDefinition choice) { return choice; - } - - // okay end this and get back to the choice - def = end(); - NamedNode p = def.getParent(); - if ("when".equals(p.getShortName())) { - return (ChoiceDefinition) p; - } else if ("otherwise".equals(p.getShortName())) { - return (ChoiceDefinition) p; } else { - return (ChoiceDefinition) def; + throw new IllegalArgumentException( + "Cannot endChoice() to find current/parent choice DSL. If you have nested choice then you may need to end().endChoice() to go back to parent choice."); } } diff --git a/core/camel-core/src/test/java/org/apache/camel/processor/NestedChoiceIssueTest.java b/core/camel-core/src/test/java/org/apache/camel/processor/NestedChoiceIssueTest.java index b46a21e2a19..c91ea223d30 100644 --- a/core/camel-core/src/test/java/org/apache/camel/processor/NestedChoiceIssueTest.java +++ b/core/camel-core/src/test/java/org/apache/camel/processor/NestedChoiceIssueTest.java @@ -18,8 +18,11 @@ package org.apache.camel.processor; import org.apache.camel.ContextTestSupport; import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.support.PluginHelper; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertNotNull; + public class NestedChoiceIssueTest extends ContextTestSupport { @Test @@ -46,6 +49,11 @@ public class NestedChoiceIssueTest extends ContextTestSupport { @Test public void testNestedChoiceLow() throws Exception { + String xml = PluginHelper.getModelToXMLDumper(context).dumpModelAsXml(context, + context.getRouteDefinition("route1")); + assertNotNull(xml); + log.info(xml); + getMockEndpoint("mock:low").expectedMessageCount(1); getMockEndpoint("mock:med").expectedMessageCount(0); getMockEndpoint("mock:big").expectedMessageCount(0); @@ -62,7 +70,7 @@ public class NestedChoiceIssueTest extends ContextTestSupport { public void configure() { from("direct:start").choice().when(header("foo").isGreaterThan(1)).choice().when(header("foo").isGreaterThan(5)) .to("mock:big").otherwise().to("mock:med") - .endChoice().otherwise().to("mock:low").end(); + .end().endChoice().otherwise().to("mock:low").end(); } }; } diff --git a/core/camel-core/src/test/java/org/apache/camel/processor/NestedChoiceOtherwiseIssueTest.java b/core/camel-core/src/test/java/org/apache/camel/processor/NestedChoiceOtherwiseIssueTest.java new file mode 100644 index 00000000000..a938e488760 --- /dev/null +++ b/core/camel-core/src/test/java/org/apache/camel/processor/NestedChoiceOtherwiseIssueTest.java @@ -0,0 +1,82 @@ +/* + * 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.processor; + +import org.apache.camel.ContextTestSupport; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.support.PluginHelper; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +public class NestedChoiceOtherwiseIssueTest extends ContextTestSupport { + + @Test + public void testNestedChoiceOtherwise() throws Exception { + String xml = PluginHelper.getModelToXMLDumper(context).dumpModelAsXml(context, + context.getRouteDefinition("myRoute")); + assertNotNull(xml); + log.info(xml); + + getMockEndpoint("mock:foo").expectedMessageCount(1); + getMockEndpoint("mock:bar").expectedMessageCount(0); + getMockEndpoint("mock:other").expectedMessageCount(0); + getMockEndpoint("mock:other2").expectedMessageCount(0); + template.sendBodyAndHeader("direct:start", "Hello World", "foo", 10); + assertMockEndpointsSatisfied(); + + resetMocks(); + getMockEndpoint("mock:foo").expectedMessageCount(0); + getMockEndpoint("mock:bar").expectedMessageCount(1); + getMockEndpoint("mock:other").expectedMessageCount(1); + getMockEndpoint("mock:other2").expectedMessageCount(0); + template.sendBodyAndHeader("direct:start", "Hello World", "bar", 11); + assertMockEndpointsSatisfied(); + + resetMocks(); + getMockEndpoint("mock:foo").expectedMessageCount(0); + getMockEndpoint("mock:bar").expectedMessageCount(0); + getMockEndpoint("mock:other").expectedMessageCount(1); + getMockEndpoint("mock:other2").expectedMessageCount(1); + template.sendBodyAndHeader("direct:start", "Hello World", "cheese", 12); + assertMockEndpointsSatisfied(); + } + + @Override + protected RouteBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() { + from("direct:start").routeId("myRoute") + .errorHandler(noErrorHandler()) + .choice() + .when(header("foo")) + .to("mock:foo") + .otherwise() + .to("mock:other") + .choice() + .when(header("bar")) + .to("mock:bar") + .endChoice() + .otherwise() + .to("mock:other2") + .end() + .end(); + } + }; + } +} diff --git a/core/camel-core/src/test/java/org/apache/camel/processor/TripleNestedChoiceIssueTest.java b/core/camel-core/src/test/java/org/apache/camel/processor/TripleNestedChoiceIssueTest.java index c06cfb6eba4..17b469ae04a 100644 --- a/core/camel-core/src/test/java/org/apache/camel/processor/TripleNestedChoiceIssueTest.java +++ b/core/camel-core/src/test/java/org/apache/camel/processor/TripleNestedChoiceIssueTest.java @@ -18,8 +18,11 @@ package org.apache.camel.processor; import org.apache.camel.ContextTestSupport; import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.support.PluginHelper; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertNotNull; + public class TripleNestedChoiceIssueTest extends ContextTestSupport { @Test @@ -60,6 +63,11 @@ public class TripleNestedChoiceIssueTest extends ContextTestSupport { @Test public void testNestedChoiceLow() throws Exception { + String xml = PluginHelper.getModelToXMLDumper(context).dumpModelAsXml(context, + context.getRouteDefinition("route1")); + assertNotNull(xml); + log.info(xml); + getMockEndpoint("mock:low").expectedMessageCount(1); getMockEndpoint("mock:med").expectedMessageCount(0); getMockEndpoint("mock:big").expectedMessageCount(0); @@ -77,7 +85,7 @@ public class TripleNestedChoiceIssueTest extends ContextTestSupport { public void configure() { from("direct:start").choice().when(header("foo").isGreaterThan(1)).choice().when(header("foo").isGreaterThan(5)) .choice().when(header("foo").isGreaterThan(10)) - .to("mock:verybig").otherwise().to("mock:big").endChoice().otherwise().to("mock:med").endChoice() + .to("mock:verybig").otherwise().to("mock:big").end().endChoice().otherwise().to("mock:med").end().endChoice() .otherwise().to("mock:low").end(); } }; diff --git a/core/camel-core/src/test/java/org/apache/camel/processor/async/AsyncNestedTripleChoiceIssueTest.java b/core/camel-core/src/test/java/org/apache/camel/processor/async/AsyncNestedTripleChoiceIssueTest.java index 036bc7ac99c..8f8a284662f 100644 --- a/core/camel-core/src/test/java/org/apache/camel/processor/async/AsyncNestedTripleChoiceIssueTest.java +++ b/core/camel-core/src/test/java/org/apache/camel/processor/async/AsyncNestedTripleChoiceIssueTest.java @@ -79,8 +79,8 @@ public class AsyncNestedTripleChoiceIssueTest extends ContextTestSupport { from("direct:start").choice().when(header("foo").isGreaterThan(1)).to("async:bye:camel").choice() .when(header("foo").isGreaterThan(5)).to("async:bye:camel2") - .choice().when(header("foo").isGreaterThan(7)).to("mock:verybig").otherwise().to("mock:big").endChoice() - .otherwise().to("mock:med").endChoice().otherwise() + .choice().when(header("foo").isGreaterThan(7)).to("mock:verybig").otherwise().to("mock:big").end().endChoice() + .otherwise().to("mock:med").end().endChoice().otherwise() .to("mock:low").end(); } }; diff --git a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_12.adoc b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_12.adoc index 7f5f35e0072..cbb45539a62 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_12.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_12.adoc @@ -27,6 +27,58 @@ to allow returning `null` as a positive answer when using `convertBodyTo` EIP an This behaviour was present in Camel v2 and some internal optimization in Camel v3 had changed this to not be the case. Using type converters that can return `null` is a rare use-case, and it's not a good practice. +=== Java DSL + +When using Choice EIP then in some situations you may need to use `.endChoice()` +to be able to either continue added more nodes to the current Choice EIP, or that you +are working with nested Choice EIPs (choice inside choice), then you may also need to use `endChoice` +to go back to the parent choice to continue from there. + +However, there has been some regressions from upgrading older Camel releases to 4.11, and therefore +we have refactored `endChoice` to work more consistent. + +For example the following code + +[source,java] +---- +from("direct:start") + .choice() + .when(header("foo").isGreaterThan(1)) + .choice() + .when(header("foo").isGreaterThan(5)) + .to("mock:big") + .otherwise() + .to("mock:med") + .endChoice() + .otherwise() + .to("mock:low") + .end(); +---- + +Should now be + +[source,java] +---- +from("direct:start") + .choice() + .when(header("foo").isGreaterThan(1)) + .choice() + .when(header("foo").isGreaterThan(5)) + .to("mock:big") + .otherwise() + .to("mock:med") + .end().endChoice() + .otherwise() + .to("mock:low") + .end(); +---- + +Notice that the `endChoice` is changed to `end().endChoice()`. This is required to be consistent +to end the current choice (inner) and then afterwards change the scope to be Choice EIP to be able to +continue in the outer Choice. Otherwise the Java DSL cannot know the scope is Choice EIP and you would +not be able to add the `otherwise` block to the outer Choice. + + === camel-as2 Add options allowing the addition of an `Authorization` header for Basic or Bearer authentication to client and