You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@camel.apache.org by Christian Müller <ch...@gmail.com> on 2012/02/27 11:41:24 UTC

DefaultUnitOfWork doesn't copy headers from the original message if it's a JmsMessage

Hello!

In DefaultUnitOfWork (line 86 -93) we have the following code:

        if
(exchange.getIn().getClass().getSimpleName().equals("JmsMessage")) {
            this.originalInMessage = new DefaultMessage();
            this.originalInMessage.setBody(exchange.getIn().getBody());
            // cannot copy headers with a JmsMessage as the underlying
javax.jms.Message object goes nuts
        } else {
            this.originalInMessage = exchange.getIn().copy();
        }

Because of this, the following unit test will fail:
    @Test
    public void testUseOriginalMessage() throws Exception {
        context.addRoutes(new RouteBuilder() {
            @Override
            public void configure() throws Exception {
                onException(Exception.class)
                    .useOriginalMessage()
                    .to(mockResult);

                from("jms:queue:foo")
                    .throwException(new Exception("forced exception for
test"));
            }
        });
        context.start();

        mockResult.expectedBodiesReceived("Hello World");
        mockResult.expectedHeaderReceived("header-key", "header-value"); //
will fail here

        template.sendBodyAndHeader("jms:queue:foo", "Hello World",
"header-key", "header-value");

        assertMockEndpointsSatisfied();
    }

I propose to change the DefaultUnitOfWork to also copy the headers in this
way:
        if
(exchange.getIn().getClass().getSimpleName().equals("JmsMessage")) {
            this.originalInMessage = new DefaultMessage();
            this.originalInMessage.setBody(exchange.getIn().getBody());

this.originalInMessage.setHeaders(exchange.getIn().getHeaders());
            // cannot copy headers with a JmsMessage as the underlying
javax.jms.Message object goes nuts
        } else {
            this.originalInMessage = exchange.getIn().copy();
        }

After this change the unit test passed. Also all unit tests in camel-core
and camel-jms passed. Any doubts for this change?

Best,
Christian

Re: DefaultUnitOfWork doesn't copy headers from the original message if it's a JmsMessage

Posted by Christian Mueller <ch...@gmail.com>.
It's also fixed for Camel 2.9.3 and Camel 2.8.6.

--
View this message in context: http://camel.465427.n5.nabble.com/DefaultUnitOfWork-doesn-t-copy-headers-from-the-original-message-if-it-s-a-JmsMessage-tp5518546p5675559.html
Sent from the Camel Development mailing list archive at Nabble.com.

Re: DefaultUnitOfWork doesn't copy headers from the original message if it's a JmsMessage

Posted by Christian Müller <ch...@gmail.com>.
Ok, applied it only to trunk for Camel 2.10.0 because it's to late for
Camel 2.9.1

Best,
Christian

On Tue, Feb 28, 2012 at 9:02 AM, Claus Ibsen <cl...@gmail.com> wrote:

> Hi
>
> Yeah the headers can most likely be copied today if the JMS brokers do
> not barf anymore.
>
> However I had hoped in the mean time we had fixed this and released
> Camel 3.0, where we can optimize the logic
> to only copy on demand. I once had some experimental code for this to
> check the routes if the useOriginalMessage option was enabled or not.
> And thus only do the copy if needed. Currently the logic is *always*
> doing a defensive copy, even if the useOriginalMessage is not enabled.
>
> As well for JmsMessage when we do a copy it will have to access the
> JMS API to gather the JMS headers and whatnot.
> And for people who do straight through messaging, this is really not
> needed.
>
> Anyway I think for Camel 2.10 we can possible copy the headers as well
> for JMS, if there is no issues.
> I would like to test this with other JMS brokers than AMQ, if possible.
> For older releases such as 2.9.x and below, I would keep current
> behavior to not introduce side-effects for people using Camel in
> production.
>
>
>
> On Mon, Feb 27, 2012 at 2:18 PM, Babak Vahdat
> <ba...@swissonline.ch> wrote:
> > Hi Christian,
> >
> > I did verify it as well and indeed copying headers would also work even
> if
> > we deal with JmsMessage as well. However I've got no idea about the
> comment:
> >
> > // cannot copy headers with a JmsMessage as the underlying
> javax.jms.Message
> > object goes nuts
> >
> > Which came across the revision [1]. My assumption is that it used to
> cause
> > problems to copy the jms headers in old days of ActiveMQ (2009) so why
> this
> > comment.
> >
> > However what I also don't get is that according to [2] the JMS
> specification
> > states that header names must be valid Java identifiers, but ActiveMQ
> > (5.5.1) seems to allow even non valid Java Identifiers as header name! I
> > tried with "%" which is not a valid Java Identifier:
> >
> > javax.lang.model.SourceVersion.RELEASE_6.isIdentifier("%");
> >
> > And the test still passed:
> >
> > mockResult.expectedHeaderReceived("%", "header-value");
> >
> > I hope Claus can help us regarding this question. BTW just in case you
> get
> > some spare time would you please take a look at [3] as well?
> >
> > Thanks.
> >
> > [1] http://svn.apache.org/viewvc?view=revision&revision=792966
> > [2] http://camel.apache.org/jms.html#JMS-MessageHeaderMapping
> > [3]
> http://camel.465427.n5.nabble.com/ObjectConverter-problem-td5517376.html
> >
> > Babak
> >
> > --
> > View this message in context:
> http://camel.465427.n5.nabble.com/DefaultUnitOfWork-doesn-t-copy-headers-from-the-original-message-if-it-s-a-JmsMessage-tp5518546p5518888.html
> > Sent from the Camel Development mailing list archive at Nabble.com.
>
>
>
> --
> Claus Ibsen
> -----------------
> FuseSource
> Email: cibsen@fusesource.com
> Web: http://fusesource.com
> Twitter: davsclaus, fusenews
> Blog: http://davsclaus.blogspot.com/
> Author of Camel in Action: http://www.manning.com/ibsen/
>

Re: DefaultUnitOfWork doesn't copy headers from the original message if it's a JmsMessage

Posted by Claus Ibsen <cl...@gmail.com>.
Hi

Yeah the headers can most likely be copied today if the JMS brokers do
not barf anymore.

However I had hoped in the mean time we had fixed this and released
Camel 3.0, where we can optimize the logic
to only copy on demand. I once had some experimental code for this to
check the routes if the useOriginalMessage option was enabled or not.
And thus only do the copy if needed. Currently the logic is *always*
doing a defensive copy, even if the useOriginalMessage is not enabled.

As well for JmsMessage when we do a copy it will have to access the
JMS API to gather the JMS headers and whatnot.
And for people who do straight through messaging, this is really not needed.

Anyway I think for Camel 2.10 we can possible copy the headers as well
for JMS, if there is no issues.
I would like to test this with other JMS brokers than AMQ, if possible.
For older releases such as 2.9.x and below, I would keep current
behavior to not introduce side-effects for people using Camel in
production.



On Mon, Feb 27, 2012 at 2:18 PM, Babak Vahdat
<ba...@swissonline.ch> wrote:
> Hi Christian,
>
> I did verify it as well and indeed copying headers would also work even if
> we deal with JmsMessage as well. However I've got no idea about the comment:
>
> // cannot copy headers with a JmsMessage as the underlying javax.jms.Message
> object goes nuts
>
> Which came across the revision [1]. My assumption is that it used to cause
> problems to copy the jms headers in old days of ActiveMQ (2009) so why this
> comment.
>
> However what I also don't get is that according to [2] the JMS specification
> states that header names must be valid Java identifiers, but ActiveMQ
> (5.5.1) seems to allow even non valid Java Identifiers as header name! I
> tried with "%" which is not a valid Java Identifier:
>
> javax.lang.model.SourceVersion.RELEASE_6.isIdentifier("%");
>
> And the test still passed:
>
> mockResult.expectedHeaderReceived("%", "header-value");
>
> I hope Claus can help us regarding this question. BTW just in case you get
> some spare time would you please take a look at [3] as well?
>
> Thanks.
>
> [1] http://svn.apache.org/viewvc?view=revision&revision=792966
> [2] http://camel.apache.org/jms.html#JMS-MessageHeaderMapping
> [3] http://camel.465427.n5.nabble.com/ObjectConverter-problem-td5517376.html
>
> Babak
>
> --
> View this message in context: http://camel.465427.n5.nabble.com/DefaultUnitOfWork-doesn-t-copy-headers-from-the-original-message-if-it-s-a-JmsMessage-tp5518546p5518888.html
> Sent from the Camel Development mailing list archive at Nabble.com.



-- 
Claus Ibsen
-----------------
FuseSource
Email: cibsen@fusesource.com
Web: http://fusesource.com
Twitter: davsclaus, fusenews
Blog: http://davsclaus.blogspot.com/
Author of Camel in Action: http://www.manning.com/ibsen/

Re: DefaultUnitOfWork doesn't copy headers from the original message if it's a JmsMessage

Posted by Babak Vahdat <ba...@swissonline.ch>.
Hi,

A short status update:

Regarding the expectation I had concerning the valid Java identifier check
that AMQ does ignore:

> However what I also don't get is that according to [2] the JMS
> specification states that header names must be valid Java identifiers, but
> ActiveMQ (5.5.1) seems to allow even non valid Java Identifiers as header
> name! I tried with "%" which is not a valid Java Identifier:

> javax.lang.model.SourceVersion.RELEASE_6.isIdentifier("%");

> And the test still passed:

> mockResult.expectedHeaderReceived("%", "header-value"); 

It's [1] which will *not* be fixed mostly because of the backward
compatibility.

[1] https://issues.apache.org/jira/browse/AMQ-2287

Babak

--
View this message in context: http://camel.465427.n5.nabble.com/DefaultUnitOfWork-doesn-t-copy-headers-from-the-original-message-if-it-s-a-JmsMessage-tp5518546p5543984.html
Sent from the Camel Development mailing list archive at Nabble.com.

Re: DefaultUnitOfWork doesn't copy headers from the original message if it's a JmsMessage

Posted by Babak Vahdat <ba...@swissonline.ch>.
Hi Christian,

I did verify it as well and indeed copying headers would also work even if
we deal with JmsMessage as well. However I've got no idea about the comment:

// cannot copy headers with a JmsMessage as the underlying javax.jms.Message
object goes nuts 

Which came across the revision [1]. My assumption is that it used to cause
problems to copy the jms headers in old days of ActiveMQ (2009) so why this
comment.

However what I also don't get is that according to [2] the JMS specification
states that header names must be valid Java identifiers, but ActiveMQ
(5.5.1) seems to allow even non valid Java Identifiers as header name! I
tried with "%" which is not a valid Java Identifier:

javax.lang.model.SourceVersion.RELEASE_6.isIdentifier("%");

And the test still passed:

mockResult.expectedHeaderReceived("%", "header-value"); 

I hope Claus can help us regarding this question. BTW just in case you get
some spare time would you please take a look at [3] as well?

Thanks.

[1] http://svn.apache.org/viewvc?view=revision&revision=792966
[2] http://camel.apache.org/jms.html#JMS-MessageHeaderMapping
[3] http://camel.465427.n5.nabble.com/ObjectConverter-problem-td5517376.html

Babak

--
View this message in context: http://camel.465427.n5.nabble.com/DefaultUnitOfWork-doesn-t-copy-headers-from-the-original-message-if-it-s-a-JmsMessage-tp5518546p5518888.html
Sent from the Camel Development mailing list archive at Nabble.com.