You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by je...@apache.org on 2021/10/29 12:30:50 UTC
[geode] branch develop updated: GEODE-9646: Update native redis
tests to consistently use version 6.2.6 (#7019)
This is an automated email from the ASF dual-hosted git repository.
jensdeppe 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 22577d7 GEODE-9646: Update native redis tests to consistently use version 6.2.6 (#7019)
22577d7 is described below
commit 22577d73fba46ee4724c1d17e8b7d228c622907a
Author: Jens Deppe <jd...@vmware.com>
AuthorDate: Fri Oct 29 05:29:38 2021 -0700
GEODE-9646: Update native redis tests to consistently use version 6.2.6 (#7019)
---
.../connection/AuthNativeRedisAcceptanceTest.java | 10 +++---
.../MemoryOverheadNativeRedisAcceptanceTest.java | 2 +-
.../java/org/apache/geode/NativeRedisTestRule.java | 2 +-
.../geode/redis/internal/proxy/RedisProxy.java | 2 +-
.../internal/proxy/RedisProxyInboundHandler.java | 13 +++++++-
.../commonTest/resources/redis-cluster-compose.yml | 14 ++++----
.../executor/AbstractUnknownIntegrationTest.java | 37 ++++++++++++++++------
.../internal/executor/UnknownIntegrationTest.java | 14 ++++----
.../pubsub/AbstractSubCommandsIntegrationTest.java | 8 +++--
.../AbstractSubscriptionsIntegrationTest.java | 3 +-
.../server/AbstractInfoIntegrationTest.java | 11 +------
.../executor/server/CommandIntegrationTest.java | 2 +-
.../executor/server/InfoIntegrationTest.java | 12 +++++++
.../executor/set/AbstractSPopIntegrationTest.java | 4 +--
.../AbstractZInterStoreIntegrationTest.java | 4 +--
.../AbstractZUnionStoreIntegrationTest.java | 4 +--
.../geode/redis/internal/RedisConstants.java | 10 ++++--
.../internal/executor/pubsub/PubSubExecutor.java | 2 +-
.../redis/internal/executor/set/SPopExecutor.java | 4 +--
.../executor/sortedset/ZInterStoreExecutor.java | 7 ++++
.../executor/sortedset/ZStoreExecutor.java | 6 ++--
.../executor/sortedset/ZUnionStoreExecutor.java | 8 +++++
.../internal/netty/ExecutionHandlerContext.java | 4 ++-
.../pubsub/AbstractSubscriptionManager.java | 4 +++
.../apache/geode/redis/internal/pubsub/PubSub.java | 2 +-
.../geode/redis/internal/pubsub/PubSubImpl.java | 4 +--
.../geode/redis/internal/pubsub/Subscriptions.java | 7 ++++
27 files changed, 136 insertions(+), 64 deletions(-)
diff --git a/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/connection/AuthNativeRedisAcceptanceTest.java b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/connection/AuthNativeRedisAcceptanceTest.java
index 2be4be8..c17401c 100644
--- a/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/connection/AuthNativeRedisAcceptanceTest.java
+++ b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/connection/AuthNativeRedisAcceptanceTest.java
@@ -15,6 +15,7 @@
package org.apache.geode.redis.internal.executor.connection;
+import static org.apache.geode.NativeRedisTestRule.DEFAULT_REDIS_IMAGE;
import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
import org.junit.After;
@@ -27,8 +28,6 @@ import org.apache.geode.test.junit.rules.IgnoreOnWindowsRule;
public class AuthNativeRedisAcceptanceTest extends AbstractAuthIntegrationTest {
- private static final String REDIS_DOCKER_IMAGE = "redis:6.2.6";
-
// Docker compose does not work on windows in CI. Ignore this test on windows
// Using a RuleChain to make sure we ignore the test before the rule comes into play
@ClassRule
@@ -59,16 +58,15 @@ public class AuthNativeRedisAcceptanceTest extends AbstractAuthIntegrationTest {
@Override
public void setupCacheWithSecurity(boolean needsWritePermission) {
- redisContainer =
- new GenericContainer<>(REDIS_DOCKER_IMAGE).withExposedPorts(6379)
- .withCommand("redis-server --requirepass " + getPassword());
+ redisContainer = new GenericContainer<>(DEFAULT_REDIS_IMAGE).withExposedPorts(6379)
+ .withCommand("redis-server --requirepass " + getPassword());
redisContainer.start();
jedis = new Jedis("localhost", redisContainer.getFirstMappedPort(), REDIS_CLIENT_TIMEOUT);
}
@Override
public void setupCacheWithoutSecurity() {
- redisContainer = new GenericContainer<>(REDIS_DOCKER_IMAGE).withExposedPorts(6379);
+ redisContainer = new GenericContainer<>(DEFAULT_REDIS_IMAGE).withExposedPorts(6379);
redisContainer.start();
jedis = new Jedis("localhost", redisContainer.getFirstMappedPort(), REDIS_CLIENT_TIMEOUT);
}
diff --git a/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/hash/MemoryOverheadNativeRedisAcceptanceTest.java b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/hash/MemoryOverheadNativeRedisAcceptanceTest.java
index 698a52e..1ddbb1c 100755
--- a/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/hash/MemoryOverheadNativeRedisAcceptanceTest.java
+++ b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/hash/MemoryOverheadNativeRedisAcceptanceTest.java
@@ -39,7 +39,7 @@ public class MemoryOverheadNativeRedisAcceptanceTest extends AbstractMemoryOverh
result.put(Measurement.SET_ENTRY, 75);
result.put(Measurement.HASH, 228);
result.put(Measurement.HASH_ENTRY, 70);
- result.put(Measurement.SORTED_SET, 1601);
+ result.put(Measurement.SORTED_SET, 961);
result.put(Measurement.SORTED_SET_ENTRY, 114);
return result;
diff --git a/geode-for-redis/src/commonTest/java/org/apache/geode/NativeRedisTestRule.java b/geode-for-redis/src/commonTest/java/org/apache/geode/NativeRedisTestRule.java
index dcddeb8..8b17141 100644
--- a/geode-for-redis/src/commonTest/java/org/apache/geode/NativeRedisTestRule.java
+++ b/geode-for-redis/src/commonTest/java/org/apache/geode/NativeRedisTestRule.java
@@ -30,7 +30,7 @@ import org.apache.geode.test.junit.rules.IgnoreOnWindowsRule;
public class NativeRedisTestRule extends ExternalResource implements Serializable {
private static final Logger logger = LogService.getLogger();
- private static final String DEFAULT_REDIS_IMAGE = "redis:5.0.6";
+ public static final String DEFAULT_REDIS_IMAGE = "redis:6.2.6";
private GenericContainer<?> redisContainer;
private final RuleChain delegate;
diff --git a/geode-for-redis/src/commonTest/java/org/apache/geode/redis/internal/proxy/RedisProxy.java b/geode-for-redis/src/commonTest/java/org/apache/geode/redis/internal/proxy/RedisProxy.java
index 3678d25..8a6d5d4 100644
--- a/geode-for-redis/src/commonTest/java/org/apache/geode/redis/internal/proxy/RedisProxy.java
+++ b/geode-for-redis/src/commonTest/java/org/apache/geode/redis/internal/proxy/RedisProxy.java
@@ -62,7 +62,7 @@ public final class RedisProxy {
p.addLast(new RedisDecoder());
p.addLast(new RedisBulkStringAggregator());
p.addLast(new RedisArrayAggregator());
- p.addLast(new RedisProxyInboundHandler("127.0.0.1", targetPort, mappings));
+ p.addLast(new RedisProxyInboundHandler(ch, "127.0.0.1", targetPort, mappings));
}
})
.childOption(ChannelOption.AUTO_READ, false)
diff --git a/geode-for-redis/src/commonTest/java/org/apache/geode/redis/internal/proxy/RedisProxyInboundHandler.java b/geode-for-redis/src/commonTest/java/org/apache/geode/redis/internal/proxy/RedisProxyInboundHandler.java
index bf203f6..9168ae3 100644
--- a/geode-for-redis/src/commonTest/java/org/apache/geode/redis/internal/proxy/RedisProxyInboundHandler.java
+++ b/geode-for-redis/src/commonTest/java/org/apache/geode/redis/internal/proxy/RedisProxyInboundHandler.java
@@ -29,11 +29,13 @@ import io.netty.channel.ChannelInitializer;
import io.netty.channel.ChannelPipeline;
import io.netty.channel.socket.SocketChannel;
import io.netty.handler.codec.redis.ArrayRedisMessage;
+import io.netty.handler.codec.redis.ErrorRedisMessage;
import io.netty.handler.codec.redis.FullBulkStringRedisMessage;
import io.netty.handler.codec.redis.RedisArrayAggregator;
import io.netty.handler.codec.redis.RedisBulkStringAggregator;
import io.netty.handler.codec.redis.RedisDecoder;
import io.netty.handler.codec.redis.RedisEncoder;
+import io.netty.handler.codec.redis.RedisMessage;
import io.netty.util.CharsetUtil;
import org.apache.logging.log4j.Logger;
@@ -46,13 +48,15 @@ public class RedisProxyInboundHandler extends ChannelInboundHandlerAdapter {
private final int remotePort;
private final Map<HostPort, HostPort> mappings;
private Channel outboundChannel;
+ private Channel inboundChannel;
private RedisProxyOutboundHandler outboundHandler;
private final ClusterSlotsResponseProcessor slotsResponseProcessor;
private final ClusterNodesResponseProcessor nodesResponseProcessor;
private MovedResponseHandler movedResponseHandler;
- public RedisProxyInboundHandler(String remoteHost, int remotePort,
+ public RedisProxyInboundHandler(Channel inboundChannel, String remoteHost, int remotePort,
Map<HostPort, HostPort> mappings) {
+ this.inboundChannel = inboundChannel;
this.remoteHost = remoteHost;
this.remotePort = remotePort;
this.mappings = mappings;
@@ -129,6 +133,13 @@ public class RedisProxyInboundHandler extends ChannelInboundHandlerAdapter {
outboundHandler.addResponseProcessor(nodesResponseProcessor);
}
break;
+ case "hello":
+ // Lettuce tries to use RESP 3 if possible but Netty's Redis codecs can't handle that yet.
+ // We implicitly fake the client into thinking this is an older Redis so that it uses
+ // RESP 2.
+ RedisMessage error = new ErrorRedisMessage("ERR unknown command");
+ inboundChannel.writeAndFlush(error);
+ return;
default:
outboundHandler.addResponseProcessor(NoopRedisResponseProcessor.INSTANCE);
}
diff --git a/geode-for-redis/src/commonTest/resources/redis-cluster-compose.yml b/geode-for-redis/src/commonTest/resources/redis-cluster-compose.yml
index f8257df..56faa93 100644
--- a/geode-for-redis/src/commonTest/resources/redis-cluster-compose.yml
+++ b/geode-for-redis/src/commonTest/resources/redis-cluster-compose.yml
@@ -14,43 +14,43 @@
version: '2'
services:
redis-node-0:
- image: docker.io/bitnami/redis-cluster:5.0
+ image: docker.io/bitnami/redis-cluster:6.2.6
environment:
- 'ALLOW_EMPTY_PASSWORD=yes'
- 'REDIS_NODES=redis-node-0 redis-node-1 redis-node-2 redis-node-3 redis-node-4 redis-node-5'
redis-node-1:
- image: docker.io/bitnami/redis-cluster:5.0
+ image: docker.io/bitnami/redis-cluster:6.2.6
environment:
- 'ALLOW_EMPTY_PASSWORD=yes'
- 'REDIS_NODES=redis-node-0 redis-node-1 redis-node-2 redis-node-3 redis-node-4 redis-node-5'
redis-node-2:
- image: docker.io/bitnami/redis-cluster:5.0
+ image: docker.io/bitnami/redis-cluster:6.2.6
environment:
- 'ALLOW_EMPTY_PASSWORD=yes'
- 'REDIS_NODES=redis-node-0 redis-node-1 redis-node-2 redis-node-3 redis-node-4 redis-node-5'
redis-node-3:
- image: docker.io/bitnami/redis-cluster:5.0
+ image: docker.io/bitnami/redis-cluster:6.2.6
environment:
- 'ALLOW_EMPTY_PASSWORD=yes'
- 'REDIS_NODES=redis-node-0 redis-node-1 redis-node-2 redis-node-3 redis-node-4 redis-node-5'
redis-node-4:
- image: docker.io/bitnami/redis-cluster:5.0
+ image: docker.io/bitnami/redis-cluster:6.2.6
environment:
- 'ALLOW_EMPTY_PASSWORD=yes'
- 'REDIS_NODES=redis-node-0 redis-node-1 redis-node-2 redis-node-3 redis-node-4 redis-node-5'
redis-node-5:
- image: docker.io/bitnami/redis-cluster:5.0
+ image: docker.io/bitnami/redis-cluster:6.2.6
environment:
- 'ALLOW_EMPTY_PASSWORD=yes'
- 'REDIS_NODES=redis-node-0 redis-node-1 redis-node-2 redis-node-3 redis-node-4 redis-node-5'
redis-cluster-init:
- image: docker.io/bitnami/redis-cluster:5.0
+ image: docker.io/bitnami/redis-cluster:6.2.6
depends_on:
- redis-node-0
- redis-node-1
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/AbstractUnknownIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/AbstractUnknownIntegrationTest.java
index 157f8a1..0fffe68 100644
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/AbstractUnknownIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/AbstractUnknownIntegrationTest.java
@@ -15,6 +15,8 @@
package org.apache.geode.redis.internal.executor;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import org.junit.After;
@@ -23,17 +25,14 @@ import org.junit.Test;
import redis.clients.jedis.Jedis;
import org.apache.geode.redis.RedisIntegrationTest;
-import org.apache.geode.test.awaitility.GeodeAwaitility;
public abstract class AbstractUnknownIntegrationTest implements RedisIntegrationTest {
- private Jedis jedis;
- private static final int REDIS_CLIENT_TIMEOUT =
- Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+ protected Jedis jedis;
@Before
public void setUp() {
- jedis = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+ jedis = new Jedis(BIND_ADDRESS, getPort(), REDIS_CLIENT_TIMEOUT);
}
@After
@@ -64,9 +63,29 @@ public abstract class AbstractUnknownIntegrationTest implements RedisIntegration
"ERR unknown command `fhqwhgads`, with args beginning with: `EVERYBODY`, ``, ");
}
- @Test // HELLO is not a recognized command until Redis 6.0.0
- public void givenHelloCommand_returnsUnknownCommandErrorWithArgumentsListed() {
- assertThatThrownBy(() -> jedis.sendCommand(() -> "HELLO".getBytes()))
- .hasMessage("ERR unknown command `HELLO`, with args beginning with: ");
+ @Test
+ public void givenInternalSMembersCommand_returnsUnknownCommandErrorWithArgumentsListed() {
+ assertThatThrownBy(
+ () -> jedis.sendCommand(() -> "INTERNALSMEMBERS".getBytes(), "something",
+ "somethingElse"))
+ .hasMessage(
+ "ERR unknown command `INTERNALSMEMBERS`, with args beginning with: `something`, `somethingElse`, ");
+ }
+
+ @Test
+ public void givenInternalPTTLCommand_returnsUnknownCommandErrorWithArgumentsListed() {
+ assertThatThrownBy(
+ () -> jedis.sendCommand(() -> "INTERNALPTTL".getBytes(), "something"))
+ .hasMessage(
+ "ERR unknown command `INTERNALPTTL`, with args beginning with: `something`, ");
}
+
+ @Test
+ public void givenInternalTypeCommand_returnsUnknownCommandErrorWithArgumentsListed() {
+ assertThatThrownBy(
+ () -> jedis.sendCommand(() -> "INTERNALTYPE".getBytes(), "something"))
+ .hasMessage(
+ "ERR unknown command `INTERNALTYPE`, with args beginning with: `something`, ");
+ }
+
}
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/UnknownIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/UnknownIntegrationTest.java
index 0afabf7..aaec32d 100644
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/UnknownIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/UnknownIntegrationTest.java
@@ -15,18 +15,15 @@
package org.apache.geode.redis.internal.executor;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
import org.junit.ClassRule;
-import redis.clients.jedis.Jedis;
+import org.junit.Test;
import org.apache.geode.redis.GeodeRedisServerRule;
-import org.apache.geode.test.awaitility.GeodeAwaitility;
public class UnknownIntegrationTest extends AbstractUnknownIntegrationTest {
- public static Jedis jedis;
- public static final int REDIS_CLIENT_TIMEOUT =
- Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
-
@ClassRule
public static GeodeRedisServerRule server = new GeodeRedisServerRule();
@@ -35,4 +32,9 @@ public class UnknownIntegrationTest extends AbstractUnknownIntegrationTest {
return server.getPort();
}
+ @Test // HELLO is not a recognized command until Redis 6.0.0
+ public void givenHelloCommand_returnsUnknownCommandErrorWithArgumentsListed() {
+ assertThatThrownBy(() -> jedis.sendCommand(() -> "HELLO".getBytes()))
+ .hasMessage("ERR unknown command `HELLO`, with args beginning with: ");
+ }
}
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/pubsub/AbstractSubCommandsIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/pubsub/AbstractSubCommandsIntegrationTest.java
index 7d738ee..0757bbe 100644
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/pubsub/AbstractSubCommandsIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/pubsub/AbstractSubCommandsIntegrationTest.java
@@ -299,12 +299,15 @@ public abstract class AbstractSubCommandsIntegrationTest implements RedisIntegra
}
@Test
- public void numpat_shouldReturnCountOfAllPatternSubscriptions_includingDuplicates() {
+ public void numpat_shouldReturnCountOfAllPatternSubscriptions_ignoringDuplicates() {
Jedis subscriber2 = new Jedis(BIND_ADDRESS, getPort(), REDIS_CLIENT_TIMEOUT);
MockSubscriber mockSubscriber2 = new MockSubscriber();
executor.submit(() -> subscriber.psubscribe(mockSubscriber, "f*"));
waitFor(() -> mockSubscriber.getSubscribedChannels() == 1);
+ mockSubscriber.psubscribe("g*");
+ mockSubscriber.subscribe("my-channel");
+ waitFor(() -> mockSubscriber.getSubscribedChannels() == 3);
executor.submit(() -> subscriber2.psubscribe(mockSubscriber2, "f*"));
waitFor(() -> mockSubscriber2.getSubscribedChannels() == 1);
@@ -312,6 +315,7 @@ public abstract class AbstractSubCommandsIntegrationTest implements RedisIntegra
assertThat(result).isEqualTo(2);
+ unsubscribeWithSuccess(mockSubscriber);
punsubscribeWithSuccess(mockSubscriber);
punsubscribeWithSuccess(mockSubscriber2);
@@ -331,7 +335,7 @@ public abstract class AbstractSubCommandsIntegrationTest implements RedisIntegra
Long result = introspector.pubsubNumPat();
- assertThat(result).isEqualTo(3);
+ assertThat(result).isEqualTo(2);
punsubscribeWithSuccess(mockSubscriber);
punsubscribeWithSuccess(patternSubscriber);
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/pubsub/AbstractSubscriptionsIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/pubsub/AbstractSubscriptionsIntegrationTest.java
index ad1423d..9a10523 100644
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/pubsub/AbstractSubscriptionsIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/pubsub/AbstractSubscriptionsIntegrationTest.java
@@ -16,6 +16,7 @@
package org.apache.geode.redis.internal.executor.pubsub;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_PUBSUB_WRONG_COMMAND;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -88,7 +89,7 @@ public abstract class AbstractSubscriptionsIntegrationTest implements RedisInteg
client.sendCommand(Protocol.Command.SUBSCRIBE, "hello");
assertThatThrownBy(() -> client.set("not", "supported")).hasMessageContaining(
- "ERR only (P)SUBSCRIBE / (P)UNSUBSCRIBE / PING / QUIT allowed in this context");
+ String.format(ERROR_PUBSUB_WRONG_COMMAND, "set"));
client.sendCommand(Protocol.Command.UNSUBSCRIBE);
}
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractInfoIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractInfoIntegrationTest.java
index 17f81e1..92ad24c 100644
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractInfoIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractInfoIntegrationTest.java
@@ -40,7 +40,7 @@ import org.apache.geode.test.awaitility.GeodeAwaitility;
public abstract class AbstractInfoIntegrationTest implements RedisIntegrationTest {
private static final String KEYSPACE_START = "db0";
- private Jedis jedis;
+ protected Jedis jedis;
private static final int REDIS_CLIENT_TIMEOUT =
Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
@@ -122,15 +122,6 @@ public abstract class AbstractInfoIntegrationTest implements RedisIntegrationTes
}
@Test
- public void shouldReturnRedisVersion() {
- String expectedResult = "redis_version:5.0.6";
-
- String actualResult = jedis.info();
-
- assertThat(actualResult).contains(expectedResult);
- }
-
- @Test
public void shouldReturnRedisMode() {
String expectedResult = "redis_mode:standalone";
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/CommandIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/CommandIntegrationTest.java
index ca5ef73..d254029 100644
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/CommandIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/CommandIntegrationTest.java
@@ -40,7 +40,7 @@ import org.apache.geode.redis.internal.RedisCommandType;
public class CommandIntegrationTest {
@ClassRule
- public static NativeRedisTestRule redisServer = new NativeRedisTestRule("redis:6.2.4");
+ public static NativeRedisTestRule redisServer = new NativeRedisTestRule();
@ClassRule
public static GeodeRedisServerRule radishServer = new GeodeRedisServerRule();
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/InfoIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/InfoIntegrationTest.java
index cdcf76c..6cdf2e8 100644
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/InfoIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/InfoIntegrationTest.java
@@ -16,8 +16,10 @@
package org.apache.geode.redis.internal.executor.server;
import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
+import static org.assertj.core.api.Assertions.assertThat;
import org.junit.ClassRule;
+import org.junit.Test;
import org.apache.geode.redis.GeodeRedisServerRule;
@@ -31,4 +33,14 @@ public class InfoIntegrationTest extends AbstractInfoIntegrationTest {
public int getPort() {
return server.getPort();
}
+
+ @Test
+ public void shouldReturnRedisVersion() {
+ String expectedResult = "redis_version:5.0.6";
+
+ String actualResult = jedis.info();
+
+ assertThat(actualResult).contains(expectedResult);
+ }
+
}
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/AbstractSPopIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/AbstractSPopIntegrationTest.java
index 91be942..5c7df57 100755
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/AbstractSPopIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/AbstractSPopIntegrationTest.java
@@ -14,8 +14,8 @@
*/
package org.apache.geode.redis.internal.executor.set;
-import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_VALUE_MUST_BE_POSITIVE;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -67,7 +67,7 @@ public abstract class AbstractSPopIntegrationTest implements RedisIntegrationTes
@Test
public void givenCountIsNotAnInteger_returnsNotIntegerError() {
assertThatThrownBy(() -> jedis.sendCommand("key", Protocol.Command.SPOP, "key", "NaN"))
- .hasMessageContaining(ERROR_NOT_INTEGER);
+ .hasMessageContaining(ERROR_VALUE_MUST_BE_POSITIVE);
}
@Test
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZInterStoreIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZInterStoreIntegrationTest.java
index 3c89e57..f6a51ef 100755
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZInterStoreIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZInterStoreIntegrationTest.java
@@ -104,14 +104,14 @@ public abstract class AbstractZInterStoreIntegrationTest implements RedisIntegra
public void shouldError_givenNumKeysOfZero() {
assertThatThrownBy(
() -> jedis.sendCommand(NEW_SET, Protocol.Command.ZINTERSTORE, NEW_SET, "0", KEY1, KEY2))
- .hasMessage("ERR " + RedisConstants.ERROR_KEY_REQUIRED);
+ .hasMessage("ERR " + RedisConstants.ERROR_KEY_REQUIRED_ZINTERSTORE);
}
@Test
public void shouldError_givenNegativeNumKeys() {
assertThatThrownBy(
() -> jedis.sendCommand(NEW_SET, Protocol.Command.ZINTERSTORE, NEW_SET, "-2", KEY1, KEY2))
- .hasMessage("ERR " + RedisConstants.ERROR_KEY_REQUIRED);
+ .hasMessage("ERR " + RedisConstants.ERROR_KEY_REQUIRED_ZINTERSTORE);
}
@Test
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZUnionStoreIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZUnionStoreIntegrationTest.java
index c1c629b..786f445 100755
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZUnionStoreIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZUnionStoreIntegrationTest.java
@@ -102,14 +102,14 @@ public abstract class AbstractZUnionStoreIntegrationTest implements RedisIntegra
public void shouldError_givenNumKeysOfZero() {
assertThatThrownBy(
() -> jedis.sendCommand(NEW_SET, Protocol.Command.ZUNIONSTORE, NEW_SET, "0", KEY1, KEY2))
- .hasMessage("ERR " + RedisConstants.ERROR_KEY_REQUIRED);
+ .hasMessage("ERR " + RedisConstants.ERROR_KEY_REQUIRED_ZUNIONSTORE);
}
@Test
public void shouldError_givenNegativeNumKeys() {
assertThatThrownBy(
() -> jedis.sendCommand(NEW_SET, Protocol.Command.ZUNIONSTORE, NEW_SET, "-2", KEY1, KEY2))
- .hasMessage("ERR " + RedisConstants.ERROR_KEY_REQUIRED);
+ .hasMessage("ERR " + RedisConstants.ERROR_KEY_REQUIRED_ZUNIONSTORE);
}
@Test
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java
index 827b603..86d1866 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java
@@ -34,6 +34,8 @@ public class RedisConstants {
public static final String ERROR_WRONG_TYPE =
"Operation against a key holding the wrong kind of value";
public static final String ERROR_NOT_INTEGER = "value is not an integer or out of range";
+ public static final String ERROR_VALUE_MUST_BE_POSITIVE =
+ "value is out of range, must be positive";
public static final String ERROR_OVERFLOW = "increment or decrement would overflow";
public static final String ERROR_NAN_OR_INFINITY = "increment would produce NaN or Infinity";
public static final String ERROR_OPERATION_PRODUCED_NAN = "resulting score is not a number (NaN)";
@@ -69,11 +71,15 @@ public class RedisConstants {
"invalid username-password pair or user is disabled.";
public static final String ERROR_AUTH_CALLED_WITHOUT_SECURITY_CONFIGURED =
"AUTH called without a Security Manager configured.";
- public static final String ERROR_KEY_REQUIRED =
- "at least 1 input key is needed for ZUNIONSTORE/ZINTERSTORE";
+ public static final String ERROR_KEY_REQUIRED_ZINTERSTORE =
+ "at least 1 input key is needed for zinterstore";
+ public static final String ERROR_KEY_REQUIRED_ZUNIONSTORE =
+ "at least 1 input key is needed for zunionstore";
public static final String ERROR_UNAUTHENTICATED_MULTIBULK =
"Protocol error: unauthenticated multibulk length";
public static final String ERROR_UNAUTHENTICATED_BULK =
"Protocol error: unauthenticated bulk length";
public static final String INTERNAL_SERVER_ERROR = "Internal server error: ";
+ public static final String ERROR_PUBSUB_WRONG_COMMAND =
+ "Can't execute '%s': only (P)SUBSCRIBE / (P)UNSUBSCRIBE / PING / QUIT / RESET are allowed in this context";
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/pubsub/PubSubExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/pubsub/PubSubExecutor.java
index 19b53f8..d75827b 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/pubsub/PubSubExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/pubsub/PubSubExecutor.java
@@ -54,7 +54,7 @@ public class PubSubExecutor implements CommandExecutor {
return RedisResponse
.error(String.format(ERROR_UNKNOWN_PUBSUB_SUBCOMMAND, new String(subCommand)));
}
- long numPatResponse = context.getPubSub().findNumberOfSubscribedPatterns();
+ long numPatResponse = context.getPubSub().findNumberOfUniqueSubscribedPatterns();
return RedisResponse.integer(numPatResponse);
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/set/SPopExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/set/SPopExecutor.java
index e5d9fdf..7596625 100755
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/set/SPopExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/set/SPopExecutor.java
@@ -15,7 +15,7 @@
package org.apache.geode.redis.internal.executor.set;
-import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_VALUE_MUST_BE_POSITIVE;
import static org.apache.geode.redis.internal.netty.Coder.bytesToLong;
import static org.apache.geode.redis.internal.netty.Coder.narrowLongToInt;
@@ -43,7 +43,7 @@ public class SPopExecutor implements CommandExecutor {
try {
popCount = narrowLongToInt(bytesToLong(commandElems.get(2)));
} catch (NumberFormatException nex) {
- return RedisResponse.error(ERROR_NOT_INTEGER);
+ return RedisResponse.error(ERROR_VALUE_MUST_BE_POSITIVE);
}
} else {
popCount = 1;
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZInterStoreExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZInterStoreExecutor.java
index 5734ce8..d6bb79a 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZInterStoreExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZInterStoreExecutor.java
@@ -15,11 +15,14 @@
package org.apache.geode.redis.internal.executor.sortedset;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_KEY_REQUIRED_ZINTERSTORE;
+
import java.util.List;
import org.apache.geode.redis.internal.RegionProvider;
import org.apache.geode.redis.internal.data.RedisKey;
import org.apache.geode.redis.internal.data.RedisSortedSet;
+import org.apache.geode.redis.internal.executor.RedisResponse;
import org.apache.geode.redis.internal.netty.Command;
import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
@@ -36,4 +39,8 @@ public class ZInterStoreExecutor extends ZStoreExecutor {
.zinterstore(regionProvider, key, keyWeights, aggregator);
}
+ @Override
+ protected RedisResponse getKeyRequiredError() {
+ return RedisResponse.error(ERROR_KEY_REQUIRED_ZINTERSTORE);
+ }
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZStoreExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZStoreExecutor.java
index 6277c10..5a1da11 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZStoreExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZStoreExecutor.java
@@ -14,7 +14,6 @@
*/
package org.apache.geode.redis.internal.executor.sortedset;
-import static org.apache.geode.redis.internal.RedisConstants.ERROR_KEY_REQUIRED;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_WEIGHT_NOT_A_FLOAT;
@@ -56,7 +55,7 @@ public abstract class ZStoreExecutor implements CommandExecutor {
if (numKeys > commandElements.size() - 2) {
return syntaxErrorResponse;
} else if (numKeys <= 0) {
- return RedisResponse.error(ERROR_KEY_REQUIRED);
+ return getKeyRequiredError();
}
} catch (NumberFormatException ex) {
return RedisResponse.error(ERROR_NOT_INTEGER);
@@ -129,7 +128,8 @@ public abstract class ZStoreExecutor implements CommandExecutor {
return keysToLock;
}
+ protected abstract RedisResponse getKeyRequiredError();
- public abstract long getResult(ExecutionHandlerContext context, Command command,
+ protected abstract long getResult(ExecutionHandlerContext context, Command command,
List<ZKeyWeight> keyWeights, ZAggregator aggregator);
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZUnionStoreExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZUnionStoreExecutor.java
index 2c0f30a..9ad52e0 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZUnionStoreExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZUnionStoreExecutor.java
@@ -14,11 +14,14 @@
*/
package org.apache.geode.redis.internal.executor.sortedset;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_KEY_REQUIRED_ZUNIONSTORE;
+
import java.util.List;
import org.apache.geode.redis.internal.RegionProvider;
import org.apache.geode.redis.internal.data.RedisKey;
import org.apache.geode.redis.internal.data.RedisSortedSet;
+import org.apache.geode.redis.internal.executor.RedisResponse;
import org.apache.geode.redis.internal.netty.Command;
import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
@@ -35,4 +38,9 @@ public class ZUnionStoreExecutor extends ZStoreExecutor {
.zunionstore(regionProvider, key, keyWeights, aggregator);
}
+ @Override
+ protected RedisResponse getKeyRequiredError() {
+ return RedisResponse.error(ERROR_KEY_REQUIRED_ZUNIONSTORE);
+ }
+
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java
index 03cb132..f49236a 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java
@@ -16,6 +16,7 @@
package org.apache.geode.redis.internal.netty;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_AUTHORIZED;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_PUBSUB_WRONG_COMMAND;
import static org.apache.geode.redis.internal.RedisProperties.REDIS_REGION_NAME_PROPERTY;
import static org.apache.geode.redis.internal.RedisProperties.getStringSystemProperty;
import static org.apache.geode.redis.internal.RegionProvider.DEFAULT_REDIS_REGION_NAME;
@@ -257,7 +258,8 @@ public class ExecutionHandlerContext extends ChannelInboundHandlerAdapter {
if (getClient().hasSubscriptions()) {
if (!command.getCommandType().isAllowedWhileSubscribed()) {
writeToChannel(RedisResponse
- .error("only (P)SUBSCRIBE / (P)UNSUBSCRIBE / PING / QUIT allowed in this context"));
+ .error(String.format(ERROR_PUBSUB_WRONG_COMMAND,
+ command.getCommandType().name().toLowerCase())));
}
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/AbstractSubscriptionManager.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/AbstractSubscriptionManager.java
index e75e9fb..3ac923f 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/AbstractSubscriptionManager.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/AbstractSubscriptionManager.java
@@ -145,6 +145,10 @@ abstract class AbstractSubscriptionManager implements SubscriptionManager {
}
}
+ public int size() {
+ return clientManagers.size();
+ }
+
@Immutable
private static final ClientSubscriptionManager EMPTY_CLIENT_MANAGER =
new ClientSubscriptionManager() {
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/PubSub.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/PubSub.java
index 81ec0e9..b15bb27 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/PubSub.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/PubSub.java
@@ -104,7 +104,7 @@ public interface PubSub {
/**
* Return a count of all pattern subscriptions including duplicates.
*/
- long findNumberOfSubscribedPatterns();
+ long findNumberOfUniqueSubscribedPatterns();
/**
* Should be called when the given client disconnects from the server.
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/PubSubImpl.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/PubSubImpl.java
index 6dfbc39..3b2c6e4 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/PubSubImpl.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/PubSubImpl.java
@@ -96,8 +96,8 @@ public class PubSubImpl implements PubSub {
}
@Override
- public long findNumberOfSubscribedPatterns() {
- return subscriptions.getPatternSubscriptionCount();
+ public long findNumberOfUniqueSubscribedPatterns() {
+ return subscriptions.getUniquePatternSubscriptionCount();
}
@Override
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/Subscriptions.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/Subscriptions.java
index c21ae24..693ed07 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/Subscriptions.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/pubsub/Subscriptions.java
@@ -162,6 +162,13 @@ public class Subscriptions {
return patternSubscriptions.getSubscriptionCount();
}
+ /**
+ * Return a count of all pattern subscriptions ignoring duplicates.
+ */
+ public int getUniquePatternSubscriptionCount() {
+ return patternSubscriptions.size();
+ }
+
@VisibleForTesting
int getChannelSubscriptionCount() {
return channelSubscriptions.getSubscriptionCount();