Qpid
  1. Qpid
  2. QPID-2363

JMS Map Message Should Support AMQP 0-10 Encoded Map As Message Body

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.7
    • Labels:
      None

      Description

      As part of the new API initiative in Qpid, the C++ client and python client have added support for a map message, where the content is a map of key,value pairs encoded using the AMQP 0-10 encoding for Maps.
      Currently the JMS Map message body is encoded as an opaque binary blob.
      We need to add support for the JMS Map message implementation to set an AMQP 0-10 encoded Map for the message body.
      This allows inteoperability btw JMS Map messages and c++, python map messages.
      The content-type for the proposed map message is "amqp/map".

      This feature is only added to the 0-10 code path as the versions below AMQP 0-10 does not have any native support for encoding maps.
      The new map message format will be the default for any map message created by a 0-10 Session.
      If it needs to send any messages using the old map message format it will need to use the following jvm arg "-Dqpid.use_legacy_map_message=true"
      Once the addressing support is finalized we could also add a per destination flag to indicate if legacy messages needs to be used or not.

      1. QPID-2363.patch
        17 kB
        Rajith Attapattu

        Activity

        Hide
        Rob Godfrey added a comment -

        I presume we are going to use a message header, or different mime type for the new header format...
        that way the receiving client can determine whether it is an old style or new style map message without the application programmer having to know up front what type they should expect to receive. I'm happy with the default format on publish to be different between the 0-8 and 0-10 however I think we should find a better way than a JVM system property for specifying this - i.e. either per destination or per connection. I would actually -1 the idea of a system property as this seems entirely the wrong granularity for this option

        Show
        Rob Godfrey added a comment - I presume we are going to use a message header, or different mime type for the new header format... that way the receiving client can determine whether it is an old style or new style map message without the application programmer having to know up front what type they should expect to receive. I'm happy with the default format on publish to be different between the 0-8 and 0-10 however I think we should find a better way than a JVM system property for specifying this - i.e. either per destination or per connection. I would actually -1 the idea of a system property as this seems entirely the wrong granularity for this option
        Hide
        Rajith Attapattu added a comment -

        Attached is a patch that implements the AMQP 0-10 encoded Map message.
        This also contains a test case and I have also tested against the C++ client and python client

        Show
        Rajith Attapattu added a comment - Attached is a patch that implements the AMQP 0-10 encoded Map message. This also contains a test case and I have also tested against the C++ client and python client
        Hide
        Rajith Attapattu added a comment -

        I presume we are going to use a message header, or different mime type for the new header format...
        that way the receiving client can determine whether it is an old style or new style map message without the application programmer having to know up front what type they should expect to receive.

        • Yes this is exactly what I have done. The new mime type is "amqp/map"

        I'm happy with the default format on publish to be different between the 0-8 and 0-10 however I think we should find a better way than a JVM system property for specifying this - i.e. either per destination or per connection. I would actually -1 the idea of a system property as this seems entirely the wrong granularity for this option.
        For simplicity I have just added a system prop until I am able to add support at the destination level . It would be pretty easy to make this a connection parameter also.
        My preferred solution is destination level and I am hoping to add that as soon as I get the new addressing patch out.

        Please be kind enough to review the patch and provide feedback.

        Show
        Rajith Attapattu added a comment - I presume we are going to use a message header, or different mime type for the new header format... that way the receiving client can determine whether it is an old style or new style map message without the application programmer having to know up front what type they should expect to receive. Yes this is exactly what I have done. The new mime type is "amqp/map" I'm happy with the default format on publish to be different between the 0-8 and 0-10 however I think we should find a better way than a JVM system property for specifying this - i.e. either per destination or per connection. I would actually -1 the idea of a system property as this seems entirely the wrong granularity for this option. For simplicity I have just added a system prop until I am able to add support at the destination level . It would be pretty easy to make this a connection parameter also. My preferred solution is destination level and I am hoping to add that as soon as I get the new addressing patch out. Please be kind enough to review the patch and provide feedback.
        Hide
        Rob Godfrey added a comment -

        Had a quick look at the patch...

        My other comments would be

        Why not allow the 0-8 codepath to decode amqp maps (or even to encode them - though I would suggest not as the default)... the code is there and for interop reasons it might be useful if you could send an 0-10 map message via the java broker to a client working in 0-8 mode...

        I don't really like the encode / decode on the message delegate... seems like it could just be on the AMQPEncodedMapMessage as the implementation of populateMapFromData / writeMapToData (this sort of links with the above)

        Show
        Rob Godfrey added a comment - Had a quick look at the patch... My other comments would be Why not allow the 0-8 codepath to decode amqp maps (or even to encode them - though I would suggest not as the default)... the code is there and for interop reasons it might be useful if you could send an 0-10 map message via the java broker to a client working in 0-8 mode... I don't really like the encode / decode on the message delegate... seems like it could just be on the AMQPEncodedMapMessage as the implementation of populateMapFromData / writeMapToData (this sort of links with the above)
        Hide
        Rajith Attapattu added a comment -

        For the 1st iteration for testing I did have the encode and decode methods in the AMQPEncodedMapMessage class.
        Later on I changed it for the following reasons.

        1. I wasn't really sure if there was sufficient interest for the new format for apps using the 0-8 client.

        2. Since all message implementations were version agnostic, I thought I'd preserve that pattern. Therefore I used version specific delegates.

        However I am not opposed to reverting to the previous version if you think there is enough interest in using this in the 0-8 client.

        Show
        Rajith Attapattu added a comment - For the 1st iteration for testing I did have the encode and decode methods in the AMQPEncodedMapMessage class. Later on I changed it for the following reasons. 1. I wasn't really sure if there was sufficient interest for the new format for apps using the 0-8 client. 2. Since all message implementations were version agnostic, I thought I'd preserve that pattern. Therefore I used version specific delegates. However I am not opposed to reverting to the previous version if you think there is enough interest in using this in the 0-8 client.
        Hide
        Rob Godfrey added a comment -

        I think its sort of strange to deliberately stop 0-8 from understanding it...

        You don't need to understand all of 0-10 to decode the map message - you just need to understand the encoding used (which happens to be the 0-10 encoding of maps in this case)

        I would much rather not have the 0-8 client fall over if it receives such a message, and instead just have it decode it.

        On a separate point... are we locked into using "amqp/map" as the MIME type (i.e. is that what other clients are already using). It would make more sense to call it "amqp-0-10/map" as AMQP 1-0 will no doubt have a different map encoding...

        Show
        Rob Godfrey added a comment - I think its sort of strange to deliberately stop 0-8 from understanding it... You don't need to understand all of 0-10 to decode the map message - you just need to understand the encoding used (which happens to be the 0-10 encoding of maps in this case) I would much rather not have the 0-8 client fall over if it receives such a message, and instead just have it decode it. On a separate point... are we locked into using "amqp/map" as the MIME type (i.e. is that what other clients are already using). It would make more sense to call it "amqp-0-10/map" as AMQP 1-0 will no doubt have a different map encoding...
        Hide
        Rajith Attapattu added a comment -

        The "amqp/map" MIME type is used by the other clients as well.
        Any change we do should be coordinated among the clients. Infact I have a JIRA for this QPID-2350.
        We should clearly document the mandatory content-types that should be supported by all clients.
        Also all clients should have a consistent mechanism for handling both the mandatory and optional content types .
        For ex. the JMS client has a few additional content-types.

        If there are no further objections/comments I will commit the patch with moving the encoding/decoding to the AMQPEncodedMapMessage class, thereby allowing support for both 0-8 and 0-10 code paths.

        Show
        Rajith Attapattu added a comment - The "amqp/map" MIME type is used by the other clients as well. Any change we do should be coordinated among the clients. Infact I have a JIRA for this QPID-2350 . We should clearly document the mandatory content-types that should be supported by all clients. Also all clients should have a consistent mechanism for handling both the mandatory and optional content types . For ex. the JMS client has a few additional content-types. If there are no further objections/comments I will commit the patch with moving the encoding/decoding to the AMQPEncodedMapMessage class, thereby allowing support for both 0-8 and 0-10 code paths.
        Hide
        Rajith Attapattu added a comment -

        My bad. It should be QPID-2226, not QPID-2350

        Show
        Rajith Attapattu added a comment - My bad. It should be QPID-2226 , not QPID-2350
        Hide
        Rafael H. Schloming added a comment -

        I haven't yet taken a detailed look at the patch, and I hope to do so shortly. On the specific issue of the mime type, I agree it makes sense to somehow indicate that it is the 0-10 encoding somewhere. I'm also wondering if there should be some indication that this is a qpid thing in there as we can't really officially make up amqp mime types. Also there has been discussion of including additional information via extra key, which would really constitute a layered encoding on top of an 0-10 map.

        Show
        Rafael H. Schloming added a comment - I haven't yet taken a detailed look at the patch, and I hope to do so shortly. On the specific issue of the mime type, I agree it makes sense to somehow indicate that it is the 0-10 encoding somewhere. I'm also wondering if there should be some indication that this is a qpid thing in there as we can't really officially make up amqp mime types. Also there has been discussion of including additional information via extra key, which would really constitute a layered encoding on top of an 0-10 map.
        Hide
        Rafael H. Schloming added a comment -

        Ok, I took a look at the patch, and I don't have anything to add that hasn't already been said. I suggest for the mime type issue we just make sure that QPID-2226 is a release blocker and deal with that in a separate commit.

        Show
        Rafael H. Schloming added a comment - Ok, I took a look at the patch, and I don't have anything to add that hasn't already been said. I suggest for the mime type issue we just make sure that QPID-2226 is a release blocker and deal with that in a separate commit.
        Hide
        Rajith Attapattu added a comment -

        Cool. I will commit the patch with the agreed modifications.

        Show
        Rajith Attapattu added a comment - Cool. I will commit the patch with the agreed modifications.
        Hide
        Rajith Attapattu added a comment -

        In rev 903911 I committed the above patch as is.
        In rev 903917, 903919, 903924, 903940 I added the modifications agreed in the discussion.

        Show
        Rajith Attapattu added a comment - In rev 903911 I committed the above patch as is. In rev 903917, 903919, 903924, 903940 I added the modifications agreed in the discussion.
        Hide
        Rajith Attapattu added a comment -

        Please note that the default for both 0-8 and 0-10 code paths are the new AMQP 0-10 encoded message format.

        Show
        Rajith Attapattu added a comment - Please note that the default for both 0-8 and 0-10 code paths are the new AMQP 0-10 encoded message format.

          People

          • Assignee:
            Rajith Attapattu
            Reporter:
            Rajith Attapattu
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development