diff --git a/docs/changelog/104956.yaml b/docs/changelog/104956.yaml new file mode 100644 index 0000000000000..fd6365baa812d --- /dev/null +++ b/docs/changelog/104956.yaml @@ -0,0 +1,5 @@ +pr: 104956 +summary: Changing `ReplicationRequestBuilder` to be an `ActionRequestLazyBuilder` +area: Ingest Node +type: enhancement +issues: [] diff --git a/server/src/main/java/org/elasticsearch/action/delete/DeleteRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/delete/DeleteRequestBuilder.java index f99bea1a64821..dac5421bdeee0 100644 --- a/server/src/main/java/org/elasticsearch/action/delete/DeleteRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/delete/DeleteRequestBuilder.java @@ -8,6 +8,7 @@ package org.elasticsearch.action.delete; +import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.action.support.WriteRequestBuilder; import org.elasticsearch.action.support.replication.ReplicationRequestBuilder; import org.elasticsearch.client.internal.ElasticsearchClient; @@ -21,15 +22,25 @@ public class DeleteRequestBuilder extends ReplicationRequestBuilder { + private String id; + private String routing; + private Long version; + private VersionType versionType; + private Long seqNo; + private Long term; + private WriteRequest.RefreshPolicy refreshPolicy; + private String refreshPolicyString; + public DeleteRequestBuilder(ElasticsearchClient client, @Nullable String index) { - super(client, TransportDeleteAction.TYPE, new DeleteRequest(index)); + super(client, TransportDeleteAction.TYPE); + setIndex(index); } /** * Sets the id of the document to delete. */ public DeleteRequestBuilder setId(String id) { - request.id(id); + this.id = id; return this; } @@ -38,7 +49,7 @@ public DeleteRequestBuilder setId(String id) { * and not the id. */ public DeleteRequestBuilder setRouting(String routing) { - request.routing(routing); + this.routing = routing; return this; } @@ -47,7 +58,7 @@ public DeleteRequestBuilder setRouting(String routing) { * version exists and no changes happened on the doc since then. */ public DeleteRequestBuilder setVersion(long version) { - request.version(version); + this.version = version; return this; } @@ -55,7 +66,7 @@ public DeleteRequestBuilder setVersion(long version) { * Sets the type of versioning to use. Defaults to {@link VersionType#INTERNAL}. */ public DeleteRequestBuilder setVersionType(VersionType versionType) { - request.versionType(versionType); + this.versionType = versionType; return this; } @@ -67,7 +78,7 @@ public DeleteRequestBuilder setVersionType(VersionType versionType) { * {@link org.elasticsearch.index.engine.VersionConflictEngineException} will be thrown. */ public DeleteRequestBuilder setIfSeqNo(long seqNo) { - request.setIfSeqNo(seqNo); + this.seqNo = seqNo; return this; } @@ -79,8 +90,59 @@ public DeleteRequestBuilder setIfSeqNo(long seqNo) { * {@link org.elasticsearch.index.engine.VersionConflictEngineException} will be thrown. */ public DeleteRequestBuilder setIfPrimaryTerm(long term) { - request.setIfPrimaryTerm(term); + this.term = term; + return this; + } + + @Override + public DeleteRequestBuilder setRefreshPolicy(WriteRequest.RefreshPolicy refreshPolicy) { + this.refreshPolicy = refreshPolicy; + return this; + } + + @Override + public DeleteRequestBuilder setRefreshPolicy(String refreshPolicy) { + this.refreshPolicyString = refreshPolicy; return this; } + @Override + public DeleteRequest request() { + validate(); + DeleteRequest request = new DeleteRequest(); + super.apply(request); + if (id != null) { + request.id(id); + } + if (routing != null) { + request.routing(routing); + } + if (version != null) { + request.version(version); + } + if (versionType != null) { + request.versionType(versionType); + } + if (seqNo != null) { + request.setIfSeqNo(seqNo); + } + if (term != null) { + request.setIfPrimaryTerm(term); + } + if (refreshPolicy != null) { + request.setRefreshPolicy(refreshPolicy); + } + if (refreshPolicyString != null) { + request.setRefreshPolicy(refreshPolicyString); + } + return request; + } + + @Override + protected void validate() throws IllegalStateException { + super.validate(); + if (refreshPolicy != null && refreshPolicyString != null) { + throw new IllegalStateException("Must use only one setRefreshPolicy method"); + } + } } diff --git a/server/src/main/java/org/elasticsearch/action/index/IndexRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/index/IndexRequestBuilder.java index b8faf39514cbe..2baf73d55ee9c 100644 --- a/server/src/main/java/org/elasticsearch/action/index/IndexRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/index/IndexRequestBuilder.java @@ -10,6 +10,7 @@ import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.DocWriteResponse; +import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.action.support.WriteRequestBuilder; import org.elasticsearch.action.support.replication.ReplicationRequestBuilder; import org.elasticsearch.client.internal.ElasticsearchClient; @@ -27,13 +28,44 @@ public class IndexRequestBuilder extends ReplicationRequestBuilder implements WriteRequestBuilder { + private String id = null; + /* + * The following variables hold information about the source of the request. Only one of sourceMap, sourceArray, sourceString, + * sourceBytesReference, or sourceBytes can actually be used. When request() is called it makes sure that only one is set. + */ + private Map sourceMap; + private Object[] sourceArray; + private XContentBuilder sourceXContentBuilder; + private String sourceString; + private BytesReference sourceBytesReference; + private byte[] sourceBytes; + // Optionally used with sourceBytes: + private Integer sourceOffset; + // Optionally used with sourceBytes: + private Integer sourceLength; + // Optionally used with sourceMap, sourceArray, sourceString, sourceBytesReference, or sourceBytes: + private XContentType sourceContentType; + + private String pipeline; + private Boolean requireAlias; + private Boolean requireDataStream; + private String routing; + private WriteRequest.RefreshPolicy refreshPolicy; + private String refreshPolicyString; + private Long ifSeqNo; + private Long ifPrimaryTerm; + private DocWriteRequest.OpType opType; + private Boolean create; + private Long version; + private VersionType versionType; public IndexRequestBuilder(ElasticsearchClient client) { - super(client, TransportIndexAction.TYPE, new IndexRequest()); + this(client, null); } public IndexRequestBuilder(ElasticsearchClient client, @Nullable String index) { - super(client, TransportIndexAction.TYPE, new IndexRequest(index)); + super(client, TransportIndexAction.TYPE); + setIndex(index); } /** @@ -41,7 +73,7 @@ public IndexRequestBuilder(ElasticsearchClient client, @Nullable String index) { * generated. */ public IndexRequestBuilder setId(String id) { - request.id(id); + this.id = id; return this; } @@ -50,7 +82,7 @@ public IndexRequestBuilder setId(String id) { * and not the id. */ public IndexRequestBuilder setRouting(String routing) { - request.routing(routing); + this.routing = routing; return this; } @@ -58,7 +90,8 @@ public IndexRequestBuilder setRouting(String routing) { * Sets the source. */ public IndexRequestBuilder setSource(BytesReference source, XContentType xContentType) { - request.source(source, xContentType); + this.sourceBytesReference = source; + this.sourceContentType = xContentType; return this; } @@ -68,7 +101,7 @@ public IndexRequestBuilder setSource(BytesReference source, XContentType xConten * @param source The map to index */ public IndexRequestBuilder setSource(Map source) { - request.source(source); + this.sourceMap = source; return this; } @@ -78,7 +111,8 @@ public IndexRequestBuilder setSource(Map source) { * @param source The map to index */ public IndexRequestBuilder setSource(Map source, XContentType contentType) { - request.source(source, contentType); + this.sourceMap = source; + this.sourceContentType = contentType; return this; } @@ -89,7 +123,8 @@ public IndexRequestBuilder setSource(Map source, XContentType content * or using the {@link #setSource(byte[], XContentType)}. */ public IndexRequestBuilder setSource(String source, XContentType xContentType) { - request.source(source, xContentType); + this.sourceString = source; + this.sourceContentType = xContentType; return this; } @@ -97,7 +132,7 @@ public IndexRequestBuilder setSource(String source, XContentType xContentType) { * Sets the content source to index. */ public IndexRequestBuilder setSource(XContentBuilder sourceBuilder) { - request.source(sourceBuilder); + this.sourceXContentBuilder = sourceBuilder; return this; } @@ -105,7 +140,8 @@ public IndexRequestBuilder setSource(XContentBuilder sourceBuilder) { * Sets the document to index in bytes form. */ public IndexRequestBuilder setSource(byte[] source, XContentType xContentType) { - request.source(source, xContentType); + this.sourceBytes = source; + this.sourceContentType = xContentType; return this; } @@ -119,7 +155,10 @@ public IndexRequestBuilder setSource(byte[] source, XContentType xContentType) { * @param xContentType The type/format of the source */ public IndexRequestBuilder setSource(byte[] source, int offset, int length, XContentType xContentType) { - request.source(source, offset, length, xContentType); + this.sourceBytes = source; + this.sourceOffset = offset; + this.sourceLength = length; + this.sourceContentType = xContentType; return this; } @@ -132,7 +171,10 @@ public IndexRequestBuilder setSource(byte[] source, int offset, int length, XCon *

*/ public IndexRequestBuilder setSource(Object... source) { - request.source(source); + if (source.length % 2 != 0) { + throw new IllegalArgumentException("The number of object passed must be even but was [" + source.length + "]"); + } + this.sourceArray = source; return this; } @@ -145,7 +187,11 @@ public IndexRequestBuilder setSource(Object... source) { *

*/ public IndexRequestBuilder setSource(XContentType xContentType, Object... source) { - request.source(xContentType, source); + if (source.length % 2 != 0) { + throw new IllegalArgumentException("The number of object passed must be even but was [" + source.length + "]"); + } + this.sourceArray = source; + this.sourceContentType = xContentType; return this; } @@ -153,7 +199,7 @@ public IndexRequestBuilder setSource(XContentType xContentType, Object... source * Sets the type of operation to perform. */ public IndexRequestBuilder setOpType(DocWriteRequest.OpType opType) { - request.opType(opType); + this.opType = opType; return this; } @@ -161,7 +207,7 @@ public IndexRequestBuilder setOpType(DocWriteRequest.OpType opType) { * Set to {@code true} to force this index to use {@link org.elasticsearch.action.index.IndexRequest.OpType#CREATE}. */ public IndexRequestBuilder setCreate(boolean create) { - request.create(create); + this.create = create; return this; } @@ -170,7 +216,7 @@ public IndexRequestBuilder setCreate(boolean create) { * version exists and no changes happened on the doc since then. */ public IndexRequestBuilder setVersion(long version) { - request.version(version); + this.version = version; return this; } @@ -178,7 +224,7 @@ public IndexRequestBuilder setVersion(long version) { * Sets the versioning type. Defaults to {@link VersionType#INTERNAL}. */ public IndexRequestBuilder setVersionType(VersionType versionType) { - request.versionType(versionType); + this.versionType = versionType; return this; } @@ -190,7 +236,7 @@ public IndexRequestBuilder setVersionType(VersionType versionType) { * {@link org.elasticsearch.index.engine.VersionConflictEngineException} will be thrown. */ public IndexRequestBuilder setIfSeqNo(long seqNo) { - request.setIfSeqNo(seqNo); + this.ifSeqNo = seqNo; return this; } @@ -202,7 +248,7 @@ public IndexRequestBuilder setIfSeqNo(long seqNo) { * {@link org.elasticsearch.index.engine.VersionConflictEngineException} will be thrown. */ public IndexRequestBuilder setIfPrimaryTerm(long term) { - request.setIfPrimaryTerm(term); + this.ifPrimaryTerm = term; return this; } @@ -210,7 +256,7 @@ public IndexRequestBuilder setIfPrimaryTerm(long term) { * Sets the ingest pipeline to be executed before indexing the document */ public IndexRequestBuilder setPipeline(String pipeline) { - request.setPipeline(pipeline); + this.pipeline = pipeline; return this; } @@ -218,7 +264,7 @@ public IndexRequestBuilder setPipeline(String pipeline) { * Sets the require_alias flag */ public IndexRequestBuilder setRequireAlias(boolean requireAlias) { - request.setRequireAlias(requireAlias); + this.requireAlias = requireAlias; return this; } @@ -226,7 +272,121 @@ public IndexRequestBuilder setRequireAlias(boolean requireAlias) { * Sets the require_data_stream flag */ public IndexRequestBuilder setRequireDataStream(boolean requireDataStream) { - request.setRequireDataStream(requireDataStream); + this.requireDataStream = requireDataStream; + return this; + } + + public IndexRequestBuilder setRefreshPolicy(WriteRequest.RefreshPolicy refreshPolicy) { + this.refreshPolicy = refreshPolicy; return this; } + + public IndexRequestBuilder setRefreshPolicy(String refreshPolicy) { + this.refreshPolicyString = refreshPolicy; + return this; + } + + @Override + public IndexRequest request() { + validate(); + IndexRequest request = new IndexRequest(); + super.apply(request); + request.id(id); + if (sourceMap != null) { + if (sourceContentType == null) { + request.source(sourceMap); + } else { + request.source(sourceMap, sourceContentType); + } + } + if (sourceArray != null) { + if (sourceContentType == null) { + request.source(sourceArray); + } else { + request.source(sourceContentType, sourceArray); + } + } + if (sourceXContentBuilder != null) { + request.source(sourceXContentBuilder); + } + if (sourceString != null && sourceContentType != null) { + request.source(sourceString, sourceContentType); + } + if (sourceBytesReference != null && sourceContentType != null) { + request.source(sourceBytesReference, sourceContentType); + } + if (sourceBytes != null && sourceContentType != null) { + if (sourceOffset != null && sourceLength != null) { + request.source(sourceBytes, sourceOffset, sourceLength, sourceContentType); + } else { + request.source(sourceBytes, sourceContentType); + } + } + if (pipeline != null) { + request.setPipeline(pipeline); + } + if (routing != null) { + request.routing(routing); + } + if (refreshPolicy != null) { + request.setRefreshPolicy(refreshPolicy); + } + if (refreshPolicyString != null) { + request.setRefreshPolicy(refreshPolicyString); + } + if (ifSeqNo != null) { + request.setIfSeqNo(ifSeqNo); + } + if (ifPrimaryTerm != null) { + request.setIfPrimaryTerm(ifPrimaryTerm); + } + if (pipeline != null) { + request.setPipeline(pipeline); + } + if (requireAlias != null) { + request.setRequireAlias(requireAlias); + } + if (requireDataStream != null) { + request.setRequireDataStream(requireDataStream); + } + if (opType != null) { + request.opType(opType); + } + if (create != null) { + request.create(create); + } + if (version != null) { + request.version(version); + } + if (versionType != null) { + request.versionType(versionType); + } + return request; + } + + @Override + protected void validate() throws IllegalStateException { + super.validate(); + int sourceFieldsSet = countSourceFieldsSet(); + if (sourceFieldsSet > 1) { + throw new IllegalStateException("Only one setSource() method may be called, but " + sourceFieldsSet + " have been"); + } + } + + /* + * Returns the number of the source fields that are non-null (ideally this will be 1). + */ + private int countSourceFieldsSet() { + return countNonNullObjects(sourceMap, sourceArray, sourceXContentBuilder, sourceString, sourceBytesReference, sourceBytes); + } + + private int countNonNullObjects(Object... objects) { + int sum = 0; + for (Object object : objects) { + if (object != null) { + sum++; + } + } + return sum; + } } diff --git a/server/src/main/java/org/elasticsearch/action/support/replication/ReplicationRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/support/replication/ReplicationRequestBuilder.java index a4d5e07103df3..94935a670afb7 100644 --- a/server/src/main/java/org/elasticsearch/action/support/replication/ReplicationRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/support/replication/ReplicationRequestBuilder.java @@ -8,7 +8,7 @@ package org.elasticsearch.action.support.replication; -import org.elasticsearch.action.ActionRequestBuilder; +import org.elasticsearch.action.ActionRequestLazyBuilder; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.ActionType; import org.elasticsearch.action.support.ActiveShardCount; @@ -18,18 +18,24 @@ public abstract class ReplicationRequestBuilder< Request extends ReplicationRequest, Response extends ActionResponse, - RequestBuilder extends ReplicationRequestBuilder> extends ActionRequestBuilder { + RequestBuilder extends ReplicationRequestBuilder> extends ActionRequestLazyBuilder< + Request, + Response> { + private String index; + private TimeValue timeout; + private String timeoutString; + private ActiveShardCount waitForActiveShards; - protected ReplicationRequestBuilder(ElasticsearchClient client, ActionType action, Request request) { - super(client, action, request); + protected ReplicationRequestBuilder(ElasticsearchClient client, ActionType action) { + super(client, action); } /** * A timeout to wait if the index operation can't be performed immediately. Defaults to {@code 1m}. */ @SuppressWarnings("unchecked") - public final RequestBuilder setTimeout(TimeValue timeout) { - request.timeout(timeout); + public RequestBuilder setTimeout(TimeValue timeout) { + this.timeout = timeout; return (RequestBuilder) this; } @@ -37,24 +43,28 @@ public final RequestBuilder setTimeout(TimeValue timeout) { * A timeout to wait if the index operation can't be performed immediately. Defaults to {@code 1m}. */ @SuppressWarnings("unchecked") - public final RequestBuilder setTimeout(String timeout) { - request.timeout(timeout); + public RequestBuilder setTimeout(String timeout) { + this.timeoutString = timeout; return (RequestBuilder) this; } @SuppressWarnings("unchecked") - public final RequestBuilder setIndex(String index) { - request.index(index); + public RequestBuilder setIndex(String index) { + this.index = index; return (RequestBuilder) this; } + public String getIndex() { + return index; + } + /** * Sets the number of shard copies that must be active before proceeding with the write. * See {@link ReplicationRequest#waitForActiveShards(ActiveShardCount)} for details. */ @SuppressWarnings("unchecked") public RequestBuilder setWaitForActiveShards(ActiveShardCount waitForActiveShards) { - request.waitForActiveShards(waitForActiveShards); + this.waitForActiveShards = waitForActiveShards; return (RequestBuilder) this; } @@ -66,4 +76,25 @@ public RequestBuilder setWaitForActiveShards(ActiveShardCount waitForActiveShard public RequestBuilder setWaitForActiveShards(final int waitForActiveShards) { return setWaitForActiveShards(ActiveShardCount.from(waitForActiveShards)); } + + protected void apply(Request request) { + if (index != null) { + request.index(index); + } + if (timeout != null) { + request.timeout(timeout); + } + if (timeoutString != null) { + request.timeout(timeoutString); + } + if (waitForActiveShards != null) { + request.waitForActiveShards(waitForActiveShards); + } + } + + protected void validate() throws IllegalStateException { + if (timeout != null && timeoutString != null) { + throw new IllegalStateException("Must use only one setTimeout method"); + } + } } diff --git a/server/src/test/java/org/elasticsearch/action/delete/DeleteRequestBuilderTests.java b/server/src/test/java/org/elasticsearch/action/delete/DeleteRequestBuilderTests.java new file mode 100644 index 0000000000000..0a59dac833ca9 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/delete/DeleteRequestBuilderTests.java @@ -0,0 +1,28 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.action.delete; + +import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.core.TimeValue; +import org.elasticsearch.test.ESTestCase; + +public class DeleteRequestBuilderTests extends ESTestCase { + + public void testValidation() { + DeleteRequestBuilder deleteRequestBuilder = new DeleteRequestBuilder(null, randomAlphaOfLength(10)); + deleteRequestBuilder.setRefreshPolicy(randomFrom(WriteRequest.RefreshPolicy.values()).toString()); + deleteRequestBuilder.setRefreshPolicy(randomFrom(WriteRequest.RefreshPolicy.values())); + expectThrows(IllegalStateException.class, deleteRequestBuilder::request); + + deleteRequestBuilder = new DeleteRequestBuilder(null, randomAlphaOfLength(10)); + deleteRequestBuilder.setTimeout(randomTimeValue()); + deleteRequestBuilder.setTimeout(TimeValue.timeValueSeconds(randomIntBetween(1, 30))); + expectThrows(IllegalStateException.class, deleteRequestBuilder::request); + } +} diff --git a/server/src/test/java/org/elasticsearch/action/index/IndexRequestBuilderTests.java b/server/src/test/java/org/elasticsearch/action/index/IndexRequestBuilderTests.java index 9af522524abc9..778bd6a1d138e 100644 --- a/server/src/test/java/org/elasticsearch/action/index/IndexRequestBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/action/index/IndexRequestBuilderTests.java @@ -9,6 +9,7 @@ package org.elasticsearch.action.index; import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.client.NoOpClient; import org.elasticsearch.threadpool.TestThreadPool; @@ -82,4 +83,20 @@ public void testSetSource() throws Exception { indexRequestBuilder.setSource(doc); assertEquals(EXPECTED_SOURCE, XContentHelper.convertToJson(indexRequestBuilder.request().source(), true)); } + + public void testValidation() { + IndexRequestBuilder indexRequestBuilder = new IndexRequestBuilder(this.testClient); + Map source = new HashMap<>(); + source.put("SomeKey", "SomeValue"); + indexRequestBuilder.setSource(source); + assertNotNull(indexRequestBuilder.request()); + indexRequestBuilder.setSource("SomeKey", "SomeValue"); + expectThrows(IllegalStateException.class, indexRequestBuilder::request); + + indexRequestBuilder = new IndexRequestBuilder(this.testClient); + indexRequestBuilder.setTimeout(randomTimeValue()); + assertNotNull(indexRequestBuilder.request()); + indexRequestBuilder.setTimeout(TimeValue.timeValueSeconds(randomIntBetween(1, 30))); + expectThrows(IllegalStateException.class, indexRequestBuilder::request); + } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index 65b28ad874431..82e02ec1db70f 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -1649,7 +1649,7 @@ public void indexRandom(boolean forceRefresh, boolean dummyDocuments, boolean ma Set indices = new HashSet<>(); builders = new ArrayList<>(builders); for (IndexRequestBuilder builder : builders) { - indices.add(builder.request().index()); + indices.add(builder.getIndex()); } Set> bogusIds = new HashSet<>(); // (index, type, id) if (random.nextBoolean() && builders.isEmpty() == false && dummyDocuments) {