[jbosscache-commits] JBoss Cache SVN: r5659 - in core/trunk/src: test/java/org/jboss/cache/transaction and 1 other directory.

jbosscache-commits at lists.jboss.org jbosscache-commits at lists.jboss.org
Thu Apr 24 07:10:33 EDT 2008


Author: manik.surtani at jboss.com
Date: 2008-04-24 07:10:33 -0400 (Thu, 24 Apr 2008)
New Revision: 5659

Modified:
   core/trunk/src/main/java/org/jboss/cache/commands/cachedata/PutDataMapCommand.java
   core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveNodeCommand.java
   core/trunk/src/test/java/org/jboss/cache/transaction/TransactionTest.java
Log:
Fixed rollbacks

Modified: core/trunk/src/main/java/org/jboss/cache/commands/cachedata/PutDataMapCommand.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/commands/cachedata/PutDataMapCommand.java	2008-04-24 10:44:47 UTC (rev 5658)
+++ core/trunk/src/main/java/org/jboss/cache/commands/cachedata/PutDataMapCommand.java	2008-04-24 11:10:33 UTC (rev 5659)
@@ -13,6 +13,7 @@
 import org.jboss.cache.optimistic.DataVersion;
 import org.jboss.cache.transaction.GlobalTransaction;
 
+import java.util.HashMap;
 import java.util.Map;
 
 /**
@@ -62,7 +63,11 @@
          log.trace("perform(" + globalTransaction + ", \"" + fqn + "\", " + data + " undo=" + createUndoOps + " erase=" + eraseContents + ")");
       }
       NodeSPI nodeSPI = cacheData.findNodeCheck(globalTransaction, fqn, false);
-      oldData = nodeSPI.getDataDirect();
+      Map dataDirect = nodeSPI.getDataDirect();
+      if (!dataDirect.isEmpty())
+      {
+         oldData = new HashMap(dataDirect); // defensive copy
+      }
       notifier.notifyNodeModified(fqn, true, NodeModifiedEvent.ModificationType.PUT_MAP, oldData, ctx);
 
       if (eraseContents) nodeSPI.clearDataDirect();
@@ -74,13 +79,14 @@
 
    public void rollback()
    {
-      if (trace)
+      if (trace) log.trace("rollback(" + globalTransaction + ", " + fqn + ", " + data + ")");
+
+      NodeSPI n = cacheData.findNode(fqn, null, true);
+      if (n != null)
       {
-         log.trace("rollback(" + globalTransaction + ", \"" + fqn + "\", " + data + ")");
+         n.clearDataDirect();
+         if (oldData != null) n.putAllDirect(oldData);
       }
-      NodeSPI n = cacheData.findNodeCheck(globalTransaction, fqn, true);
-      n.clearDataDirect();
-      n.putAllDirect(oldData);
    }
 
    public Object accept(InvocationContext ctx, CommandsVisitor handler) throws Throwable

Modified: core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveNodeCommand.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveNodeCommand.java	2008-04-24 10:44:47 UTC (rev 5658)
+++ core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveNodeCommand.java	2008-04-24 11:10:33 UTC (rev 5659)
@@ -14,6 +14,9 @@
 import org.jboss.cache.optimistic.DataVersion;
 import org.jboss.cache.transaction.GlobalTransaction;
 
+import java.util.HashMap;
+import java.util.Map;
+
 /**
  * Implements functionality defined by {@link org.jboss.cache.CacheSPI#removeNode(org.jboss.cache.Fqn)}
  *
@@ -36,6 +39,7 @@
    private boolean eviction;
    private Fqn parentFqn;
    private NodeSPI targetNode;
+   private Map originalData;
 
    public RemoveNodeCommand()
    {
@@ -99,6 +103,11 @@
       if (globalTransaction != null && createUndoOps && !eviction && found)
       {
          parentFqn = parentNode.getFqn();
+         Map targetData = targetNode.getDataDirect();
+         if (!targetData.isEmpty())
+         {
+            originalData = new HashMap(targetNode.getDataDirect());
+         }
       }
 
       notifyAfterEviction(ctx);
@@ -137,26 +146,31 @@
 
    public void rollback()
    {
-      String childName = (String) targetNode.getFqn().getLastElement();
-      if (trace)
+      if (targetNode != null)
       {
-         log.trace("rollback(\"" + parentFqn + "\", \"" + childName + "\", node=" + targetNode + ")");
-      }
+         Object childName = targetNode.getFqn().getLastElement();
+         if (trace)
+         {
+            log.trace("rollback(parent: " + parentFqn + ", child: " + childName + ", node=" + targetNode + ")");
+         }
 
-      if (parentFqn == null || childName == null || targetNode == null)
-      {
-         log.error("parent_fqn or childName or childNode was null");
-         return;
+         if (parentFqn == null || childName == null)
+         {
+            log.error("parent fqn or childName or childNode was null");
+            return;
+         }
+         NodeSPI parentNode = cacheData.findNode(parentFqn);
+         if (parentNode == null)
+         {
+            log.warn("node " + parentFqn + " not found");
+            return;
+         }
+         parentNode.addChild(childName, targetNode);
+         targetNode.markAsDeleted(false, true);
+         targetNode.clearDataDirect();
+         if (originalData != null) targetNode.putAllDirect(originalData);
+         targetNode.setValid(true, true);
       }
-      NodeSPI parentNode = cacheData.findNode(parentFqn);
-      if (parentNode == null)
-      {
-         log.warn("node " + parentFqn + " not found");
-         return;
-      }
-      parentNode.addChild(childName, targetNode);
-      targetNode.markAsDeleted(false, true);
-      targetNode.setValid(true, true);
    }
 
    public Object accept(InvocationContext ctx, CommandsVisitor handler) throws Throwable

Modified: core/trunk/src/test/java/org/jboss/cache/transaction/TransactionTest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/transaction/TransactionTest.java	2008-04-24 10:44:47 UTC (rev 5658)
+++ core/trunk/src/test/java/org/jboss/cache/transaction/TransactionTest.java	2008-04-24 11:10:33 UTC (rev 5659)
@@ -17,6 +17,7 @@
 import org.jboss.cache.NodeSPI;
 import org.jboss.cache.lock.IsolationLevel;
 import org.jboss.cache.lock.NodeLock;
+import org.jboss.cache.util.CachePrinter;
 import static org.testng.AssertJUnit.*;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
@@ -709,26 +710,18 @@
       }
    }
 
-   public void testDoubleRemovalOfSameData()
+   public void testDoubleRemovalOfSameData() throws Exception
    {
-      try
-      {
-         tx.begin();
-         cache.put("/foo/1", "item", 1);
-         assertEquals(cache.get("/foo/1", "item"), 1);
-         cache.removeNode("/foo/1");
-         assertNull(cache.get("/foo/1", "item"));
-         cache.removeNode("/foo/1");
-         assertNull(cache.get("/foo/1", "item"));
-         tx.rollback();
-         assertFalse(cache.exists("/foo/1"));
-         assertNull(cache.get("/foo/1", "item"));
-      }
-      catch (Throwable t)
-      {
-         t.printStackTrace();
-         fail(t.toString());
-      }
+      tx.begin();
+      cache.put("/foo/1", "item", 1);
+      assertEquals(cache.get("/foo/1", "item"), 1);
+      cache.removeNode("/foo/1");
+      assertNull(cache.get("/foo/1", "item"));
+      cache.removeNode("/foo/1");
+      assertNull(cache.get("/foo/1", "item"));
+      tx.rollback();
+      assertFalse(cache.exists("/foo/1"));
+      assertNull(cache.get("/foo/1", "item"));
    }
 
    /**
@@ -761,7 +754,7 @@
    /**
     * put(Fqn, Map) with a previous non-null map
     */
-   public void testputDataRollback2()
+   public void testputDataRollback2() throws Exception
    {
       Map<String, Comparable> m1, m2;
       m1 = new HashMap<String, Comparable>();
@@ -771,29 +764,21 @@
       m2.put("other", "bla");
       m2.put("name", "Michelle");
 
-      try
-      {
-         cache.put("/bela/ban", m1);
-         tx.begin();
+      cache.put("/bela/ban", m1);
+      tx.begin();
 
-         cache.put("/bela/ban", m2);
-         Map tmp = cache.getNode("/bela/ban").getData();
-         assertEquals(3, tmp.size());
-         assertEquals("Michelle", tmp.get("name"));
-         assertEquals(tmp.get("id"), 322649);
-         assertEquals("bla", tmp.get("other"));
-         tx.rollback();
+      cache.put("/bela/ban", m2);
+      Map tmp = cache.getNode("/bela/ban").getData();
+      assertEquals(3, tmp.size());
+      assertEquals("Michelle", tmp.get("name"));
+      assertEquals(tmp.get("id"), 322649);
+      assertEquals("bla", tmp.get("other"));
+      tx.rollback();
 
-         tmp = cache.getNode("/bela/ban").getData();
-         assertEquals(2, tmp.size());
-         assertEquals("Bela", tmp.get("name"));
-         assertEquals(tmp.get("id"), 322649);
-      }
-      catch (Throwable t)
-      {
-         t.printStackTrace();
-         fail(t.toString());
-      }
+      tmp = cache.getNode("/bela/ban").getData();
+      assertEquals(2, tmp.size());
+      assertEquals("Bela", tmp.get("name"));
+      assertEquals(tmp.get("id"), 322649);
    }
 
    public void testPutRollback()
@@ -834,22 +819,25 @@
 
    public void testSimpleRollbackTransactions() throws Exception
    {
-      final Fqn<String> FQN = Fqn.fromString("/a/b/c");
+      final Fqn<String> fqn = Fqn.fromString("/a/b/c");
       tx.begin();
-      cache.put(FQN, "entry", "commit");
+      cache.put(fqn, "entry", "commit");
       tx.commit();
 
       tx.begin();
-      cache.put(FQN, "entry", "rollback");
-      cache.removeNode(FQN);
+      cache.put(fqn, "entry", "rollback");
+      cache.removeNode(fqn);
       tx.rollback();
-      assertEquals("Node should keep the commited value", "commit", cache.getNode(FQN).get("entry"));
+      assertEquals("Node should keep the commited value", "commit", cache.getNode(fqn).get("entry"));
 
       tx.begin();
-      cache.removeNode(FQN);
-      cache.put(FQN, "entry", "rollback");
+      cache.removeNode(fqn);
+      cache.put(fqn, "entry", "rollback");
       tx.rollback();
-      assertEquals("Node should keep the commited value", "commit", cache.getNode(FQN).get("entry"));// THIS FAILS
+
+      System.out.println("Cache: " + CachePrinter.printCacheDetails(cache));
+
+      assertEquals("Node should keep the commited value", "commit", cache.getNode(fqn).get("entry"));// THIS FAILS
    }
 
    private TransactionManager startTransaction() throws Exception




More information about the jbosscache-commits mailing list