From 2034299a751a79796bc509ffdf9db4c4c8000dae Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Tue, 11 Feb 2025 16:08:40 +0100 Subject: [PATCH 1/4] lifecycle response fix --- .../co/elastic/clients/json/JsonpUtils.java | 9 ++++++ .../json/jackson/JacksonJsonpParser.java | 18 +++++++++++- .../elasticsearch/model/VariantsTest.java | 28 +++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java b/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java index 97928a5ab..e2adb0bef 100644 --- a/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java +++ b/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java @@ -245,6 +245,15 @@ public static Map.Entry lookAheadFieldValue( JsonObject object = parser.getObject(); String result = object.getString(name, null); + if (result == null) { + // checking if instead of a string it's a boolean + try{ + result = String.valueOf(object.getBoolean(name)); + } catch (NullPointerException e) { + // suppressed in favor of JsonpMappingException below + } + } + if (result == null) { result = defaultValue; } diff --git a/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java b/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java index 4a7676ebb..5883588de 100644 --- a/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java +++ b/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java @@ -336,7 +336,23 @@ public Map.Entry lookAheadFieldValue(String name, String def if (fieldName.equals(name)) { // Found tb.copyCurrentEvent(parser); - expectNextEvent(JsonToken.VALUE_STRING); + try { + expectNextEvent(JsonToken.VALUE_STRING); + } + catch (UnexpectedJsonEventException e) { + // checking if instead of a string it's a boolean + String details = e.getMessage(); + if (details.contains("VALUE_TRUE")){ + expectEvent(JsonToken.VALUE_TRUE); + } + else if (details.contains("VALUE_FALSE")){ + expectEvent(JsonToken.VALUE_FALSE); + } + // not a boolean either, can throw exception + else{ + throw e; + } + } tb.copyCurrentEvent(parser); return new AbstractMap.SimpleImmutableEntry<>( diff --git a/java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java b/java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java index f432c3c7e..8ff445d8f 100644 --- a/java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java +++ b/java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java @@ -26,11 +26,15 @@ import co.elastic.clients.elasticsearch._types.query_dsl.Query; import co.elastic.clients.elasticsearch._types.query_dsl.QueryBuilders; import co.elastic.clients.elasticsearch.core.SearchRequest; +import co.elastic.clients.elasticsearch.ilm.ExplainLifecycleResponse; +import co.elastic.clients.elasticsearch.ilm.explain_lifecycle.LifecycleExplainManaged; +import co.elastic.clients.elasticsearch.ilm.explain_lifecycle.LifecycleExplainUnmanaged; import co.elastic.clients.elasticsearch.indices.GetMappingResponse; import co.elastic.clients.json.JsonData; import co.elastic.clients.testkit.ModelTestCase; import org.junit.jupiter.api.Test; +import java.io.StringReader; import java.util.function.Consumer; public class VariantsTest extends ModelTestCase { @@ -281,4 +285,28 @@ public void testContainerWithOptionalVariants() { assertEquals(2.0, fsq2.functionScore().functions().get(0).linear().untyped().placement().decay(), 0.001); } } + + @Test + public void testBooleanVariantTag() { + + String jsonT = "{\"indices\":{\"test\":{\"index\":\"test\",\"managed\":true,\"policy\":\"my_policy\",\"index_creation_date_millis\":1736785235558,\"time_since_index_creation\":\"27.75d\",\"lifecycle_date_millis\":1736785235558,\"age\":\"27.75d\",\"phase\":\"warm\",\"phase_time_millis\":1739183166898,\"action\":\"migrate\",\"action_time_millis\":1739183166898,\"step\":\"check-migration\",\"step_time_millis\":1739183166898,\"step_info\":{\"message\":\"Waiting for all shard copies to be active\",\"shards_left_to_allocate\":-1,\"all_shards_active\":false,\"number_of_replicas\":1},\"phase_execution\":{\"policy\":\"my_policy\",\"phase_definition\":{\"min_age\":\"10d\",\"actions\":{\"forcemerge\":{\"max_num_segments\":1}}},\"version\":1,\"modified_date_in_millis\":1739183005443}}}}"; + + ExplainLifecycleResponse respT = fromJson(jsonT,ExplainLifecycleResponse.class); + + // if managed is "true" then the variant class must be Managed + assertTrue(respT.indices().get("test").isTrue()); + assertTrue(respT.indices().get("test")._get().getClass().equals(LifecycleExplainManaged.class)); + + String jsonF = "{\"indices\":{\"test\":{\"index\":\"test\",\"managed\":false}}}"; + + ExplainLifecycleResponse respF = fromJson(jsonF,ExplainLifecycleResponse.class); + + // if managed is "false" then the variant class must be Unmanaged + assertTrue(respF.indices().get("test").isFalse()); + assertTrue(respF.indices().get("test")._get().getClass().equals(LifecycleExplainUnmanaged.class)); + + // roundtrip isn't the same + // ExplainLifecycleResponse respT2 = checkJsonRoundtrip(respT, jsonT); + // ExplainLifecycleResponse respF2 = checkJsonRoundtrip(respF, jsonF); + } } From f997002e0900a5a54ee8ef3bc63a8e0ad78f30bb Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Tue, 11 Feb 2025 16:23:39 +0100 Subject: [PATCH 2/4] checkstyle --- .../clients/elasticsearch/model/VariantsTest.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java b/java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java index 8ff445d8f..62be390c5 100644 --- a/java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java +++ b/java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java @@ -34,7 +34,6 @@ import co.elastic.clients.testkit.ModelTestCase; import org.junit.jupiter.api.Test; -import java.io.StringReader; import java.util.function.Consumer; public class VariantsTest extends ModelTestCase { @@ -289,7 +288,16 @@ public void testContainerWithOptionalVariants() { @Test public void testBooleanVariantTag() { - String jsonT = "{\"indices\":{\"test\":{\"index\":\"test\",\"managed\":true,\"policy\":\"my_policy\",\"index_creation_date_millis\":1736785235558,\"time_since_index_creation\":\"27.75d\",\"lifecycle_date_millis\":1736785235558,\"age\":\"27.75d\",\"phase\":\"warm\",\"phase_time_millis\":1739183166898,\"action\":\"migrate\",\"action_time_millis\":1739183166898,\"step\":\"check-migration\",\"step_time_millis\":1739183166898,\"step_info\":{\"message\":\"Waiting for all shard copies to be active\",\"shards_left_to_allocate\":-1,\"all_shards_active\":false,\"number_of_replicas\":1},\"phase_execution\":{\"policy\":\"my_policy\",\"phase_definition\":{\"min_age\":\"10d\",\"actions\":{\"forcemerge\":{\"max_num_segments\":1}}},\"version\":1,\"modified_date_in_millis\":1739183005443}}}}"; + String jsonT = "{\"indices\":{\"test\":{\"index\":\"test\",\"managed\":true,\"policy\":\"my_policy\"," + + "\"index_creation_date_millis\":1736785235558,\"time_since_index_creation\":\"27.75d\"," + + "\"lifecycle_date_millis\":1736785235558,\"age\":\"27.75d\",\"phase\":\"warm\"," + + "\"phase_time_millis\":1739183166898,\"action\":\"migrate\",\"action_time_millis\":1739183166898," + + "\"step\":\"check-migration\",\"step_time_millis\":1739183166898," + + "\"step_info\":{\"message\":\"Waiting for all shard copies to be active\"," + + "\"shards_left_to_allocate\":-1,\"all_shards_active\":false,\"number_of_replicas\":1}," + + "\"phase_execution\":{\"policy\":\"my_policy\",\"phase_definition\":{\"min_age\":\"10d\"," + + "\"actions\":{\"forcemerge\":{\"max_num_segments\":1}}},\"version\":1," + + "\"modified_date_in_millis\":1739183005443}}}}"; ExplainLifecycleResponse respT = fromJson(jsonT,ExplainLifecycleResponse.class); From 661c2ed6ce43b86cf47549dc865c10a292566a53 Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Tue, 11 Feb 2025 16:34:59 +0100 Subject: [PATCH 3/4] update test --- .../elasticsearch/model/VariantsTest.java | 61 ++++++++++++++----- 1 file changed, 46 insertions(+), 15 deletions(-) diff --git a/java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java b/java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java index 62be390c5..1ffbe6ade 100644 --- a/java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java +++ b/java-client/src/test/java/co/elastic/clients/elasticsearch/model/VariantsTest.java @@ -288,16 +288,44 @@ public void testContainerWithOptionalVariants() { @Test public void testBooleanVariantTag() { - String jsonT = "{\"indices\":{\"test\":{\"index\":\"test\",\"managed\":true,\"policy\":\"my_policy\"," + - "\"index_creation_date_millis\":1736785235558,\"time_since_index_creation\":\"27.75d\"," + - "\"lifecycle_date_millis\":1736785235558,\"age\":\"27.75d\",\"phase\":\"warm\"," + - "\"phase_time_millis\":1739183166898,\"action\":\"migrate\",\"action_time_millis\":1739183166898," + - "\"step\":\"check-migration\",\"step_time_millis\":1739183166898," + - "\"step_info\":{\"message\":\"Waiting for all shard copies to be active\"," + - "\"shards_left_to_allocate\":-1,\"all_shards_active\":false,\"number_of_replicas\":1}," + - "\"phase_execution\":{\"policy\":\"my_policy\",\"phase_definition\":{\"min_age\":\"10d\"," + - "\"actions\":{\"forcemerge\":{\"max_num_segments\":1}}},\"version\":1," + - "\"modified_date_in_millis\":1739183005443}}}}"; + String jsonT = "{\n" + + " \"indices\": {\n" + + " \"test\": {\n" + + " \"index\": \"test\",\n" + + " \"managed\": true,\n" + + " \"policy\": \"my_policy\",\n" + + " \"index_creation_date_millis\": 1736785235558,\n" + + " \"time_since_index_creation\": \"27.75d\",\n" + + " \"lifecycle_date_millis\": 1736785235558,\n" + + " \"age\": \"27.75d\",\n" + + " \"phase\": \"warm\",\n" + + " \"phase_time_millis\": 1739183166898,\n" + + " \"action\": \"migrate\",\n" + + " \"action_time_millis\": 1739183166898,\n" + + " \"step\": \"check-migration\",\n" + + " \"step_time_millis\": 1739183166898,\n" + + " \"step_info\": {\n" + + " \"message\": \"Waiting for all shard copies to be active\",\n" + + " \"shards_left_to_allocate\": -1,\n" + + " \"all_shards_active\": false,\n" + + " \"number_of_replicas\": 1\n" + + " },\n" + + " \"phase_execution\": {\n" + + " \"policy\": \"my_policy\",\n" + + " \"phase_definition\": {\n" + + " \"min_age\": \"10d\",\n" + + " \"actions\": {\n" + + " \"forcemerge\": {\n" + + " \"max_num_segments\": 1\n" + + " }\n" + + " }\n" + + " },\n" + + " \"version\": 1,\n" + + " \"modified_date_in_millis\": 1739183005443\n" + + " }\n" + + " }\n" + + " }\n" + + "}"; ExplainLifecycleResponse respT = fromJson(jsonT,ExplainLifecycleResponse.class); @@ -305,16 +333,19 @@ public void testBooleanVariantTag() { assertTrue(respT.indices().get("test").isTrue()); assertTrue(respT.indices().get("test")._get().getClass().equals(LifecycleExplainManaged.class)); - String jsonF = "{\"indices\":{\"test\":{\"index\":\"test\",\"managed\":false}}}"; + String jsonF = "{\n" + + " \"indices\": {\n" + + " \"test\": {\n" + + " \"index\": \"test\",\n" + + " \"managed\": false\n" + + " }\n" + + " }\n" + + "}"; ExplainLifecycleResponse respF = fromJson(jsonF,ExplainLifecycleResponse.class); // if managed is "false" then the variant class must be Unmanaged assertTrue(respF.indices().get("test").isFalse()); assertTrue(respF.indices().get("test")._get().getClass().equals(LifecycleExplainUnmanaged.class)); - - // roundtrip isn't the same - // ExplainLifecycleResponse respT2 = checkJsonRoundtrip(respT, jsonT); - // ExplainLifecycleResponse respF2 = checkJsonRoundtrip(respF, jsonF); } } From 979edf77511bf6bc92b5e9a3242a8bae64bcba0e Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Mon, 17 Feb 2025 15:09:09 +0100 Subject: [PATCH 4/4] cleaner code --- .../co/elastic/clients/json/JsonpUtils.java | 21 ++++++++---- .../json/jackson/JacksonJsonpParser.java | 34 +++++++++---------- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java b/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java index e2adb0bef..74f888d04 100644 --- a/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java +++ b/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java @@ -243,14 +243,17 @@ public static Map.Entry lookAheadFieldValue( } else { // Unbuffered path: parse the object into a JsonObject, then extract the value and parse it again JsonObject object = parser.getObject(); - String result = object.getString(name, null); - if (result == null) { - // checking if instead of a string it's a boolean - try{ - result = String.valueOf(object.getBoolean(name)); - } catch (NullPointerException e) { - // suppressed in favor of JsonpMappingException below + String result = null; + JsonValue value = object.get(name); + // Handle enums and booleans promoted to enums + if (value != null) { + if (value.getValueType() == JsonValue.ValueType.STRING) { + result = ((JsonString) value).getString(); + } else if (value.getValueType() == JsonValue.ValueType.TRUE) { + result = "true"; + } else if (value.getValueType() == JsonValue.ValueType.FALSE) { + result = "false"; } } @@ -258,6 +261,10 @@ public static Map.Entry lookAheadFieldValue( result = defaultValue; } + if (result == null) { + result = defaultValue; + } + if (result == null) { throw new JsonpMappingException("Property '" + name + "' not found", location); } diff --git a/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java b/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java index 5883588de..b1f526725 100644 --- a/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java +++ b/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java @@ -336,27 +336,27 @@ public Map.Entry lookAheadFieldValue(String name, String def if (fieldName.equals(name)) { // Found tb.copyCurrentEvent(parser); - try { - expectNextEvent(JsonToken.VALUE_STRING); - } - catch (UnexpectedJsonEventException e) { - // checking if instead of a string it's a boolean - String details = e.getMessage(); - if (details.contains("VALUE_TRUE")){ - expectEvent(JsonToken.VALUE_TRUE); - } - else if (details.contains("VALUE_FALSE")){ - expectEvent(JsonToken.VALUE_FALSE); - } - // not a boolean either, can throw exception - else{ - throw e; - } + + String result = null; + switch (parser.nextToken()) { + case VALUE_STRING: + result = parser.getText(); + break; + // Handle booleans promoted to enums + case VALUE_TRUE: + result = "true"; + break; + case VALUE_FALSE: + result = "false"; + break; + default: + expectEvent(JsonToken.VALUE_STRING); } + tb.copyCurrentEvent(parser); return new AbstractMap.SimpleImmutableEntry<>( - parser.getText(), + result, new JacksonJsonpParser( JsonParserSequence.createFlattened(false, tb.asParser(), parser), mapper