Remove UpdateSettingsTestHelper class (#32557)

* Remove UpdateSettingsTestHelper class

By making the `settings()` method public on `UpdateSettingsRequest` (I think it
should have been in the first place) we can get rid of this class entirely. Mock
response objects are now constructed by parsing JSON without making the
constructor public.

Relates to #29823

server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java

@@ -88,7 +88,7 @@ public class UpdateSettingsRequest extends AcknowledgedRequest<UpdateSettingsReq
         return indices;
     }
 
-    Settings settings() {
+    public Settings settings() {
         return settings;
     }
 

server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsResponse.java

@@ -30,7 +30,7 @@ public class UpdateSettingsResponse extends AcknowledgedResponse {
     UpdateSettingsResponse() {
     }
 
-    UpdateSettingsResponse(boolean acknowledged) {
+    public UpdateSettingsResponse(boolean acknowledged) {
         super(acknowledged);
     }
 

x-pack/plugin/core/src/test/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsTestHelper.java

@@ -1,48 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License;
- * you may not use this file except in compliance with the Elastic License.
- *
- */
-package org.elasticsearch.action.admin.indices.settings.put;
-
-    import org.elasticsearch.common.settings.Settings;
-
-import java.util.Set;
-import java.util.stream.Collectors;
-
-import static org.hamcrest.Matchers.anyOf;
-import static org.hamcrest.Matchers.equalTo;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThat;
-
-public final class UpdateSettingsTestHelper {
-
-    // NORELEASE this isn't nice but it's currently the only way to inspect the
-    // settings in an update settings request. Need to see if we can make the
-    // getter public in ES
-    public static void assertSettingsRequest(UpdateSettingsRequest request, Settings expectedSettings, String... expectedIndices) {
-        assertNotNull(request);
-        assertArrayEquals(expectedIndices, request.indices());
-        assertEquals(expectedSettings, request.settings());
-    }
-
-    public static void assertSettingsRequestContainsValueFrom(UpdateSettingsRequest request, String settingsKey,
-            Set<String> acceptableValues, boolean assertOnlyKeyInSettings, String... expectedIndices) {
-        assertNotNull(request);
-        assertArrayEquals(expectedIndices, request.indices());
-        assertThat(request.settings().get(settingsKey), anyOf(acceptableValues.stream().map(e -> equalTo(e)).collect(Collectors.toList())));
-        if (assertOnlyKeyInSettings) {
-            assertEquals(1, request.settings().size());
-        }
-    }
-
-    // NORELEASE this isn't nice but it's currently the only way to create an
-    // UpdateSettingsResponse. Need to see if we can make the constructor public
-    // in ES
-    public static UpdateSettingsResponse createMockResponse(boolean acknowledged) {
-        return new UpdateSettingsResponse(acknowledged);
-    }
-}

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStepTests.java

@@ -10,7 +10,6 @@ import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest;
 import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsResponse;
-import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsTestHelper;
 import org.elasticsearch.client.AdminClient;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.client.IndicesAdminClient;
@@ -39,8 +38,13 @@ import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
+import java.io.IOException;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.stream.Collectors;
+
+import static org.hamcrest.Matchers.anyOf;
+import static org.hamcrest.Matchers.equalTo;
 
 public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase<SetSingleNodeAllocateStep> {
 
@@ -80,7 +84,18 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase<SetSing
         return new SetSingleNodeAllocateStep(instance.getKey(), instance.getNextStepKey(), client);
     }
 
-    public void testPerformActionNoAttrs() {
+    public static void assertSettingsRequestContainsValueFrom(UpdateSettingsRequest request, String settingsKey,
+                                                              Set<String> acceptableValues, boolean assertOnlyKeyInSettings,
+                                                              String... expectedIndices) {
+        assertNotNull(request);
+        assertArrayEquals(expectedIndices, request.indices());
+        assertThat(request.settings().get(settingsKey), anyOf(acceptableValues.stream().map(e -> equalTo(e)).collect(Collectors.toList())));
+        if (assertOnlyKeyInSettings) {
+            assertEquals(1, request.settings().size());
+        }
+    }
+
+    public void testPerformActionNoAttrs() throws IOException {
         IndexMetaData indexMetaData = IndexMetaData.builder(randomAlphaOfLength(10)).settings(settings(Version.CURRENT))
                 .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5)).build();
         Index index = indexMetaData.getIndex();
@@ -101,7 +116,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase<SetSing
         assertNodeSelected(indexMetaData, index, validNodeNames, nodes);
     }
 
-    public void testPerformActionAttrsAllNodesValid() {
+    public void testPerformActionAttrsAllNodesValid() throws IOException {
         int numAttrs = randomIntBetween(1, 10);
         String[][] validAttrs = new String[numAttrs][2];
         for (int i = 0; i < numAttrs; i++) {
@@ -132,7 +147,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase<SetSing
         assertNodeSelected(indexMetaData, index, validNodeNames, nodes);
     }
 
-    public void testPerformActionAttrsSomeNodesValid() {
+    public void testPerformActionAttrsSomeNodesValid() throws IOException {
         String[] validAttr = new String[] { "box_type", "valid" };
         String[] invalidAttr = new String[] { "box_type", "not_valid" };
         Settings.Builder indexSettings = settings(Version.CURRENT);
@@ -237,7 +252,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase<SetSing
                 UpdateSettingsRequest request = (UpdateSettingsRequest) invocation.getArguments()[0];
                 @SuppressWarnings("unchecked")
                 ActionListener<UpdateSettingsResponse> listener = (ActionListener<UpdateSettingsResponse>) invocation.getArguments()[1];
-                UpdateSettingsTestHelper.assertSettingsRequestContainsValueFrom(request,
+                assertSettingsRequestContainsValueFrom(request,
                         IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_name", validNodeNames, true,
                         indexMetaData.getIndex().getName());
                 listener.onFailure(exception);
@@ -325,7 +340,8 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase<SetSing
         Mockito.verifyZeroInteractions(client);
     }
 
-    private void assertNodeSelected(IndexMetaData indexMetaData, Index index, Set<String> validNodeNames, DiscoveryNodes.Builder nodes) {
+    private void assertNodeSelected(IndexMetaData indexMetaData, Index index,
+                                    Set<String> validNodeNames, DiscoveryNodes.Builder nodes) throws IOException {
         ImmutableOpenMap.Builder<String, IndexMetaData> indices = ImmutableOpenMap.<String, IndexMetaData> builder().fPut(index.getName(),
                 indexMetaData);
         IndexRoutingTable.Builder indexRoutingTable = IndexRoutingTable.builder(index)
@@ -340,6 +356,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase<SetSing
 
         Mockito.when(client.admin()).thenReturn(adminClient);
         Mockito.when(adminClient.indices()).thenReturn(indicesClient);
+
         Mockito.doAnswer(new Answer<Void>() {
 
             @Override
@@ -347,10 +364,10 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase<SetSing
                 UpdateSettingsRequest request = (UpdateSettingsRequest) invocation.getArguments()[0];
                 @SuppressWarnings("unchecked")
                 ActionListener<UpdateSettingsResponse> listener = (ActionListener<UpdateSettingsResponse>) invocation.getArguments()[1];
-                UpdateSettingsTestHelper.assertSettingsRequestContainsValueFrom(request,
+                assertSettingsRequestContainsValueFrom(request,
                         IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_name", validNodeNames, true,
                         indexMetaData.getIndex().getName());
-                listener.onResponse(UpdateSettingsTestHelper.createMockResponse(true));
+                listener.onResponse(new UpdateSettingsResponse(true));
                 return null;
             }
 

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateSettingsStepTests.java

@@ -11,7 +11,6 @@ import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest;
 import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsResponse;
-import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsTestHelper;
 import org.elasticsearch.client.AdminClient;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.client.IndicesAdminClient;
@@ -24,6 +23,8 @@ import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
+import static org.hamcrest.Matchers.equalTo;
+
 public class UpdateSettingsStepTests extends AbstractStepTestCase<UpdateSettingsStep> {
 
     private Client client;
@@ -70,7 +71,7 @@ public class UpdateSettingsStepTests extends AbstractStepTestCase<UpdateSettings
         return new UpdateSettingsStep(instance.getKey(), instance.getNextStepKey(), instance.getClient(), instance.getSettings());
     }
 
-    public void testPerformAction() {
+    public void testPerformAction() throws Exception {
         IndexMetaData indexMetaData = IndexMetaData.builder(randomAlphaOfLength(10)).settings(settings(Version.CURRENT))
             .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5)).build();
 
@@ -81,6 +82,7 @@ public class UpdateSettingsStepTests extends AbstractStepTestCase<UpdateSettings
 
         Mockito.when(client.admin()).thenReturn(adminClient);
         Mockito.when(adminClient.indices()).thenReturn(indicesClient);
+
         Mockito.doAnswer(new Answer<Void>() {
 
             @Override
@@ -88,8 +90,9 @@ public class UpdateSettingsStepTests extends AbstractStepTestCase<UpdateSettings
                 UpdateSettingsRequest request = (UpdateSettingsRequest) invocation.getArguments()[0];
                 @SuppressWarnings("unchecked")
                 ActionListener<UpdateSettingsResponse> listener = (ActionListener<UpdateSettingsResponse>) invocation.getArguments()[1];
-                UpdateSettingsTestHelper.assertSettingsRequest(request, step.getSettings(), indexMetaData.getIndex().getName());
+                assertThat(request.settings(), equalTo(step.getSettings()));
-                listener.onResponse(UpdateSettingsTestHelper.createMockResponse(true));
+                assertThat(request.indices(), equalTo(new String[] {indexMetaData.getIndex().getName()}));
+                listener.onResponse(new UpdateSettingsResponse(true));
                 return null;
             }
 
@@ -135,7 +138,8 @@ public class UpdateSettingsStepTests extends AbstractStepTestCase<UpdateSettings
                 UpdateSettingsRequest request = (UpdateSettingsRequest) invocation.getArguments()[0];
                 @SuppressWarnings("unchecked")
                 ActionListener<UpdateSettingsResponse> listener = (ActionListener<UpdateSettingsResponse>) invocation.getArguments()[1];
-                UpdateSettingsTestHelper.assertSettingsRequest(request, step.getSettings(), indexMetaData.getIndex().getName());
+                assertThat(request.settings(), equalTo(step.getSettings()));
+                assertThat(request.indices(), equalTo(new String[] {indexMetaData.getIndex().getName()}));
                 listener.onFailure(exception);
                 return null;
             }