Mule
  1. Mule
  2. MULE-4646

When sync endpoint and disableTempDesintations are used together JMSMessageDispatcher should return null rather than NullPayload

    Details

    • Type: Improvement Improvement
    • Status: Reopened
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.2.3 (EE only)
    • Fix Version/s: 2.2.4 (EE only)
    • Component/s: Transport: JMS
    • Labels:
      None
    • User impact:
      Medium
    • Similar Issues:
      MULE-4520Dispatchers doSend() methods should return message with NullPayload rather than null
      MULE-4617JMSReplyToHandler tries to send to JMS endpoint synchronously (creating temp destination) rather than dispatching
      MULE-3648Nested router should check for null response (caused when async back channel times out) and throw an exception rather than return null
      MULE-4405Proxy generated for component binding throws ClassCastException by NullPayload
      MULE-5806<poll> should not trigger flow if nested processor returns NullPayload
      MULE-4621MuleClient.send() uses async (rather than sync) outbound endpoints
      MULE-1936ChainingRouter does not handle NullPayload properly
      MULE-4561Synchronous transports return request message rather than error when security exceptions occurs
      MULE-5741DefaultExceptionStrategy returns message with NullPayload rather than original message with exceptionPayload set
      MULE-4192 Synchronous inbound endpoints with PassThroughComponent and synchronous outbound endpoints should return a NullPayload

      Description

      This is because in this case no response is expected and therefore the service should behave in the an "outbound aysnc" mode and return the result of component inovcation and not the NullPayload response from outbound.

      1. JmsResponseTransformerTestCase.java
        0.8 kB
        Puneet Gupta
      2. responsetransformer-test.xml
        2 kB
        Puneet Gupta

        Activity

        Hide
        Travis Carlson added a comment -

        JmsTemporaryReplyToTestCase is failing for WMQ after the change.

        Show
        Travis Carlson added a comment - JmsTemporaryReplyToTestCase is failing for WMQ after the change.
        Hide
        Travis Carlson added a comment -

        Needed to update transports/wmq/src/test/resources/integration/jms-temporary-replyTo.xml in EE which overrides the CE config

        https://dev.ee.mulesource.com/fisheye/changelog/products/?cs=17179

        Show
        Travis Carlson added a comment - Needed to update transports/wmq/src/test/resources/integration/jms-temporary-replyTo.xml in EE which overrides the CE config https://dev.ee.mulesource.com/fisheye/changelog/products/?cs=17179
        Hide
        Puneet Gupta added a comment -

        We changed all of the dispatchers to return NullPayload instead null. returning null short circuits all processing. No response transformers of further endpoints are processes. A endpoint should never return null. If this is breaking a test case, check the test case or code.

        Dan F spent a lot of time fixing this for Atlas. please dont change stuff and break a lot of other functionality. I am at POC this is messing me up.

        Show
        Puneet Gupta added a comment - We changed all of the dispatchers to return NullPayload instead null. returning null short circuits all processing. No response transformers of further endpoints are processes. A endpoint should never return null. If this is breaking a test case, check the test case or code. Dan F spent a lot of time fixing this for Atlas. please dont change stuff and break a lot of other functionality. I am at POC this is messing me up.
        Hide
        Travis Carlson added a comment -

        It looks like the change which Puneet is talking about was yours Dan:
        http://fisheye.codehaus.org/changelog/mule/?cs=16181

        And it seems to be very intentional based on the in-line comment:

        // In this case a response was never expected so we return null and not NullPayload.
        // This generally happens when dispatch is used for an asynchronous endpoint but can also occur when send() is used
        // and disableTempDestinations is set.
        return returnOriginalMessageAsReply ? new DefaultMuleMessage(msg) : null;

        Show
        Travis Carlson added a comment - It looks like the change which Puneet is talking about was yours Dan: http://fisheye.codehaus.org/changelog/mule/?cs=16181 And it seems to be very intentional based on the in-line comment: // In this case a response was never expected so we return null and not NullPayload. // This generally happens when dispatch is used for an asynchronous endpoint but can also occur when send() is used // and disableTempDestinations is set. return returnOriginalMessageAsReply ? new DefaultMuleMessage(msg) : null;
        Hide
        Daniel Feist added a comment -

        This is just for a single specific case after discussions with AP.

        Show
        Daniel Feist added a comment - This is just for a single specific case after discussions with AP.
        Hide
        Puneet Gupta added a comment -

        It is a regression. Run the attached test case against 2.4 and then against 2.5 snapshot. If regressions don't matter then I probably have to start using my own branch and build my own distributions.

        Show
        Puneet Gupta added a comment - It is a regression. Run the attached test case against 2.4 and then against 2.5 snapshot. If regressions don't matter then I probably have to start using my own branch and build my own distributions.
        Hide
        Andrew Perepelytsya added a comment -

        No it is not. If you want the original message returned with JMSMessageID set, there's a returnOriginalMessage or similar flag for jms to do that. Check with Travis C for details.

        Show
        Andrew Perepelytsya added a comment - No it is not. If you want the original message returned with JMSMessageID set, there's a returnOriginalMessage or similar flag for jms to do that. Check with Travis C for details.
        Hide
        Puneet Gupta added a comment -

        I dont want the original message, I want the message from my transformer. I did not have to use this flag before and it used to work. why would using a newer version require me to change config. That is a regression cause it breaks a users existing code.

        Show
        Puneet Gupta added a comment - I dont want the original message, I want the message from my transformer. I did not have to use this flag before and it used to work. why would using a newer version require me to change config. That is a regression cause it breaks a users existing code.
        Hide
        Daniel Feist added a comment -

        This is a minor regression compared to 2.2.3 yes, but it preserves the 2.2.2 behavior in this case where a response is not expected. In the cases where a response is expected NullPayload is returned and the previous improvement remains in place.

        PG/AP: Maybe you can discuss this between you a bit?

        Show
        Daniel Feist added a comment - This is a minor regression compared to 2.2.3 yes, but it preserves the 2.2.2 behavior in this case where a response is not expected. In the cases where a response is expected NullPayload is returned and the previous improvement remains in place. PG/AP: Maybe you can discuss this between you a bit?

          People

          • Assignee:
            Daniel Feist
            Reporter:
            Daniel Feist
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development