[ILM] refactor ExplainLifecycleRequest to enforce indices (#35753)

Previously, it was possible to initialize a request
that does not include any indices. This results in undefined
behavior when generating the URL path to send to ES. By
enforcing indices to be defined, users will be more explicit
about the intention.

Author: talevy

client/rest-high-level/src/main/java/org/elasticsearch/client/IndexLifecycleRequestConverters.java

@@ -126,10 +126,9 @@ static Request lifecycleManagementStatus(LifecycleManagementStatusRequest lifecy
     }
 
     static Request explainLifecycle(ExplainLifecycleRequest explainLifecycleRequest) {
-        String[] indices = explainLifecycleRequest.indices() == null ? Strings.EMPTY_ARRAY : explainLifecycleRequest.indices();
         Request request = new Request(HttpGet.METHOD_NAME,
             new RequestConverters.EndpointBuilder()
-                .addCommaSeparatedPathParts(indices)
+                .addCommaSeparatedPathParts(explainLifecycleRequest.getIndices())
                 .addPathPartAsIs("_ilm")
                 .addPathPartAsIs("explain")
             .build());

client/rest-high-level/src/main/java/org/elasticsearch/client/indexlifecycle/ExplainLifecycleRequest.java

@@ -22,33 +22,27 @@
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.client.TimedRequest;
 import org.elasticsearch.client.ValidationException;
-import org.elasticsearch.common.Strings;
 
 import java.util.Arrays;
 import java.util.Objects;
 import java.util.Optional;
 
 /**
  * The request object used by the Explain Lifecycle API.
- *
- * Multiple indices may be queried in the same request using the
- * {@link #indices(String...)} method
  */
 public class ExplainLifecycleRequest extends TimedRequest {
 
-    private String[] indices = Strings.EMPTY_ARRAY;
+    private final String[] indices;
     private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpen();
 
-    public ExplainLifecycleRequest() {
-        super();
-    }
-
-    public ExplainLifecycleRequest indices(String... indices) {
+    public ExplainLifecycleRequest(String... indices) {
+        if (indices.length == 0) {
+            throw new IllegalArgumentException("Must at least specify one index to explain");
+        }
         this.indices = indices;
-        return this;
     }
 
-    public String[] indices() {
+    public String[] getIndices() {
         return indices;
     }
 
@@ -60,15 +54,15 @@ public ExplainLifecycleRequest indicesOptions(IndicesOptions indicesOptions) {
     public IndicesOptions indicesOptions() {
         return indicesOptions;
     }
-    
+
     @Override
     public Optional<ValidationException> validate() {
         return Optional.empty();
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(Arrays.hashCode(indices()), indicesOptions());
+        return Objects.hash(Arrays.hashCode(indices), indicesOptions);
     }
 
     @Override
@@ -80,13 +74,13 @@ public boolean equals(Object obj) {
             return false;
         }
         ExplainLifecycleRequest other = (ExplainLifecycleRequest) obj;
-        return Objects.deepEquals(indices(), other.indices()) &&
+        return Objects.deepEquals(getIndices(), other.getIndices()) &&
                 Objects.equals(indicesOptions(), other.indicesOptions());
     }
 
     @Override
     public String toString() {
-        return "ExplainLifecycleRequest [indices()=" + Arrays.toString(indices()) + ", indicesOptions()=" + indicesOptions() + "]";
+        return "ExplainLifecycleRequest [indices()=" + Arrays.toString(indices) + ", indicesOptions()=" + indicesOptions + "]";
     }
 
 }

client/rest-high-level/src/test/java/org/elasticsearch/client/IndexLifecycleIT.java

@@ -182,8 +182,7 @@ public void testExplainLifecycle() throws Exception {
 
         createIndex("squash", Settings.EMPTY);
 
-        ExplainLifecycleRequest req = new ExplainLifecycleRequest();
-        req.indices("foo-01", "baz-01", "squash");
+        ExplainLifecycleRequest req = new ExplainLifecycleRequest("foo-01", "baz-01", "squash");
         ExplainLifecycleResponse response = execute(req, highLevelClient().indexLifecycle()::explainLifecycle,
                 highLevelClient().indexLifecycle()::explainLifecycleAsync);
         Map<String, IndexLifecycleExplainResponse> indexResponses = response.getIndexResponses();

client/rest-high-level/src/test/java/org/elasticsearch/client/IndexLifecycleRequestConvertersTests.java

@@ -142,17 +142,15 @@ public void testLifecycleManagementStatus() throws Exception {
     }
 
     public void testExplainLifecycle() throws Exception {
-        ExplainLifecycleRequest req = new ExplainLifecycleRequest();
-        String[] indices = rarely() ? null : randomIndicesNames(0, 10);
-        req.indices(indices);
+        ExplainLifecycleRequest req = new ExplainLifecycleRequest(randomIndicesNames(1, 10));
         Map<String, String> expectedParams = new HashMap<>();
         setRandomMasterTimeout(req, expectedParams);
         setRandomIndicesOptions(req::indicesOptions, req::indicesOptions, expectedParams);
 
         Request request = IndexLifecycleRequestConverters.explainLifecycle(req);
         assertThat(request.getMethod(), equalTo(HttpGet.METHOD_NAME));
-        String idxString = Strings.arrayToCommaDelimitedString(indices);
-        assertThat(request.getEndpoint(), equalTo("/" + (idxString.isEmpty() ? "" : (idxString + "/")) + "_ilm/explain"));
+        String idxString = Strings.arrayToCommaDelimitedString(req.getIndices());
+        assertThat(request.getEndpoint(), equalTo("/" + idxString + "/" + "_ilm/explain"));
         assertThat(request.getParameters(), equalTo(expectedParams));
     }
 

client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/ILMDocumentationIT.java

@@ -482,7 +482,7 @@ public void testRetryPolicy() throws Exception {
                     .build());
             client.indices().create(createIndexRequest, RequestOptions.DEFAULT);
             assertBusy(() -> assertNotNull(client.indexLifecycle()
-                .explainLifecycle(new ExplainLifecycleRequest().indices("my_index"), RequestOptions.DEFAULT)
+                .explainLifecycle(new ExplainLifecycleRequest("my_index"), RequestOptions.DEFAULT)
                 .getIndexResponses().get("my_index").getFailedStep()));
         }
 

client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/ExplainLifecycleRequestTests.java

@@ -25,17 +25,21 @@
 
 import java.util.Arrays;
 
+import static org.hamcrest.Matchers.equalTo;
+
 public class ExplainLifecycleRequestTests extends ESTestCase {
 
     public void testEqualsAndHashcode() {
         EqualsHashCodeTestUtils.checkEqualsAndHashCode(createTestInstance(), this::copy, this::mutateInstance);
     }
 
+    public void testEmptyIndices() {
+        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, ExplainLifecycleRequest::new);
+        assertThat(exception.getMessage(), equalTo("Must at least specify one index to explain"));
+    }
+
     private ExplainLifecycleRequest createTestInstance() {
-        ExplainLifecycleRequest request = new ExplainLifecycleRequest();
-        if (randomBoolean()) {
-            request.indices(generateRandomStringArray(20, 20, false, true));
-        }
+        ExplainLifecycleRequest request = new ExplainLifecycleRequest(generateRandomStringArray(20, 20, false, true));
         if (randomBoolean()) {
             IndicesOptions indicesOptions = IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(),
                     randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean());
@@ -45,11 +49,11 @@ private ExplainLifecycleRequest createTestInstance() {
     }
 
     private ExplainLifecycleRequest mutateInstance(ExplainLifecycleRequest instance) {
-        String[] indices = instance.indices();
+        String[] indices = instance.getIndices();
         IndicesOptions indicesOptions = instance.indicesOptions();
         switch (between(0, 1)) {
         case 0:
-            indices = randomValueOtherThanMany(i -> Arrays.equals(i, instance.indices()),
+            indices = randomValueOtherThanMany(i -> Arrays.equals(i, instance.getIndices()),
                     () -> generateRandomStringArray(20, 10, false, true));
             break;
         case 1:
@@ -59,15 +63,13 @@ private ExplainLifecycleRequest mutateInstance(ExplainLifecycleRequest instance)
         default:
             throw new AssertionError("Illegal randomisation branch");
         }
-        ExplainLifecycleRequest newRequest = new ExplainLifecycleRequest();
-        newRequest.indices(indices);
+        ExplainLifecycleRequest newRequest = new ExplainLifecycleRequest(indices);
         newRequest.indicesOptions(indicesOptions);
         return newRequest;
     }
 
     private ExplainLifecycleRequest copy(ExplainLifecycleRequest original) {
-        ExplainLifecycleRequest copy = new ExplainLifecycleRequest();
-        copy.indices(original.indices());
+        ExplainLifecycleRequest copy = new ExplainLifecycleRequest(original.getIndices());
         copy.indicesOptions(original.indicesOptions());
         return copy;
     }