Skip to content

Commit

Permalink
GEODE-9826: SCARD command supported (apache#7160)
Browse files Browse the repository at this point in the history
  • Loading branch information
Kris-10-0 authored Dec 7, 2021
1 parent ee1471f commit 7e01634
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 92 deletions.
1 change: 1 addition & 0 deletions geode-docs/tools_modules/geode_for_redis.html.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ If the server is functioning properly, you should see a response of `PONG`.
- QUIT <br/>
- RENAME <br/>
- SADD <br/>
- SCARD <br/>
- SDIFF <br/>
- SET <br/>
- SETNX <br/>
Expand Down
1 change: 1 addition & 0 deletions geode-for-redis/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ Geode for Redis implements a subset of the full Redis command set.
- RENAME
- RENAMENX
- SADD
- SCARD
- SDIFF
- SET
- SETEX
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* 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 SCardNativeRedisAcceptanceTest extends AbstractSCardIntegrationTest {
@ClassRule
public static NativeRedisClusterTestRule redis = new NativeRedisClusterTestRule();

@Override
public int getPort() {
return redis.getExposedPorts().get(0);
}

@Override
public void flushAll() {
redis.flushAll();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,11 @@ public void testSadd() {
runCommandAndAssertNoStatUpdates(SET_KEY, k -> jedis.sadd(k, "member"));
}

@Test
public void testScard() {
runCommandAndAssertHitsAndMisses(SET_KEY, k -> jedis.scard(k));
}

@Test
public void testSdiff() {
runMultiKeyCommandAndAssertHitsAndMisses(SET_KEY, (k1, k2) -> jedis.sdiff(k1, k2));
Expand Down Expand Up @@ -528,11 +533,6 @@ public void testSrandmember() {
runCommandAndAssertHitsAndMisses(SET_KEY, k -> jedis.srandmember(k));
}

@Test
public void testScard() {
runCommandAndAssertHitsAndMisses(SET_KEY, k -> jedis.scard(k));
}

@Test
public void testSscan() {
runCommandAndAssertHitsAndMisses(SET_KEY, k -> jedis.sscan(k, "0"));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* 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.assertExactNumberOfArgs;
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.assertThatThrownBy;
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;

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 AbstractSCardIntegrationTest implements RedisIntegrationTest {
private JedisCluster jedis;
private static final String key = "{user1}setKey";

@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 scardWrongNumOfArgs_returnsError() {
assertExactNumberOfArgs(jedis, Protocol.Command.SCARD, 1);
}

@Test
public void scardSet_returnsSize() {
String[] values = new String[] {"Jeff", "Natalie", "Michelle", "Joe", "Kelley"};
jedis.sadd(key, values);
assertThat(jedis.scard(key)).isEqualTo(values.length);
}

@Test
public void scardNonExistentSet_returnsZero() {
assertThat(jedis.scard("{user1}nonExistentSet")).isEqualTo(0);
}

@Test
public void scardAfterSadd_returnsSize() {
String[] valuesInitial = new String[] {"Jeff", "Natalie", "Michelle", "Joe", "Kelley"};
String[] valuesToAdd = new String[] {"one", "two", "three"};

jedis.sadd(key, valuesInitial);
long size = jedis.scard(key);
assertThat(size).isEqualTo(valuesInitial.length);
jedis.sadd(key, valuesToAdd);
assertThat(jedis.scard(key)).isEqualTo(size + valuesToAdd.length);
}


@Test
public void scardWithConcurrentSAdd_returnsCorrectValue() {
String[] valuesInitial = new String[] {"one", "two", "three"};
String[] valuesToAdd = new String[] {"pear", "apple", "plum", "orange", "peach"};
jedis.sadd(key, valuesInitial);

final AtomicLong scardReference = new AtomicLong();
new ConcurrentLoopingThreads(1000,
i -> jedis.sadd(key, valuesToAdd),
i -> scardReference.set(jedis.scard(key)))
.runWithAction(() -> {
assertThat(scardReference).satisfiesAnyOf(
scardResult -> assertThat(scardResult.get()).isEqualTo(valuesInitial.length),
scardResult -> assertThat(scardResult.get())
.isEqualTo(valuesInitial.length + valuesToAdd.length));
jedis.srem(key, valuesToAdd);
});
}

@Test
public void scardWithWrongKeyType_returnsWrongTypeError() {
jedis.set("ding", "dong");
assertThatThrownBy(() -> jedis.scard("ding")).hasMessageContaining(ERROR_WRONG_TYPE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,12 @@
package org.apache.geode.redis.internal.commands.executor.set;

import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertExactNumberOfArgs;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;

import org.junit.After;
import org.junit.Before;
Expand Down Expand Up @@ -63,26 +57,6 @@ public void saddErrors_givenTooFewArguments() {
assertAtLeastNArgs(jedis, Protocol.Command.SADD, 2);
}

@Test
public void scardErrors_givenWrongNumberOfArguments() {
assertExactNumberOfArgs(jedis, Protocol.Command.SCARD, 1);
}

@Test
public void testSAddSCard() {
int elements = 10;
String key = generator.generate('x');
String[] members = generateStrings(elements, 'x');

Long response = jedis.sadd(key, members);
assertThat(response).isEqualTo(members.length);

Long response2 = jedis.sadd(key, members);
assertThat(response2).isEqualTo(0L);

assertThat(jedis.scard(key)).isEqualTo(members.length);
}

@Test
public void testSAdd_withExistingKey_ofWrongType_shouldReturnError() {
String key = "key";
Expand Down Expand Up @@ -124,66 +98,6 @@ public void testSAdd_canStoreBinaryData() {
assertThat(result).containsExactly(blob);
}

@Test
public void testConcurrentSAddSCard_sameKeyPerClient()
throws InterruptedException, ExecutionException {
int elements = 1000;

String key = generator.generate('x');
String[] members1 = generateStrings(elements, 'y');
String[] members2 = generateStrings(elements, 'z');

ExecutorService pool = Executors.newFixedThreadPool(2);
Callable<Integer> callable1 = () -> doABunchOfSAdds(key, members1, jedis);
Callable<Integer> callable2 = () -> doABunchOfSAdds(key, members2, jedis);
Future<Integer> future1 = pool.submit(callable1);
Future<Integer> future2 = pool.submit(callable2);

assertThat(future1.get()).isEqualTo(members1.length);
assertThat(future2.get()).isEqualTo(members2.length);

assertThat(jedis.scard(key)).isEqualTo(members1.length + members2.length);

pool.shutdown();
}

@Test
public void testConcurrentSAddSCard_differentKeyPerClient()
throws InterruptedException, ExecutionException {
int elements = 1000;
String key1 = generator.generate('x');
String key2 = generator.generate('y');

String[] strings = generateStrings(elements, 'y');

ExecutorService pool = Executors.newFixedThreadPool(2);
Callable<Integer> callable1 = () -> doABunchOfSAdds(key1, strings, jedis);
Callable<Integer> callable2 = () -> doABunchOfSAdds(key2, strings, jedis);
Future<Integer> future1 = pool.submit(callable1);
Future<Integer> future2 = pool.submit(callable2);

assertThat(future1.get()).isEqualTo(strings.length);
assertThat(future2.get()).isEqualTo(strings.length);

assertThat(jedis.scard(key1)).isEqualTo(strings.length);
assertThat(jedis.scard(key2)).isEqualTo(strings.length);

pool.shutdown();
}

private int doABunchOfSAdds(String key, String[] strings, JedisCluster jedis) {
int successes = 0;

for (int i = 0; i < strings.length; i++) {
Long reply = jedis.sadd(key, strings[i]);
if (reply == 1L) {
successes++;
Thread.yield();
}
}
return successes;
}

@Test
public void srandmember_withStringFails() {
jedis.set("string", "value");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* 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 SCardIntegrationTest extends AbstractSCardIntegrationTest {
@ClassRule
public static GeodeRedisServerRule server = new GeodeRedisServerRule();

@Override
public int getPort() {
return server.getPort();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ public enum RedisCommandType {
/************* Sets *****************/

SADD(new SAddExecutor(), SUPPORTED, new Parameter().min(3).flags(WRITE, DENYOOM, FAST)),
SCARD(new SCardExecutor(), SUPPORTED, new Parameter().exact(2).flags(READONLY, FAST)),
SDIFF(new SDiffExecutor(), SUPPORTED,
new Parameter().min(2).lastKey(-1).flags(READONLY, SORT_FOR_SCRIPT)),
SMEMBERS(new SMembersExecutor(), SUPPORTED,
Expand Down Expand Up @@ -341,7 +342,6 @@ public enum RedisCommandType {

/**************** Sets *****************/

SCARD(new SCardExecutor(), UNSUPPORTED, new Parameter().exact(2).flags(READONLY, FAST)),
SDIFFSTORE(new SDiffStoreExecutor(), UNSUPPORTED,
new Parameter().min(3).lastKey(-1).flags(WRITE, DENYOOM)),
SINTER(new SInterExecutor(), UNSUPPORTED,
Expand Down

0 comments on commit 7e01634

Please sign in to comment.