QPID-3942: embed xid struct in more readable form into error messages or log statements

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

5 files changed, 43 insertions, 31 deletions

diff --git a/cpp/src/qpid/broker/DtxManager.cpp b/cpp/src/qpid/broker/DtxManager.cpp
index febd547478..d482c2c327 100644
--- a/cpp/src/qpid/broker/DtxManager.cpp
+++ b/cpp/src/qpid/broker/DtxManager.cpp
@@ -21,6 +21,7 @@
 #include "qpid/broker/DtxManager.h"
 #include "qpid/broker/DtxTimeout.h"
 #include "qpid/framing/reply_exceptions.h"
+#include "qpid/framing/StructHelper.h"
 #include "qpid/log/Statement.h"
 #include "qpid/sys/Timer.h"
 #include "qpid/ptr_map.h"
@@ -55,7 +56,7 @@ void DtxManager::recover(const std::string& xid, std::auto_ptr<TPCTransactionCon
 
 bool DtxManager::prepare(const std::string& xid)
 {
-    QPID_LOG(debug, "preparing: " << xid);
+    QPID_LOG(debug, "preparing: " << convert(xid));
     try {
         return getWork(xid)->prepare();
     } catch (DtxTimeoutException& e) {
@@ -66,7 +67,7 @@ bool DtxManager::prepare(const std::string& xid)
 
 bool DtxManager::commit(const std::string& xid, bool onePhase)
 {
-    QPID_LOG(debug, "committing: " << xid);
+    QPID_LOG(debug, "committing: " << convert(xid));
     try {
         bool result = getWork(xid)->commit(onePhase);
         remove(xid);
@@ -79,7 +80,7 @@ bool DtxManager::commit(const std::string& xid, bool onePhase)
 
 void DtxManager::rollback(const std::string& xid)
 {
-    QPID_LOG(debug, "rolling back: " << xid);
+    QPID_LOG(debug, "rolling back: " << convert(xid));
     try {
         getWork(xid)->rollback();
         remove(xid);
@@ -94,7 +95,7 @@ DtxWorkRecord* DtxManager::getWork(const std::string& xid)
     Mutex::ScopedLock locker(lock);
     WorkMap::iterator i = work.find(xid);
     if (i == work.end()) {
-        throw NotFoundException(QPID_MSG("Unrecognised xid " << xid));
+        throw NotFoundException(QPID_MSG("Unrecognised xid " << convert(xid)));
     }
     return ptr_map_ptr(i);
 }
@@ -109,7 +110,7 @@ void DtxManager::remove(const std::string& xid)
     Mutex::ScopedLock locker(lock);
     WorkMap::iterator i = work.find(xid);
     if (i == work.end()) {
-        throw NotFoundException(QPID_MSG("Unrecognised xid " << xid));
+        throw NotFoundException(QPID_MSG("Unrecognised xid " << convert(xid)));
     } else {
         work.erase(i);
     }
@@ -120,7 +121,7 @@ DtxWorkRecord* DtxManager::createWork(const std::string& xid)
     Mutex::ScopedLock locker(lock);
     WorkMap::iterator i = work.find(xid);
     if (i != work.end()) {
-        throw NotAllowedException(QPID_MSG("Xid " << xid << " is already known (use 'join' to add work to an existing xid)"));
+        throw NotAllowedException(QPID_MSG("Xid " << convert(xid) << " is already known (use 'join' to add work to an existing xid)"));
     } else {
         std::string ncxid = xid; // Work around const correctness problems in ptr_map.
         return ptr_map_ptr(work.insert(ncxid, new DtxWorkRecord(ncxid, store)).first);
@@ -175,3 +176,19 @@ void DtxManager::setStore (TransactionalStore* _store)
 {
     store = _store;
 }
+
+std::string DtxManager::convert(const qpid::framing::Xid& xid)
+{
+    qpid::framing::StructHelper helper;
+    std::string encoded;
+    helper.encode(xid, encoded);
+    return encoded;
+}
+
+qpid::framing::Xid DtxManager::convert(const std::string& xid)
+{
+    qpid::framing::StructHelper helper;
+    qpid::framing::Xid decoded;
+    helper.decode(decoded, xid);
+    return decoded;
+}
diff --git a/cpp/src/qpid/broker/DtxManager.h b/cpp/src/qpid/broker/DtxManager.h
index 11895695a3..6f03189f66 100644
--- a/cpp/src/qpid/broker/DtxManager.h
+++ b/cpp/src/qpid/broker/DtxManager.h
@@ -26,6 +26,7 @@
 #include "qpid/broker/DtxWorkRecord.h"
 #include "qpid/broker/TransactionalStore.h"
 #include "qpid/framing/amqp_types.h"
+#include "qpid/framing/Xid.h"
 #include "qpid/sys/Mutex.h"
 #include "qpid/ptr_map.h"
 
@@ -74,6 +75,8 @@ public:
     }
     DtxWorkRecord* getWork(const std::string& xid);
     bool exists(const std::string& xid);
+    static std::string convert(const framing::Xid& xid);
+    static framing::Xid convert(const std::string& xid);
 };
 
 }
diff --git a/cpp/src/qpid/broker/DtxWorkRecord.cpp b/cpp/src/qpid/broker/DtxWorkRecord.cpp
index a413fe418d..2c26fec49f 100644
--- a/cpp/src/qpid/broker/DtxWorkRecord.cpp
+++ b/cpp/src/qpid/broker/DtxWorkRecord.cpp
@@ -19,6 +19,7 @@
  *
  */
 #include "qpid/broker/DtxWorkRecord.h"
+#include "qpid/broker/DtxManager.h"
 #include "qpid/framing/reply_exceptions.h"
 #include <boost/format.hpp>
 #include <boost/mem_fn.hpp>
@@ -73,7 +74,7 @@ bool DtxWorkRecord::commit(bool onePhase)
         if (prepared) {
             //already prepared i.e. 2pc
             if (onePhase) {
-                throw IllegalStateException(QPID_MSG("Branch with xid " << xid << " has been prepared, one-phase option not valid!"));
+                throw IllegalStateException(QPID_MSG("Branch with xid " << DtxManager::convert(xid) << " has been prepared, one-phase option not valid!"));
             }
 
             store->commit(*txn);
@@ -84,7 +85,7 @@ bool DtxWorkRecord::commit(bool onePhase)
         } else {
             //1pc commit optimisation, don't need a 2pc transaction context:
             if (!onePhase) {
-                throw IllegalStateException(QPID_MSG("Branch with xid " << xid << " has not been prepared, one-phase option required!"));
+                throw IllegalStateException(QPID_MSG("Branch with xid " << DtxManager::convert(xid) << " has not been prepared, one-phase option required!"));
             }
             std::auto_ptr<TransactionContext> localtxn = store->begin();
             if (prepare(localtxn.get())) {
@@ -116,10 +117,10 @@ void DtxWorkRecord::add(DtxBuffer::shared_ptr ops)
 {
     Mutex::ScopedLock locker(lock);
     if (expired) {
-        throw DtxTimeoutException(QPID_MSG("Branch with xid " << xid << " has timed out."));
+        throw DtxTimeoutException(QPID_MSG("Branch with xid " << DtxManager::convert(xid) << " has timed out."));
     }
     if (completed) {
-        throw CommandInvalidException(QPID_MSG("Branch with xid " << xid << " has been completed!"));
+        throw CommandInvalidException(QPID_MSG("Branch with xid " << DtxManager::convert(xid) << " has been completed!"));
     }
     work.push_back(ops);
 }
@@ -133,7 +134,7 @@ bool DtxWorkRecord::check()
         //iterate through all DtxBuffers and ensure they are all ended
         for (Work::iterator i = work.begin(); i != work.end(); i++) {
             if (!(*i)->isEnded()) {
-                throw IllegalStateException(QPID_MSG("Branch with xid " << xid << " not completed!"));
+                throw IllegalStateException(QPID_MSG("Branch with xid " << DtxManager::convert(xid) << " not completed!"));
             } else if ((*i)->isRollbackOnly()) {
                 rolledback = true;
             }
diff --git a/cpp/src/qpid/broker/SessionAdapter.cpp b/cpp/src/qpid/broker/SessionAdapter.cpp
index 82a5f31872..7c689d56fb 100644
--- a/cpp/src/qpid/broker/SessionAdapter.cpp
+++ b/cpp/src/qpid/broker/SessionAdapter.cpp
@@ -549,13 +549,6 @@ void SessionAdapter::TxHandlerImpl::rollback()
     state.rollback();
 }
 
-std::string SessionAdapter::DtxHandlerImpl::convert(const framing::Xid& xid)
-{
-    std::string encoded;
-    encode(xid, encoded);
-    return encoded;
-}
-
 void SessionAdapter::DtxHandlerImpl::select()
 {
     state.selectDtx();
@@ -567,7 +560,7 @@ XaResult SessionAdapter::DtxHandlerImpl::end(const Xid& xid,
 {
     try {
         if (fail) {
-            state.endDtx(convert(xid), true);
+            state.endDtx(DtxManager::convert(xid), true);
             if (suspend) {
                 throw CommandInvalidException(QPID_MSG("End and suspend cannot both be set."));
             } else {
@@ -575,9 +568,9 @@ XaResult SessionAdapter::DtxHandlerImpl::end(const Xid& xid,
             }
         } else {
             if (suspend) {
-                state.suspendDtx(convert(xid));
+                state.suspendDtx(DtxManager::convert(xid));
             } else {
-                state.endDtx(convert(xid), false);
+                state.endDtx(DtxManager::convert(xid), false);
             }
             return XaResult(XA_STATUS_XA_OK);
         }
@@ -595,9 +588,9 @@ XaResult SessionAdapter::DtxHandlerImpl::start(const Xid& xid,
     }
     try {
         if (resume) {
-            state.resumeDtx(convert(xid));
+            state.resumeDtx(DtxManager::convert(xid));
         } else {
-            state.startDtx(convert(xid), getBroker().getDtxManager(), join);
+            state.startDtx(DtxManager::convert(xid), getBroker().getDtxManager(), join);
         }
         return XaResult(XA_STATUS_XA_OK);
     } catch (const DtxTimeoutException& /*e*/) {
@@ -608,7 +601,7 @@ XaResult SessionAdapter::DtxHandlerImpl::start(const Xid& xid,
 XaResult SessionAdapter::DtxHandlerImpl::prepare(const Xid& xid)
 {
     try {
-        bool ok = getBroker().getDtxManager().prepare(convert(xid));
+        bool ok = getBroker().getDtxManager().prepare(DtxManager::convert(xid));
         return XaResult(ok ? XA_STATUS_XA_OK : XA_STATUS_XA_RBROLLBACK);
     } catch (const DtxTimeoutException& /*e*/) {
         return XaResult(XA_STATUS_XA_RBTIMEOUT);
@@ -619,7 +612,7 @@ XaResult SessionAdapter::DtxHandlerImpl::commit(const Xid& xid,
                             bool onePhase)
 {
     try {
-        bool ok = getBroker().getDtxManager().commit(convert(xid), onePhase);
+        bool ok = getBroker().getDtxManager().commit(DtxManager::convert(xid), onePhase);
         return XaResult(ok ? XA_STATUS_XA_OK : XA_STATUS_XA_RBROLLBACK);
     } catch (const DtxTimeoutException& /*e*/) {
         return XaResult(XA_STATUS_XA_RBTIMEOUT);
@@ -630,7 +623,7 @@ XaResult SessionAdapter::DtxHandlerImpl::commit(const Xid& xid,
 XaResult SessionAdapter::DtxHandlerImpl::rollback(const Xid& xid)
 {
     try {
-        getBroker().getDtxManager().rollback(convert(xid));
+        getBroker().getDtxManager().rollback(DtxManager::convert(xid));
         return XaResult(XA_STATUS_XA_OK);
     } catch (const DtxTimeoutException& /*e*/) {
         return XaResult(XA_STATUS_XA_RBTIMEOUT);
@@ -660,7 +653,7 @@ void SessionAdapter::DtxHandlerImpl::forget(const Xid& xid)
 
 DtxGetTimeoutResult SessionAdapter::DtxHandlerImpl::getTimeout(const Xid& xid)
 {
-    uint32_t timeout = getBroker().getDtxManager().getTimeout(convert(xid));
+    uint32_t timeout = getBroker().getDtxManager().getTimeout(DtxManager::convert(xid));
     return DtxGetTimeoutResult(timeout);
 }
 
@@ -668,7 +661,7 @@ DtxGetTimeoutResult SessionAdapter::DtxHandlerImpl::getTimeout(const Xid& xid)
 void SessionAdapter::DtxHandlerImpl::setTimeout(const Xid& xid,
                                                 uint32_t timeout)
 {
-    getBroker().getDtxManager().setTimeout(convert(xid), timeout);
+    getBroker().getDtxManager().setTimeout(DtxManager::convert(xid), timeout);
 }
 
 
diff --git a/cpp/src/qpid/broker/SessionAdapter.h b/cpp/src/qpid/broker/SessionAdapter.h
index 8987c4812f..bc056538b1 100644
--- a/cpp/src/qpid/broker/SessionAdapter.h
+++ b/cpp/src/qpid/broker/SessionAdapter.h
@@ -226,10 +226,8 @@ class Queue;
         void rollback();
     };
 
-    class DtxHandlerImpl : public DtxHandler, public HandlerHelper, private framing::StructHelper
+    class DtxHandlerImpl : public DtxHandler, public HandlerHelper
     {
-        std::string convert(const framing::Xid& xid);
-
       public:
         DtxHandlerImpl(SemanticState& session) : HandlerHelper(session) {}