Tighter RestGlobalContext yet more

Removes more getters in favor of pushing the HeadersAndContextCopyClient into the global context. BaseRestHandler is now very very simple.
elastic · Jan 12, 2016 · 1ef2eac · 1ef2eac
1 parent 16edb4c
commit 1ef2eac
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 59 deletions.
diff --git a/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java b/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java
@@ -19,15 +19,7 @@
 
 package org.elasticsearch.rest;
 
-import java.util.Set;
-
-import org.elasticsearch.action.Action;
-import org.elasticsearch.action.ActionListener;
-import org.elasticsearch.action.ActionRequest;
-import org.elasticsearch.action.ActionRequestBuilder;
-import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.client.Client;
-import org.elasticsearch.client.FilterClient;
 import org.elasticsearch.common.ParseFieldMatcher;
 import org.elasticsearch.common.component.AbstractComponent;
 
@@ -52,36 +44,8 @@ protected BaseRestHandler(RestGlobalContext context) {
 
     @Override
     public final void handleRequest(RestRequest request, RestChannel channel) throws Exception {
-        handleRequest(request, channel, new HeadersAndContextCopyClient(context.getClient(), request, context.relevantHeaders()));
+        handleRequest(request, channel, context.createClient(request));
     }
 
     protected abstract void handleRequest(RestRequest request, RestChannel channel, Client client) throws Exception;
-
-    static final class HeadersAndContextCopyClient extends FilterClient {
-
-        private final RestRequest restRequest;
-        private final Set<String> headers;
-
-        HeadersAndContextCopyClient(Client in, RestRequest restRequest, Set<String> headers) {
-            super(in);
-            this.restRequest = restRequest;
-            this.headers = headers;
-        }
-
-        private static void copyHeadersAndContext(ActionRequest<?> actionRequest, RestRequest restRequest, Set<String> headers) {
-            for (String usefulHeader : headers) {
-                String headerValue = restRequest.header(usefulHeader);
-                if (headerValue != null) {
-                    actionRequest.putHeader(usefulHeader, headerValue);
-                }
-            }
-            actionRequest.copyContextFrom(restRequest);
-        }
-
-        @Override
-        protected <Request extends ActionRequest, Response extends ActionResponse, RequestBuilder extends ActionRequestBuilder<Request, Response, RequestBuilder>> void doExecute(Action<Request, Response, RequestBuilder> action, Request request, ActionListener<Response> listener) {
-            copyHeadersAndContext(request, restRequest, headers);
-            super.doExecute(action, request, listener);
-        }
-    }
 }
diff --git a/core/src/main/java/org/elasticsearch/rest/RestGlobalContext.java b/core/src/main/java/org/elasticsearch/rest/RestGlobalContext.java
@@ -21,7 +21,13 @@
 
 import java.util.Set;
 
+import org.elasticsearch.action.Action;
+import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.ActionRequest;
+import org.elasticsearch.action.ActionRequestBuilder;
+import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.client.Client;
+import org.elasticsearch.client.FilterClient;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.indices.query.IndicesQueriesRegistry;
 
@@ -47,22 +53,43 @@ public Settings getSettings() {
     }
 
     /**
-     * Returns the REST headers that 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
-     * {@link RestController#registerRelevantHeaders(String...)}.
+     * Create the client to be used to process a request. This client will copy
+     * headers from the rest request into the internal requests but otherwise
+     * simply wraps a globally shared client.
      */
-    public Set<String> relevantHeaders() {
-        return controller.relevantHeaders();
-    }
-
-    public Client getClient() {
-        return client;
+    public Client createClient(RestRequest request) {
+        return new HeadersAndContextCopyClient(client, request, controller.relevantHeaders());
     }
 
     public IndicesQueriesRegistry getIndicesQueriesRegistry() {
         return indicesQueriesRegistry;
     }
+
+    static final class HeadersAndContextCopyClient extends FilterClient {
+
+        private final RestRequest restRequest;
+        private final Set<String> headers;
+
+        HeadersAndContextCopyClient(Client in, RestRequest restRequest, Set<String> headers) {
+            super(in);
+            this.restRequest = restRequest;
+            this.headers = headers;
+        }
+
+        private static void copyHeadersAndContext(ActionRequest<?> actionRequest, RestRequest restRequest, Set<String> headers) {
+            for (String usefulHeader : headers) {
+                String headerValue = restRequest.header(usefulHeader);
+                if (headerValue != null) {
+                    actionRequest.putHeader(usefulHeader, headerValue);
+                }
+            }
+            actionRequest.copyContextFrom(restRequest);
+        }
+
+        @Override
+        protected <Request extends ActionRequest, Response extends ActionResponse, RequestBuilder extends ActionRequestBuilder<Request, Response, RequestBuilder>> void doExecute(Action<Request, Response, RequestBuilder> action, Request request, ActionListener<Response> listener) {
+            copyHeadersAndContext(request, restRequest, headers);
+            super.doExecute(action, request, listener);
+        }
+    }
 }
diff --git a/core/src/test/java/org/elasticsearch/rest/HeadersAndContextCopyClientTests.java b/core/src/test/java/org/elasticsearch/rest/HeadersAndContextCopyClientTests.java
@@ -19,6 +19,15 @@
 
 package org.elasticsearch.rest;
 
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionRequestBuilder;
 import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest;
@@ -34,18 +43,10 @@
 import org.elasticsearch.client.Requests;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.rest.RestGlobalContext.HeadersAndContextCopyClient;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.rest.FakeRestRequest;
 
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
-
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.Matchers.is;
@@ -373,7 +374,7 @@ private static Map<String, String> randomHeadersFrom(Map<String, String> headers
     }
 
     private static Client client(Client noOpClient, RestRequest restRequest, Set<String> usefulRestHeaders) {
-        return new BaseRestHandler.HeadersAndContextCopyClient(noOpClient, restRequest, usefulRestHeaders);
+        return new HeadersAndContextCopyClient(noOpClient, restRequest, usefulRestHeaders);
     }
 
     private static void putHeaders(ActionRequest<?> request, Map<String, String> headers) {