-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6816/#review10902
-----------------------------------------------------------


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.

- Robbie Gemmell


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
> 
>

Reply via email to