diff --git a/CHANGELOG.md b/CHANGELOG.md index d94963b43b..a2176a3eae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,8 @@ This section is for maintaining a changelog for all breaking changes for the cli ### Fixed - Fix partial success results for msearch_template ([#709](https://github.com/opensearch-project/opensearch-java/pull/709)) +- Fix deserialization of failures in GetTasksResponse ([#727](https://github.com/opensearch-project/opensearch-java/pull/727)) + ### Security diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/_types/TaskFailure.java b/java-client/src/main/java/org/opensearch/client/opensearch/_types/TaskFailure.java new file mode 100644 index 0000000000..59358636c6 --- /dev/null +++ b/java-client/src/main/java/org/opensearch/client/opensearch/_types/TaskFailure.java @@ -0,0 +1,229 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.client.opensearch._types; + +// typedef: _types.TaskFailure + +import jakarta.json.stream.JsonGenerator; +import java.util.function.Function; +import javax.annotation.Nullable; +import org.opensearch.client.json.*; +import org.opensearch.client.util.ApiTypeHelper; +import org.opensearch.client.util.ObjectBuilder; +import org.opensearch.client.util.ObjectBuilderBase; + +@JsonpDeserializable +public class TaskFailure implements JsonpSerializable { + + private final ErrorCause cause; + + @Nullable + private final String id; + + @Nullable + private final String index; + + @Nullable + private final Integer status; + + @Nullable + private final String type; + + // --------------------------------------------------------------------------------------------- + + private TaskFailure(Builder builder) { + + this.index = builder.index; + this.id = builder.id; + this.cause = ApiTypeHelper.requireNonNull(builder.cause, this, "cause"); + this.status = builder.status; + this.type = builder.type; + + } + + public static TaskFailure of(Function> fn) { + return fn.apply(new Builder()).build(); + } + + /** + * API name: {@code index} + */ + @Nullable + public final String index() { + return this.index; + } + + /** + * API name: {@code id} + */ + @Nullable + public final String id() { + return this.id; + } + + /** + * Required - API name: {@code cause} + */ + public final ErrorCause cause() { + return this.cause; + } + + /** + * API name: {@code status} + */ + @Nullable + public final Integer status() { + return this.status; + } + + /** + * API name: {@code type} + */ + @Nullable + public final String type() { + return this.type; + } + + /** + * Serialize this object to JSON. + */ + public void serialize(JsonGenerator generator, JsonpMapper mapper) { + generator.writeStartObject(); + serializeInternal(generator, mapper); + generator.writeEnd(); + } + + protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { + + if (this.index != null) { + generator.writeKey("index"); + generator.write(this.index); + + } + if (this.id != null) { + generator.writeKey("id"); + generator.write(this.id); + } + + generator.writeKey("cause"); + this.cause.serialize(generator, mapper); + + if (this.status != null) { + generator.writeKey("status"); + generator.write(this.status); + } + if (this.type != null) { + generator.writeKey("type"); + generator.write(this.type); + } + + } + + // --------------------------------------------------------------------------------------------- + + /** + * Builder for {@link TaskFailure}. + */ + + public static class Builder extends ObjectBuilderBase implements ObjectBuilder { + @Nullable + private String index; + + @Nullable + private String id; + + private ErrorCause cause; + + @Nullable + private Integer status; + + @Nullable + private String type; + + /** + * API name: {@code index} + */ + public final Builder index(@Nullable String value) { + this.index = value; + return this; + } + + /** + * API name: {@code id} + */ + public final Builder id(@Nullable String value) { + this.id = value; + return this; + } + + /** + * Required - API name: {@code cause} + */ + public final Builder cause(ErrorCause value) { + this.cause = value; + return this; + } + + /** + * Required - API name: {@code cause} + */ + public final Builder cause(Function> fn) { + return this.cause(fn.apply(new ErrorCause.Builder()).build()); + } + + /** + * API name: {@code status} + */ + public final Builder status(@Nullable Integer value) { + this.status = value; + return this; + } + + /** + * API name: {@code type} + */ + public final Builder type(@Nullable String value) { + this.type = value; + return this; + } + + /** + * Builds a {@link TaskFailure}. + * + * @throws NullPointerException + * if some of the required fields are null. + */ + public TaskFailure build() { + _checkSingleUse(); + + return new TaskFailure(this); + } + } + + // --------------------------------------------------------------------------------------------- + + /** + * Json deserializer for {@link TaskFailure} + */ + public static final JsonpDeserializer _DESERIALIZER = ObjectBuilderDeserializer.lazy( + Builder::new, + TaskFailure::setupTaskFailureDeserializer + ); + + protected static void setupTaskFailureDeserializer(ObjectDeserializer op) { + + op.add(Builder::index, JsonpDeserializer.stringDeserializer(), "index"); + op.add(Builder::id, JsonpDeserializer.stringDeserializer(), "id"); + op.add(Builder::cause, ErrorCause._DESERIALIZER, "cause"); + op.add(Builder::status, JsonpDeserializer.integerDeserializer(), "status"); + op.add(Builder::type, JsonpDeserializer.stringDeserializer(), "type"); + + } + +} diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/tasks/GetTasksResponse.java b/java-client/src/main/java/org/opensearch/client/opensearch/tasks/GetTasksResponse.java index 1e4357eabb..f916212500 100644 --- a/java-client/src/main/java/org/opensearch/client/opensearch/tasks/GetTasksResponse.java +++ b/java-client/src/main/java/org/opensearch/client/opensearch/tasks/GetTasksResponse.java @@ -35,6 +35,7 @@ import jakarta.json.stream.JsonGenerator; import java.util.function.Function; import javax.annotation.Nullable; +import org.opensearch.client.json.JsonData; import org.opensearch.client.json.JsonpDeserializable; import org.opensearch.client.json.JsonpDeserializer; import org.opensearch.client.json.JsonpMapper; @@ -55,7 +56,7 @@ public class GetTasksResponse implements JsonpSerializable { private final Info task; @Nullable - private final Status response; + private final JsonData response; @Nullable private final ErrorCause error; @@ -93,7 +94,7 @@ public final Info task() { * API name: {@code response} */ @Nullable - public final Status response() { + public final JsonData response() { return this.response; } @@ -147,7 +148,7 @@ public static class Builder extends ObjectBuilderBase implements ObjectBuilder> fn) { /** * API name: {@code response} */ - public final Builder response(@Nullable Status value) { + public final Builder response(@Nullable JsonData value) { this.response = value; return this; } - /** - * API name: {@code response} - */ - public final Builder response(Function> fn) { - return this.response(fn.apply(new Status.Builder()).build()); - } - /** * API name: {@code error} */ @@ -232,7 +226,7 @@ protected static void setupGetTasksResponseDeserializer(ObjectDeserializer> fn) { - return this.status(fn.apply(new Status.Builder()).build()); - } - /** * Required - API name: {@code type} */ @@ -472,7 +466,7 @@ protected static void setupInfoDeserializer(ObjectDeserializer op) op.add(Builder::node, JsonpDeserializer.stringDeserializer(), "node"); op.add(Builder::runningTimeInNanos, JsonpDeserializer.longDeserializer(), "running_time_in_nanos"); op.add(Builder::startTimeInMillis, JsonpDeserializer.longDeserializer(), "start_time_in_millis"); - op.add(Builder::status, Status._DESERIALIZER, "status"); + op.add(Builder::status, JsonData._DESERIALIZER, "status"); op.add(Builder::type, JsonpDeserializer.stringDeserializer(), "type"); op.add(Builder::parentTaskId, JsonpDeserializer.stringDeserializer(), "parent_task_id"); diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/tasks/Status.java b/java-client/src/main/java/org/opensearch/client/opensearch/tasks/Status.java index 59e5ad85ad..e334ca383b 100644 --- a/java-client/src/main/java/org/opensearch/client/opensearch/tasks/Status.java +++ b/java-client/src/main/java/org/opensearch/client/opensearch/tasks/Status.java @@ -43,6 +43,7 @@ import org.opensearch.client.json.ObjectBuilderDeserializer; import org.opensearch.client.json.ObjectDeserializer; import org.opensearch.client.opensearch._types.Retries; +import org.opensearch.client.opensearch._types.TaskFailure; import org.opensearch.client.opensearch._types.Time; import org.opensearch.client.util.ApiTypeHelper; import org.opensearch.client.util.ObjectBuilder; @@ -63,7 +64,7 @@ public class Status implements JsonpSerializable { private final long noops; - private final List failures; + private final List failures; private final float requestsPerSecond; @@ -158,7 +159,7 @@ public final long noops() { /** * API name: {@code failures} */ - public final List failures() { + public final List failures() { return this.failures; } @@ -274,9 +275,8 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { if (ApiTypeHelper.isDefined(this.failures)) { generator.writeKey("failures"); generator.writeStartArray(); - for (String item0 : this.failures) { - generator.write(item0); - + for (TaskFailure item0 : this.failures) { + item0.serialize(generator, mapper); } generator.writeEnd(); @@ -343,7 +343,7 @@ public static class Builder extends ObjectBuilderBase implements ObjectBuilder failures; + private List failures; private Float requestsPerSecond; @@ -416,7 +416,7 @@ public final Builder noops(long value) { *

* Adds all elements of list to failures. */ - public final Builder failures(List list) { + public final Builder failures(List list) { this.failures = _listAddAll(this.failures, list); return this; } @@ -426,7 +426,7 @@ public final Builder failures(List list) { *

* Adds one or more values to failures. */ - public final Builder failures(String value, String... values) { + public final Builder failures(TaskFailure value, TaskFailure... values) { this.failures = _listAdd(this.failures, value, values); return this; } @@ -570,7 +570,7 @@ protected static void setupStatusDeserializer(ObjectDeserializer op.add(Builder::created, JsonpDeserializer.longDeserializer(), "created"); op.add(Builder::deleted, JsonpDeserializer.longDeserializer(), "deleted"); op.add(Builder::noops, JsonpDeserializer.longDeserializer(), "noops"); - op.add(Builder::failures, JsonpDeserializer.arrayDeserializer(JsonpDeserializer.stringDeserializer()), "failures"); + op.add(Builder::failures, JsonpDeserializer.arrayDeserializer(TaskFailure._DESERIALIZER), "failures"); op.add(Builder::requestsPerSecond, JsonpDeserializer.floatDeserializer(), "requests_per_second"); op.add(Builder::retries, Retries._DESERIALIZER, "retries"); op.add(Builder::throttled, Time._DESERIALIZER, "throttled"); diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/core/GetTasksResponseTest.java b/java-client/src/test/java/org/opensearch/client/opensearch/core/GetTasksResponseTest.java new file mode 100644 index 0000000000..ab81a9ee8e --- /dev/null +++ b/java-client/src/test/java/org/opensearch/client/opensearch/core/GetTasksResponseTest.java @@ -0,0 +1,175 @@ +package org.opensearch.client.opensearch.core; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.StringReader; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.junit.Assert; +import org.junit.Test; +import org.opensearch.client.json.JsonpMapper; +import org.opensearch.client.json.jsonb.JsonbJsonpMapper; +import org.opensearch.client.opensearch.tasks.GetTasksResponse; +import org.opensearch.client.opensearch.tasks.Status; + +public class GetTasksResponseTest extends Assert { + + @Test + public void test() throws JsonProcessingException { + + final JsonpMapper mapper = new JsonbJsonpMapper(); + final String taskResponse = new ObjectMapper().writeValueAsString(createJsonMap()); + final var parser = mapper.jsonProvider().createParser(new StringReader(taskResponse)); + + final GetTasksResponse tasksResponse = GetTasksResponse._DESERIALIZER.deserialize(parser, mapper); + + // Deserialize the JsonData to a typed Status response + final Status response = tasksResponse.response().to(Status.class); + final Status taskStatus = tasksResponse.task().status().to(Status.class); + + assertTrue(tasksResponse.completed()); + + // Assertions for "task" field + assertEquals("test-node-id", tasksResponse.task().node()); + assertEquals(298365, tasksResponse.task().id()); + assertEquals("transport", tasksResponse.task().type()); + assertEquals("indices:data/write/delete/byquery", tasksResponse.task().action()); + assertEquals(1, taskStatus.total()); + assertEquals(0, taskStatus.updated()); + assertEquals(0, taskStatus.created()); + assertEquals(0, taskStatus.deleted()); + assertEquals(1, taskStatus.batches()); + assertEquals(1, taskStatus.noops()); + assertEquals(1, taskStatus.versionConflicts()); + assertEquals(0, taskStatus.retries().bulk()); + assertEquals(0, taskStatus.retries().search()); + assertEquals(0, taskStatus.throttledMillis()); + assertEquals(-1f, taskStatus.requestsPerSecond(), 0.01); + assertEquals(0, taskStatus.throttledUntilMillis()); + assertEquals("test-description", tasksResponse.task().description()); + assertEquals(1698853787531L, tasksResponse.task().startTimeInMillis()); + assertEquals(13688917, tasksResponse.task().runningTimeInNanos()); + assertTrue(tasksResponse.task().cancellable()); + assertEquals(new HashMap<>(), tasksResponse.task().headers()); + + // Assertions for "response" field + assertEquals(13L, (long) response.took()); + assertFalse(response.timedOut()); + assertEquals(1, response.total()); + assertEquals(0, response.updated()); + assertEquals(0, response.created()); + assertEquals(0, response.deleted()); + assertEquals(1, response.batches()); + assertEquals(0, response.noops()); + assertEquals(1, response.versionConflicts()); + assertEquals(0, response.retries().bulk()); + assertEquals(0, response.retries().search()); + assertEquals("0s", response.throttled()._toJsonString()); + assertEquals(0, response.throttledMillis()); + assertEquals(-1f, response.requestsPerSecond(), 0.01); + assertEquals("0s", response.throttledUntil()._toJsonString()); + assertEquals(0, response.throttledUntilMillis()); + + // Assertions for "failures" field within "response" + final var failures = response.failures(); + assertNotNull(failures); + assertEquals(1, failures.size()); + + final var failure = failures.get(0); + assertEquals("test-index", failure.index()); + assertEquals("test-failure-id", failure.id()); + + // Assertions for "cause" field within "failures" + final var cause = failure.cause(); + assertNotNull(cause); + assertEquals("version_conflict_engine_exception", cause.type()); + assertEquals("version conflict", cause.reason()); + assertEquals(409, (int) failure.status()); + assertEquals("_doc", failure.type()); + } + + private static Map createJsonMap() { + Map resultMap = new HashMap<>(); + + resultMap.put("completed", true); + + Map taskMap = new HashMap<>(); + taskMap.put("node", "test-node-id"); + taskMap.put("id", 298365); + taskMap.put("type", "transport"); + taskMap.put("action", "indices:data/write/delete/byquery"); + + Map statusMap = new HashMap<>(); + statusMap.put("total", 1); + statusMap.put("updated", 0); + statusMap.put("created", 0); + statusMap.put("deleted", 0); + statusMap.put("batches", 1); + statusMap.put("noops", 1); + statusMap.put("version_conflicts", 1); + + Map retriesMap = new HashMap<>(); + retriesMap.put("bulk", 0); + retriesMap.put("search", 0); + statusMap.put("retries", retriesMap); + + statusMap.put("throttled_millis", 0); + statusMap.put("requests_per_second", -1); + statusMap.put("throttled_until_millis", 0); + + taskMap.put("status", statusMap); + + taskMap.put("description", "test-description"); + taskMap.put("start_time_in_millis", 1698853787531L); + taskMap.put("running_time_in_nanos", 13688917); + taskMap.put("cancellable", true); + taskMap.put("headers", new HashMap<>()); + + resultMap.put("task", taskMap); + resultMap.put("response", getResponseMap()); + + return resultMap; + } + + private static Map getResponseMap() { + Map responseMap = new HashMap<>(); + responseMap.put("took", 13); + responseMap.put("timed_out", false); + responseMap.put("total", 1); + responseMap.put("updated", 0); + responseMap.put("created", 0); + responseMap.put("deleted", 0); + responseMap.put("batches", 1); + responseMap.put("noops", 0); + responseMap.put("version_conflicts", 1); + + Map responseRetriesMap = new HashMap<>(); + responseRetriesMap.put("bulk", 0); + responseRetriesMap.put("search", 0); + responseMap.put("retries", responseRetriesMap); + + responseMap.put("throttled", "0s"); + responseMap.put("throttled_millis", 0); + responseMap.put("requests_per_second", -1); + responseMap.put("throttled_until", "0s"); + responseMap.put("throttled_until_millis", 0); + + Map failuresMap = new HashMap<>(); + failuresMap.put("index", "test-index"); + failuresMap.put("id", "test-failure-id"); + + Map causeMap = new HashMap<>(); + causeMap.put("type", "version_conflict_engine_exception"); + causeMap.put("reason", "version conflict"); + causeMap.put("index", "test-index"); + + failuresMap.put("cause", causeMap); + failuresMap.put("status", 409); + failuresMap.put("type", "_doc"); + + responseMap.put("failures", List.of(failuresMap)); + + return responseMap; + } +} diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCatClientIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCatClientIT.java index 230f067ce8..5cd390e38f 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCatClientIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCatClientIT.java @@ -101,7 +101,8 @@ public void testCatRecovery() throws Exception { createIndex("test-cat-recovery-index"); RecoveryResponse recoveryResponse = javaClient().cat() .recovery( - r -> r.headers( + r -> r.index("*,-.*") // exclude system indices + .headers( "index,shard,type,stage,source_host,source_node," + "target_host,target_node,repository,snapshot,files,files_recovered,files_percent,files_total" ).bytes(Bytes.Bytes) diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractTasksIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractTasksIT.java new file mode 100644 index 0000000000..ddf14deabd --- /dev/null +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractTasksIT.java @@ -0,0 +1,62 @@ +package org.opensearch.client.opensearch.integTest; + +import java.io.IOException; +import org.junit.Test; +import org.opensearch.client.opensearch.tasks.Status; + +public abstract class AbstractTasksIT extends OpenSearchJavaClientTestCase { + + @Test + public void getTasks_waitForCompletionFalse_jsonDataStatusCanBeDeserialized() throws IOException, InterruptedException { + String indexName = "test_index_tasks_response"; + javaClient().indices().create(_1 -> _1.index(indexName)); + javaClient().index(b -> b.index(indexName).id("a").document(new IndexData("test"))); + // Ensure the document is indexed + Thread.sleep(3000); + + final var deleteByQueryResponse = javaClient().deleteByQuery( + d -> d.index(indexName) + .query(q -> q.match(m -> m.queryName("match").query(_q -> _q.stringValue("test")).field("title"))) + .waitForCompletion(false) + ); + + Thread.sleep(3000); + + // Create a task to be used to deserialize a task status from as the test subject + final var tasksResponse = javaClient().tasks().get(t -> t.taskId(deleteByQueryResponse.task())); + + assertTrue(tasksResponse.completed()); + + // Deserialize the JsonData to a typed Status response + assertNotNull(tasksResponse.task().status()); + final Status taskStatus = tasksResponse.task().status().to(Status.class); + + // Ensure the JsonData can be deserialized + assertEquals(1, taskStatus.total()); + assertEquals(1, taskStatus.deleted()); + assertEquals(0, taskStatus.created()); + assertEquals(0, taskStatus.noops()); + assertEquals(0, taskStatus.failures().size()); + } + + public class IndexData { + private String title; + + public IndexData(String title) { + this.title = title; + } + + public String getTitle() { + return title; + } + + public void setTitle(String title) { + this.title = title; + } + + @Override + public String toString() { + return String.format("IndexData{title='%s'}", title); + } + } +} diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/OpenSearchJavaClientTestCase.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/OpenSearchJavaClientTestCase.java index 6403569faa..a672b8762a 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/OpenSearchJavaClientTestCase.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/OpenSearchJavaClientTestCase.java @@ -45,7 +45,12 @@ import org.opensearch.test.rest.OpenSearchRestTestCase; public abstract class OpenSearchJavaClientTestCase extends OpenSearchRestTestCase implements OpenSearchTransportSupport { - private static final List systemIndices = List.of(".opensearch-observability", ".opendistro_security", ".plugins-ml-config"); + private static final List systemIndices = List.of( + ".opensearch-observability", + ".opendistro_security", + ".plugins-ml-config", + ".tasks" + ); private static OpenSearchClient javaClient; private static OpenSearchClient adminJavaClient; diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/TasksIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/TasksIT.java new file mode 100644 index 0000000000..a3b7c95231 --- /dev/null +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/TasksIT.java @@ -0,0 +1,5 @@ +package org.opensearch.client.opensearch.integTest.httpclient5; + +import org.opensearch.client.opensearch.integTest.AbstractTasksIT; + +public class TasksIT extends AbstractTasksIT implements HttpClient5TransportSupport {} diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/restclient/TasksIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/restclient/TasksIT.java new file mode 100644 index 0000000000..7484a229df --- /dev/null +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/restclient/TasksIT.java @@ -0,0 +1,16 @@ +package org.opensearch.client.opensearch.integTest.restclient; + +import java.io.IOException; +import org.apache.hc.core5.http.HttpHost; +import org.opensearch.client.json.jackson.JacksonJsonpMapper; +import org.opensearch.client.opensearch.integTest.AbstractTasksIT; +import org.opensearch.client.transport.OpenSearchTransport; +import org.opensearch.client.transport.rest_client.RestClientTransport; +import org.opensearch.common.settings.Settings; + +public class TasksIT extends AbstractTasksIT { + @Override + public OpenSearchTransport buildTransport(Settings settings, HttpHost[] hosts) throws IOException { + return new RestClientTransport(buildClient(settings, hosts), new JacksonJsonpMapper()); + } +}