Spring Integration
  1. Spring Integration
  2. INT-2460

Allow message ids to be set when using a custom deserializer

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.1 GA
    • Fix Version/s: None
    • Component/s: Core
    • Environment:
      N/A

      Description

      Background

      I have created a mailing Gateway for my application which is configured as follows:

      Gateway -> Direct Channel -> Transformer -> Queue Channel (JDBCMessageStore) -> Outbound Mail Channel Adaptor

      This was working fine when the transformer was creating a SimpleMailMessage which is Serializable. The DefaultSerializer and DefaultDeserializer were able to serialize and then deserialize the entire GenericMessage<SimpleMailMessage>.

      However, in order to send HTML e-mail (a common requirement) I need to replace SimpleMailMessage with MimeMailMessage. This is not serializable. The answer is to replace the (de)serializer with a custom implementation.

      This custom implementation will populate a Serializable object with the properties we need from the MimeMailMessage along with the MessageHeaders. This can then be serialized as before. When we deserialize we need to create a new GenericMessage set the payload to be a new MimeMailMessage and restore the headers.

      This currently doesn't work because when we create a new GenericMessage specifying the MessageHeaders a new id gets generated. This means that the existing message in the database doesn't get removed after being picked up by the poller and the outbound mail adaptor keeps sending it.

      Fix

      The fix must be to allow the id to be specified if we need to. The code says:

      /**
      	 * The key for the Message ID. This is an automatically generated UUID and
      	 * should never be explicitly set in the header map <b>except</b> in the
      	 * case of Message deserialization where the serialized Message's generated
      	 * UUID is being restored.
      	 */
      

      This suggests that we can set it when we do a complete deserialization of the message and headers together (as done by the DefaultDeserializer therefore it seems reasonable that if custom deserializers can be supplied they should also be able to restore the id.

      I have cloned the Github repo and made the following code change to MessageHeaders:

      if (!this.headers.containsKey(ID)) {
      			if (MessageHeaders.idGenerator == null){
      				this.headers.put(ID, UUID.randomUUID());
      			}
      			else {
      				this.headers.put(ID, MessageHeaders.idGenerator.generateId());
      			}
      		}
      

      However this causes a test failure in MessageHeadersTests#testIdOverwritten so clearly it is considered important that the id cannot be set. As I said in my last reply on the forum that functionality seems to be at odds with allowing flexible custom (de)serializers.

        Activity

        Hide
        Alex Barnes added a comment -

        Thinking about this some more, you could have some second id which is set when deserializing:

        public static final String DESERIALIZED_ID = "deserialized_id";

        Which you could use in the MessageHeaders constructor:

        if (this.headers.containsKey(DESERIALIZED_ID)) {
        			this.headers.put(ID, this.headers.get(DESERIALIZED_ID));
        		} else {
        			if (MessageHeaders.idGenerator == null){
        				this.headers.put(ID, UUID.randomUUID());
        			}
        			else {
        				this.headers.put(ID, MessageHeaders.idGenerator.generateId());
        			}
        		}
        

        This doesn't interfere with the existing behaviour (all existing tests pass) and would still allow restoration of the complete message headers when deserializing the headers.

        Show
        Alex Barnes added a comment - Thinking about this some more, you could have some second id which is set when deserializing: public static final String DESERIALIZED_ID = "deserialized_id" ; Which you could use in the MessageHeaders constructor: if ( this .headers.containsKey(DESERIALIZED_ID)) { this .headers.put(ID, this .headers.get(DESERIALIZED_ID)); } else { if (MessageHeaders.idGenerator == null ){ this .headers.put(ID, UUID.randomUUID()); } else { this .headers.put(ID, MessageHeaders.idGenerator.generateId()); } } This doesn't interfere with the existing behaviour (all existing tests pass) and would still allow restoration of the complete message headers when deserializing the headers.
        Hide
        Artem Bilan added a comment -

        I think we just should have in the JDBCMessageStore inside getMessage() something like:
        AbstractKeyValueMessageStore#normalizeMessage
        The sample of its usage is in AbstractKeyValueMessageStore#getMessage

        And that's all: not many other changes especially in the immutable objects as MessageHeaders.

        @Alex, now you can use that solution inside your deserializer, but as temporal decision.

        Show
        Artem Bilan added a comment - I think we just should have in the JDBCMessageStore inside getMessage() something like: AbstractKeyValueMessageStore#normalizeMessage The sample of its usage is in AbstractKeyValueMessageStore#getMessage And that's all: not many other changes especially in the immutable objects as MessageHeaders . @Alex, now you can use that solution inside your deserializer, but as temporal decision.
        Hide
        Alex Barnes added a comment -

        Artem,

        Thanks for your reply. I have just put this code:

        Map innerMap = (Map) new DirectFieldAccessor(normalizedMessage.getHeaders()).getPropertyValue("headers");
        		innerMap.put(MessageHeaders.ID, message.getHeaders().getId());
        		innerMap.put(MessageHeaders.TIMESTAMP, message.getHeaders().getTimestamp());
        

        in my deserializer and the message is picked up by the poller removed from the database correctly. This is a perfectly good solution for the time being.

        I understand your concerns with regards to changing immutable classes, however my changes are in the constructor and so don't affect the immutability.

        All I'm suggesting is that there is a bit of a inconsistency here:

        • You've stated that the ID may be specified when deserializing a complete message using the DefaultDeserializer
        • You've also stated that it is possible to implement Deserializer and provide a custom implementation. However, that implementation doesn't have the ability to set the ID.
        Show
        Alex Barnes added a comment - Artem, Thanks for your reply. I have just put this code: Map innerMap = (Map) new DirectFieldAccessor(normalizedMessage.getHeaders()).getPropertyValue( "headers" ); innerMap.put(MessageHeaders.ID, message.getHeaders().getId()); innerMap.put(MessageHeaders.TIMESTAMP, message.getHeaders().getTimestamp()); in my deserializer and the message is picked up by the poller removed from the database correctly. This is a perfectly good solution for the time being. I understand your concerns with regards to changing immutable classes, however my changes are in the constructor and so don't affect the immutability. All I'm suggesting is that there is a bit of a inconsistency here: You've stated that the ID may be specified when deserializing a complete message using the DefaultDeserializer You've also stated that it is possible to implement Deserializer and provide a custom implementation. However, that implementation doesn't have the ability to set the ID.
        Hide
        Artem Bilan added a comment -

        You've stated that the ID may be specified when deserializing a complete message using the DefaultDeserializer

        Where have you seen it? Deserializer strategy takes InputStream and provides some Object. In this case DefaultDeserializer just makes objectInputStream.readObject() there is no any word about MessageHeaders.ID. Here it's enough: returned Object will be full correct Message<?> with MessageHeaders.ID, which was in the Message before serialization.

        You've also stated that it is possible to implement Deserializer and provide a custom implementation. However, that implementation doesn't have the ability to set the ID.

        That's why I suggest to modify JDBCMessageStore#getMessage() with normalization Message logic after deserialization.

        Show
        Artem Bilan added a comment - You've stated that the ID may be specified when deserializing a complete message using the DefaultDeserializer Where have you seen it? Deserializer strategy takes InputStream and provides some Object. In this case DefaultDeserializer just makes objectInputStream.readObject() there is no any word about MessageHeaders.ID . Here it's enough: returned Object will be full correct Message<?> with MessageHeaders.ID, which was in the Message before serialization. You've also stated that it is possible to implement Deserializer and provide a custom implementation. However, that implementation doesn't have the ability to set the ID. That's why I suggest to modify JDBCMessageStore#getMessage() with normalization Message logic after deserialization.
        Hide
        Oleg Zhurakousky added a comment -

        Artem

        I have not had a chance to look at it in details (on holidays), but from the first glimpse here is what I see

        Although you are right where there is nothing in the default deserialization about setting Message IDs we do use Reflection to reset the ID (sort of an internal detail). Just look at MongoDbMessageStore (line 369). I think the point is that its okay to use a reflection to reset ID for he cases like this, but I doubt its worth to expose some strategy for that since we are talking about very uniques case where the Message is stored only for the purpose of resilience (backup) and not as a result of some processing which means it is still the same message in all that it might mean and therefore must be deserialized as such.

        Show
        Oleg Zhurakousky added a comment - Artem I have not had a chance to look at it in details (on holidays), but from the first glimpse here is what I see Although you are right where there is nothing in the default deserialization about setting Message IDs we do use Reflection to reset the ID (sort of an internal detail). Just look at MongoDbMessageStore (line 369). I think the point is that its okay to use a reflection to reset ID for he cases like this, but I doubt its worth to expose some strategy for that since we are talking about very uniques case where the Message is stored only for the purpose of resilience (backup) and not as a result of some processing which means it is still the same message in all that it might mean and therefore must be deserialized as such.
        Hide
        Artem Bilan added a comment -

        Oleg

        Not a question!
        But as you see on discussion here, custom Deserializer implementation provides some new Message which ID isn't the same as JDBCMessageStore#getMessage() UUID id parameter.
        As I understand Alex uses MessageBuilder in his Deserializer implementation.
        I don't suggest to expose some strategy I've just shown some similar solution.

        Show
        Artem Bilan added a comment - Oleg Not a question! But as you see on discussion here, custom Deserializer implementation provides some new Message which ID isn't the same as JDBCMessageStore#getMessage() UUID id parameter. As I understand Alex uses MessageBuilder in his Deserializer implementation. I don't suggest to expose some strategy I've just shown some similar solution.
        Hide
        Alex Barnes added a comment -

        Both,

        Thanks for your reply. I have a feeling that I am not explaining myself very well!

        Artem -


        Where have you seen it? Deserializer strategy takes InputStream and provides some Object. In this case DefaultDeserializer just makes objectInputStream.readObject() there is no any word about MessageHeaders.ID.

        You're right that there is no explicit mention of the message id in the DefaultDeserializer. The comment I refer to is on the ID field on MessageHeaders itself. It says that you should never set the id except when Deserializing. Since I am deserializing I believe that I should be able to set the id!

        As you say, if you're deserializing the complete message (payload and headers) you can use reflection to set the id (an implementation detail as Oleg says). However, if you're supplying a custom deserializer it's probably because your payload isn't serializable (in my case a MimeMailMessage so you have to pick the bits of the payload you wish to serialize along with the headers. Then when you come to deserialize you have to create a new message - restoring the bits of the payload you were able to serialize along with the complete headers.

        Anyway, it sound like you have an approach in mind for this. So I'm happy to let this go.

        Show
        Alex Barnes added a comment - Both, Thanks for your reply. I have a feeling that I am not explaining myself very well! Artem - Where have you seen it? Deserializer strategy takes InputStream and provides some Object. In this case DefaultDeserializer just makes objectInputStream.readObject() there is no any word about MessageHeaders.ID. You're right that there is no explicit mention of the message id in the DefaultDeserializer . The comment I refer to is on the ID field on MessageHeaders itself. It says that you should never set the id except when Deserializing. Since I am deserializing I believe that I should be able to set the id! As you say, if you're deserializing the complete message (payload and headers) you can use reflection to set the id (an implementation detail as Oleg says). However, if you're supplying a custom deserializer it's probably because your payload isn't serializable (in my case a MimeMailMessage so you have to pick the bits of the payload you wish to serialize along with the headers. Then when you come to deserialize you have to create a new message - restoring the bits of the payload you were able to serialize along with the complete headers. Anyway, it sound like you have an approach in mind for this. So I'm happy to let this go.

          People

          • Assignee:
            Mark Fisher
            Reporter:
            Alex Barnes
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated: