Spring Integration
  1. Spring Integration
  2. INT-1731

MessageTransformingHandler does not (contrary to the manual) act like a filter when null is returned from the transforming delegate

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Complete
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.2
    • Component/s: Documentation
    • Labels:
      None

      Description

      According to the Spring Integration Reference Manual, 6.1.2 Configuring Transformer:

      "If the return value is null, then no reply Message will be sent (effectively the same behavior as a Message Filter returning false)"

      However, we have found that this is not the case.
      The MessageTransformingHandler class sets 'requiresReply' to true when initialised. which results in the AbstractReplyProducingMessageHandler.handleMessageInternal() method throwing a MessageHandlingException ("handler 'XYZ' requires a reply, but no reply was received")

      My first preference is that MessageTransformingHandler would conform to the manual and behave like a Filter.
      However, this may be a non-backwards-compatible change for some users.

      An acceptable alternative might be to add a 'requires-reply' attribute to the '<transformer/>' element to allow this behaviour to be disabled. (An update to the quoted documentation would also be required in this case.)

        Activity

        Hide
        Mark Fisher added a comment - - edited

        Thank you for reporting this. We definitely need to - at the very least - update the reference manual.

        We actually spent quite a bit of time debating this particular issue, and decided (for 2.0) that a "Message Transformer" should always be expected to transform the source Message into a valid target Message. For any component that might return NULL (and not be considered an error), a 'service-activator' could be used. Its 'requires-reply' value is FALSE by default, but can be set to TRUE in order to have Exceptions thrown for NULL return values.

        I would be interested in hearing about any use-case where the Message Transformer might return NULL - particularly if there is such a use-case where a service-activator is not a more appropriate choice.

        Show
        Mark Fisher added a comment - - edited Thank you for reporting this. We definitely need to - at the very least - update the reference manual. We actually spent quite a bit of time debating this particular issue, and decided (for 2.0) that a "Message Transformer" should always be expected to transform the source Message into a valid target Message. For any component that might return NULL (and not be considered an error), a 'service-activator' could be used. Its 'requires-reply' value is FALSE by default, but can be set to TRUE in order to have Exceptions thrown for NULL return values. I would be interested in hearing about any use-case where the Message Transformer might return NULL - particularly if there is such a use-case where a service-activator is not a more appropriate choice.
        Hide
        Graham Lea added a comment -

        We're using the Claim Check pattern and wanted to use a Transformer to convert the payload from the entity ID back into the Entity itself, by using a Repository as the Transformer delegate.

        Our Entity can (validly) go missing in the between when it entered the flow and when the reconstitution occurs, in which case the Repository returns null, and processing for that message need not continue (i.e. the implied Filter behaviour is perfect).

        FWIW, our workaround was to add another method to our Repository that checked the existence of the Entity, returning a boolean, and we used that in a <filter/> directly before the <transformer/>.

        Show
        Graham Lea added a comment - We're using the Claim Check pattern and wanted to use a Transformer to convert the payload from the entity ID back into the Entity itself, by using a Repository as the Transformer delegate. Our Entity can (validly) go missing in the between when it entered the flow and when the reconstitution occurs, in which case the Repository returns null, and processing for that message need not continue (i.e. the implied Filter behaviour is perfect). FWIW, our workaround was to add another method to our Repository that checked the existence of the Entity, returning a boolean, and we used that in a <filter/> directly before the <transformer/>.
        Hide
        Graham Lea added a comment -

        I think you probably have more relevant knowledge than I do for deciding whether a service-activator is a more appropriate.
        Let me know what you think.

        Show
        Graham Lea added a comment - I think you probably have more relevant knowledge than I do for deciding whether a service-activator is a more appropriate. Let me know what you think.
        Hide
        Oleg Zhurakousky added a comment -

        Graham

        I think what you said up above filter -> transformer was one of the main reasons why we decided on ensuring that transformer must always return non-null result. The basic principle is that if something needs to be done conditionally that is the responsibility of the filter. Mixing it with the transformer would couple two concerns together. Transformer (by definition) should only be dealing with the valid data on the input and the output side. In your case it means that filter will check for the existence of data for the claim check and will delegate to its output-channel (input to transformer) only if claim check is valid.

        Show
        Oleg Zhurakousky added a comment - Graham I think what you said up above filter -> transformer was one of the main reasons why we decided on ensuring that transformer must always return non-null result. The basic principle is that if something needs to be done conditionally that is the responsibility of the filter. Mixing it with the transformer would couple two concerns together. Transformer (by definition) should only be dealing with the valid data on the input and the output side. In your case it means that filter will check for the existence of data for the claim check and will delegate to its output-channel (input to transformer) only if claim check is valid.
        Hide
        Graham Lea added a comment -

        I can see how the "single responsibility" argument makes sense here.

        Theoretically, there could be a race condition between the Filter and the Transformer, although practically, at least in our case, this can't happen because of transactional semantics (which would probably be true for most people doing the same thing).

        It's still a little ugly, though, to have to make the same Repository call twice, once for a null check and a second time to actually do the transform.

        Would you entertain the idea of a 'requires-reply' attribute on the <transformer/> (defaulting to true as in the current implementation)?

        Show
        Graham Lea added a comment - I can see how the "single responsibility" argument makes sense here. Theoretically, there could be a race condition between the Filter and the Transformer, although practically, at least in our case, this can't happen because of transactional semantics (which would probably be true for most people doing the same thing). It's still a little ugly, though, to have to make the same Repository call twice, once for a null check and a second time to actually do the transform. Would you entertain the idea of a 'requires-reply' attribute on the <transformer/> (defaulting to true as in the current implementation)?
        Hide
        Oleg Zhurakousky added a comment - - edited

        Well,

        You said it yourself, that in your use case "...that message need not continue...", so you really don't need a filter.
        You can simply have a transformer checking with Repository and if it return null, have transformer return the original message which is a Claim Check. The downstream endpoint will see that it is not a full message but the Claim Check and will proceed accordingly.
        In other words i assume that in your code you are doing some null checking and all I am saying is instead of null checking check for some value that would tell you that it is a claim check.

        Show
        Oleg Zhurakousky added a comment - - edited Well, You said it yourself, that in your use case "...that message need not continue..." , so you really don't need a filter. You can simply have a transformer checking with Repository and if it return null, have transformer return the original message which is a Claim Check. The downstream endpoint will see that it is not a full message but the Claim Check and will proceed accordingly. In other words i assume that in your code you are doing some null checking and all I am saying is instead of null checking check for some value that would tell you that it is a claim check.
        Hide
        Oleg Zhurakousky added a comment -

        Sorry, i'll take that back. You said need NOT continue
        So in this case the exception will be thrown and you can simply ignore it. Right?

        Show
        Oleg Zhurakousky added a comment - Sorry, i'll take that back. You said need NOT continue So in this case the exception will be thrown and you can simply ignore it. Right?
        Hide
        Oleg Zhurakousky added a comment -

        Graham

        I've fixed the documentation and will resolve the issue. However I understand that our discussion might not be over. I would suggest to move this discussion to Spring Forums http://forum.springsource.org/forumdisplay.php?f=42 as it is a better place which can also facilitate more community participation.

        Show
        Oleg Zhurakousky added a comment - Graham I've fixed the documentation and will resolve the issue. However I understand that our discussion might not be over. I would suggest to move this discussion to Spring Forums http://forum.springsource.org/forumdisplay.php?f=42 as it is a better place which can also facilitate more community participation.
        Hide
        Graham Lea added a comment -

        Hi Oleg,

        I think between yourself and Mark there's enough good workaround suggestions above that this isn't really an issue.
        Following the Transformer with a Filter or Router, or just using a Service Activator instead, would all work perfectly fine.

        Thanks for your time,

        Graham.

        Show
        Graham Lea added a comment - Hi Oleg, I think between yourself and Mark there's enough good workaround suggestions above that this isn't really an issue. Following the Transformer with a Filter or Router, or just using a Service Activator instead, would all work perfectly fine. Thanks for your time, Graham.

          People

          • Assignee:
            Oleg Zhurakousky
            Reporter:
            Graham Lea
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: