diff --git a/source/java/org/alfresco/repo/cache/DefaultSimpleCache.java b/source/java/org/alfresco/repo/cache/DefaultSimpleCache.java index 948d0ef641..68434810ac 100644 --- a/source/java/org/alfresco/repo/cache/DefaultSimpleCache.java +++ b/source/java/org/alfresco/repo/cache/DefaultSimpleCache.java @@ -19,39 +19,35 @@ package org.alfresco.repo.cache; import java.io.Serializable; +import java.util.AbstractMap; import java.util.Collection; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; -import java.util.Map.Entry; import org.springframework.beans.factory.BeanNameAware; +import org.springframework.beans.factory.InitializingBean; + +import com.googlecode.concurrentlinkedhashmap.ConcurrentLinkedHashMap; +import com.googlecode.concurrentlinkedhashmap.Weighers; /** - * {@link SimpleCache} implementation backed by a {@link LinkedHashMap}. + * {@link SimpleCache} implementation backed by a {@link ConcurrentLinkedHashMap}. * * @author Matt Ward */ public final class DefaultSimpleCache - implements SimpleCache, BeanNameAware + implements SimpleCache, BeanNameAware, InitializingBean { - private final Map map; - private int maxItems = 0; + private Map> map; + private int maxItems = 1000000; private String cacheName; + /** + * Default constructor. {@link #afterPropertiesSet()} MUST be called before the cache + * may be used when the cache is constructed using the default constructor. + */ public DefaultSimpleCache() { - // Create a LinkedHashMap with accessOrder true, i.e. iteration order - // will be least recently accessed first. Eviction policy will therefore be LRU. - // The map will have a bounded size determined by the maxItems member variable. - map = (Map) Collections.synchronizedMap(new LinkedHashMap(16, 0.75f, true) { - private static final long serialVersionUID = 1L; - @Override - protected boolean removeEldestEntry(Entry eldest) - { - return maxItems > 0 && size() > maxItems; - } - }); } @Override @@ -69,13 +65,19 @@ public final class DefaultSimpleCache @Override public V get(K key) { - return map.get(key); + AbstractMap.SimpleImmutableEntry kvp = map.get(key); + if (kvp == null) + { + return null; + } + return kvp.getValue(); } @Override public void put(K key, V value) { - map.put(key, value); + AbstractMap.SimpleImmutableEntry kvp = new AbstractMap.SimpleImmutableEntry(key, value); + map.put(key, kvp); } @Override @@ -97,19 +99,14 @@ public final class DefaultSimpleCache } /** - * Sets the maximum number of items that the cache will hold. Setting - * this value will cause the cache to be emptied. A value of zero - * will allow the cache to grow unbounded. + * Sets the maximum number of items that the cache will hold. The cache + * must be re-initialised if already in existence using {@link #afterPropertiesSet()}. * * @param maxItems */ - public void setMaxItems(int maxItems) + public synchronized void setMaxItems(int maxItems) { - synchronized(map) - { - map.clear(); - this.maxItems = maxItems; - } + this.maxItems = maxItems; } /** @@ -123,4 +120,26 @@ public final class DefaultSimpleCache { this.cacheName = cacheName; } + + + /** + * Initialise the cache. + * + * @throws Exception + */ + @Override + public synchronized void afterPropertiesSet() throws Exception + { + if (maxItems < 1) + { + throw new IllegalArgumentException("maxItems property must be a positive integer."); + } + + // The map will have a bounded size determined by the maxItems member variable. + map = new ConcurrentLinkedHashMap.Builder>() + .maximumWeightedCapacity(maxItems) + .concurrencyLevel(32) + .weigher(Weighers.singleton()) + .build(); + } } diff --git a/source/java/org/alfresco/repo/cache/DefaultSimpleCacheTest.java b/source/java/org/alfresco/repo/cache/DefaultSimpleCacheTest.java index 7f6dd7171e..0ad7d19462 100644 --- a/source/java/org/alfresco/repo/cache/DefaultSimpleCacheTest.java +++ b/source/java/org/alfresco/repo/cache/DefaultSimpleCacheTest.java @@ -23,7 +23,10 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import java.util.ArrayList; +import java.util.Collections; import java.util.Iterator; +import java.util.List; import org.junit.Before; import org.junit.Test; @@ -38,32 +41,19 @@ public class DefaultSimpleCacheTest private DefaultSimpleCache cache; @Before - public void setUp() + public void setUp() throws Exception { cache = new DefaultSimpleCache(); + cache.setMaxItems(100); + cache.afterPropertiesSet(); } @Test - public void unboundedSizeCache() - { - cache.put(1, "1"); - cache.put(2, "2"); - cache.put(3, "3"); - cache.put(4, "4"); - cache.put(5, "5"); - - assertEquals("1", cache.get(1)); - assertEquals("2", cache.get(2)); - assertEquals("3", cache.get(3)); - assertEquals("4", cache.get(4)); - assertEquals("5", cache.get(5)); - } - - @Test - public void boundedSizeCache() + public void boundedSizeCache() throws Exception { // We'll only keep the LAST 3 items cache.setMaxItems(3); + cache.afterPropertiesSet(); cache.put(1, "1"); cache.put(2, "2"); @@ -136,7 +126,10 @@ public class DefaultSimpleCacheTest cache.put(12, "red"); cache.put(43, "olive"); - Iterator it = cache.getKeys().iterator(); + List keys = new ArrayList(cache.getKeys()); + Collections.sort(keys); + + Iterator it = keys.iterator(); assertEquals(3, it.next().intValue()); assertEquals(12, it.next().intValue()); assertEquals(43, it.next().intValue()); @@ -144,14 +137,20 @@ public class DefaultSimpleCacheTest } @Test - public void clearUponSetMaxItems() + public void noConcurrentModificationException() { cache.put(1, "1"); - assertTrue(cache.contains(1)); + cache.put(2, "2"); + cache.put(3, "3"); + cache.put(4, "4"); + + Iterator i = cache.getKeys().iterator(); + i.next(); + i.next(); - cache.setMaxItems(10); + cache.put(5, "5"); - // The item should have gone. - assertFalse(cache.contains(1)); + // Causes a ConcurrentModificationException with a java.util.LinkedHashMap + i.next(); } }