You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by rajith attapattu <ra...@gmail.com> on 2012/08/29 04:34:33 UTC

Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

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

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


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6816/#review11830
-----------------------------------------------------------


Is this missing the added files?

- Gordon Sim


On Sept. 21, 2012, 3:52 p.m., rajith attapattu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6816/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2012, 3:52 p.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/AMQConnection.java 1388554 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1388554 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/JMSStreamMessage.java 1388554 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/MessageFactoryRegistry.java 1388554 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/ConnectionURL.java 1388554 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/Session.java 1388554 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java 1388554 
> 
> Diff: https://reviews.apache.org/r/6816/diff/
> 
> 
> Testing
> -------
> 
> AMQPEncodedListMessageTest is added to cover several cases.
> 
> 
> Thanks,
> 
> rajith attapattu
> 
>


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by rajith attapattu <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6816/
-----------------------------------------------------------

(Updated Sept. 28, 2012, 5:15 p.m.)


Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Keith Wall.


Changes
-------

Made the following changes.
1. Switched the default encoding to legacy format.
2. It now allows elements of different types.
3. Added several unit tests in favour of a system test.


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 (updated)
-----

  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1391510 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1391510 
  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/JMSStreamMessage.java 1391510 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/MessageFactoryRegistry.java 1391510 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/ConnectionURL.java 1391510 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/ListMessage.java PRE-CREATION 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/Session.java 1391510 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/AMQConnectionUnitTest.java 1391510 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/AMQSession_0_10Test.java 1391510 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java 1391510 

Diff: https://reviews.apache.org/r/6816/diff/


Testing
-------

AMQPEncodedListMessageTest is added to cover several cases.


Thanks,

rajith attapattu


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by rajith attapattu <ra...@gmail.com>.

> On Sept. 25, 2012, 9:38 a.m., Robbie Gemmell wrote:
> > "1. Improved error handling code. Added a check to verify that all elements in the list are of the same type."
> > 
> > Why restrict the elements to a single type? I don't recall JMS placing such a restriction on Map or Stream messages, so why would we?
> > 
> > "2. Stream messages are now encoded as an AMQP list. The legacy behaviour can be restored using a connection arg or a jvm arg, similar to the way we handle map messages."
> > 
> > If the original patch didn't do this, why does this one need to do so? This seems like a separate issue, and something that should be discussed on the dev list before implementation. For example, it may make sense to ship the client with the new format support but the default set the other way, and then flip it in a future release, such that support is present before it becomes the default and people don't need to upgrade all their clients at the same time in order to actually use it, and in the meantime anyone who does want to do so can use the system property.
> > 
> > "3. Add tests to cover the List, Stream and Map interface as well as NestedLists."
> > 
> > I don't see any tests at all in the latest diff, and I know from our email discussion that they still aren't unit tests. This code should be amongst the easier bits of the client to unit test, and it needs unit testing far more than it needs system tests.

1.Gordon also had the same comment about the restriction. I will be removing this.

2. The original patch required the application to use a non standard interface if it needed list message support.
   This approach (just like map messaging) will allow the user to encode a message as amqp/list while using the standard Stream Message interface.

   However I agree with you about switching the default, allowing time for people to make a more manageable upgrade. We could look to switch it around for 0.22 release.

3. I missed the Test class from my diff. I will have to rework the test anyways to accomadate #1. I will also add unit tests.


- rajith


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


On Sept. 24, 2012, 2:24 p.m., rajith attapattu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6816/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2012, 2:24 p.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/AMQConnection.java 1389396 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1389396 
>   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/JMSStreamMessage.java 1389396 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/MessageFactoryRegistry.java 1389396 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/ConnectionURL.java 1389396 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/ListMessage.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/Session.java 1389396 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java 1389396 
> 
> Diff: https://reviews.apache.org/r/6816/diff/
> 
> 
> Testing
> -------
> 
> AMQPEncodedListMessageTest is added to cover several cases.
> 
> 
> Thanks,
> 
> rajith attapattu
> 
>


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by Robbie Gemmell <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6816/#review11884
-----------------------------------------------------------


"1. Improved error handling code. Added a check to verify that all elements in the list are of the same type."

Why restrict the elements to a single type? I don't recall JMS placing such a restriction on Map or Stream messages, so why would we?

"2. Stream messages are now encoded as an AMQP list. The legacy behaviour can be restored using a connection arg or a jvm arg, similar to the way we handle map messages."

If the original patch didn't do this, why does this one need to do so? This seems like a separate issue, and something that should be discussed on the dev list before implementation. For example, it may make sense to ship the client with the new format support but the default set the other way, and then flip it in a future release, such that support is present before it becomes the default and people don't need to upgrade all their clients at the same time in order to actually use it, and in the meantime anyone who does want to do so can use the system property.

"3. Add tests to cover the List, Stream and Map interface as well as NestedLists."

I don't see any tests at all in the latest diff, and I know from our email discussion that they still aren't unit tests. This code should be amongst the easier bits of the client to unit test, and it needs unit testing far more than it needs system tests.

- Robbie Gemmell


On Sept. 24, 2012, 2:24 p.m., rajith attapattu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6816/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2012, 2:24 p.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/AMQConnection.java 1389396 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1389396 
>   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/JMSStreamMessage.java 1389396 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/MessageFactoryRegistry.java 1389396 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/ConnectionURL.java 1389396 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/ListMessage.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/Session.java 1389396 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java 1389396 
> 
> Diff: https://reviews.apache.org/r/6816/diff/
> 
> 
> Testing
> -------
> 
> AMQPEncodedListMessageTest is added to cover several cases.
> 
> 
> Thanks,
> 
> rajith attapattu
> 
>


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by rajith attapattu <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6816/
-----------------------------------------------------------

(Updated Sept. 24, 2012, 2:24 p.m.)


Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Keith Wall.


Changes
-------

Added the missing files. Thx Gordon for pointing it out.


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 (updated)
-----

  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1389396 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1389396 
  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/JMSStreamMessage.java 1389396 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/MessageFactoryRegistry.java 1389396 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/ConnectionURL.java 1389396 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/ListMessage.java PRE-CREATION 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/Session.java 1389396 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java 1389396 

Diff: https://reviews.apache.org/r/6816/diff/


Testing
-------

AMQPEncodedListMessageTest is added to cover several cases.


Thanks,

rajith attapattu


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by rajith attapattu <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6816/
-----------------------------------------------------------

(Updated Sept. 21, 2012, 3:52 p.m.)


Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and Keith Wall.


Changes
-------

Submitting the original patch provided by Siddesh with the following modifications.

1. Improved error handling code. Added a check to verify that all elements in the list are of the same type.
2. Stream messages are now encoded as an AMQP list. The legacy behaviour can be restored using a connection arg or a jvm arg, similar to the way we handle map messages.
3. Add tests to cover the List, Stream and Map interface as well as NestedLists.


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 (updated)
-----

  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java 1388554 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1388554 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/JMSStreamMessage.java 1388554 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/MessageFactoryRegistry.java 1388554 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/ConnectionURL.java 1388554 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/Session.java 1388554 
  http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java 1388554 

Diff: https://reviews.apache.org/r/6816/diff/


Testing
-------

AMQPEncodedListMessageTest is added to cover several cases.


Thanks,

rajith attapattu


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by rajith attapattu <ra...@gmail.com>.

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

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.


- rajith


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


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by Robbie Gemmell <ro...@gmail.com>.

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

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)


- Robbie


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


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by Gordon Sim <gs...@redhat.com>.

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


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by Robbie Gemmell <ro...@gmail.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by rajith attapattu <ra...@gmail.com>.

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

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. 


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


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by rajith attapattu <ra...@gmail.com>.

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


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by Gordon Sim <gs...@redhat.com>.

> 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.
> 
> rajith attapattu wrote:
>     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.

Applications are written to work with particular interfaces to the message, yes. However that is a different thing from the form in which the message was encoded. I think a question to ask is whether a standard JMS application should be able to receive a message encoded as an AMQP list when using the Qpid JMS library. If so, then coding that application to StreamMessage is one option. That might work well for sending simple 'structs', sequential primitive values. Another option is to use the MapMessage interface, treating the values as the items. This need not be as ugly as you fear, e.g.

  if (m instanceof MapMessage) {
    MapMessage map = (MapMessage) m;
    for (String key : map.getMapNames()) {
      Object value = map.getObject(key);
      //process value
    } 
  }

An application like that could potentially be ported to other JMS implementations (though the underlying encoding in which the messages were actually sent would likely differ).

For code that can be Qpid specific, I don't see why a ListMessage interface is any worse than special keys, nor do I see that the lifecycle of the client is that relevant. The question is whether the 'interface' (whether an actual java type or a convention for a special key and property) will continue to be supported. The main use case I'm aware of for list messages at present is QMFv2. If we want to make it an explicitly supported thing, I think a ListMessage type is perfectly reasonable. If we want it simply to be a workaround only, then that perhaps changes things. Might be worth a mail to the user list as well soliciting opinons.


- Gordon


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


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by Gordon Sim <gs...@redhat.com>.

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

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.


- Gordon


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


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by rajith attapattu <ra...@gmail.com>.

> 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(?).
> > 
> >

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


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


Re: Review Request: Implement a ListMessage interface to accept amqp/list messages as java.util.List

Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6816/#review10835
-----------------------------------------------------------


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(?).



- Gordon Sim


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