diff --git a/source/java/org/alfresco/repo/cache/CacheTest.java b/source/java/org/alfresco/repo/cache/CacheTest.java index b4815a1e78..42ad41b1aa 100644 --- a/source/java/org/alfresco/repo/cache/CacheTest.java +++ b/source/java/org/alfresco/repo/cache/CacheTest.java @@ -33,6 +33,9 @@ import javax.transaction.UserTransaction; import junit.framework.TestCase; import net.sf.ehcache.CacheManager; +import org.alfresco.error.AlfrescoRuntimeException; +import org.alfresco.repo.transaction.AlfrescoTransactionSupport; +import org.alfresco.repo.transaction.TransactionListenerAdapter; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.alfresco.service.ServiceRegistry; import org.alfresco.service.transaction.TransactionService; @@ -168,13 +171,27 @@ public class CacheTest extends TestCase assertFalse("Transactionally removed item found in keys", transactionalKeys.contains(newGlobalOne)); assertTrue("Transactionally added item not found in keys", transactionalKeys.contains(updatedTxnThree)); + // Register a post-commit stresser. We do this here so that it is registered after the transactional cache + PostCommitCacheUser listener = new PostCommitCacheUser(transactionalCache, updatedTxnThree); + AlfrescoTransactionSupport.bindListener(listener); + // commit the transaction txn.commit(); + // Check the post-commit stresser + if (listener.e != null) + { + throw listener.e; + } + // check that backing cache was updated with the in-transaction changes assertFalse("Item was not removed from backing cache", backingCache.contains(newGlobalOne)); assertNull("Item could still be fetched from backing cache", backingCache.get(newGlobalOne)); assertEquals("Item not updated in backing cache", "XXX", backingCache.get(updatedTxnThree)); + + // Check that the transactional cache serves get requests + assertEquals("Transactional cache must serve post-commit get requests", "XXX", + transactionalCache.get(updatedTxnThree)); } catch (Throwable e) { @@ -186,6 +203,53 @@ public class CacheTest extends TestCase } } + /** + * This transaction listener attempts to use the cache in the afterCommit phase. Technically the + * transaction has finished, but the transaction resources are still available. + * + * @author Derek Hulley + * @since 2.1 + */ + private class PostCommitCacheUser extends TransactionListenerAdapter + { + private final SimpleCache transactionalCache; + private final String key; + private Throwable e; + private PostCommitCacheUser(SimpleCache transactionalCache, String key) + { + this.transactionalCache = transactionalCache; + this.key = key; + } + @Override + public void afterCommit() + { + try + { + transactionalCache.get(key); + } + catch (Throwable e) + { + this.e = e; + return; + } + try + { + transactionalCache.put(key, "ZZZ"); + e = new RuntimeException("Transactional caches should not allow puts in the after-commit phase"); + } + catch (AlfrescoRuntimeException e) + { + // Expected + } + } + @Override + public int hashCode() + { + return -100000; + } + + } + /** * Preloads the cache, then performs a simultaneous addition of N new values and * removal of the N preloaded values. diff --git a/source/java/org/alfresco/repo/cache/TransactionalCache.java b/source/java/org/alfresco/repo/cache/TransactionalCache.java index 6f27ae7f85..5fa0fd03f9 100644 --- a/source/java/org/alfresco/repo/cache/TransactionalCache.java +++ b/source/java/org/alfresco/repo/cache/TransactionalCache.java @@ -291,47 +291,55 @@ public class TransactionalCache if (AlfrescoTransactionSupport.getTransactionId() != null) { TransactionData txnData = getTransactionData(); - try + if (txnData.isClosed) { - if (!txnData.isClearOn) // deletions cache only useful before a clear + // This check could have been done in the first if block, but that would have added another call to the + // txn resources. + } + else // The txn is still active + { + try { - // check to see if the key is present in the transaction's removed items - if (txnData.removedItemsCache.get(key) != null) + if (!txnData.isClearOn) // deletions cache only useful before a clear { - // it has been removed in this transaction + // check to see if the key is present in the transaction's removed items + if (txnData.removedItemsCache.get(key) != null) + { + // it has been removed in this transaction + if (isDebugEnabled) + { + logger.debug("get returning null - item has been removed from transactional cache: \n" + + " cache: " + this + "\n" + + " key: " + key); + } + return null; + } + } + + // check for the item in the transaction's new/updated items + Element element = txnData.updatedItemsCache.get(key); + if (element != null) + { + CacheBucket bucket = (CacheBucket) element.getValue(); + V value = bucket.getValue(); + // element was found in transaction-specific updates/additions if (isDebugEnabled) { - logger.debug("get returning null - item has been removed from transactional cache: \n" + + logger.debug("Found item in transactional cache: \n" + " cache: " + this + "\n" + - " key: " + key); + " key: " + key + "\n" + + " value: " + value); } - return null; + return value; } } - - // check for the item in the transaction's new/updated items - Element element = txnData.updatedItemsCache.get(key); - if (element != null) + catch (CacheException e) { - CacheBucket bucket = (CacheBucket) element.getValue(); - V value = bucket.getValue(); - // element was found in transaction-specific updates/additions - if (isDebugEnabled) - { - logger.debug("Found item in transactional cache: \n" + - " cache: " + this + "\n" + - " key: " + key + "\n" + - " value: " + value); - } - return value; + throw new AlfrescoRuntimeException("Cache failure", e); } + // check if the cleared flag has been set - cleared flag means ignore shared as unreliable + ignoreSharedCache = txnData.isClearOn; } - catch (CacheException e) - { - throw new AlfrescoRuntimeException("Cache failure", e); - } - // check if the cleared flag has been set - cleared flag means ignore shared as unreliable - ignoreSharedCache = txnData.isClearOn; } // no value found - must we ignore the shared cache? if (!ignoreSharedCache) @@ -384,6 +392,11 @@ public class TransactionalCache else // transaction present { TransactionData txnData = getTransactionData(); + // Ensure that the cache isn't being modified + if (txnData.isClosed) + { + throw new AlfrescoRuntimeException("onCommit cache modifications are not allowed."); + } // we have a transaction - add the item into the updated cache for this transaction // are we in an overflow condition? if (txnData.updatedItemsCache.getMemoryStoreSize() >= maxCacheSize) @@ -444,6 +457,11 @@ public class TransactionalCache else // transaction present { TransactionData txnData = getTransactionData(); + // Ensure that the cache isn't being modified + if (txnData.isClosed) + { + throw new AlfrescoRuntimeException("onCommit cache modifications are not allowed."); + } // is the shared cache going to be cleared? if (txnData.isClearOn) { @@ -509,6 +527,11 @@ public class TransactionalCache } TransactionData txnData = getTransactionData(); + // Ensure that the cache isn't being modified + if (txnData.isClosed) + { + throw new AlfrescoRuntimeException("onCommit cache modifications are not allowed."); + } // the shared cache must be cleared at the end of the transaction // and also serves to ensure that the shared cache will be ignored // for the remainder of the transaction @@ -630,6 +653,7 @@ public class TransactionalCache { cacheManager.removeCache(txnData.updatedItemsCache.getName()); cacheManager.removeCache(txnData.removedItemsCache.getName()); + txnData.isClosed = true; } /** @@ -760,8 +784,9 @@ public class TransactionalCache /** Data holder to bind data to the transaction */ private class TransactionData { - public Cache updatedItemsCache; - public Cache removedItemsCache; - public boolean isClearOn; + private Cache updatedItemsCache; + private Cache removedItemsCache; + private boolean isClearOn; + private boolean isClosed; } } diff --git a/source/java/org/alfresco/repo/transaction/AlfrescoTransactionSupport.java b/source/java/org/alfresco/repo/transaction/AlfrescoTransactionSupport.java index 64988ad93a..102ceb1805 100644 --- a/source/java/org/alfresco/repo/transaction/AlfrescoTransactionSupport.java +++ b/source/java/org/alfresco/repo/transaction/AlfrescoTransactionSupport.java @@ -27,6 +27,7 @@ package org.alfresco.repo.transaction; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -461,7 +462,7 @@ public abstract class AlfrescoTransactionSupport private final Set daoServices; private final Set integrityCheckers; private final Set lucenes; - private final Set listeners; + private final LinkedHashSet listeners; private final Map resources; /** @@ -476,7 +477,7 @@ public abstract class AlfrescoTransactionSupport daoServices = new HashSet(3); integrityCheckers = new HashSet(3); lucenes = new HashSet(3); - listeners = new HashSet(5); + listeners = new LinkedHashSet(5); resources = new HashMap(17); }