qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From acon...@apache.org
Subject svn commit: r1561828 - in /qpid/trunk/qpid/cpp/src/qpid/ha: Backup.cpp BrokerReplicator.cpp BrokerReplicator.h HaBroker.cpp Primary.cpp PrimaryTxObserver.cpp PrimaryTxObserver.h QueueReplicator.cpp QueueReplicator.h TxReplicator.cpp TxReplicator.h
Date Mon, 27 Jan 2014 20:09:25 GMT
Author: aconway
Date: Mon Jan 27 20:09:25 2014
New Revision: 1561828

URL: http://svn.apache.org/r1561828
Log:
NO-JIRA: Minor refactor to improve code safety: calling shared_from_this on creation.

Previous anti-pattern: Classes need to call shared_from_this during creation,
but can't call it in the ctor so had a separate initiailize function that the
user was required to call immediately after the constructor. Possible for user
to forget.

Improved pattern: Introduce public static create() functions to call constructor
and initialize, make constructor and initialize private.

Modified:
    qpid/trunk/qpid/cpp/src/qpid/ha/Backup.cpp
    qpid/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp
    qpid/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h
    qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp
    qpid/trunk/qpid/cpp/src/qpid/ha/Primary.cpp
    qpid/trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.cpp
    qpid/trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.h
    qpid/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp
    qpid/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h
    qpid/trunk/qpid/cpp/src/qpid/ha/TxReplicator.cpp
    qpid/trunk/qpid/cpp/src/qpid/ha/TxReplicator.h

Modified: qpid/trunk/qpid/cpp/src/qpid/ha/Backup.cpp
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/Backup.cpp?rev=1561828&r1=1561827&r2=1561828&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/Backup.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/Backup.cpp Mon Jan 27 20:09:25 2014
@@ -71,8 +71,7 @@ void Backup::setBrokerUrl(const Url& bro
             settings.mechanism, settings.username, settings.password,
             false);               // no amq.failover - don't want to use client URL.
         link = result.first;
-        replicator.reset(new BrokerReplicator(haBroker, link));
-        replicator->initialize();
+        replicator = BrokerReplicator::create(haBroker, link);
         broker.getExchanges().registerExchange(replicator);
     }
     link->setUrl(brokers);          // Outside the lock, once set link doesn't change.

Modified: qpid/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp?rev=1561828&r1=1561827&r2=1561828&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp Mon Jan 27 20:09:25 2014
@@ -274,6 +274,12 @@ template <class EventType> std::string k
 }
 }
 
+boost::shared_ptr<BrokerReplicator> BrokerReplicator::create(
+    HaBroker& hb, const boost::shared_ptr<broker::Link>& l) {
+    boost::shared_ptr<BrokerReplicator> br(new BrokerReplicator(hb, l));
+    br->initialize();
+    return br;
+}
 
 BrokerReplicator::BrokerReplicator(HaBroker& hb, const boost::shared_ptr<Link>&
l)
     : Exchange(QPID_CONFIGURATION_REPLICATOR),
@@ -763,15 +769,10 @@ boost::shared_ptr<QueueReplicator> Broke
     const boost::shared_ptr<Queue>& queue)
 {
     if (replicationTest.getLevel(*queue) == ALL) {
-        boost::shared_ptr<QueueReplicator> qr;
-        if (TxReplicator::isTxQueue(queue->getName())){
-            qr.reset(new TxReplicator(haBroker, queue, link));
-        }
-        else {
-            qr.reset(new QueueReplicator(haBroker, queue, link));
-        }
-        qr->activate();
-        return qr;
+        if (TxReplicator::isTxQueue(queue->getName())) 
+            return TxReplicator::create(haBroker, queue, link);
+        else
+            return QueueReplicator::create(haBroker, queue, link);
     }
     return boost::shared_ptr<QueueReplicator>();
 }

Modified: qpid/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h?rev=1561828&r1=1561827&r2=1561828&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h Mon Jan 27 20:09:25 2014
@@ -75,10 +75,11 @@ class BrokerReplicator : public broker::
   public:
     typedef boost::shared_ptr<QueueReplicator> QueueReplicatorPtr;
 
-    BrokerReplicator(HaBroker&, const boost::shared_ptr<broker::Link>&);
+    static boost::shared_ptr<BrokerReplicator> create(
+        HaBroker&, const boost::shared_ptr<broker::Link>&);
+
     ~BrokerReplicator();
 
-    void initialize();          // Must be called  immediately after constructor.
     void shutdown();
 
     // Exchange methods
@@ -98,6 +99,9 @@ class BrokerReplicator : public broker::
     QueueReplicatorPtr findQueueReplicator(const std::string& qname);
 
   private:
+    BrokerReplicator(HaBroker&, const boost::shared_ptr<broker::Link>&);
+    void initialize();          // Called in create()
+
     typedef std::pair<boost::shared_ptr<broker::Queue>, bool> CreateQueueResult;
     typedef std::pair<boost::shared_ptr<broker::Exchange>, bool> CreateExchangeResult;
 

Modified: qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp?rev=1561828&r1=1561827&r2=1561828&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp Mon Jan 27 20:09:25 2014
@@ -176,9 +176,7 @@ Manageable::status_t HaBroker::Managemen
           shared_ptr<broker::Link> link = result.first;
           link->setUrl(url);
           // Create a queue replicator
-          shared_ptr<QueueReplicator> qr(
-              new QueueReplicator(*this, queue, link));
-          qr->activate();
+          shared_ptr<QueueReplicator> qr(QueueReplicator::create(*this, queue, link));
           broker.getExchanges().registerExchange(qr);
           break;
       }

Modified: qpid/trunk/qpid/cpp/src/qpid/ha/Primary.cpp
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/Primary.cpp?rev=1561828&r1=1561827&r2=1561828&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/Primary.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/Primary.cpp Mon Jan 27 20:09:25 2014
@@ -408,9 +408,8 @@ void Primary::setCatchupQueues(const Rem
 shared_ptr<PrimaryTxObserver> Primary::makeTxObserver(
     const boost::intrusive_ptr<broker::TxBuffer>& txBuffer)
 {
-    shared_ptr<PrimaryTxObserver> observer(
-        new PrimaryTxObserver(*this, haBroker, txBuffer));
-    observer->initialize();
+    shared_ptr<PrimaryTxObserver> observer =
+        PrimaryTxObserver::create(*this, haBroker, txBuffer);
     txMap[observer->getTxQueue()->getName()] = observer;
     return observer;
 }

Modified: qpid/trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.cpp
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.cpp?rev=1561828&r1=1561827&r2=1561828&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.cpp Mon Jan 27 20:09:25 2014
@@ -82,6 +82,14 @@ class PrimaryTxObserver::Exchange : publ
 
 const string PrimaryTxObserver::Exchange::TYPE_NAME(string(QPID_HA_PREFIX)+"primary-tx-observer");
 
+boost::shared_ptr<PrimaryTxObserver> PrimaryTxObserver::create(
+    Primary& p, HaBroker& hb, const boost::intrusive_ptr<broker::TxBuffer>&
tx) {
+    boost::shared_ptr<PrimaryTxObserver> pto(new PrimaryTxObserver(p, hb, tx));
+    pto->initialize();
+    return pto;
+}
+
+
 PrimaryTxObserver::PrimaryTxObserver(
     Primary& p, HaBroker& hb, const boost::intrusive_ptr<broker::TxBuffer>&
tx
 ) :

Modified: qpid/trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.h
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.h?rev=1561828&r1=1561827&r2=1561828&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.h Mon Jan 27 20:09:25 2014
@@ -66,11 +66,10 @@ class PrimaryTxObserver : public broker:
                           public boost::enable_shared_from_this<PrimaryTxObserver>
 {
   public:
-    PrimaryTxObserver(Primary&, HaBroker&, const boost::intrusive_ptr<broker::TxBuffer>&);
-    ~PrimaryTxObserver();
+    static boost::shared_ptr<PrimaryTxObserver> create(
+        Primary&, HaBroker&, const boost::intrusive_ptr<broker::TxBuffer>&);
 
-    /** Call immediately after constructor, uses shared_from_this. */
-    void initialize();
+    ~PrimaryTxObserver();
 
     void enqueue(const QueuePtr&, const broker::Message&);
     void dequeue(const QueuePtr& queue, QueuePosition, ReplicationId);
@@ -96,6 +95,9 @@ class PrimaryTxObserver : public broker:
         ENDED                   ///< Commit or rollback sent, local transaction ended.
     };
 
+    PrimaryTxObserver(Primary&, HaBroker&, const boost::intrusive_ptr<broker::TxBuffer>&);
+    void initialize();
+
     void checkState(State expect, const std::string& msg);
     void end(sys::Mutex::ScopedLock&);
     void txPrepareOkEvent(const std::string& data);

Modified: qpid/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp?rev=1561828&r1=1561827&r2=1561828&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp Mon Jan 27 20:09:25 2014
@@ -108,6 +108,14 @@ class QueueReplicator::QueueObserver : p
     boost::shared_ptr<QueueReplicator> queueReplicator;
 };
 
+boost::shared_ptr<QueueReplicator> QueueReplicator::create(
+    HaBroker& hb, boost::shared_ptr<broker::Queue> q, boost::shared_ptr<broker::Link>
l)
+{
+    boost::shared_ptr<QueueReplicator> qr(new QueueReplicator(hb, q, l));
+    qr->initialize();
+    return qr;
+}
+
 QueueReplicator::QueueReplicator(HaBroker& hb,
                                  boost::shared_ptr<Queue> q,
                                  boost::shared_ptr<Link> l)
@@ -144,9 +152,7 @@ QueueReplicator::QueueReplicator(HaBroke
 
 QueueReplicator::~QueueReplicator() {}
 
-// This must be called immediately after the constructor.
-// It has to be separate so we can call shared_from_this().
-void QueueReplicator::activate() {
+void QueueReplicator::initialize() {
     Mutex::ScopedLock l(lock);
     QPID_LOG(debug, logPrefix << "Created");
     if (!queue) return;         // Already destroyed

Modified: qpid/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h?rev=1561828&r1=1561827&r2=1561828&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h Mon Jan 27 20:09:25 2014
@@ -67,13 +67,11 @@ class QueueReplicator : public broker::E
     /*** Copy QueueReplicators from the registry */
     static void copy(broker::ExchangeRegistry&, Vector& result);
 
-    QueueReplicator(HaBroker&,
-                    boost::shared_ptr<broker::Queue> q,
-                    boost::shared_ptr<broker::Link> l);
+    static boost::shared_ptr<QueueReplicator> create(
+        HaBroker&, boost::shared_ptr<broker::Queue> q, boost::shared_ptr<broker::Link>
l);
 
     ~QueueReplicator();
 
-    void activate();        // Must be called immediately after constructor.
     void disconnect();      // Called when we are disconnected from the primary.
 
     std::string getType() const;
@@ -97,6 +95,11 @@ class QueueReplicator : public broker::E
     typedef boost::function<void(const std::string&, sys::Mutex::ScopedLock&)>
DispatchFn;
     typedef qpid::sys::unordered_map<std::string, DispatchFn> DispatchMap;
 
+    QueueReplicator(
+        HaBroker&, boost::shared_ptr<broker::Queue>, boost::shared_ptr<broker::Link>);
+
+    void initialize();          // Called as part of create()
+
     virtual void deliver(const broker::Message&);
     virtual void destroy();             // Called when the queue is destroyed.
 

Modified: qpid/trunk/qpid/cpp/src/qpid/ha/TxReplicator.cpp
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/TxReplicator.cpp?rev=1561828&r1=1561827&r2=1561828&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/TxReplicator.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/TxReplicator.cpp Mon Jan 27 20:09:25 2014
@@ -70,6 +70,16 @@ string TxReplicator::getTxId(const strin
 
 string TxReplicator::getType() const { return ReplicatingSubscription::QPID_TX_REPLICATOR;
}
 
+boost::shared_ptr<TxReplicator> TxReplicator::create(
+    HaBroker& hb,
+    const boost::shared_ptr<broker::Queue>& txQueue,
+    const boost::shared_ptr<broker::Link>& link)
+{
+    boost::shared_ptr<TxReplicator> tr(new TxReplicator(hb, txQueue, link));
+    tr->initialize();
+    return tr;
+}
+
 TxReplicator::TxReplicator(
     HaBroker& hb,
     const boost::shared_ptr<broker::Queue>& txQueue,

Modified: qpid/trunk/qpid/cpp/src/qpid/ha/TxReplicator.h
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/TxReplicator.h?rev=1561828&r1=1561827&r2=1561828&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/TxReplicator.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/TxReplicator.h Mon Jan 27 20:09:25 2014
@@ -58,7 +58,9 @@ class TxReplicator : public QueueReplica
     static bool isTxQueue(const std::string& queue);
     static std::string getTxId(const std::string& queue);
 
-    TxReplicator(HaBroker&, const QueuePtr& txQueue, const LinkPtr& link);
+    static boost::shared_ptr<TxReplicator> create(
+        HaBroker&, const QueuePtr& txQueue, const LinkPtr& link);
+
     ~TxReplicator();
 
     std::string getType() const;
@@ -78,6 +80,7 @@ class TxReplicator : public QueueReplica
     typedef qpid::sys::unordered_map<std::string, DispatchFunction> DispatchMap;
     typedef qpid::sys::unordered_map<std::string, ReplicationIdSet> DequeueMap;
 
+    TxReplicator(HaBroker&, const QueuePtr& txQueue, const LinkPtr& link);
     void sendMessage(const broker::Message&, sys::Mutex::ScopedLock&);
     void enqueue(const std::string& data, sys::Mutex::ScopedLock&);
     void dequeue(const std::string& data, sys::Mutex::ScopedLock&);



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


Mime
View raw message