Internal: deduplicate useful headers that get copied from REST to tra…

…nsport requests

The useful headers are now stored into a `Set` instead of an array so we can easily deduplicate them. A set is also returned instead of an array by the `usefulHeaders` static getter.

Relates to #6513

Closes #7590

src/main/java/org/elasticsearch/rest/BaseRestHandler.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.rest;
 
+import com.google.common.collect.Sets;
 import org.elasticsearch.action.*;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.client.ClusterAdminClient;
@@ -27,31 +28,28 @@
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.settings.Settings;
 
+import java.util.Collections;
+import java.util.Set;
+
 /**
  * Base handler for REST requests
  */
 public abstract class BaseRestHandler extends AbstractComponent implements RestHandler {
 
-    // non volatile since the assumption is that useful headers are registered on startup
-    private static String[] usefulHeaders = new String[0];
+    private static Set<String> usefulHeaders = Sets.newCopyOnWriteArraySet();
 
     /**
      * Controls which REST headers get copied over from a {@link org.elasticsearch.rest.RestRequest} to
      * its corresponding {@link org.elasticsearch.transport.TransportRequest}(s).
      *
      * By default no headers get copied but it is possible to extend this behaviour via plugins by calling this method.
      */
-    public static synchronized void addUsefulHeaders(String... headers) {
-        String[] copy = new String[usefulHeaders.length + headers.length];
-        System.arraycopy(usefulHeaders, 0, copy, 0 , usefulHeaders.length);
-        System.arraycopy(headers, 0, copy, usefulHeaders.length, headers.length);
-        usefulHeaders = copy;
+    public static void addUsefulHeaders(String... headers) {
+        Collections.addAll(usefulHeaders, headers);
     }
 
-    static String[] usefulHeaders() {
-        String[] copy = new String[usefulHeaders.length];
-        System.arraycopy(usefulHeaders, 0, copy, 0 , usefulHeaders.length);
-        return copy;
+    static Set<String> usefulHeaders() {
+        return usefulHeaders;
     }
 
     private final Client client;
@@ -63,19 +61,19 @@ protected BaseRestHandler(Settings settings, Client client) {
 
     @Override
     public final void handleRequest(RestRequest request, RestChannel channel) throws Exception {
-        handleRequest(request, channel, usefulHeaders.length == 0 ? client : new HeadersCopyClient(client, request, usefulHeaders));
+        handleRequest(request, channel, usefulHeaders.size() == 0 ? client : new HeadersCopyClient(client, request, usefulHeaders));
     }
 
     protected abstract void handleRequest(RestRequest request, RestChannel channel, Client client) throws Exception;
 
     static final class HeadersCopyClient extends FilterClient {
 
         private final RestRequest restRequest;
-        private final String[] usefulHeaders;
+        private final Set<String> usefulHeaders;
         private final IndicesAdmin indicesAdmin;
         private final ClusterAdmin clusterAdmin;
 
-        HeadersCopyClient(Client in, RestRequest restRequest, String[] usefulHeaders) {
+        HeadersCopyClient(Client in, RestRequest restRequest, Set<String> usefulHeaders) {
             super(in);
             this.restRequest = restRequest;
             this.usefulHeaders = usefulHeaders;

src/test/java/org/elasticsearch/rest/HeadersCopyClientTests.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.rest;
 
-import com.google.common.collect.Lists;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.*;
 import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest;
@@ -54,35 +53,38 @@ public class HeadersCopyClientTests extends ElasticsearchTestCase {
     @Test
     public void testAddUsefulHeaders() throws InterruptedException {
         //take the existing headers into account to make sure this test runs with tests.iters>1 as the list is static
-        final List<String> headers = Lists.newArrayList(BaseRestHandler.usefulHeaders());
+        final Set<String> headers = new HashSet<>();
+        headers.addAll(BaseRestHandler.usefulHeaders());
         int iterations = randomIntBetween(1, 5);
 
         ExecutorService executorService = Executors.newFixedThreadPool(iterations);
         for (int i = 0; i < iterations; i++) {
             int headersCount = randomInt(10);
-            final String[] newHeaders = new String[headersCount];
+            final Set<String> newHeaders = new HashSet<>();
             for (int j = 0; j < headersCount; j++) {
                 String usefulHeader = randomRealisticUnicodeOfLengthBetween(1, 30);
-                newHeaders[j] = usefulHeader;
+                newHeaders.add(usefulHeader);
                 headers.add(usefulHeader);
             }
 
             executorService.submit(new Runnable() {
                 @Override
                 public void run() {
-                    BaseRestHandler.addUsefulHeaders(newHeaders);
+                    BaseRestHandler.addUsefulHeaders(newHeaders.toArray(new String[newHeaders.size()]));
                 }
             });
         }
 
         executorService.shutdown();
         assertThat(executorService.awaitTermination(1, TimeUnit.SECONDS), equalTo(true));
-        String[] usefulHeaders = BaseRestHandler.usefulHeaders();
+        String[] usefulHeaders = BaseRestHandler.usefulHeaders().toArray(new String[BaseRestHandler.usefulHeaders().size()]);
         assertThat(usefulHeaders.length, equalTo(headers.size()));
 
         Arrays.sort(usefulHeaders);
-        Collections.sort(headers);
-        assertThat(usefulHeaders, equalTo(headers.toArray(new String[headers.size()])));
+        String[] headersArray = new String[headers.size()];
+        headersArray = headers.toArray(headersArray);
+        Arrays.sort(headersArray);
+        assertThat(usefulHeaders, equalTo(headersArray));
     }
 
     @Test
@@ -310,7 +312,7 @@ private static Client client(Client noOpClient, RestRequest restRequest, Set<Str
         if (usefulRestHeaders.isEmpty() && randomBoolean()) {
             return noOpClient;
         }
-        return new HeadersCopyClient(noOpClient, restRequest, usefulRestHeaders.toArray(new String[usefulRestHeaders.size()]));
+        return new HeadersCopyClient(noOpClient, restRequest, usefulRestHeaders);
     }
 
     private static void putHeaders(ActionRequest<?> request, Map<String, String> headers) {