[geode] branch develop updated: GEODE-8276: fix KEYS command to handle non-ASCII keys (#5272)

[je...@apache.org]

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 9ea7a7c  GEODE-8276: fix KEYS command to handle non-ASCII keys (#5272)
9ea7a7c is described below

commit 9ea7a7cc58720041e31735643266f13e637bf002
Author: Darrel Schneider <ds...@pivotal.io>
AuthorDate: Thu Jun 18 10:44:27 2020 -0700

    GEODE-8276: fix KEYS command to handle non-ASCII keys (#5272)
---
 .../key/KeysNativeRedisAcceptanceTest.java         | 49 +++++++++++
 .../internal/executor/key/KeysIntegrationTest.java | 96 ++++++++++++++++++++++
 .../redis/internal/executor/key/KeysExecutor.java  |  4 +-
 3 files changed, 147 insertions(+), 2 deletions(-)

diff --git a/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/key/KeysNativeRedisAcceptanceTest.java b/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/key/KeysNativeRedisAcceptanceTest.java
new file mode 100644
index 0000000..960b76d
--- /dev/null
+++ b/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/key/KeysNativeRedisAcceptanceTest.java
@@ -0,0 +1,49 @@
+/*
+ * 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 org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestRule;
+import org.testcontainers.containers.GenericContainer;
+import redis.clients.jedis.Jedis;
+
+import org.apache.geode.test.junit.categories.RedisTest;
+import org.apache.geode.test.junit.rules.IgnoreOnWindowsRule;
+
+@Category({RedisTest.class})
+public class KeysNativeRedisAcceptanceTest extends KeysIntegrationTest {
+
+  // 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
+  public static TestRule ignoreOnWindowsRule = new IgnoreOnWindowsRule();
+
+  @BeforeClass
+  public static void setUp() {
+    GenericContainer redisContainer = new GenericContainer<>("redis:5.0.6").withExposedPorts(6379);
+    redisContainer.start();
+    jedis = new Jedis("localhost", redisContainer.getFirstMappedPort(), REDIS_CLIENT_TIMEOUT);
+  }
+
+  @AfterClass
+  public static void tearDown() {
+    jedis.close();
+  }
+
+}
diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/KeysIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/KeysIntegrationTest.java
new file mode 100644
index 0000000..8ff4273
--- /dev/null
+++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/key/KeysIntegrationTest.java
@@ -0,0 +1,96 @@
+/*
+ * 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.assertj.core.api.Assertions.assertThat;
+
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+
+import org.apache.geode.redis.GeodeRedisServerRule;
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+
+public class KeysIntegrationTest {
+
+  public static Jedis jedis;
+  public static int REDIS_CLIENT_TIMEOUT =
+      Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+
+  @ClassRule
+  public static GeodeRedisServerRule server = new GeodeRedisServerRule();
+
+  @BeforeClass
+  public static void setUp() {
+    jedis = new Jedis("localhost", server.getPort(), REDIS_CLIENT_TIMEOUT);
+  }
+
+  @After
+  public void flushAll() {
+    jedis.flushAll();
+  }
+
+  @AfterClass
+  public static void tearDown() {
+    jedis.close();
+  }
+
+  @Test
+  public void keys_givenSplat_withASCIIdata_returnsExpectedMatches() {
+    jedis.set("string1", "v1");
+    jedis.sadd("set1", "member1");
+    jedis.hset("hash1", "key1", "field1");
+    assertThat(jedis.keys("*")).containsExactlyInAnyOrder("string1", "set1", "hash1");
+    assertThat(jedis.keys("s*")).containsExactlyInAnyOrder("string1", "set1");
+    assertThat(jedis.keys("h*")).containsExactlyInAnyOrder("hash1");
+    assertThat(jedis.keys("foo*")).isEmpty();
+  }
+
+  @Test
+  public void keys_givenSplat_withBinaryData_returnsExpectedMatches() {
+    byte[] stringKey =
+        new byte[] {(byte) 0xac, (byte) 0xed, 0, 4, 0, 5, 's', 't', 'r', 'i', 'n', 'g', '1'};
+    byte[] value = new byte[] {'v', '1'};
+    jedis.set(stringKey, value);
+    byte[] setKey = new byte[] {(byte) 0xac, (byte) 0xed, 0, 4, 0, 5, 's', 'e', 't', '1'};
+    byte[] member = new byte[] {'m', '1'};
+    jedis.sadd(setKey, member);
+    byte[] hashKey = new byte[] {(byte) 0xac, (byte) 0xed, 0, 4, 0, 5, 'h', 'a', 's', 'h', '1'};
+    byte[] key = new byte[] {'k', 'e', 'y', '1'};
+    jedis.hset(hashKey, key, value);
+    assertThat(jedis.exists(stringKey));
+    assertThat(jedis.exists(setKey));
+    assertThat(jedis.exists(hashKey));
+    assertThat(jedis.keys(new byte[] {'*'})).containsExactlyInAnyOrder(stringKey, setKey, hashKey);
+    assertThat(jedis.keys(new byte[] {(byte) 0xac, (byte) 0xed, 0, 4, 0, 5, 's', '*'}))
+        .containsExactlyInAnyOrder(stringKey, setKey);
+    assertThat(jedis.keys(new byte[] {(byte) 0xac, (byte) 0xed, 0, 4, 0, 5, 'h', '*'}))
+        .containsExactlyInAnyOrder(hashKey);
+    assertThat(jedis.keys(new byte[] {'f', '*'})).isEmpty();
+  }
+
+  @Test
+  public void keys_givenBinaryValue_withExactMatch_preservesBinaryData() {
+    byte[] stringKey =
+        new byte[] {(byte) 0xac, (byte) 0xed, 0, 4, 0, 5, 's', 't', 'r', 'i', 'n', 'g', '1'};
+    jedis.set(stringKey, stringKey);
+    assertThat(jedis.keys(stringKey)).containsExactlyInAnyOrder(stringKey);
+  }
+
+}
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/key/KeysExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/key/KeysExecutor.java
index 6ba0a59..fd30a15 100755
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/key/KeysExecutor.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/key/KeysExecutor.java
@@ -44,7 +44,7 @@ public class KeysExecutor extends AbstractExecutor {
 
     String glob = Coder.bytesToString(commandElems.get(1));
     Set<ByteArrayWrapper> allKeys = getDataRegion(context).keySet();
-    List<String> matchingKeys = new ArrayList<String>();
+    List<ByteArrayWrapper> matchingKeys = new ArrayList<>();
 
     Pattern pattern;
     try {
@@ -56,7 +56,7 @@ public class KeysExecutor extends AbstractExecutor {
     for (ByteArrayWrapper bytesKey : allKeys) {
       String key = bytesKey.toString();
       if (pattern.matcher(key).matches()) {
-        matchingKeys.add(key);
+        matchingKeys.add(bytesKey);
       }
     }