This is an automated email from the ASF dual-hosted git repository.
ringles pushed a commit to branch
GEODE-9892-Create-Infrastructure-for-Redis-Lists
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to
refs/heads/GEODE-9892-Create-Infrastructure-for-Redis-Lists by this push:
new 0ea15f8 Some sizing dev, native list integration tests running
0ea15f8 is described below
commit 0ea15f8be04c7a26ca2c965b1c0f731f7917c7a6
Author: Ray Ingles <[email protected]>
AuthorDate: Thu Jan 6 16:40:25 2022 -0500
Some sizing dev, native list integration tests running
---
.../list/ListsNativeRedisAcceptanceTest.java | 38 ++++++
.../list/AbstractListsIntegrationTest.java | 136 +++++++++++++++++++++
.../executor/list/ListsIntegrationTest.java | 31 +++++
.../geode/redis/internal/data/RedisList.java | 4 +-
.../internal/data/collections/SizeableList.java | 15 ++-
.../data/collections/SizeableListTest.java | 83 +++++++++++++
6 files changed, 300 insertions(+), 7 deletions(-)
diff --git
a/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/list/ListsNativeRedisAcceptanceTest.java
b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/list/ListsNativeRedisAcceptanceTest.java
new file mode 100755
index 0000000..37df9b2
--- /dev/null
+++
b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/list/ListsNativeRedisAcceptanceTest.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.commands.executor.list;
+
+
+import org.junit.ClassRule;
+
+import org.apache.geode.redis.NativeRedisClusterTestRule;
+import
org.apache.geode.redis.internal.commands.executor.set.AbstractSetsIntegrationTest;
+
+public class ListsNativeRedisAcceptanceTest extends
AbstractListsIntegrationTest {
+
+ @ClassRule
+ public static NativeRedisClusterTestRule redis = new
NativeRedisClusterTestRule();
+
+ @Override
+ public int getPort() {
+ return redis.getExposedPorts().get(0);
+ }
+
+ @Override
+ public void flushAll() {
+ redis.flushAll();
+ }
+
+}
diff --git
a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/AbstractListsIntegrationTest.java
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/AbstractListsIntegrationTest.java
new file mode 100755
index 0000000..04e1244
--- /dev/null
+++
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/AbstractListsIntegrationTest.java
@@ -0,0 +1,136 @@
+/*
+ * 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.commands.executor.list;
+
+import static
org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.exceptions.JedisDataException;
+
+import org.apache.geode.redis.RedisIntegrationTest;
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+
+public abstract class AbstractListsIntegrationTest implements
RedisIntegrationTest {
+
+ private JedisCluster jedis;
+ private static final int REDIS_CLIENT_TIMEOUT =
+ Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+
+ @Before
+ public void setUp() {
+ jedis = new JedisCluster(new HostAndPort("localhost", getPort()),
REDIS_CLIENT_TIMEOUT);
+ }
+
+ @After
+ public void tearDown() {
+ flushAll();
+ jedis.close();
+ }
+
+ @Test
+ public void lpushErrors_givenTooFewArguments() {
+ assertAtLeastNArgs(jedis, Protocol.Command.LPUSH, 2);
+ }
+
+ @Test
+ public void testLPush_withExistingKey_ofWrongType_shouldReturnError() {
+ String key = "key";
+ String stringValue = "preexistingValue";
+ String[] setValue = new String[1];
+ setValue[0] = "set value that should never get added";
+
+ jedis.set(key, stringValue);
+ assertThatThrownBy(() -> jedis.lpush(key, setValue))
+ .hasMessageContaining("Operation against a key holding the wrong kind
of value");
+ }
+
+ @Test
+ public void
testLPush_withExistingKey_ofWrongType_shouldNotOverWriteExistingKey() {
+ String key = "key";
+ String stringValue = "preexistingValue";
+ String[] setValue = new String[1];
+ setValue[0] = "set value that should never get added";
+
+ jedis.set(key, stringValue);
+
+ assertThatThrownBy(() -> jedis.lpush(key,
setValue)).isInstanceOf(JedisDataException.class);
+
+ String result = jedis.get(key);
+
+ assertThat(result).isEqualTo(stringValue);
+ }
+
+ @Test
+ public void testLPush_canStoreBinaryData() {
+ byte[] blob = new byte[256];
+ for (int i = 0; i < 256; i++) {
+ blob[i] = (byte) i;
+ }
+
+ jedis.lpush("key".getBytes(), blob, blob);
+
+ byte[] result = jedis.lpop("key".getBytes());
+ assertThat(result).containsExactly(blob);
+ result = jedis.lpop("key".getBytes());
+ assertThat(result).containsExactly(blob);
+ }
+
+ @Test
+ public void lpop_withStringFails() {
+ jedis.set("string", "value");
+ assertThatThrownBy(() ->
jedis.lpop("string")).hasMessageContaining("WRONGTYPE");
+ }
+
+ @Test
+ public void lpop_withNonExistentKey_returnsNull() {
+ assertThat(jedis.lpop("non existent")).isNull();
+ }
+
+ @Test
+ public void lpopCount_withNonExistentKey_returnsNull() {
+ assertThat(jedis.lpop("non existent", 3)).isNull();
+ }
+
+ @Test
+ public void lpop_returnsOneMember() {
+ jedis.lpush("key", "m1", "m2");
+ String result = jedis.lpop("key");
+ assertThat(result).isIn("m1", "m2");
+ }
+
+ @Test
+ public void lpopCount_returnsTwoUniqueMembers() {
+ jedis.lpush("key", "m1", "m2", "m3");
+ List<String> results = jedis.lpop("key", 2);
+ assertThat(results).hasSize(2);
+ assertThat(results).containsAnyOf("m1", "m2", "m3");
+ assertThat(results.get(0)).isNotEqualTo(results.get(1));
+ }
+
+ @Test
+ public void lpopNegativeCount_returnsError() {
+ jedis.lpush("key", "m1", "m2", "m3");
+ assertThatThrownBy(() -> jedis.lpop("key",
-3)).hasMessageContaining("value is out of range, must be positive");
+ }
+}
diff --git
a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/ListsIntegrationTest.java
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/ListsIntegrationTest.java
new file mode 100755
index 0000000..d5fd0de
--- /dev/null
+++
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/ListsIntegrationTest.java
@@ -0,0 +1,31 @@
+/*
+ * 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.commands.executor.list;
+
+import org.junit.ClassRule;
+
+import org.apache.geode.redis.GeodeRedisServerRule;
+
+public class ListsIntegrationTest extends AbstractListsIntegrationTest {
+
+ @ClassRule
+ public static GeodeRedisServerRule server = new GeodeRedisServerRule();
+
+ @Override
+ public int getPort() {
+ return server.getPort();
+ }
+
+}
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
index a4a2e89..ae05e1e 100644
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
+++
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
@@ -23,8 +23,8 @@ import static
org.apache.geode.redis.internal.data.RedisDataType.REDIS_LIST;
import java.io.DataInput;
import java.io.DataOutput;
import java.io.IOException;
-import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedList;
import java.util.List;
import java.util.Objects;
@@ -87,7 +87,7 @@ public class RedisList extends AbstractRedisData {
return this.elements;
}
- List<byte[]> popped = new ArrayList<>();
+ List<byte[]> popped = new LinkedList<>();
while (popped.size() < popCount) {
popped.add(elements.remove(0));
}
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/collections/SizeableList.java
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/collections/SizeableList.java
index 4f72243..7249f5e 100644
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/collections/SizeableList.java
+++
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/collections/SizeableList.java
@@ -19,11 +19,12 @@ import static
org.apache.geode.internal.JvmSizeUtils.memoryOverhead;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.LinkedList;
import java.util.List;
import org.apache.geode.internal.size.Sizeable;
-public abstract class SizeableList<K> extends ArrayList<K> implements Sizeable
{
+public abstract class SizeableList<K> extends LinkedList<K> implements
Sizeable {
private static final long serialVersionUID = 9174920505089089517L;
private static final int SIZEABLE_LIST_OVERHEAD =
memoryOverhead(SizeableList.class);
@@ -32,18 +33,22 @@ public abstract class SizeableList<K> extends ArrayList<K>
implements Sizeable {
private List<K> elements;
public SizeableList(Collection<K> collection) {
- elements = new ArrayList<>(collection);
+ elements = new ArrayList<>(collection.size());
+ for (K element: collection) {
+ add(element);
+ }
}
+
public SizeableList(int size) {
- elements = new ArrayList<>(size);
+ elements = new LinkedList<>();
}
@Override
public boolean add(K k) {
boolean added = elements.add(k);
if (added) {
- memberOverhead += sizeElement(k);
+ memberOverhead += sizeElement(k) + 4; // TODO: Figure out sizing.
}
return added;
}
@@ -66,4 +71,4 @@ public abstract class SizeableList<K> extends ArrayList<K>
implements Sizeable {
}
protected abstract int sizeElement(K element);
-}
+}
\ No newline at end of file
diff --git
a/geode-for-redis/src/test/java/org/apache/geode/redis/internal/data/collections/SizeableListTest.java
b/geode-for-redis/src/test/java/org/apache/geode/redis/internal/data/collections/SizeableListTest.java
new file mode 100644
index 0000000..d18ebbb
--- /dev/null
+++
b/geode-for-redis/src/test/java/org/apache/geode/redis/internal/data/collections/SizeableListTest.java
@@ -0,0 +1,83 @@
+/*
+ * 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.data.collections;
+
+import it.unimi.dsi.fastutil.bytes.ByteArrays;
+import org.apache.geode.cache.util.ObjectSizer;
+import org.apache.geode.internal.size.ReflectionObjectSizer;
+import org.apache.geode.redis.internal.data.RedisList;
+import org.apache.geode.redis.internal.data.RedisSet;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class SizeableListTest {
+ private final ObjectSizer sizer = ReflectionObjectSizer.getInstance();
+
+ private int expectedSize(SizeableList list) {
+ return sizer.sizeof(list);
+ }
+
+ private RedisList.ElementList createTestElementList(int size) {
+ List<byte[]> initialElements = new ArrayList<>(size);
+ for (int i = 0; i < size; ++i) {
+ initialElements.add(new byte[] {(byte) i});
+ }
+ RedisList.ElementList list = new RedisList.ElementList(0);
+ return list;
+ }
+
+ @Test
+ public void getSizeInBytesForEmptyElementList() {
+ RedisList.ElementList list = createTestElementList(0);
+ assertThat(list.getSizeInBytes()).isEqualTo(expectedSize(list));
+ }
+
+ @Test
+ public void getSizeInBytesForSingleElementList() {
+ RedisList.ElementList list = createTestElementList(1);
+ assertThat(list.getSizeInBytes()).isEqualTo(expectedSize(list));
+ }
+
+ @Test
+ public void getSizeInBytesIsAccurateForByteArrays() {
+ List<byte[]> initialElements = new ArrayList<>();
+ int initialNumberOfElements = 100;
+ int elementsToAdd = 100;
+ for (int i = 0; i < initialNumberOfElements; ++i) {
+ initialElements.add(new byte[] {(byte) i});
+ }
+ // Create a set with an initial size and confirm that it correctly reports
its size
+ RedisList.ElementList list = new RedisList.ElementList(initialElements);
+ assertThat(list.getSizeInBytes()).isEqualTo(expectedSize(list));
+
+ // Add enough members to force a resize and assert that the size is
correct after each add
+ for (int i = initialNumberOfElements; i < initialNumberOfElements +
elementsToAdd; ++i) {
+ list.add(new byte[] {(byte) i});
+ assertThat(list.getSizeInBytes()).isEqualTo(expectedSize(list));
+ }
+ assertThat(list.size()).isEqualTo(initialNumberOfElements + elementsToAdd);
+
+ // Remove all the members and assert that the size is correct after each
remove
+ for (int i = 0; i < initialNumberOfElements + elementsToAdd; ++i) {
+ list.remove(new byte[] {(byte) i});
+ assertThat(list.getSizeInBytes()).isEqualTo(expectedSize(list));
+ }
+ assertThat(list.size()).isEqualTo(0);
+ }
+}
