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 2018/03/21 12:13:48 UTC

[camel] 02/03: CAMEL-11962: Fixed adviceWith not working with onException.

This is an automated email from the ASF dual-hosted git repository.

davsclaus pushed a commit to branch camel-2.19.x
in repository https://gitbox.apache.org/repos/asf/camel.git

commit 1f1a8316b37e982dc9d5f2c8b97a754a1b9ede73
Author: Claus Ibsen <cl...@gmail.com>
AuthorDate: Wed Mar 21 11:00:22 2018 +0100

    CAMEL-11962: Fixed adviceWith not working with onException.
---
 .../camel/builder/AdviceWithRouteBuilder.java      | 23 +++++++------
 .../org/apache/camel/builder/AdviceWithTasks.java  | 17 ++++++++--
 .../camel/issues/AdviceWithOnCompletionTest.java   |  1 +
 ...t.java => AdviceWithOnExceptionRemoveTest.java} | 38 +++++++++++++++-------
 4 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/camel-core/src/main/java/org/apache/camel/builder/AdviceWithRouteBuilder.java b/camel-core/src/main/java/org/apache/camel/builder/AdviceWithRouteBuilder.java
index f233297..021bfd9 100644
--- a/camel-core/src/main/java/org/apache/camel/builder/AdviceWithRouteBuilder.java
+++ b/camel-core/src/main/java/org/apache/camel/builder/AdviceWithRouteBuilder.java
@@ -31,7 +31,6 @@ import org.apache.camel.util.ObjectHelper;
  * <p/>
  * <b>Important:</b> It is recommended to only advice a given route once (you can of course advice multiple routes).
  * If you do it multiple times, then it may not work as expected, especially when any kind of error handling is involved.
- * The Camel team plan for Camel 3.0 to support this as internal refactorings in the routing engine is needed to support this properly.
  *
  * @see org.apache.camel.model.RouteDefinition#adviceWith(org.apache.camel.CamelContext, RouteBuilder)
  */
@@ -41,16 +40,16 @@ public abstract class AdviceWithRouteBuilder extends RouteBuilder {
     private final List<AdviceWithTask> adviceWithTasks = new ArrayList<AdviceWithTask>();
 
     /**
-     * Sets the original route which we advice.
+     * Sets the original route to be adviced.
      *
-     * @param originalRoute the original route we advice.
+     * @param originalRoute the original route.
      */
     public void setOriginalRoute(RouteDefinition originalRoute) {
         this.originalRoute = originalRoute;
     }
 
     /**
-     * Gets the original route we advice.
+     * Gets the original route to be adviced.
      *
      * @return the original route.
      */
@@ -69,7 +68,7 @@ public abstract class AdviceWithRouteBuilder extends RouteBuilder {
     }
 
     /**
-     * Mock all endpoints in the route.
+     * Mock all endpoints in the route (incl onException etc).
      *
      * @throws Exception can be thrown if error occurred
      */
@@ -78,7 +77,7 @@ public abstract class AdviceWithRouteBuilder extends RouteBuilder {
     }
 
     /**
-     * Mock all endpoints matching the given pattern.
+     * Mock all endpoints in the route (incl onException etc) matching the given pattern.
      *
      * @param pattern the pattern(s).
      * @throws Exception can be thrown if error occurred
@@ -124,7 +123,7 @@ public abstract class AdviceWithRouteBuilder extends RouteBuilder {
     }
 
     /**
-     * Weaves by matching id of the nodes in the route.
+     * Weaves by matching id of the nodes in the route (incl onException etc).
      * <p/>
      * Uses the {@link org.apache.camel.util.EndpointHelper#matchPattern(String, String)} matching algorithm.
      *
@@ -138,7 +137,7 @@ public abstract class AdviceWithRouteBuilder extends RouteBuilder {
     }
 
     /**
-     * Weaves by matching the to string representation of the nodes in the route.
+     * Weaves by matching the to string representation of the nodes in the route (incl onException etc).
      * <p/>
      * Uses the {@link org.apache.camel.util.EndpointHelper#matchPattern(String, String)} matching algorithm.
      *
@@ -152,7 +151,7 @@ public abstract class AdviceWithRouteBuilder extends RouteBuilder {
     }
 
     /**
-     * Weaves by matching sending to endpoints with the given uri of the nodes in the route.
+     * Weaves by matching sending to endpoints with the given uri of the nodes in the route (incl onException etc).
      * <p/>
      * Uses the {@link org.apache.camel.util.EndpointHelper#matchPattern(String, String)} matching algorithm.
      *
@@ -166,7 +165,7 @@ public abstract class AdviceWithRouteBuilder extends RouteBuilder {
     }
 
     /**
-     * Weaves by matching type of the nodes in the route.
+     * Weaves by matching type of the nodes in the route (incl onException etc).
      *
      * @param type the processor type
      * @return the builder
@@ -177,7 +176,7 @@ public abstract class AdviceWithRouteBuilder extends RouteBuilder {
     }
 
     /**
-     * Weaves by adding the nodes to the start of the route.
+     * Weaves by adding the nodes to the start of the route (excl onException etc).
      *
      * @return the builder
      */
@@ -187,7 +186,7 @@ public abstract class AdviceWithRouteBuilder extends RouteBuilder {
     }
 
     /**
-     * Weaves by adding the nodes to the end of the route.
+     * Weaves by adding the nodes to the end of the route (excl onException etc).
      *
      * @return the builder
      */
diff --git a/camel-core/src/main/java/org/apache/camel/builder/AdviceWithTasks.java b/camel-core/src/main/java/org/apache/camel/builder/AdviceWithTasks.java
index a604111..41d31cc 100644
--- a/camel-core/src/main/java/org/apache/camel/builder/AdviceWithTasks.java
+++ b/camel-core/src/main/java/org/apache/camel/builder/AdviceWithTasks.java
@@ -27,6 +27,7 @@ import org.apache.camel.model.FromDefinition;
 import org.apache.camel.model.ProcessorDefinition;
 import org.apache.camel.model.ProcessorDefinitionHelper;
 import org.apache.camel.model.RouteDefinition;
+import org.apache.camel.model.TransactedDefinition;
 import org.apache.camel.util.EndpointHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -431,10 +432,22 @@ public final class AdviceWithTasks {
         List<ProcessorDefinition<?>> matched = new ArrayList<ProcessorDefinition<?>>();
 
         List<ProcessorDefinition<?>> outputs = new ArrayList<>();
+
+        // if we are in first|last mode then we should
         // skip abstract nodes in the beginning as they are cross cutting functionality such as onException, onCompletion etc
+        // and the user want to select first or last outputs in the route (not cross cutting functionality)
+        boolean skip = selectFirst || selectLast;
+
         for (ProcessorDefinition output : route.getOutputs()) {
-            boolean invalid = outputs.isEmpty() && output.isAbstract();
-            if (!invalid) {
+            // special for transacted, which we need to unwrap
+            if (output instanceof TransactedDefinition) {
+                outputs.addAll(output.getOutputs());
+            } else if (skip) {
+                boolean invalid = outputs.isEmpty() && output.isAbstract();
+                if (!invalid) {
+                    outputs.add(output);
+                }
+            } else {
                 outputs.add(output);
             }
         }
diff --git a/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnCompletionTest.java b/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnCompletionTest.java
index 83c4c06..b7ee011 100644
--- a/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnCompletionTest.java
+++ b/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnCompletionTest.java
@@ -28,6 +28,7 @@ public class AdviceWithOnCompletionTest extends ContextTestSupport {
     public void testAdviceOnCompletion() throws Exception {
         getMockEndpoint("mock:done").expectedMessageCount(1);
         getMockEndpoint("mock:advice").expectedMessageCount(1);
+        getMockEndpoint("mock:result").expectedMessageCount(1);
 
         context.getRouteDefinitions().get(0).adviceWith(context, new AdviceWithRouteBuilder() {
             @Override
diff --git a/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnCompletionTest.java b/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnExceptionRemoveTest.java
similarity index 50%
copy from camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnCompletionTest.java
copy to camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnExceptionRemoveTest.java
index 83c4c06..e477847 100644
--- a/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnCompletionTest.java
+++ b/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnExceptionRemoveTest.java
@@ -17,26 +17,32 @@
 package org.apache.camel.issues;
 
 import org.apache.camel.ContextTestSupport;
+import org.apache.camel.Exchange;
+import org.apache.camel.Processor;
 import org.apache.camel.builder.AdviceWithRouteBuilder;
 import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.model.RouteDefinition;
 
 /**
  * @version 
  */
-public class AdviceWithOnCompletionTest extends ContextTestSupport {
+public class AdviceWithOnExceptionRemoveTest extends ContextTestSupport {
 
-    public void testAdviceOnCompletion() throws Exception {
-        getMockEndpoint("mock:done").expectedMessageCount(1);
-        getMockEndpoint("mock:advice").expectedMessageCount(1);
-
-        context.getRouteDefinitions().get(0).adviceWith(context, new AdviceWithRouteBuilder() {
+    public void testAdviceWithOnException() throws Exception {
+        RouteDefinition route = context.getRouteDefinitions().get(0);
+        route.adviceWith(context, new AdviceWithRouteBuilder() {
             @Override
             public void configure() throws Exception {
-                weaveAddFirst().to("mock:advice");
+                weaveById("removeMe").remove();
             }
         });
+        context.start();
+
+        getMockEndpoint("mock:a").expectedBodiesReceived("Hello World");
+        getMockEndpoint("mock:b").expectedMessageCount(0);
+        getMockEndpoint("mock:handled").expectedBodiesReceived("Hello World");
 
-        template.sendBody("direct:advice", "Hello World");
+        template.sendBody("direct:start", "Hello World");
 
         assertMockEndpointsSatisfied();
     }
@@ -46,11 +52,19 @@ public class AdviceWithOnCompletionTest extends ContextTestSupport {
         return new RouteBuilder() {
             @Override
             public void configure() throws Exception {
-                onCompletion().to("mock:done");
+                onException(IllegalArgumentException.class)
+                    .process(new Processor() {
+                        @Override
+                        public void process(Exchange exchange) throws Exception {
+                            exchange.getIn().setBody("I changed this");
+                        }
+                    }).id("removeMe")
+                    .handled(true).to("mock:handled");
 
-                from("direct:advice")
-                    .log("Advice ${body}")
-                    .to("mock:result");
+                from("direct:start")
+                    .to("mock:a").id("a")
+                    .throwException(new IllegalArgumentException("Forced"))
+                    .to("mock:b").id("b");
             }
         };
     }

-- 
To stop receiving notification emails like this one, please contact
davsclaus@apache.org.