diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractDecrByIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractDecrByIntegrationTest.java index 5444af5bbaa8..5bc100506324 100755 --- a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractDecrByIntegrationTest.java +++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractDecrByIntegrationTest.java @@ -14,6 +14,7 @@ */ package org.apache.geode.redis.internal.executor.string; +import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -94,6 +95,14 @@ public void testDecrBy() { assertThat(ex).isNotNull(); } + @Test + public void decrbyMoreThanMaxLongThrowsArithmeticException() { + jedis.set("somekey", "1"); + assertThatThrownBy( + () -> jedis.sendCommand(Protocol.Command.DECRBY, "somekey", "9223372036854775809")) + .hasMessageContaining(ERROR_NOT_INTEGER); + } + private String randString() { return Long.toHexString(Double.doubleToLongBits(Math.random())); } diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrIntegrationTest.java index 686fd131ad5a..9dd632da5fc1 100755 --- a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrIntegrationTest.java +++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrIntegrationTest.java @@ -14,6 +14,8 @@ */ package org.apache.geode.redis.internal.executor.string; +import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER; +import static org.apache.geode.redis.internal.RedisConstants.ERROR_OVERFLOW; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -22,10 +24,8 @@ import org.junit.Test; import redis.clients.jedis.Jedis; import redis.clients.jedis.Protocol; -import redis.clients.jedis.exceptions.JedisDataException; import org.apache.geode.redis.ConcurrentLoopingThreads; -import org.apache.geode.redis.internal.RedisConstants; import org.apache.geode.test.awaitility.GeodeAwaitility; import org.apache.geode.test.dunit.rules.RedisPortSupplier; @@ -87,11 +87,7 @@ public void testIncr_whenOverflow_shouldReturnError() { String max64BitIntegerValue = "9223372036854775807"; jedis.set(key, max64BitIntegerValue); - try { - jedis.incr(key); - } catch (JedisDataException e) { - assertThat(e.getMessage()).contains(RedisConstants.ERROR_OVERFLOW); - } + assertThatThrownBy(() -> jedis.incr(key)).hasMessageContaining(ERROR_OVERFLOW); assertThat(jedis.get(key)).isEqualTo(max64BitIntegerValue); } @@ -99,13 +95,9 @@ public void testIncr_whenOverflow_shouldReturnError() { public void testIncr_whenWrongType_shouldReturnError() { String key = "key"; String nonIntegerValue = "I am not a number! I am a free man!"; - assertThat(jedis.set(key, nonIntegerValue)).isEqualTo("OK"); - try { - jedis.incr(key); - } catch (JedisDataException e) { - assertThat(e.getMessage()).contains(RedisConstants.ERROR_NOT_INTEGER); - } + assertThat(jedis.set(key, nonIntegerValue)).isEqualTo("OK"); + assertThatThrownBy(() -> jedis.incr(key)).hasMessageContaining(ERROR_NOT_INTEGER); assertThat(jedis.get(key)).isEqualTo(nonIntegerValue); } diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractStrLenIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractStrLenIntegrationTest.java index a113f591e86b..600430a30f8c 100755 --- a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractStrLenIntegrationTest.java +++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractStrLenIntegrationTest.java @@ -83,6 +83,14 @@ public void testStrlen_requestWrongType_shouldReturnError() { .hasMessageContaining(RedisConstants.ERROR_WRONG_TYPE); } + @Test + public void testStrlen_withEmptyByte() { + byte[] key = new byte[] {0}; + jedis.set(key, new byte[] {}); + + assertThat(jedis.strlen(key)).isEqualTo(0); + } + @Test public void testStrlen_withBinaryData() { byte[] zero = new byte[] {0}; @@ -91,4 +99,22 @@ public void testStrlen_withBinaryData() { assertThat(jedis.strlen(zero)).isEqualTo(1); } + @Test + public void testStrlen_withIntData() { + byte[] key = new byte[] {0}; + byte[] value = new byte[] {1, 0, 0}; + jedis.set(key, value); + + assertThat(jedis.strlen(key)).isEqualTo(value.length); + } + + @Test + public void testStrlen_withFloatData() { + byte[] key = new byte[] {0}; + byte[] value = new byte[] {'0', '.', '9'}; + jedis.set(key, value); + + assertThat(jedis.strlen(key)).isEqualTo(value.length); + } + } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisString.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisString.java index 18b8179f9961..b75cd711dad6 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisString.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisString.java @@ -16,8 +16,6 @@ package org.apache.geode.redis.internal.data; - - import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; @@ -46,6 +44,14 @@ public RedisString(ByteArrayWrapper value) { // for serialization public RedisString() {} + public ByteArrayWrapper get() { + return new ByteArrayWrapper(value.toBytes()); + } + + public void set(ByteArrayWrapper value) { + valueSet(value); + } + public int append(ByteArrayWrapper appendValue, Region region, ByteArrayWrapper key) { @@ -55,14 +61,6 @@ public int append(ByteArrayWrapper appendValue, return value.length(); } - public ByteArrayWrapper get() { - return new ByteArrayWrapper(value.toBytes()); - } - - public void set(ByteArrayWrapper value) { - valueSet(value); - } - public long incr(Region region, ByteArrayWrapper key) throws NumberFormatException, ArithmeticException { long longValue = parseValueAsLong(); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/DecrByExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/DecrByExecutor.java index 85c2c49ce55b..b2b48d172147 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/DecrByExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/DecrByExecutor.java @@ -15,6 +15,8 @@ package org.apache.geode.redis.internal.executor.string; +import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER; + import java.util.List; import org.apache.geode.redis.internal.data.ByteArrayWrapper; @@ -25,9 +27,6 @@ public class DecrByExecutor extends StringExecutor { - private static final String ERROR_DECREMENT_NOT_USABLE = - "The decrementation on this key must be numeric"; - private static final int DECREMENT_INDEX = 2; @Override @@ -42,7 +41,7 @@ public RedisResponse executeCommand(Command command, ExecutionHandlerContext con try { decrement = Long.parseLong(decrString); } catch (NumberFormatException e) { - return RedisResponse.error(ERROR_DECREMENT_NOT_USABLE); + return RedisResponse.error(ERROR_NOT_INTEGER); } long value = getRedisStringCommands(context).decrby(key, decrement); diff --git a/geode-redis/src/test/java/org/apache/geode/redis/internal/data/RedisStringTest.java b/geode-redis/src/test/java/org/apache/geode/redis/internal/data/RedisStringTest.java index 6859ba00c14f..e214354f1b38 100644 --- a/geode-redis/src/test/java/org/apache/geode/redis/internal/data/RedisStringTest.java +++ b/geode-redis/src/test/java/org/apache/geode/redis/internal/data/RedisStringTest.java @@ -17,6 +17,7 @@ package org.apache.geode.redis.internal.data; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; import java.io.IOException; @@ -41,16 +42,256 @@ public static void beforeClass() { } @Test - public void confirmSerializationIsStable() throws IOException, ClassNotFoundException { - RedisString o1 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1, 2, 3})); - o1.setExpirationTimestampNoDelta(1000); + public void constructorSetsValue() { + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {0, 1, 2}); + RedisString string = new RedisString(byteArrayWrapper); + ByteArrayWrapper returnedByteArrayWrapper = string.get(); + assertThat(returnedByteArrayWrapper).isNotNull(); + assertThat(returnedByteArrayWrapper.value).isEqualTo(byteArrayWrapper.value); + } + + @Test + public void setSetsValue() { + RedisString string = new RedisString(); + string.set(new ByteArrayWrapper(new byte[] {0, 1, 2})); + ByteArrayWrapper returnedByteArrayWrapper = string.get(); + assertThat(returnedByteArrayWrapper).isNotNull(); + assertThat(returnedByteArrayWrapper.value) + .isEqualTo(new ByteArrayWrapper(new byte[] {0, 1, 2}).value); + } + + @Test + public void getReturnsSetValue() { + RedisString string = new RedisString(new ByteArrayWrapper(new byte[] {0, 1})); + ByteArrayWrapper returnedByteArrayWrapper = string.get(); + assertThat(returnedByteArrayWrapper).isNotNull(); + assertThat(returnedByteArrayWrapper.value) + .isEqualTo(new ByteArrayWrapper(new byte[] {0, 1}).value); + } + + @Test + public void appendResizesByteArray() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + RedisString redisString = new RedisString(new ByteArrayWrapper(new byte[] {0, 1})); + ByteArrayWrapper part2 = new ByteArrayWrapper(new byte[] {2, 3, 4, 5}); + int redisStringSize = redisString.strlen(); + int part2Size = part2.length(); + int appendedStringSize = redisString.append(part2, region, null); + assertThat(appendedStringSize).isEqualTo(redisStringSize + part2Size); + } + + @Test + public void appendStoresStableDelta() throws IOException { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + RedisString o1 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1})); + ByteArrayWrapper part2 = new ByteArrayWrapper(new byte[] {2, 3}); + o1.append(part2, region, null); + assertThat(o1.hasDelta()).isTrue(); + assertThat(o1.get()).isEqualTo(new ByteArrayWrapper(new byte[] {0, 1, 2, 3})); HeapDataOutputStream out = new HeapDataOutputStream(100); - DataSerializer.writeObject(o1, out); + o1.toDelta(out); + assertThat(o1.hasDelta()).isFalse(); ByteArrayDataInput in = new ByteArrayDataInput(out.toByteArray()); - RedisString o2 = DataSerializer.readObject(in); + RedisString o2 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1})); + assertThat(o2).isNotEqualTo(o1); + o2.fromDelta(in); + assertThat(o2.get()).isEqualTo(new ByteArrayWrapper(new byte[] {0, 1, 2, 3})); assertThat(o2).isEqualTo(o1); } + @Test + public void serializationIsStable() throws IOException, ClassNotFoundException { + RedisString o1 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1, 2, 3})); + o1.setExpirationTimestampNoDelta(1000); + HeapDataOutputStream outputStream = new HeapDataOutputStream(100); + DataSerializer.writeObject(o1, outputStream); + ByteArrayDataInput dataInput = new ByteArrayDataInput(outputStream.toByteArray()); + RedisString o2 = DataSerializer.readObject(dataInput); + assertThat(o2).isEqualTo(o1); + } + + @Test + public void incrThrowsArithmeticErrorWhenNotALong() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0', ' ', '1'}); + RedisString string = new RedisString(byteArrayWrapper); + assertThatThrownBy(() -> string.incr(region, byteArrayWrapper)) + .isInstanceOf(NumberFormatException.class); + } + + @Test + public void incrErrorsWhenValueOverflows() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper( + // max value for signed long + new byte[] {'9', '2', '2', '3', '3', '7', '2', '0', '3', '6', '8', '5', '4', '7', '7', '5', + '8', '0', '7'}); + RedisString string = new RedisString(byteArrayWrapper); + assertThatThrownBy(() -> string.incr(region, byteArrayWrapper)) + .isInstanceOf(ArithmeticException.class); + } + + @Test + public void incrIncrementsValueAtGivenKey() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0'}); + RedisString string = new RedisString(byteArrayWrapper); + string.incr(region, byteArrayWrapper); + assertThat(string.get().toString()).isEqualTo("11"); + } + + @Test + public void incrbyThrowsNumberFormatExceptionWhenNotALong() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0', ' ', '1'}); + RedisString string = new RedisString(byteArrayWrapper); + assertThatThrownBy(() -> string.incrby(region, byteArrayWrapper, 2L)) + .isInstanceOf(NumberFormatException.class); + } + + @Test + public void incrbyErrorsWhenValueOverflows() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper( + // max value for signed long + new byte[] {'9', '2', '2', '3', '3', '7', '2', '0', '3', '6', '8', '5', '4', '7', '7', '5', + '8', '0', '7'}); + RedisString string = new RedisString(byteArrayWrapper); + assertThatThrownBy(() -> string.incrby(region, byteArrayWrapper, 2L)) + .isInstanceOf(ArithmeticException.class); + } + + @Test + public void incrbyIncrementsValueByGivenLong() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0'}); + RedisString string = new RedisString(byteArrayWrapper); + string.incrby(region, byteArrayWrapper, 2L); + assertThat(string.get().toString()).isEqualTo("12"); + } + + @Test + public void incrbyfloatThrowsArithmeticErrorWhenNotADouble() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0', ' ', '1'}); + RedisString string = new RedisString(byteArrayWrapper); + assertThatThrownBy(() -> string.incrbyfloat(region, byteArrayWrapper, 1.1)) + .isInstanceOf(NumberFormatException.class); + } + + @Test + public void incrbyfloatIncrementsValueByGivenFloat() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0'}); + RedisString string = new RedisString(byteArrayWrapper); + string.incrbyfloat(region, byteArrayWrapper, 2.20); + assertThat(string.get().toString()).isEqualTo("12.2"); + } + + @Test + public void decrThrowsNumberFormatExceptionWhenNotALong() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {0}); + RedisString string = new RedisString(byteArrayWrapper); + assertThatThrownBy(() -> string.decr(region, byteArrayWrapper)) + .isInstanceOf(NumberFormatException.class); + } + + @Test + public void decrThrowsArithmeticExceptionWhenDecrementingMin() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper( + new byte[] {'-', '9', '2', '2', '3', '3', '7', '2', '0', '3', '6', '8', '5', '4', '7', '7', + '5', + '8', '0', '8'}); + RedisString string = new RedisString(byteArrayWrapper); + assertThatThrownBy(() -> string.decr(region, byteArrayWrapper)) + .isInstanceOf(ArithmeticException.class); + } + + @Test + public void decrDecrementsValue() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0'}); + RedisString string = new RedisString(byteArrayWrapper); + string.decr(region, byteArrayWrapper); + assertThat(string.get().toString()).isEqualTo("9"); + } + + @Test + public void decrbyThrowsNumberFormatExceptionWhenNotALong() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {1}); + RedisString string = new RedisString(byteArrayWrapper); + assertThatThrownBy(() -> string.decrby(region, byteArrayWrapper, 2)) + .isInstanceOf(NumberFormatException.class); + } + + @Test + public void decrbyThrowsArithmeticExceptionWhenDecrementingMin() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper( + new byte[] {'-', '9', '2', '2', '3', '3', '7', '2', '0', '3', '6', '8', '5', '4', '7', '7', + '5', + '8', '0', '7'}); + RedisString string = new RedisString(byteArrayWrapper); + assertThatThrownBy(() -> string.decrby(region, byteArrayWrapper, 2)) + .isInstanceOf(ArithmeticException.class); + } + + @Test + public void decrbyDecrementsValue() { + // allows unchecked cast of mock to Region + @SuppressWarnings("unchecked") + Region region = mock(Region.class); + ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0'}); + RedisString string = new RedisString(byteArrayWrapper); + string.decrby(region, byteArrayWrapper, 2); + assertThat(string.get().toString()).isEqualTo("8"); + } + + @Test + public void strlenReturnsStringLength() { + RedisString string = new RedisString(new ByteArrayWrapper(new byte[] {1, 2, 3, 4})); + assertThat(string.strlen()).isEqualTo(4); + } + + @Test + public void strlenReturnsLengthOfEmptyString() { + RedisString string = new RedisString(new ByteArrayWrapper(new byte[] {})); + assertThat(string.strlen()).isEqualTo(0); + } + @Test public void equals_returnsFalse_givenDifferentExpirationTimes() { RedisString o1 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1, 2, 3})); @@ -78,25 +319,6 @@ public void equals_returnsTrue_givenEqualValueBytesAndExpiration() { assertThat(o1).isEqualTo(o2); } - @SuppressWarnings("unchecked") - @Test - public void append_stores_delta_that_is_stable() throws IOException { - Region region = mock(Region.class); - RedisString o1 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1})); - ByteArrayWrapper part2 = new ByteArrayWrapper(new byte[] {2, 3}); - o1.append(part2, region, null); - assertThat(o1.hasDelta()).isTrue(); - assertThat(o1.get()).isEqualTo(new ByteArrayWrapper(new byte[] {0, 1, 2, 3})); - HeapDataOutputStream out = new HeapDataOutputStream(100); - o1.toDelta(out); - assertThat(o1.hasDelta()).isFalse(); - ByteArrayDataInput in = new ByteArrayDataInput(out.toByteArray()); - RedisString o2 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1})); - assertThat(o2).isNotEqualTo(o1); - o2.fromDelta(in); - assertThat(o2.get()).isEqualTo(new ByteArrayWrapper(new byte[] {0, 1, 2, 3})); - assertThat(o2).isEqualTo(o1); - } @SuppressWarnings("unchecked") @Test