Add transcoder support on Cache get, putIfAbsent. (#1970)

Closes #1966.

Author: mikereiche

src/main/java/org/springframework/data/couchbase/cache/CouchbaseCache.java

@@ -15,6 +15,7 @@
  */
 package org.springframework.data.couchbase.cache;
 
+
 import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.Collection;
@@ -23,14 +24,20 @@
 import java.util.concurrent.Callable;
 
 import org.springframework.cache.support.AbstractValueAdaptingCache;
-import org.springframework.cache.support.SimpleValueWrapper;
 import org.springframework.core.convert.ConversionFailedException;
 import org.springframework.core.convert.ConversionService;
 import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.util.Assert;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.ReflectionUtils;
 
+/**
+ * Couchbase-backed Cache Methods that take a Class return non-wrapped objects - cache-miss cannot be distinguished from
+ * cached null - this is what AbstractValueAdaptingCache does Methods that do not take a Class return wrapped objects -
+ * the wrapper is null for cache-miss - the exception is T get(final Object key, final Callable<T> valueLoader), which
+ * does not return a wrapper because if there is a cache-miss, it gets the value from valueLoader (and caches it). There
+ * are anomalies with get(key, ValueLoader) - which returns non-wrapped object.
+ */
 public class CouchbaseCache extends AbstractValueAdaptingCache {
 
 	private final String name;
@@ -70,11 +77,18 @@ public CouchbaseCacheWriter getNativeCache() {
 		return cacheWriter;
 	}
 
+	/**
+	 * same as inherited, but passes clazz for transcoder
+	 */
+	protected Object lookup(final Object key, Class<?> clazz) {
+		return cacheWriter.get(cacheConfig.getCollectionName(), createCacheKey(key), cacheConfig.getValueTranscoder(),
+				clazz);
+	}
+
 	@Override
 	protected Object lookup(final Object key) {
-		return cacheWriter.get(cacheConfig.getCollectionName(), createCacheKey(key), cacheConfig.getValueTranscoder());
+		return lookup(key, Object.class);
 	}
-
 	/**
 	 * Returns the configuration for this {@link CouchbaseCache}.
 	 */
@@ -97,33 +111,56 @@ public synchronized <T> T get(final Object key, final Callable<T> valueLoader) {
 	}
 
 	@Override
-	public void put(final Object key, final Object value) {
-		if (!isAllowNullValues() && value == null) {
+	@SuppressWarnings("unchecked")
+	public <T> T get(final Object key, Class<T> type) {
+		Object value = this.fromStoreValue(this.lookup(key, type));
+		if (value != null && type != null && !type.isInstance(value)) {
+			throw new IllegalStateException("Cached value is not of required type [" + type.getName() + "]: " + value);
+		} else {
+			return (T) value;
+		}
+	}
 
-			throw new IllegalArgumentException(String.format(
-					"Cache '%s' does not allow 'null' values. Avoid storing null via '@Cacheable(unless=\"#result == null\")' or "
-							+ "configure CouchbaseCache to allow 'null' via CouchbaseCacheConfiguration.",
-					name));
+	public synchronized <T> T get(final Object key, final Callable<T> valueLoader, Class<T> type) {
+		T value = get(key, type);
+		if (value == null) { // cannot distinguish between cache miss and cached null
+			value = valueFromLoader(key, valueLoader);
+			put(key, value);
 		}
+		return value;
+	}
 
+	@Override
+	public void put(final Object key, final Object value) {
 		cacheWriter.put(cacheConfig.getCollectionName(), createCacheKey(key), toStoreValue(value), cacheConfig.getExpiry(),
 				cacheConfig.getValueTranscoder());
 	}
 
 	@Override
 	public ValueWrapper putIfAbsent(final Object key, final Object value) {
-		if (!isAllowNullValues() && value == null) {
-			return get(key);
-		}
 
 		Object result = cacheWriter.putIfAbsent(cacheConfig.getCollectionName(), createCacheKey(key), toStoreValue(value),
 				cacheConfig.getExpiry(), cacheConfig.getValueTranscoder());
 
-		if (result == null) {
-			return null;
-		}
+		return toValueWrapper(result);
+	}
 
-		return new SimpleValueWrapper(result);
+	/**
+	 * Not sure why this isn't in AbstractValueAdaptingCache
+	 * 
+	 * @param key
+	 * @param value
+	 * @param clazz
+	 * @return
+	 * @param <T>
+	 */
+	@SuppressWarnings("unchecked")
+	public <T> T putIfAbsent(final Object key, final Object value, final Class<T> clazz) {
+
+		Object result = cacheWriter.putIfAbsent(cacheConfig.getCollectionName(), createCacheKey(key),
+				toStoreValue(value), cacheConfig.getExpiry(), cacheConfig.getValueTranscoder(), clazz);
+
+		return (T) result;
 	}
 
 	@Override
@@ -168,6 +205,9 @@ protected String createCacheKey(final Object key) {
 	 * @throws IllegalStateException if {@code key} cannot be converted to {@link String}.
 	 */
 	protected String convertKey(final Object key) {
+		if (key == null) {
+			throw new IllegalArgumentException(String.format("Cache '%s' does not allow 'null' key.", name));
+		}
 		if (key instanceof String) {
 			return (String) key;
 		}

src/main/java/org/springframework/data/couchbase/cache/CouchbaseCacheWriter.java

@@ -48,6 +48,20 @@ public interface CouchbaseCacheWriter {
 	Object putIfAbsent(String collectionName, String key, Object value, @Nullable Duration expiry,
 			@Nullable Transcoder transcoder);
 
+	/**
+	 * Write the given value to Couchbase if the key does not already exist.
+	 *
+	 * @param collectionName The cache name must not be {@literal null}.
+	 * @param key The key for the cache entry. Must not be {@literal null}.
+	 * @param value The value stored for the key. Must not be {@literal null}.
+	 * @param expiry Optional expiration time. Can be {@literal null}.
+	 * @param transcoder Optional transcoder to use. Can be {@literal null}.
+	 * @param clazz Optional class for contentAs(clazz)
+	 */
+	@Nullable
+	Object putIfAbsent(String collectionName, String key, Object value, @Nullable Duration expiry,
+			@Nullable Transcoder transcoder, @Nullable Class<?> clazz);
+
 	/**
 	 * Get the binary value representation from Couchbase stored for the given key.
 	 *
@@ -59,6 +73,18 @@ Object putIfAbsent(String collectionName, String key, Object value, @Nullable Du
 	@Nullable
 	Object get(String collectionName, String key, @Nullable Transcoder transcoder);
 
+	/**
+	 * Get the binary value representation from Couchbase stored for the given key.
+	 *
+	 * @param collectionName must not be {@literal null}.
+	 * @param key must not be {@literal null}.
+	 * @param transcoder Optional transcoder to use. Can be {@literal null}.
+	 * @param clazz Optional class for contentAs(clazz)
+	 * @return {@literal null} if key does not exist.
+	 */
+	@Nullable
+	Object get(String collectionName, String key, @Nullable Transcoder transcoder, @Nullable Class<?> clazz);
+
 	/**
 	 * Remove the given key from Couchbase.
 	 *

src/main/java/org/springframework/data/couchbase/cache/DefaultCouchbaseCacheWriter.java

@@ -18,12 +18,14 @@
 
 import static com.couchbase.client.core.io.CollectionIdentifier.DEFAULT_COLLECTION;
 import static com.couchbase.client.core.io.CollectionIdentifier.DEFAULT_SCOPE;
-import static com.couchbase.client.java.kv.GetOptions.*;
-import static com.couchbase.client.java.kv.InsertOptions.*;
-import static com.couchbase.client.java.kv.UpsertOptions.*;
-import static com.couchbase.client.java.query.QueryOptions.*;
+import static com.couchbase.client.java.kv.GetOptions.getOptions;
+import static com.couchbase.client.java.kv.InsertOptions.insertOptions;
+import static com.couchbase.client.java.kv.UpsertOptions.upsertOptions;
+import static com.couchbase.client.java.query.QueryOptions.queryOptions;
 import static com.couchbase.client.java.query.QueryScanConsistency.REQUEST_PLUS;
 
+import io.micrometer.common.lang.Nullable;
+
 import java.time.Duration;
 
 import org.springframework.data.couchbase.CouchbaseClientFactory;
@@ -65,6 +67,22 @@ public void put(final String collectionName, final String key, final Object valu
 	@Override
 	public Object putIfAbsent(final String collectionName, final String key, final Object value, final Duration expiry,
 			final Transcoder transcoder) {
+		return putIfAbsent(collectionName, key, value, expiry, transcoder, Object.class);
+	}
+
+	/**
+	 * same as above, plus clazz
+	 * 
+	 * @param collectionName
+	 * @param key
+	 * @param value
+	 * @param expiry
+	 * @param transcoder
+	 * @param clazz
+	 */
+	@Override
+	public Object putIfAbsent(final String collectionName, final String key, final Object value, final Duration expiry,
+			final Transcoder transcoder, @Nullable final Class<?> clazz) {
 		InsertOptions options = insertOptions();
 
 		if (expiry != null) {
@@ -79,15 +97,20 @@ public Object putIfAbsent(final String collectionName, final String key, final O
 			return null;
 		} catch (final DocumentExistsException ex) {
 			// If the document exists, return the current one per contract
-			return get(collectionName, key, transcoder);
+			return get(collectionName, key, transcoder, clazz);
 		}
 	}
 
 	@Override
 	public Object get(final String collectionName, final String key, final Transcoder transcoder) {
-		// TODO .. the decoding side transcoding needs to be figured out?
+		return get(collectionName, key, transcoder, Object.class);
+	}
+
+	@Override
+	public Object get(final String collectionName, final String key, final Transcoder transcoder,
+			final Class<?> clazz) {
 		try {
-			return getCollection(collectionName).get(key, getOptions().transcoder(transcoder)).contentAs(Object.class);
+			return getCollection(collectionName).get(key, getOptions().transcoder(transcoder)).contentAs(clazz);
 		} catch (DocumentNotFoundException ex) {
 			return null;
 		}

src/test/java/org/springframework/data/couchbase/cache/CacheUser.java

@@ -26,9 +26,11 @@
  */
 class CacheUser implements Serializable {
 	// private static final long serialVersionUID = 8817717605659870262L;
-	String firstname;
-	String lastname;
-	String id;
+	String firstname; // must have getter/setter for Serialize/Deserialize
+	String lastname; // must have getter/setter for Serialize/Deserialize
+	String id; // must have getter/setter for Serialize/Deserialize
+
+	public CacheUser() {};
 
 	public CacheUser(String id, String firstname, String lastname) {
 		this.id = id;
@@ -40,6 +42,25 @@ public String getId() {
 		return id;
 	}
 
+	public String getFirstname() {
+		return firstname;
+	}
+
+	public String getLastname() {
+		return lastname;
+	}
+
+	public void setId(String id) {
+		this.id = id;
+	}
+
+	public void setFirstname(String firstname) {
+		this.firstname = firstname;
+	}
+
+	public void setLastname(String lastname) {
+		this.lastname = lastname;
+	}
 	// define equals for assertEquals()
 	public boolean equals(Object o) {
 		if (o == null) {

src/test/java/org/springframework/data/couchbase/cache/CouchbaseCacheCollectionIntegrationTests.java

@@ -68,6 +68,32 @@ void cachePutGet() {
 		assertEquals(user2, cache.get(user2.getId()).get()); // get user2
 	}
 
+	@Test
+	void cacheGetValueLoaderWithClass() {
+		CacheUser user1 = new CacheUser(UUID.randomUUID().toString(), "first1", "last1");
+		assertNull(cache.get(user1.getId(), CacheUser.class)); // was not put -> cacheMiss
+		assertEquals(user1, cache.get(user1.getId(), () -> user1)); // put and get user1
+		assertEquals(user1, cache.get(user1.getId(), () -> user1, CacheUser.class)); // already put, get user1
+
+		CacheUser user2 = new CacheUser(UUID.randomUUID().toString(), "first2", "last2");
+		assertNull(cache.get(user2.getId(), CacheUser.class)); // was not put -> cacheMiss
+		assertEquals(user2, cache.get(user2.getId(), () -> user2, CacheUser.class)); // put and get user2
+		assertEquals(user2, cache.get(user2.getId(), () -> user2, CacheUser.class)); // already put, get user2
+	}
+
+	@Test
+	void cacheGetValueLoaderNoClass() {
+		CacheUser user1 = new CacheUser(UUID.randomUUID().toString(), "first1", "last1");
+		assertNull(cache.get(user1.getId())); // was not put -> cacheMiss
+		assertEquals(user1, cache.get(user1.getId(), () -> user1)); // put and get user1
+		assertEquals(user1, cache.get(user1.getId(), () -> user1)); // already put, get user1
+
+		CacheUser user2 = new CacheUser(UUID.randomUUID().toString(), "first2", "last2");
+		assertNull(cache.get(user2.getId())); // was not put -> cacheMiss
+		assertEquals(user2, cache.get(user2.getId(), () -> user2)); // put and get user2
+		assertEquals(user2, cache.get(user2.getId(), () -> user2)); // already put, get user2
+	}
+
 	@Test
 	void cacheEvict() {
 		CacheUser user1 = new CacheUser(UUID.randomUUID().toString(), "first1", "last1");