DonalEvans commented on a change in pull request #6605:
URL: https://github.com/apache/geode/pull/6605#discussion_r651172298
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisData.java
##########
@@ -89,6 +96,25 @@ public boolean rename(Region<RedisKey, RedisData> region,
RedisKey oldKey, Redis
return false;
}
+ @Override
+ public byte[] dump() throws IOException {
Review comment:
This method never throws an `IOException` so that can be removed.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/key/DumpExecutor.java
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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.apache.geode.redis.internal.executor.key;
+
+import java.util.List;
+
+import org.apache.geode.redis.internal.data.RedisKey;
+import org.apache.geode.redis.internal.executor.AbstractExecutor;
+import org.apache.geode.redis.internal.executor.RedisResponse;
+import org.apache.geode.redis.internal.netty.Command;
+import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+
+public class DumpExecutor extends AbstractExecutor {
+ @Override
+ public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context)
+ throws Exception {
Review comment:
This method does not throw an `Exception` so this can be removed.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/key/RestoreExecutor.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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.apache.geode.redis.internal.executor.key;
+
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_INVALID_TTL;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static
org.apache.geode.redis.internal.RedisConstants.ERROR_RESTORE_KEY_EXISTS;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+
+import java.util.List;
+
+import org.apache.geode.redis.internal.RedisException;
+import org.apache.geode.redis.internal.RedisRestoreKeyExistsException;
+import org.apache.geode.redis.internal.data.RedisKey;
+import org.apache.geode.redis.internal.executor.AbstractExecutor;
+import org.apache.geode.redis.internal.executor.RedisResponse;
+import org.apache.geode.redis.internal.netty.Coder;
+import org.apache.geode.redis.internal.netty.Command;
+import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+
+public class RestoreExecutor extends AbstractExecutor {
+
+ private static final int TTL_INDEX = 2;
+
+ @Override
+ public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context)
+ throws Exception {
Review comment:
This method doesn't throw an `Exception` so this can be removed.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/key/RestoreExecutor.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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.apache.geode.redis.internal.executor.key;
+
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_INVALID_TTL;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static
org.apache.geode.redis.internal.RedisConstants.ERROR_RESTORE_KEY_EXISTS;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+
+import java.util.List;
+
+import org.apache.geode.redis.internal.RedisException;
+import org.apache.geode.redis.internal.RedisRestoreKeyExistsException;
+import org.apache.geode.redis.internal.data.RedisKey;
+import org.apache.geode.redis.internal.executor.AbstractExecutor;
+import org.apache.geode.redis.internal.executor.RedisResponse;
+import org.apache.geode.redis.internal.netty.Coder;
+import org.apache.geode.redis.internal.netty.Command;
+import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+
+public class RestoreExecutor extends AbstractExecutor {
+
+ private static final int TTL_INDEX = 2;
+
+ @Override
+ public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context)
+ throws Exception {
+ List<byte[]> commandElems = command.getProcessedCommand();
+ RedisKey key = command.getKey();
+
+ byte[] ttlByteArray = commandElems.get(TTL_INDEX);
+ long ttl;
+ try {
+ ttl = Coder.bytesToLong(ttlByteArray);
+ } catch (NumberFormatException e) {
+ return RedisResponse.error(ERROR_NOT_INTEGER);
+ }
+
+ if (ttl < 0) {
+ return RedisResponse.error(ERROR_INVALID_TTL);
+ }
+
+ RestoreOptions options;
+ if (commandElems.size() > 4) {
+ options = parseOptions(commandElems.subList(4, commandElems.size()));
+ } else {
+ options = new RestoreOptions();
+ }
+
+ try {
+ context.getKeyCommands().restore(key, ttl, commandElems.get(3), options);
+ } catch (RedisRestoreKeyExistsException redisRestoreKeyExistsException) {
+ return RedisResponse.busykey(ERROR_RESTORE_KEY_EXISTS);
Review comment:
Will the busy key response ever have a message other than
`ERROR_RESTORE_KEY_EXISTS`? If not, it might be better to pull that constant
down into the `getBusyKeyResponse()` method.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java
##########
@@ -234,7 +235,10 @@ private RedisResponse
getExceptionResponse(ChannelHandlerContext ctx, Throwable
}
}
- if (cause instanceof NumberFormatException) {
+
+ if (cause instanceof RedisException) {
+ response = RedisResponse.error(cause.getMessage());
+ } else if (cause instanceof NumberFormatException) {
response = RedisResponse.error(cause.getMessage());
} else if (cause instanceof ArithmeticException) {
response = RedisResponse.error(cause.getMessage());
Review comment:
These three cases all have the same behaviour (calling
`RedisResponse.error(cause.getMessage())`) which is already found further down
in this if/else block. Would it make sense to combine all the checks into one
case:
```
} else if (cause instanceof IllegalStateException
|| cause instanceof RedisParametersMismatchException
|| cause instanceof RedisException
|| cause instanceof NumberFormatException
|| cause instanceof ArithmeticException) {
response = RedisResponse.error(cause.getMessage());
```
##########
File path:
geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/AbstractDumpRestoreIntegrationTest.java
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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.apache.geode.redis.internal.executor.key;
+
+import static
org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertExactNumberOfArgs;
+import static
org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import io.lettuce.core.KeyValue;
+import io.lettuce.core.RestoreArgs;
+import io.lettuce.core.cluster.RedisClusterClient;
+import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+
+import org.apache.geode.internal.InternalDataSerializer;
+import org.apache.geode.internal.serialization.DataSerializableFixedID;
+import org.apache.geode.redis.RedisIntegrationTest;
+import org.apache.geode.redis.internal.data.RedisHash;
+import org.apache.geode.redis.internal.data.RedisSet;
+import org.apache.geode.redis.internal.data.RedisSortedSet;
+import org.apache.geode.redis.internal.data.RedisString;
+
+public abstract class AbstractDumpRestoreIntegrationTest implements
RedisIntegrationTest {
+
+ private RedisAdvancedClusterCommands<String, String> lettuce;
+ private JedisCluster jedis;
+ private static String STRING_VALUE;
+ private static byte[] RESTORE_BYTES;
+
+ @BeforeClass
+ public static void setupClass() {
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_SET_ID,
+ RedisSet.class);
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_STRING_ID,
+ RedisString.class);
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_HASH_ID,
+ RedisHash.class);
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_SORTED_SET_ID,
+ RedisSortedSet.class);
+ }
+
+ @Before
+ public void setup() {
+ jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()),
REDIS_CLIENT_TIMEOUT);
+
+ RedisClusterClient client =
+ RedisClusterClient.create(String.format("redis://%s:%d", BIND_ADDRESS,
getPort()));
+ lettuce = client.connect().sync();
+
+ STRING_VALUE = "It's a mad, mad, mad, mad, mad world";
+ lettuce.set("set-dump-value", STRING_VALUE);
+ RESTORE_BYTES = lettuce.dump("set-dump-value");
+ }
+
+ @After
+ public void tearDown() {
+ flushAll();
+ }
+
+ @Test
+ public void dumpTakesExactlyOneArgument() {
+ assertExactNumberOfArgs(jedis, Protocol.Command.DUMP, 1);
+ }
+
+ @Test
+ public void restoreErrorsWithUnknownOption() {
+ assertThatThrownBy(
+ () -> jedis.sendCommand("key", Protocol.Command.RESTORE, "key", "0",
"", "FOO"))
+ .hasMessageContaining("ERR syntax error");
+ }
+
+ @Test
+ public void restoreFails_whenKeyAlreadyExists() {
+ lettuce.set("restored", "already exists");
+
+ assertThatThrownBy(() -> lettuce.restore("restored", 0, RESTORE_BYTES))
+ .hasMessage("BUSYKEY Target key name already exists.");
+ }
+
+ @Test
+ public void restoreFails_whenTTLisNegative() {
+ assertThatThrownBy(() -> lettuce.restore("restored", -1, RESTORE_BYTES))
+ .hasMessage("ERR Invalid TTL value, must be >= 0");
+ }
+
+ @Test
+ public void restoreFails_withInvalidBytes() {
+ assertThatThrownBy(() -> lettuce.restore("restored", 0, new byte[] {0, 1,
2, 3}))
+ .hasMessage("ERR DUMP payload version or checksum are wrong");
+ }
+
+ @Test
+ public void dumpAndRestoreString() {
+ lettuce.set("dumped", STRING_VALUE);
+
+ byte[] rawBytes = lettuce.dump("dumped");
+ lettuce.restore("restored", 0, rawBytes);
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.ttl("restored")).isEqualTo(-1);
+ }
+
+ @Test
+ public void restore_withTTL_setsTTL() {
+ lettuce.restore("restored", 2000, RESTORE_BYTES);
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.pttl("restored"))
+ .isLessThan(2000)
Review comment:
It seems unlikely, but could this test possibly fail due to reaching
this assertion before the ttl has had time to decrease from the initially set
value? This assertion could then be `.isLessThatOrEqualTo(2000)` to guard
against that.
##########
File path:
geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/AbstractDumpRestoreIntegrationTest.java
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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.apache.geode.redis.internal.executor.key;
+
+import static
org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertExactNumberOfArgs;
+import static
org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import io.lettuce.core.KeyValue;
+import io.lettuce.core.RestoreArgs;
+import io.lettuce.core.cluster.RedisClusterClient;
+import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+
+import org.apache.geode.internal.InternalDataSerializer;
+import org.apache.geode.internal.serialization.DataSerializableFixedID;
+import org.apache.geode.redis.RedisIntegrationTest;
+import org.apache.geode.redis.internal.data.RedisHash;
+import org.apache.geode.redis.internal.data.RedisSet;
+import org.apache.geode.redis.internal.data.RedisSortedSet;
+import org.apache.geode.redis.internal.data.RedisString;
+
+public abstract class AbstractDumpRestoreIntegrationTest implements
RedisIntegrationTest {
+
+ private RedisAdvancedClusterCommands<String, String> lettuce;
+ private JedisCluster jedis;
+ private static String STRING_VALUE;
+ private static byte[] RESTORE_BYTES;
+
+ @BeforeClass
+ public static void setupClass() {
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_SET_ID,
+ RedisSet.class);
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_STRING_ID,
+ RedisString.class);
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_HASH_ID,
+ RedisHash.class);
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_SORTED_SET_ID,
+ RedisSortedSet.class);
+ }
+
+ @Before
+ public void setup() {
+ jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()),
REDIS_CLIENT_TIMEOUT);
+
+ RedisClusterClient client =
+ RedisClusterClient.create(String.format("redis://%s:%d", BIND_ADDRESS,
getPort()));
+ lettuce = client.connect().sync();
+
+ STRING_VALUE = "It's a mad, mad, mad, mad, mad world";
+ lettuce.set("set-dump-value", STRING_VALUE);
+ RESTORE_BYTES = lettuce.dump("set-dump-value");
+ }
+
+ @After
+ public void tearDown() {
+ flushAll();
+ }
+
+ @Test
+ public void dumpTakesExactlyOneArgument() {
+ assertExactNumberOfArgs(jedis, Protocol.Command.DUMP, 1);
+ }
+
+ @Test
+ public void restoreErrorsWithUnknownOption() {
+ assertThatThrownBy(
+ () -> jedis.sendCommand("key", Protocol.Command.RESTORE, "key", "0",
"", "FOO"))
+ .hasMessageContaining("ERR syntax error");
+ }
+
+ @Test
+ public void restoreFails_whenKeyAlreadyExists() {
+ lettuce.set("restored", "already exists");
+
+ assertThatThrownBy(() -> lettuce.restore("restored", 0, RESTORE_BYTES))
+ .hasMessage("BUSYKEY Target key name already exists.");
+ }
+
+ @Test
+ public void restoreFails_whenTTLisNegative() {
+ assertThatThrownBy(() -> lettuce.restore("restored", -1, RESTORE_BYTES))
+ .hasMessage("ERR Invalid TTL value, must be >= 0");
+ }
+
+ @Test
+ public void restoreFails_withInvalidBytes() {
+ assertThatThrownBy(() -> lettuce.restore("restored", 0, new byte[] {0, 1,
2, 3}))
+ .hasMessage("ERR DUMP payload version or checksum are wrong");
+ }
+
+ @Test
+ public void dumpAndRestoreString() {
+ lettuce.set("dumped", STRING_VALUE);
+
+ byte[] rawBytes = lettuce.dump("dumped");
+ lettuce.restore("restored", 0, rawBytes);
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.ttl("restored")).isEqualTo(-1);
+ }
+
+ @Test
+ public void restore_withTTL_setsTTL() {
+ lettuce.restore("restored", 2000, RESTORE_BYTES);
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.pttl("restored"))
+ .isLessThan(2000)
+ .isGreaterThan(0);
+ }
+
+ @Test
+ public void restore_withReplace() {
+ lettuce.set("restored", "already exists");
+ lettuce.expire("restored", 10);
+
+ lettuce.restore("restored", RESTORE_BYTES, new RestoreArgs().replace());
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.pttl("restored")).isEqualTo(-1);
+ }
+
+ @Test
+ public void restore_withAbsTTL() {
+ long absttl = System.currentTimeMillis() + 10000;
+ lettuce.restore("restored", RESTORE_BYTES, new
RestoreArgs().ttl(absttl).absttl());
+
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.ttl("restored")).isGreaterThan(1);
Review comment:
Would it be possible to also add an assertion here that ttl is less than
the expected value, similar to what's done in the `restore_withTTL_setsTTL()`
test?
##########
File path:
geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/AbstractDumpRestoreIntegrationTest.java
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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.apache.geode.redis.internal.executor.key;
+
+import static
org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertExactNumberOfArgs;
+import static
org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import io.lettuce.core.KeyValue;
+import io.lettuce.core.RestoreArgs;
+import io.lettuce.core.cluster.RedisClusterClient;
+import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+
+import org.apache.geode.internal.InternalDataSerializer;
+import org.apache.geode.internal.serialization.DataSerializableFixedID;
+import org.apache.geode.redis.RedisIntegrationTest;
+import org.apache.geode.redis.internal.data.RedisHash;
+import org.apache.geode.redis.internal.data.RedisSet;
+import org.apache.geode.redis.internal.data.RedisSortedSet;
+import org.apache.geode.redis.internal.data.RedisString;
+
+public abstract class AbstractDumpRestoreIntegrationTest implements
RedisIntegrationTest {
+
+ private RedisAdvancedClusterCommands<String, String> lettuce;
+ private JedisCluster jedis;
+ private static String STRING_VALUE;
+ private static byte[] RESTORE_BYTES;
+
+ @BeforeClass
+ public static void setupClass() {
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_SET_ID,
+ RedisSet.class);
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_STRING_ID,
+ RedisString.class);
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_HASH_ID,
+ RedisHash.class);
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_SORTED_SET_ID,
+ RedisSortedSet.class);
+ }
+
+ @Before
+ public void setup() {
+ jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()),
REDIS_CLIENT_TIMEOUT);
+
+ RedisClusterClient client =
+ RedisClusterClient.create(String.format("redis://%s:%d", BIND_ADDRESS,
getPort()));
+ lettuce = client.connect().sync();
+
+ STRING_VALUE = "It's a mad, mad, mad, mad, mad world";
+ lettuce.set("set-dump-value", STRING_VALUE);
+ RESTORE_BYTES = lettuce.dump("set-dump-value");
+ }
+
+ @After
+ public void tearDown() {
+ flushAll();
+ }
+
+ @Test
+ public void dumpTakesExactlyOneArgument() {
+ assertExactNumberOfArgs(jedis, Protocol.Command.DUMP, 1);
+ }
+
+ @Test
+ public void restoreErrorsWithUnknownOption() {
+ assertThatThrownBy(
+ () -> jedis.sendCommand("key", Protocol.Command.RESTORE, "key", "0",
"", "FOO"))
+ .hasMessageContaining("ERR syntax error");
+ }
+
+ @Test
+ public void restoreFails_whenKeyAlreadyExists() {
+ lettuce.set("restored", "already exists");
+
+ assertThatThrownBy(() -> lettuce.restore("restored", 0, RESTORE_BYTES))
+ .hasMessage("BUSYKEY Target key name already exists.");
+ }
+
+ @Test
+ public void restoreFails_whenTTLisNegative() {
+ assertThatThrownBy(() -> lettuce.restore("restored", -1, RESTORE_BYTES))
+ .hasMessage("ERR Invalid TTL value, must be >= 0");
+ }
+
+ @Test
+ public void restoreFails_withInvalidBytes() {
+ assertThatThrownBy(() -> lettuce.restore("restored", 0, new byte[] {0, 1,
2, 3}))
+ .hasMessage("ERR DUMP payload version or checksum are wrong");
+ }
+
+ @Test
+ public void dumpAndRestoreString() {
+ lettuce.set("dumped", STRING_VALUE);
+
+ byte[] rawBytes = lettuce.dump("dumped");
+ lettuce.restore("restored", 0, rawBytes);
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.ttl("restored")).isEqualTo(-1);
+ }
+
+ @Test
+ public void restore_withTTL_setsTTL() {
+ lettuce.restore("restored", 2000, RESTORE_BYTES);
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.pttl("restored"))
+ .isLessThan(2000)
+ .isGreaterThan(0);
+ }
+
+ @Test
+ public void restore_withReplace() {
+ lettuce.set("restored", "already exists");
+ lettuce.expire("restored", 10);
+
+ lettuce.restore("restored", RESTORE_BYTES, new RestoreArgs().replace());
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.pttl("restored")).isEqualTo(-1);
+ }
+
+ @Test
+ public void restore_withAbsTTL() {
+ long absttl = System.currentTimeMillis() + 10000;
+ lettuce.restore("restored", RESTORE_BYTES, new
RestoreArgs().ttl(absttl).absttl());
+
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.ttl("restored")).isGreaterThan(1);
+ }
+
+ @Test
+ public void restore_withAbsTTL_ofZero() {
+ lettuce.restore("restored", RESTORE_BYTES, new
RestoreArgs().ttl(0).absttl());
+
+ String response = lettuce.get("restored");
+
+ assertThat(lettuce.ttl("restored")).isEqualTo(-1);
+ assertThat(response).isEqualTo(STRING_VALUE);
+ }
+
+ @Test
+ public void restore_withReplaceAndAbsttl() {
+ lettuce.set("restored", "already exists");
+ lettuce.expire("restored", 5);
+ long absttl = System.currentTimeMillis() + 10000;
+ lettuce.restore("restored", RESTORE_BYTES, new
RestoreArgs().ttl(absttl).absttl().replace());
+
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.ttl("restored")).isGreaterThan(5);
+ }
+
+ @Test
+ public void dumpAndRestoreSet() {
+ Set<String> smembers = new HashSet<>();
+ for (int i = 0; i < 100; i++) {
+ lettuce.sadd("set", "member-" + i);
+ smembers.add("member-" + i);
+ }
+ byte[] dump = lettuce.dump("set");
+
+ lettuce.restore("restored", 0, dump);
+ Set<String> result = lettuce.smembers("restored");
+
+ assertThat(result).containsExactlyInAnyOrderElementsOf(smembers);
+ }
+
+ @Test
+ public void dumpAndRestoreHash() {
+ Map<String, String> hashy = new HashMap<>();
+ for (int i = 0; i < 100; i++) {
+ lettuce.hset("hash", "field-" + i, "value-" + i);
+ hashy.put("field-" + i, "value-" + i);
+ }
+ byte[] dump = lettuce.dump("hash");
+
+ lettuce.restore("restored", 0, dump);
+ String[] fields = Arrays.copyOf(hashy.keySet().toArray(), hashy.size(),
String[].class);
+ List<KeyValue<String, String>> restored = lettuce.hmget("restored",
fields);
+ Map<String, String> restoredMap = new HashMap<>();
+ restored.forEach(e -> restoredMap.put(e.getKey(), e.getValue()));
+
+ assertThat(restored.size()).isEqualTo(hashy.size());
+ assertThat(restoredMap).containsAllEntriesOf(hashy);
+ }
+
+ @Test
+ public void dumpAndRestoreSortedSet() {
+ Set<String> smembers = new HashSet<>();
+ for (int i = 0; i < 100; i++) {
+ lettuce.sadd("set", "member-" + i);
Review comment:
I think this should probably be `zadd`.
##########
File path:
geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/AbstractDumpRestoreIntegrationTest.java
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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.apache.geode.redis.internal.executor.key;
+
+import static
org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertExactNumberOfArgs;
+import static
org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import io.lettuce.core.KeyValue;
+import io.lettuce.core.RestoreArgs;
+import io.lettuce.core.cluster.RedisClusterClient;
+import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+
+import org.apache.geode.internal.InternalDataSerializer;
+import org.apache.geode.internal.serialization.DataSerializableFixedID;
+import org.apache.geode.redis.RedisIntegrationTest;
+import org.apache.geode.redis.internal.data.RedisHash;
+import org.apache.geode.redis.internal.data.RedisSet;
+import org.apache.geode.redis.internal.data.RedisSortedSet;
+import org.apache.geode.redis.internal.data.RedisString;
+
+public abstract class AbstractDumpRestoreIntegrationTest implements
RedisIntegrationTest {
+
+ private RedisAdvancedClusterCommands<String, String> lettuce;
+ private JedisCluster jedis;
+ private static String STRING_VALUE;
+ private static byte[] RESTORE_BYTES;
+
+ @BeforeClass
+ public static void setupClass() {
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_SET_ID,
+ RedisSet.class);
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_STRING_ID,
+ RedisString.class);
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_HASH_ID,
+ RedisHash.class);
+ InternalDataSerializer.getDSFIDSerializer().registerDSFID(
+ DataSerializableFixedID.REDIS_SORTED_SET_ID,
+ RedisSortedSet.class);
+ }
+
+ @Before
+ public void setup() {
+ jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()),
REDIS_CLIENT_TIMEOUT);
+
+ RedisClusterClient client =
+ RedisClusterClient.create(String.format("redis://%s:%d", BIND_ADDRESS,
getPort()));
+ lettuce = client.connect().sync();
+
+ STRING_VALUE = "It's a mad, mad, mad, mad, mad world";
+ lettuce.set("set-dump-value", STRING_VALUE);
+ RESTORE_BYTES = lettuce.dump("set-dump-value");
+ }
+
+ @After
+ public void tearDown() {
+ flushAll();
+ }
+
+ @Test
+ public void dumpTakesExactlyOneArgument() {
+ assertExactNumberOfArgs(jedis, Protocol.Command.DUMP, 1);
+ }
+
+ @Test
+ public void restoreErrorsWithUnknownOption() {
+ assertThatThrownBy(
+ () -> jedis.sendCommand("key", Protocol.Command.RESTORE, "key", "0",
"", "FOO"))
+ .hasMessageContaining("ERR syntax error");
+ }
+
+ @Test
+ public void restoreFails_whenKeyAlreadyExists() {
+ lettuce.set("restored", "already exists");
+
+ assertThatThrownBy(() -> lettuce.restore("restored", 0, RESTORE_BYTES))
+ .hasMessage("BUSYKEY Target key name already exists.");
+ }
+
+ @Test
+ public void restoreFails_whenTTLisNegative() {
+ assertThatThrownBy(() -> lettuce.restore("restored", -1, RESTORE_BYTES))
+ .hasMessage("ERR Invalid TTL value, must be >= 0");
+ }
+
+ @Test
+ public void restoreFails_withInvalidBytes() {
+ assertThatThrownBy(() -> lettuce.restore("restored", 0, new byte[] {0, 1,
2, 3}))
+ .hasMessage("ERR DUMP payload version or checksum are wrong");
+ }
+
+ @Test
+ public void dumpAndRestoreString() {
+ lettuce.set("dumped", STRING_VALUE);
+
+ byte[] rawBytes = lettuce.dump("dumped");
+ lettuce.restore("restored", 0, rawBytes);
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.ttl("restored")).isEqualTo(-1);
+ }
+
+ @Test
+ public void restore_withTTL_setsTTL() {
+ lettuce.restore("restored", 2000, RESTORE_BYTES);
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.pttl("restored"))
+ .isLessThan(2000)
+ .isGreaterThan(0);
+ }
+
+ @Test
+ public void restore_withReplace() {
+ lettuce.set("restored", "already exists");
+ lettuce.expire("restored", 10);
+
+ lettuce.restore("restored", RESTORE_BYTES, new RestoreArgs().replace());
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.pttl("restored")).isEqualTo(-1);
+ }
+
+ @Test
+ public void restore_withAbsTTL() {
+ long absttl = System.currentTimeMillis() + 10000;
+ lettuce.restore("restored", RESTORE_BYTES, new
RestoreArgs().ttl(absttl).absttl());
+
+ String response = lettuce.get("restored");
+
+ assertThat(response).isEqualTo(STRING_VALUE);
+ assertThat(lettuce.ttl("restored")).isGreaterThan(1);
+ }
+
+ @Test
+ public void restore_withAbsTTL_ofZero() {
Review comment:
Would there be any value in a test for restore with an AbsTTL that's in
the past? I.e. `long absttl = System.currentTimeMillis() - 10000;`
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisData.java
##########
@@ -89,6 +96,25 @@ public boolean rename(Region<RedisKey, RedisData> region,
RedisKey oldKey, Redis
return false;
}
+ @Override
+ public byte[] dump() throws IOException {
+ return new byte[0];
+ }
+
+ @Override
+ public RedisData restore(byte[] data, boolean replaceExisting) throws
Exception {
Review comment:
This method never throws an `Exception` (`RedisException` is a
`RuntimeException`) so that can be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
