From 5f48ea8ed48130fcec36f6c4b0a7cd53701089db Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Wed, 18 Jun 2014 12:52:26 +0200 Subject: [PATCH] DATAREDIS-317 - Report inappropriate RedisTemplate more eagerly in RedisAtomic wrappers. We now validate the given RedisTemplate in the constructor of RedisAtomicLong/RedisAtomicInteger/RedisAtomicDouble to report an inappropriate RedisTemplate more eagerly. Original pull request: #83. --- .../support/atomic/RedisAtomicDouble.java | 16 ++++- .../support/atomic/RedisAtomicInteger.java | 19 +++++- .../redis/support/atomic/RedisAtomicLong.java | 18 +++++- .../atomic/AbstractRedisAtomicsTests.java | 28 +++++++++ .../atomic/RedisAtomicDoubleTests.java | 60 ++++++++++++++++--- .../atomic/RedisAtomicIntegerTests.java | 59 ++++++++++++++++-- .../support/atomic/RedisAtomicLongTests.java | 60 +++++++++++++++++-- 7 files changed, 235 insertions(+), 25 deletions(-) create mode 100644 src/test/java/org/springframework/data/redis/support/atomic/AbstractRedisAtomicsTests.java diff --git a/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicDouble.java b/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicDouble.java index 916a32a15..992cd1eeb 100644 --- a/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicDouble.java +++ b/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicDouble.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-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.core.RedisTemplate; import org.springframework.data.redis.core.SessionCallback; import org.springframework.data.redis.core.ValueOperations; import org.springframework.data.redis.serializer.GenericToStringSerializer; +import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.data.redis.serializer.StringRedisSerializer; import org.springframework.util.Assert; @@ -36,10 +37,12 @@ import org.springframework.util.Assert; * operations. * * @author Jennifer Hickey + * @author Thomas Darimont */ -@SuppressWarnings("serial") public class RedisAtomicDouble extends Number implements Serializable, BoundKeyOperations { + private static final long serialVersionUID = 1L; + private volatile String key; private ValueOperations operations; private RedisOperations generalOps; @@ -94,13 +97,17 @@ public class RedisAtomicDouble extends Number implements Serializable, BoundKeyO * * @param redisCounter the redis counter * @param template the template + * @see #RedisAtomicDouble(String, RedisConnectionFactory, double) */ public RedisAtomicDouble(String redisCounter, RedisOperations template) { this(redisCounter, template, null); } /** - * Constructs a new RedisAtomicDouble instance. + * Constructs a new RedisAtomicDouble instance. Note: You need to configure the given {@code template} + * with appropriate {@link RedisSerializer} for the key and value. As an alternative one could use the + * {@link #RedisAtomicDouble(String, RedisConnectionFactory, Double)} constructor which uses appropriate default + * serializers. * * @param redisCounter the redis counter * @param template the template @@ -111,8 +118,11 @@ public class RedisAtomicDouble extends Number implements Serializable, BoundKeyO } private RedisAtomicDouble(String redisCounter, RedisOperations template, Double initialValue) { + Assert.hasText(redisCounter, "a valid counter name is required"); Assert.notNull(template, "a valid template is required"); + Assert.notNull(template.getKeySerializer(), "a valid key serializer in template is required"); + Assert.notNull(template.getValueSerializer(), "a valid value serializer in template is required"); this.key = redisCounter; this.generalOps = template; diff --git a/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicInteger.java b/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicInteger.java index 16325abc4..224460aac 100644 --- a/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicInteger.java +++ b/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicInteger.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,7 +28,9 @@ import org.springframework.data.redis.core.RedisTemplate; import org.springframework.data.redis.core.SessionCallback; import org.springframework.data.redis.core.ValueOperations; import org.springframework.data.redis.serializer.GenericToStringSerializer; +import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.data.redis.serializer.StringRedisSerializer; +import org.springframework.util.Assert; /** * Atomic integer backed by Redis. Uses Redis atomic increment/decrement and watch/multi/exec operations for CAS @@ -36,9 +38,12 @@ import org.springframework.data.redis.serializer.StringRedisSerializer; * * @see java.util.concurrent.atomic.AtomicInteger * @author Costin Leau + * @author Thomas Darimont */ public class RedisAtomicInteger extends Number implements Serializable, BoundKeyOperations { + private static final long serialVersionUID = 1L; + private volatile String key; private ValueOperations operations; private RedisOperations generalOps; @@ -69,13 +74,17 @@ public class RedisAtomicInteger extends Number implements Serializable, BoundKey * * @param redisCounter the redis counter * @param template the template + * @see #RedisAtomicInteger(String, RedisConnectionFactory, int) */ public RedisAtomicInteger(String redisCounter, RedisOperations template) { this(redisCounter, template, null); } /** - * Constructs a new RedisAtomicInteger instance. + * Constructs a new RedisAtomicInteger instance. Note: You need to configure the given {@code template} + * with appropriate {@link RedisSerializer} for the key and value. As an alternative one could use the + * {@link #RedisAtomicInteger(String, RedisConnectionFactory, Integer)} constructor which uses appropriate default + * serializers. * * @param redisCounter the redis counter * @param template the template @@ -107,6 +116,12 @@ public class RedisAtomicInteger extends Number implements Serializable, BoundKey } private RedisAtomicInteger(String redisCounter, RedisOperations template, Integer initialValue) { + + Assert.hasText(redisCounter, "a valid counter name is required"); + Assert.notNull(template, "a valid template is required"); + Assert.notNull(template.getKeySerializer(), "a valid key serializer in template is required"); + Assert.notNull(template.getValueSerializer(), "a valid value serializer in template is required"); + this.key = redisCounter; this.generalOps = template; this.operations = generalOps.opsForValue(); diff --git a/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicLong.java b/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicLong.java index 318040600..a2e738f36 100644 --- a/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicLong.java +++ b/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicLong.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.core.RedisTemplate; import org.springframework.data.redis.core.SessionCallback; import org.springframework.data.redis.core.ValueOperations; import org.springframework.data.redis.serializer.GenericToStringSerializer; +import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.data.redis.serializer.StringRedisSerializer; import org.springframework.util.Assert; @@ -37,9 +38,12 @@ import org.springframework.util.Assert; * * @see java.util.concurrent.atomic.AtomicLong * @author Costin Leau + * @author Thomas Darimont */ public class RedisAtomicLong extends Number implements Serializable, BoundKeyOperations { + private static final long serialVersionUID = 1L; + private volatile String key; private ValueOperations operations; private RedisOperations generalOps; @@ -94,6 +98,7 @@ public class RedisAtomicLong extends Number implements Serializable, BoundKeyOpe * * @param redisCounter the redis counter * @param template the template + * @see #RedisAtomicLong(String, RedisConnectionFactory, long) */ public RedisAtomicLong(String redisCounter, RedisOperations template) { this(redisCounter, template, null); @@ -101,6 +106,14 @@ public class RedisAtomicLong extends Number implements Serializable, BoundKeyOpe /** * Constructs a new RedisAtomicLong instance. + *

+ * Note: You need to configure the given {@code template} with appropriate {@link RedisSerializer} for the key and + * value. The key serializer must be able to deserialize to a {@link String} and the value serializer must be able to + * deserialize to a {@link Long}. + *

+ * As an alternative one could use the {@link #RedisAtomicLong(String, RedisConnectionFactory, Long)} constructor + * which uses appropriate default serializers, in this case {@link StringRedisSerializer} for the key and + * {@link GenericToStringSerializer} for the value. * * @param redisCounter the redis counter * @param template the template @@ -111,8 +124,11 @@ public class RedisAtomicLong extends Number implements Serializable, BoundKeyOpe } private RedisAtomicLong(String redisCounter, RedisOperations template, Long initialValue) { + Assert.hasText(redisCounter, "a valid counter name is required"); Assert.notNull(template, "a valid template is required"); + Assert.notNull(template.getKeySerializer(), "a valid key serializer in template is required"); + Assert.notNull(template.getValueSerializer(), "a valid value serializer in template is required"); this.key = redisCounter; this.generalOps = template; diff --git a/src/test/java/org/springframework/data/redis/support/atomic/AbstractRedisAtomicsTests.java b/src/test/java/org/springframework/data/redis/support/atomic/AbstractRedisAtomicsTests.java new file mode 100644 index 000000000..4cef18760 --- /dev/null +++ b/src/test/java/org/springframework/data/redis/support/atomic/AbstractRedisAtomicsTests.java @@ -0,0 +1,28 @@ +/* + * 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.atomic; + +import org.junit.Rule; +import org.junit.rules.ExpectedException; + +/** + * @author Thomas Darimont + */ +public abstract class AbstractRedisAtomicsTests { + + @Rule public ExpectedException expectedException = ExpectedException.none(); + +} diff --git a/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicDoubleTests.java b/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicDoubleTests.java index 126ce30ec..09d1bf7ca 100644 --- a/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicDoubleTests.java +++ b/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicDoubleTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-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. @@ -15,11 +15,9 @@ */ package org.springframework.data.redis.support.atomic; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assume.assumeTrue; +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; +import static org.junit.Assume.*; import java.util.Collection; import java.util.Date; @@ -37,14 +35,18 @@ import org.springframework.data.redis.RedisTestProfileValueSource; import org.springframework.data.redis.connection.ConnectionUtils; import org.springframework.data.redis.connection.RedisConnection; import org.springframework.data.redis.connection.RedisConnectionFactory; +import org.springframework.data.redis.core.RedisTemplate; +import org.springframework.data.redis.serializer.GenericToStringSerializer; +import org.springframework.data.redis.serializer.StringRedisSerializer; /** * Integration test of {@link RedisAtomicDouble} * * @author Jennifer Hickey + * @author Thomas Darimont */ @RunWith(Parameterized.class) -public class RedisAtomicDoubleTests { +public class RedisAtomicDoubleTests extends AbstractRedisAtomicsTests { private RedisAtomicDouble doubleCounter; @@ -164,4 +166,48 @@ public class RedisAtomicDoubleTests { assertEquals("5.6", new String(factory.getConnection().get("foodouble".getBytes()))); assertNull(factory.getConnection().get((getClass().getSimpleName() + ":double").getBytes())); } + + /** + * @see DATAREDIS-317 + */ + @Test + public void testShouldThrowExceptionIfRedisAtomicDoubleIsUsedWithRedisTemplateAndNoKeySerializer() { + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("a valid key serializer in template is required"); + + new RedisAtomicDouble("foo", new RedisTemplate()); + } + + /** + * @see DATAREDIS-317 + */ + @Test + public void testShouldThrowExceptionIfRedisAtomicDoubleIsUsedWithRedisTemplateAndNoValueSerializer() { + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("a valid value serializer in template is required"); + + RedisTemplate template = new RedisTemplate(); + template.setKeySerializer(new StringRedisSerializer()); + new RedisAtomicDouble("foo", template); + } + + /** + * @see DATAREDIS-317 + */ + @Test + public void testShouldBeAbleToUseRedisAtomicDoubleWithProperlyConfiguredRedisTemplate() { + + RedisTemplate template = new RedisTemplate(); + template.setConnectionFactory(factory); + template.setKeySerializer(new StringRedisSerializer()); + template.setValueSerializer(new GenericToStringSerializer(Double.class)); + template.afterPropertiesSet(); + + RedisAtomicDouble ral = new RedisAtomicDouble("DATAREDIS-317.atomicDouble", template); + ral.set(32.23); + + assertThat(ral.get(), is(32.23)); + } } diff --git a/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicIntegerTests.java b/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicIntegerTests.java index 788f79be0..bd28071eb 100644 --- a/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicIntegerTests.java +++ b/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicIntegerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-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. @@ -15,10 +15,9 @@ */ package org.springframework.data.redis.support.atomic; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assume.assumeTrue; +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; +import static org.junit.Assume.*; import java.util.Collection; import java.util.concurrent.CountDownLatch; @@ -35,15 +34,19 @@ import org.springframework.data.redis.ConnectionFactoryTracker; import org.springframework.data.redis.connection.ConnectionUtils; import org.springframework.data.redis.connection.RedisConnection; import org.springframework.data.redis.connection.RedisConnectionFactory; +import org.springframework.data.redis.core.RedisTemplate; +import org.springframework.data.redis.serializer.GenericToStringSerializer; +import org.springframework.data.redis.serializer.StringRedisSerializer; /** * Integration test of {@link RedisAtomicInteger} * * @author Costin Leau * @author Jennifer Hickey + * @author Thomas Darimont */ @RunWith(Parameterized.class) -public class RedisAtomicIntegerTests { +public class RedisAtomicIntegerTests extends AbstractRedisAtomicsTests { private RedisAtomicInteger intCounter; private RedisConnectionFactory factory; @@ -133,4 +136,48 @@ public class RedisAtomicIntegerTests { assertFalse("counter already modified", failed.get()); } + + /** + * @see DATAREDIS-317 + */ + @Test + public void testShouldThrowExceptionIfRedisAtomicIntegerIsUsedWithRedisTemplateAndNoKeySerializer() { + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("a valid key serializer in template is required"); + + new RedisAtomicInteger("foo", new RedisTemplate()); + } + + /** + * @see DATAREDIS-317 + */ + @Test + public void testShouldThrowExceptionIfRedisAtomicIntegerIsUsedWithRedisTemplateAndNoValueSerializer() { + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("a valid value serializer in template is required"); + + RedisTemplate template = new RedisTemplate(); + template.setKeySerializer(new StringRedisSerializer()); + new RedisAtomicInteger("foo", template); + } + + /** + * @see DATAREDIS-317 + */ + @Test + public void testShouldBeAbleToUseRedisAtomicIntegerWithProperlyConfiguredRedisTemplate() { + + RedisTemplate template = new RedisTemplate(); + template.setConnectionFactory(factory); + template.setKeySerializer(new StringRedisSerializer()); + template.setValueSerializer(new GenericToStringSerializer(Integer.class)); + template.afterPropertiesSet(); + + RedisAtomicInteger ral = new RedisAtomicInteger("DATAREDIS-317.atomicInteger", template); + ral.set(32); + + assertThat(ral.get(), is(32)); + } } diff --git a/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicLongTests.java b/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicLongTests.java index 9fc0a7170..aeee29e86 100644 --- a/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicLongTests.java +++ b/src/test/java/org/springframework/data/redis/support/atomic/RedisAtomicLongTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-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. @@ -15,10 +15,9 @@ */ package org.springframework.data.redis.support.atomic; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assume.assumeTrue; +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; +import static org.junit.Assume.*; import java.util.Collection; @@ -32,15 +31,19 @@ import org.springframework.data.redis.ConnectionFactoryTracker; import org.springframework.data.redis.connection.ConnectionUtils; import org.springframework.data.redis.connection.RedisConnection; import org.springframework.data.redis.connection.RedisConnectionFactory; +import org.springframework.data.redis.core.RedisTemplate; +import org.springframework.data.redis.serializer.GenericToStringSerializer; +import org.springframework.data.redis.serializer.StringRedisSerializer; /** * Integration test of {@link RedisAtomicLong} * * @author Costin Leau * @author Jennifer Hickey + * @author Thomas Darimont */ @RunWith(Parameterized.class) -public class RedisAtomicLongTests { +public class RedisAtomicLongTests extends AbstractRedisAtomicsTests { private RedisAtomicLong longCounter; private RedisConnectionFactory factory; @@ -103,4 +106,49 @@ public class RedisAtomicLongTests { RedisAtomicLong keyCopy = new RedisAtomicLong(longCounter.getKey(), factory); assertEquals(longCounter.get(), keyCopy.get()); } + + /** + * @see DATAREDIS-317 + */ + @Test + public void testShouldThrowExceptionIfAtomicLongIsUsedWithRedisTemplateAndNoKeySerializer() { + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("a valid key serializer in template is required"); + + RedisTemplate template = new RedisTemplate(); + new RedisAtomicLong("foo", template); + } + + /** + * @see DATAREDIS-317 + */ + @Test + public void testShouldThrowExceptionIfAtomicLongIsUsedWithRedisTemplateAndNoValueSerializer() { + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("a valid value serializer in template is required"); + + RedisTemplate template = new RedisTemplate(); + template.setKeySerializer(new StringRedisSerializer()); + new RedisAtomicLong("foo", template); + } + + /** + * @see DATAREDIS-317 + */ + @Test + public void testShouldBeAbleToUseRedisAtomicLongWithProperlyConfiguredRedisTemplate() { + + RedisTemplate template = new RedisTemplate(); + template.setConnectionFactory(factory); + template.setKeySerializer(new StringRedisSerializer()); + template.setValueSerializer(new GenericToStringSerializer(Long.class)); + template.afterPropertiesSet(); + + RedisAtomicLong ral = new RedisAtomicLong("DATAREDIS-317.atomicLong", template); + ral.set(32L); + + assertThat(ral.get(), is(32L)); + } }