You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by da...@apache.org on 2021/03/19 06:56:49 UTC

[camel] branch master updated: CAMEL-16162: camel-core - OnCompletion EIP - Only 1 allowed

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 <cl...@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: