----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/937/#review876 -----------------------------------------------------------
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java <https://reviews.apache.org/r/937/#comment1910> This doesn't seem to distinguish between link- and node- level bindings(?). http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java <https://reviews.apache.org/r/937/#comment1909> If it will remove only one binding why use an array to collect old bindings? Why not just locate the binding to be removed and remove it? It's not clear from this explanation why there would only be one binding however. In fact I don't really understand what is going on in this bit of code at all. Is this trying to remove the default binding added above in line 1329 of AMQSession_0_10? http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/AddressHelper.java <https://reviews.apache.org/r/937/#comment1908> Again, does this not mean that you can't distinguish between list and node bindings? - Gordon On 2011-06-20 17:29:56, rajith attapattu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/937/ > ----------------------------------------------------------- > > (Updated 2011-06-20 17:29:56) > > > Review request for qpid. > > > Summary > ------- > > The patch makes the following changes > > 1. AMQSession_0_10.java > A default binding is only added if there are no explicit bindings > specified via x-bindings > > 2. BasicMessageConsumer_0_10.java > When the same destination (Topic) is used to create two different > consumers the code creates a copy of the destination to ensure the second > consumer gets it's own unique temp queue. When doing so we need to ensure we > delete any bindings for the previous temp queue. If we don't remove old > bindings and if there were no explicit bindings specified via x-bindings, > then the second consumers queue will not be bound due to the logic mentioned > in [1]. (Bcos the previous binding is treated as explicit bindings). > > 3. AddressHelper.java > The second part of the JIRA covers a different bug - i.e x-binding > specified in the link properties are not used if the node type if a queue. > I added code to read the x-bindings in link props if there are no > x-bindings specified in the node props. > > 4. Modified test cases > 1. To ensure that a default binding is not added when explicit bindings > are specified. > 2. To fix an existing test case that relied on a default binding even > when x-bindings is specified. > (*) I still need to add (or modify an existing test case) to cover the case > where x-bindings are specified in link props when the node type if a queue. > > > This addresses bug QPID-3265. > https://issues.apache.org/jira/browse/QPID-3265 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java > 1137691 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java > 1137691 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/messaging/address/AddressHelper.java > 1137691 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/client/destination/AddressBasedDestinationTest.java > 1137691 > > Diff: https://reviews.apache.org/r/937/diff > > > Testing > ------- > > The use cases specified in the JIRA were manually tested in addition to the > above mentioned automated test cases. > > > Thanks, > > rajith > >
