> On Aug. 29, 2012, 7:01 a.m., Gordon Sim wrote:
> > I quite liked the approach taken in Siddhesh's patch (as attached to the
> > JIRAmany months ago now), where a list message can be processed by the
> > receiver in a variety of ways: (i) the elements of the list are accessible
> > through a MapMessage interface using their indices as the keys, (ii) the
> > message can be treated as a StreamMessage, (iii) a non-standard ListMessage
> > interface is introduced for those who prefer something a little more
> > explicit and don't mind the non-standard interface.
> >
> > For the receiver side these approaches seem a little more natural to me. I
> > would argue the first two are actually closer to the spirit of working
> > within the standard interfaces. I don't think the third option is
> > necessarily any less standard than a special key & property either.
> >
> > The view from the sender side is perhaps a little different however. I
> > *think* Siddhesh's patch requires list messages to be sent as the
> > non-standard ListMessage implementation for them to be actually encoded as
> > an AMQP list. It would be nice to be able to use the more standard
> > interfaces as the receiver can, even if just for the symmetry. The trick
> > there would be determining how to encode the message in the AMQP type
> > system. A message property seems like the 'best' option there. Perhaps it
> > could be more directly tied to content-type though. Do we expose that at
> > all through a custom proprty key in JMS?
> >
> > It should certainly also be possible to add a List object as the value to a
> > particular key in a MapMessage (non-standard JMS behaviour of course, so
> > perhaps configurable) which your patch does and his may not(?).
> >
> >
>
> rajith attapattu wrote:
> I did look at the patch very closely and debated several options before
> settling down with the one I proposed. Here's why,
>
> I was looking at using either a StreamMessage or as a special case of Map
> Message on both ends (for symmetry).
> I chose MapMessage as you can easily do getObject() to retrieve a
> java.util.List and from then onwards the application gets to work with the
> List interface which is the most natural one for java developers.
> With StreamMessage it's not very intuitive or natural.
>
> Here's why I disliked the existing patch
> ========================================
> (1) I really dislike introducing a Qpid specific interface and I don't
> believe it's the same as using a special key & message property. The latter
> certainly gives more flexibility to the application and us as the vendor.
>
> (2) I think it's more natural to allow the application deal with a
> java.util.List than to allow access to elements using
> getObject("index-as-a-string").
> With the latter we will then need to handle array-index-out-bounds,
> typos in the indicies, non numeric keys etc.. where's we don't need to worry
> with a List.
>
> (3) Further to the above point, Java has syntatic sugar (for each ..etc)
> for List processing which will not be possible with the above approach.
>
> (3) Multiple interfaces to access the same message content is error prone
> and confusing IMO.
>
>
> rajith attapattu wrote:
> The JMS Message interface does not expose the content-type nor do we
> expose it via a message header. In our implementation content-types are fixed
> for different JMS Message types. We use the content types to figure out which
> JMS message type to use. We don't have a way of letting an application to
> dynamically map content-types to various message types. Ex text/xml to be
> treated as a TextMessage.
> (I added this as a feature in my experimental Qpid API implementation
> over the C++ client and will likely purpuse this when we do the new JMS
> client).
>
> The current MapMessage implementation already allows Nested lists and
> Maps as entries (in addition UUIDs). So currently you can easily shuffle
> Lists across using JMS.
> What we didn't have is a way to interpret messages which it's payload was
> encoded as a List and marked with content-type "amqp/list".
>
> Currently by default (can be changed via a system property) all Map
> Messages are encoded using AMQP type system instead of our previously used
> proprietary encoding.
>
> Gordon Sim wrote:
> I think the last point in your first comment above is where we first
> diverge. I don't see any problem allowing different ways to access the data.
> I think that allowing an application that expects to receive MapMessages to
> continue to work reasonably even when the message was encoded as a list is a
> reasonable option. Likewise for an application that expects to receive a
> StreamMessage.
>
> I do take your point that getting an actual java.util.List is sometimes
> preferable. I don't see that as mutually exclusive with the above options
> however. As for the for-each support, the ListMessage can be Iteratable (it
> already has an iterator() method on it in the patch). Likewise when treating
> the received message as a MapMessage you can use for-each with the keys.
>
> I think your approach also introduces a "Qpid specific 'interface'" even
> if not an actual interface in the Java sense. Compare:
>
> if (m instanceof MapMessage && m.getBooleanProperty("encode-as-list")) {
> for (Object o : (List) ((MapMessage) m).getObject("amqp/list")) {
> //process element
> }
> }
>
> with:
>
> if (m instanceof ListMessage) {
> for (Object o : (ListMessage) m) {
> //process element
> }
> }
>
> I don't think the first is any more sympathetic to the standard or any
> more likely to be portable. I think the second is actually clearer. Further,
> with Siddhesh's patch you can stick to standard JMS by using the
> StreamMessage, or even the MapMessage with the slight downside (for some
> cases) that the keys are entirely artificial.
>
> With regard to the second comment, I would think exposing the content
> type via a custom property would be more generally useful than a special
> 'encode-as-list' option.
>
> I think I still favour Siddhesh's approach myself but will let others
> have a say now.
Appreciate your comments and feedback.
Frankly most applications know what type of message they are expecting. So I'm
not entirely convinced the need to have list message support via Map and Stream
Message.
We should probably choose one and stick with it.
(*) I certainly dislike introducing an interface at this time in the life cycle
of the client.
(*) IMO msg.getObject("amqp/list") is cleaner than msg.getObject("1"),
msg.getObject(2) ..etc
The latter introduces a whole lot of headaches of managing a list in
addition to not being iterable.
I've already highlighted in detail as to why I disagree with the existing patch.
But if the majority feels that it's the right approach I certainly will accept
that.
- rajith
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6816/#review10835
-----------------------------------------------------------
On Aug. 29, 2012, 2:34 a.m., rajith attapattu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6816/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2012, 2:34 a.m.)
>
>
> Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Keith
> Wall.
>
>
> Description
> -------
>
> We have a need to support amqp encoded list messages that are understood
> across all language clients. In JMS this is a bit difficult as there is no
> direct list message interface like the map message.
> I wanted to introduce a way of getting list message support while using only
> JMS interfaces.
>
> The list message support is implemented as a special case of Map message,
> where the only entry is a list (it can be nested lists as well).
> The application notifies the client of their intention via a message header.
> MapMessage m = _session.createMapMessage();
> m.setBooleanProperty("encode-as-list", true);
> m.setObject("amqp/list", myList);
> _producer.send(m);
>
> The resulting message body will be encoded as an AMQP 0-10 list. A C++ or
> python client can then directly read it as a list message.
>
> On the receiving side, it will be presented as a MapMessage with a single
> entry retrieved using "amqp/list".
> List list = (List)m.getObject("amqp/list");
>
> Please feel free to provide feedback, including alternative ways of
> implementing this.
> All feedback are most welcomed!
>
>
> This addresses bug QPID-3906.
> https://issues.apache.org/jira/browse/QPID-3906
>
>
> Diffs
> -----
>
>
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQPEncodedListMessage.java
> PRE-CREATION
>
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQPEncodedListMessageFactory.java
> PRE-CREATION
>
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQPEncodedMapMessage.java
> 1378417
>
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AbstractJMSMessage.java
> 1378417
>
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/MessageFactoryRegistry.java
> 1378417
>
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/message/AMQPEncodedListMessageTest.java
> PRE-CREATION
>
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/message/AMQPEncodedMapMessageTest.java
> 1378417
>
> Diff: https://reviews.apache.org/r/6816/diff/
>
>
> Testing
> -------
>
> AMQPEncodedListMessageTest is added to cover several cases.
>
>
> Thanks,
>
> rajith attapattu
>
>
