QPID-1732 - When an exchange is deleted, the binding-count for bound queues is not adjusted for deleted bindings.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@828674 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 85 insertions, 9 deletions

diff --git a/cpp/src/qpid/broker/DirectExchange.cpp b/cpp/src/qpid/broker/DirectExchange.cpp
index 29fe47beac..094f59cdec 100644
--- a/cpp/src/qpid/broker/DirectExchange.cpp
+++ b/cpp/src/qpid/broker/DirectExchange.cpp
@@ -80,7 +80,6 @@ bool DirectExchange::bind(Queue::shared_ptr queue, const string& routingKey, con
             propagate = bk.fedBinding.addOrigin(fedOrigin);
             if (mgmtExchange != 0) {
                 mgmtExchange->inc_bindingCount();
-                ((_qmf::Queue*) queue->GetManagementObject())->inc_bindingCount();
             }
         } else {
             return false;
@@ -130,7 +129,6 @@ bool DirectExchange::unbind(Queue::shared_ptr queue, const string& routingKey, c
             propagate = bk.fedBinding.delOrigin();
             if (mgmtExchange != 0) {
                 mgmtExchange->dec_bindingCount();
-                ((_qmf::Queue*) queue->GetManagementObject())->dec_bindingCount();
             }
         } else {
             return false;
diff --git a/cpp/src/qpid/broker/Exchange.cpp b/cpp/src/qpid/broker/Exchange.cpp
index 757127eef2..9468ba0b62 100644
--- a/cpp/src/qpid/broker/Exchange.cpp
+++ b/cpp/src/qpid/broker/Exchange.cpp
@@ -306,6 +306,7 @@ Exchange::Binding::Binding(const string& _key, Queue::shared_ptr _queue, Exchang
                             if (!origin.empty())
                                 mgmtBinding->set_origin(origin);
                             agent->addObject (mgmtBinding, agent->allocateId(this));
+                            static_cast<_qmf::Queue*>(mo)->inc_bindingCount();
                         }
                 }
         }
@@ -314,8 +315,12 @@ Exchange::Binding::Binding(const string& _key, Queue::shared_ptr _queue, Exchang
 
 Exchange::Binding::~Binding ()
 {
-    if (mgmtBinding != 0)
+    if (mgmtBinding != 0) {
+        ManagementObject* mo = queue->GetManagementObject();
+        if (mo != 0)
+            static_cast<_qmf::Queue*>(mo)->dec_bindingCount();
         mgmtBinding->resourceDestroy ();
+    }
 }
 
 ManagementObject* Exchange::Binding::GetManagementObject () const
diff --git a/cpp/src/qpid/broker/FanOutExchange.cpp b/cpp/src/qpid/broker/FanOutExchange.cpp
index b7d46a33fe..6d840b50df 100644
--- a/cpp/src/qpid/broker/FanOutExchange.cpp
+++ b/cpp/src/qpid/broker/FanOutExchange.cpp
@@ -66,7 +66,6 @@ bool FanOutExchange::bind(Queue::shared_ptr queue, const string& /*key*/, const
             propagate = fedBinding.addOrigin(fedOrigin);
             if (mgmtExchange != 0) {
                 mgmtExchange->inc_bindingCount();
-                ((_qmf::Queue*) queue->GetManagementObject())->inc_bindingCount();
             }
         } else {
             return false;
@@ -95,7 +94,6 @@ bool FanOutExchange::unbind(Queue::shared_ptr queue, const string& /*key*/, cons
         propagate = fedBinding.delOrigin();
         if (mgmtExchange != 0) {
             mgmtExchange->dec_bindingCount();
-            ((_qmf::Queue*) queue->GetManagementObject())->dec_bindingCount();
         }
     } else {
         return false;
diff --git a/cpp/src/qpid/broker/HeadersExchange.cpp b/cpp/src/qpid/broker/HeadersExchange.cpp
index a7c90156e1..38cc0e4050 100644
--- a/cpp/src/qpid/broker/HeadersExchange.cpp
+++ b/cpp/src/qpid/broker/HeadersExchange.cpp
@@ -82,7 +82,6 @@ bool HeadersExchange::bind(Queue::shared_ptr queue, const string& bindingKey, co
     if (bindings.add_unless(binding, MatchArgs(queue, args))) {
         if (mgmtExchange != 0) {
             mgmtExchange->inc_bindingCount();
-            ((_qmf::Queue*) queue->GetManagementObject())->inc_bindingCount();
         }
         routeIVE();
         return true;
@@ -95,7 +94,6 @@ bool HeadersExchange::unbind(Queue::shared_ptr queue, const string& bindingKey,
     if (bindings.remove_if(MatchKey(queue, bindingKey))) {
         if (mgmtExchange != 0) {
             mgmtExchange->dec_bindingCount();
-            ((_qmf::Queue*) queue->GetManagementObject())->dec_bindingCount();
         }
         return true;
     } else {
diff --git a/cpp/src/qpid/broker/TopicExchange.cpp b/cpp/src/qpid/broker/TopicExchange.cpp
index cb04742677..dd57549b5d 100644
--- a/cpp/src/qpid/broker/TopicExchange.cpp
+++ b/cpp/src/qpid/broker/TopicExchange.cpp
@@ -212,7 +212,6 @@ bool TopicExchange::bind(Queue::shared_ptr queue, const string& routingKey, cons
             propagate = bk.fedBinding.addOrigin(fedOrigin);
             if (mgmtExchange != 0) {
                 mgmtExchange->inc_bindingCount();
-                ((_qmf::Queue*) queue->GetManagementObject())->inc_bindingCount();
             }
         }
     } else if (fedOp == fedOpUnbind) {
@@ -273,7 +272,6 @@ bool TopicExchange::unbind(Queue::shared_ptr queue, const string& constRoutingKe
     if(qv.empty()) bindings.erase(bi);
     if (mgmtExchange != 0) {
         mgmtExchange->dec_bindingCount();
-        ((_qmf::Queue*) queue->GetManagementObject())->dec_bindingCount();
     }
 
     if (propagate)
diff --git a/python/tests_0-10/management.py b/python/tests_0-10/management.py
index 53573f309e..9dd03bbda4 100644
--- a/python/tests_0-10/management.py
+++ b/python/tests_0-10/management.py
@@ -317,3 +317,82 @@ class ManagementTest (TestBase010):
         conn = self.connect()
         session = conn.session("my-named-session")
         session.queue_declare(queue="whatever", exclusive=True, auto_delete=True)
+
+    def test_binding_count_on_queue(self):
+        self.startQmf()
+        conn = self.connect()
+        session = self.session
+
+        QUEUE = "binding_test_queue"
+        EX_DIR = "binding_test_exchange_direct"
+        EX_FAN = "binding_test_exchange_fanout"
+        EX_TOPIC = "binding_test_exchange_topic"
+        EX_HDR = "binding_test_exchange_headers"
+
+        #
+        # Create a test queue
+        #
+        session.queue_declare(queue=QUEUE, exclusive=True, auto_delete=True)
+        queue = self.qmf.getObjects(_class="queue", name=QUEUE)[0]
+        if not queue:
+            self.fail("Queue not found")
+        self.assertEqual(queue.bindingCount, 1, "wrong initial binding count")
+
+        #
+        # Create an exchange of each supported type
+        #
+        session.exchange_declare(exchange=EX_DIR, type="direct")
+        session.exchange_declare(exchange=EX_FAN, type="fanout")
+        session.exchange_declare(exchange=EX_TOPIC, type="topic")
+        session.exchange_declare(exchange=EX_HDR, type="headers")
+
+        #
+        # Bind each exchange to the test queue
+        #
+        match = {}
+        match['x-match'] = "all"
+        match['key'] = "value"
+        session.exchange_bind(exchange=EX_DIR, queue=QUEUE, binding_key="key1")
+        session.exchange_bind(exchange=EX_DIR, queue=QUEUE, binding_key="key2")
+        session.exchange_bind(exchange=EX_FAN, queue=QUEUE)
+        session.exchange_bind(exchange=EX_TOPIC, queue=QUEUE, binding_key="key1.#")
+        session.exchange_bind(exchange=EX_TOPIC, queue=QUEUE, binding_key="key2.#")
+        session.exchange_bind(exchange=EX_HDR, queue=QUEUE, binding_key="key1", arguments=match)
+        match['key2'] = "value2"
+        session.exchange_bind(exchange=EX_HDR, queue=QUEUE, binding_key="key2", arguments=match)
+
+        #
+        # Verify that the queue's binding count accounts for the new bindings
+        #
+        queue.update()
+        self.assertEqual(queue.bindingCount, 8,
+                         "added bindings not accounted for (expected 8, got %d)" % queue.bindingCount)
+
+        #
+        # Remove some of the bindings
+        #
+        session.exchange_unbind(exchange=EX_DIR, queue=QUEUE, binding_key="key2")
+        session.exchange_unbind(exchange=EX_TOPIC, queue=QUEUE, binding_key="key2.#")
+        session.exchange_unbind(exchange=EX_HDR, queue=QUEUE, binding_key="key2")
+
+        #
+        # Verify that the queue's binding count accounts for the deleted bindings
+        #
+        queue.update()
+        self.assertEqual(queue.bindingCount, 5,
+                         "deleted bindings not accounted for (expected 5, got %d)" % queue.bindingCount)
+        #
+        # Delete the exchanges
+        #
+        session.exchange_delete(exchange=EX_DIR)
+        session.exchange_delete(exchange=EX_FAN)
+        session.exchange_delete(exchange=EX_TOPIC)
+        session.exchange_delete(exchange=EX_HDR)
+
+        #
+        # Verify that the queue's binding count accounts for the lost bindings
+        #
+        queue.update()
+        self.assertEqual(queue.bindingCount, 1,
+                         "deleted bindings not accounted for (expected 1, got %d)" % queue.bindingCount)
+