> On Aug. 30, 2012, 6:20 p.m., Robbie Gemmell wrote:
> > I dont particularly like the getObject("amqp/list") call (discounting the
> > fact that MapMessages are only supposed to carry primitive values, an area
> > of the JMS spec that your posts above suggest we are already breaking [in
> > addition to preventing other vendors from resending them] if we support
> > nesting things in maps, and the abuse of the amqp namespace that presumably
> > is being carried over from our other clients). I agree with Gordon that
> > this call together with the magic 'encode as list' setter makes it is no
> > less Qpid-specific than introducing an interface would, people would still
> > have to edit code later if we changed how those things worked or they move
> > vendor. Thats a risk you take when using extensions though. Other vendors
> > (IBM, Tibco etc) often tend to use interfaces to introduce extensions to
> > the Session etc so I'm not sure we need to worry about being paticularly
> > out of company on that front if we did it, and as far as I can tell only a
> > very small subset of users will care about using this stuff anyway.
> >
> > Regardless of which approach wins out this patch seems to have an
> > abnormally small number of unit tests (i.e none), which is somewhat
> > disheartening.
>
> rajith attapattu wrote:
> Cool, it appears most favor the original patch. I will go with it.
> This patch does have tests, all though they are not unit tests. It does
> test for null, empty, with content and with nested lists etc.
>
> I will resubmit the original patch after some reworking.
>
> Robbie Gemmell wrote:
> I havent actually looked at the original patch, just this one and the
> discussion here / on list.
>
> I saw the system tests...like I said, no unit tests ;)
> (This stuff seems like it should be more easily unit tested than many
> areas of the client are, and I know we have unit tests for at least some of
> the other message classes...though we should have a lot more)
We do already allow nesting of maps. There is a note about that, pointing out
that it violates standard JMS rules, in the programming guide. Unfortunatley if
you want to allow AMQP encoded message bodies, you can't rule out nesting.
However I do agree that not requiring nesting is another benefit of Siddhesh's
patch.
All the clients use amqp/map, amqp/list etc as the content type for message
bodies encoded in the AMQP type system. There was some debate at the time about
including the protocol version more explicitly, but as I recall a preference
for the less noisy (but perhaps more ambiguous) unqualified form was used. It
is true that these content types are not explicitly defined in any AMQP
specification. However, other than the minor version ambiguity, I think they
are the most obvious and clear descriptions of the content. I don't see this as
particularly bad abuse of the AMQP namespace. Sadly, AMQP 1.0 forbids the
setting of a content type for amqp encoded data (at least if it is identified
as intended with a particular data code). Using 'amqp/list' as the special key
is perhaps a step higher on the abuse scale though.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6816/#review10902
-----------------------------------------------------------
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
>
>
