Reliable null value handling in ConcurrentMapCache, GuavaCache, JCacheCache

The 4.2 variant of this fix includes a common AbstractValueAdaptingCache base class and a common NullValue holder class.

Issue: SPR-13553

Author: jhoeller

spring-context-support/src/main/java/org/springframework/cache/ehcache/EhCacheCache.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -63,7 +63,7 @@ public final Ehcache getNativeCache() {
 	@Override
 	public ValueWrapper get(Object key) {
 		Element element = this.cache.get(key);
-		return toWrapper(element);
+		return toValueWrapper(element);
 	}
 
 	@Override
@@ -85,7 +85,7 @@ public void put(Object key, Object value) {
 	@Override
 	public ValueWrapper putIfAbsent(Object key, Object value) {
 		Element existingElement = this.cache.putIfAbsent(new Element(key, value));
-		return toWrapper(existingElement);
+		return toValueWrapper(existingElement);
 	}
 
 	@Override
@@ -98,7 +98,7 @@ public void clear() {
 		this.cache.removeAll();
 	}
 
-	private ValueWrapper toWrapper(Element element) {
+	private ValueWrapper toValueWrapper(Element element) {
 		return (element != null ? new SimpleValueWrapper(element.getObjectValue()) : null);
 	}
 

spring-context-support/src/main/java/org/springframework/cache/guava/GuavaCache.java

@@ -16,37 +16,31 @@
 
 package org.springframework.cache.guava;
 
-import java.io.Serializable;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 
 import com.google.common.cache.LoadingCache;
 import com.google.common.util.concurrent.UncheckedExecutionException;
 
-import org.springframework.cache.Cache;
-import org.springframework.cache.support.SimpleValueWrapper;
+import org.springframework.cache.support.AbstractValueAdaptingCache;
 import org.springframework.util.Assert;
 
 /**
- * Spring {@link Cache} adapter implementation on top of a
- * Guava {@link com.google.common.cache.Cache} instance.
+ * Spring {@link org.springframework.cache.Cache} adapter implementation
+ * on top of a Guava {@link com.google.common.cache.Cache} instance.
  *
  * <p>Requires Google Guava 12.0 or higher.
  *
  * @author Juergen Hoeller
  * @author Stephane Nicoll
  * @since 4.0
  */
-public class GuavaCache implements Cache {
-
-	private static final Object NULL_HOLDER = new NullHolder();
+public class GuavaCache extends AbstractValueAdaptingCache {
 
 	private final String name;
 
 	private final com.google.common.cache.Cache<Object, Object> cache;
 
-	private final boolean allowNullValues;
-
 
 	/**
 	 * Create a {@link GuavaCache} instance with the specified name and the
@@ -67,11 +61,11 @@ public GuavaCache(String name, com.google.common.cache.Cache<Object, Object> cac
 	 * values for this cache
 	 */
 	public GuavaCache(String name, com.google.common.cache.Cache<Object, Object> cache, boolean allowNullValues) {
+		super(allowNullValues);
 		Assert.notNull(name, "Name must not be null");
 		Assert.notNull(cache, "Cache must not be null");
 		this.name = name;
 		this.cache = cache;
-		this.allowNullValues = allowNullValues;
 	}
 
 
@@ -85,32 +79,23 @@ public final com.google.common.cache.Cache<Object, Object> getNativeCache() {
 		return this.cache;
 	}
 
-	public final boolean isAllowNullValues() {
-		return this.allowNullValues;
-	}
-
 	@Override
 	public ValueWrapper get(Object key) {
 		if (this.cache instanceof LoadingCache) {
 			try {
 				Object value = ((LoadingCache<Object, Object>) this.cache).get(key);
-				return toWrapper(value);
+				return toValueWrapper(value);
 			}
 			catch (ExecutionException ex) {
 				throw new UncheckedExecutionException(ex.getMessage(), ex);
 			}
 		}
-		return toWrapper(this.cache.getIfPresent(key));
+		return super.get(key);
 	}
 
 	@Override
-	@SuppressWarnings("unchecked")
-	public <T> T get(Object key, Class<T> type) {
-		Object value = fromStoreValue(this.cache.getIfPresent(key));
-		if (value != null && type != null && !type.isInstance(value)) {
-			throw new IllegalStateException("Cached value is not of required type [" + type.getName() + "]: " + value);
-		}
-		return (T) value;
+	protected Object lookup(Object key) {
+		return this.cache.getIfPresent(key);
 	}
 
 	@Override
@@ -123,7 +108,7 @@ public ValueWrapper putIfAbsent(Object key, final Object value) {
 		try {
 			PutIfAbsentCallable callable = new PutIfAbsentCallable(value);
 			Object result = this.cache.get(key, callable);
-			return (callable.called ? null : toWrapper(result));
+			return (callable.called ? null : toValueWrapper(result));
 		}
 		catch (ExecutionException ex) {
 			throw new IllegalStateException(ex);
@@ -141,42 +126,6 @@ public void clear() {
 	}
 
 
-	/**
-	 * Convert the given value from the internal store to a user value
-	 * returned from the get method (adapting {@code null}).
-	 * @param storeValue the store value
-	 * @return the value to return to the user
-	 */
-	protected Object fromStoreValue(Object storeValue) {
-		if (this.allowNullValues && storeValue == NULL_HOLDER) {
-			return null;
-		}
-		return storeValue;
-	}
-
-	/**
-	 * Convert the given user value, as passed into the put method,
-	 * to a value in the internal store (adapting {@code null}).
-	 * @param userValue the given user value
-	 * @return the value to store
-	 */
-	protected Object toStoreValue(Object userValue) {
-		if (this.allowNullValues && userValue == null) {
-			return NULL_HOLDER;
-		}
-		return userValue;
-	}
-
-	private ValueWrapper toWrapper(Object value) {
-		return (value != null ? new SimpleValueWrapper(fromStoreValue(value)) : null);
-	}
-
-
-	@SuppressWarnings("serial")
-	private static class NullHolder implements Serializable {
-	}
-
-
 	private class PutIfAbsentCallable implements Callable<Object> {
 
 		private final Object value;

spring-context-support/src/main/java/org/springframework/cache/jcache/JCacheCache.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,10 +16,7 @@
 
 package org.springframework.cache.jcache;
 
-import java.io.Serializable;
-
-import org.springframework.cache.Cache;
-import org.springframework.cache.support.SimpleValueWrapper;
+import org.springframework.cache.support.AbstractValueAdaptingCache;
 import org.springframework.util.Assert;
 
 /**
@@ -32,14 +29,10 @@
  * @author Stephane Nicoll
  * @since 3.2
  */
-public class JCacheCache implements Cache {
-
-	private static final Object NULL_HOLDER = new NullHolder();
+public class JCacheCache extends AbstractValueAdaptingCache {
 
 	private final javax.cache.Cache<Object, Object> cache;
 
-	private final boolean allowNullValues;
-
 
 	/**
 	 * Create an {@link org.springframework.cache.jcache.JCacheCache} instance.
@@ -55,9 +48,9 @@ public JCacheCache(javax.cache.Cache<Object, Object> jcache) {
 	 * @param allowNullValues whether to accept and convert null values for this cache
 	 */
 	public JCacheCache(javax.cache.Cache<Object, Object> jcache, boolean allowNullValues) {
+		super(allowNullValues);
 		Assert.notNull(jcache, "Cache must not be null");
 		this.cache = jcache;
-		this.allowNullValues = allowNullValues;
 	}
 
 
@@ -71,24 +64,9 @@ public final javax.cache.Cache<Object, Object> getNativeCache() {
 		return this.cache;
 	}
 
-	public final boolean isAllowNullValues() {
-		return this.allowNullValues;
-	}
-
 	@Override
-	public ValueWrapper get(Object key) {
-		Object value = this.cache.get(key);
-		return (value != null ? new SimpleValueWrapper(fromStoreValue(value)) : null);
-	}
-
-	@Override
-	@SuppressWarnings("unchecked")
-	public <T> T get(Object key, Class<T> type) {
-		Object value = fromStoreValue(this.cache.get(key));
-		if (value != null && type != null && !type.isInstance(value)) {
-			throw new IllegalStateException("Cached value is not of required type [" + type.getName() + "]: " + value);
-		}
-		return (T) value;
+	protected Object lookup(Object key) {
+		return this.cache.get(key);
 	}
 
 	@Override
@@ -112,36 +90,4 @@ public void clear() {
 		this.cache.removeAll();
 	}
 
-
-	/**
-	 * Convert the given value from the internal store to a user value
-	 * returned from the get method (adapting {@code null}).
-	 * @param storeValue the store value
-	 * @return the value to return to the user
-	 */
-	protected Object fromStoreValue(Object storeValue) {
-		if (this.allowNullValues && storeValue == NULL_HOLDER) {
-			return null;
-		}
-		return storeValue;
-	}
-
-	/**
-	 * Convert the given user value, as passed into the put method,
-	 * to a value in the internal store (adapting {@code null}).
-	 * @param userValue the given user value
-	 * @return the value to store
-	 */
-	protected Object toStoreValue(Object userValue) {
-		if (this.allowNullValues && userValue == null) {
-			return NULL_HOLDER;
-		}
-		return userValue;
-	}
-
-
-	@SuppressWarnings("serial")
-	private static class NullHolder implements Serializable {
-	}
-
 }