diff --git a/src/main/java/org/springframework/data/couchbase/cache/CouchbaseCache.java b/src/main/java/org/springframework/data/couchbase/cache/CouchbaseCache.java index 19fd9fe19..740cbb078 100644 --- a/src/main/java/org/springframework/data/couchbase/cache/CouchbaseCache.java +++ b/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,7 +24,6 @@ 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; @@ -31,6 +31,13 @@ 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 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 get(final Object key, final Callable valueLoader) { } @Override - public void put(final Object key, final Object value) { - if (!isAllowNullValues() && value == null) { + @SuppressWarnings("unchecked") + public T get(final Object key, Class 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 get(final Object key, final Callable valueLoader, Class 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 + */ + @SuppressWarnings("unchecked") + public T putIfAbsent(final Object key, final Object value, final Class 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; } diff --git a/src/main/java/org/springframework/data/couchbase/cache/CouchbaseCacheWriter.java b/src/main/java/org/springframework/data/couchbase/cache/CouchbaseCacheWriter.java index 58ae44dde..05d6f856a 100644 --- a/src/main/java/org/springframework/data/couchbase/cache/CouchbaseCacheWriter.java +++ b/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. * diff --git a/src/main/java/org/springframework/data/couchbase/cache/DefaultCouchbaseCacheWriter.java b/src/main/java/org/springframework/data/couchbase/cache/DefaultCouchbaseCacheWriter.java index 73fe4ad29..f25e38459 100644 --- a/src/main/java/org/springframework/data/couchbase/cache/DefaultCouchbaseCacheWriter.java +++ b/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; } diff --git a/src/test/java/org/springframework/data/couchbase/cache/CacheUser.java b/src/test/java/org/springframework/data/couchbase/cache/CacheUser.java index cd0ebf367..b95336ea2 100644 --- a/src/test/java/org/springframework/data/couchbase/cache/CacheUser.java +++ b/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) { diff --git a/src/test/java/org/springframework/data/couchbase/cache/CouchbaseCacheCollectionIntegrationTests.java b/src/test/java/org/springframework/data/couchbase/cache/CouchbaseCacheCollectionIntegrationTests.java index 99fe7c5c3..38349f57c 100644 --- a/src/test/java/org/springframework/data/couchbase/cache/CouchbaseCacheCollectionIntegrationTests.java +++ b/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"); diff --git a/src/test/java/org/springframework/data/couchbase/cache/CouchbaseCacheCollectionTranscoderIntegrationTests.java b/src/test/java/org/springframework/data/couchbase/cache/CouchbaseCacheCollectionTranscoderIntegrationTests.java new file mode 100644 index 000000000..f65db706d --- /dev/null +++ b/src/test/java/org/springframework/data/couchbase/cache/CouchbaseCacheCollectionTranscoderIntegrationTests.java @@ -0,0 +1,140 @@ +/* + * Copyright 2022-2024 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.data.couchbase.cache; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.util.UUID; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.couchbase.core.CouchbaseTemplate; +import org.springframework.data.couchbase.domain.Config; +import org.springframework.data.couchbase.util.Capabilities; +import org.springframework.data.couchbase.util.ClusterType; +import org.springframework.data.couchbase.util.CollectionAwareIntegrationTests; +import org.springframework.data.couchbase.util.IgnoreWhen; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; + +/** + * CouchbaseCache tests Theses tests rely on a cb server running. + * + * @author Michael Reiche + */ +@IgnoreWhen(clusterTypes = ClusterType.MOCKED, missesCapabilities = { Capabilities.COLLECTIONS }) +@SpringJUnitConfig(Config.class) +@DirtiesContext +class CouchbaseCacheCollectionTranscoderIntegrationTests extends CollectionAwareIntegrationTests { + + volatile CouchbaseCache cache; + + @Autowired CouchbaseTemplate couchbaseTemplate; + + @BeforeEach + @Override + public void beforeEach() { + super.beforeEach(); + cache = CouchbaseCacheManager.create(couchbaseTemplate.getCouchbaseClientFactory()).createCouchbaseCache( + "myCache", CouchbaseCacheConfiguration.defaultCacheConfig().collection("my_collection").valueTranscoder( + couchbaseTemplate.getCouchbaseClientFactory().getCluster().environment().transcoder())); + + cache.clear(); + } + + @Test + void cachePutGet() { + CacheUser user1 = new CacheUser(UUID.randomUUID().toString(), "first1", "last1"); + CacheUser user2 = new CacheUser(UUID.randomUUID().toString(), "first2", "last2"); + assertNull(cache.get(user1.getId(), CacheUser.class)); // was not put -> cacheMiss + cache.put(user1.getId(), user1); // put user1 + cache.put(user2.getId(), user2); // put user2 + assertEquals(user1, cache.get(user1.getId(), CacheUser.class)); // get user1 + assertEquals(user2, cache.get(user2.getId(), CacheUser.class)); // 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, CacheUser.class)); // already put, get user1, needs class + + 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 , CacheUser.class)); // already put, get user2, needs class + } + + @Test + void cacheEvict() { + CacheUser user1 = new CacheUser(UUID.randomUUID().toString(), "first1", "last1"); + CacheUser user2 = new CacheUser(UUID.randomUUID().toString(), "first2", "last2"); + cache.put(user1.getId(), user1); // put user1 + cache.put(user2.getId(), user2); // put user2 + cache.evict(user1.getId()); // evict user1 + assertNull(cache.get(user1.getId(), CacheUser.class)); // get user1 -> not present + assertEquals(user2, cache.get(user2.getId(), CacheUser.class)); // get user2 -> present + } + + @Test + void cacheClear() { + CacheUser user1 = new CacheUser(UUID.randomUUID().toString(), "first1", "last1"); + CacheUser user2 = new CacheUser(UUID.randomUUID().toString(), "first2", "last2"); + cache.put(user1.getId(), user1); // put user1 + cache.put(user2.getId(), user2); // put user2 + cache.clear(); + assertNull(cache.get(user1.getId(), CacheUser.class)); // get user1 -> not present + assertNull(cache.get(user2.getId(), CacheUser.class)); // get user2 -> not present + } + + @Test + void cacheHitMiss() { + CacheUser user1 = new CacheUser(UUID.randomUUID().toString(), "first1", "last1"); + CacheUser user2 = new CacheUser(UUID.randomUUID().toString(), "first2", "last2"); + assertNull(cache.get(user2.getId(), CacheUser.class)); // get user2 -> cacheMiss + // cache.put(user1.getId(), null); // JsonTranscoder cannot handle null + // assertNotNull(cache.get(user1.getId(),CacheUser.class)); // cacheHit null + // assertNull(cache.get(user1.getId(),CacheUser.class)); // fetch cached null + } + + @Test + void cachePutIfAbsent() { + CacheUser user1 = new CacheUser(UUID.randomUUID().toString(), "first1", "last1"); + CacheUser user2 = new CacheUser(UUID.randomUUID().toString(), "first2", "last2"); + assertNull(cache.putIfAbsent(user1.getId(), user1, CacheUser.class)); // should put user1, return null + assertEquals(user1, cache.putIfAbsent(user1.getId(), user2, CacheUser.class)); // should not put user2, should + // return user1 + assertEquals(user1, cache.get(user1.getId(), CacheUser.class)); // user1.getId() is still user1 + } + +}