[
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: [email protected]
For additional commands, e-mail: [email protected]
