Code cleanup

  - Removed IdAllocator (it's no longer needed)
  - Cleaned up the calls to ManagementAgent::addObject to handle durable objects
  - Removed the deferred call to addObject for durable objects
  - Removed unneeded calls to self._checkClosed() in qmf.console.Agent


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

15 files changed, 34 insertions, 185 deletions

diff --git a/qpid/cpp/src/Makefile.am b/qpid/cpp/src/Makefile.am
index 757f08339c..7a78cb08b8 100644
--- a/qpid/cpp/src/Makefile.am
+++ b/qpid/cpp/src/Makefile.am
@@ -639,7 +639,6 @@ libqpidbroker_la_SOURCES = \
   qpid/broker/TxPublish.h \
   qpid/broker/Vhost.cpp \
   qpid/broker/Vhost.h \
-  qpid/management/IdAllocator.h \
   qpid/management/ManagementAgent.cpp \
   qpid/management/ManagementAgent.h \
   qpid/management/ManagementDirectExchange.cpp \
diff --git a/qpid/cpp/src/qpid/broker/Broker.cpp b/qpid/cpp/src/qpid/broker/Broker.cpp
index 50ab543baf..c95efaa1ef 100644
--- a/qpid/cpp/src/qpid/broker/Broker.cpp
+++ b/qpid/cpp/src/qpid/broker/Broker.cpp
@@ -185,7 +185,7 @@ Broker::Broker(const Broker::Options& conf) :
         else
             mgmtObject->clr_dataDir();
 
-        managementAgent->addObject(mgmtObject, 0x1000000000000002LL);
+        managementAgent->addObject(mgmtObject, 0, true);
 
         // Since there is currently no support for virtual hosts, a placeholder object
         // representing the implied single virtual host is added here to keep the
diff --git a/qpid/cpp/src/qpid/broker/Exchange.cpp b/qpid/cpp/src/qpid/broker/Exchange.cpp
index fed6698c36..1188cf0e5b 100644
--- a/qpid/cpp/src/qpid/broker/Exchange.cpp
+++ b/qpid/cpp/src/qpid/broker/Exchange.cpp
@@ -132,7 +132,7 @@ Exchange::Exchange (const string& _name, Manageable* parent, Broker* b) :
             mgmtExchange = new _qmf::Exchange (agent, this, parent, _name);
             mgmtExchange->set_durable(durable);
             mgmtExchange->set_autoDelete(false);
-            agent->addObject (mgmtExchange);
+            agent->addObject(mgmtExchange, 0, durable);
         }
     }
 }
@@ -151,15 +151,7 @@ Exchange::Exchange(const string& _name, bool _durable, const qpid::framing::Fiel
             mgmtExchange->set_durable(durable);
             mgmtExchange->set_autoDelete(false);
             mgmtExchange->set_arguments(ManagementAgent::toMap(args));
-            if (!durable) {
-                if (name.empty()) {
-                    agent->addObject (mgmtExchange, 0x1000000000000004LL);  // Special default exchange ID
-                } else if (name == QPID_MANAGEMENT) {
-                    agent->addObject (mgmtExchange, 0x1000000000000005LL);  // Special management exchange ID
-                } else {
-                    agent->addObject (mgmtExchange, agent->allocateId(this));
-                }
-            }
+            agent->addObject(mgmtExchange, 0, durable);
         }
     }
 
@@ -192,11 +184,6 @@ void Exchange::setAlternate(Exchange::shared_ptr _alternate)
 
 void Exchange::setPersistenceId(uint64_t id) const
 {
-    if (mgmtExchange != 0 && persistenceId == 0)
-    {
-        ManagementAgent* agent = broker->getManagementAgent();
-        agent->addObject (mgmtExchange, 0x2000000000000000LL + id);
-    }
     persistenceId = id;
 }
 
@@ -330,20 +317,19 @@ void Exchange::Binding::startManagement()
         Broker* broker = parent->getBroker();
         if (broker != 0) {
             ManagementAgent* agent = broker->getManagementAgent();
-            if (agent != 0)
-                {
-                    ManagementObject* mo = queue->GetManagementObject();
-                    if (mo != 0)
-                        {
-                            management::ObjectId queueId = mo->getObjectId();
-                            mgmtBinding = new _qmf::Binding
-                                (agent, this, (Manageable*) parent, queueId, key, ManagementAgent::toMap(args));
-                            if (!origin.empty())
-                                mgmtBinding->set_origin(origin);
-                            agent->addObject (mgmtBinding, agent->allocateId(this));
-                            static_cast<_qmf::Queue*>(mo)->inc_bindingCount();
-                        }
+            if (agent != 0) {
+                ManagementObject* mo = queue->GetManagementObject();
+                if (mo != 0) {
+                    management::ObjectId queueId = mo->getObjectId();
+
+                    mgmtBinding = new _qmf::Binding
+                        (agent, this, (Manageable*) parent, queueId, key, ManagementAgent::toMap(args));
+                    if (!origin.empty())
+                        mgmtBinding->set_origin(origin);
+                    agent->addObject(mgmtBinding);
+                    static_cast<_qmf::Queue*>(mo)->inc_bindingCount();
                 }
+            }
         }
     }
 }
diff --git a/qpid/cpp/src/qpid/broker/Link.cpp b/qpid/cpp/src/qpid/broker/Link.cpp
index cdba18ccf9..c88cabbda7 100644
--- a/qpid/cpp/src/qpid/broker/Link.cpp
+++ b/qpid/cpp/src/qpid/broker/Link.cpp
@@ -74,8 +74,7 @@ Link::Link(LinkRegistry*  _links,
         if (agent != 0)
         {
             mgmtObject = new _qmf::Link(agent, this, parent, _host, _port, _transport, _durable);
-            if (!durable)
-                agent->addObject(mgmtObject);
+            agent->addObject(mgmtObject, 0, durable);
         }
     }
     setStateLH(STATE_WAITING);
@@ -353,10 +352,6 @@ void Link::notifyConnectionForced(const string text)
 
 void Link::setPersistenceId(uint64_t id) const
 {
-    if (mgmtObject != 0 && persistenceId == 0) {
-        ManagementAgent* agent = broker->getManagementAgent();
-        agent->addObject(mgmtObject, id);
-    }
     persistenceId = id;
 }
 
diff --git a/qpid/cpp/src/qpid/broker/Queue.cpp b/qpid/cpp/src/qpid/broker/Queue.cpp
index d3a464a6fe..3b7461e094 100644
--- a/qpid/cpp/src/qpid/broker/Queue.cpp
+++ b/qpid/cpp/src/qpid/broker/Queue.cpp
@@ -105,19 +105,12 @@ Queue::Queue(const string& _name, bool _autodelete,
     broker(b),
     deleted(false)
 {
-    if (parent != 0 && broker != 0)
-    {
+    if (parent != 0 && broker != 0) {
         ManagementAgent* agent = broker->getManagementAgent();
 
-        if (agent != 0)
-        {
+        if (agent != 0) {
             mgmtObject = new _qmf::Queue(agent, this, parent, _name, _store != 0, _autodelete, _owner != 0);
-
-            // Add the object to the management agent only if this queue is not durable.
-            // If it's durable, we will add it later when the queue is assigned a persistenceId.
-            if (store == 0) {
-                agent->addObject (mgmtObject, agent->allocateId(this));
-            }
+            agent->addObject(mgmtObject, 0, store != 0);
         }
     }
 }
@@ -125,7 +118,7 @@ Queue::Queue(const string& _name, bool _autodelete,
 Queue::~Queue()
 {
     if (mgmtObject != 0)
-        mgmtObject->resourceDestroy ();
+        mgmtObject->resourceDestroy();
 }
 
 bool isLocalTo(const OwnershipToken* token, boost::intrusive_ptr<Message>& msg)
@@ -942,16 +935,11 @@ uint64_t Queue::getPersistenceId() const
 
 void Queue::setPersistenceId(uint64_t _persistenceId) const
 {
-    if (mgmtObject != 0 && persistenceId == 0)
+    if (mgmtObject != 0 && persistenceId == 0 && externalQueueStore)
     {
-        ManagementAgent* agent = broker->getManagementAgent();
-        agent->addObject (mgmtObject, 0x3000000000000000LL + _persistenceId);
-
-        if (externalQueueStore) {
-            ManagementObject* childObj = externalQueueStore->GetManagementObject();
-            if (childObj != 0)
-                childObj->setReference(mgmtObject->getObjectId());
-        }
+        ManagementObject* childObj = externalQueueStore->GetManagementObject();
+        if (childObj != 0)
+            childObj->setReference(mgmtObject->getObjectId());
     }
     persistenceId = _persistenceId;
 }
diff --git a/qpid/cpp/src/qpid/broker/SemanticState.cpp b/qpid/cpp/src/qpid/broker/SemanticState.cpp
index d4fa465f37..cb06db086a 100644
--- a/qpid/cpp/src/qpid/broker/SemanticState.cpp
+++ b/qpid/cpp/src/qpid/broker/SemanticState.cpp
@@ -281,7 +281,7 @@ SemanticState::ConsumerImpl::ConsumerImpl(SemanticState* _parent,
         {
             mgmtObject = new _qmf::Subscription(agent, this, ms , queue->GetManagementObject()->getObjectId() ,name,
                                                 !acquire, ackExpected, exclusive, ManagementAgent::toMap(arguments));
-            agent->addObject (mgmtObject, agent->allocateId(this));
+            agent->addObject (mgmtObject);
             mgmtObject->set_creditMode("WINDOW");
         }
     }
diff --git a/qpid/cpp/src/qpid/broker/SessionState.cpp b/qpid/cpp/src/qpid/broker/SessionState.cpp
index 4d5fe14690..ddf68cad2f 100644
--- a/qpid/cpp/src/qpid/broker/SessionState.cpp
+++ b/qpid/cpp/src/qpid/broker/SessionState.cpp
@@ -81,7 +81,7 @@ SessionState::SessionState(
             mgmtObject->set_detachedLifespan (0);
             mgmtObject->clr_expireTime();
             if (rateFlowcontrol) mgmtObject->set_maxClientRate(maxRate);
-            agent->addObject (mgmtObject, agent->allocateId(this));
+            agent->addObject(mgmtObject);
         }
     }
     attach(h);
diff --git a/qpid/cpp/src/qpid/broker/System.cpp b/qpid/cpp/src/qpid/broker/System.cpp
index 90c6b13cd3..8cd2edda76 100644
--- a/qpid/cpp/src/qpid/broker/System.cpp
+++ b/qpid/cpp/src/qpid/broker/System.cpp
@@ -78,7 +78,7 @@ System::System (string _dataDir, Broker* broker) : mgmtObject(0)
         mgmtObject->set_version  (version);
         mgmtObject->set_machine  (machine);
 
-        agent->addObject (mgmtObject, 0x1000000000000001LL);
+        agent->addObject(mgmtObject, 0, true);
     }
 }
 
diff --git a/qpid/cpp/src/qpid/broker/Vhost.cpp b/qpid/cpp/src/qpid/broker/Vhost.cpp
index df37cba255..a9ca3b42ab 100644
--- a/qpid/cpp/src/qpid/broker/Vhost.cpp
+++ b/qpid/cpp/src/qpid/broker/Vhost.cpp
@@ -38,7 +38,7 @@ Vhost::Vhost (qpid::management::Manageable* parentBroker, Broker* broker) : mgmt
         if (agent != 0)
         {
             mgmtObject = new _qmf::Vhost(agent, this, parentBroker, "/");
-            agent->addObject (mgmtObject, 0x1000000000000003LL);
+            agent->addObject(mgmtObject, 0, true);
         }
     }
 }
diff --git a/qpid/cpp/src/qpid/cluster/Cluster.cpp b/qpid/cpp/src/qpid/cluster/Cluster.cpp
index 924297fcba..a5581cff5e 100644
--- a/qpid/cpp/src/qpid/cluster/Cluster.cpp
+++ b/qpid/cpp/src/qpid/cluster/Cluster.cpp
@@ -157,7 +157,6 @@
 #include "qpid/framing/MessageTransferBody.h"
 #include "qpid/log/Helpers.h"
 #include "qpid/log/Statement.h"
-#include "qpid/management/IdAllocator.h"
 #include "qpid/management/ManagementAgent.h"
 #include "qpid/memory.h"
 #include "qpid/sys/Thread.h"
diff --git a/qpid/cpp/src/qpid/cluster/ClusterPlugin.cpp b/qpid/cpp/src/qpid/cluster/ClusterPlugin.cpp
index 3192896956..955487ee03 100644
--- a/qpid/cpp/src/qpid/cluster/ClusterPlugin.cpp
+++ b/qpid/cpp/src/qpid/cluster/ClusterPlugin.cpp
@@ -32,7 +32,6 @@
 #include "qpid/log/Statement.h"
 
 #include "qpid/management/ManagementAgent.h"
-#include "qpid/management/IdAllocator.h"
 #include "qpid/broker/Exchange.h"
 #include "qpid/broker/Message.h"
 #include "qpid/broker/Queue.h"
@@ -48,7 +47,6 @@ namespace cluster {
 
 using namespace std;
 using broker::Broker;
-using management::IdAllocator;
 using management::ManagementAgent;
 
 
@@ -77,46 +75,6 @@ struct ClusterOptions : public Options {
     }
 };
 
-struct UpdateClientIdAllocator : management::IdAllocator
-{
-    qpid::sys::AtomicValue<uint64_t> sequence;
-
-    UpdateClientIdAllocator() : sequence(0x4000000000000000LL) {}
-
-    uint64_t getIdFor(management::Manageable* m)
-    {
-        if (isUpdateQueue(m) || isUpdateExchange(m) || isUpdateSession(m) || isUpdateBinding(m)) {
-            return ++sequence;
-        } else {
-            return 0;
-        }
-    }
-
-    bool isUpdateQueue(management::Manageable* manageable)
-    {
-        qpid::broker::Queue* queue = dynamic_cast<qpid::broker::Queue*>(manageable);
-        return queue && queue->getName() == UpdateClient::UPDATE;
-    }
-
-    bool isUpdateExchange(management::Manageable* manageable)
-    {
-        qpid::broker::Exchange* exchange = dynamic_cast<qpid::broker::Exchange*>(manageable);
-        return exchange && exchange->getName() == UpdateClient::UPDATE;
-    }
-
-    bool isUpdateSession(management::Manageable* manageable)
-    {
-        broker::SessionState* session = dynamic_cast<broker::SessionState*>(manageable);
-        return session && session->getId().getName() == UpdateClient::UPDATE;
-    }
-
-    bool isUpdateBinding(management::Manageable* manageable)
-    {
-        broker::Exchange::Binding* binding = dynamic_cast<broker::Exchange::Binding*>(manageable);
-        return binding && binding->queue->getName() == UpdateClient::UPDATE;
-    }
-};
-
 struct ClusterPlugin : public Plugin {
 
     ClusterSettings settings;
@@ -139,11 +97,6 @@ struct ClusterPlugin : public Plugin {
         broker->setConnectionFactory(
             boost::shared_ptr<sys::ConnectionCodec::Factory>(
                 new ConnectionCodec::Factory(broker->getConnectionFactory(), *cluster)));
-        ManagementAgent* mgmt = broker->getManagementAgent();
-        if (mgmt) {
-            std::auto_ptr<IdAllocator> allocator(new UpdateClientIdAllocator());
-            mgmt->setAllocator(allocator);
-        }
     }
 
     void disallowManagementMethods(ManagementAgent* agent) {
diff --git a/qpid/cpp/src/qpid/management/IdAllocator.h b/qpid/cpp/src/qpid/management/IdAllocator.h
deleted file mode 100644
index 6f49d6d13f..0000000000
--- a/qpid/cpp/src/qpid/management/IdAllocator.h
+++ /dev/null
@@ -1,42 +0,0 @@
-#ifndef QPID_MANAGEMENT_IDALLOCATOR_H
-#define QPID_MANAGEMENT_IDALLOCATOR_H
-
-/*
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- * 
- *   http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-
-#include "qpid/management/Manageable.h"
-
-namespace qpid {
-namespace management {
-
-/**
- * Interface through which plugins etc can control the mgmt object id
- * allocation for special cases
- */
-struct IdAllocator
-{
-    virtual uint64_t getIdFor(Manageable* object) = 0;
-    virtual ~IdAllocator() {}
-};
-
-}} // namespace qpid::management
-
-#endif  /*!QPID_MANAGEMENT_IDALLOCATOR_H*/
diff --git a/qpid/cpp/src/qpid/management/ManagementAgent.cpp b/qpid/cpp/src/qpid/management/ManagementAgent.cpp
index 1a1415a06e..58a5d9bdd0 100644
--- a/qpid/cpp/src/qpid/management/ManagementAgent.cpp
+++ b/qpid/cpp/src/qpid/management/ManagementAgent.cpp
@@ -21,7 +21,6 @@
  
 #include "qpid/management/ManagementAgent.h"
 #include "qpid/management/ManagementObject.h"
-#include "qpid/management/IdAllocator.h"
 #include "qpid/broker/DeliverableMessage.h"
 #include "qpid/log/Statement.h"
 #include <qpid/broker/Message.h>
@@ -247,18 +246,13 @@ void ManagementAgent::registerEvent (const string&  packageName,
 }
 
 // Deprecated:  V1 objects
-ObjectId ManagementAgent::addObject(ManagementObject* object, uint64_t persistId)
+ObjectId ManagementAgent::addObject(ManagementObject* object, uint64_t persistId, bool persistent)
 {
     uint16_t sequence;
     uint64_t objectNum;
 
-    if (persistId == 0) {
-        sequence  = bootSequence;
-        objectNum = nextObjectId++;
-    } else {
-        sequence  = 0;
-        objectNum = persistId;
-    }
+    sequence = persistent ? 0 : bootSequence;
+    objectNum = persistId ? persistId : nextObjectId++;
 
     ObjectId objId(0 /*flags*/, sequence, brokerBank, objectNum);
     objId.setV2Key(*object);   // let object generate the v2 key
@@ -2170,19 +2164,6 @@ ManagementObjectMap::iterator ManagementAgent::numericFind(const ObjectId& oid)
     return iter;
 }
 
-void ManagementAgent::setAllocator(auto_ptr<IdAllocator> a)
-{
-    sys::Mutex::ScopedLock lock (userLock);
-    allocator = a;
-}
-
-uint64_t ManagementAgent::allocateId(Manageable* object)
-{
-    sys::Mutex::ScopedLock lock (userLock);
-    if (allocator.get()) return allocator->getIdFor(object);
-    return 0;
-}
-
 void ManagementAgent::disallow(const string& className, const string& methodName, const string& message) {
     disallowed[make_pair(className, methodName)] = message;
 }
diff --git a/qpid/cpp/src/qpid/management/ManagementAgent.h b/qpid/cpp/src/qpid/management/ManagementAgent.h
index dd781c36d3..a87cc917fe 100644
--- a/qpid/cpp/src/qpid/management/ManagementAgent.h
+++ b/qpid/cpp/src/qpid/management/ManagementAgent.h
@@ -42,8 +42,6 @@
 namespace qpid {
 namespace management {
 
-struct IdAllocator;
-
 class ManagementAgent
 {
 private:
@@ -95,10 +93,11 @@ public:
                                              uint8_t*    md5Sum,
                                              ManagementObject::writeSchemaCall_t schemaCall);
     QPID_BROKER_EXTERN ObjectId addObject   (ManagementObject* object,
-                                             uint64_t          persistId = 0);
+                                             uint64_t          persistId = 0,
+                                             bool              persistent = false);
     QPID_BROKER_EXTERN ObjectId addObject   (ManagementObject*  object,
                                              const std::string& key,
-                                             bool               persistent = true);
+                                             bool               persistent = false);
     QPID_BROKER_EXTERN void raiseEvent(const ManagementEvent& event,
                                        severity_t severity = SEV_DEFAULT);
     QPID_BROKER_EXTERN void clientAdded     (const std::string& routingKey);
@@ -113,9 +112,6 @@ public:
 
     const framing::Uuid& getUuid() const { return uuid; }
 
-    void setAllocator(std::auto_ptr<IdAllocator> allocator);
-    uint64_t allocateId(Manageable* object);
-
     /** Disallow a method. Attempts to call it will receive an exception with message. */
     void disallow(const std::string& className, const std::string& methodName, const std::string& message);
 
@@ -280,8 +276,6 @@ private:
     const qpid::sys::AbsTime     startTime;
     bool                         suppressed;
 
-    std::auto_ptr<IdAllocator> allocator;
-
     typedef std::pair<std::string,std::string> MethodName;
     typedef std::map<MethodName, std::string> DisallowedMethods;
     DisallowedMethods disallowed;
diff --git a/qpid/extras/qmf/src/py/qmf/console.py b/qpid/extras/qmf/src/py/qmf/console.py
index fa8708d717..5281423624 100644
--- a/qpid/extras/qmf/src/py/qmf/console.py
+++ b/qpid/extras/qmf/src/py/qmf/console.py
@@ -2404,22 +2404,18 @@ class Agent:
 
 
   def getBroker(self):
-    self._checkClosed()
     return self.broker
 
 
   def getBrokerBank(self):
-    self._checkClosed()
     return self.brokerBank
 
 
   def getAgentBank(self):
-    self._checkClosed()
     return self.agentBank
 
 
   def getV2RoutingKey(self):
-    self._checkClosed()
     if self.agentBank == '0':
       return 'broker'
     return self.agentBank