Skip to content

Add transcoder support on Cache get, putIfAbsent. #1970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.springframework.data.couchbase.cache;


import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -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;
Expand Down Expand Up @@ -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}.
*/
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading
Loading