Camel
  1. Camel
  2. CAMEL-2249

Wrong handling of useMessageIDAsCorrelationID

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0
    • Fix Version/s: 2.2.0
    • Component/s: camel-jms
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      Camel jms seems to contain two bugs in correlation id handling.

      The first shows when you have a sender that has UseMessageIDAsCorrelationID="false" and a server that has UseMessageIDAsCorrelationID="true". If you send a message with correlationId="a" then the response message will contain correlationId="<request message id>". Even if this could be a valid behaviour as you wanted UseMessageIDAsCorrelationID="true" I don´t think it makes sense as the sender will not be able to correlate the message. So for this case I propose to only set the correlation id to the request message id on the server if the correlation id of the request was not set.

      The second bug seems to hide the first bug. Perhaps someone found a quick (and wrong way to make the tests work). It shows when you set UseMessageIDAsCorrelationID="true" on both client and server. If you send a message with correlation id = "a" the client sends it out with this correlation id. The server then sets the correlation id to the request message id (first bug). Then on the client the reply is received. After that the correlation id is set back to "a" on the client. So the tests think all is well. This part of the code should be removed.

      I have marked both problems in the code with FIXME markers in my patch. I can also provide a patch with the solution but first I wanted to only show the problem and provide a failing test.

      Hope my explanations were not to confused

      1. camel-jms-2010-01-08.patch
        13 kB
        Christian Schneider
      2. camel-jms-correlation.patch
        6 kB
        Christian Schneider

        Activity

        Hide
        Christian Schneider added a comment -

        Patch that shows where the problems are and that contains a currently failing unit test.

        Show
        Christian Schneider added a comment - Patch that shows where the problems are and that contains a currently failing unit test.
        Hide
        Marat Bedretdinov added a comment - - edited

        Hi Christian,

        I generally agree that JMS Message correlationID value should supersede the messageID value. Realize though that both configuration assertions and the statement I just made in the previous sentence are all out of band agreements. So one can argue either way.

        However, let me give you an example when this will break down. Certain brokers, namely IBM MQ will modify your correlationID value using their proprietary encoding. So what you're going to get back in the reply message will be very different from what you set on the request message on the producer side.

        So here the scenario:

        produce -> 1 -> consume - > produce -> 2 -> consume
        --------------- ActiveMQ ------------------------------ IBM MQ
        consume <- 4 <- produce <- consume <- 3 <- produce

        1. correlationID = C, messageID = M
        2. correlationID = C!, messageID = M1
        3. correlationID = M1, messageID = M2
        4. correlationID = C

        C! - is IBM mangled C value

        I'd check with Suemas and ask him to review your changes and run against the tests suites as I suspect if your changes were to be applied certain tests that depend on Camel will fail.

        Cheers,
        Marat

        Show
        Marat Bedretdinov added a comment - - edited Hi Christian, I generally agree that JMS Message correlationID value should supersede the messageID value. Realize though that both configuration assertions and the statement I just made in the previous sentence are all out of band agreements. So one can argue either way. However, let me give you an example when this will break down. Certain brokers, namely IBM MQ will modify your correlationID value using their proprietary encoding. So what you're going to get back in the reply message will be very different from what you set on the request message on the producer side. So here the scenario: produce -> 1 -> consume - > produce -> 2 -> consume --------------- ActiveMQ ------------------------------ IBM MQ consume <- 4 <- produce <- consume <- 3 <- produce 1. correlationID = C, messageID = M 2. correlationID = C!, messageID = M1 3. correlationID = M1, messageID = M2 4. correlationID = C C! - is IBM mangled C value I'd check with Suemas and ask him to review your changes and run against the tests suites as I suspect if your changes were to be applied certain tests that depend on Camel will fail. Cheers, Marat
        Hide
        Christian Schneider added a comment -

        I was just browsing the net for some more info. One page is from oracle:
        http://download.oracle.com/docs/cd/E13171_01/alsb/docs25/interopjms/MsgIDPatternforJMS.html

        There they describe how IBM MQ uses the correlation id. So if the useMessageIdAsCorrelationId was meant in this way it could be correct. Still I would rather expect the client to not set a correaltion id if it wants this pattern.

        The following thread is also interesting: http://www.theserverside.com/discussions/thread.tss?thread_id=44779
        It says that the message id of a message can change over it´s lifetime. So if I understand this correctly it could mean that setting the correlation id to the message id could fail as this id could be different to the client´s message id.

        Show
        Christian Schneider added a comment - I was just browsing the net for some more info. One page is from oracle: http://download.oracle.com/docs/cd/E13171_01/alsb/docs25/interopjms/MsgIDPatternforJMS.html There they describe how IBM MQ uses the correlation id. So if the useMessageIdAsCorrelationId was meant in this way it could be correct. Still I would rather expect the client to not set a correaltion id if it wants this pattern. The following thread is also interesting: http://www.theserverside.com/discussions/thread.tss?thread_id=44779 It says that the message id of a message can change over it´s lifetime. So if I understand this correctly it could mean that setting the correlation id to the message id could fail as this id could be different to the client´s message id.
        Hide
        Christian Schneider added a comment -

        Are there any news about this issue? We still have the incompatibility between CXF / Camel and pure CXF. I can help some more by providing a complete patch with a solution. It would help me though if we could discuss how the "useMessageIdAsCorrelationId" feature should behave before I do the coding.

        Especially I need to know if the correlation id of the incoming message should be used as correlation id in the reply instead of the messageId if it is set.

        Show
        Christian Schneider added a comment - Are there any news about this issue? We still have the incompatibility between CXF / Camel and pure CXF. I can help some more by providing a complete patch with a solution. It would help me though if we could discuss how the "useMessageIdAsCorrelationId" feature should behave before I do the coding. Especially I need to know if the correlation id of the incoming message should be used as correlation id in the reply instead of the messageId if it is set.
        Hide
        David J. M. Karlsen added a comment -

        How about making it configurable, but with Collection by MessageID as default.
        (And act according to the "spec" in oracle link).
        This is the pattern I've seen most widespread and how I used it for JMS/MQ/zOS integration.

        "Correlation by MessageID is commonly used by many IBM MQ applications as well as JMS applications and is the standard method to correlate request and response."

        Show
        David J. M. Karlsen added a comment - How about making it configurable, but with Collection by MessageID as default. (And act according to the "spec" in oracle link). This is the pattern I've seen most widespread and how I used it for JMS/MQ/zOS integration. "Correlation by MessageID is commonly used by many IBM MQ applications as well as JMS applications and is the standard method to correlate request and response."
        Hide
        Christian Schneider added a comment -

        I also like using the messageId for correaltion. The question is what to do when the sender already has set a correlation id. Currently camel will discard the correlation id and still set the message id as correaltion id of the response. I think this is wrong as the sender expresses that he expects to get the correlation id back that he set on the request message.

        So I think we can do one of two things to correct this:
        The first way is to always use the correlation id of the request if it was set.
        The second way could be to add a config option that controls if the above behaviour should be actived.

        Which solution should be done? Are there other possible solutions?

        Show
        Christian Schneider added a comment - I also like using the messageId for correaltion. The question is what to do when the sender already has set a correlation id. Currently camel will discard the correlation id and still set the message id as correaltion id of the response. I think this is wrong as the sender expresses that he expects to get the correlation id back that he set on the request message. So I think we can do one of two things to correct this: The first way is to always use the correlation id of the request if it was set. The second way could be to add a config option that controls if the above behaviour should be actived. Which solution should be done? Are there other possible solutions?
        Hide
        Claus Ibsen added a comment -

        SI is also discussing a similar issue at their forum
        http://forum.springsource.org/showthread.php?t=82368

        Show
        Claus Ibsen added a comment - SI is also discussing a similar issue at their forum http://forum.springsource.org/showthread.php?t=82368
        Hide
        David J. M. Karlsen added a comment -

        I'd say make it configurable - it shouldn't be a lot of work to support both "protocols".
        Log warnings if correlationId is set when not expected etc.

        Show
        David J. M. Karlsen added a comment - I'd say make it configurable - it shouldn't be a lot of work to support both "protocols". Log warnings if correlationId is set when not expected etc.
        Hide
        Claus Ibsen added a comment -

        Yeah add an option to control the desired behavior.

        And remember we need unit tests to cover this as JMS is always tricky

        Show
        Claus Ibsen added a comment - Yeah add an option to control the desired behavior. And remember we need unit tests to cover this as JMS is always tricky
        Hide
        Christian Schneider added a comment -

        Added a fix and test. If useMessageId as correlationId is true then the correlationId is still forced to be the messageId. The difference now is that when useMessageIdasCorrelationId is false and the incoming correlationid is not set then the correlationId is set to the messageId (as a fallback).

        Show
        Christian Schneider added a comment - Added a fix and test. If useMessageId as correlationId is true then the correlationId is still forced to be the messageId. The difference now is that when useMessageIdasCorrelationId is false and the incoming correlationid is not set then the correlationId is set to the messageId (as a fallback).
        Hide
        Claus Ibsen added a comment -

        trunk: 897159.

        Thanks for the patch Christian.

        Show
        Claus Ibsen added a comment - trunk: 897159. Thanks for the patch Christian.
        Hide
        Christian Schneider added a comment -

        Did you have the chance to look into the code block I removed from the JmsProducer

        // FIXME remove: I think this does not make sense
        +// if (correlationId != null)

        { +// message.setJMSCorrelationID(correlationId); +// exchange.getOut().setHeader("JMSCorrelationID", correlationId); +// }

        It looked wrong to me to set the incoming correlation id to something different but perhaps it has some reason I did not understand.

        Show
        Christian Schneider added a comment - Did you have the chance to look into the code block I removed from the JmsProducer // FIXME remove: I think this does not make sense +// if (correlationId != null) { +// message.setJMSCorrelationID(correlationId); +// exchange.getOut().setHeader("JMSCorrelationID", correlationId); +// } It looked wrong to me to set the incoming correlation id to something different but perhaps it has some reason I did not understand.
        Hide
        Christian Schneider added a comment -

        I just looked into the code you committed and the comment explained what the code block was good for. So I guess my question is answered.

        Show
        Christian Schneider added a comment - I just looked into the code you committed and the comment explained what the code block was good for. So I guess my question is answered.

          People

          • Assignee:
            Claus Ibsen
            Reporter:
            Christian Schneider
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development