This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/master by this push: new 9dd9a35 CAMEL-16162: camel-core - OnCompletion EIP - Only 1 allowed 9dd9a35 is described below commit 9dd9a35621a834856cff5346552a4097912797c4 Author: Claus Ibsen <claus.ib...@gmail.com> AuthorDate: Fri Mar 19 07:56:21 2021 +0100 CAMEL-16162: camel-core - OnCompletion EIP - Only 1 allowed --- .../SpringOnCompletionCompleteAndFailureTest.java | 50 +++++++++++++++++++++ .../SpringOnCompletionCompleteAndFailureTest.xml | 39 +++++++++++++++++ .../apache/camel/model/ProcessorDefinition.java | 19 ++++++-- .../apache/camel/model/RouteDefinitionHelper.java | 14 ++++++ .../OnCompletionCompleteAndFailureTest.java | 51 ++++++++++++++++++++++ .../ROOT/pages/camel-3x-upgrade-guide-3_9.adoc | 15 +++++++ 6 files changed, 184 insertions(+), 4 deletions(-) diff --git a/components/camel-spring-xml/src/test/java/org/apache/camel/spring/processor/SpringOnCompletionCompleteAndFailureTest.java b/components/camel-spring-xml/src/test/java/org/apache/camel/spring/processor/SpringOnCompletionCompleteAndFailureTest.java new file mode 100644 index 0000000..b334928 --- /dev/null +++ b/components/camel-spring-xml/src/test/java/org/apache/camel/spring/processor/SpringOnCompletionCompleteAndFailureTest.java @@ -0,0 +1,50 @@ +/* + * 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.spring.processor; + +import org.apache.camel.spring.SpringTestSupport; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.context.support.AbstractXmlApplicationContext; +import org.springframework.context.support.ClassPathXmlApplicationContext; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class SpringOnCompletionCompleteAndFailureTest extends SpringTestSupport { + + @Override + public void setUp() throws Exception { + try { + super.setUp(); + Assertions.fail("Should throw exception"); + } catch (Exception e) { + assertEquals("Only 1 onCompletion is allowed per route.", e.getMessage()); + } + } + + @Override + protected AbstractXmlApplicationContext createApplicationContext() { + return new ClassPathXmlApplicationContext( + "org/apache/camel/spring/processor/SpringOnCompletionCompleteAndFailureTest.xml"); + } + + @Test + public void testOnCompletion() throws Exception { + // noop + } + +} diff --git a/components/camel-spring-xml/src/test/resources/org/apache/camel/spring/processor/SpringOnCompletionCompleteAndFailureTest.xml b/components/camel-spring-xml/src/test/resources/org/apache/camel/spring/processor/SpringOnCompletionCompleteAndFailureTest.xml new file mode 100644 index 0000000..3089c99 --- /dev/null +++ b/components/camel-spring-xml/src/test/resources/org/apache/camel/spring/processor/SpringOnCompletionCompleteAndFailureTest.xml @@ -0,0 +1,39 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + + 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. + +--> +<beans xmlns="http://www.springframework.org/schema/beans" + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:schemaLocation=" + http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd + http://camel.apache.org/schema/spring http://camel.apache.org/schema/spring/camel-spring.xsd + "> + + <camelContext xmlns="http://camel.apache.org/schema/spring"> + <route> + <from uri="direct:start"/> + <onCompletion onCompleteOnly="true"> + <to uri="mock:ok"/> + </onCompletion> + <onCompletion onFailureOnly="true"> + <to uri="mock:error"/> + </onCompletion> + <to uri="mock:result"/> + </route> + </camelContext> +</beans> 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 25479ee..8820308 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 @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Comparator; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.concurrent.TimeUnit; @@ -3557,11 +3558,21 @@ public abstract class ProcessorDefinition<Type extends ProcessorDefinition<Type> */ public OnCompletionDefinition onCompletion() { OnCompletionDefinition answer = new OnCompletionDefinition(); - // we must remove all existing on completion definition (as they are - // global) - // and thus we are the only one as route scoped should override any - // global scoped + + Iterator<OnCompletionDefinition> it + = ProcessorDefinitionHelper.filterTypeInOutputs(getOutputs(), OnCompletionDefinition.class); + // check if there is a clash + while (it.hasNext()) { + OnCompletionDefinition ocd = it.next(); + if (ocd.isRouteScoped()) { + throw new IllegalArgumentException("Only 1 onCompletion is allowed per route."); + } + } + // remove all on completions as they would be global scoped and we add a route scoped which + // should override the global answer.removeAllOnCompletionDefinition(this); + + // create new block with the onCompletion popBlock(); addOutput(answer); pushBlock(answer); diff --git a/core/camel-core-model/src/main/java/org/apache/camel/model/RouteDefinitionHelper.java b/core/camel-core-model/src/main/java/org/apache/camel/model/RouteDefinitionHelper.java index abda679..9b99990 100644 --- a/core/camel-core-model/src/main/java/org/apache/camel/model/RouteDefinitionHelper.java +++ b/core/camel-core-model/src/main/java/org/apache/camel/model/RouteDefinitionHelper.java @@ -636,6 +636,20 @@ public final class RouteDefinitionHelper { List<OnCompletionDefinition> onCompletions) { List<OnCompletionDefinition> completions = new ArrayList<>(); + // check if there is a clash + Iterator<OnCompletionDefinition> it + = ProcessorDefinitionHelper.filterTypeInOutputs(abstracts, OnCompletionDefinition.class); + int count = 0; + while (it.hasNext()) { + OnCompletionDefinition ocd = it.next(); + if (ocd.isRouteScoped()) { + count++; + } + } + if (count > 1) { + throw new IllegalArgumentException("Only 1 onCompletion is allowed per route."); + } + // find the route scoped onCompletions for (ProcessorDefinition out : abstracts) { if (out instanceof OnCompletionDefinition) { diff --git a/core/camel-core/src/test/java/org/apache/camel/processor/OnCompletionCompleteAndFailureTest.java b/core/camel-core/src/test/java/org/apache/camel/processor/OnCompletionCompleteAndFailureTest.java new file mode 100644 index 0000000..96072b5 --- /dev/null +++ b/core/camel-core/src/test/java/org/apache/camel/processor/OnCompletionCompleteAndFailureTest.java @@ -0,0 +1,51 @@ +/* + * 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.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +public class OnCompletionCompleteAndFailureTest extends ContextTestSupport { + + @Override + public boolean isUseRouteBuilder() { + return false; + } + + @Test + public void testInvalid() throws Exception { + try { + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + from("direct:start") + .onCompletion().onCompleteOnly().to("mock:ok").end() + .onCompletion().onFailureOnly().to("mock:error").end() + .to("mock:result"); + } + }); + fail("Should throw exception"); + } catch (IllegalArgumentException e) { + assertEquals("Only 1 onCompletion is allowed per route.", e.getMessage()); + } + } + +} diff --git a/docs/user-manual/modules/ROOT/pages/camel-3x-upgrade-guide-3_9.adoc b/docs/user-manual/modules/ROOT/pages/camel-3x-upgrade-guide-3_9.adoc index c784f82..9e52652 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-3x-upgrade-guide-3_9.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-3x-upgrade-guide-3_9.adoc @@ -50,6 +50,21 @@ exchange properties then before they would get mixed together. What we have done The `choice` and `filter` EIPs no longer store exchange property `Exchange.FILTER_MATCHED`. The reason is the information is seldom in use, and by removing we were able to optimize camel-core. +=== OnCompletion EIP + +Camel now validates that a route has only 1 onCompletion. Previously users could have code such as: + +[source,java] +---- +from("direct:start") + .onCompletion().onCompleteOnly().to("mock:ok").end() + .onCompletion().onFailureOnly().to("mock:error").end() + .to("mock:result"); +---- + +Which would lead to the last onCompletion override the first, meaning that only `onCompletion().onFailureOnly()` +would be active. Now this is checked on startup and Camel reports an error. + === Modularized camel-spring The `camel-spring` component has been modularized into: