diff --git a/gradle.properties b/gradle.properties index f09fb8eb53..e49ffa81b5 100644 --- a/gradle.properties +++ b/gradle.properties @@ -4,7 +4,7 @@ jredisVersion=06052013 jedisVersion=2.4.1 springVersion=3.2.8.RELEASE log4jVersion=1.2.17 -version=1.3.0.BUILD-SNAPSHOT +version=1.3.0.DATAREDIS-286-SNAPSHOT srpVersion=0.7 jacksonVersion=1.8.8 fasterXmlJacksonVersion=2.2.0 diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java index 26d51b3388..5c5216798e 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java @@ -26,6 +26,7 @@ import java.util.Properties; import java.util.Queue; import java.util.Set; +import java.util.concurrent.TimeUnit; import org.springframework.core.convert.converter.Converter; import org.springframework.dao.DataAccessException; @@ -737,6 +738,18 @@ public Boolean exists(byte[] key) { } public Boolean expire(byte[] key, long seconds) { + + /* + * @see DATAREDIS-286 to avoid overflow in Jedis + * + * TODO Remove this workaround when we upgrade to a Jedis version that contains a + * fix for: https://github.com/xetorthio/jedis/pull/575 + */ + if (seconds > Integer.MAX_VALUE) { + + return pExpireAt(key, time() + TimeUnit.SECONDS.toMillis(seconds)); + } + try { if (isPipelined()) { pipeline(new JedisResult(pipeline.expire(key, (int) seconds), JedisConverters.longToBoolean())); @@ -912,6 +925,18 @@ public Long ttl(byte[] key) { } public Boolean pExpire(byte[] key, long millis) { + + /* + * @see DATAREDIS-286 to avoid overflow in Jedis + * + * TODO Remove this workaround when we upgrade to a Jedis version that contains a + * fix for: https://github.com/xetorthio/jedis/pull/575 + */ + if (millis > Integer.MAX_VALUE) { + + return pExpireAt(key, time() + millis); + } + try { if (isPipelined()) { pipeline(new JedisResult(pipeline.pexpire(key, (int) millis), JedisConverters.longToBoolean())); diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionIntegrationTests.java index 3419df7b69..026a9a6d82 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionIntegrationTests.java @@ -16,6 +16,7 @@ package org.springframework.data.redis.connection.jedis; +import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; import java.util.HashSet; @@ -333,4 +334,35 @@ public void testExecuteShouldConvertArrayReplyCorrectly() { AllOf.allOf(IsInstanceOf.instanceOf(List.class), IsCollectionContaining.hasItems("awesome".getBytes(), "cool".getBytes(), "supercalifragilisticexpialidocious".getBytes()))); } + + /** + * @see DATAREDIS-286 + */ + @Test + public void expireShouldSupportExiprationForValuesLargerThanInteger() { + + connection.set("expireKey", "foo"); + + long seconds = ((long) Integer.MAX_VALUE) + 1; + connection.expire("expireKey", seconds); + long ttl = connection.ttl("expireKey"); + + assertThat(ttl, is(seconds)); + } + + /** + * @see DATAREDIS-286 + */ + @Test + public void pExpireShouldSupportExiprationForValuesLargerThanInteger() { + + connection.set("pexpireKey", "foo"); + + long millis = ((long) Integer.MAX_VALUE) + 10; + connection.pExpire("pexpireKey", millis); + long ttl = connection.pTtl("pexpireKey"); + + assertTrue(String.format("difference between millis=%s and ttl=%s should not be greater than 20ms but is %s", + millis, ttl, millis - ttl), millis - ttl < 20L); + } } diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTestSuite.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTestSuite.java index 545ad2e4ef..b965dc643c 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTestSuite.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTestSuite.java @@ -17,6 +17,11 @@ import static org.hamcrest.core.IsEqual.*; import static org.junit.Assert.*; +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; + +import java.util.Arrays; +import java.util.concurrent.TimeUnit; import org.junit.Before; import org.junit.Test; @@ -24,11 +29,11 @@ import org.junit.runners.Suite; import org.junit.runners.Suite.SuiteClasses; import org.mockito.ArgumentCaptor; -import org.mockito.Matchers; import org.springframework.data.redis.connection.AbstractConnectionUnitTestBase; import org.springframework.data.redis.connection.RedisServerCommands.ShutdownOption; import org.springframework.data.redis.connection.jedis.JedisConnectionUnitTestSuite.JedisConnectionPipelineUnitTests; import org.springframework.data.redis.connection.jedis.JedisConnectionUnitTestSuite.JedisConnectionUnitTests; +import org.springframework.data.redis.core.TimeoutUtils; import redis.clients.jedis.Client; import redis.clients.jedis.Jedis; @@ -43,10 +48,13 @@ public class JedisConnectionUnitTestSuite { public static class JedisConnectionUnitTests extends AbstractConnectionUnitTestBase { protected JedisConnection connection; + private Jedis jedisSpy; @Before public void setUp() { - connection = new JedisConnection(new MockedClientJedis("http://localhost:1234", getNativeRedisConnectionMock())); + + jedisSpy = spy(new MockedClientJedis("http://localhost:1234", getNativeRedisConnectionMock())); + connection = new JedisConnection(jedisSpy); } /** @@ -69,8 +77,7 @@ public void shutdownNosaveShouldBeSentCorrectlyUsingLuaScript() { connection.shutdown(ShutdownOption.NOSAVE); ArgumentCaptor captor = ArgumentCaptor.forClass(byte[].class); - verifyNativeConnectionInvocation().eval(captor.capture(), Matchers.any(byte[].class), - Matchers.any(byte[][].class)); + verifyNativeConnectionInvocation().eval(captor.capture(), any(byte[].class), any(byte[][].class)); assertThat(captor.getValue(), equalTo("return redis.call('SHUTDOWN','NOSAVE')".getBytes())); } @@ -84,12 +91,28 @@ public void shutdownSaveShouldBeSentCorrectlyUsingLuaScript() { connection.shutdown(ShutdownOption.SAVE); ArgumentCaptor captor = ArgumentCaptor.forClass(byte[].class); - verifyNativeConnectionInvocation().eval(captor.capture(), Matchers.any(byte[].class), - Matchers.any(byte[][].class)); + verifyNativeConnectionInvocation().eval(captor.capture(), any(byte[].class), any(byte[][].class)); assertThat(captor.getValue(), equalTo("return redis.call('SHUTDOWN','SAVE')".getBytes())); } + /** + * @see DATAREDIS-286 + */ + @Test + public void pExpireHavingIntOverflowShouldUseRedisServerTimeAsReferenceForPExpireAt() { + + long msec = Long.valueOf((long) Integer.MAX_VALUE + 1); + long expected = msec + TimeoutUtils.toMillis(1, TimeUnit.SECONDS); + + /* redis time as list containing [0] = seconds, [1] = microseconds + * @see http://redis.io/commands/time + */ + when(jedisSpy.time()).thenReturn(Arrays.asList("1", "0")); + + connection.pExpire("foo".getBytes(), msec); + verifyNativeConnectionInvocation().pexpireAt(any(byte[].class), eq(expected)); + } } public static class JedisConnectionPipelineUnitTests extends JedisConnectionUnitTests { @@ -129,5 +152,6 @@ public MockedClientJedis(String host, Client client) { super(host); this.client = client; } + } }