From 9e561c2921e0340ad54e930cf56b3c4498fb7934 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20B=C3=BCscher?= <cbuescher@posteo.de>
Date: Fri, 6 Mar 2020 14:55:48 +0100
Subject: [PATCH] Fix AbstractBulkByScrollRequest slices parameter via Rest
 (#53068)

Currently the AbstractBulkByScrollRequest accepts slice values of 0 via its
`setSlices` method, denoting the "auto" slicing behaviour that is usable by
settting the "slices=auto" parameter on rest requests. When using the High Level
Rest Client, however, we send the 0 value as an integer, which is then rejected
as invalid by `AbstractBulkByScrollRequest#parseSlices`. Instead of making
parsing of the rest request more lenient, this PR opts for changing the
RequestConverter logic in the client to translate 0 values to "auto" on the rest
requests.

Closes #53044
---
 .../client/RequestConverters.java             |  4 ++++
 .../client/RequestConvertersTests.java        | 21 +++++++++++++++----
 .../reindex/AbstractBulkByScrollRequest.java  |  1 +
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java
index 98631364f36..8e0000ed0f2 100644
--- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java
+++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java
@@ -925,6 +925,10 @@ final class RequestConverters {
         }
 
         Params withSlices(int slices) {
+            if (slices == 0) {
+                // translate to "auto" value in rest request so the receiving end doesn't throw error
+                return putParam("slices", AbstractBulkByScrollRequest.AUTO_SLICES_VALUE);
+            }
             return putParam("slices", String.valueOf(slices));
         }
 
diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java
index f6ecf38e7d8..70f427e50bc 100644
--- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java
+++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java
@@ -83,6 +83,7 @@ import org.elasticsearch.index.rankeval.RankEvalRequest;
 import org.elasticsearch.index.rankeval.RankEvalSpec;
 import org.elasticsearch.index.rankeval.RatedRequest;
 import org.elasticsearch.index.rankeval.RestRankEvalAction;
+import org.elasticsearch.index.reindex.AbstractBulkByScrollRequest;
 import org.elasticsearch.index.reindex.DeleteByQueryRequest;
 import org.elasticsearch.index.reindex.ReindexRequest;
 import org.elasticsearch.index.reindex.RemoteInfo;
@@ -497,9 +498,13 @@ public class RequestConvertersTests extends ESTestCase {
             reindexRequest.setSourceQuery(new TermQueryBuilder("foo", "fooval"));
         }
         if (randomBoolean()) {
-            int slices = randomInt(100);
+            int slices = randomIntBetween(0,4);
             reindexRequest.setSlices(slices);
-            expectedParams.put("slices", String.valueOf(slices));
+            if (slices == 0) {
+                expectedParams.put("slices", AbstractBulkByScrollRequest.AUTO_SLICES_VALUE);
+            } else {
+                expectedParams.put("slices", Integer.toString(slices));
+            }
         } else {
             expectedParams.put("slices", "1");
         }
@@ -567,7 +572,11 @@ public class RequestConvertersTests extends ESTestCase {
         }
         if (randomBoolean()) {
             int slices = randomIntBetween(0, 4);
-            expectedParams.put("slices", Integer.toString(slices));
+            if (slices == 0) {
+                expectedParams.put("slices", AbstractBulkByScrollRequest.AUTO_SLICES_VALUE);
+            } else {
+                expectedParams.put("slices", Integer.toString(slices));
+            }
             updateByQueryRequest.setSlices(slices);
         } else {
             expectedParams.put("slices", "1");
@@ -632,7 +641,11 @@ public class RequestConvertersTests extends ESTestCase {
         }
         if (randomBoolean()) {
             int slices = randomIntBetween(0, 4);
-            expectedParams.put("slices", Integer.toString(slices));
+            if (slices == 0) {
+                expectedParams.put("slices", AbstractBulkByScrollRequest.AUTO_SLICES_VALUE);
+            } else {
+                expectedParams.put("slices", Integer.toString(slices));
+            }
             deleteByQueryRequest.setSlices(slices);
         } else {
             expectedParams.put("slices", "1");
diff --git a/server/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequest.java b/server/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequest.java
index fcb6113d3d7..b2b8868cffd 100644
--- a/server/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequest.java
+++ b/server/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequest.java
@@ -402,6 +402,7 @@ public abstract class AbstractBulkByScrollRequest<Self extends AbstractBulkByScr
 
     /**
      * The number of slices this task should be divided into. Defaults to 1 meaning the task isn't sliced into subtasks.
+     * A value of 0 is equivalent to the "auto" slices parameter of the Rest API.
      */
     public Self setSlices(int slices) {
         if (slices < 0) {