From 0e7929f0d3e976b84389b465e688ec6f2f39e79f Mon Sep 17 00:00:00 2001 From: Christian Winkler Date: Mon, 27 Nov 2023 15:13:44 +0100 Subject: [PATCH 1/5] Added scriptedUpsert option to UpdateOperation (#744) Signed-off-by: Christian Winkler --- .../opensearch/core/bulk/UpdateOperation.java | 26 ++++++++++++++++--- .../core/bulk/UpdateOperationData.java | 20 ++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperation.java b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperation.java index 15d30fe0c4..3e55a900b4 100644 --- a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperation.java +++ b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperation.java @@ -32,11 +32,12 @@ package org.opensearch.client.opensearch.core.bulk; -import jakarta.json.stream.JsonGenerator; import java.util.Arrays; import java.util.Iterator; import java.util.function.Function; + import javax.annotation.Nullable; + import org.opensearch.client.json.JsonpMapper; import org.opensearch.client.json.JsonpSerializer; import org.opensearch.client.json.NdJsonpSerializable; @@ -44,6 +45,8 @@ import org.opensearch.client.util.ApiTypeHelper; import org.opensearch.client.util.ObjectBuilder; +import jakarta.json.stream.JsonGenerator; + // typedef: _global.bulk.UpdateOperation public class UpdateOperation extends BulkOperationBase implements NdJsonpSerializable, BulkOperationVariant { @@ -69,7 +72,8 @@ private UpdateOperation(Builder builder) { } - public static UpdateOperation of(Function, ObjectBuilder>> fn) { + public static UpdateOperation + of(Function, ObjectBuilder>> fn) { return fn.apply(new Builder<>()).build(); } @@ -102,6 +106,7 @@ public final Integer retryOnConflict() { return this.retryOnConflict; } + @Override protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { super.serializeInternal(generator, mapper); @@ -125,7 +130,7 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { public static class Builder extends BulkOperationBase.AbstractBuilder> implements - ObjectBuilder> { + ObjectBuilder> { private UpdateOperationData data; @@ -144,6 +149,9 @@ public static class Builder extends BulkOperationBase.AbstractBuilder @Nullable private Boolean docAsUpsert; + @Nullable + private Boolean scriptedUpsert; + @Nullable private TDocument upsert; @@ -166,6 +174,14 @@ public final Builder docAsUpsert(@Nullable Boolean value) { return this; } + /** + * API name: {@code scripted_upsert} + */ + public final Builder scriptedUpsert(@Nullable Boolean value) { + this.scriptedUpsert = value; + return this; + } + /** * API name: {@code upsert} */ @@ -218,17 +234,19 @@ protected Builder self() { * @throws NullPointerException * if some of the required fields are null. */ + @Override public UpdateOperation build() { _checkSingleUse(); data = new UpdateOperationData.Builder().document(document) .docAsUpsert(docAsUpsert) + .scriptedUpsert(scriptedUpsert) .script(script) .upsert(upsert) .tDocumentSerializer(tDocumentSerializer) .build(); - return new UpdateOperation(this); + return new UpdateOperation<>(this); } } diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java index 572930d0fd..832ab5753d 100644 --- a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java +++ b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java @@ -24,6 +24,9 @@ public class UpdateOperationData implements JsonpSerializable { @Nullable private final Boolean docAsUpsert; + @Nullable + private final Boolean scriptedUpsert; + @Nullable private final TDocument upsert; @@ -36,6 +39,7 @@ public class UpdateOperationData implements JsonpSerializable { private UpdateOperationData(Builder builder) { this.document = builder.document; this.docAsUpsert = builder.docAsUpsert; + this.scriptedUpsert = builder.scriptedUpsert; this.script = builder.script; this.upsert = builder.upsert; this.tDocumentSerializer = builder.tDocumentSerializer; @@ -55,6 +59,11 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { generator.write(this.docAsUpsert); } + if (this.scriptedUpsert != null) { + generator.writeKey("scripted_upsert"); + generator.write(scriptedUpsert); + } + if (this.document != null) { generator.writeKey("doc"); JsonpUtils.serialize(document, generator, tDocumentSerializer, mapper); @@ -87,6 +96,9 @@ public static class Builder extends BulkOperationBase.AbstractBuilder @Nullable private Boolean docAsUpsert; + @Nullable + private Boolean scriptedUpsert; + @Nullable private TDocument upsert; @@ -109,6 +121,14 @@ public final Builder docAsUpsert(@Nullable Boolean value) { return this; } + /** + * API name: {@code scripted_upsert} + */ + public final Builder scriptedUpsert(@Nullable Boolean value) { + this.scriptedUpsert = value; + return this; + } + /** * API name: {@code upsert} */ From ef3720c6f548a75d8411a01d0349d48a50e7ea8b Mon Sep 17 00:00:00 2001 From: Christian Winkler Date: Mon, 27 Nov 2023 15:29:58 +0100 Subject: [PATCH 2/5] Added detectNoop option to UpdateOperation (#744) Signed-off-by: Christian Winkler --- .../opensearch/core/bulk/UpdateOperation.java | 12 +++++++ .../core/bulk/UpdateOperationData.java | 31 ++++++++++++++++--- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperation.java b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperation.java index 3e55a900b4..473fe3bd5e 100644 --- a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperation.java +++ b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperation.java @@ -152,6 +152,9 @@ public static class Builder extends BulkOperationBase.AbstractBuilder @Nullable private Boolean scriptedUpsert; + @Nullable + private Boolean detectNoop; + @Nullable private TDocument upsert; @@ -182,6 +185,14 @@ public final Builder scriptedUpsert(@Nullable Boolean value) { return this; } + /** + * API name: {@code detect_noop} + */ + public final Builder detectNoop(@Nullable Boolean value) { + this.detectNoop = value; + return this; + } + /** * API name: {@code upsert} */ @@ -241,6 +252,7 @@ public UpdateOperation build() { data = new UpdateOperationData.Builder().document(document) .docAsUpsert(docAsUpsert) .scriptedUpsert(scriptedUpsert) + .detectNoop(detectNoop) .script(script) .upsert(upsert) .tDocumentSerializer(tDocumentSerializer) diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java index 832ab5753d..7f99ecf6af 100644 --- a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java +++ b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java @@ -8,8 +8,8 @@ package org.opensearch.client.opensearch.core.bulk; -import jakarta.json.stream.JsonGenerator; import javax.annotation.Nullable; + import org.opensearch.client.json.JsonpMapper; import org.opensearch.client.json.JsonpSerializable; import org.opensearch.client.json.JsonpSerializer; @@ -17,6 +17,8 @@ import org.opensearch.client.opensearch._types.Script; import org.opensearch.client.util.ObjectBuilder; +import jakarta.json.stream.JsonGenerator; + public class UpdateOperationData implements JsonpSerializable { @Nullable private final TDocument document; @@ -27,6 +29,9 @@ public class UpdateOperationData implements JsonpSerializable { @Nullable private final Boolean scriptedUpsert; + @Nullable + private final Boolean detectNoop; + @Nullable private final TDocument upsert; @@ -40,6 +45,7 @@ private UpdateOperationData(Builder builder) { this.document = builder.document; this.docAsUpsert = builder.docAsUpsert; this.scriptedUpsert = builder.scriptedUpsert; + this.detectNoop = builder.detectNoop; this.script = builder.script; this.upsert = builder.upsert; this.tDocumentSerializer = builder.tDocumentSerializer; @@ -64,6 +70,11 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { generator.write(scriptedUpsert); } + if (this.detectNoop != null) { + generator.writeKey("detect_noop"); + generator.write(scriptedUpsert); + } + if (this.document != null) { generator.writeKey("doc"); JsonpUtils.serialize(document, generator, tDocumentSerializer, mapper); @@ -85,7 +96,7 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { */ public static class Builder extends BulkOperationBase.AbstractBuilder> implements - ObjectBuilder> { + ObjectBuilder> { @Nullable private TDocument document; @@ -99,6 +110,9 @@ public static class Builder extends BulkOperationBase.AbstractBuilder @Nullable private Boolean scriptedUpsert; + @Nullable + private Boolean detectNoop; + @Nullable private TDocument upsert; @@ -128,7 +142,15 @@ public final Builder scriptedUpsert(@Nullable Boolean value) { this.scriptedUpsert = value; return this; } - + + /** + * API name: {@code detect_noop} + */ + public final Builder detectNoop(@Nullable Boolean value) { + this.detectNoop = value; + return this; + } + /** * API name: {@code upsert} */ @@ -165,10 +187,11 @@ protected Builder self() { * @throws NullPointerException * if some of the required fields are null. */ + @Override public UpdateOperationData build() { _checkSingleUse(); - return new UpdateOperationData(this); + return new UpdateOperationData<>(this); } } } From 824eced4abab2b5e5fcf43a30ed8fb536d389b09 Mon Sep 17 00:00:00 2001 From: Christian Winkler Date: Tue, 28 Nov 2023 08:30:34 +0100 Subject: [PATCH 3/5] updated changelog Signed-off-by: Christian Winkler --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42442e2a17..6c6104fe7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Removed ### Fixed +- Fix missing properties on UpdateOperation ([#744](https://github.com/opensearch-project/opensearch-java/pull/744)) ### Security From 39184fab6c7a501a8133426d6ac02e83f7d61146 Mon Sep 17 00:00:00 2001 From: Christian Winkler Date: Wed, 29 Nov 2023 09:58:52 +0100 Subject: [PATCH 4/5] Added tests Signed-off-by: Christian Winkler --- .../core/bulk/UpdateOperationData.java | 2 +- .../opensearch/integTest/AbstractCrudIT.java | 98 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java index 7f99ecf6af..635514c3e4 100644 --- a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java +++ b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java @@ -72,7 +72,7 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { if (this.detectNoop != null) { generator.writeKey("detect_noop"); - generator.write(scriptedUpsert); + generator.write(detectNoop); } if (this.document != null) { diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java index 16a9d5ff57..ced5e3ede8 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java @@ -392,6 +392,104 @@ public void testBulkUpdateScriptUpsert() throws IOException { assertTrue(getResponse.found()); assertEquals(1337, getResponse.source().getIntValue()); } + + public void testBulkUpdateScriptedUpsertUpdate() throws IOException { + final String id = "777"; + + final AppData appData = new AppData(); + appData.setIntValue(1337); + appData.setMsg("foo"); + + assertEquals(Result.Created, javaClient().index(b -> b.index("index").id(id).document(appData)).result()); + + final BulkOperation op = new BulkOperation.Builder().update( + o -> o.index("index") + .id(id) + .scriptedUpsert(true) + .upsert(Collections.emptyMap()) + .script( + Script.of( + s -> s.inline( + new InlineScript.Builder().lang("painless") + .source("ctx._source.intValue = ctx?._source?.intValue == null ? 7777 : 9999") + .build())))) + .build(); + + BulkRequest bulkRequest = new BulkRequest.Builder().operations(op).build(); + BulkResponse bulkResponse = javaClient().bulk(bulkRequest); + + assertTrue(bulkResponse.took() > 0); + assertEquals(1, bulkResponse.items().size()); + + final GetResponse getResponse = javaClient().get(b -> b.index("index").id(id), AppData.class); + assertTrue(getResponse.found()); + assertEquals(9999, getResponse.source().getIntValue()); + } + + public void testBulkUpdateScriptedUpsertInsert() throws IOException { + final String id = "778"; + + final BulkOperation op = new BulkOperation.Builder().update( + o -> o.index("index") + .id(id) + .scriptedUpsert(true) + .upsert(Collections.emptyMap()) + .script( + Script.of( + s -> s.inline( + new InlineScript.Builder().lang("painless") + .source("ctx._source.intValue = ctx?._source?.intValue == null ? 7777 : 9999") + .build())))) + .build(); + + BulkRequest bulkRequest = new BulkRequest.Builder().operations(op).build(); + BulkResponse bulkResponse = javaClient().bulk(bulkRequest); + + assertTrue(bulkResponse.took() > 0); + assertEquals(1, bulkResponse.items().size()); + + final GetResponse getResponse = javaClient().get(b -> b.index("index").id(id), AppData.class); + assertTrue(getResponse.found()); + assertEquals(7777, getResponse.source().getIntValue()); + } + + public void testBulkUpdateDetectNoop() throws IOException { + final String id = "779"; + + final AppData appData = new AppData(); + appData.setIntValue(1337); + appData.setMsg("foo"); + + assertEquals(Result.Created, javaClient().index(b -> b.index("index").id(id).document(appData)).result()); + + BulkOperation op = new BulkOperation.Builder().update( + o -> o.index("index") + .id(id) + .detectNoop(true) + .document(appData)) + .build(); + + BulkRequest bulkRequest = new BulkRequest.Builder().operations(op).build(); + BulkResponse bulkResponse = javaClient().bulk(bulkRequest); + + assertTrue(bulkResponse.took() > 0); + assertEquals(1, bulkResponse.items().size()); + assertEquals(Result.NoOp.jsonValue(), bulkResponse.items().get(0).result()); + + op = new BulkOperation.Builder().update( + o -> o.index("index") + .id(id) + .detectNoop(false) + .document(appData)) + .build(); + + bulkRequest = new BulkRequest.Builder().operations(op).build(); + bulkResponse = javaClient().bulk(bulkRequest); + assertTrue(bulkResponse.took() > 0); + assertEquals(1, bulkResponse.items().size()); + assertEquals(Result.Updated.jsonValue(), bulkResponse.items().get(0).result()); + + } public void testBulkUpdateUpsert() throws IOException { final String id = "100"; From d06be141985d26e94e617153ded6f63980186b3e Mon Sep 17 00:00:00 2001 From: Christian Winkler Date: Wed, 29 Nov 2023 11:18:37 +0100 Subject: [PATCH 5/5] Running spotlessApply Signed-off-by: Christian Winkler --- .../opensearch/core/bulk/UpdateOperation.java | 10 ++----- .../core/bulk/UpdateOperationData.java | 6 ++-- .../opensearch/integTest/AbstractCrudIT.java | 30 ++++++++----------- 3 files changed, 18 insertions(+), 28 deletions(-) diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperation.java b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperation.java index 473fe3bd5e..eca1eb8e01 100644 --- a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperation.java +++ b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperation.java @@ -32,12 +32,11 @@ package org.opensearch.client.opensearch.core.bulk; +import jakarta.json.stream.JsonGenerator; import java.util.Arrays; import java.util.Iterator; import java.util.function.Function; - import javax.annotation.Nullable; - import org.opensearch.client.json.JsonpMapper; import org.opensearch.client.json.JsonpSerializer; import org.opensearch.client.json.NdJsonpSerializable; @@ -45,8 +44,6 @@ import org.opensearch.client.util.ApiTypeHelper; import org.opensearch.client.util.ObjectBuilder; -import jakarta.json.stream.JsonGenerator; - // typedef: _global.bulk.UpdateOperation public class UpdateOperation extends BulkOperationBase implements NdJsonpSerializable, BulkOperationVariant { @@ -72,8 +69,7 @@ private UpdateOperation(Builder builder) { } - public static UpdateOperation - of(Function, ObjectBuilder>> fn) { + public static UpdateOperation of(Function, ObjectBuilder>> fn) { return fn.apply(new Builder<>()).build(); } @@ -130,7 +126,7 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { public static class Builder extends BulkOperationBase.AbstractBuilder> implements - ObjectBuilder> { + ObjectBuilder> { private UpdateOperationData data; diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java index 635514c3e4..07f02684de 100644 --- a/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java +++ b/java-client/src/main/java/org/opensearch/client/opensearch/core/bulk/UpdateOperationData.java @@ -8,8 +8,8 @@ package org.opensearch.client.opensearch.core.bulk; +import jakarta.json.stream.JsonGenerator; import javax.annotation.Nullable; - import org.opensearch.client.json.JsonpMapper; import org.opensearch.client.json.JsonpSerializable; import org.opensearch.client.json.JsonpSerializer; @@ -17,8 +17,6 @@ import org.opensearch.client.opensearch._types.Script; import org.opensearch.client.util.ObjectBuilder; -import jakarta.json.stream.JsonGenerator; - public class UpdateOperationData implements JsonpSerializable { @Nullable private final TDocument document; @@ -96,7 +94,7 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { */ public static class Builder extends BulkOperationBase.AbstractBuilder> implements - ObjectBuilder> { + ObjectBuilder> { @Nullable private TDocument document; diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java index ced5e3ede8..ed6fedc024 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java @@ -392,7 +392,7 @@ public void testBulkUpdateScriptUpsert() throws IOException { assertTrue(getResponse.found()); assertEquals(1337, getResponse.source().getIntValue()); } - + public void testBulkUpdateScriptedUpsertUpdate() throws IOException { final String id = "777"; @@ -412,8 +412,11 @@ public void testBulkUpdateScriptedUpsertUpdate() throws IOException { s -> s.inline( new InlineScript.Builder().lang("painless") .source("ctx._source.intValue = ctx?._source?.intValue == null ? 7777 : 9999") - .build())))) - .build(); + .build() + ) + ) + ) + ).build(); BulkRequest bulkRequest = new BulkRequest.Builder().operations(op).build(); BulkResponse bulkResponse = javaClient().bulk(bulkRequest); @@ -439,8 +442,11 @@ public void testBulkUpdateScriptedUpsertInsert() throws IOException { s -> s.inline( new InlineScript.Builder().lang("painless") .source("ctx._source.intValue = ctx?._source?.intValue == null ? 7777 : 9999") - .build())))) - .build(); + .build() + ) + ) + ) + ).build(); BulkRequest bulkRequest = new BulkRequest.Builder().operations(op).build(); BulkResponse bulkResponse = javaClient().bulk(bulkRequest); @@ -462,12 +468,7 @@ public void testBulkUpdateDetectNoop() throws IOException { assertEquals(Result.Created, javaClient().index(b -> b.index("index").id(id).document(appData)).result()); - BulkOperation op = new BulkOperation.Builder().update( - o -> o.index("index") - .id(id) - .detectNoop(true) - .document(appData)) - .build(); + BulkOperation op = new BulkOperation.Builder().update(o -> o.index("index").id(id).detectNoop(true).document(appData)).build(); BulkRequest bulkRequest = new BulkRequest.Builder().operations(op).build(); BulkResponse bulkResponse = javaClient().bulk(bulkRequest); @@ -476,12 +477,7 @@ public void testBulkUpdateDetectNoop() throws IOException { assertEquals(1, bulkResponse.items().size()); assertEquals(Result.NoOp.jsonValue(), bulkResponse.items().get(0).result()); - op = new BulkOperation.Builder().update( - o -> o.index("index") - .id(id) - .detectNoop(false) - .document(appData)) - .build(); + op = new BulkOperation.Builder().update(o -> o.index("index").id(id).detectNoop(false).document(appData)).build(); bulkRequest = new BulkRequest.Builder().operations(op).build(); bulkResponse = javaClient().bulk(bulkRequest);