This is an automated email from the ASF dual-hosted git repository. jensdeppe pushed a commit to branch support/1.15 in repository https://gitbox.apache.org/repos/asf/geode.git
commit 0ade491c33f55176d7ea7fa9d07fa028e9857c90 Author: Kris10 <kod...@vmware.com> AuthorDate: Thu Jan 20 14:11:41 2022 -0800 GEODE-9837: SUNIONSTORE Command Support (#7284) (cherry picked from commit 3a36962edfcd30aa3afa3a50813c63bfc155f699) --- .../tools_modules/geode_for_redis.html.md.erb | 1 + geode-for-redis/README.md | 1 + .../set/SUnionStoreNativeRedisAcceptanceTest.java | 36 +++ .../server/AbstractHitsMissesIntegrationTest.java | 12 +- .../set/AbstractSUnionIntegrationTest.java | 141 ------------ .../set/AbstractSUnionStoreIntegrationTest.java | 242 +++++++++++++++++++++ .../executor/set/SUnionStoreIntegrationTest.java | 31 +++ .../redis/internal/commands/RedisCommandType.java | 4 +- .../commands/executor/set/SetOpExecutor.java | 16 +- .../apache/geode/redis/internal/data/RedisSet.java | 18 +- 10 files changed, 344 insertions(+), 158 deletions(-) diff --git a/geode-docs/tools_modules/geode_for_redis.html.md.erb b/geode-docs/tools_modules/geode_for_redis.html.md.erb index 9451dc7..748ddf8 100644 --- a/geode-docs/tools_modules/geode_for_redis.html.md.erb +++ b/geode-docs/tools_modules/geode_for_redis.html.md.erb @@ -125,6 +125,7 @@ If the server is functioning properly, you should see a response of `PONG`. - STRLEN <br/> - SUBSCRIBE <br/> - SUNION <br/> + - SUNIONSTORE <br/> - TTL <br/> - TYPE <br/> - UNSUBSCRIBE <br/> diff --git a/geode-for-redis/README.md b/geode-for-redis/README.md index 2162229..fa392d9 100644 --- a/geode-for-redis/README.md +++ b/geode-for-redis/README.md @@ -215,6 +215,7 @@ Geode for Redis implements a subset of the full Redis command set. - STRLEN - SUBSCRIBE - SUNION +- SUNIONSTORE - TTL - TYPE - UNSUBSCRIBE diff --git a/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreNativeRedisAcceptanceTest.java b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreNativeRedisAcceptanceTest.java new file mode 100644 index 0000000..8f63afa --- /dev/null +++ b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreNativeRedisAcceptanceTest.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.commands.executor.set; + +import org.junit.ClassRule; + +import org.apache.geode.redis.NativeRedisClusterTestRule; + +public class SUnionStoreNativeRedisAcceptanceTest extends AbstractSUnionStoreIntegrationTest { + + @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/server/AbstractHitsMissesIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/server/AbstractHitsMissesIntegrationTest.java index 1c9a0da..5923e07 100644 --- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/server/AbstractHitsMissesIntegrationTest.java +++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/server/AbstractHitsMissesIntegrationTest.java @@ -419,6 +419,12 @@ public abstract class AbstractHitsMissesIntegrationTest implements RedisIntegrat runMultiKeyCommandAndAssertHitsAndMisses(SET_KEY, (k1, k2) -> jedis.sunion(k1, k2)); } + @Test + public void testSunionstore() { + runMultiKeyCommandAndAssertNoStatUpdates(SET_KEY, + (k1, k2) -> jedis.sunionstore(HASHTAG + "dest", k1, k2)); + } + /************* Hash related commands *************/ @Test public void testHset() { @@ -565,12 +571,6 @@ public abstract class AbstractHitsMissesIntegrationTest implements RedisIntegrat (k1, k2) -> jedis.sinterstore(HASHTAG + "dest", k1, k2)); } - @Test - public void testSunionstore() { - runMultiKeyCommandAndAssertNoStatUpdates(SET_KEY, - (k1, k2) -> jedis.sunionstore(HASHTAG + "dest", k1, k2)); - } - /************* Helper Methods *************/ private void runCommandAndAssertHitsAndMisses(String key, Consumer<String> command) { Map<String, String> info = RedisTestHelper.getInfo(jedis); diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionIntegrationTest.java index aa64424..7e6ac15 100755 --- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionIntegrationTest.java +++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionIntegrationTest.java @@ -22,9 +22,6 @@ import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CL import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; @@ -189,142 +186,4 @@ public abstract class AbstractSUnionIntegrationTest implements RedisIntegrationT jedis.sadd(SET_KEY_2, unionMembers); }); } - - @Test - public void testSUnionStore() { - String[] firstSet = new String[] {"pear", "apple", "plum", "orange", "peach"}; - String[] secondSet = new String[] {"apple", "microsoft", "linux", "peach"}; - String[] thirdSet = new String[] {"luigi", "bowser", "peach", "mario"}; - jedis.sadd("{tag1}set1", firstSet); - jedis.sadd("{tag1}set2", secondSet); - jedis.sadd("{tag1}set3", thirdSet); - - Long resultSize = jedis.sunionstore("{tag1}result", "{tag1}set1", "{tag1}set2"); - assertThat(resultSize).isEqualTo(7); - - Set<String> resultSet = jedis.smembers("{tag1}result"); - String[] expected = - new String[] {"pear", "apple", "plum", "orange", "peach", "microsoft", "linux"}; - assertThat(resultSet).containsExactlyInAnyOrder(expected); - - Long notEmptyResultSize = - jedis.sunionstore("{tag1}notempty", "{tag1}nonexistent", "{tag1}set1"); - Set<String> notEmptyResultSet = jedis.smembers("{tag1}notempty"); - assertThat(notEmptyResultSize).isEqualTo(firstSet.length); - assertThat(notEmptyResultSet).containsExactlyInAnyOrder(firstSet); - - jedis.sadd("{tag1}newEmpty", "born2die"); - jedis.srem("{tag1}newEmpty", "born2die"); - Long newNotEmptySize = - jedis.sunionstore("{tag1}newNotEmpty", "{tag1}set2", "{tag1}newEmpty"); - Set<String> newNotEmptySet = jedis.smembers("{tag1}newNotEmpty"); - assertThat(newNotEmptySize).isEqualTo(secondSet.length); - assertThat(newNotEmptySet).containsExactlyInAnyOrder(secondSet); - } - - @Test - public void testSUnionStore_withNonExistentKeys() { - String[] firstSet = new String[] {"pear", "apple", "plum", "orange", "peach"}; - jedis.sadd("{tag1}set1", firstSet); - - Long resultSize = - jedis.sunionstore("{tag1}set1", "{tag1}nonExistent1", "{tag1}nonExistent2"); - assertThat(resultSize).isEqualTo(0); - assertThat(jedis.exists("{tag1}set1")).isFalse(); - } - - @Test - public void testSUnionStore_withNonExistentKeys_andNonSetTarget() { - jedis.set("{tag1}string1", "stringValue"); - - Long resultSize = - jedis.sunionstore("{tag1}string1", "{tag1}nonExistent1", "{tag1}nonExistent2"); - assertThat(resultSize).isEqualTo(0); - assertThat(jedis.exists("{tag1}set1")).isFalse(); - } - - @Test - public void testSUnionStore_withNonSetKey() { - String[] firstSet = new String[] {"pear", "apple", "plum", "orange", "peach"}; - jedis.sadd("{tag1}set1", firstSet); - jedis.set("{tag1}string1", "value1"); - - assertThatThrownBy(() -> jedis.sunionstore("{tag1}set1", "{tag1}string1")) - .hasMessage("WRONGTYPE Operation against a key holding the wrong kind of value"); - assertThat(jedis.exists("{tag1}set1")).isTrue(); - } - - @Test - public void testConcurrentSUnionStore() throws InterruptedException { - int ENTRIES = 100; - int SUBSET_SIZE = 100; - - Set<String> masterSet = new HashSet<>(); - for (int i = 0; i < ENTRIES; i++) { - masterSet.add("master-" + i); - } - - List<Set<String>> otherSets = new ArrayList<>(); - for (int i = 0; i < ENTRIES; i++) { - Set<String> oneSet = new HashSet<>(); - for (int j = 0; j < SUBSET_SIZE; j++) { - oneSet.add("set-" + i + "-" + j); - } - otherSets.add(oneSet); - } - - jedis.sadd("{tag1}master", masterSet.toArray(new String[] {})); - - for (int i = 0; i < ENTRIES; i++) { - jedis.sadd("{tag1}set-" + i, otherSets.get(i).toArray(new String[] {})); - } - - Runnable runnable1 = () -> { - for (int i = 0; i < ENTRIES; i++) { - jedis.sunionstore("{tag1}master", "{tag1}master", "{tag1}set-" + i); - Thread.yield(); - } - }; - - Runnable runnable2 = () -> { - for (int i = 0; i < ENTRIES; i++) { - jedis.sunionstore("{tag1}master", "{tag1}master", "{tag1}set-" + i); - Thread.yield(); - } - }; - - Thread thread1 = new Thread(runnable1); - Thread thread2 = new Thread(runnable2); - - thread1.start(); - thread2.start(); - thread1.join(); - thread2.join(); - - otherSets.forEach(masterSet::addAll); - - assertThat(jedis.smembers("{tag1}master").toArray()) - .containsExactlyInAnyOrder(masterSet.toArray()); - } - - @Test - public void doesNotThrowExceptions_whenConcurrentSaddAndSunionExecute() { - final int ENTRIES = 1000; - Set<String> set1 = new HashSet<>(); - Set<String> set2 = new HashSet<>(); - for (int i = 0; i < ENTRIES; i++) { - set1.add("value-1-" + i); - set2.add("value-2-" + i); - } - - jedis.sadd("{player1}key1", set1.toArray(new String[] {})); - jedis.sadd("{player1}key2", set2.toArray(new String[] {})); - - new ConcurrentLoopingThreads(ENTRIES, - i -> jedis.sunion("{player1}key1", "{player1}key2"), - i -> jedis.sadd("{player1}key1", "newValue-1-" + i), - i -> jedis.sadd("{player1}key2", "newValue-2-" + i)) - .runInLockstep(); - } - } diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionStoreIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionStoreIntegrationTest.java new file mode 100644 index 0000000..7d0ee9b --- /dev/null +++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionStoreIntegrationTest.java @@ -0,0 +1,242 @@ +/* + * 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.set; + +import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs; +import static org.apache.geode.redis.internal.RedisConstants.ERROR_DIFFERENT_SLOTS; +import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_TYPE; +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.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.concurrent.atomic.AtomicLong; + +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 org.apache.geode.redis.ConcurrentLoopingThreads; +import org.apache.geode.redis.RedisIntegrationTest; + +public abstract class AbstractSUnionStoreIntegrationTest implements RedisIntegrationTest { + private JedisCluster jedis; + private static final String DESTINATION_KEY = "{tag1}destinationKey"; + private static final String[] DESTINATION_MEMBERS = + {"six", "seven", "eight", "nine", "ten", "one", "two"}; + private static final String SET_KEY_1 = "{tag1}setKey1"; + private static final String[] SET_MEMBERS_1 = {"one", "two", "three", "four", "five"}; + private static final String NON_EXISTENT_SET = "{tag1}nonExistentSet"; + private static final String SET_KEY_2 = "{tag1}setKey2"; + + @Before + public void setUp() { + jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()), REDIS_CLIENT_TIMEOUT); + } + + @After + public void tearDown() { + flushAll(); + jedis.close(); + } + + @Test + public void sunionstore_givenTooFewArguments_returnsError() { + assertAtLeastNArgs(jedis, Protocol.Command.SUNION, 1); + } + + @Test + public void sunionstore_withExistentSet_returnsUnionSize_storesUnion() { + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + assertThat(jedis.sunionstore(DESTINATION_KEY, SET_KEY_1)).isEqualTo(SET_MEMBERS_1.length); + assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(SET_MEMBERS_1); + } + + @Test + public void sunionstore_withNonExistentSet_returnsZero_destKeyDoesNotExist() { + assertThat(jedis.sunionstore(DESTINATION_KEY, NON_EXISTENT_SET)).isEqualTo(0); + assertThat(jedis.exists(DESTINATION_KEY)).isFalse(); + } + + @Test + public void sunionstore_withOneExistentAndOneNonExistentSet_returnsUnionSize_storesUnion() { + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + assertThat(jedis.sunionstore(DESTINATION_KEY, SET_KEY_1, NON_EXISTENT_SET)) + .isEqualTo(SET_MEMBERS_1.length); + assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(SET_MEMBERS_1); + } + + @Test + public void sunionstore_withOneNonExistentAndOneExistentSet_returnsUnionSize_storesUnion() { + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + assertThat(jedis.sunionstore(DESTINATION_KEY, NON_EXISTENT_SET, SET_KEY_1)) + .isEqualTo(SET_MEMBERS_1.length); + assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(SET_MEMBERS_1); + } + + @Test + public void sunionstore_withNonOverlappingSets_returnsUnionSize_storesUnion() { + String[] secondSetMembers = new String[] {"apple", "microsoft", "linux", "peach"}; + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + jedis.sadd(SET_KEY_2, secondSetMembers); + + String[] result = + {"one", "two", "three", "four", "five", "apple", "microsoft", "linux", "peach"}; + assertThat(jedis.sunionstore(DESTINATION_KEY, SET_KEY_1, SET_KEY_2)).isEqualTo(result.length); + assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(result); + + } + + @Test + public void sunionstore_withSomeSharedValues_returnsUnionSize_storesUnion() { + String[] secondSetMembers = new String[] {"one", "two", "linux", "peach"}; + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + jedis.sadd(SET_KEY_2, secondSetMembers); + + String[] result = {"one", "two", "three", "four", "five", "linux", "peach"}; + assertThat(jedis.sunionstore(DESTINATION_KEY, SET_KEY_1, SET_KEY_2)).isEqualTo(result.length); + assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(result); + } + + @Test + public void sunionstore_withAllSharedValues_returnsUnionSize_storesUnion() { + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + jedis.sadd(SET_KEY_2, SET_MEMBERS_1); + + assertThat(jedis.sunionstore(DESTINATION_KEY, SET_KEY_1, SET_KEY_2)) + .isEqualTo(SET_MEMBERS_1.length); + assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(SET_MEMBERS_1); + } + + @Test + public void sunionstore_withMultipleNonExistentSets_returnsZero_destKeyDoesNotExist() { + assertThat(jedis.sunionstore(DESTINATION_KEY, NON_EXISTENT_SET, "{tag1}nonExistentSet2")) + .isEqualTo(0); + assertThat(jedis.exists(DESTINATION_KEY)).isFalse(); + } + + @Test + public void sunionstore_withExistentSet_returnsUnionSize_destKeyOverwrittenWithUnion() { + jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS); + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + assertThat(jedis.sunionstore(DESTINATION_KEY, SET_KEY_1)).isEqualTo(SET_MEMBERS_1.length); + assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(SET_MEMBERS_1); + } + + @Test + public void sunionstore_withNonExistentSet_returnsZero_destKeyIsDeleted() { + jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS); + assertThat(jedis.sunionstore(DESTINATION_KEY, NON_EXISTENT_SET)).isEqualTo(0); + assertThat(jedis.exists(DESTINATION_KEY)).isFalse(); + } + + @Test + public void sunionstore_withNonSetDestKey_withExistentSet_returnsUnionSize_destKeyOverwrittenWithUnion() { + String stringKey = "{tag1}ding"; + jedis.set(stringKey, "dong"); + + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + assertThat(jedis.sunionstore(stringKey, SET_KEY_1)).isEqualTo(SET_MEMBERS_1.length); + assertThat(jedis.smembers(stringKey)).containsExactlyInAnyOrder(SET_MEMBERS_1); + } + + @Test + public void sunionstore_withNonSetDestKey_withNonExistentSet_returnsZero_destKeyIsDeleted() { + String stringKey = "{tag1}ding"; + jedis.set(stringKey, "dong"); + + assertThat(jedis.sunionstore(stringKey, NON_EXISTENT_SET)).isEqualTo(0); + assertThat(jedis.exists(DESTINATION_KEY)).isFalse(); + } + + @Test + public void sunionstore_withNonExistentDest_withNonSetKeyAsFirstKey_returnsWrongTypeError() { + String stringKey = "{tag1}ding"; + jedis.set(stringKey, "dong"); + + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + jedis.sadd(SET_KEY_2, "doorbell"); + assertThatThrownBy(() -> jedis.sunionstore(DESTINATION_KEY, stringKey, SET_KEY_1, SET_KEY_2)) + .hasMessageContaining(ERROR_WRONG_TYPE); + } + + @Test + public void sunionstore_withExistentDest_withNonSetKeyAsThirdKey_returnsWrongTypeError() { + String stringKey = "{tag1}ding"; + jedis.set(stringKey, "dong"); + + jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS); + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + jedis.sadd(SET_KEY_2, "doorbell"); + assertThatThrownBy(() -> jedis.sunionstore(DESTINATION_KEY, SET_KEY_1, SET_KEY_2, stringKey)) + .hasMessageContaining(ERROR_WRONG_TYPE); + } + + @Test + public void sunionstore_withNonExistentDest_withNonSetKeyAsThirdKeyAndNonExistentSetAsFirstKey_returnsWrongTypeError() { + String stringKey = "{tag1}ding"; + jedis.set(stringKey, "dong"); + + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + jedis.sadd(SET_KEY_2, "doorbell"); + assertThatThrownBy( + () -> jedis.sunionstore(DESTINATION_KEY, NON_EXISTENT_SET, SET_KEY_1, stringKey)) + .hasMessageContaining(ERROR_WRONG_TYPE); + } + + @Test + public void sunionstore_withSetsFromDifferentSlots_returnsCrossSlotError() { + String setKeyDifferentSlot = "{tag2}setKey2"; + String[] secondSetMembers = new String[] {"one", "two", "linux", "peach"}; + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + jedis.sadd(setKeyDifferentSlot, secondSetMembers); + + assertThatThrownBy(() -> jedis.sunionstore(DESTINATION_KEY, SET_KEY_1, setKeyDifferentSlot)) + .hasMessageContaining(ERROR_DIFFERENT_SLOTS); + } + + @Test + public void ensureSetConsistency_whenRunningConcurrently() { + String[] secondSetMembers = new String[] {"one", "two", "linux", "peach"}; + jedis.sadd(SET_KEY_1, SET_MEMBERS_1); + jedis.sadd(SET_KEY_2, secondSetMembers); + + String[] unionMembers = {"one", "two", "three", "four", "five", "linux", "peach"}; + final AtomicLong sunionstoreResultReference = new AtomicLong(0); + new ConcurrentLoopingThreads(1000, + i -> jedis.srem(SET_KEY_2, secondSetMembers), + i -> sunionstoreResultReference + .set(jedis.sunionstore(DESTINATION_KEY, SET_KEY_1, SET_KEY_2))) + .runWithAction(() -> { + // Check sunionstore return size of union + assertThat(sunionstoreResultReference).satisfiesAnyOf( + sunionstoreSize -> assertThat(sunionstoreSize.get()) + .isEqualTo(SET_MEMBERS_1.length), + sunionstoreSize -> assertThat(sunionstoreSize.get()) + .isEqualTo(unionMembers.length)); + // Checks if values were stored in destination key + assertThat(DESTINATION_KEY).satisfiesAnyOf( + key -> assertThat(jedis.smembers(DESTINATION_KEY)) + .containsExactlyInAnyOrder(SET_MEMBERS_1), + key -> assertThat(jedis.smembers(DESTINATION_KEY)) + .containsExactlyInAnyOrder(unionMembers)); + jedis.sadd(SET_KEY_2, secondSetMembers); + jedis.del(DESTINATION_KEY); + }); + } +} diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreIntegrationTest.java new file mode 100644 index 0000000..ff365ff --- /dev/null +++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreIntegrationTest.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.set; + +import org.junit.ClassRule; + +import org.apache.geode.redis.GeodeRedisServerRule; + +public class SUnionStoreIntegrationTest extends AbstractSUnionStoreIntegrationTest { + + @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/commands/RedisCommandType.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java index 3a461de..ef1d703 100755 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java +++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java @@ -253,6 +253,8 @@ public enum RedisCommandType { SREM(new SRemExecutor(), SUPPORTED, new Parameter().min(3).flags(WRITE, FAST)), SUNION(new SUnionExecutor(), SUPPORTED, new Parameter().min(2).lastKey(-1).flags(READONLY, SORT_FOR_SCRIPT)), + SUNIONSTORE(new SUnionStoreExecutor(), SUPPORTED, + new Parameter().min(3).lastKey(-1).flags(WRITE, DENYOOM)), /************ Sorted Sets **************/ @@ -357,8 +359,6 @@ public enum RedisCommandType { new Parameter().min(2).flags(READONLY, RANDOM)), SSCAN(new SScanExecutor(), UNSUPPORTED, new Parameter().min(3).flags(READONLY, RANDOM), new Parameter().odd(ERROR_SYNTAX).firstKey(0)), - SUNIONSTORE(new SUnionStoreExecutor(), UNSUPPORTED, - new Parameter().min(3).lastKey(-1).flags(WRITE, DENYOOM)), /*************** Server ****************/ diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SetOpExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SetOpExecutor.java index e57a0d0..b83c8e2 100755 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SetOpExecutor.java +++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SetOpExecutor.java @@ -19,6 +19,7 @@ import static org.apache.geode.redis.internal.data.RedisSet.sdiff; import static org.apache.geode.redis.internal.data.RedisSet.sdiffstore; import static org.apache.geode.redis.internal.data.RedisSet.sinter; import static org.apache.geode.redis.internal.data.RedisSet.sunion; +import static org.apache.geode.redis.internal.data.RedisSet.sunionstore; import java.util.ArrayList; import java.util.List; @@ -57,8 +58,7 @@ public abstract class SetOpExecutor implements CommandExecutor { } /* - * SINTERSTORE, SUNIONSTORE currently use the else part of the code - * for their implementation. + * SINTERSTORE currently use the else part of the code for their implementation. * TODO: Once the above commands have been implemented remove the if else * Refactor so the implementation is in the executor. After delete doActualSetOperation, * doStoreSetOp, doStoreSetOpWhileLocked, computeStoreSetOp, fetchSets @@ -74,11 +74,21 @@ public abstract class SetOpExecutor implements CommandExecutor { Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), new ArrayList<>(setKeys), () -> sdiff(regionProvider, setKeys)); return RedisResponse.array(resultSet, true); + } else if (command.isOfType(RedisCommandType.SINTER)) { Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), new ArrayList<>(setKeys), () -> sinter(regionProvider, setKeys)); return RedisResponse.array(resultSet, true); - } else if (command.isOfType(RedisCommandType.SUNION)) { + + } else if (command.isOfType(RedisCommandType.SUNION) + || command.isOfType(RedisCommandType.SUNIONSTORE)) { + if (isStorage()) { + RedisKey destinationKey = command.getKey(); + int resultSize = context.lockedExecute(destinationKey, new ArrayList<>(setKeys), + () -> sunionstore(regionProvider, destinationKey, setKeys)); + return RedisResponse.integer(resultSize); + } + Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), new ArrayList<>(setKeys), () -> sunion(regionProvider, setKeys)); return RedisResponse.array(resultSet, true); diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java index cf162ec..f17d909 100644 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java +++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java @@ -96,6 +96,12 @@ public class RedisSet extends AbstractRedisData { return calculateUnion(regionProvider, keys, true); } + public static int sunionstore(RegionProvider regionProvider, RedisKey destinationKey, + List<RedisKey> keys) { + MemberSet union = calculateUnion(regionProvider, keys, false); + return setOpStoreResult(regionProvider, destinationKey, union); + } + private static MemberSet calculateUnion(RegionProvider regionProvider, List<RedisKey> keys, boolean updateStats) { MemberSet union = new MemberSet(); @@ -180,10 +186,10 @@ public class RedisSet extends AbstractRedisData { @VisibleForTesting static int setOpStoreResult(RegionProvider regionProvider, RedisKey destinationKey, - MemberSet diff) { + MemberSet result) { RedisSet destinationSet = regionProvider.getTypedRedisDataElseRemove(REDIS_SET, destinationKey, false); - if (diff.isEmpty()) { + if (result.isEmpty()) { if (destinationSet != null) { regionProvider.getDataRegion().remove(destinationKey); } @@ -192,14 +198,14 @@ public class RedisSet extends AbstractRedisData { if (destinationSet != null) { destinationSet.persistNoDelta(); - destinationSet.members = diff; + destinationSet.members = result; destinationSet.storeChanges(regionProvider.getDataRegion(), destinationKey, - new ReplaceByteArrays(diff)); + new ReplaceByteArrays(result)); } else { - regionProvider.getDataRegion().put(destinationKey, new RedisSet(diff)); + regionProvider.getDataRegion().put(destinationKey, new RedisSet(result)); } - return diff.size(); + return result.size(); } public Pair<BigInteger, List<Object>> sscan(GlobPattern matchPattern, int count,