From 019498842597acb47cbc764ce173a5ba0729e5bd Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 22 Dec 2015 09:48:49 +0100 Subject: [PATCH] Store by value support for ConcurrentMapCacheManager ConcurrentMapCacheManager and ConcurrentMapCache now support the serialization of cache entries via a new `storeByValue` attribute. If it is explicitly enabled, the cache value is first serialized and that content is stored in the cache. The net result is that any further change made on the object returned from the annotated method is not applied on the copy held in the cache. Issue: SPR-13758 --- .../cache/concurrent/ConcurrentMapCache.java | 89 +++++++++++++++++++ .../concurrent/ConcurrentMapCacheManager.java | 45 +++++++++- .../cache/AbstractCacheTests.java | 2 +- .../ConcurrentMapCacheManagerTests.java | 18 ++++ .../concurrent/ConcurrentMapCacheTests.java | 57 +++++++++++- 5 files changed, 207 insertions(+), 4 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCache.java b/spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCache.java index b8aba44242..a6e9ee02ac 100644 --- a/spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCache.java +++ b/spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCache.java @@ -16,11 +16,15 @@ package org.springframework.cache.concurrent; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import org.springframework.cache.support.AbstractValueAdaptingCache; +import org.springframework.core.serializer.support.SerializationDelegate; import org.springframework.util.Assert; /** @@ -38,6 +42,7 @@ import org.springframework.util.Assert; * * @author Costin Leau * @author Juergen Hoeller + * @author Stephane Nicoll * @since 3.1 */ public class ConcurrentMapCache extends AbstractValueAdaptingCache { @@ -46,6 +51,8 @@ public class ConcurrentMapCache extends AbstractValueAdaptingCache { private final ConcurrentMap store; + private final SerializationDelegate serialization; + /** * Create a new ConcurrentMapCache with the specified name. @@ -74,13 +81,40 @@ public class ConcurrentMapCache extends AbstractValueAdaptingCache { * (adapting them to an internal null holder value) */ public ConcurrentMapCache(String name, ConcurrentMap store, boolean allowNullValues) { + this(name, store, allowNullValues, null); + } + + /** + * Create a new ConcurrentMapCache with the specified name and the + * given internal {@link ConcurrentMap} to use. If the + * {@link SerializationDelegate} is specified, + * {@link #isStoreByValue() store-by-value} is enabled + * @param name the name of the cache + * @param store the ConcurrentMap to use as an internal store + * @param allowNullValues whether to allow {@code null} values + * (adapting them to an internal null holder value) + * @param serialization the {@link SerializationDelegate} to use + * to serialize cache entry or {@code null} to store the reference + */ + protected ConcurrentMapCache(String name, ConcurrentMap store, + boolean allowNullValues, SerializationDelegate serialization) { + super(allowNullValues); Assert.notNull(name, "Name must not be null"); Assert.notNull(store, "Store must not be null"); this.name = name; this.store = store; + this.serialization = serialization; } + /** + * Return whether this cache stores a copy of each entry ({@code true}) or + * a reference ({@code false}, default). If store by value is enabled, each + * entry in the cache must be serializable. + */ + public final boolean isStoreByValue() { + return this.serialization != null; + } @Override public final String getName() { @@ -142,4 +176,59 @@ public class ConcurrentMapCache extends AbstractValueAdaptingCache { this.store.clear(); } + @Override + protected Object toStoreValue(Object userValue) { + Object storeValue = super.toStoreValue(userValue); + if (this.serialization != null) { + try { + return serializeValue(storeValue); + } + catch (Exception ex) { + throw new IllegalArgumentException("Failed to serialize cache value '" + + userValue + "'. Does it implement Serializable?", ex); + } + } + else { + return storeValue; + } + } + + private Object serializeValue(Object storeValue) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try { + this.serialization.serialize(storeValue, out); + return out.toByteArray(); + } + finally { + out.close(); + } + } + + @Override + protected Object fromStoreValue(Object storeValue) { + if (this.serialization != null) { + try { + return super.fromStoreValue(deserializeValue(storeValue)); + } + catch (Exception ex) { + throw new IllegalArgumentException("Failed to deserialize cache value '" + + storeValue + "'", ex); + } + } + else { + return super.fromStoreValue(storeValue); + } + + } + + private Object deserializeValue(Object storeValue) throws IOException { + ByteArrayInputStream in = new ByteArrayInputStream((byte[]) storeValue); + try { + return this.serialization.deserialize(in); + } + finally { + in.close(); + } + } + } diff --git a/spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCacheManager.java b/spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCacheManager.java index cb2a117e20..9a9f15f8db 100644 --- a/spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCacheManager.java +++ b/spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCacheManager.java @@ -23,8 +23,10 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.cache.Cache; import org.springframework.cache.CacheManager; +import org.springframework.core.serializer.support.SerializationDelegate; /** * {@link CacheManager} implementation that lazily builds {@link ConcurrentMapCache} @@ -44,7 +46,7 @@ import org.springframework.cache.CacheManager; * @since 3.1 * @see ConcurrentMapCache */ -public class ConcurrentMapCacheManager implements CacheManager { +public class ConcurrentMapCacheManager implements CacheManager, BeanClassLoaderAware { private final ConcurrentMap cacheMap = new ConcurrentHashMap(16); @@ -52,6 +54,10 @@ public class ConcurrentMapCacheManager implements CacheManager { private boolean allowNullValues = true; + private boolean storeByValue = false; + + private SerializationDelegate serialization; + /** * Construct a dynamic ConcurrentMapCacheManager, @@ -114,6 +120,37 @@ public class ConcurrentMapCacheManager implements CacheManager { return this.allowNullValues; } + /** + * Specify whether this cache manager stores a copy of each entry ({@code true} + * or the reference ({@code false} for all of its caches. + *

Default is "false" so that the value itself is stored and no serializable + * contract is required on cached values. + *

Note: A change of the store-by-value setting will reset all existing caches, + * if any, to reconfigure them with the new store-by-value requirement. + */ + public void setStoreByValue(boolean storeByValue) { + if (storeByValue != this.storeByValue) { + this.storeByValue = storeByValue; + // Need to recreate all Cache instances with the new store-by-value configuration... + for (Map.Entry entry : this.cacheMap.entrySet()) { + entry.setValue(createConcurrentMapCache(entry.getKey())); + } + } + } + + /** + * Return whether this cache manager stores a copy of each entry or + * a reference for all its caches. If store by value is enabled, any + * cache entry must be serializable. + */ + public boolean isStoreByValue() { + return this.storeByValue; + } + + @Override + public void setBeanClassLoader(ClassLoader classLoader) { + this.serialization = new SerializationDelegate(classLoader); + } @Override public Collection getCacheNames() { @@ -141,7 +178,11 @@ public class ConcurrentMapCacheManager implements CacheManager { * @return the ConcurrentMapCache (or a decorator thereof) */ protected Cache createConcurrentMapCache(String name) { - return new ConcurrentMapCache(name, isAllowNullValues()); + SerializationDelegate actualSerialization = + this.storeByValue ? serialization : null; + return new ConcurrentMapCache(name, new ConcurrentHashMap(256), + isAllowNullValues(), actualSerialization); + } } diff --git a/spring-context/src/test/java/org/springframework/cache/AbstractCacheTests.java b/spring-context/src/test/java/org/springframework/cache/AbstractCacheTests.java index 6e6c98ec7f..c71f133d79 100644 --- a/spring-context/src/test/java/org/springframework/cache/AbstractCacheTests.java +++ b/spring-context/src/test/java/org/springframework/cache/AbstractCacheTests.java @@ -211,7 +211,7 @@ public abstract class AbstractCacheTests { results.forEach(r -> assertThat(r, is(1))); // Only one method got invoked } - private String createRandomKey() { + protected String createRandomKey() { return UUID.randomUUID().toString(); } diff --git a/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheManagerTests.java b/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheManagerTests.java index 14fdb5472d..1598b0fa39 100644 --- a/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheManagerTests.java +++ b/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheManagerTests.java @@ -25,6 +25,7 @@ import static org.junit.Assert.*; /** * @author Juergen Hoeller + * @author Stephane Nicoll */ public class ConcurrentMapCacheManagerTests { @@ -120,4 +121,21 @@ public class ConcurrentMapCacheManagerTests { assertNull(cache1y.get("key3")); } + @Test + public void testChangeStoreByValue() { + ConcurrentMapCacheManager cm = new ConcurrentMapCacheManager("c1", "c2"); + assertFalse(cm.isStoreByValue()); + Cache cache1 = cm.getCache("c1"); + assertTrue(cache1 instanceof ConcurrentMapCache); + assertFalse(((ConcurrentMapCache)cache1).isStoreByValue()); + cache1.put("key", "value"); + + cm.setStoreByValue(true); + assertTrue(cm.isStoreByValue()); + Cache cache1x = cm.getCache("c1"); + assertTrue(cache1x instanceof ConcurrentMapCache); + assertTrue(cache1x != cache1); + assertNull(cache1x.get("key")); + } + } diff --git a/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java b/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java index c0a9e00d28..6836ba7030 100644 --- a/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java +++ b/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java @@ -16,13 +16,19 @@ package org.springframework.cache.concurrent; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import org.junit.Before; -import org.junit.Ignore; +import org.junit.Test; import org.springframework.cache.AbstractCacheTests; +import org.springframework.core.serializer.support.SerializationDelegate; + +import static org.junit.Assert.*; /** * @author Costin Leau @@ -53,4 +59,53 @@ public class ConcurrentMapCacheTests extends AbstractCacheTests content = new ArrayList<>(); + content.addAll(Arrays.asList("one", "two", "three")); + serializeCache.put(key, content); + content.remove(0); + List entry = (List) serializeCache.get(key).get(); + assertEquals(3, entry.size()); + assertEquals("one", entry.get(0)); + } + + @Test + public void testNonSerializableContent() { + ConcurrentMapCache serializeCache = createCacheWithStoreByValue(); + + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("Failed to serialize"); + thrown.expectMessage(this.cache.getClass().getName()); + serializeCache.put(createRandomKey(), this.cache); + } + + @Test + public void testInvalidSerializedContent() { + ConcurrentMapCache serializeCache = createCacheWithStoreByValue(); + + String key = createRandomKey(); + this.nativeCache.put(key, "Some garbage"); + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("Failed to deserialize"); + thrown.expectMessage("Some garbage"); + serializeCache.get(key); + } + + + private ConcurrentMapCache createCacheWithStoreByValue() { + return new ConcurrentMapCache(CACHE_NAME, nativeCache, true, + new SerializationDelegate(ConcurrentMapCacheTests.class.getClassLoader())); + } + }