Repository: fppt/jedis-mock (java)

Issue:
SimpleOperationsTest.sscanIterates is sometimes failing (flaky test)
When I try building the mock and running the test, one test case sometimes (in about 40% of the attempts) fails:

```
sscanIterates(com.github.fppt.jedismock.comparisontests.SimpleOperationsTest)  Time elapsed: 0.007 sec  <<< FAILURE!
org.junit.experimental.theories.internal.ParameterizedAssertionError: sscanIterates("redis.clients.jedis.Jedis@647e447" <from jedis[0]>)
        at org.junit.Assert.fail(Assert.java:88)
        at org.junit.Assert.failNotEquals(Assert.java:834)
        at org.junit.Assert.assertEquals(Assert.java:645)
        at org.junit.Assert.assertEquals(Assert.java:631)
        at com.github.fppt.jedismock.comparisontests.SimpleOperationsTest.sscanIterates(SimpleOperationsTest.java:640)
```
When trying to use debugger, I found out that the test measures iterations in sscan, expecting it to take 5 iterations with 10 "page" size to fetch 45 elements.

However, sometimes I've seen in the debugger that higher amount of elements are returned per call (between 11 and 13 instead of 10), and as the sscan then completes in only 4 iterations instead of 5, it ends as assertion failure.

Possibly some race condition with other test case modifying default "page" size?

I am using OpenJDK 12 for the build.

Pull request description:
sscanIterates test should not count iterations
According to the Redis documentation (https://redis.io/commands/scan), there is no guarantee on the count of items returned by scan, sscan or other related methods. Even specifying a COUNT, the scan methods only use this as a suggestion.

As a result, the sscanIterates test that was counting the number of times the sscan iterates would sometimes fail (if sscan returned more than 10 elements at a time, which is completely valid). Instead of counting the number of iterations, which can never be predicted, I've modified the test to make sure all elements were returned after all iterations complete.

Also removing the `jedis.del(key)` line as the entire DB is erased at the start of the test already.

Fixes #64 

Patch:
diff --git a/src/main/java/com/github/fppt/jedismock/operations/OperationFactory.java b/src/main/java/com/github/fppt/jedismock/operations/OperationFactory.java
index 64d17e3..c7bc7da 100644
--- a/src/main/java/com/github/fppt/jedismock/operations/OperationFactory.java
+++ b/src/main/java/com/github/fppt/jedismock/operations/OperationFactory.java
@@ -68,6 +68,7 @@ public class OperationFactory {
         TRANSACTIONAL_OPERATIONS.put("hset", RO_hset::new);
         TRANSACTIONAL_OPERATIONS.put("hdel", RO_hdel::new);
         TRANSACTIONAL_OPERATIONS.put("hkeys", RO_hkeys::new);
+        TRANSACTIONAL_OPERATIONS.put("hlen", RO_hlen::new);
         TRANSACTIONAL_OPERATIONS.put("hgetall", RO_hgetall::new);
         TRANSACTIONAL_OPERATIONS.put("sinter", RO_sinter::new);
         TRANSACTIONAL_OPERATIONS.put("hmget", RO_hmget::new);
@@ -80,6 +81,7 @@ public class OperationFactory {
         TRANSACTIONAL_OPERATIONS.put("zadd", RO_zadd::new);
         TRANSACTIONAL_OPERATIONS.put("zcard", RO_zcard::new);
         TRANSACTIONAL_OPERATIONS.put("zrange", RO_zrange::new);
+        TRANSACTIONAL_OPERATIONS.put("zrangebylex", RO_zrangebylex::new);
         TRANSACTIONAL_OPERATIONS.put("zrem", RO_zrem::new);
     }
 
@@ -104,6 +106,8 @@ public class OperationFactory {
                 return Optional.of(new RO_unsubscribe(state, params));
             case "quit":
                 return Optional.of(new RO_quit(state));
+            case "auth":
+                return Optional.of(new RO_auth(state));
             case "exec":
                 return Optional.of(new RO_exec(state));
             default:
diff --git a/src/main/java/com/github/fppt/jedismock/operations/RO_auth.java b/src/main/java/com/github/fppt/jedismock/operations/RO_auth.java
new file mode 100644
index 0000000..75d4b92
--- /dev/null
+++ b/src/main/java/com/github/fppt/jedismock/operations/RO_auth.java
@@ -0,0 +1,19 @@
+package com.github.fppt.jedismock.operations;
+
+import com.github.fppt.jedismock.server.Response;
+import com.github.fppt.jedismock.server.Slice;
+import com.github.fppt.jedismock.storage.OperationExecutorState;
+
+public class RO_auth implements RedisOperation {
+    private OperationExecutorState state;
+
+    public RO_auth(OperationExecutorState state) {
+        this.state = state;
+    }
+
+    @Override
+    public Slice execute() {
+        state.owner().sendResponse(Response.clientResponse("auth", Response.OK), "auth");
+        return Response.SKIP;
+    }
+}
diff --git a/src/main/java/com/github/fppt/jedismock/operations/RO_hlen.java b/src/main/java/com/github/fppt/jedismock/operations/RO_hlen.java
new file mode 100644
index 0000000..1a1f4bf
--- /dev/null
+++ b/src/main/java/com/github/fppt/jedismock/operations/RO_hlen.java
@@ -0,0 +1,20 @@
+package com.github.fppt.jedismock.operations;
+
+import com.github.fppt.jedismock.server.Response;
+import com.github.fppt.jedismock.server.Slice;
+import com.github.fppt.jedismock.storage.RedisBase;
+
+import java.util.List;
+import java.util.Map;
+
+public class RO_hlen extends AbstractRedisOperation {
+    public RO_hlen(RedisBase base, List<Slice> params) {
+        super(base, params);
+    }
+
+    Slice response() {
+        Slice key = params().get(0);
+        Map<Slice, Slice> map = base().getFieldsAndValues(key);
+        return Response.integer(map.size());
+    }
+}
diff --git a/src/main/java/com/github/fppt/jedismock/operations/RO_zrangebylex.java b/src/main/java/com/github/fppt/jedismock/operations/RO_zrangebylex.java
new file mode 100644
index 0000000..8eaabd6
--- /dev/null
+++ b/src/main/java/com/github/fppt/jedismock/operations/RO_zrangebylex.java
@@ -0,0 +1,62 @@
+package com.github.fppt.jedismock.operations;
+
+import com.github.fppt.jedismock.server.Response;
+import com.github.fppt.jedismock.server.Slice;
+import com.github.fppt.jedismock.storage.RedisBase;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+
+class RO_zrangebylex extends AbstractRedisOperation {
+
+    private static final String START_UNBOUNDED = "-";
+    private static final String END_UNBOUNDED = "+";
+    private static final String INCLUSIVE_PREFIX = "[";
+    private static final String EXCLUSIVE_PREFIX = "(";
+
+    RO_zrangebylex(RedisBase base, List<Slice> params) {
+        super(base, params);
+    }
+
+    @Override
+    Slice response() {
+        Slice key = params().get(0);
+        LinkedHashMap<Slice, Double> map = getDataFromBase(key, new LinkedHashMap<>());
+
+        String start = params().get(1).toString();
+        if (!START_UNBOUNDED.equals(start)
+                && !start.startsWith(INCLUSIVE_PREFIX)
+                && !start.startsWith(EXCLUSIVE_PREFIX)) {
+            throw new RuntimeException("Valid start must start with '" + INCLUSIVE_PREFIX + "' or '"
+                    + EXCLUSIVE_PREFIX + "'");
+        }
+
+        final Predicate<Slice> compareToStart = p -> START_UNBOUNDED.equals(start) ||
+                (start.startsWith(INCLUSIVE_PREFIX)
+                        ? p.toString().compareTo(start.substring(1)) >= 0
+                        : p.toString().compareTo(start.substring(1)) > 0);
+
+        String end = params().get(2).toString();
+        if (!END_UNBOUNDED.equals(end)
+                && !end.startsWith(INCLUSIVE_PREFIX)
+                && !end.startsWith(EXCLUSIVE_PREFIX)) {
+            throw new RuntimeException("Valid end must start with '" + INCLUSIVE_PREFIX + "' or '"
+                    + EXCLUSIVE_PREFIX + "'");
+        }
+
+        final Predicate<Slice> compareToEnd = p -> END_UNBOUNDED.equals(end) ||
+                (end.startsWith(INCLUSIVE_PREFIX)
+                        ? p.toString().compareTo(end.substring(1)) <= 0
+                        : p.toString().compareTo(end.substring(1)) < 0);
+
+        List<Slice> values = map.keySet().stream()
+                .filter(compareToStart.and(compareToEnd))
+                .sorted()
+                .map(Response::bulkString)
+                .collect(Collectors.toList());
+
+        return Response.array(values);
+    }
+}


Test patch:
diff --git a/src/test/java/com/github/fppt/jedismock/comparisontests/SimpleOperationsTest.java b/src/test/java/com/github/fppt/jedismock/comparisontests/SimpleOperationsTest.java
index c8112e0..a58a1a0 100644
--- a/src/test/java/com/github/fppt/jedismock/comparisontests/SimpleOperationsTest.java
+++ b/src/test/java/com/github/fppt/jedismock/comparisontests/SimpleOperationsTest.java
@@ -361,6 +361,18 @@ public class SimpleOperationsTest extends ComparisonBase {
         assertEquals(result, toCompare);
     }
 
+    @Theory
+    public void whenHLen_EnsureCorrectLengthReturned(Jedis jedis) {
+        jedis.flushDB();
+
+        jedis.hset(HASH, FIELD_1, VALUE_1);
+        jedis.hset(HASH, FIELD_2, VALUE_2);
+
+        long result = jedis.hlen(HASH);
+
+        assertEquals(2, result);
+    }
+
     @Theory
     public void whenUsingHsinter_EnsureSetIntersectionIsReturned(Jedis jedis) {
         String key1 = "my-set-key-1";
@@ -580,7 +592,7 @@ public class SimpleOperationsTest extends ComparisonBase {
         jedis.flushDB();
 
         String key = "sscankey";
-        jedis.del(key);
+
         String[] values = new String[45];
         for (int i = 0; i < 45; i++) {
             values[i] = (45 - i) + "_value_" + i;
@@ -588,16 +600,17 @@ public class SimpleOperationsTest extends ComparisonBase {
         jedis.sadd(key, values);
         String cursor = null;
 
-        int iterations = 0;
+        Set<String> results = new HashSet<>();
         while (cursor == null || !cursor.equals(ScanParams.SCAN_POINTER_START)) {
             if (cursor == null) {
                 cursor = ScanParams.SCAN_POINTER_START;
             }
             ScanResult<String> result = jedis.sscan(key, cursor);
             cursor = result.getStringCursor();
-            iterations++;
+            results.addAll(result.getResult());
         }
-        assertEquals(5, iterations);
+
+        assertTrue(results.containsAll(Arrays.asList(values)));
     }
 
     @Theory
@@ -770,4 +783,34 @@ public class SimpleOperationsTest extends ComparisonBase {
         assertEquals("myvalue4", results.get(3).getElement());
         assertEquals(20d, results.get(3).getScore(), 0);
     }
+
+    @Theory
+    public void zrangebylexKeysCorrectOrder(Jedis jedis) {
+        jedis.flushDB();
+
+        String key = "mykey";
+        Map<String, Double> members = new HashMap<>();
+        members.put("bbb", 0d);
+        members.put("ddd", 0d);
+        members.put("ccc", 0d);
+        members.put("aaa", 0d);
+
+        long result = jedis.zadd(key, members);
+
+        assertEquals(4L, result);
+
+        List<String> results = new ArrayList<>(jedis.zrangeByLex(key, "-", "+"));
+
+        assertEquals(4, results.size());
+        assertEquals("aaa", results.get(0));
+        assertEquals("bbb", results.get(1));
+        assertEquals("ccc", results.get(2));
+        assertEquals("ddd", results.get(3));
+
+        results = new ArrayList<>(jedis.zrangeByLex(key, "[bbb", "(ddd"));
+
+        assertEquals(2, results.size());
+        assertEquals("bbb", results.get(0));
+        assertEquals("ccc", results.get(1));
+    }
 }