camel-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From davscl...@apache.org
Subject git commit: CAMEL-6357: Routing engine avoid clearning headers if end user copied headers the wrong way. Improved SetBody EIP to check if OUT message exists when setting message.
Date Mon, 13 May 2013 16:49:29 GMT
Updated Branches:
  refs/heads/master bce6ea8e1 -> 4c9943510


CAMEL-6357: Routing engine avoid clearning headers if end user copied headers the wrong way.
Improved SetBody EIP to check if OUT message exists when setting message.


Project: http://git-wip-us.apache.org/repos/asf/camel/repo
Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/4c994351
Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/4c994351
Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/4c994351

Branch: refs/heads/master
Commit: 4c9943510a46b3a8a372155680eefe6929fc3e67
Parents: bce6ea8
Author: Claus Ibsen <davsclaus@apache.org>
Authored: Mon May 13 18:49:16 2013 +0200
Committer: Claus Ibsen <davsclaus@apache.org>
Committed: Mon May 13 18:49:16 2013 +0200

----------------------------------------------------------------------
 .../src/main/java/org/apache/camel/Message.java    |    3 +
 .../java/org/apache/camel/impl/MessageSupport.java |   18 +++-
 .../apache/camel/processor/SetBodyProcessor.java   |   12 ++-
 .../org/apache/camel/impl/MessageSupportTest.java  |   20 ++++
 .../camel/issues/SetBodyTryCatchIssueTest.java     |   73 +++++++++++++++
 5 files changed, 119 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/camel/blob/4c994351/camel-core/src/main/java/org/apache/camel/Message.java
----------------------------------------------------------------------
diff --git a/camel-core/src/main/java/org/apache/camel/Message.java b/camel-core/src/main/java/org/apache/camel/Message.java
index 4018ea1..179871f 100644
--- a/camel-core/src/main/java/org/apache/camel/Message.java
+++ b/camel-core/src/main/java/org/apache/camel/Message.java
@@ -160,6 +160,9 @@ public interface Message {
 
     /**
      * Set all the headers associated with this message
+     * <p/>
+     * <b>Important:</b> If you want to copy headers from another {@link Message}
to this {@link Message}, then
+     * use <tt>getHeaders().putAll(other)</tt> to copy the headers, where <tt>other</tt>
is the other headers.
      *
      * @param headers headers to set
      */

http://git-wip-us.apache.org/repos/asf/camel/blob/4c994351/camel-core/src/main/java/org/apache/camel/impl/MessageSupport.java
----------------------------------------------------------------------
diff --git a/camel-core/src/main/java/org/apache/camel/impl/MessageSupport.java b/camel-core/src/main/java/org/apache/camel/impl/MessageSupport.java
index 3dd2420..c199a15 100644
--- a/camel-core/src/main/java/org/apache/camel/impl/MessageSupport.java
+++ b/camel-core/src/main/java/org/apache/camel/impl/MessageSupport.java
@@ -135,11 +135,21 @@ public abstract class MessageSupport implements Message {
         setBody(that.getBody());
         setFault(that.isFault());
 
-        if (hasHeaders()) {
-            getHeaders().clear();
+        // the headers may be the same instance if the end user has made some mistake
+        // and set the OUT message with the same header instance of the IN message etc
+        boolean sameHeadersInstance = false;
+        if (hasHeaders() && that.hasHeaders() && getHeaders() == that.getHeaders())
{
+            sameHeadersInstance = true;
         }
-        if (that.hasHeaders()) {
-            getHeaders().putAll(that.getHeaders());
+
+        if (!sameHeadersInstance) {
+            if (hasHeaders()) {
+                // okay its safe to clear the headers
+                getHeaders().clear();
+            }
+            if (that.hasHeaders()) {
+                getHeaders().putAll(that.getHeaders());
+            }
         }
         
         if (hasAttachments()) {

http://git-wip-us.apache.org/repos/asf/camel/blob/4c994351/camel-core/src/main/java/org/apache/camel/processor/SetBodyProcessor.java
----------------------------------------------------------------------
diff --git a/camel-core/src/main/java/org/apache/camel/processor/SetBodyProcessor.java b/camel-core/src/main/java/org/apache/camel/processor/SetBodyProcessor.java
index 42ee0f5..23b3995 100644
--- a/camel-core/src/main/java/org/apache/camel/processor/SetBodyProcessor.java
+++ b/camel-core/src/main/java/org/apache/camel/processor/SetBodyProcessor.java
@@ -25,7 +25,7 @@ import org.apache.camel.impl.DefaultMessage;
 import org.apache.camel.support.ServiceSupport;
 
 /**
- * A processor which sets the body on the IN message with an {@link Expression}
+ * A processor which sets the body on the IN or OUT message with an {@link Expression}
  */
 public class SetBodyProcessor extends ServiceSupport implements Processor, Traceable {
     private final Expression expression;
@@ -37,13 +37,19 @@ public class SetBodyProcessor extends ServiceSupport implements Processor,
Trace
     public void process(Exchange exchange) throws Exception {
         Object newBody = expression.evaluate(exchange, Object.class);
 
-        Message old = exchange.getIn();
+        boolean out = exchange.hasOut();
+        Message old = out ? exchange.getOut() : exchange.getIn();
 
         // create a new message container so we do not drag specialized message objects along
         Message msg = new DefaultMessage();
         msg.copyFrom(old);
         msg.setBody(newBody);
-        exchange.setIn(msg);
+
+        if (out) {
+            exchange.setOut(msg);
+        } else {
+            exchange.setIn(msg);
+        }
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/camel/blob/4c994351/camel-core/src/test/java/org/apache/camel/impl/MessageSupportTest.java
----------------------------------------------------------------------
diff --git a/camel-core/src/test/java/org/apache/camel/impl/MessageSupportTest.java b/camel-core/src/test/java/org/apache/camel/impl/MessageSupportTest.java
index 083e8f6..910f95d 100644
--- a/camel-core/src/test/java/org/apache/camel/impl/MessageSupportTest.java
+++ b/camel-core/src/test/java/org/apache/camel/impl/MessageSupportTest.java
@@ -16,6 +16,8 @@
  */
 package org.apache.camel.impl;
 
+import java.util.Map;
+
 import org.apache.camel.ContextTestSupport;
 import org.apache.camel.Exchange;
 import org.apache.camel.InvalidPayloadException;
@@ -63,4 +65,22 @@ public class MessageSupportTest extends ContextTestSupport {
         
         assertNotNull(in.getMessageId());
     }
+
+    public void testCopyFromSameHeadersInstance() {
+        Exchange exchange = new DefaultExchange(context);
+
+        Message in = exchange.getIn();
+        Map<String, Object> headers = in.getHeaders();
+        headers.put("foo", 123);
+
+        Message out = new DefaultMessage();
+        out.setBody("Bye World");
+        out.setHeaders(headers);
+
+        out.copyFrom(in);
+
+        assertEquals(123, headers.get("foo"));
+        assertEquals(123, in.getHeader("foo"));
+        assertEquals(123, out.getHeader("foo"));
+    }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/camel/blob/4c994351/camel-core/src/test/java/org/apache/camel/issues/SetBodyTryCatchIssueTest.java
----------------------------------------------------------------------
diff --git a/camel-core/src/test/java/org/apache/camel/issues/SetBodyTryCatchIssueTest.java
b/camel-core/src/test/java/org/apache/camel/issues/SetBodyTryCatchIssueTest.java
new file mode 100644
index 0000000..a6f4b7d
--- /dev/null
+++ b/camel-core/src/test/java/org/apache/camel/issues/SetBodyTryCatchIssueTest.java
@@ -0,0 +1,73 @@
+/**
+ * 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.issues;
+
+import java.util.Map;
+
+import org.apache.camel.ContextTestSupport;
+import org.apache.camel.Exchange;
+import org.apache.camel.builder.RouteBuilder;
+
+/**
+ * @version 
+ */
+public class SetBodyTryCatchIssueTest extends ContextTestSupport {
+
+    public void testSetBody() throws Exception {
+        getMockEndpoint("mock:bar").expectedBodiesReceived("Hello World");
+        getMockEndpoint("mock:result").expectedBodiesReceived("123");
+
+        Object out = template.requestBody("direct:start", "Hello World");
+        assertEquals("123", out);
+
+        assertMockEndpointsSatisfied();
+    }
+
+    @Override
+    protected RouteBuilder createRouteBuilder() throws Exception {
+        return new RouteBuilder() {
+            @Override
+            public void configure() throws Exception {
+                from("direct:start")
+                    .setHeader("foo", constant("123"))
+                    .doTry()
+                        .setHeader("bar", constant("456"))
+                        .to("mock:bar")
+                        .bean(SetBodyTryCatchIssueTest.class, "doSomething")
+                    .doCatch(IllegalArgumentException.class)
+                        // empty block
+                    .end()
+                    .setBody(header("foo"))
+                    .to("mock:result");
+            }
+        };
+    }
+
+    public static void doSomething(Exchange exchange) throws Exception {
+        Map<String, Object> headers = exchange.getIn().getHeaders();
+
+        exchange.getOut().setBody("Bye World");
+        // we copy the headers by mistake by setting it as a reference from the IN
+        // but we should ideally do as below instead
+        // but we want to let Camel handle this situation as well, otherwise headers may
appear as lost
+        // exchange.getOut().getHeaders().putAll(headers);
+        exchange.getOut().setHeaders(headers);
+
+        throw new IllegalArgumentException("Forced");
+    }
+
+}


Mime
View raw message