> On 2011-06-22 16:07:41, rajith attapattu wrote: > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java, > > line 134 > > <https://reviews.apache.org/r/937/diff/1/?file=21364#file21364line134> > > > > You mean the removal of the specific binding or the reason behind > > copying the destination, setting queue name to null and removing the > > binding for the old queue ? > > > > I assume you are looking for more context on what this is done in > > MessageConsumer? > > > > This is bcos if the same topic destination is used to create two > > different consumers we need to ensure they both maintain their own unique > > temp queue while retaining the rest of attributes (ex > > subject/bindings/x-subscribe props ..etc) the same.
Seems like there is a mixing up of state that is associated with the address and state associated with the link created from that address(?). More importantly though, if bindings are removed does that not alter the destination? Is it only the default binding that should be removed (since it will be added back in)? Is it possible that any other link could be removed (e.g. if the link name was specified)? If it is just the default binding that should be removed, could the code not be more explicit? - Gordon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/937/#review880 ----------------------------------------------------------- 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 > >
