[jbosscache-commits] JBoss Cache SVN: r7728 - in core/branches/3.0.X/src: test/java/org/jboss/cache/api/mvcc and 1 other directory.

Author: manik.surtani(a)jboss.com Date: 2009-02-19 07:47:56 -0500 (Thu, 19 Feb 2009) New Revision: 7728 Added: core/branches/3.0.X/src/test/java/org/jboss/cache/api/mvcc/GetChildrenNamesAfterRemoveTest.java Modified: core/branches/3.0.X/src/main/java/org/jboss/cache/invocation/CacheInvocationDelegate.java Log: JBCACHE-1480 - getChildrenNames retrieves removed children in a transaction Modified: core/branches/3.0.X/src/main/java/org/jboss/cache/invocation/CacheInvocationDelegate.java =================================================================== --- core/branches/3.0.X/src/main/java/org/jboss/cache/invocation/CacheInvocationDelegate.java 2009-02-19 12:03:50 UTC (rev 7727) +++ core/branches/3.0.X/src/main/java/org/jboss/cache/invocation/CacheInvocationDelegate.java 2009-02-19 12:47:56 UTC (rev 7728) @@ -64,20 +64,20 @@ import javax.transaction.Transaction; import javax.transaction.TransactionManager; import java.util.Collections; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; /** - * The delegate that users (and ChainedInterceptor authors) interact with when they create a cache by using a cache factory. - * This wrapper delegates calls down the interceptor chain. + * The delegate that users (and ChainedInterceptor authors) interact with when they create a cache by using a cache + * factory. This wrapper delegates calls down the interceptor chain. * * @author Manik Surtani (<a href="mailto:manik AT jboss DOT org">manik AT jboss DOT org</a>) * @since 2.1.0 */ @NonVolatile -public class CacheInvocationDelegate<K, V> extends AbstractInvocationDelegate implements CacheSPI<K, V> -{ +public class CacheInvocationDelegate<K, V> extends AbstractInvocationDelegate implements CacheSPI<K, V> { private static final Log log = LogFactory.getLog(CacheInvocationDelegate.class); // this stuff is needed since the SPI has methods to retrieve these. @@ -100,8 +100,7 @@ public void initialize(StateTransferManager stateTransferManager, CacheLoaderManager cacheLoaderManager, Notifier notifier, TransactionManager transactionManager, BuddyManager buddyManager, TransactionTable transactionTable, RPCManager rpcManager, RegionManager regionManager, Marshaller marshaller, - CommandsFactory commandsFactory, DataContainer dataContainer, MVCCNodeHelper mvccHelper, BatchContainer batchContainer) - { + CommandsFactory commandsFactory, DataContainer dataContainer, MVCCNodeHelper mvccHelper, BatchContainer batchContainer) { this.stateTransferManager = stateTransferManager; this.cacheLoaderManager = cacheLoaderManager; this.notifier = notifier; @@ -118,13 +117,11 @@ } @Start - void setNodeLockingScheme() - { + void setNodeLockingScheme() { usingMvcc = configuration.getNodeLockingScheme() == NodeLockingScheme.MVCC; } - private void reset() - { + private void reset() { this.usingMvcc = false; this.stateTransferManager = null; this.cacheLoaderManager = null; @@ -138,156 +135,125 @@ } @Override - public String toString() - { + public String toString() { return dataContainer == null ? super.toString() : dataContainer.toString(); } - public Configuration getConfiguration() - { + public Configuration getConfiguration() { return configuration; } - public NodeSPI<K, V> getRoot() - { + public NodeSPI<K, V> getRoot() { return getNode(Fqn.ROOT); } - public TransactionManager getTransactionManager() - { + public TransactionManager getTransactionManager() { return transactionManager; } - public void addInterceptor(CommandInterceptor i, int position) - { + public void addInterceptor(CommandInterceptor i, int position) { invoker.addInterceptor(i, position); } - public void addInterceptor(CommandInterceptor i, Class<? extends CommandInterceptor> afterInterceptor) - { + public void addInterceptor(CommandInterceptor i, Class<? extends CommandInterceptor> afterInterceptor) { invoker.addAfterInterceptor(i, afterInterceptor); } - public List<CommandInterceptor> getInterceptorChain() - { + public List<CommandInterceptor> getInterceptorChain() { return invoker.asList(); } - public void removeInterceptor(int position) - { + public void removeInterceptor(int position) { invoker.removeInterceptor(position); } - public void removeInterceptor(Class<? extends CommandInterceptor> interceptorType) - { + public void removeInterceptor(Class<? extends CommandInterceptor> interceptorType) { invoker.removeInterceptor(interceptorType); } - public CacheLoaderManager getCacheLoaderManager() - { + public CacheLoaderManager getCacheLoaderManager() { return cacheLoaderManager; } - public BuddyManager getBuddyManager() - { + public BuddyManager getBuddyManager() { return buddyManager; } - public TransactionTable getTransactionTable() - { + public TransactionTable getTransactionTable() { return transactionTable; } - public RPCManager getRPCManager() - { + public RPCManager getRPCManager() { return rpcManager; } - public StateTransferManager getStateTransferManager() - { + public StateTransferManager getStateTransferManager() { return stateTransferManager; } - public String getClusterName() - { + public String getClusterName() { return configuration.getClusterName(); } - public int getNumberOfAttributes() - { + public int getNumberOfAttributes() { return dataContainer.getNumberOfAttributes(); } - public int getNumberOfNodes() - { + public int getNumberOfNodes() { return dataContainer.getNumberOfNodes(); } - public RegionManager getRegionManager() - { + public RegionManager getRegionManager() { return regionManager; } - public GlobalTransaction getCurrentTransaction(Transaction tx, boolean createIfNotExists) - { + public GlobalTransaction getCurrentTransaction(Transaction tx, boolean createIfNotExists) { return transactionTable.getCurrentTransaction(tx, createIfNotExists); } - public GlobalTransaction getCurrentTransaction() - { + public GlobalTransaction getCurrentTransaction() { return transactionTable.getCurrentTransaction(); } - public Set<Fqn> getInternalFqns() - { + public Set<Fqn> getInternalFqns() { return dataContainer.getInternalFqns(); } - public int getNumberOfLocksHeld() - { + public int getNumberOfLocksHeld() { return dataContainer.getNumberOfLocksHeld(); } - public boolean exists(String fqn) - { + public boolean exists(String fqn) { return exists(Fqn.fromString(fqn)); } - public boolean exists(Fqn fqn) - { - if (usingMvcc) - { + public boolean exists(Fqn fqn) { + if (usingMvcc) { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); ExistsCommand command = commandsFactory.buildExistsNodeCommand(fqn); return (Boolean) invoker.invoke(ctx, command); - } - else - { + } else { return peek(fqn, false) != null; } } - public Notifier getNotifier() - { + public Notifier getNotifier() { return notifier; } - public Marshaller getMarshaller() - { + public Marshaller getMarshaller() { return marshaller; } - public GravitateResult gravitateData(Fqn fqn, boolean searchBuddyBackupSubtrees, InvocationContext ctx) - { + public GravitateResult gravitateData(Fqn fqn, boolean searchBuddyBackupSubtrees, InvocationContext ctx) { cacheStatusCheck(ctx); GravitateDataCommand command = commandsFactory.buildGravitateDataCommand(fqn, searchBuddyBackupSubtrees); return (GravitateResult) invoker.invoke(ctx, command); } @SuppressWarnings("unchecked") - public NodeSPI<K, V> peek(Fqn fqn, boolean includeDeletedNodes, boolean includeInvalidNodes) - { + public NodeSPI<K, V> peek(Fqn fqn, boolean includeDeletedNodes, boolean includeInvalidNodes) { // TODO: clean this up somehow! Anyway, this method should NOT be used outside of testing frameworks. return (usingMvcc) ? mvccPeek(fqn) @@ -295,8 +261,7 @@ } @SuppressWarnings("unchecked") - public NodeSPI<K, V> peek(Fqn fqn, boolean includeDeletedNodes) - { + public NodeSPI<K, V> peek(Fqn fqn, boolean includeDeletedNodes) { // TODO: clean this up somehow! Anyway, this method should NOT be used outside of testing frameworks. return (usingMvcc) ? mvccPeek(fqn) @@ -304,124 +269,100 @@ } @SuppressWarnings("unchecked") - private NodeSPI<K, V> mvccPeek(Fqn f) - { + private NodeSPI<K, V> mvccPeek(Fqn f) { NodeSPI<K, V> n; - try - { + try { n = mvccHelper.wrapNodeForReading(getInvocationContext(), f, false); } - catch (InterruptedException e) - { + catch (InterruptedException e) { throw new CacheException(e); } if (n == null || n.isNullNode()) return null; return n; } - public void addCacheListener(Object listener) - { + public void addCacheListener(Object listener) { notifier.addCacheListener(listener); } - public void removeCacheListener(Object listener) - { + public void removeCacheListener(Object listener) { notifier.removeCacheListener(listener); } - public Set<Object> getCacheListeners() - { + public Set<Object> getCacheListeners() { return notifier.getCacheListeners(); } - public void create() throws CacheException - { + public void create() throws CacheException { componentRegistry.create(); } - public void start() throws CacheException - { + public void start() throws CacheException { componentRegistry.start(); } - public void stop() - { + public void stop() { componentRegistry.stop(); } - public void destroy() - { + public void destroy() { reset(); componentRegistry.destroy(); } - public CacheStatus getCacheStatus() - { + public CacheStatus getCacheStatus() { return componentRegistry.getState(); } - public InvocationContext getInvocationContext() - { + public InvocationContext getInvocationContext() { assertIsConstructed(); return invocationContextContainer.get(); } - public void setInvocationContext(InvocationContext ctx) - { + public void setInvocationContext(InvocationContext ctx) { assertIsConstructed(); // assume a null ctx is meant to "un-set" the context? - if (ctx == null) - { + if (ctx == null) { invocationContextContainer.remove(); - } - else - { + } else { invocationContextContainer.set(ctx); } } - public Address getLocalAddress() - { + public Address getLocalAddress() { if (rpcManager == null) return null; return rpcManager.getLocalAddress(); } - public List<Address> getMembers() - { + public List<Address> getMembers() { if (rpcManager == null) return null; return rpcManager.getMembers(); } - public String getVersion() - { + public String getVersion() { return Version.printVersion(); } - public void move(Fqn nodeToMove, Fqn newParent) throws NodeNotExistsException - { + public void move(Fqn nodeToMove, Fqn newParent) throws NodeNotExistsException { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); MoveCommand command = commandsFactory.buildMoveCommand(nodeToMove, newParent); invoker.invoke(ctx, command); } - public void move(String nodeToMove, String newParent) throws NodeNotExistsException - { + public void move(String nodeToMove, String newParent) throws NodeNotExistsException { move(Fqn.fromString(nodeToMove), Fqn.fromString(newParent)); } - public boolean removeRegion(Fqn fqn) - { + public boolean removeRegion(Fqn fqn) { return regionManager.removeRegion(fqn); } - public Region getRegion(Fqn fqn, boolean createIfAbsent) - { + public Region getRegion(Fqn fqn, boolean createIfAbsent) { return regionManager.getRegion(fqn, createIfAbsent); } - public void evict(Fqn fqn, boolean recursive) - { + public void evict(Fqn fqn, boolean recursive) { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); EvictCommand c = commandsFactory.buildEvictFqnCommand(fqn); @@ -429,47 +370,38 @@ invoker.invoke(ctx, c); } - public void evict(Fqn fqn) - { + public void evict(Fqn fqn) { evict(fqn, false); } @SuppressWarnings("unchecked") - public V get(Fqn fqn, K key) - { + public V get(Fqn fqn, K key) { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); GetKeyValueCommand command = commandsFactory.buildGetKeyValueCommand(fqn, key, true); return (V) invoker.invoke(ctx, command); } - public V get(String fqn, K key) - { + public V get(String fqn, K key) { return get(Fqn.fromString(fqn), key); } - public boolean removeNode(Fqn fqn) - { + public boolean removeNode(Fqn fqn) { // special case if we are removing the root. Remove all children instead. - if (fqn.isRoot()) - { + if (fqn.isRoot()) { boolean result = true; // we need to preserve options InvocationContext ctx = getInvocationContext(); Option o = ctx.getOptionOverrides(); Set<Fqn> internalFqns = getInternalFqns(); - for (Object childName : peek(fqn, false, false).getChildrenNames()) - { - if (!internalFqns.contains(Fqn.fromElements(childName))) - { + for (Object childName : peek(fqn, false, false).getChildrenNames()) { + if (!internalFqns.contains(Fqn.fromElements(childName))) { ctx.setOptionOverrides(o); result = removeNode(Fqn.fromRelativeElements(fqn, childName)) && result; } } return result; - } - else - { + } else { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); GlobalTransaction tx = transactionTable.getCurrentTransaction(); @@ -479,28 +411,24 @@ } } - public boolean removeNode(String fqn) - { + public boolean removeNode(String fqn) { return removeNode(Fqn.fromString(fqn)); } @SuppressWarnings("unchecked") - public NodeSPI<K, V> getNode(Fqn fqn) - { + public NodeSPI<K, V> getNode(Fqn fqn) { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); GetNodeCommand command = commandsFactory.buildGetNodeCommand(fqn); return (NodeSPI<K, V>) invoker.invoke(ctx, command); } - public NodeSPI<K, V> getNode(String fqn) - { + public NodeSPI<K, V> getNode(String fqn) { return getNode(Fqn.fromString(fqn)); } @SuppressWarnings("unchecked") - public V remove(Fqn fqn, K key) throws CacheException - { + public V remove(Fqn fqn, K key) throws CacheException { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); GlobalTransaction tx = transactionTable.getCurrentTransaction(); @@ -508,45 +436,36 @@ return (V) invoker.invoke(ctx, command); } - public V remove(String fqn, K key) - { + public V remove(String fqn, K key) { return remove(Fqn.fromString(fqn), key); } - public void put(Fqn fqn, Map<? extends K, ? extends V> data) - { + public void put(Fqn fqn, Map<? extends K, ? extends V> data) { invokePut(fqn, data, false); } - public void put(String fqn, Map<? extends K, ? extends V> data) - { + public void put(String fqn, Map<? extends K, ? extends V> data) { put(Fqn.fromString(fqn), data); } - public void putForExternalRead(Fqn fqn, K key, V value) - { + public void putForExternalRead(Fqn fqn, K key, V value) { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); // if the node exists then this should be a no-op. - if (peek(fqn, false, false) == null) - { + if (peek(fqn, false, false) == null) { getInvocationContext().getOptionOverrides().setFailSilently(true); getInvocationContext().getOptionOverrides().setForceAsynchronous(true); PutForExternalReadCommand command = commandsFactory.buildPutForExternalReadCommand(null, fqn, key, value); invoker.invoke(ctx, command); - } - else - { - if (log.isDebugEnabled()) - { + } else { + if (log.isDebugEnabled()) { log.debug("putForExternalRead() called with Fqn " + fqn + " and this node already exists. This method is hence a no op."); } } } @SuppressWarnings("unchecked") - public V put(Fqn fqn, K key, V value) - { + public V put(Fqn fqn, K key, V value) { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); GlobalTransaction tx = transactionTable.getCurrentTransaction(); @@ -554,29 +473,25 @@ return (V) invoker.invoke(ctx, command); } - public V put(String fqn, K key, V value) - { + public V put(String fqn, K key, V value) { return put(Fqn.fromString(fqn), key, value); } @SuppressWarnings("unchecked") - public Map<K, V> getData(Fqn fqn) - { + public Map<K, V> getData(Fqn fqn) { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); GetDataMapCommand command = commandsFactory.buildGetDataMapCommand(fqn); return (Map<K, V>) invoker.invoke(ctx, command); } - public Set<K> getKeys(String fqn) - { + public Set<K> getKeys(String fqn) { return getKeys(Fqn.fromString(fqn)); } @SuppressWarnings("unchecked") - public Set<K> getKeys(Fqn fqn) - { + public Set<K> getKeys(Fqn fqn) { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); GetKeysCommand command = commandsFactory.buildGetKeysCommand(fqn); @@ -586,85 +501,79 @@ /** * Removes the keys and properties from a node. */ - public void clearData(String fqn) throws CacheException - { + public void clearData(String fqn) throws CacheException { clearData(Fqn.fromString(fqn)); } /** * Removes the keys and properties from a named node. */ - public void clearData(Fqn fqn) - { + public void clearData(Fqn fqn) { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); GlobalTransaction tx = getCurrentTransaction(); invoker.invoke(ctx, commandsFactory.buildClearDataCommand(tx, fqn)); } - public void startBatch() - { - if (!configuration.isInvocationBatchingEnabled()) - { + public void startBatch() { + if (!configuration.isInvocationBatchingEnabled()) { throw new ConfigurationException("Invocation batching not enabled in current configuration! Please use the <invocationBatching /> element."); } batchContainer.startBatch(); } - public void endBatch(boolean successful) - { - if (!configuration.isInvocationBatchingEnabled()) - { + public void endBatch(boolean successful) { + if (!configuration.isInvocationBatchingEnabled()) { throw new ConfigurationException("Invocation batching not enabled in current configuration! Please use the <invocationBatching /> element."); } batchContainer.endBatch(successful); } @SuppressWarnings("unchecked") - public Set<Object> getChildrenNames(Fqn fqn) - { + public Set<Object> getChildrenNames(Fqn fqn) { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); GetChildrenNamesCommand command = commandsFactory.buildGetChildrenNamesCommand(fqn); Set<Object> retval = (Set<Object>) invoker.invoke(ctx, command); - if (retval != null) - { - retval = Immutables.immutableSetWrap(retval); // this is already copied in the command + + // this is needed to work around JBCACHE-1480 + if (retval != null && !retval.isEmpty()) { + for (Iterator i = retval.iterator(); i.hasNext();) { + Object child = getNode(Fqn.fromRelativeElements(fqn, i.next())); + if (child == null) i.remove(); + } } - else - { + + if (retval != null) { + retval = Immutables.immutableSetWrap(retval); // this is already copied in the command + } else { retval = Collections.emptySet(); } + return retval; } @SuppressWarnings("unchecked") - public Set<String> getChildrenNames(String fqn) - { + public Set<String> getChildrenNames(String fqn) { return (Set) getChildrenNames(Fqn.fromString(fqn)); } - public ComponentRegistry getComponentRegistry() - { + public ComponentRegistry getComponentRegistry() { return componentRegistry; } - public DataContainer getDataContainer() - { + public DataContainer getDataContainer() { return dataContainer; } - protected void cacheStatusCheck(InvocationContext ctx) - { + protected void cacheStatusCheck(InvocationContext ctx) { assertIsConstructed(); - if (!ctx.getOptionOverrides().isSkipCacheStatusCheck() && !componentRegistry.invocationsAllowed(true)) - { + if (!ctx.getOptionOverrides().isSkipCacheStatusCheck() && !componentRegistry.invocationsAllowed(true)) { throw new IllegalStateException("Cache not in STARTED state!"); } } - private void invokePut(Fqn fqn, Map<? extends K, ? extends V> data, boolean erase) - { + private void invokePut(Fqn fqn, Map<? extends K, ? extends V> data, boolean erase) { InvocationContext ctx = invocationContextContainer.get(); cacheStatusCheck(ctx); PutDataMapCommand command = commandsFactory.buildPutDataMapCommand(null, fqn, data); @@ -674,13 +583,11 @@ // TODO: Add these to the public interface in 3.1.0. - public void setData(Fqn fqn, Map<? extends K, ? extends V> data) - { + public void setData(Fqn fqn, Map<? extends K, ? extends V> data) { invokePut(fqn, data, true); } - public void setData(String fqn, Map<? extends K, ? extends V> data) - { + public void setData(String fqn, Map<? extends K, ? extends V> data) { setData(Fqn.fromString(fqn), data); } } Added: core/branches/3.0.X/src/test/java/org/jboss/cache/api/mvcc/GetChildrenNamesAfterRemoveTest.java =================================================================== --- core/branches/3.0.X/src/test/java/org/jboss/cache/api/mvcc/GetChildrenNamesAfterRemoveTest.java (rev 0) +++ core/branches/3.0.X/src/test/java/org/jboss/cache/api/mvcc/GetChildrenNamesAfterRemoveTest.java 2009-02-19 12:47:56 UTC (rev 7728) @@ -0,0 +1,54 @@ +package org.jboss.cache.api.mvcc; + +import org.jboss.cache.AbstractSingleCacheTest; +import org.jboss.cache.CacheSPI; +import org.jboss.cache.Fqn; +import org.jboss.cache.Node; +import org.jboss.cache.UnitTestCacheFactory; +import org.testng.annotations.Test; + +import javax.transaction.TransactionManager; +import java.util.Collections; +import java.util.Set; + +@Test(groups = {"functional", "mvcc"}, sequential = true, testName = "api.mvcc.GetChildrenNamesAfterRemoveTest") +public class GetChildrenNamesAfterRemoveTest extends AbstractSingleCacheTest { + private CacheSPI<String, String> cache; + + public CacheSPI createCache() { + // start a single cache instance + UnitTestCacheFactory<String, String> cf = new UnitTestCacheFactory<String, String>(); + cache = (CacheSPI<String, String>) cf.createCache("configs/local-tx.xml", false, getClass()); + cache.getConfiguration().setEvictionConfig(null); + cache.start(); + return cache; + } + + public void testRemove() throws Exception { + TransactionManager tm = cache.getTransactionManager(); + Fqn<String> testFqn = Fqn.fromElements("test1"); + + tm.begin(); + assertEmpty(testFqn); + cache.put(testFqn, "x", "x"); + assertNotEmpty(testFqn); + cache.removeNode(testFqn); + assertEmpty(testFqn); + tm.commit(); + assertEmpty(testFqn); + } + + private void assertNotEmpty(Fqn<String> testFqn) { + Set<Node<String, String>> children = cache.getNode(testFqn.getParent()).getChildren(); + assert !children.isEmpty() : "Node " + testFqn + " should not be a leaf, but getChildren() returns: " + children; + Set<Object> childrenNames = cache.getNode(testFqn.getParent()).getChildrenNames(); + assert childrenNames.equals(Collections.singleton(testFqn.getLastElement())) : "Retrieving children names on " + testFqn + " should return test1 but is: " + childrenNames; + } + + private void assertEmpty(Fqn<String> testFqn) { + Set<Node<String, String>> children = cache.getNode(testFqn.getParent()).getChildren(); + assert children.isEmpty() : "Children should be empty but is " + children; + Set<Object> childrenNames = cache.getNode(testFqn.getParent()).getChildrenNames(); + assert childrenNames.isEmpty() : "Children names should be empty but is " + childrenNames; + } +}