Merged HEAD-BUG-FIX (4.3/Cloud) to HEAD (4.3/Cloud)

59851: Merged BRANCHES/DEV/mward/head_bf_gdata_upgd to BRANCHES/DEV/HEAD-BUG-FIX (local cache improvements):
      59586: DefaultSimpleCache max size is Integer.MAX_VALUE when configured with maxItems of 0.
      59590: maxItems for DefaultSimpleCache is now an optional feature.
      59592: non-clustered caches will not use size-based eviction when {cacheName}.eviction-policy=NONE, to match clustered caches.
      59594: Added TTL to DefaultSimpleCache - not yet configurable through the factory.
      59602: DefaultCacheFactory can create caches with a time-to-live setting enabled.
      59620: Organise imports for DefaultSimpleCache
      59622: DefaultSimpleCache: changed field name from map to cache, as this makes more sense with changing to use of CacheBuilder.
      59627: Added maxIdleSecs property to DefaultSimpleCache - not yet configurable from the cache factory.
      59629: DefaultCacheFactory now supports maxIdleSeconds property for configuring non-clustered caches.
      59633: Updated description of supported non-clustered cache properties in caches.properties


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@62192 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/caches.properties

@@ -1,7 +1,20 @@
 #
 # Cache configuration
 #
-# Note that the Community edition supports only the *.maxItems properties.
+# Note: Only the following properties are supported by non-clustered (e.g. cluster.type=local) caches:
+#
+# maxItems              The maximum size a cache may reach. Use zero to set to Integer.MAX_VALUE.
+# eviction-policy       When set to "NONE", the cache will not have a bounded capacity (i.e. maxItems does not apply).
+#                       Any other value will cause maxItems to be enabled: use LRU or LFU if using clustered caches
+#                       so that the value is compatible in both modes (required during startup). Please note, that
+#                       the actual value (e.g. LRU) is of no consequence for the non-clustered caches: eviction is
+#                       performed as for any Google Guava CacheBuilder created Cache.
+# timeToLiveSeconds     Cache items will expire once this time has passed after creation.
+# maxIdleSeconds        Cache items will expire when not accessed for this period.
+#
+# tx.maxItems           Not strictly speaking a supported property (as the TransactionalCache is a separate entity),
+#                       but where a TransactionalCache bean has been defined, the convention has been to use
+#                       {cacheName}.tx.maxItems to specify its capacity.
 #
 cache.propertyValueCache.tx.maxItems=1000
 cache.propertyValueCache.maxItems=10000

source/java/org/alfresco/repo/cache/DefaultCacheFactory.java

@@ -19,7 +19,6 @@
 package org.alfresco.repo.cache;
 
 import java.io.Serializable;
-import java.util.Properties;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -35,6 +34,7 @@ import org.apache.commons.logging.LogFactory;
 public class DefaultCacheFactory<K extends Serializable, V> extends AbstractCacheFactory<K, V>
 {
     private static final Log log = LogFactory.getLog(DefaultCacheFactory.class);
+    private static final String EVICT_NONE = "NONE";
     
     
     @Override
@@ -46,7 +46,10 @@ public class DefaultCacheFactory<K extends Serializable, V> extends AbstractCach
     private SimpleCache<K, V> createLocalCache(String cacheName)
     {
         int maxItems = maxItems(cacheName);
-        DefaultSimpleCache<K, V> cache = new DefaultSimpleCache<K, V>(maxItems, cacheName);
+        boolean useMaxItems = useMaxItems(cacheName);
+        int ttlSecs = ttlSeconds(cacheName);
+        int maxIdleSeconds = maxIdleSeconds(cacheName);
+        DefaultSimpleCache<K, V> cache = new DefaultSimpleCache<K, V>(maxItems, useMaxItems, ttlSecs, maxIdleSeconds, cacheName);
         if (log.isDebugEnabled())
         {
             log.debug("Creating cache: " + cache);
@@ -60,4 +63,24 @@ public class DefaultCacheFactory<K extends Serializable, V> extends AbstractCach
         Integer maxItems = Integer.parseInt(maxItemsStr); 
         return maxItems.intValue();
     }
+    
+    private boolean useMaxItems(String cacheName)
+    {
+        String evictionPolicy = getProperty(cacheName, "eviction-policy", EVICT_NONE);
+        return !evictionPolicy.equals(EVICT_NONE);
+    }
+    
+    private int ttlSeconds(String cacheName)
+    {
+        String ttlSecsStr = getProperty(cacheName, "timeToLiveSeconds", "0");
+        Integer ttlSecs = Integer.parseInt(ttlSecsStr);
+        return ttlSecs;
+    }
+    
+    private int maxIdleSeconds(String cacheName)
+    {
+        String maxIdleSecsStr = getProperty(cacheName, "maxIdleSeconds", "0");
+        Integer maxIdleSecs = Integer.parseInt(maxIdleSecsStr);
+        return maxIdleSecs;
+    }
 }

source/java/org/alfresco/repo/cache/DefaultSimpleCache.java

@@ -21,6 +21,7 @@ package org.alfresco.repo.cache;
 import java.io.Serializable;
 import java.util.AbstractMap;
 import java.util.Collection;
+import java.util.concurrent.TimeUnit;
 
 import org.springframework.beans.factory.BeanNameAware;
 
@@ -35,61 +36,94 @@ import com.google.common.cache.CacheBuilder;
 public final class DefaultSimpleCache<K extends Serializable, V extends Object>
     implements SimpleCache<K, V>, BeanNameAware
 {
-    private static final int DEFAULT_CAPACITY = 200000;
-    private Cache<K, AbstractMap.SimpleImmutableEntry<K, V>> map;
+    private static final int DEFAULT_CAPACITY = Integer.MAX_VALUE;
+    private Cache<K, AbstractMap.SimpleImmutableEntry<K, V>> cache;
     private String cacheName;
     private final int maxItems;
+    private final boolean useMaxItems;
+    private final int ttlSecs;
+    private final int maxIdleSecs;
     
     /**
      * Construct a cache using the specified capacity and name.
      * 
-     * @param maxItems The cache capacity.
+     * @param maxItems The cache capacity. 0 = use {@link #DEFAULT_CAPACITY}
+     * @param useMaxItems Whether the maxItems value should be applied as a size-cap for the cache.
+     * @param cacheName An arbitrary cache name.
      */
-    public DefaultSimpleCache(int maxItems, String cacheName)
+    @SuppressWarnings("unchecked")
+    public DefaultSimpleCache(int maxItems, boolean useMaxItems, int ttlSecs, int maxIdleSecs, String cacheName)
     {
-        if (maxItems < 1)
-        {
-            throw new IllegalArgumentException("maxItems must be a positive integer, but was " + maxItems);
-        }
-        else if (maxItems == 0)
+        if (maxItems == 0)
         {
             maxItems = DEFAULT_CAPACITY;
         }
+        else if (maxItems < 0)
+        {
+            throw new IllegalArgumentException("maxItems may not be negative, but was " + maxItems);
+        }
         this.maxItems = maxItems;
+        this.useMaxItems = useMaxItems;
+        this.ttlSecs = ttlSecs;
+        this.maxIdleSecs = maxIdleSecs;
         setBeanName(cacheName);
         
         // The map will have a bounded size determined by the maxItems member variable.
-        map = CacheBuilder.newBuilder()
-                    .maximumSize(maxItems)
-                    .concurrencyLevel(32)
-                    .build();
+        @SuppressWarnings("rawtypes")
+        CacheBuilder builder = CacheBuilder.newBuilder();
+        
+        if (useMaxItems)
+        {
+            builder.maximumSize(maxItems);
+        }
+        if (ttlSecs > 0)
+        {
+            builder.expireAfterWrite(ttlSecs, TimeUnit.SECONDS);
+        }
+        if (maxIdleSecs > 0)
+        {
+            builder.expireAfterAccess(maxIdleSecs, TimeUnit.SECONDS);
+        }
+        builder.concurrencyLevel(32);
+        
+        cache = (Cache<K, AbstractMap.SimpleImmutableEntry<K, V>>) builder.build();
     }
     
     /**
-     * Default constructor. Initialises the cache with a default capacity {@link #DEFAULT_CAPACITY}
-     * and no name.
+     * Create a size limited, named cache with no other features enabled.
+     * 
+     * @param maxItems
+     * @param cacheName
+     */
+    public DefaultSimpleCache(int maxItems, String cacheName)
+    {
+        this(maxItems, true, 0, 0, cacheName);
+    }
+    
+    /**
+     * Default constructor. Initialises the cache with no size limit and no name.
      */
     public DefaultSimpleCache()
     {
-        this(DEFAULT_CAPACITY, null);
+        this(0, false, 0, 0, null);
     }
     
     @Override
     public boolean contains(K key)
     {
-        return map.asMap().containsKey(key);
+        return cache.asMap().containsKey(key);
     }
 
     @Override
     public Collection<K> getKeys()
     {
-        return map.asMap().keySet();
+        return cache.asMap().keySet();
     }
 
     @Override
     public V get(K key)
     {
-        AbstractMap.SimpleImmutableEntry<K, V> kvp = map.getIfPresent(key);
+        AbstractMap.SimpleImmutableEntry<K, V> kvp = cache.getIfPresent(key);
         if (kvp == null)
         {
             return null;
@@ -111,26 +145,26 @@ public final class DefaultSimpleCache<K extends Serializable, V extends Object>
     public boolean putAndCheckUpdate(K key, V value)
     {
         AbstractMap.SimpleImmutableEntry<K, V> kvp = new AbstractMap.SimpleImmutableEntry<K, V>(key, value);
-        AbstractMap.SimpleImmutableEntry<K, V> priorKVP = map.asMap().put(key, kvp);
+        AbstractMap.SimpleImmutableEntry<K, V> priorKVP = cache.asMap().put(key, kvp);
         return priorKVP != null && (! priorKVP.equals(kvp));
     }
     
     @Override
     public void remove(K key)
     {
-        map.invalidate(key);
+        cache.invalidate(key);
     }
 
     @Override
     public void clear()
     {
-        map.invalidateAll();
+        cache.invalidateAll();
     }
 
     @Override
     public String toString()
     {
-        return "DefaultSimpleCache[maxItems=" + maxItems + ", cacheName=" + cacheName + "]";
+        return "DefaultSimpleCache[maxItems=" + maxItems + ", useMaxItems=" + useMaxItems + ", cacheName=" + cacheName + "]";
     }
     
     /**
@@ -143,7 +177,36 @@ public final class DefaultSimpleCache<K extends Serializable, V extends Object>
         return maxItems;
     }
     
+    /**
+     * Is a size-cap in use?
+     * 
+     * @return useMaxItems
+     */
+    public boolean isUseMaxItems()
+    {
+        return this.useMaxItems;
+    }
     
+    /**
+     * Get the time-to-live setting in seconds.
+     * 
+     * @return ttlSecs
+     */
+    public int getTTLSecs()
+    {
+        return this.ttlSecs;
+    }
+
+    /**
+     * Get the time-to-idle setting in seconds.
+     * 
+     * @return maxIdleSecs
+     */
+    public int getMaxIdleSecs()
+    {
+        return this.maxIdleSecs;
+    }
+
     /**
      * Retrieve the name of this cache.
      * 

source/test-java/org/alfresco/repo/cache/DefaultCacheFactoryTest.java

@@ -18,7 +18,7 @@
  */
 package org.alfresco.repo.cache;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
 
 import java.util.Properties;
 
@@ -41,7 +41,21 @@ public class DefaultCacheFactoryTest
     {
         cacheFactory = new DefaultCacheFactory<String, String>();
         properties = new Properties();
+        // cache.someCache
         properties.setProperty("cache.someCache.maxItems", "4");
+        properties.setProperty("cache.someCache.eviction-policy", "EVICT"); // Anything but NONE
+        // cache.noSizeLimit
+        properties.setProperty("cache.noSizeLimit.maxItems", "2"); // No effect
+        properties.setProperty("cache.noSizeLimit.eviction-policy", "NONE");
+        // cache.withTTL
+        properties.setProperty("cache.withTTL.maxItems", "0");
+        properties.setProperty("cache.withTTL.eviction-policy", "NONE");
+        properties.setProperty("cache.withTTL.timeToLiveSeconds", "6");
+        // cache.withMaxIdle
+        properties.setProperty("cache.withMaxIdle.maxItems", "0");
+        properties.setProperty("cache.withMaxIdle.eviction-policy", "NONE");
+        properties.setProperty("cache.withMaxIdle.maxIdleSeconds", "7");
+        
         cacheFactory.setProperties(properties);
     }
 
@@ -51,5 +65,32 @@ public class DefaultCacheFactoryTest
         cache = (DefaultSimpleCache<String, String>) cacheFactory.createCache("cache.someCache");
         assertEquals(4, cache.getMaxItems());
         assertEquals("cache.someCache", cache.getCacheName());
+        assertTrue(cache.isUseMaxItems());
+    }
+    
+    @Test
+    public void canCreateUnboundedCache()
+    {
+        cache = (DefaultSimpleCache<String, String>) cacheFactory.createCache("cache.noSizeLimit");
+        assertEquals(2, cache.getMaxItems());
+        assertEquals("cache.noSizeLimit", cache.getCacheName());
+        assertFalse(cache.isUseMaxItems());        
+    }
+    
+    @Test
+    public void canCreateCacheWithTTL()
+    {
+        cache = (DefaultSimpleCache<String, String>) cacheFactory.createCache("cache.withTTL");
+        assertEquals("cache.withTTL", cache.getCacheName());
+        assertEquals(6, cache.getTTLSecs());        
+    }
+    
+    @Test
+    public void canCreateCacheWithMaxIdle()
+    {
+        cache = (DefaultSimpleCache<String, String>) cacheFactory.createCache("cache.withMaxIdle");
+        assertEquals("cache.withMaxIdle", cache.getCacheName());
+        assertEquals(0, cache.getTTLSecs());        
+        assertEquals(7, cache.getMaxIdleSecs());        
     }
 }

source/test-java/org/alfresco/repo/cache/DefaultSimpleCacheTest.java

@@ -18,9 +18,7 @@
  */
 package org.alfresco.repo.cache;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
+import static org.junit.Assert.*;
 
 import org.junit.Test;
 
@@ -34,14 +32,14 @@ public class DefaultSimpleCacheTest extends SimpleCacheTestBase<DefaultSimpleCac
     @Override
     protected DefaultSimpleCache<Integer, String> createCache()
     {
-        return new DefaultSimpleCache<Integer, String>(100, getClass().getName());
+        return new DefaultSimpleCache<Integer, String>(100, true, 0, 0, getClass().getName());
     }
     
     @Test
     public void boundedSizeCache() throws Exception
     {
         // We'll only keep the LAST 3 items
-        cache = new DefaultSimpleCache<Integer, String>(3, getClass().getName());
+        cache = new DefaultSimpleCache<Integer, String>(3, true, 0, 0, getClass().getName());
         
         cache.put(1, "1");
         cache.put(2, "2");
@@ -61,6 +59,31 @@ public class DefaultSimpleCacheTest extends SimpleCacheTestBase<DefaultSimpleCac
         assertEquals("3", cache.get(3));
         assertEquals("4", cache.get(4));
         assertEquals("5", cache.get(5));
+        
+        assertTrue(cache.isUseMaxItems());
+    }
+    
+    @Test
+    public void defaultMaxItems()
+    {
+        // maxItems of 0 results in a capacity of Integer.MAX_VALUE - this is to match Hazelcast cache behaviour.
+        cache = new DefaultSimpleCache<Integer, String>(0, true, 0, 0, getClass().getName());
+        assertEquals(Integer.MAX_VALUE, cache.getMaxItems());
+        assertTrue(cache.isUseMaxItems());
+    }
+    
+    @Test
+    public void sizeLimitConstructor()
+    {
+        cache = new DefaultSimpleCache<Integer, String>(123, getClass().getName());
+        assertEquals(123, cache.getMaxItems());
+        assertTrue(cache.isUseMaxItems());
+    }
+    
+    @Test(expected=IllegalArgumentException.class)
+    public void noNegativeMaxItems()
+    {
+        cache = new DefaultSimpleCache<Integer, String>(-1, true, 0, 0, getClass().getName());
     }
     
     @Test
@@ -101,4 +124,72 @@ public class DefaultSimpleCacheTest extends SimpleCacheTestBase<DefaultSimpleCac
         assertEquals(false, cache.putAndCheckUpdate(104, "104"));
         assertEquals(true, cache.putAndCheckUpdate(104, null));
     }
+    
+    // TODO: Timer-based tests are not ideal. An alternative approach is to factor out the CacheBuilder.newBuilder()
+    // call to a protected method, override that in this test class to return a mock and use the mock to check
+    // that the Cache is being configured correctly, e.g. assert that expireAfterWrite(int, TimeUnit) is called.
+    @Test
+    public void cachesCanHaveTTL()
+    {
+        // TTL of 7 seconds
+        cache = new DefaultSimpleCache<Integer, String>(0, false, 7, 0, getClass().getName());
+        assertFalse(cache.isUseMaxItems());
+        
+        cache.put(1, "1");
+        assertTrue(cache.contains(1));
+        assertFalse(cache.contains(2));
+        assertFalse(cache.contains(3));
+        
+        sleep(5);
+        cache.put(2, "2");
+        assertTrue(cache.contains(1));
+        assertTrue(cache.contains(2));
+        assertFalse(cache.contains(3));
+        
+        sleep(5);
+        cache.put(3, "3");
+        assertFalse(cache.contains(1));
+        assertTrue(cache.contains(2)); // Only ~5 seconds have passed for this key
+        assertTrue(cache.contains(3));
+    }
+     
+    @Test
+    public void cachesCanHaveTTI()
+    {
+        cache = new DefaultSimpleCache<Integer, String>(0, false, 0, 8, getClass().getName());
+        assertFalse(cache.isUseMaxItems());
+        assertEquals(0, cache.getTTLSecs());
+        assertEquals(8, cache.getMaxIdleSecs());
+        
+        cache.put(1, "1");
+        assertEquals("1", cache.get(1));
+        
+        sleep(4);
+        // cause zeroing of idle time
+        assertEquals("1", cache.get(1));
+        
+        sleep(4);
+        // cause zeroing of idle time
+        assertEquals("1", cache.get(1));
+        
+        sleep(4);
+        // At least 12 seconds have passed, but the item should still be present. 
+        assertEquals("1", cache.get(1));
+        
+        sleep(10);
+        // time-to-idle now exceeded without access
+        assertNotEquals("1", cache.get(1));      
+    }
+    
+    private void sleep(int seconds)
+    {
+        try
+        {
+            Thread.sleep(seconds * 1000);
+        }
+        catch (InterruptedException error)
+        {
+            throw new RuntimeException(error);
+        }
+    }
 }