From e23f6f1d3321550ccac7c2a892ce2fc55eb13e63 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 26 Jul 2019 12:03:12 +0200 Subject: [PATCH] DATAES-603 - Polishing. Add nullable annotations to nullable properties in BulkOptions. Add assertions to non-nullable method arguments. Reformat code. Make methods static where possible. Original pull request: #296. --- .../core/ElasticsearchRestTemplate.java | 15 +++-- .../core/ElasticsearchTemplate.java | 19 +++--- .../elasticsearch/core/query/BulkOptions.java | 63 ++++++++++++------- 3 files changed, 62 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java index 5e814855b..aa981eda3 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java @@ -741,6 +741,10 @@ public class ElasticsearchRestTemplate @Override public void bulkIndex(List queries, BulkOptions bulkOptions) { + + Assert.notNull(queries, "List of IndexQuery must not be null"); + Assert.notNull(bulkOptions, "BulkOptions must not be null"); + BulkRequest bulkRequest = new BulkRequest(); setBulkOptions(bulkRequest, bulkOptions); for (IndexQuery query : queries) { @@ -755,6 +759,10 @@ public class ElasticsearchRestTemplate @Override public void bulkUpdate(List queries, BulkOptions bulkOptions) { + + Assert.notNull(queries, "List of UpdateQuery must not be null"); + Assert.notNull(bulkOptions, "BulkOptions must not be null"); + BulkRequest bulkRequest = new BulkRequest(); setBulkOptions(bulkRequest, bulkOptions); for (UpdateQuery query : queries) { @@ -767,30 +775,25 @@ public class ElasticsearchRestTemplate } } - private void setBulkOptions(BulkRequest bulkRequest, BulkOptions bulkOptions) { + private static void setBulkOptions(BulkRequest bulkRequest, BulkOptions bulkOptions) { if (bulkOptions.getTimeout() != null) { - bulkRequest.timeout(bulkOptions.getTimeout()); } if (bulkOptions.getRefreshPolicy() != null) { - bulkRequest.setRefreshPolicy(bulkOptions.getRefreshPolicy()); } if (bulkOptions.getWaitForActiveShards() != null) { - bulkRequest.waitForActiveShards(bulkOptions.getWaitForActiveShards()); } if (bulkOptions.getPipeline() != null) { - bulkRequest.pipeline(bulkOptions.getPipeline()); } if (bulkOptions.getRoutingId() != null) { - bulkRequest.routing(bulkOptions.getRoutingId()); } } diff --git a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java index 0ab18c093..ea551b1a4 100755 --- a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java @@ -635,7 +635,11 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< } @Override - public void bulkIndex(List queries, @Nullable BulkOptions bulkOptions) { + public void bulkIndex(List queries, BulkOptions bulkOptions) { + + Assert.notNull(queries, "List of IndexQuery must not be null"); + Assert.notNull(bulkOptions, "BulkOptions must not be null"); + BulkRequestBuilder bulkRequest = client.prepareBulk(); setBulkOptions(bulkRequest, bulkOptions); for (IndexQuery query : queries) { @@ -645,7 +649,11 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< } @Override - public void bulkUpdate(List queries, @Nullable BulkOptions bulkOptions) { + public void bulkUpdate(List queries, BulkOptions bulkOptions) { + + Assert.notNull(queries, "List of UpdateQuery must not be null"); + Assert.notNull(bulkOptions, "BulkOptions must not be null"); + BulkRequestBuilder bulkRequest = client.prepareBulk(); setBulkOptions(bulkRequest, bulkOptions); for (UpdateQuery query : queries) { @@ -654,30 +662,25 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< checkForBulkUpdateFailure(bulkRequest.execute().actionGet()); } - private void setBulkOptions(BulkRequestBuilder bulkRequest, BulkOptions bulkOptions) { + private static void setBulkOptions(BulkRequestBuilder bulkRequest, BulkOptions bulkOptions) { if (bulkOptions.getTimeout() != null) { - bulkRequest.setTimeout(bulkOptions.getTimeout()); } if (bulkOptions.getRefreshPolicy() != null) { - bulkRequest.setRefreshPolicy(bulkOptions.getRefreshPolicy()); } if (bulkOptions.getWaitForActiveShards() != null) { - bulkRequest.setWaitForActiveShards(bulkOptions.getWaitForActiveShards()); } if (bulkOptions.getPipeline() != null) { - bulkRequest.pipeline(bulkOptions.getPipeline()); } if (bulkOptions.getRoutingId() != null) { - bulkRequest.routing(bulkOptions.getRoutingId()); } } diff --git a/src/main/java/org/springframework/data/elasticsearch/core/query/BulkOptions.java b/src/main/java/org/springframework/data/elasticsearch/core/query/BulkOptions.java index b79c542f8..d2bccc169 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/query/BulkOptions.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/query/BulkOptions.java @@ -19,6 +19,7 @@ import java.util.List; import org.elasticsearch.action.support.ActiveShardCount; import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.unit.TimeValue; /** @@ -29,28 +30,21 @@ import org.elasticsearch.common.unit.TimeValue; * {@link BulkOptionsBuilder#build()} to get the BulkOptions object. * * @author Peter-Josef Meisch + * @author Mark Paluch * @since 3.2 */ public class BulkOptions { private static final BulkOptions defaultOptions = builder().build(); - private final TimeValue timeout; - private final WriteRequest.RefreshPolicy refreshPolicy; - private final ActiveShardCount waitForActiveShards; - private final String pipeline; - private final String routingId; + private final @Nullable TimeValue timeout; + private final @Nullable WriteRequest.RefreshPolicy refreshPolicy; + private final @Nullable ActiveShardCount waitForActiveShards; + private final @Nullable String pipeline; + private final @Nullable String routingId; - public static BulkOptionsBuilder builder() { - return new BulkOptionsBuilder(); - } - - public static BulkOptions defaultOptions() { - return defaultOptions; - } - - private BulkOptions(TimeValue timeout, WriteRequest.RefreshPolicy refreshPolicy, ActiveShardCount waitForActiveShards, - String pipeline, String routingId) { + private BulkOptions(@Nullable TimeValue timeout, @Nullable WriteRequest.RefreshPolicy refreshPolicy, + @Nullable ActiveShardCount waitForActiveShards, @Nullable String pipeline, @Nullable String routingId) { this.timeout = timeout; this.refreshPolicy = refreshPolicy; this.waitForActiveShards = waitForActiveShards; @@ -58,32 +52,59 @@ public class BulkOptions { this.routingId = routingId; } + @Nullable public TimeValue getTimeout() { return timeout; } + @Nullable public WriteRequest.RefreshPolicy getRefreshPolicy() { return refreshPolicy; } + @Nullable public ActiveShardCount getWaitForActiveShards() { return waitForActiveShards; } + @Nullable public String getPipeline() { return pipeline; } + @Nullable public String getRoutingId() { return routingId; } - public static final class BulkOptionsBuilder { - private TimeValue timeout; - private WriteRequest.RefreshPolicy refreshPolicy; - private ActiveShardCount waitForActiveShards; - private String pipeline; - private String routingId; + /** + * Create a new {@link BulkOptionsBuilder} to build {@link BulkOptions}. + * + * @return a new {@link BulkOptionsBuilder} to build {@link BulkOptions}. + */ + public static BulkOptionsBuilder builder() { + return new BulkOptionsBuilder(); + } + + /** + * Return default {@link BulkOptions}. + * + * @return default {@link BulkOptions}. + */ + public static BulkOptions defaultOptions() { + return defaultOptions; + } + + /** + * Builder for {@link BulkOptions}. + */ + public static class BulkOptionsBuilder { + + private @Nullable TimeValue timeout; + private @Nullable WriteRequest.RefreshPolicy refreshPolicy; + private @Nullable ActiveShardCount waitForActiveShards; + private @Nullable String pipeline; + private @Nullable String routingId; private BulkOptionsBuilder() {}