[ https://issues.apache.org/jira/browse/QPID-4969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13696920#comment-13696920 ]
Chuck Rolke commented on QPID-4969: ----------------------------------- Gordon's patch (thanks for that!) still fails Fraser's case. I propose something to satisfy both camps: When a binding with a duplicate key and duplicate args comes up then just pass on it. Don't create a new binding nor complain just let the old binding continue to exist. When a binding with a duplicate key but different args comes up then it gets failed. This means that one binding can't specify 'any' and another specify 'all' but have both survive with the same key. This scheme gets Fraser's test case to pass but still fails this issue's test case. {noformat} Index: cpp/src/tests/ExchangeTest.cpp =================================================================== --- cpp/src/tests/ExchangeTest.cpp (revision 1498469) +++ cpp/src/tests/ExchangeTest.cpp (working copy) @@ -138,7 +138,7 @@ args3.setInt("b", 6); headers.bind(a, "", &args1); - headers.bind(a, "", &args3); + headers.bind(a, "other", &args3);//need to use different binding key to correctly identify second binding headers.bind(b, "", &args2); headers.bind(c, "", &args1); Index: cpp/src/qpid/broker/HeadersExchange.h =================================================================== --- cpp/src/qpid/broker/HeadersExchange.h (revision 1498469) +++ cpp/src/qpid/broker/HeadersExchange.h (working copy) @@ -48,7 +48,7 @@ const Queue::shared_ptr queue; const qpid::framing::FieldTable* args; MatchArgs(Queue::shared_ptr q, const qpid::framing::FieldTable* a); - bool operator()(BoundKey & bk); + bool operator()(const BoundKey & bk); }; struct MatchKey @@ -56,7 +56,7 @@ const Queue::shared_ptr queue; const std::string& key; MatchKey(Queue::shared_ptr q, const std::string& k); - bool operator()(BoundKey & bk); + bool operator()(const BoundKey & bk); }; struct FedUnbindModifier Index: cpp/src/qpid/broker/HeadersExchange.cpp =================================================================== --- cpp/src/qpid/broker/HeadersExchange.cpp (revision 1498469) +++ cpp/src/qpid/broker/HeadersExchange.cpp (working copy) @@ -189,7 +189,7 @@ // the non federation args in case a federated propagate is needed FieldTable extra_args; getNonFedArgs(args, extra_args); - + if (fedOp.empty() || fedOp == fedOpBind) { // x-match arg MUST be present for a bind call std::string x_match_value = getMatch(args); @@ -198,18 +198,31 @@ throw InternalErrorException(QPID_MSG("Invalid or missing x-match value binding to headers exchange. Must be a string [\"all\" or \"any\"]")); } + bool duplicateBinding(false); Bindings::ConstPtr p = bindings.snapshot(); if (p.get()) { + MatchArgs matchArgs(queue, &extra_args); + MatchKey matchKey(queue, bindingKey); for (std::vector<BoundKey>::const_iterator i = p->begin(); i != p->end(); ++i) { - if (queue == i->binding->queue && bindingKey == i->binding->key) { - throw InternalErrorException(QPID_MSG("Exchange: " << getName() - << ", binding key: " << bindingKey - << " Duplicate binding key not allowed." )); + if (matchKey(*i)) { + // bindings with same keys may be duplicates + if (matchArgs(*i)) { + // duplicate bindings (same key, args) are not added anew + duplicateBinding = true; + } else { + // bindings with same key but different args are not allowed + throw InternalErrorException(QPID_MSG("Exchange: " << getName() + << ", binding key: " << bindingKey + << " Duplicate binding key not allowed." )); + } + break; + } else { + // bindings with different keys are not duplicates } } } - { + if (!duplicateBinding) { Mutex::ScopedLock l(lock); //NOTE: do not include the fed op/tags/origin in the //arguments as when x-match is 'all' these would prevent @@ -372,7 +385,7 @@ //--------- HeadersExchange::MatchArgs::MatchArgs(Queue::shared_ptr q, const qpid::framing::FieldTable* a) : queue(q), args(a) {} -bool HeadersExchange::MatchArgs::operator()(BoundKey & bk) +bool HeadersExchange::MatchArgs::operator()(const BoundKey & bk) { return bk.binding->queue == queue && bk.binding->args == *args; } @@ -380,7 +393,7 @@ //--------- HeadersExchange::MatchKey::MatchKey(Queue::shared_ptr q, const std::string& k) : queue(q), key(k) {} -bool HeadersExchange::MatchKey::operator()(BoundKey & bk) +bool HeadersExchange::MatchKey::operator()(const BoundKey & bk) { return bk.binding->queue == queue && bk.binding->key == key; } {noformat} > C++ Broker headers exchange allows creation of bindings with duplicate keys > --------------------------------------------------------------------------- > > Key: QPID-4969 > URL: https://issues.apache.org/jira/browse/QPID-4969 > Project: Qpid > Issue Type: Bug > Components: C++ Broker > Affects Versions: 0.22 > Reporter: Chuck Rolke > Assignee: Chuck Rolke > Fix For: 0.23 > > > The test case: > {code} > qpid-config add queue MyQueue --durable > qpid-config bind amq.match MyQueue SomeKey any property1=value1 > qpid-config bind amq.match MyQueue SomeKey all property1=value1 > {code} > Causes a management error as two bindings are created with > amq.match,MyQueue,SomeKey managementId. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org