From 27a7ab0738432c858fdec696cded69144ae78268 Mon Sep 17 00:00:00 2001 From: Tijn Porcelijn Date: Thu, 21 Feb 2013 17:13:31 +0100 Subject: [PATCH] Fix for (hash) increment treats long values as int While the (hash) increment() method accepts a long argument (and Redis HINCRBY/INCRBY/DECRBY treats arguments as 64 bits), it is cast to a 32bits signed int in the JedisConnection class. - Removed offensive (& unnecessary) casts - Extended integration tests with incrBy, decrBy and hIncrBy to demonstrate handling of large numbers - Fixed typos in test support (CollectionTestParams, AbstractConnectionIntegrationTests) Issue: DATAREDIS-117 --- .../connection/jedis/JedisConnection.java | 20 ++++++------- .../data/redis/cache/RedisCacheTest.java | 3 +- .../AbstractConnectionIntegrationTests.java | 4 +-- .../JedisConnectionIntegrationTests.java | 29 ++++++++++++++++++- .../collections/CollectionTestParams.java | 2 +- 5 files changed, 42 insertions(+), 16 deletions(-) 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 d0cd721ef..282d45bd8 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 @@ -1048,14 +1048,14 @@ public class JedisConnection implements RedisConnection { public Long decrBy(byte[] key, long value) { try { if (isQueueing()) { - transaction.decrBy(key, (int) value); + transaction.decrBy(key, value); return null; } if (isPipelined()) { - pipeline.decrBy(key, (int) value); + pipeline.decrBy(key, value); return null; } - return jedis.decrBy(key, (int) value); + return jedis.decrBy(key, value); } catch (Exception ex) { throw convertJedisAccessException(ex); } @@ -1082,14 +1082,14 @@ public class JedisConnection implements RedisConnection { public Long incrBy(byte[] key, long value) { try { if (isQueueing()) { - transaction.incrBy(key, (int) value); + transaction.incrBy(key, value); return null; } if (isPipelined()) { - pipeline.incrBy(key, (int) value); + pipeline.incrBy(key, value); return null; } - return jedis.incrBy(key, (int) value); + return jedis.incrBy(key, value); } catch (Exception ex) { throw convertJedisAccessException(ex); } @@ -2254,14 +2254,14 @@ public class JedisConnection implements RedisConnection { public Long hIncrBy(byte[] key, byte[] field, long delta) { try { if (isQueueing()) { - transaction.hincrBy(key, field, (int) delta); + transaction.hincrBy(key, field, delta); return null; } if (isPipelined()) { - pipeline.hincrBy(key, field, (int) delta); + pipeline.hincrBy(key, field, delta); return null; } - return jedis.hincrBy(key, field, (int) delta); + return jedis.hincrBy(key, field, delta); } catch (Exception ex) { throw convertJedisAccessException(ex); } @@ -2437,4 +2437,4 @@ public class JedisConnection implements RedisConnection { throw new RedisSubscribedConnectionException("Cannot execute command - connection is subscribed"); } } -} \ No newline at end of file +} diff --git a/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java b/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java index 4317c30ae..78adfc066 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java @@ -76,7 +76,6 @@ public class RedisCacheTest extends AbstractNativeCacheTest { public void setUp() throws Exception { ConnectionFactoryTracker.add(template.getConnectionFactory()); super.setUp(); - } @AfterClass @@ -161,4 +160,4 @@ public class RedisCacheTest extends AbstractNativeCacheTest { assertNotNull(cache); assertTrue(redisCM.getCacheNames().contains(cacheName)); } -} \ No newline at end of file +} diff --git a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java index 232f07844..f8d54d8c5 100644 --- a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java @@ -141,7 +141,7 @@ public abstract class AbstractConnectionIntegrationTests { try { connection.decr((String) null); } catch (Exception ex) { - // excepted + // expected } } @@ -389,4 +389,4 @@ public abstract class AbstractConnectionIntegrationTests { assertTrue(hashMap.containsKey(key1)); assertTrue(hashMap.containsKey(key2)); } -} \ No newline at end of file +} 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 5bbea32bf..693271708 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.junit.Assert.assertEquals; import org.junit.Test; import org.springframework.data.redis.SettingsUtils; import org.springframework.data.redis.connection.AbstractConnectionIntegrationTests; @@ -60,6 +61,32 @@ public class JedisConnectionIntegrationTests extends AbstractConnectionIntegrati System.out.println(connection.exec()); } + @Test + public void testIncrDecrBy() { + String key = "test.count"; + long largeNumber = 0x123456789L; // > 32bits + connection.set(key.getBytes(), "0".getBytes()); + connection.incrBy(key.getBytes(), largeNumber); + assertEquals(largeNumber, Long.valueOf(new String(connection.get(key.getBytes()))).longValue()); + connection.decrBy(key.getBytes(), largeNumber); + assertEquals(0, Long.valueOf(new String(connection.get(key.getBytes()))).longValue()); + connection.decrBy(key.getBytes(), 2*largeNumber); + assertEquals(-2*largeNumber, Long.valueOf(new String(connection.get(key.getBytes()))).longValue()); + } + + @Test + public void testHashIncrDecrBy() { + byte[] key = "test.hcount".getBytes(); + byte[] hkey = "hashkey".getBytes(); + + long largeNumber = 0x123456789L; // > 32bits + connection.hSet(key, hkey, "0".getBytes()); + connection.hIncrBy(key, hkey, largeNumber); + assertEquals(largeNumber, Long.valueOf(new String(connection.hGet(key, hkey))).longValue()); + connection.hIncrBy(key, hkey, -2*largeNumber); + assertEquals(-largeNumber, Long.valueOf(new String(connection.hGet(key, hkey))).longValue()); + } + // @Test // public void setAdd() { // connection.sadd("s1", "1"); @@ -87,4 +114,4 @@ public class JedisConnectionIntegrationTests extends AbstractConnectionIntegrati // for (Object object : s3) { // System.out.println(object); // } -} \ No newline at end of file +} diff --git a/src/test/java/org/springframework/data/redis/support/collections/CollectionTestParams.java b/src/test/java/org/springframework/data/redis/support/collections/CollectionTestParams.java index 3b1001a7f..970334791 100644 --- a/src/test/java/org/springframework/data/redis/support/collections/CollectionTestParams.java +++ b/src/test/java/org/springframework/data/redis/support/collections/CollectionTestParams.java @@ -169,7 +169,7 @@ public abstract class CollectionTestParams { lettuceConnFactory.setHostName(SettingsUtils.getHost()); lettuceConnFactory.afterPropertiesSet(); - RedisTemplate stringTemplateLtc = new StringRedisTemplate(srConnFactory); + RedisTemplate stringTemplateLtc = new StringRedisTemplate(lettuceConnFactory); RedisTemplate personTemplateLtc = new RedisTemplate(); personTemplateLtc.setConnectionFactory(lettuceConnFactory); personTemplateLtc.afterPropertiesSet();