GERONIMO-5519 match up the xid with the name during recovery.  Add some more tracing. Merge from 2.1 branch rev 984273

git-svn-id: https://svn.apache.org/repos/asf/geronimo/components/txmanager/trunk@984471 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoverTask.java b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoverTask.java
index 2ce0294..f55cac0 100644
--- a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoverTask.java
+++ b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoverTask.java
@@ -43,7 +43,7 @@
         this.recoverableTransactionManager = recoverableTransactionManager;
     }
 
-    @Override
+//    @Override
     public void run() {
         try {
             NamedXAResource namedXAResource = namedXAResourceFactory.getNamedXAResource();
diff --git a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java
index 69db34b..d3d1668 100644
--- a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java
+++ b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/RecoveryImpl.java
@@ -74,9 +74,11 @@
         }
         for (XidBranchesPair xidBranchesPair : preparedXids) {
             Xid xid = xidBranchesPair.getXid();
+            log.trace("read prepared global xid from log: " + toString(xid));
             if (xidFactory.matchesGlobalId(xid.getGlobalTransactionId())) {
                 ourXids.put(new ByteArrayWrapper(xid.getGlobalTransactionId()), xidBranchesPair);
                 for (TransactionBranchInfo transactionBranchInfo : xidBranchesPair.getBranches()) {
+                    log.trace("read branch from log: " + transactionBranchInfo.toString());
                     String name = transactionBranchInfo.getResourceName();
                     Set<XidBranchesPair> transactionsForName = nameToOurTxMap.get(name);
                     if (transactionsForName == null) {
@@ -86,6 +88,7 @@
                     transactionsForName.add(xidBranchesPair);
                 }
             } else {
+                log.trace("read external prepared xid from log: " + toString(xid));
                 TransactionImpl externalTx = new ExternalTransaction(xid, txLog, retryScheduler, xidBranchesPair.getBranches());
                 externalXids.put(xid, externalTx);
                 externalGlobalIdMap.put(xid.getGlobalTransactionId(), externalTx);
@@ -99,6 +102,7 @@
         Xid[] prepared = xaResource.recover(XAResource.TMSTARTRSCAN + XAResource.TMENDRSCAN);
         for (int i = 0; prepared != null && i < prepared.length; i++) {
             Xid xid = prepared[i];
+            log.trace("considering recovered xid from\n name: " + xaResource.getName() + "\n " + toString(xid));
             ByteArrayWrapper globalIdWrapper = new ByteArrayWrapper(xid.getGlobalTransactionId());
             XidBranchesPair xidNamesPair = ourXids.get(globalIdWrapper);
             
@@ -107,7 +111,8 @@
                 // Only commit if this NamedXAResource was the XAResource for the transaction.
                 // Otherwise, wait for recoverResourceManager to be called for the actual XAResource 
                 // This is a bit wasteful, but given our management of XAResources by "name", is about the best we can do.
-                if (isNameInTransaction(xidNamesPair, name)) {
+                if (isNameInTransaction(xidNamesPair, name, xid)) {
+                    log.trace("This xid was prepared from this XAResource: committing");
                     try {
                         xaResource.commit(xid, false);
                     } catch(XAException e) {
@@ -115,9 +120,12 @@
                         log.error("Recovery error", e);
                     }
                     removeNameFromTransaction(xidNamesPair, name, true);
+                } else {
+                    log.trace("This xid was prepared from another XAResource, ignoring");
                 }
             } else if (xidFactory.matchesGlobalId(xid.getGlobalTransactionId())) {
                 //ours, but prepare not logged
+                log.trace("this xid was initiated from this tm but not prepared: rolling back");
                 try {
                     xaResource.rollback(xid);
                 } catch (XAException e) {
@@ -129,6 +137,7 @@
                 TransactionImpl externalTx = externalGlobalIdMap.get(xid.getGlobalTransactionId());
                 if (externalTx == null) {
                     //we did not prepare this branch, rollback.
+                    log.trace("this xid is from an external transaction and was not prepared: rolling back");
                     try {
                         xaResource.rollback(xid);
                     } catch (XAException e) {
@@ -136,6 +145,7 @@
                         log.error("Could not roll back", e);
                     }
                 } else {
+                    log.trace("this xid is from an external transaction and was prepared in this tm.  Waiting for instructions from transaction originator");
                     //we prepared this branch, must wait for commit/rollback command.
                     externalTx.addBranchXid(xaResource, xid);
                 }
@@ -150,15 +160,21 @@
         }
     }
 
-    private boolean isNameInTransaction(XidBranchesPair xidBranchesPair, String name) {
+    private boolean isNameInTransaction(XidBranchesPair xidBranchesPair, String name, Xid xid) {
         for (TransactionBranchInfo transactionBranchInfo : xidBranchesPair.getBranches()) {
-            if (name.equals(transactionBranchInfo.getResourceName())) {
+            if (name.equals(transactionBranchInfo.getResourceName()) && equals(xid, transactionBranchInfo.getBranchXid())) {
                 return true;
             }
         }
         return false;
     }
-    
+
+    private boolean equals(Xid xid1, Xid xid2) {
+        return xid1.getFormatId() == xid1.getFormatId()
+                && Arrays.equals(xid1.getBranchQualifier(), xid2.getBranchQualifier())
+                && Arrays.equals(xid1.getGlobalTransactionId(), xid2.getGlobalTransactionId());
+    }
+
     private void removeNameFromTransaction(XidBranchesPair xidBranchesPair, String name, boolean warn) {
         int removed = 0;
         for (Iterator branches = xidBranchesPair.getBranches().iterator(); branches.hasNext();) {
@@ -206,6 +222,28 @@
         return new HashMap<Xid, TransactionImpl>(externalXids);
     }
 
+    private static String toString(Xid xid) {
+        if (xid instanceof XidImpl) {
+            return xid.toString();
+        }
+        StringBuilder s = new StringBuilder();
+        s.append("[Xid:class=").append(xid.getClass().getSimpleName()).append(":globalId=");
+        byte[] globalId = xid.getGlobalTransactionId();
+        for (int i = 0; i < globalId.length; i++) {
+            s.append(Integer.toHexString(globalId[i]));
+        }
+        s.append(",length=").append(globalId.length);
+        s.append(",branchId=");
+        byte[] branchId = xid.getBranchQualifier();
+        for (int i = 0; i < branchId.length; i++) {
+            s.append(Integer.toHexString(branchId[i]));
+        }
+        s.append(",length=");
+        s.append(branchId.length);
+        s.append("]");
+        return s.toString();
+    }
+
     private static class ByteArrayWrapper {
         private final byte[] bytes;
         private final int hashCode;
diff --git a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionBranchInfoImpl.java b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionBranchInfoImpl.java
index 1aaf002..5211b02 100644
--- a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionBranchInfoImpl.java
+++ b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionBranchInfoImpl.java
@@ -32,6 +32,8 @@
     private final String resourceName;
 
     public TransactionBranchInfoImpl(Xid branchXid, String resourceName) {
+        if (resourceName == null) throw new NullPointerException("resourceName");
+        if (branchXid == null) throw new NullPointerException("branchXid");
         this.branchXid = branchXid;
         this.resourceName = resourceName;
     }
@@ -43,4 +45,16 @@
     public String getResourceName() {
         return resourceName;
     }
+
+    @Override
+    public String toString() {
+        StringBuilder b = new StringBuilder("[Transaction branch:\n");
+        b.append(" name:").append(resourceName);
+        b.append("\n branchId: ");
+        for (byte i : branchXid.getBranchQualifier()) {
+            b.append(Integer.toHexString(i));
+        }
+        b.append("\n]\n");
+        return b.toString();
+    }
 }
diff --git a/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/AbstractRecoveryTest.java b/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/AbstractRecoveryTest.java
index 78292fe..3c11f5d 100644
--- a/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/AbstractRecoveryTest.java
+++ b/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/AbstractRecoveryTest.java
@@ -49,8 +49,8 @@
 
     public void test2ResOnlineAfterRecoveryStart() throws Exception {
         Xid[] xids = getXidArray(XID_COUNT);
-        MockXAResource xares1 = new MockXAResource(RM1, xids);
-        MockXAResource xares2 = new MockXAResource(RM2, xids);
+        MockXAResource xares1 = new MockXAResource(RM1);
+        MockXAResource xares2 = new MockXAResource(RM2);
         MockTransactionInfo[] txInfos = makeTxInfos(xids);
         addBranch(txInfos, xares1);
         addBranch(txInfos, xares2);
@@ -89,9 +89,9 @@
         tmp.addAll(xids23List);
         Xid[] xids3 = (Xid[]) tmp.toArray(new Xid[6]);
 
-        MockXAResource xares1 = new MockXAResource(RM1, xids1);
-        MockXAResource xares2 = new MockXAResource(RM2, xids2);
-        MockXAResource xares3 = new MockXAResource(RM3, xids3);
+        MockXAResource xares1 = new MockXAResource(RM1);
+        MockXAResource xares2 = new MockXAResource(RM2);
+        MockXAResource xares3 = new MockXAResource(RM3);
         MockTransactionInfo[] txInfos12 = makeTxInfos(xids12);
         addBranch(txInfos12, xares1);
         addBranch(txInfos12, xares2);
@@ -140,12 +140,13 @@
         return xids;
     }
 
-    private void addBranch(MockTransactionInfo[] txInfos, MockXAResource xaRes) {
+    private void addBranch(MockTransactionInfo[] txInfos, MockXAResource xaRes) throws XAException {
         for (int i = 0; i < txInfos.length; i++) {
             MockTransactionInfo txInfo = txInfos[i];
             Xid globalXid = txInfo.globalXid;
             Xid branchXid = xidFactory.createBranch(globalXid, branchCounter++);
-            txInfo.branches.add(new MockTransactionBranchInfo(xaRes.getName(), branchXid));
+            xaRes.start(branchXid, 0);
+            txInfo.branches.add(new TransactionBranchInfoImpl(branchXid, xaRes.getName()));
         }
     }
 
@@ -161,13 +162,12 @@
     private static class MockXAResource implements NamedXAResource {
 
         private final String name;
-        private final Xid[] xids;
-        private final List committed = new ArrayList();
-        private final List rolledBack = new ArrayList();
+        private final List<Xid> xids = new ArrayList<Xid>();
+        private final List<Xid> committed = new ArrayList<Xid>();
+        private final List<Xid> rolledBack = new ArrayList<Xid>();
 
-        public MockXAResource(String name, Xid[] xids) {
+        public MockXAResource(String name) {
             this.name = name;
-            this.xids = xids;
         }
 
         public String getName() {
@@ -197,7 +197,7 @@
         }
 
         public Xid[] recover(int flag) throws XAException {
-            return xids;
+            return xids.toArray(new Xid[xids.size()]);
         }
 
         public void rollback(Xid xid) throws XAException {
@@ -209,6 +209,7 @@
         }
 
         public void start(Xid xid, int flags) throws XAException {
+            xids.add(xid);
         }
 
         public List getCommitted() {
@@ -232,21 +233,4 @@
         }
     }
 
-    private static class MockTransactionBranchInfo implements TransactionBranchInfo {
-        private final String name;
-        private final Xid branchXid;
-
-        public MockTransactionBranchInfo(String name, Xid branchXid) {
-            this.name = name;
-            this.branchXid = branchXid;
-        }
-
-        public String getResourceName() {
-            return name;
-        }
-
-        public Xid getBranchXid() {
-            return branchXid;
-        }
-    }
 }
diff --git a/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/MockLog.java b/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/MockLog.java
index f4283f7..21593c0 100644
--- a/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/MockLog.java
+++ b/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/MockLog.java
@@ -33,9 +33,9 @@
  * */
 public class MockLog implements TransactionLog {
 
-    final Map prepared = new HashMap();
-    final List committed = new ArrayList();
-    final List rolledBack = new ArrayList();
+    final Map<Xid, Recovery.XidBranchesPair> prepared = new HashMap<Xid, Recovery.XidBranchesPair>();
+    final List<Xid> committed = new ArrayList<Xid>();
+    final List<Xid> rolledBack = new ArrayList<Xid>();
 
     public void begin(Xid xid) throws LogException {
     }
@@ -56,8 +56,8 @@
         rolledBack.add(xid);
     }
 
-    public Collection recover(XidFactory xidFactory) throws LogException {
-        Map copy = new HashMap(prepared);
+    public Collection<Recovery.XidBranchesPair> recover(XidFactory xidFactory) throws LogException {
+        Map<Xid, Recovery.XidBranchesPair> copy = new HashMap<Xid, Recovery.XidBranchesPair>(prepared);
         copy.keySet().removeAll(committed);
         copy.keySet().removeAll(rolledBack);
         return copy.values();
diff --git a/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java b/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java
index ecfe76e..3720b64 100644
--- a/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java
+++ b/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/RecoveryTest.java
@@ -40,6 +40,7 @@
     private final String RM1 = "rm1";
     private final String RM2 = "rm2";
     private final String RM3 = "rm3";
+    private int count = 0;
 
     public void testCommittedRMToBeRecovered() throws Exception {
         MockLog mockLog = new MockLog();
@@ -48,7 +49,7 @@
         // to recover. This means that the presumed abort protocol has failed
         // right after the commit of the RM and before the force-write of the
         // committed log record.
-        MockXAResource xares1 = new MockXAResource(RM1, new Xid[0]);
+        MockXAResource xares1 = new MockXAResource(RM1);
         MockTransactionInfo[] txInfos = makeTxInfos(xids);
         addBranch(txInfos, xares1);
         prepareLog(mockLog, txInfos);
@@ -58,7 +59,7 @@
         assertTrue(recovery.getExternalXids().isEmpty());
         assertTrue(!recovery.localRecoveryComplete());
         recovery.recoverResourceManager(xares1);
-        assertEquals(0, xares1.committed.size());
+        assertEquals(1, xares1.committed.size());
         assertTrue(recovery.localRecoveryComplete());
         
     }
@@ -66,8 +67,8 @@
     public void test2ResOnlineAfterRecoveryStart() throws Exception {
         MockLog mockLog = new MockLog();
         Xid[] xids = getXidArray(3);
-        MockXAResource xares1 = new MockXAResource(RM1, xids);
-        MockXAResource xares2 = new MockXAResource(RM2, xids);
+        MockXAResource xares1 = new MockXAResource(RM1);
+        MockXAResource xares2 = new MockXAResource(RM2);
         MockTransactionInfo[] txInfos = makeTxInfos(xids);
         addBranch(txInfos, xares1);
         addBranch(txInfos, xares2);
@@ -86,10 +87,12 @@
 
     }
 
-    private void addBranch(MockTransactionInfo[] txInfos, MockXAResource xaRes) {
+    private void addBranch(MockTransactionInfo[] txInfos, MockXAResource xaRes) throws XAException {
         for (int i = 0; i < txInfos.length; i++) {
             MockTransactionInfo txInfo = txInfos[i];
-            txInfo.branches.add(new MockTransactionBranchInfo(xaRes.getName()));
+            Xid xid = xidFactory.createBranch(txInfo.globalXid, count++);
+            xaRes.start(xid, 0);
+            txInfo.branches.add(new TransactionBranchInfoImpl(xid, xaRes.getName()));
         }
     }
 
@@ -97,7 +100,7 @@
         MockTransactionInfo[] txInfos = new MockTransactionInfo[xids.length];
         for (int i = 0; i < xids.length; i++) {
             Xid xid = xids[i];
-            txInfos[i] = new MockTransactionInfo(xid, new ArrayList());
+            txInfos[i] = new MockTransactionInfo(xid, new ArrayList<TransactionBranchInfo>());
         }
         return txInfos;
     }
@@ -123,9 +126,9 @@
         tmp.addAll(xids23List);
         Xid[] xids3 = (Xid[]) tmp.toArray(new Xid[6]);
 
-        MockXAResource xares1 = new MockXAResource(RM1, xids1);
-        MockXAResource xares2 = new MockXAResource(RM2, xids2);
-        MockXAResource xares3 = new MockXAResource(RM3, xids3);
+        MockXAResource xares1 = new MockXAResource(RM1);
+        MockXAResource xares2 = new MockXAResource(RM2);
+        MockXAResource xares3 = new MockXAResource(RM3);
         MockTransactionInfo[] txInfos12 = makeTxInfos(xids12);
         addBranch(txInfos12, xares1);
         addBranch(txInfos12, xares2);
@@ -174,13 +177,12 @@
     private static class MockXAResource implements NamedXAResource {
 
         private final String name;
-        private final Xid[] xids;
-        private final List committed = new ArrayList();
-        private final List rolledBack = new ArrayList();
+        private final List<Xid> xids = new ArrayList<Xid>();
+        private final List<Xid> committed = new ArrayList<Xid>();
+        private final List<Xid> rolledBack = new ArrayList<Xid>();
 
-        public MockXAResource(String name, Xid[] xids) {
+        public MockXAResource(String name) {
             this.name = name;
-            this.xids = xids;
         }
 
         public String getName() {
@@ -210,7 +212,7 @@
         }
 
         public Xid[] recover(int flag) throws XAException {
-            return xids;
+            return xids.toArray(new Xid[xids.size()]);
         }
 
         public void rollback(Xid xid) throws XAException {
@@ -222,6 +224,7 @@
         }
 
         public void start(Xid xid, int flags) throws XAException {
+            xids.add(xid);
         }
 
         public List getCommitted() {
@@ -237,27 +240,12 @@
 
     private static class MockTransactionInfo {
         private Xid globalXid;
-        private List branches;
+        private List<TransactionBranchInfo> branches;
 
-        public MockTransactionInfo(Xid globalXid, List branches) {
+        public MockTransactionInfo(Xid globalXid, List<TransactionBranchInfo> branches) {
             this.globalXid = globalXid;
             this.branches = branches;
         }
     }
 
-    private static class MockTransactionBranchInfo implements TransactionBranchInfo {
-        private String name;
-
-        public MockTransactionBranchInfo(String name) {
-            this.name = name;
-        }
-
-        public String getResourceName() {
-            return name;
-        }
-
-        public Xid getBranchXid() {
-            return null;
-        }
-    }
 }