You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by sa...@apache.org on 2020/11/02 14:23:00 UTC
[geode] branch develop updated: GEODE-8668: Implement Redis SELECT
command (#5682)
This is an automated email from the ASF dual-hosted git repository.
sabbey37 pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 7e095b8 GEODE-8668: Implement Redis SELECT command (#5682)
7e095b8 is described below
commit 7e095b8bd7e8a57e631415d32c3877e111353faf
Author: Sarah <41...@users.noreply.github.com>
AuthorDate: Mon Nov 2 09:21:14 2020 -0500
GEODE-8668: Implement Redis SELECT command (#5682)
---
.../tools_modules/redis_api_for_geode.html.md.erb | 2 +-
geode-redis/README.md | 39 +++++++-------
.../SelectNativeRedisAcceptanceTest.java | 33 ++++++++++++
.../connection/AbstractSelectIntegrationTest.java | 63 ++++++++++++++++++++++
.../executor/connection/SelectIntegrationTest.java | 44 +++++++++++++++
.../geode/redis/internal/RedisCommandType.java | 3 +-
.../geode/redis/internal/RedisConstants.java | 1 +
.../executor/connection/SelectExecutor.java | 38 +++++++++++++
.../redis/internal/SupportedCommandsJUnitTest.java | 2 +-
9 files changed, 202 insertions(+), 23 deletions(-)
diff --git a/geode-docs/tools_modules/redis_api_for_geode.html.md.erb b/geode-docs/tools_modules/redis_api_for_geode.html.md.erb
index 6daa22f..6bc90d3 100644
--- a/geode-docs/tools_modules/redis_api_for_geode.html.md.erb
+++ b/geode-docs/tools_modules/redis_api_for_geode.html.md.erb
@@ -78,7 +78,7 @@ The following Redis API for <%=vars.product_name%> commands are **unsupported**.
commands are available to use, but have not been fully tested. There is no guarantee they will work
exactly as expected.
-- **Connection**: ECHO
+- **Connection**: ECHO, SELECT
- **Hashes**: HDEL, HEXISTS, HGET, HINCRBY, HINCRBYFLOAT, HKEYS, HLEN, HMGET, HSCAN, HSETNX, HVALS
- **Keys**: SCAN, UNLINK
- **Server**: DBSIZE, FLUSHALL (no async option), FLUSHDB (no async option), SHUTDOWN, TIME
diff --git a/geode-redis/README.md b/geode-redis/README.md
index da17516..5133522 100644
--- a/geode-redis/README.md
+++ b/geode-redis/README.md
@@ -191,24 +191,24 @@ start server \
| | SCARD | CLUSTER ADDSLOTS |
| | SDIFF | CLUSTER BUMPEPOCH |
| | SDIFFSTORE | CLUSTER COUNT-FAILURE-REPORTS |
-| | SETBIT | CLUSTER COUNTKEYSINSLOT |
-| | SETEX | CLUSTER DELSLOTS |
-| | SETNX | CLUSTER FAILOVER |
-| | SETRANGE | CLUSTER FLUSHSLOTS |
-| | SHUTDOWN | CLUSTER FORGET |
-| | SINTER | CLUSTER GETKEYSINSLOT |
-| | SINTERSTORE | CLUSTER INFO |
-| | SISMEMBER | CLUSTER KEYSLOT |
-| | SMOVE | CLUSTER MEET |
-| | SPOP | CLUSTER MYID |
-| | SRANDMEMBER | CLUSTER NODES |
-| | SSCAN | CLUSTER REPLICAS |
-| | STRLEN | CLUSTER REPLICATE |
-| | SUNION | CLUSTER RESET |
-| | SUNIONSTORE | CLUSTER SAVECONFIG |
-| | TIME | CLUSTER SET-CONFIG-EPOCH |
-| | UNLINK [1] | CLUSTER SETSLOT |
-| | | CLUSTER SLAVES |
+| | SELECT | CLUSTER COUNTKEYSINSLOT |
+| | SETBIT | CLUSTER DELSLOTS |
+| | SETEX | CLUSTER FAILOVER |
+| | SETNX | CLUSTER FLUSHSLOTS |
+| | SETRANGE | CLUSTER FORGET |
+| | SHUTDOWN | CLUSTER GETKEYSINSLOT |
+| | SINTER | CLUSTER INFO |
+| | SINTERSTORE | CLUSTER KEYSLOT |
+| | SISMEMBER | CLUSTER MEET |
+| | SMOVE | CLUSTER MYID |
+| | SPOP | CLUSTER NODES |
+| | SRANDMEMBER | CLUSTER REPLICAS |
+| | SSCAN | CLUSTER REPLICATE |
+| | STRLEN | CLUSTER RESET |
+| | SUNION | CLUSTER SAVECONFIG |
+| | SUNIONSTORE | CLUSTER SET-CONFIG-EPOCH |
+| | TIME | CLUSTER SETSLOT |
+| | UNLINK [1] | CLUSTER SLAVES |
| | | CLUSTER SLOTS |
| | | COMMAND |
| | | COMMAND COUNT |
@@ -288,7 +288,6 @@ start server \
| | | SCRIPT FLUSH |
| | | SCRIPT KILL |
| | | SCRIPT LOAD |
-| | | SELECT |
| | | SLAVEOF |
| | | SLOWLOG |
| | | SORT |
@@ -334,7 +333,7 @@ start server \
| | | ZSCAN |
| | | ZSCORE |
| | | ZUNIONSTORE |
-| | | |
+| | | |
**NOTES:**
diff --git a/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/connection/SelectNativeRedisAcceptanceTest.java b/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/connection/SelectNativeRedisAcceptanceTest.java
new file mode 100644
index 0000000..ff2ca1e
--- /dev/null
+++ b/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/connection/SelectNativeRedisAcceptanceTest.java
@@ -0,0 +1,33 @@
+/*
+ * 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.connection;
+
+import org.junit.ClassRule;
+
+import org.apache.geode.NativeRedisTestRule;
+
+public class SelectNativeRedisAcceptanceTest extends AbstractSelectIntegrationTest {
+
+ @ClassRule
+ public static NativeRedisTestRule redis = new NativeRedisTestRule();
+
+ @Override
+ public int getPort() {
+ return redis.getPort();
+ }
+
+}
diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/AbstractSelectIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/AbstractSelectIntegrationTest.java
new file mode 100644
index 0000000..e73ac2a
--- /dev/null
+++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/AbstractSelectIntegrationTest.java
@@ -0,0 +1,63 @@
+/*
+ * 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.connection;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+import redis.clients.jedis.Protocol;
+
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.rules.RedisPortSupplier;
+
+public abstract class AbstractSelectIntegrationTest implements RedisPortSupplier {
+
+ private static final int REDIS_CLIENT_TIMEOUT =
+ Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+ protected static Jedis jedis;
+
+ @Before
+ public void setUp() {
+ jedis = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+ }
+
+ @After
+ public void tearDown() {
+ jedis.close();
+ }
+
+ @Test
+ public void givenLessThanTwoArguments_returnsWrongNumberOfArgumentsError() {
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.SELECT))
+ .hasMessage("ERR wrong number of arguments for 'select' command");
+ }
+
+ @Test
+ public void givenMoreThanTwoArguments_returnsWrongNumberOfArgumentsError() {
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.SELECT, "notALong", "extraArg"))
+ .hasMessage("ERR wrong number of arguments for 'select' command");
+ }
+
+ @Test
+ public void givenIndexOfZero_returnsOk() {
+ assertThat(jedis.select(0)).isEqualTo("OK");
+ }
+}
diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/SelectIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/SelectIntegrationTest.java
new file mode 100644
index 0000000..85a4a50
--- /dev/null
+++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/SelectIntegrationTest.java
@@ -0,0 +1,44 @@
+/*
+ * 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.connection;
+
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SELECT;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.ClassRule;
+import org.junit.Test;
+import redis.clients.jedis.Protocol;
+
+import org.apache.geode.redis.GeodeRedisServerRule;
+
+public class SelectIntegrationTest extends AbstractSelectIntegrationTest {
+
+ @ClassRule
+ public static GeodeRedisServerRule server = new GeodeRedisServerRule();
+
+ @Override
+ public int getPort() {
+ return server.getPort();
+ }
+
+ // our SELECT implementation diverges from Redis and only supports DB 0
+ @Test
+ public void givenAnyDBIndexOtherThanZero_returnsSelectError() {
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.SELECT, "9223372036854775807"))
+ .hasMessageContaining(ERROR_SELECT);
+ }
+}
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
index 5b8394b..8c0ebf1 100755
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
@@ -35,6 +35,7 @@ import org.apache.geode.redis.internal.executor.connection.AuthExecutor;
import org.apache.geode.redis.internal.executor.connection.EchoExecutor;
import org.apache.geode.redis.internal.executor.connection.PingExecutor;
import org.apache.geode.redis.internal.executor.connection.QuitExecutor;
+import org.apache.geode.redis.internal.executor.connection.SelectExecutor;
import org.apache.geode.redis.internal.executor.hash.HDelExecutor;
import org.apache.geode.redis.internal.executor.hash.HExistsExecutor;
import org.apache.geode.redis.internal.executor.hash.HGetAllExecutor;
@@ -182,6 +183,7 @@ public enum RedisCommandType {
***************************************/
ECHO(new EchoExecutor(), UNSUPPORTED, new ExactParameterRequirements(2)),
+ SELECT(new SelectExecutor(), UNSUPPORTED, new ExactParameterRequirements(2)),
/***************************************
*************** Keys ******************
@@ -329,7 +331,6 @@ public enum RedisCommandType {
RPUSHX(null, UNIMPLEMENTED),
SAVE(null, UNIMPLEMENTED),
SCRIPT(null, UNIMPLEMENTED),
- SELECT(null, UNIMPLEMENTED),
SLAVEOF(null, UNIMPLEMENTED),
REPLICAOF(null, UNIMPLEMENTED),
SLOWLOG(null, UNIMPLEMENTED),
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java
index b152fd9..acfeefe 100644
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java
@@ -30,6 +30,7 @@ public class RedisConstants {
public static final String SERVER_ERROR_MESSAGE =
"The server had an internal error please try again";
public static final String SERVER_ERROR_SHUTDOWN = "The server is shutting down";
+ public static final String ERROR_SELECT = "Only DB 0 supported";
public static final String ERROR_CURSOR = "invalid cursor";
public static final String ERROR_UNKNOWN_COMMAND =
"unknown command `%s`, with args beginning with: %s";
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/connection/SelectExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/connection/SelectExecutor.java
new file mode 100755
index 0000000..2c1060f
--- /dev/null
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/connection/SelectExecutor.java
@@ -0,0 +1,38 @@
+/*
+ * 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.connection;
+
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SELECT;
+
+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 SelectExecutor extends AbstractExecutor {
+
+ @Override
+ public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) {
+ String dbIndexString = command.getStringKey();
+
+ if (!dbIndexString.equals("0")) {
+ return RedisResponse.error(ERROR_SELECT);
+ }
+
+ return RedisResponse.ok();
+ }
+
+}
diff --git a/geode-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java b/geode-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
index 0a3610d..751bed9 100644
--- a/geode-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
+++ b/geode-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
@@ -99,6 +99,7 @@ public class SupportedCommandsJUnitTest {
"SCARD",
"SDIFF",
"SDIFFSTORE",
+ "SELECT",
"SETBIT",
"SETEX",
"SETNX",
@@ -181,7 +182,6 @@ public class SupportedCommandsJUnitTest {
"RPUSHX",
"SAVE",
"SCRIPT",
- "SELECT",
"SLAVEOF",
"REPLICAOF",
"SLOWLOG",