json.arrpop forces index=-1 with root path (#3214) (#3217)

* Fix json.arrpop with root path

Fix IntegrationTest

Add Integration Test

Fix test

Update the logic

Update logic

Update the logic

Correct the test

Typo

* Add the root path even index = -1

* Only add root path if index != -1

Author: thachlp

src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java

@@ -106,13 +106,14 @@ Command<K, V, List<JsonValue>> jsonArrpop(K key, JsonPath jsonPath, int index) {
 
         CommandArgs<K, V> args = new CommandArgs<>(codec).addKey(key);
 
-        if (jsonPath != null && !jsonPath.isRootPath()) {
-            // OPTIONAL as per API
-            args.add(jsonPath.toString());
-
+        if (jsonPath != null) {
             if (index != -1) {
                 // OPTIONAL as per API
+                args.add(jsonPath.toString());
                 args.add(index);
+            } else if (!jsonPath.isRootPath()) {
+                // OPTIONAL as per API
+                args.add(jsonPath.toString());
             }
         }
 

src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java

@@ -157,7 +157,7 @@ void shouldCorrectlyConstructJsonArrpopNoIndex() {
     }
 
     @Test
-    void shouldCorrectlyConstructJsonArrpopRootPath() {
+    void shouldCorrectlyConstructJsonArrpopRootPathNoIndex() {
         Command<String, String, List<JsonValue>> command = builder.jsonArrpop(MY_KEY, JsonPath.ROOT_PATH, -1);
         ByteBuf buf = Unpooled.directBuffer();
         command.encode(buf);
@@ -166,6 +166,16 @@ void shouldCorrectlyConstructJsonArrpopRootPath() {
                 .isEqualTo("*2\r\n" + "$11\r\n" + "JSON.ARRPOP\r\n" + "$15\r\n" + "bikes:inventory\r\n");
     }
 
+    @Test
+    void shouldCorrectlyConstructJsonArrpopRootPath() {
+        Command<String, String, List<JsonValue>> command = builder.jsonArrpop(MY_KEY, JsonPath.ROOT_PATH, 1);
+        ByteBuf buf = Unpooled.directBuffer();
+        command.encode(buf);
+
+        assertThat(buf.toString(StandardCharsets.UTF_8)).isEqualTo("*4\r\n" + "$11\r\n" + "JSON.ARRPOP\r\n" + "$15\r\n"
+                + "bikes:inventory\r\n" + "$1\r\n" + "$\r\n" + "$1\r\n" + "1\r\n");
+    }
+
     @Test
     void shouldCorrectlyConstructJsonArrtrim() {
         JsonRangeArgs range = JsonRangeArgs.Builder.start(0).stop(1);

src/test/java/io/lettuce/core/json/RedisJsonIntegrationTests.java

@@ -163,8 +163,16 @@ public void jsonArrpopEmptyArray() {
         redis.jsonSet("myKey", JsonPath.ROOT_PATH, value);
         List<JsonValue> result = redis.jsonArrpop("myKey");
         assertThat(result.toString()).isEqualTo("[\"one\"]");
-        result = redis.jsonArrpop("myKey", JsonPath.ROOT_PATH, 0);
-        assertThat(result.get(0).isNull()).isTrue();
+        assertThat(redis.jsonGet("myKey").get(0).toString()).isEqualTo("[]");
+    }
+
+    @Test
+    public void jsonArrpopWithRootPathAndIndex() {
+        JsonValue value = redis.getJsonParser().createJsonValue("[\"one\",\"two\",\"three\"]");
+        redis.jsonSet("myKey", JsonPath.ROOT_PATH, value);
+        List<JsonValue> result = redis.jsonArrpop("myKey", JsonPath.ROOT_PATH, 1);
+        assertThat(result.toString()).isEqualTo("[\"two\"]");
+        assertThat(redis.jsonGet("myKey").get(0).toString()).isEqualTo("[\"one\",\"three\"]");
     }
 
     @ParameterizedTest(name = "With {0} as path")