From bd95177ff8f4197ad8938d0b2f476190fa4efb32 Mon Sep 17 00:00:00 2001 From: Thach Le Date: Sat, 15 Mar 2025 16:57:17 +0700 Subject: [PATCH 1/3] 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 --- .../lettuce/core/RedisJsonCommandBuilder.java | 18 +++++++++++------- .../core/RedisJsonCommandBuilderUnitTests.java | 12 +++++++++++- .../core/json/RedisJsonIntegrationTests.java | 12 ++++++++++-- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java b/src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java index bd713854cb..2d0bf2e9e7 100644 --- a/src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java +++ b/src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java @@ -106,13 +106,17 @@ Command> jsonArrpop(K key, JsonPath jsonPath, int index) { CommandArgs args = new CommandArgs<>(codec).addKey(key); - if (jsonPath != null && !jsonPath.isRootPath()) { - // OPTIONAL as per API - args.add(jsonPath.toString()); - - if (index != -1) { - // OPTIONAL as per API - args.add(index); + if (jsonPath != null) { + if (!jsonPath.isRootPath()) { + args.add(jsonPath.toString()); + if (index != -1) { + args.add(index); + } + } else { + if (index != -1) { + args.add(jsonPath.toString()); + args.add(index); + } } } diff --git a/src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java b/src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java index fbb2ae3fbb..28df5f01a9 100644 --- a/src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java +++ b/src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java @@ -157,7 +157,7 @@ void shouldCorrectlyConstructJsonArrpopNoIndex() { } @Test - void shouldCorrectlyConstructJsonArrpopRootPath() { + void shouldCorrectlyConstructJsonArrpopRootPathNoIndex() { Command> 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> 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); diff --git a/src/test/java/io/lettuce/core/json/RedisJsonIntegrationTests.java b/src/test/java/io/lettuce/core/json/RedisJsonIntegrationTests.java index 116b127712..225dd749f7 100644 --- a/src/test/java/io/lettuce/core/json/RedisJsonIntegrationTests.java +++ b/src/test/java/io/lettuce/core/json/RedisJsonIntegrationTests.java @@ -163,8 +163,16 @@ public void jsonArrpopEmptyArray() { redis.jsonSet("myKey", JsonPath.ROOT_PATH, value); List 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 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") From acb2514523710cb61e53b851cddf535f093503e0 Mon Sep 17 00:00:00 2001 From: Thach Le Date: Mon, 17 Mar 2025 15:42:12 +0700 Subject: [PATCH 2/3] Add the root path even index = -1 --- .../io/lettuce/core/RedisJsonCommandBuilder.java | 16 ++++++---------- .../core/RedisJsonCommandBuilderUnitTests.java | 2 +- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java b/src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java index 2d0bf2e9e7..0940bc0844 100644 --- a/src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java +++ b/src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java @@ -107,16 +107,12 @@ Command> jsonArrpop(K key, JsonPath jsonPath, int index) { CommandArgs args = new CommandArgs<>(codec).addKey(key); if (jsonPath != null) { - if (!jsonPath.isRootPath()) { - args.add(jsonPath.toString()); - if (index != -1) { - args.add(index); - } - } else { - if (index != -1) { - args.add(jsonPath.toString()); - args.add(index); - } + // OPTIONAL as per API + args.add(jsonPath.toString()); + + if (index != -1) { + // OPTIONAL as per API + args.add(index); } } diff --git a/src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java b/src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java index 28df5f01a9..1cecbfb30f 100644 --- a/src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java +++ b/src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java @@ -163,7 +163,7 @@ void shouldCorrectlyConstructJsonArrpopRootPathNoIndex() { command.encode(buf); assertThat(buf.toString(StandardCharsets.UTF_8)) - .isEqualTo("*2\r\n" + "$11\r\n" + "JSON.ARRPOP\r\n" + "$15\r\n" + "bikes:inventory\r\n"); + .isEqualTo("*3\r\n" + "$11\r\n" + "JSON.ARRPOP\r\n" + "$15\r\n" + "bikes:inventory\r\n" + "$1\r\n" + "$\r\n"); } @Test From b31082e69bd57d87879b23fbcfb1a1cdfb5c7c1a Mon Sep 17 00:00:00 2001 From: Thach Le Date: Mon, 17 Mar 2025 16:30:06 +0700 Subject: [PATCH 3/3] Only add root path if index != -1 --- src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java | 7 ++++--- .../io/lettuce/core/RedisJsonCommandBuilderUnitTests.java | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java b/src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java index 0940bc0844..42e8e40cfd 100644 --- a/src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java +++ b/src/main/java/io/lettuce/core/RedisJsonCommandBuilder.java @@ -107,12 +107,13 @@ Command> jsonArrpop(K key, JsonPath jsonPath, int index) { CommandArgs args = new CommandArgs<>(codec).addKey(key); if (jsonPath != null) { - // OPTIONAL as per API - args.add(jsonPath.toString()); - 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()); } } diff --git a/src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java b/src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java index 1cecbfb30f..28df5f01a9 100644 --- a/src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java +++ b/src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java @@ -163,7 +163,7 @@ void shouldCorrectlyConstructJsonArrpopRootPathNoIndex() { command.encode(buf); assertThat(buf.toString(StandardCharsets.UTF_8)) - .isEqualTo("*3\r\n" + "$11\r\n" + "JSON.ARRPOP\r\n" + "$15\r\n" + "bikes:inventory\r\n" + "$1\r\n" + "$\r\n"); + .isEqualTo("*2\r\n" + "$11\r\n" + "JSON.ARRPOP\r\n" + "$15\r\n" + "bikes:inventory\r\n"); } @Test