Author: mircea.markus Date: 2008-06-11 10:20:32 -0400 (Wed, 11 Jun 2008) New Revision: 5974 Added: core/trunk/src/test/java/org/jboss/cache/commands/RollbackOnNoOpTest.java core/trunk/src/test/java/org/jboss/cache/commands/write/MoveCommandTest.java Modified: core/trunk/src/main/java/org/jboss/cache/DataContainerImpl.java core/trunk/src/main/java/org/jboss/cache/commands/write/ClearDataCommand.java core/trunk/src/main/java/org/jboss/cache/commands/write/MoveCommand.java core/trunk/src/main/java/org/jboss/cache/commands/write/RemoveKeyCommand.java core/trunk/src/main/java/org/jboss/cache/commands/write/RemoveNodeCommand.java core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java core/trunk/src/main/java/org/jboss/cache/loader/AbstractCacheLoader.java core/trunk/src/main/java/org/jboss/cache/loader/JDBCCacheLoader.java core/trunk/src/main/java/org/jboss/cache/transaction/DummyTransaction.java core/trunk/src/test/java/org/jboss/cache/commands/StructuralNodesOnRollbackTest.java core/trunk/src/test/java/org/jboss/cache/commands/read/AbstractDataCommandTest.java core/trunk/src/test/java/org/jboss/cache/commands/read/ExistsCommandTest.java core/trunk/src/test/java/org/jboss/cache/commands/read/GetChildrenNamesCommandTest.java core/trunk/src/test/java/org/jboss/cache/commands/read/GetDataMapCommandTest.java core/trunk/src/test/java/org/jboss/cache/commands/read/GetKeysCommandTest.java core/trunk/src/test/java/org/jboss/cache/commands/write/ClearDataCommandTest.java core/trunk/src/test/java/org/jboss/cache/commands/write/CreateNodeCommandTest.java core/trunk/src/test/java/org/jboss/cache/commands/write/EvictCommandTest.java core/trunk/src/test/java/org/jboss/cache/commands/write/InvalidateCommandTest.java core/trunk/src/test/java/org/jboss/cache/commands/write/OptimisticInvalidateCommandTest.java core/trunk/src/test/java/org/jboss/cache/commands/write/RemoveKeyCommandTest.java Log: http://jira.jboss.com/jira/browse/JBCACHE-1363 Modified: core/trunk/src/main/java/org/jboss/cache/DataContainerImpl.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/DataContainerImpl.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/main/java/org/jboss/cache/DataContainerImpl.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -137,11 +137,6 @@ } } - public NodeSPI peekVersioned(Fqn fqn, DataVersion version) - { - return peekVersioned(fqn, version, false); - } - public NodeSPI peekStrict(GlobalTransaction gtx, Fqn fqn, boolean includeInvalid) { NodeSPI n = peekVersioned(fqn, null, includeInvalid); @@ -156,6 +151,11 @@ return n; } + public NodeSPI peekVersioned(Fqn fqn, DataVersion version) + { + return peekVersioned(fqn, version, false); + } + public NodeSPI peekVersioned(Fqn fqn, DataVersion version, boolean includeInvalidNodes) { if (fqn == null) return null; Modified: core/trunk/src/main/java/org/jboss/cache/commands/write/ClearDataCommand.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/commands/write/ClearDataCommand.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/main/java/org/jboss/cache/commands/write/ClearDataCommand.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -75,8 +75,14 @@ public void rollback() { if (trace) log.trace("rollback(" + globalTransaction + ", \"" + fqn + "\", " + originalData + ")"); - NodeSPI nodeSPI = dataContainer.peekStrict(globalTransaction, fqn, true); - nodeSPI.putAllDirect(originalData); + NodeSPI nodeSpi = dataContainer.peek(fqn, false, true); + if (nodeSpi == null) + { + if (trace) log.trace("Not rolling back node clearance for node: " + fqn + " as it does not exist in the cache. " + + "This might be the result of an NoOp clear operation"); + return; + } + nodeSpi.putAllDirect(originalData); } public Object acceptVisitor(InvocationContext ctx, Visitor visitor) throws Throwable Modified: core/trunk/src/main/java/org/jboss/cache/commands/write/MoveCommand.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/commands/write/MoveCommand.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/main/java/org/jboss/cache/commands/write/MoveCommand.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -2,11 +2,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.jboss.cache.DataContainerImpl; -import org.jboss.cache.Fqn; -import org.jboss.cache.InvocationContext; -import org.jboss.cache.NodeNotExistsException; -import org.jboss.cache.NodeSPI; +import org.jboss.cache.*; import org.jboss.cache.commands.ReversibleCommand; import org.jboss.cache.commands.Visitor; import org.jboss.cache.commands.read.AbstractDataCommand; @@ -37,7 +33,7 @@ { } - public void initialize(Notifier notifier, DataContainerImpl dataContainer) + public void initialize(Notifier notifier, DataContainer dataContainer) { this.notifier = notifier; this.dataContainer = dataContainer; @@ -77,7 +73,7 @@ } - private void moveFqns(NodeSPI node, Fqn newBase) + private void adjustFqn(NodeSPI node, Fqn newBase) { Fqn newFqn = Fqn.fromRelativeElements(newBase, node.getFqn().getLastElement()); node.setFqn(newFqn); @@ -88,27 +84,26 @@ return visitor.visitMoveCommand(ctx, this); } - private void move(Fqn nodeToMoveFqn, Fqn newParentFqn, boolean skipNotifications, InvocationContext ctx) + private void move(Fqn toMoveFqn, Fqn newParentFqn, boolean skipNotifications, InvocationContext ctx) { // the actual move algorithm. - NodeSPI newParent = dataContainer.peek(newParentFqn); - + NodeSPI newParent = dataContainer.peek(newParentFqn, false, false); if (newParent == null) { throw new NodeNotExistsException("New parent node " + newParentFqn + " does not exist when attempting to move node!!"); } - NodeSPI node = dataContainer.peek(nodeToMoveFqn); + NodeSPI node = dataContainer.peek(toMoveFqn, false, false); if (node == null) { - throw new NodeNotExistsException("Node " + nodeToMoveFqn + " does not exist when attempting to move node!!"); + throw new NodeNotExistsException("Node " + toMoveFqn + " does not exist when attempting to move node!!"); } if (trace) log.trace("Moving " + fqn + " to sit under " + to); NodeSPI oldParent = node.getParent(); - Object nodeName = nodeToMoveFqn.getLastElement(); + Object nodeName = toMoveFqn.getLastElement(); // now that we have the parent and target nodes: // first correct the pointers at the pruning point @@ -118,13 +113,13 @@ // notify if (!skipNotifications) - notifier.notifyNodeMoved(nodeToMoveFqn, Fqn.fromRelativeElements(newParentFqn, nodeToMoveFqn.getLastElement()), true, ctx); + notifier.notifyNodeMoved(toMoveFqn, Fqn.fromRelativeElements(newParentFqn, toMoveFqn.getLastElement()), true, ctx); // now adjust Fqns of node and all children. - moveFqns(node, newParent.getFqn()); + adjustFqn(node, newParent.getFqn()); if (!skipNotifications) - notifier.notifyNodeMoved(nodeToMoveFqn, Fqn.fromRelativeElements(newParentFqn, nodeToMoveFqn.getLastElement()), false, ctx); + notifier.notifyNodeMoved(toMoveFqn, Fqn.fromRelativeElements(newParentFqn, toMoveFqn.getLastElement()), false, ctx); } public Fqn getTo() Modified: core/trunk/src/main/java/org/jboss/cache/commands/write/RemoveKeyCommand.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/commands/write/RemoveKeyCommand.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/main/java/org/jboss/cache/commands/write/RemoveKeyCommand.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -77,7 +77,6 @@ public void rollback() { NodeSPI targetNode = dataContainer.peek(fqn, false, true); - if (targetNode == null) throw new NodeNotExistsException("No such node: " + fqn); if (oldValue != null) { targetNode.putDirect(key, oldValue); Modified: core/trunk/src/main/java/org/jboss/cache/commands/write/RemoveNodeCommand.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/commands/write/RemoveNodeCommand.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/main/java/org/jboss/cache/commands/write/RemoveNodeCommand.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -47,7 +47,7 @@ public Object perform(InvocationContext ctx) { if (trace) log.trace("perform(" + globalTransaction + ", \"" + fqn + ")"); - // Find the node. This will add the temporarily created parent nodes to the TX's node list if globalTransaction != null) + // Find the node targetNode = dataContainer.peekVersioned(fqn, dataVersion, true); if (targetNode == null) { Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -624,7 +624,8 @@ } catch (Throwable e) { - log.warn("Rollback had a problem", e); + log.error("Rollback had a problem", e); + throw new CacheException("A failure occured durring rollback!", e); } finally { @@ -968,11 +969,17 @@ switch (status) { case Status.STATUS_COMMITTED: - boolean onePhaseCommit = isOnePhaseCommit(); - if (log.isDebugEnabled()) log.debug("Running commit phase. One phase? " + onePhaseCommit); - runCommitPhase(ctx, gtx, modifications, onePhaseCommit); - log.debug("Finished commit phase"); - break; + try + { + boolean onePhaseCommit = isOnePhaseCommit(); + if (log.isDebugEnabled()) log.debug("Running commit phase. One phase? " + onePhaseCommit); + runCommitPhase(ctx, gtx, modifications, onePhaseCommit); + log.debug("Finished commit phase"); + break; + } catch (Exception e) + { + log.trace("Caught exception on After completion", e); + } case Status.STATUS_UNKNOWN: log.warn("Received JTA STATUS_UNKNOWN in afterCompletion()! XA resources may not be in sync. The app should manually clean up resources at this point."); case Status.STATUS_MARKED_ROLLBACK: @@ -986,11 +993,6 @@ throw new IllegalStateException("illegal status: " + status); } } - catch (Exception th) - { - log.trace("Caught exception ", th); - - } finally { // clean up the tx table Modified: core/trunk/src/main/java/org/jboss/cache/loader/AbstractCacheLoader.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/loader/AbstractCacheLoader.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/main/java/org/jboss/cache/loader/AbstractCacheLoader.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -24,10 +24,7 @@ import java.io.ObjectInputStream; import java.io.ObjectOutputStream; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; /** @@ -73,14 +70,9 @@ storeState(Fqn.ROOT, is); } + @SuppressWarnings("unchecked") public void storeState(Fqn subtree, ObjectInputStream in) throws Exception { -// ClassLoader currentCL = Thread.currentThread().getContextClassLoader(); -// try -// { -// // Set the TCCL to any classloader registered for subtree -// setUnmarshallingClassLoader(subtree); - // remove entire existing state this.remove(subtree); @@ -103,20 +95,26 @@ } List nodeData = (List) objectFromStream; - - //for (nd = (NodeData) in.readObject(); nd != null && !nd.isMarker(); nd = (NodeData) in.readObject()) - for (Object o : nodeData) + for (Object aNodeData : nodeData) { - NodeData nd = (NodeData) o; - if (nd.isMarker()) break; - + NodeData nd = (NodeData) aNodeData; if (nd.isExceptionMarker()) { NodeDataExceptionMarker ndem = (NodeDataExceptionMarker) nd; throw new CacheException("State provider cacheloader at node " + ndem.getCacheNodeIdentity() + " threw exception during loadState (see Caused by)", ndem.getCause()); } + } + storeStateHelper(subtree, nodeData, moveToBuddy); + } + protected void storeStateHelper(Fqn subtree, List nodeData, boolean moveToBuddy) throws Exception + { + for (Object aNodeData : nodeData) + { + NodeData nd = (NodeData) aNodeData; + if (nd.isMarker()) break; + Fqn fqn; if (moveToBuddy) { fqn = BuddyFqnTransformer.getBackupFqn(subtree, nd.getFqn()); @@ -135,14 +133,6 @@ this.put(fqn, null);// creates a node with null attributes } } - - // read marker off stack -// cache.getMarshaller().objectFromObjectStream(in); -// } -// finally -// { -// Thread.currentThread().setContextClassLoader(currentCL); -// } } public void loadEntireState(ObjectOutputStream os) throws Exception @@ -243,10 +233,6 @@ /** * Do a preorder traversal: visit the node first, then the node's children - * - * @param fqn Start node - * @param out - * @throws Exception */ protected void loadStateHelper(Fqn fqn, ObjectOutputStream out) throws Exception { @@ -259,9 +245,9 @@ protected void getNodeDataList(Fqn fqn, List<NodeData> list) throws Exception { Map<Object, Object> attrs; - Set<?> children_names; - String child_name; - Fqn tmp_fqn; + Set<?> childrenNames; + String childName; + Fqn tmpFqn; NodeData nd; // first handle the current node @@ -278,16 +264,16 @@ list.add(nd); // then visit the children - children_names = getChildrenNames(fqn); - if (children_names == null) + childrenNames = getChildrenNames(fqn); + if (childrenNames == null) { return; } - for (Object children_name : children_names) + for (Object childrenName : childrenNames) { - child_name = (String) children_name; - tmp_fqn = Fqn.fromRelativeElements(fqn, child_name); - if (!cache.getInternalFqns().contains(tmp_fqn)) getNodeDataList(tmp_fqn, list); + childName = (String) childrenName; + tmpFqn = Fqn.fromRelativeElements(fqn, childName); + if (!cache.getInternalFqns().contains(tmpFqn)) getNodeDataList(tmpFqn, list); } } Modified: core/trunk/src/main/java/org/jboss/cache/loader/JDBCCacheLoader.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/loader/JDBCCacheLoader.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/main/java/org/jboss/cache/loader/JDBCCacheLoader.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -4,11 +4,13 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.jboss.cache.Fqn; +import org.jboss.cache.buddyreplication.BuddyFqnTransformer; import org.jboss.cache.config.CacheLoaderConfig; import org.jboss.cache.config.CacheLoaderConfig.IndividualCacheLoaderConfig; import org.jboss.cache.marshall.NodeData; import java.io.InputStream; +import java.io.ObjectInputStream; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -78,6 +80,34 @@ _put(name, attributes); } + @Override + protected void storeStateHelper(Fqn subtree, List nodeData, boolean moveToBuddy) throws Exception + { + for (Object aNodeData : nodeData) + { + NodeData nd = (NodeData) aNodeData; + if (nd.isMarker()) break; + Fqn fqn; + if (moveToBuddy) + { + fqn = BuddyFqnTransformer.getBackupFqn(subtree, nd.getFqn()); + } + else + { + fqn = nd.getFqn(); + } + + if (nd.getAttributes() != null) + { + this.put(fqn, nd.getAttributes(), true);// creates a node with 0 or more attributes + } + else + { + this.put(fqn, null);// creates a node with null attributes + } + } + } + /** * As per interface's contrect. * Performance Note: O(1) db calls. @@ -291,6 +321,12 @@ } } + @Override + public void storeState(Fqn subtree, ObjectInputStream in) throws Exception + { + super.storeState(subtree, in); + } + public IndividualCacheLoaderConfig getConfig() { return config; Modified: core/trunk/src/main/java/org/jboss/cache/transaction/DummyTransaction.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/transaction/DummyTransaction.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/main/java/org/jboss/cache/transaction/DummyTransaction.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -102,6 +102,10 @@ status = Status.STATUS_ROLLEDBACK; notifyAfterCompletion(Status.STATUS_ROLLEDBACK); } + catch (RuntimeException re) + { + throw re; + } catch (Throwable t) { } @@ -281,10 +285,15 @@ { s.afterCompletion(status); } - catch (Throwable t) + catch (RuntimeException t) { log.error("afterCompletion() failed for " + s, t); + throw t; } + catch (Exception t) + { + log.error("afterCompletion() failed for " + s, t); + } } participants.clear(); } Added: core/trunk/src/test/java/org/jboss/cache/commands/RollbackOnNoOpTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/RollbackOnNoOpTest.java (rev 0) +++ core/trunk/src/test/java/org/jboss/cache/commands/RollbackOnNoOpTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -0,0 +1,78 @@ +package org.jboss.cache.commands; + +import org.testng.annotations.Test; +import static org.testng.AssertJUnit.*; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.AfterMethod; +import org.jboss.cache.CacheSPI; +import org.jboss.cache.DefaultCacheFactory; +import org.jboss.cache.factories.UnitTestCacheConfigurationFactory; +import org.jboss.cache.config.Configuration; + +import javax.transaction.TransactionManager; +import java.util.HashMap; + +/** + * @author Mircea.Markus(a)jboss.com + * @since 2.2 + */ +@Test(groups = "functional") +public class RollbackOnNoOpTest +{ + private CacheSPI<Object, Object> cache; + private TransactionManager txMgr; + + @BeforeMethod(alwaysRun = true) + public void setUp() + { + Configuration cacheConfig = UnitTestCacheConfigurationFactory.createConfiguration(Configuration.CacheMode.LOCAL, false); + cache = (CacheSPI<Object, Object>) new DefaultCacheFactory().createCache(cacheConfig, false); + cache.start(); + txMgr = cache.getTransactionManager(); + } + + @AfterMethod(alwaysRun = true) + public void tearDown() + { + cache.stop(); + cache.destroy(); + } + + public void testRollbackOnRemoveNodeDoesNotFail() throws Exception + { + txMgr.begin(); + cache.remove("/blah/blah", "non-exist"); + txMgr.rollback(); + } + + public void testRollbackOnClearData() throws Exception + { + txMgr.begin(); + cache.clearData("/blah/blah"); + txMgr.rollback(); + } + + public void testCreateNodeCommand() throws Exception + { + cache.put("/blah/blah", "key", "value"); + txMgr.begin(); + cache.clearData("/blah/blah"); + txMgr.rollback(); + assert cache.get("/blah/blah", "key") != null; + } + + public void testRemoveKeyCommand() throws Exception + { + txMgr.begin(); + cache.remove("/blah/blah","key"); + txMgr.rollback(); + } + + public void testRemoveNodeCommand() throws Exception + { + cache.put("/blah/blah", "key", "value"); + txMgr.begin(); + cache.removeNode("/blah"); + txMgr.rollback(); + } +} Modified: core/trunk/src/test/java/org/jboss/cache/commands/StructuralNodesOnRollbackTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/StructuralNodesOnRollbackTest.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/test/java/org/jboss/cache/commands/StructuralNodesOnRollbackTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -53,16 +53,6 @@ assert !cache.exists("/a/b"); } - public void testPutForExternalRead() throws Exception - { - assert !cache.exists("/a/b"); - txMgr.begin(); - cache.putForExternalRead(Fqn.fromString("/a/b/c"), "key", "value"); - assert cache.exists("/a/b"); - txMgr.rollback(); - assert !cache.exists("/a/b"); - } - public void testPutKeyValueCommand() throws Exception { assert !cache.exists("/a/b"); Modified: core/trunk/src/test/java/org/jboss/cache/commands/read/AbstractDataCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/read/AbstractDataCommandTest.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/test/java/org/jboss/cache/commands/read/AbstractDataCommandTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -17,13 +17,13 @@ public abstract class AbstractDataCommandTest { protected Fqn testFqn = Fqn.fromString("/testfqn"); - protected DataContainer containerMock; + protected DataContainer container; protected InvocationContext ctx; @BeforeMethod final public void setUp() { - containerMock = createMock(DataContainer.class); + container = createMock(DataContainer.class); ctx = new InvocationContext(); moreSetup(); } Modified: core/trunk/src/test/java/org/jboss/cache/commands/read/ExistsCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/read/ExistsCommandTest.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/test/java/org/jboss/cache/commands/read/ExistsCommandTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -16,18 +16,18 @@ protected void moreSetup() { command = new ExistsCommand(testFqn); - command.initialize(containerMock); + command.initialize(container); } public void testPerform() { - expect(containerMock.exists(testFqn)).andReturn(Boolean.FALSE); - replay(containerMock); + expect(container.exists(testFqn)).andReturn(Boolean.FALSE); + replay(container); assert Boolean.FALSE == command.perform(null); - reset(containerMock); + reset(container); - expect(containerMock.exists(testFqn)).andReturn(Boolean.TRUE); - replay(containerMock); + expect(container.exists(testFqn)).andReturn(Boolean.TRUE); + replay(container); assert Boolean.TRUE == command.perform(null); } } Modified: core/trunk/src/test/java/org/jboss/cache/commands/read/GetChildrenNamesCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/read/GetChildrenNamesCommandTest.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/test/java/org/jboss/cache/commands/read/GetChildrenNamesCommandTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -24,30 +24,30 @@ { nodes = new MockNodesFixture(); command = new GetChildrenNamesCommand(testFqn); - command.initialize(containerMock); + command.initialize(container); } public void testPerformNoChildren() { NodeSpiMock node = new NodeSpiMock(testFqn); - expect(containerMock.peek(testFqn)).andReturn(node); - replay(containerMock); + expect(container.peek(testFqn)).andReturn(node); + replay(container); Set result = (Set) command.perform(null); assert result.isEmpty() : "empty result expected"; } public void testPerformInexistingNode() { - expect(containerMock.peek(testFqn)).andReturn(null); - replay(containerMock); + expect(container.peek(testFqn)).andReturn(null); + replay(container); Set result = (Set) command.perform(null); assert result == null : "empty result expected"; } public void testNodeWithChildren() { - expect(containerMock.peek(testFqn)).andReturn(nodes.adfNode); - replay(containerMock); + expect(container.peek(testFqn)).andReturn(nodes.adfNode); + replay(container); Set result = (Set) command.perform(null); assert result.size() == 2; assert result.contains("h"); @@ -57,8 +57,8 @@ public void testNodeInvalidChildren() { nodes.adfgNode.markAsDeleted(true); - expect(containerMock.peek(testFqn)).andReturn(nodes.adfNode); - replay(containerMock); + expect(container.peek(testFqn)).andReturn(nodes.adfNode); + replay(container); Set result = (Set) command.perform(null); assert result.size() == 1; assert result.contains("h"); Modified: core/trunk/src/test/java/org/jboss/cache/commands/read/GetDataMapCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/read/GetDataMapCommandTest.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/test/java/org/jboss/cache/commands/read/GetDataMapCommandTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -20,13 +20,13 @@ protected void moreSetup() { command = new GetDataMapCommand(testFqn); - command.initialize(containerMock); + command.initialize(container); } public void testForNonexistentNode() { - expect(containerMock.peek(testFqn)).andReturn(null); - replay(containerMock); + expect(container.peek(testFqn)).andReturn(null); + replay(container); assert null == command.perform(null); } @@ -35,8 +35,8 @@ NodeSpiMock node = new NodeSpiMock(testFqn); node.putDirect("k1","v1"); node.putDirect("k2","v2"); - expect(containerMock.peek(testFqn)).andReturn(node); - replay(containerMock); + expect(container.peek(testFqn)).andReturn(node); + replay(container); Map result = (Map) command.perform(null); assert 2 == result.entrySet().size(); assert result.get("k1").equals("v1"); Modified: core/trunk/src/test/java/org/jboss/cache/commands/read/GetKeysCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/read/GetKeysCommandTest.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/test/java/org/jboss/cache/commands/read/GetKeysCommandTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -5,7 +5,6 @@ import static org.easymock.EasyMock.replay; import org.jboss.cache.mock.NodeSpiMock; -import java.util.Map; import java.util.Set; /** @@ -23,13 +22,13 @@ protected void moreSetup() { command = new GetKeysCommand(testFqn); - command.initialize(containerMock); + command.initialize(container); } public void testForNonexistentNode() { - expect(containerMock.peek(testFqn)).andReturn(null); - replay(containerMock); + expect(container.peek(testFqn)).andReturn(null); + replay(container); assert null == command.perform(null); } @@ -38,8 +37,8 @@ NodeSpiMock node = new NodeSpiMock(testFqn); node.putDirect("k1", "v1"); node.putDirect("k2", "v2"); - expect(containerMock.peek(testFqn)).andReturn(node); - replay(containerMock); + expect(container.peek(testFqn)).andReturn(node); + replay(container); Set result = (Set) command.perform(null); assert 2 == result.size(); assert result.contains("k1"); Modified: core/trunk/src/test/java/org/jboss/cache/commands/write/ClearDataCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/write/ClearDataCommandTest.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/test/java/org/jboss/cache/commands/write/ClearDataCommandTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -46,10 +46,26 @@ //now do a rollback control.reset(); - expect(container.peekStrict(globalTransaction, fqn, true)).andReturn(nodes.aNode); + expect(container.peek(fqn, false, true)).andReturn(nodes.aNode); control.replay(); command.rollback(); assert nodes.aNode.dataSize() == 1; assert nodes.aNode.getData().get("key").equals("value"); } + + /** + * If clearing data on an inexistent node, the rollback should not fail + */ + public void testNoOpRollback() + { + expect(container.peek(fqn, false, true)).andReturn(null); + control.replay(); + try + { + command.rollback(); + } catch (Exception e) + { + assert false : "should not fail but expect this scenarion"; + } + } } Modified: core/trunk/src/test/java/org/jboss/cache/commands/write/CreateNodeCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/write/CreateNodeCommandTest.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/test/java/org/jboss/cache/commands/write/CreateNodeCommandTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -3,7 +3,6 @@ import static org.easymock.EasyMock.*; import org.testng.annotations.Test; -import org.testng.annotations.BeforeMethod; import org.jboss.cache.commands.read.AbstractDataCommandTest; import org.jboss.cache.mock.MockNodesFixture; @@ -25,7 +24,7 @@ protected void moreSetup() { command = new CreateNodeCommand(testFqn); - command.initialize(containerMock); + command.initialize(container); createdNodes = new ArrayList(); result = new Object[2]; result[0] = this.createdNodes; @@ -33,8 +32,8 @@ public void testPerformNoNodesCreated() { - expect(containerMock.createNodes(testFqn)).andReturn(result); - replay(containerMock); + expect(container.createNodes(testFqn)).andReturn(result); + replay(container); assert null == command.perform(ctx); assert command.getNewlyCreated().isEmpty(); } @@ -47,8 +46,8 @@ createdNodes.add(nodes.abcNode); result[1] = nodes.abcNode; - expect(containerMock.createNodes(testFqn)).andReturn(result); - replay(containerMock); + expect(container.createNodes(testFqn)).andReturn(result); + replay(container); assert nodes.abcNode == command.perform(ctx); assert command.getNewlyCreated().size() == 3; assert command.getNewlyCreated().contains(nodes.a); @@ -62,14 +61,14 @@ createdNodes.add(nodes.aNode); createdNodes.add(nodes.abNode); createdNodes.add(nodes.abcNode); - expect(containerMock.createNodes(testFqn)).andReturn(result); - expect(containerMock.removeFromDataStructure(nodes.a, true)).andReturn(Boolean.TRUE); - expect(containerMock.removeFromDataStructure(nodes.ab, true)).andReturn(Boolean.TRUE); - expect(containerMock.removeFromDataStructure(nodes.abc, true)).andReturn(Boolean.TRUE); - replay(containerMock); + expect(container.createNodes(testFqn)).andReturn(result); + expect(container.removeFromDataStructure(nodes.a, true)).andReturn(Boolean.TRUE); + expect(container.removeFromDataStructure(nodes.ab, true)).andReturn(Boolean.TRUE); + expect(container.removeFromDataStructure(nodes.abc, true)).andReturn(Boolean.TRUE); + replay(container); command.perform(ctx); command.rollback(); - verify(containerMock); + verify(container); } } Modified: core/trunk/src/test/java/org/jboss/cache/commands/write/EvictCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/write/EvictCommandTest.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/test/java/org/jboss/cache/commands/write/EvictCommandTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -30,16 +30,16 @@ { control = createStrictControl(); notifier = control.createMock(Notifier.class); - containerMock = control.createMock(DataContainer.class); + container = control.createMock(DataContainer.class); command = new EvictCommand(testFqn); - command.initialize(notifier, containerMock); + command.initialize(notifier, container); nodes = new MockNodesFixture(); } public void testResidentNodesEviction() { nodes.abNode.setResident(true); - expect(containerMock.peek(testFqn, false,false)).andReturn(nodes.abNode); + expect(container.peek(testFqn, false,false)).andReturn(nodes.abNode); control.replay(); assert Boolean.TRUE == command.perform(ctx); control.verify(); @@ -47,9 +47,9 @@ public void testSimpleEviction() { - expect(containerMock.peek(testFqn, false,false)).andReturn(nodes.abNode); + expect(container.peek(testFqn, false,false)).andReturn(nodes.abNode); notifier.notifyNodeEvicted(testFqn, true, ctx); - expect(containerMock.evict(testFqn)).andReturn(true); + expect(container.evict(testFqn)).andReturn(true); notifier.notifyNodeEvicted(testFqn, false, ctx); control.replay(); assert Boolean.TRUE == command.perform(ctx); @@ -62,18 +62,18 @@ nodesToEvict.add(nodes.a); nodesToEvict.add(nodes.ab); command.setRecursive(true); - expect(containerMock.peek(testFqn, false,false)).andReturn(nodes.aNode); + expect(container.peek(testFqn, false,false)).andReturn(nodes.aNode); - expect(containerMock.getNodesForEviction(testFqn, true)).andReturn(nodesToEvict); + expect(container.getNodesForEviction(testFqn, true)).andReturn(nodesToEvict); control.checkOrder(false); //evict a notifier.notifyNodeEvicted(nodes.a, true, ctx); - expect(containerMock.evict(nodes.a)).andReturn(true); + expect(container.evict(nodes.a)).andReturn(true); notifier.notifyNodeEvicted(nodes.a, false, ctx); //evict b notifier.notifyNodeEvicted(nodes.ab, true, ctx); - expect(containerMock.evict(nodes.ab)).andReturn(true); + expect(container.evict(nodes.ab)).andReturn(true); notifier.notifyNodeEvicted(nodes.ab, false, ctx); control.replay(); Modified: core/trunk/src/test/java/org/jboss/cache/commands/write/InvalidateCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/write/InvalidateCommandTest.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/test/java/org/jboss/cache/commands/write/InvalidateCommandTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -33,12 +33,12 @@ { control = createStrictControl(); notifier = control.createMock(Notifier.class); - containerMock = control.createMock(DataContainer.class); + container = control.createMock(DataContainer.class); tmMock = control.createMock(TransactionManager.class); spiMock = control.createMock(CacheSPI.class); command = new InvalidateCommand(testFqn); - command.initialize(spiMock, containerMock, notifier); + command.initialize(spiMock, container, notifier); nodes = new MockNodesFixture(); } @@ -54,7 +54,7 @@ { expect(spiMock.getNode(testFqn)).andReturn(nodes.adfNode); notifier.notifyNodeEvicted(testFqn, true, ctx); - expect(containerMock.evict(testFqn)).andReturn(Boolean.TRUE); + expect(container.evict(testFqn)).andReturn(Boolean.TRUE); notifier.notifyNodeEvicted(testFqn, false, ctx); control.replay(); assert null == command.perform(ctx); @@ -70,7 +70,7 @@ nodes.adfgNode.put("key","value"); expect(spiMock.getNode(Fqn.ROOT)).andReturn(nodes.root); notifier.notifyNodeEvicted(Fqn.ROOT, true, ctx); - expect(containerMock.evict(Fqn.ROOT)).andReturn(Boolean.TRUE); + expect(container.evict(Fqn.ROOT)).andReturn(Boolean.TRUE); notifier.notifyNodeEvicted(Fqn.ROOT, false, ctx); control.replay(); assert null == command.perform(ctx); Added: core/trunk/src/test/java/org/jboss/cache/commands/write/MoveCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/write/MoveCommandTest.java (rev 0) +++ core/trunk/src/test/java/org/jboss/cache/commands/write/MoveCommandTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -0,0 +1,72 @@ +package org.jboss.cache.commands.write; + +import static org.easymock.EasyMock.*; +import org.jboss.cache.notifications.Notifier; +import org.jboss.cache.mock.MockNodesFixture; +import org.jboss.cache.commands.read.AbstractDataCommandTest; +import org.jboss.cache.DataContainer; +import org.jboss.cache.Fqn; +import org.jboss.cache.NodeNotExistsException; +import org.easymock.IMocksControl; +import static org.easymock.EasyMock.createStrictControl; +import org.testng.annotations.Test; + +/** + * Tester class for {@link org.jboss.cache.commands.write.MoveCommand}. + * + * @author Mircea.Markus(a)jboss.com + * @since 2.2 + */ +@Test(groups = "unit") +public class MoveCommandTest extends AbstractDataCommandTest +{ + MoveCommand command; + Notifier notifier; + IMocksControl control; + MockNodesFixture nodes; + + Fqn source = Fqn.fromString("/source"); + Fqn destination = Fqn.fromString("/destination"); + + protected void moreSetup() + { + control = createStrictControl(); + notifier = control.createMock(Notifier.class); + container = control.createMock(DataContainer.class); + command = new MoveCommand(source, destination); + command.initialize(notifier, container); + nodes = new MockNodesFixture(); + } + + public void testFailsOnMissingSource() + { + control.checkOrder(false); + expect(container.peek(source, false, false)).andReturn(null); + expect(container.peek(destination, false, false)).andReturn(nodes.adfgNode); + control.replay(); + try + { + command.perform(ctx); + assert false : "should have thrown an exception as the source is null"; + } catch (NodeNotExistsException e) + { + //expected + } + } + + public void testFailsOnMissingDestination() + { + control.checkOrder(false); + expect(container.peek(source, false, false)).andReturn(nodes.adfgNode); + expect(container.peek(destination, false, false)).andReturn(null); + control.replay(); + try + { + command.perform(ctx); + assert false : "should have thrown an exception as the source is null"; + } catch (NodeNotExistsException e) + { + //expected + } + } +} Modified: core/trunk/src/test/java/org/jboss/cache/commands/write/OptimisticInvalidateCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/write/OptimisticInvalidateCommandTest.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/test/java/org/jboss/cache/commands/write/OptimisticInvalidateCommandTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -38,7 +38,7 @@ { control = createStrictControl(); notifier = control.createMock(Notifier.class); - containerMock = control.createMock(DataContainer.class); + container = control.createMock(DataContainer.class); tmMock = control.createMock(TransactionManager.class); spiMock = control.createMock(CacheSPI.class); nodes = new MockNodesFixture(); @@ -46,7 +46,7 @@ command = new OptimisticInvalidateCommand(testFqn); dataVersion = new DefaultDataVersion(10); command.setDataVersion(dataVersion); - command.initialize(spiMock, containerMock, notifier); + command.initialize(spiMock, container, notifier); command.initialize(tmMock); } @@ -55,10 +55,10 @@ nodes.adfNode.put("key", "value"); nodes.adfNode.setDataLoaded(true); expect(spiMock.getNode(testFqn)).andReturn(nodes.adfNode); - expect(containerMock.peekVersioned(testFqn, dataVersion)).andReturn(nodes.adfNode); + expect(container.peekVersioned(testFqn, dataVersion)).andReturn(nodes.adfNode); notifier.notifyNodeEvicted(testFqn, true, ctx); notifier.notifyNodeEvicted(testFqn, false, ctx); - expect(containerMock.peek(testFqn, false, true)).andReturn(nodes.adfNode); + expect(container.peek(testFqn, false, true)).andReturn(nodes.adfNode); control.replay(); assert null == command.perform(ctx); @@ -75,7 +75,7 @@ nodes.adfNode.put("key", "value"); nodes.adfNode.setDataLoaded(true); expect(spiMock.getNode(testFqn)).andReturn(nodes.adfNode); - expect(containerMock.peekVersioned(testFqn, dataVersion)).andThrow(new RuntimeException()); + expect(container.peekVersioned(testFqn, dataVersion)).andThrow(new RuntimeException()); control.replay(); try @@ -98,11 +98,11 @@ { nodes.adfNode.setValid(false, true); expect(spiMock.getNode(testFqn)).andReturn(null); - expect(containerMock.peek(testFqn, false, true)).andReturn(nodes.adfNode); - expect(containerMock.peekVersioned(testFqn, dataVersion)).andReturn(nodes.adfNode); + expect(container.peek(testFqn, false, true)).andReturn(nodes.adfNode); + expect(container.peekVersioned(testFqn, dataVersion)).andReturn(nodes.adfNode); notifier.notifyNodeEvicted(testFqn, true, ctx); notifier.notifyNodeEvicted(testFqn, false, ctx); - expect(containerMock.peek(testFqn, false, true)).andReturn(nodes.adfNode); + expect(container.peek(testFqn, false, true)).andReturn(nodes.adfNode); control.replay(); assert null == command.perform(ctx); Modified: core/trunk/src/test/java/org/jboss/cache/commands/write/RemoveKeyCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/write/RemoveKeyCommandTest.java 2008-06-11 13:22:08 UTC (rev 5973) +++ core/trunk/src/test/java/org/jboss/cache/commands/write/RemoveKeyCommandTest.java 2008-06-11 14:20:32 UTC (rev 5974) @@ -85,4 +85,21 @@ assert "newValue".equals(nodes.adfgNode.getData().get(key)); control.verify(); } + + /** + * On an no-op scenario the user will try to remove a key on an unexisting node. + * When rollback is being called, the node might not exist in the cache and we should know how to handle that. + */ + public void testRollbackOnNoOp() + { + expect(container.peek(fqn, false, true)).andReturn(null); + control.replay(); + try + { + command.rollback(); + } catch (Exception ex) + { + assert false : "No exception should be thrown here."; + } + } }