diff --git a/src/main/java/org/springframework/data/redis/core/BoundKeyOperations.java b/src/main/java/org/springframework/data/redis/core/BoundKeyOperations.java index 15dfef6b6..abd4326f8 100644 --- a/src/main/java/org/springframework/data/redis/core/BoundKeyOperations.java +++ b/src/main/java/org/springframework/data/redis/core/BoundKeyOperations.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2013 the original author or authors. + * Copyright 2011-2014 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. @@ -28,6 +28,7 @@ import org.springframework.data.redis.connection.DataType; *

* * @author Costin Leau + * @author Christoph Strobl */ public interface BoundKeyOperations { @@ -77,7 +78,8 @@ public interface BoundKeyOperations { Boolean persist(); /** - * Renames the key. + * Renames the key.
+ * Note: The new name for empty collections will be propagated on add of first element. * * @param newKey new key */ diff --git a/src/main/java/org/springframework/data/redis/support/collections/AbstractRedisCollection.java b/src/main/java/org/springframework/data/redis/support/collections/AbstractRedisCollection.java index 1dd7f526f..b79622fc9 100644 --- a/src/main/java/org/springframework/data/redis/support/collections/AbstractRedisCollection.java +++ b/src/main/java/org/springframework/data/redis/support/collections/AbstractRedisCollection.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2013 the original author or authors. + * Copyright 2011-2014 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. @@ -27,6 +27,7 @@ import org.springframework.data.redis.core.RedisOperations; * works only with normal, non-pipeline/multi-exec connections as it requires a reply to be sent right away. * * @author Costin Leau + * @author Christoph Strobl */ public abstract class AbstractRedisCollection extends AbstractCollection implements RedisCollection { @@ -48,6 +49,7 @@ public abstract class AbstractRedisCollection extends AbstractCollection i return operations; } + @Override public boolean addAll(Collection c) { boolean modified = false; for (E e : c) { @@ -56,10 +58,7 @@ public abstract class AbstractRedisCollection extends AbstractCollection i return modified; } - public abstract boolean add(E e); - - public abstract void clear(); - + @Override public boolean containsAll(Collection c) { boolean contains = true; for (Object object : c) { @@ -68,8 +67,7 @@ public abstract class AbstractRedisCollection extends AbstractCollection i return contains; } - public abstract boolean remove(Object o); - + @Override public boolean removeAll(Collection c) { boolean modified = false; for (Object object : c) { @@ -78,6 +76,60 @@ public abstract class AbstractRedisCollection extends AbstractCollection i return modified; } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.core.BoundKeyOperations#expire(long, java.util.concurrent.TimeUnit) + */ + @Override + public Boolean expire(long timeout, TimeUnit unit) { + return operations.expire(key, timeout, unit); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.core.BoundKeyOperations#expireAt(java.util.Date) + */ + @Override + public Boolean expireAt(Date date) { + return operations.expireAt(key, date); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.core.BoundKeyOperations#getExpire() + */ + @Override + public Long getExpire() { + return operations.getExpire(key); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.core.BoundKeyOperations#persist() + */ + @Override + public Boolean persist() { + return operations.persist(key); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.core.BoundKeyOperations#rename(java.lang.Object) + */ + @Override + public void rename(final String newKey) { + if (!this.isEmpty()) { + CollectionUtils.rename(key, newKey, operations); + } + key = newKey; + } + + protected void checkResult(Object obj) { + if (obj == null) { + throw new IllegalStateException("Cannot read collection with Redis connection in pipeline/multi-exec mode"); + } + } + public boolean equals(Object o) { if (o == this) return true; @@ -105,30 +157,4 @@ public abstract class AbstractRedisCollection extends AbstractCollection i return sb.toString(); } - public Boolean expire(long timeout, TimeUnit unit) { - return operations.expire(key, timeout, unit); - } - - public Boolean expireAt(Date date) { - return operations.expireAt(key, date); - } - - public Long getExpire() { - return operations.getExpire(key); - } - - public Boolean persist() { - return operations.persist(key); - } - - public void rename(final String newKey) { - CollectionUtils.rename(key, newKey, operations); - key = newKey; - } - - protected void checkResult(Object obj) { - if (obj == null) { - throw new IllegalStateException("Cannot read collection with Redis connection in pipeline/multi-exec mode"); - } - } } diff --git a/src/test/java/org/springframework/data/redis/support/BoundKeyOperationsTest.java b/src/test/java/org/springframework/data/redis/support/BoundKeyOperationsTest.java index af8bb4a8f..7602103af 100644 --- a/src/test/java/org/springframework/data/redis/support/BoundKeyOperationsTest.java +++ b/src/test/java/org/springframework/data/redis/support/BoundKeyOperationsTest.java @@ -19,21 +19,23 @@ import static org.junit.Assert.*; import static org.junit.Assume.*; import java.util.Collection; -import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.TimeUnit; import org.junit.After; import org.junit.AfterClass; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; +import org.springframework.dao.DataAccessException; import org.springframework.data.redis.ConnectionFactoryTracker; import org.springframework.data.redis.ObjectFactory; import org.springframework.data.redis.connection.ConnectionUtils; +import org.springframework.data.redis.connection.RedisConnection; import org.springframework.data.redis.core.BoundKeyOperations; +import org.springframework.data.redis.core.RedisCallback; import org.springframework.data.redis.core.RedisTemplate; import org.springframework.data.redis.support.atomic.RedisAtomicInteger; import org.springframework.data.redis.support.atomic.RedisAtomicLong; @@ -42,13 +44,20 @@ import org.springframework.data.redis.support.atomic.RedisAtomicLong; * @author Costin Leau * @author Jennifer Hickey * @author Thomas Darimont + * @author Christoph Strobl */ @RunWith(Parameterized.class) public class BoundKeyOperationsTest { + + @SuppressWarnings("rawtypes")// private BoundKeyOperations keyOps; + private ObjectFactory objFactory; + + @SuppressWarnings("rawtypes")// private RedisTemplate template; + @SuppressWarnings("rawtypes") public BoundKeyOperationsTest(BoundKeyOperations keyOps, ObjectFactory objFactory, RedisTemplate template) { this.objFactory = objFactory; @@ -57,8 +66,23 @@ public class BoundKeyOperationsTest { ConnectionFactoryTracker.add(template.getConnectionFactory()); } + @Before + public void setUp() { + populateBoundKey(); + } + + @SuppressWarnings("unchecked") @After - public void stop() {} + public void tearDown() { + template.execute(new RedisCallback() { + + @Override + public Object doInRedis(RedisConnection connection) throws DataAccessException { + connection.flushDb(); + return null; + } + }); + } @AfterClass public static void cleanUp() { @@ -70,23 +94,16 @@ public class BoundKeyOperationsTest { return BoundKeyParams.testParams(); } + @SuppressWarnings("unchecked") @Test public void testRename() throws Exception { - // DATAREDIS-188 Infinite loop renaming a non-existent Collection when using Lettuce - assumeTrue(!ConnectionUtils.isLettuce(template.getConnectionFactory())); + Object key = keyOps.getKey(); - assertNotNull(key); - // RedisAtomicInteger/Long need to be reset, as they may be created - // at start of test run and underlying key wiped out by other tests - try { - keyOps.getClass().getMethod("set", int.class).invoke(keyOps, 0); - } catch (NoSuchMethodException e) {} - try { - keyOps.getClass().getMethod("set", long.class).invoke(keyOps, 0l); - } catch (NoSuchMethodException e) {} Object newName = objFactory.instance(); + keyOps.rename(newName); assertEquals(newName, keyOps.getKey()); + keyOps.rename(key); assertEquals(key, keyOps.getKey()); } @@ -97,9 +114,8 @@ public class BoundKeyOperationsTest { @Test public void testExpire() throws Exception { - populateBoundKey(); - assertEquals(keyOps.getClass().getName() + " -> " + keyOps.getKey(), Long.valueOf(-1), keyOps.getExpire()); + if (keyOps.expire(10, TimeUnit.SECONDS)) { long expire = keyOps.getExpire().longValue(); assertTrue(expire <= 10 && expire > 5); @@ -113,22 +129,20 @@ public class BoundKeyOperationsTest { public void testPersist() throws Exception { assumeTrue(!ConnectionUtils.isJredis(template.getConnectionFactory())); - populateBoundKey(); - keyOps.persist(); assertEquals(keyOps.getClass().getName() + " -> " + keyOps.getKey(), Long.valueOf(-1), keyOps.getExpire()); if (keyOps.expire(10, TimeUnit.SECONDS)) { assertTrue(keyOps.getExpire().longValue() > 0); } + keyOps.persist(); assertEquals(keyOps.getClass().getName() + " -> " + keyOps.getKey(), -1, keyOps.getExpire().longValue()); } @SuppressWarnings({ "unchecked", "rawtypes" }) private void populateBoundKey() { - - if (keyOps instanceof List || keyOps instanceof Set) { + if (keyOps instanceof Collection) { ((Collection) keyOps).add("dummy"); } else if (keyOps instanceof Map) { ((Map) keyOps).put("dummy", "dummy"); diff --git a/src/test/java/org/springframework/data/redis/support/collections/AbstractRedisCollectionUnitTests.java b/src/test/java/org/springframework/data/redis/support/collections/AbstractRedisCollectionUnitTests.java new file mode 100644 index 000000000..664dcd654 --- /dev/null +++ b/src/test/java/org/springframework/data/redis/support/collections/AbstractRedisCollectionUnitTests.java @@ -0,0 +1,119 @@ +/* + * Copyright 2014 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 + * + * http://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.redis.support.collections; + +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.data.redis.connection.DataType; +import org.springframework.data.redis.connection.RedisConnection; +import org.springframework.data.redis.connection.RedisConnectionFactory; +import org.springframework.data.redis.core.RedisTemplate; + +/** + * @author Christoph Strobl + */ +@RunWith(MockitoJUnitRunner.class) +public class AbstractRedisCollectionUnitTests { + + private AbstractRedisCollection collection; + + @SuppressWarnings("rawtypes")// + private RedisTemplate redisTemplateSpy; + private @Mock RedisConnectionFactory connectionFactoryMock; + private @Mock RedisConnection connectionMock; + + @SuppressWarnings({ "unchecked", "rawtypes" }) + @Before + public void setUp() { + + redisTemplateSpy = spy(new RedisTemplate() { + + public Boolean hasKey(Object key) { + return false; + } + }); + redisTemplateSpy.setConnectionFactory(connectionFactoryMock); + redisTemplateSpy.afterPropertiesSet(); + + collection = new AbstractRedisCollection("key", redisTemplateSpy) { + + private List delegate = new ArrayList(); + + @Override + public boolean add(String value) { + return this.delegate.add(value); + }; + + @Override + public DataType getType() { + return DataType.LIST; + } + + @Override + public Iterator iterator() { + return this.delegate.iterator(); + } + + @Override + public int size() { + return this.delegate.size(); + } + + @Override + public boolean isEmpty() { + return this.delegate.isEmpty(); + } + + }; + + when(connectionFactoryMock.getConnection()).thenReturn(connectionMock); + } + + /** + * @see DATAREDIS-188 + */ + @SuppressWarnings("unchecked") + @Test + public void testRenameOfEmptyCollectionShouldNotTriggerRedisOperation() { + + collection.rename("new-key"); + verify(redisTemplateSpy, never()).rename(eq("key"), eq("new-key")); + } + + /** + * @see DATAREDIS-188 + */ + @SuppressWarnings("unchecked") + @Test + public void testRenameCollectionShouldTriggerRedisOperation() { + + when(redisTemplateSpy.hasKey(anyObject())).thenReturn(Boolean.TRUE); + + collection.add("spring-data-redis"); + collection.rename("new-key"); + verify(redisTemplateSpy, times(1)).rename(eq("key"), eq("new-key")); + } +}