Author: manik.surtani(a)jboss.com Date: 2007-10-23 04:53:16 -0400 (Tue, 23 Oct 2007) New Revision: 4667 Modified: core/trunk/src/main/java/org/jboss/cache/CacheImpl.java core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java core/trunk/src/test/java/org/jboss/cache/api/NodeReplicatedMoveOptimisticTest.java core/trunk/src/test/java/org/jboss/cache/api/NodeReplicatedMoveTest.java core/trunk/src/test/java/org/jboss/cache/options/ForceCacheModeTest.java Log: Fixed missing notifications on invalidation, added a null check and patched some tests Modified: core/trunk/src/main/java/org/jboss/cache/CacheImpl.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/CacheImpl.java 2007-10-23 01:40:26 UTC (rev 4666) +++ core/trunk/src/main/java/org/jboss/cache/CacheImpl.java 2007-10-23 08:53:16 UTC (rev 4667) @@ -2734,25 +2734,8 @@ { Node<K, V> node = get(fqn); // force interceptor chain, load if necessary from cache loader. - if (node != null) + if (node == null) { - // mark the node to be removed (and all children) as invalid so anyone holding a direct reference to it will - // be aware that it is no longer valid. - ((NodeSPI) node).setValid(false, true); - - if (configuration.isNodeLockingOptimistic()) - _removeData(null, fqn, false, false, true, versionToInvalidate); - else - _evict(fqn); - - if (versionToInvalidate != null) - { - NodeSPI n = peek(fqn, false, true); - n.setVersion(versionToInvalidate); - } - } - else - { // if pessimistic locking, just return. if (!configuration.isNodeLockingOptimistic()) return; @@ -2784,12 +2767,23 @@ } nodeSPI = (NodeSPI) root.getChild(fqn); } - log.trace("Retrieved node. Setting version to " + versionToInvalidate + " and marking as invalid"); - nodeSPI.setVersion(versionToInvalidate); - // mark the node to be removed (and all children) as invalid so anyone holding a direct reference to it will - // be aware that it is no longer valid. - nodeSPI.setValid(false, true); + node = nodeSPI; } + + if (configuration.isNodeLockingOptimistic()) + _removeData(null, fqn, false, false, true, versionToInvalidate); + else + _evict(fqn); + + // mark the node to be removed (and all children) as invalid so anyone holding a direct reference to it will + // be aware that it is no longer valid. + ((NodeSPI) node).setValid(false, true); + + if (versionToInvalidate != null) + { + NodeSPI n = peek(fqn, false, true); + n.setVersion(versionToInvalidate); + } } /** Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java 2007-10-23 01:40:26 UTC (rev 4666) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java 2007-10-23 08:53:16 UTC (rev 4667) @@ -229,7 +229,8 @@ for (Fqn child : deltas.get(1)) { // mark it as invalid so any direct references are marked as such - underlyingNode.getChildDirect(child.getLastElement()).setValid(false, true); + NodeSPI childNode = underlyingNode.getChildDirect(child.getLastElement()); + if (childNode != null) childNode.setValid(false, true); if (!useTombstones) { Modified: core/trunk/src/test/java/org/jboss/cache/api/NodeReplicatedMoveOptimisticTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/NodeReplicatedMoveOptimisticTest.java 2007-10-23 01:40:26 UTC (rev 4666) +++ core/trunk/src/test/java/org/jboss/cache/api/NodeReplicatedMoveOptimisticTest.java 2007-10-23 08:53:16 UTC (rev 4667) @@ -6,6 +6,9 @@ */ package org.jboss.cache.api; +import org.testng.annotations.Test; + +@Test(groups = {"functional", "jgroups"}) public class NodeReplicatedMoveOptimisticTest extends NodeReplicatedMoveTest { public NodeReplicatedMoveOptimisticTest() Modified: core/trunk/src/test/java/org/jboss/cache/api/NodeReplicatedMoveTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/NodeReplicatedMoveTest.java 2007-10-23 01:40:26 UTC (rev 4666) +++ core/trunk/src/test/java/org/jboss/cache/api/NodeReplicatedMoveTest.java 2007-10-23 08:53:16 UTC (rev 4667) @@ -6,20 +6,23 @@ */ package org.jboss.cache.api; +import org.jboss.cache.Cache; +import org.jboss.cache.CacheImpl; import org.jboss.cache.CacheSPI; import org.jboss.cache.DefaultCacheFactory; import org.jboss.cache.Fqn; import org.jboss.cache.Node; +import org.jboss.cache.NodeSPI; import org.jboss.cache.config.Configuration; import org.jboss.cache.config.Configuration.CacheMode; import org.jboss.cache.factories.UnitTestCacheConfigurationFactory; -import static org.testng.AssertJUnit.*; +import static org.testng.AssertJUnit.assertEquals; +import static org.testng.AssertJUnit.assertNull; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import javax.transaction.TransactionManager; -import java.util.Collections; @Test(groups = {"functional", "jgroups"}) public class NodeReplicatedMoveTest @@ -107,8 +110,8 @@ assertEquals(vA, cache1.getRoot().getChild(A).get(k)); assertEquals(vB, cache1.getRoot().getChild(A).getChild(B).get(k)); - assert cache2.getRoot().getChild(A) != null : "Should not be null"; - assert cache2.getRoot().getChild(A).getChild(B) != null : "Should not be null"; + assertInvalidated(cache2, A, "Should be invalidated"); + assertInvalidated(cache2, new Fqn(A, B.getLastElement()), "Should be invalidated"); // now move... cache1.move(nodeB.getFqn(), Fqn.ROOT); @@ -116,16 +119,24 @@ assertEquals(vA, cache1.getRoot().getChild(A).get(k)); assertEquals(vB, cache1.getRoot().getChild(B).get(k)); - assert cache2.getRoot().getChild(A) != null : "Should not be null"; - assert cache2.getRoot().getChild(B) != null : "Should not be null"; + assertInvalidated(cache2, A, "Should be invalidated"); + assertInvalidated(cache2, B, "Should be invalidated"); // now make sure a node exists on cache 2 - cache2.getRoot().getChild(A).put("k2", "v2"); + cache2.getRoot().addChild(A).put("k2", "v2"); // te invalidation will happen in afterCompletion, hence no exception! - cache1.move(cache1.getRoot().getChild(B).getFqn(), cache1.getRoot().getChild(A).getFqn());// should throw an NPE + cache1.move(B, A);// should throw an NPE } + private void assertInvalidated(Cache cache, Fqn fqn, String msg) + { + assert cache.getRoot().getChild(fqn) == null : msg; + NodeSPI n = ((CacheImpl) cache).peek(fqn, true, true); + assert n != null : msg; + assert !n.isValid() : msg; + } + public void testReplTxCommit() throws Exception { Fqn A_B = new Fqn<String>(A, B); Modified: core/trunk/src/test/java/org/jboss/cache/options/ForceCacheModeTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/options/ForceCacheModeTest.java 2007-10-23 01:40:26 UTC (rev 4666) +++ core/trunk/src/test/java/org/jboss/cache/options/ForceCacheModeTest.java 2007-10-23 08:53:16 UTC (rev 4667) @@ -6,15 +6,6 @@ */ package org.jboss.cache.options; -import static org.testng.AssertJUnit.assertEquals; -import static org.testng.AssertJUnit.assertFalse; -import static org.testng.AssertJUnit.assertNull; -import static org.testng.AssertJUnit.assertTrue; - -import java.util.concurrent.CountDownLatch; - -import javax.transaction.TransactionManager; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.jboss.cache.CacheException; @@ -22,22 +13,23 @@ import org.jboss.cache.DefaultCacheFactory; import org.jboss.cache.Fqn; import org.jboss.cache.config.Configuration; -import org.jboss.cache.config.Option; import org.jboss.cache.config.Configuration.CacheMode; import org.jboss.cache.config.Configuration.NodeLockingScheme; +import org.jboss.cache.config.Option; import org.jboss.cache.misc.TestingUtil; import org.jboss.cache.notifications.annotation.CacheListener; import org.jboss.cache.notifications.annotation.NodeEvicted; import org.jboss.cache.notifications.annotation.NodeModified; import org.jboss.cache.notifications.annotation.NodeRemoved; import org.jboss.cache.notifications.event.NodeEvent; -import org.jboss.cache.notifications.event.NodeEvictedEvent; -import org.jboss.cache.notifications.event.NodeModifiedEvent; -import org.jboss.cache.notifications.event.NodeRemovedEvent; +import static org.testng.AssertJUnit.*; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; +import javax.transaction.TransactionManager; +import java.util.concurrent.CountDownLatch; + /** * Tests functionality of {@link Option#setForceAsynchronous(boolean)} and * {@link Option#setForceSynchronous(boolean)}. @@ -464,25 +456,11 @@ boolean blocked; @NodeModified - public void nodeModified(NodeModifiedEvent event) - { - block(event); - } - @NodeRemoved - public void nodeRemoved(NodeRemovedEvent event) - { - block(event); - } - @NodeEvicted - public void nodeEvicted(NodeEvictedEvent event) + public void block(NodeEvent event) { - block(event); - } - - private void block(NodeEvent event) - { + log.warn("Received event notification " + event); if (event.isPre() == false && FQNA.equals(event.getFqn())) { blocked = true;