DATAREDIS-286 - Avoid overflow in JedisConnection#expire / #pexpire.

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

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()));

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);
+	}
 }

src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTestSuite.java

@@ -17,18 +17,23 @@
 
 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;
 import org.junit.runner.RunWith;
 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<Client> {
 
 		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<byte[]> 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<byte[]> 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;
 		}
+
 	}
 }