From 64ff4efbc912f92c7582f1d8b2fe4b450c60ca44 Mon Sep 17 00:00:00 2001 From: Tihomir Mateev Date: Mon, 10 Mar 2025 15:28:45 +0200 Subject: [PATCH 1/5] Handle Null values better when working with JSON --- .../io/lettuce/core/json/DefaultJsonParser.java | 9 +++++++++ .../lettuce/core/json/UnproccessedJsonValue.java | 5 +++++ .../lettuce/core/output/JsonValueListOutput.java | 15 ++++++++++++--- .../core/json/RedisJsonIntegrationTests.java | 10 ++++++++++ 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/lettuce/core/json/DefaultJsonParser.java b/src/main/java/io/lettuce/core/json/DefaultJsonParser.java index afab7afa70..dd20fa41ca 100644 --- a/src/main/java/io/lettuce/core/json/DefaultJsonParser.java +++ b/src/main/java/io/lettuce/core/json/DefaultJsonParser.java @@ -10,6 +10,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.NullNode; import java.io.IOException; import java.nio.ByteBuffer; @@ -61,6 +62,10 @@ public JsonValue fromObject(Object object) { } private JsonValue parse(String value) { + if (value == null) { + return DelegateJsonValue.wrap(NullNode.getInstance()); + } + ObjectMapper mapper = new ObjectMapper(); try { JsonNode root = mapper.readTree(value); @@ -72,6 +77,10 @@ private JsonValue parse(String value) { } private JsonValue parse(ByteBuffer byteBuffer) { + if (byteBuffer == null) { + return DelegateJsonValue.wrap(NullNode.getInstance()); + } + ObjectMapper mapper = new ObjectMapper(); try { byte[] bytes = new byte[byteBuffer.remaining()]; diff --git a/src/main/java/io/lettuce/core/json/UnproccessedJsonValue.java b/src/main/java/io/lettuce/core/json/UnproccessedJsonValue.java index 0a00167503..18f662aeb3 100644 --- a/src/main/java/io/lettuce/core/json/UnproccessedJsonValue.java +++ b/src/main/java/io/lettuce/core/json/UnproccessedJsonValue.java @@ -45,6 +45,11 @@ public UnproccessedJsonValue(ByteBuffer bytes, JsonParser theParser) { @Override public String toString() { + + if (unprocessedData == null) { + return null; + } + if (isDeserialized()) { return jsonValue.toString(); } diff --git a/src/main/java/io/lettuce/core/output/JsonValueListOutput.java b/src/main/java/io/lettuce/core/output/JsonValueListOutput.java index 389b696624..568ba08bcb 100644 --- a/src/main/java/io/lettuce/core/output/JsonValueListOutput.java +++ b/src/main/java/io/lettuce/core/output/JsonValueListOutput.java @@ -39,9 +39,18 @@ public void set(ByteBuffer bytes) { multi(1); } - ByteBuffer fetched = ByteBuffer.allocate(bytes.remaining()); - fetched.put(bytes); - fetched.flip(); + // if (bytes != null) { + // output.add(parser.createJsonValue(bytes)); + // return; + // } + + ByteBuffer fetched = null; + if (bytes != null) { + fetched = ByteBuffer.allocate(bytes.remaining()); + fetched.put(bytes); + fetched.flip(); + } + output.add(parser.loadJsonValue(fetched)); } diff --git a/src/test/java/io/lettuce/core/json/RedisJsonIntegrationTests.java b/src/test/java/io/lettuce/core/json/RedisJsonIntegrationTests.java index e471050c47..116b127712 100644 --- a/src/test/java/io/lettuce/core/json/RedisJsonIntegrationTests.java +++ b/src/test/java/io/lettuce/core/json/RedisJsonIntegrationTests.java @@ -157,6 +157,16 @@ void jsonArrpop(String path) { "{\"id\":\"bike:3\",\"model\":\"Weywot\",\"description\":\"This bike gives kids aged six years and old"); } + @Test + public void jsonArrpopEmptyArray() { + JsonValue value = redis.getJsonParser().createJsonValue("[\"one\"]"); + 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(); + } + @ParameterizedTest(name = "With {0} as path") @ValueSource(strings = { BIKE_COLORS_V1, BIKE_COLORS_V2 }) void jsonArrtrim(String path) { From 6dc0ee6894df140519f8f845f41e1dc9f1d22c1b Mon Sep 17 00:00:00 2001 From: Tihomir Mateev Date: Mon, 10 Mar 2025 15:32:57 +0200 Subject: [PATCH 2/5] Incomplete submit after some refactoring --- .../java/io/lettuce/core/json/UnproccessedJsonValue.java | 4 +++- .../java/io/lettuce/core/output/JsonValueListOutput.java | 5 ----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/lettuce/core/json/UnproccessedJsonValue.java b/src/main/java/io/lettuce/core/json/UnproccessedJsonValue.java index 18f662aeb3..92e98418a6 100644 --- a/src/main/java/io/lettuce/core/json/UnproccessedJsonValue.java +++ b/src/main/java/io/lettuce/core/json/UnproccessedJsonValue.java @@ -166,7 +166,9 @@ private void lazilyDeserialize() { try { if (!isDeserialized()) { jsonValue = parser.createJsonValue(unprocessedData); - unprocessedData.clear(); + if (unprocessedData != null) { + unprocessedData.clear(); + } } } finally { lock.unlock(); diff --git a/src/main/java/io/lettuce/core/output/JsonValueListOutput.java b/src/main/java/io/lettuce/core/output/JsonValueListOutput.java index 568ba08bcb..e415f46a72 100644 --- a/src/main/java/io/lettuce/core/output/JsonValueListOutput.java +++ b/src/main/java/io/lettuce/core/output/JsonValueListOutput.java @@ -39,11 +39,6 @@ public void set(ByteBuffer bytes) { multi(1); } - // if (bytes != null) { - // output.add(parser.createJsonValue(bytes)); - // return; - // } - ByteBuffer fetched = null; if (bytes != null) { fetched = ByteBuffer.allocate(bytes.remaining()); From 4a984d89c149faa1d12811a9d86a3ca953d04757 Mon Sep 17 00:00:00 2001 From: Tihomir Mateev Date: Mon, 10 Mar 2025 15:56:40 +0200 Subject: [PATCH 3/5] Sanitize the CI output when running on CI --- .github/workflows/integration.yml | 4 ++-- Makefile | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 289a2b5689..78e172f55e 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -49,7 +49,7 @@ jobs: - name: Set up Docker Compose environment run: | mkdir -m 777 $REDIS_ENV_WORK_DIR - make start version=${{ matrix.redis_version }} + DOCKER_COMPOSE_ARGS="--progress plain" make start version=${{ matrix.redis_version }} - name: Maven offline run: | mvn -q dependency:go-offline @@ -65,7 +65,7 @@ jobs: TERM: dumb - name: Tear down Docker Compose environment run: | - docker compose $COMPOSE_ENV_FILES -f src/test/resources/docker-env/docker-compose.yml down + DOCKER_COMPOSE_ARGS="--progress plain" make stop - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v4 with: diff --git a/Makefile b/Makefile index 6df7a01fe2..270f709d5c 100644 --- a/Makefile +++ b/Makefile @@ -27,7 +27,7 @@ start: echo "Environment work directory: $(REDIS_ENV_WORK_DIR)"; \ rm -rf "$(REDIS_ENV_WORK_DIR)"; \ mkdir -p "$(REDIS_ENV_WORK_DIR)"; \ - docker compose $$env_files -f src/test/resources/docker-env/docker-compose.yml up -d; \ + docker compose $(DOCKER_COMPOSE_ARGS) $$env_files -f src/test/resources/docker-env/docker-compose.yml up -d; \ echo "Started test environment with Redis version $$version." @@ -38,7 +38,7 @@ test-coverage: mvn -DskipITs=false clean compile verify jacoco:report -P$(PROFILE) stop: - docker compose --env-file src/test/resources/docker-env/.env -f src/test/resources/docker-env/docker-compose.yml down; \ + docker compose $(DOCKER_COMPOSE_ARGS) --env-file src/test/resources/docker-env/.env -f src/test/resources/docker-env/docker-compose.yml down; \ rm -rf "$(REDIS_ENV_WORK_DIR)" clean: From 2183cdd70b133b526f19e63ec0066fa12c1812df Mon Sep 17 00:00:00 2001 From: Tihomir Mateev Date: Mon, 10 Mar 2025 16:01:35 +0200 Subject: [PATCH 4/5] Plishing --- .github/workflows/integration.yml | 4 ++-- Makefile | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 78e172f55e..7b3a020202 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -49,7 +49,7 @@ jobs: - name: Set up Docker Compose environment run: | mkdir -m 777 $REDIS_ENV_WORK_DIR - DOCKER_COMPOSE_ARGS="--progress plain" make start version=${{ matrix.redis_version }} + DOCKER_COMPOSE_ARGS="--progress quiet" make start version=${{ matrix.redis_version }} - name: Maven offline run: | mvn -q dependency:go-offline @@ -65,7 +65,7 @@ jobs: TERM: dumb - name: Tear down Docker Compose environment run: | - DOCKER_COMPOSE_ARGS="--progress plain" make stop + DOCKER_COMPOSE_ARGS="--progress quiet" make stop - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v4 with: diff --git a/Makefile b/Makefile index 270f709d5c..dddb7cdf13 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ test-coverage: mvn -DskipITs=false clean compile verify jacoco:report -P$(PROFILE) stop: - docker compose $(DOCKER_COMPOSE_ARGS) --env-file src/test/resources/docker-env/.env -f src/test/resources/docker-env/docker-compose.yml down; \ + docker compose $(DOCKER_COMPOSE_ARGS) $$env_files -f src/test/resources/docker-env/docker-compose.yml down; \ rm -rf "$(REDIS_ENV_WORK_DIR)" clean: From 7c638cbddbffc4752d435ef786d7b52cec5e284b Mon Sep 17 00:00:00 2001 From: Tihomir Mateev Date: Mon, 10 Mar 2025 16:05:44 +0200 Subject: [PATCH 5/5] Never submit irrelevant changes in your PR --- .github/workflows/integration.yml | 4 ++-- Makefile | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 7b3a020202..289a2b5689 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -49,7 +49,7 @@ jobs: - name: Set up Docker Compose environment run: | mkdir -m 777 $REDIS_ENV_WORK_DIR - DOCKER_COMPOSE_ARGS="--progress quiet" make start version=${{ matrix.redis_version }} + make start version=${{ matrix.redis_version }} - name: Maven offline run: | mvn -q dependency:go-offline @@ -65,7 +65,7 @@ jobs: TERM: dumb - name: Tear down Docker Compose environment run: | - DOCKER_COMPOSE_ARGS="--progress quiet" make stop + docker compose $COMPOSE_ENV_FILES -f src/test/resources/docker-env/docker-compose.yml down - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v4 with: diff --git a/Makefile b/Makefile index dddb7cdf13..6df7a01fe2 100644 --- a/Makefile +++ b/Makefile @@ -27,7 +27,7 @@ start: echo "Environment work directory: $(REDIS_ENV_WORK_DIR)"; \ rm -rf "$(REDIS_ENV_WORK_DIR)"; \ mkdir -p "$(REDIS_ENV_WORK_DIR)"; \ - docker compose $(DOCKER_COMPOSE_ARGS) $$env_files -f src/test/resources/docker-env/docker-compose.yml up -d; \ + docker compose $$env_files -f src/test/resources/docker-env/docker-compose.yml up -d; \ echo "Started test environment with Redis version $$version." @@ -38,7 +38,7 @@ test-coverage: mvn -DskipITs=false clean compile verify jacoco:report -P$(PROFILE) stop: - docker compose $(DOCKER_COMPOSE_ARGS) $$env_files -f src/test/resources/docker-env/docker-compose.yml down; \ + docker compose --env-file src/test/resources/docker-env/.env -f src/test/resources/docker-env/docker-compose.yml down; \ rm -rf "$(REDIS_ENV_WORK_DIR)" clean: