Turn RestChannel into an interface

In #17133 we introduce request size limit handling and need a custom
channel implementation. In order to ensure we delegate all methods
it is better to have this channel implement an interface instead of
an abstract base class (so changes on the interface turn into
compile errors).

Relates #17133

core/src/main/java/org/elasticsearch/http/HttpChannel.java

@@ -1,33 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.http;
-
-import org.elasticsearch.rest.RestChannel;
-import org.elasticsearch.rest.RestRequest;
-
-/**
- *
- */
-public abstract class HttpChannel extends RestChannel {
-
-    protected HttpChannel(RestRequest request, boolean detailedErrorsEnabled) {
-        super(request, detailedErrorsEnabled);
-    }
-}

core/src/main/java/org/elasticsearch/http/HttpRequest.java

@@ -1,29 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.http;
-
-import org.elasticsearch.rest.RestRequest;
-
-/**
- *
- */
-public abstract class HttpRequest extends RestRequest {
-
-}

core/src/main/java/org/elasticsearch/http/HttpServer.java

@@ -27,6 +27,7 @@ import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.node.service.NodeService;
 import org.elasticsearch.rest.BytesRestResponse;
+import org.elasticsearch.rest.RestChannel;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.RestStatus;
@@ -93,7 +94,7 @@ public class HttpServer extends AbstractLifecycleComponent<HttpServer> implement
         return transport.stats();
     }
 
-    public void dispatchRequest(HttpRequest request, HttpChannel channel, ThreadContext threadContext) {
+    public void dispatchRequest(RestRequest request, RestChannel channel, ThreadContext threadContext) {
         if (request.rawPath().equals("/favicon.ico")) {
             handleFavicon(request, channel);
             return;
@@ -101,7 +102,7 @@ public class HttpServer extends AbstractLifecycleComponent<HttpServer> implement
         restController.dispatchRequest(request, channel, threadContext);
     }
 
-    void handleFavicon(HttpRequest request, HttpChannel channel) {
+    void handleFavicon(RestRequest request, RestChannel channel) {
         if (request.method() == RestRequest.Method.GET) {
             try {
                 try (InputStream stream = getClass().getResourceAsStream("/config/favicon.ico")) {

core/src/main/java/org/elasticsearch/http/HttpServerAdapter.java

@@ -20,11 +20,13 @@
 package org.elasticsearch.http;
 
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.rest.RestChannel;
+import org.elasticsearch.rest.RestRequest;
 
 /**
  *
  */
 public interface HttpServerAdapter {
 
-    void dispatchRequest(HttpRequest request, HttpChannel channel, ThreadContext context);
+    void dispatchRequest(RestRequest request, RestChannel channel, ThreadContext context);
 }

core/src/main/java/org/elasticsearch/http/netty/NettyHttpChannel.java

@@ -24,10 +24,10 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.ReleasableBytesStreamOutput;
 import org.elasticsearch.common.lease.Releasable;
 import org.elasticsearch.common.netty.ReleaseChannelFutureListener;
-import org.elasticsearch.http.HttpChannel;
 import org.elasticsearch.http.netty.cors.CorsHandler;
 import org.elasticsearch.http.netty.pipelining.OrderedDownstreamChannelEvent;
 import org.elasticsearch.http.netty.pipelining.OrderedUpstreamMessageEvent;
+import org.elasticsearch.rest.AbstractRestChannel;
 import org.elasticsearch.rest.RestResponse;
 import org.elasticsearch.rest.RestStatus;
 import org.jboss.netty.buffer.ChannelBuffer;
@@ -53,7 +53,7 @@ import static org.jboss.netty.handler.codec.http.HttpHeaders.Names.CONNECTION;
 import static org.jboss.netty.handler.codec.http.HttpHeaders.Values.CLOSE;
 import static org.jboss.netty.handler.codec.http.HttpHeaders.Values.KEEP_ALIVE;
 
-public final class NettyHttpChannel extends HttpChannel {
+public final class NettyHttpChannel extends AbstractRestChannel {
 
     private final NettyHttpServerTransport transport;
     private final Channel channel;

core/src/main/java/org/elasticsearch/http/netty/NettyHttpRequest.java

@@ -22,7 +22,7 @@ package org.elasticsearch.http.netty;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.bytes.ChannelBufferBytesReference;
-import org.elasticsearch.http.HttpRequest;
+import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.support.RestUtils;
 import org.jboss.netty.channel.Channel;
 import org.jboss.netty.handler.codec.http.HttpMethod;
@@ -34,7 +34,7 @@ import java.util.Map;
 /**
  *
  */
-public class NettyHttpRequest extends HttpRequest {
+public class NettyHttpRequest extends RestRequest {
 
     private final org.jboss.netty.handler.codec.http.HttpRequest request;
     private final Channel channel;

core/src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java

@@ -42,9 +42,7 @@ import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.common.util.concurrent.EsExecutors;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.http.BindHttpException;
-import org.elasticsearch.http.HttpChannel;
 import org.elasticsearch.http.HttpInfo;
-import org.elasticsearch.http.HttpRequest;
 import org.elasticsearch.http.HttpServerAdapter;
 import org.elasticsearch.http.HttpServerTransport;
 import org.elasticsearch.http.HttpStats;
@@ -53,6 +51,8 @@ import org.elasticsearch.http.netty.cors.CorsConfigBuilder;
 import org.elasticsearch.http.netty.cors.CorsHandler;
 import org.elasticsearch.http.netty.pipelining.HttpPipeliningHandler;
 import org.elasticsearch.monitor.jvm.JvmInfo;
+import org.elasticsearch.rest.RestChannel;
+import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.support.RestUtils;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.BindTransportException;
@@ -483,7 +483,7 @@ public class NettyHttpServerTransport extends AbstractLifecycleComponent<HttpSer
         return corsConfig;
     }
 
-    protected void dispatchRequest(HttpRequest request, HttpChannel channel) {
+    protected void dispatchRequest(RestRequest request, RestChannel channel) {
         httpServerAdapter.dispatchRequest(request, channel, threadPool.getThreadContext());
     }
 

core/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java

@@ -0,0 +1,113 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.elasticsearch.rest;
+
+import org.elasticsearch.common.Nullable;
+import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.common.xcontent.XContentType;
+
+import java.io.IOException;
+
+public abstract class AbstractRestChannel implements RestChannel {
+
+    protected final RestRequest request;
+    protected final boolean detailedErrorsEnabled;
+
+    private BytesStreamOutput bytesOut;
+
+    protected AbstractRestChannel(RestRequest request, boolean detailedErrorsEnabled) {
+        this.request = request;
+        this.detailedErrorsEnabled = detailedErrorsEnabled;
+    }
+
+    @Override
+    public XContentBuilder newBuilder() throws IOException {
+        return newBuilder(request.hasContent() ? request.content() : null, request.hasParam("filter_path"));
+    }
+
+    @Override
+    public XContentBuilder newErrorBuilder() throws IOException {
+        // Disable filtering when building error responses
+        return newBuilder(request.hasContent() ? request.content() : null, false);
+    }
+
+    @Override
+    public XContentBuilder newBuilder(@Nullable BytesReference autoDetectSource, boolean useFiltering) throws IOException {
+        XContentType contentType = XContentType.fromMediaTypeOrFormat(request.param("format", request.header("Accept")));
+        if (contentType == null) {
+            // try and guess it from the auto detect source
+            if (autoDetectSource != null) {
+                contentType = XContentFactory.xContentType(autoDetectSource);
+            }
+        }
+        if (contentType == null) {
+            // default to JSON
+            contentType = XContentType.JSON;
+        }
+
+        String[] filters = useFiltering ? request.paramAsStringArrayOrEmptyIfAll("filter_path") :  null;
+        XContentBuilder builder = new XContentBuilder(XContentFactory.xContent(contentType), bytesOutput(), filters);
+        if (request.paramAsBoolean("pretty", false)) {
+            builder.prettyPrint().lfAtEnd();
+        }
+
+        builder.humanReadable(request.paramAsBoolean("human", builder.humanReadable()));
+
+        String casing = request.param("case");
+        if (casing != null && "camelCase".equals(casing)) {
+            builder.fieldCaseConversion(XContentBuilder.FieldCaseConversion.CAMELCASE);
+        } else {
+            // we expect all REST interfaces to write results in underscore casing, so
+            // no need for double casing
+            builder.fieldCaseConversion(XContentBuilder.FieldCaseConversion.NONE);
+        }
+        return builder;
+    }
+
+    /**
+     * A channel level bytes output that can be reused. It gets reset on each call to this
+     * method.
+     */
+    @Override
+    public final BytesStreamOutput bytesOutput() {
+        if (bytesOut == null) {
+            bytesOut = newBytesOutput();
+        } else {
+            bytesOut.reset();
+        }
+        return bytesOut;
+    }
+
+    protected BytesStreamOutput newBytesOutput() {
+        return new BytesStreamOutput();
+    }
+
+    @Override
+    public RestRequest request() {
+        return this.request;
+    }
+
+    @Override
+    public boolean detailedErrorsEnabled() {
+        return detailedErrorsEnabled;
+    }
+}

core/src/main/java/org/elasticsearch/rest/RestChannel.java

@@ -23,91 +23,27 @@ import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.common.xcontent.XContentFactory;
-import org.elasticsearch.common.xcontent.XContentType;
 
 import java.io.IOException;
 
 /**
  * A channel used to construct bytes / builder based outputs, and send responses.
  */
-public abstract class RestChannel {
+public interface RestChannel {
+    XContentBuilder newBuilder() throws IOException;
 
-    protected final RestRequest request;
-    protected final boolean detailedErrorsEnabled;
+    XContentBuilder newErrorBuilder() throws IOException;
 
-    private BytesStreamOutput bytesOut;
+    XContentBuilder newBuilder(@Nullable BytesReference autoDetectSource, boolean useFiltering) throws IOException;
 
-    protected RestChannel(RestRequest request, boolean detailedErrorsEnabled) {
-        this.request = request;
-        this.detailedErrorsEnabled = detailedErrorsEnabled;
-    }
+    BytesStreamOutput bytesOutput();
 
-    public XContentBuilder newBuilder() throws IOException {
-        return newBuilder(request.hasContent() ? request.content() : null, request.hasParam("filter_path"));
-    }
-
-    public XContentBuilder newErrorBuilder() throws IOException {
-        // Disable filtering when building error responses
-        return newBuilder(request.hasContent() ? request.content() : null, false);
-    }
-
-    public XContentBuilder newBuilder(@Nullable BytesReference autoDetectSource, boolean useFiltering) throws IOException {
-        XContentType contentType = XContentType.fromMediaTypeOrFormat(request.param("format", request.header("Accept")));
-        if (contentType == null) {
-            // try and guess it from the auto detect source
-            if (autoDetectSource != null) {
-                contentType = XContentFactory.xContentType(autoDetectSource);
-            }
-        }
-        if (contentType == null) {
-            // default to JSON
-            contentType = XContentType.JSON;
-        }
-
-        String[] filters = useFiltering ? request.paramAsStringArrayOrEmptyIfAll("filter_path") :  null;
-        XContentBuilder builder = new XContentBuilder(XContentFactory.xContent(contentType), bytesOutput(), filters);
-        if (request.paramAsBoolean("pretty", false)) {
-            builder.prettyPrint().lfAtEnd();
-        }
-
-        builder.humanReadable(request.paramAsBoolean("human", builder.humanReadable()));
-
-        String casing = request.param("case");
-        if (casing != null && "camelCase".equals(casing)) {
-            builder.fieldCaseConversion(XContentBuilder.FieldCaseConversion.CAMELCASE);
-        } else {
-            // we expect all REST interfaces to write results in underscore casing, so
-            // no need for double casing
-            builder.fieldCaseConversion(XContentBuilder.FieldCaseConversion.NONE);
-        }
-        return builder;
-    }
+    RestRequest request();
 
     /**
-     * A channel level bytes output that can be reused. It gets reset on each call to this
-     * method.
+     * @return true iff an error response should contain additional details like exception traces.
      */
-    public final BytesStreamOutput bytesOutput() {
-        if (bytesOut == null) {
-            bytesOut = newBytesOutput();
-        } else {
-            bytesOut.reset();
-        }
-        return bytesOut;
-    }
-
-    protected BytesStreamOutput newBytesOutput() {
-        return new BytesStreamOutput();
-    }
-
-    public RestRequest request() {
-        return this.request;
-    }
-
-    public boolean detailedErrorsEnabled() {
-        return detailedErrorsEnabled;
-    }
+    boolean detailedErrorsEnabled();
 
-    public abstract void sendResponse(RestResponse response);
-}
+    void sendResponse(RestResponse response);
+}

core/src/test/java/org/elasticsearch/rest/BytesRestResponseTests.java

@@ -156,7 +156,7 @@ public class BytesRestResponseTests extends ESTestCase {
         }
     }
 
-    private static class SimpleExceptionRestChannel extends RestChannel {
+    private static class SimpleExceptionRestChannel extends AbstractRestChannel {
 
         SimpleExceptionRestChannel(RestRequest request) {
             super(request, false);
@@ -167,7 +167,7 @@ public class BytesRestResponseTests extends ESTestCase {
         }
     }
 
-    private static class DetailedExceptionRestChannel extends RestChannel {
+    private static class DetailedExceptionRestChannel extends AbstractRestChannel {
 
         DetailedExceptionRestChannel(RestRequest request) {
             super(request, true);

core/src/test/java/org/elasticsearch/rest/RestFilterChainTests.java

@@ -152,7 +152,7 @@ public class RestFilterChainTests extends ESTestCase {
         assertThat(fakeRestChannel.errors.get(), equalTo(additionalContinueCount));
     }
 
-    private static class FakeRestChannel extends RestChannel {
+    private static class FakeRestChannel extends AbstractRestChannel {
 
         private final CountDownLatch latch;
         AtomicInteger responses = new AtomicInteger();

core/src/test/java/org/elasticsearch/rest/action/support/RestTableTests.java

@@ -21,7 +21,7 @@ package org.elasticsearch.rest.action.support;
 
 import org.elasticsearch.common.Table;
 import org.elasticsearch.common.xcontent.XContentType;
-import org.elasticsearch.rest.RestChannel;
+import org.elasticsearch.rest.AbstractRestChannel;
 import org.elasticsearch.rest.RestResponse;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.rest.FakeRestRequest;
@@ -139,7 +139,7 @@ public class RestTableTests extends ESTestCase {
         table.addCell("foo");
         table.addCell("foo");
         table.endRow();
-        RestResponse response = buildResponse(table, new RestChannel(requestWithAcceptHeader, true) {
+        RestResponse response = buildResponse(table, new AbstractRestChannel(requestWithAcceptHeader, true) {
             @Override
             public void sendResponse(RestResponse response) {
             }

core/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java

@@ -50,7 +50,7 @@ import org.elasticsearch.indices.ttl.IndicesTTLService;
 import org.elasticsearch.node.Node;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.repositories.RepositoryMissingException;
-import org.elasticsearch.rest.RestChannel;
+import org.elasticsearch.rest.AbstractRestChannel;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.RestResponse;
 import org.elasticsearch.rest.action.admin.cluster.repositories.get.RestGetRepositoriesAction;
@@ -640,7 +640,7 @@ public class DedicatedClusterSnapshotRestoreIT extends AbstractSnapshotIntegTest
         getRepoRequest.params().put("repository", "test-repo");
         final CountDownLatch getRepoLatch = new CountDownLatch(1);
         final AtomicReference<AssertionError> getRepoError = new AtomicReference<>();
-        getRepoAction.handleRequest(getRepoRequest, new RestChannel(getRepoRequest, true) {
+        getRepoAction.handleRequest(getRepoRequest, new AbstractRestChannel(getRepoRequest, true) {
             @Override
             public void sendResponse(RestResponse response) {
                 try {
@@ -661,7 +661,7 @@ public class DedicatedClusterSnapshotRestoreIT extends AbstractSnapshotIntegTest
         RestRequest clusterStateRequest = new FakeRestRequest();
         final CountDownLatch clusterStateLatch = new CountDownLatch(1);
         final AtomicReference<AssertionError> clusterStateError = new AtomicReference<>();
-        clusterStateAction.handleRequest(clusterStateRequest, new RestChannel(clusterStateRequest, true) {
+        clusterStateAction.handleRequest(clusterStateRequest, new AbstractRestChannel(clusterStateRequest, true) {
             @Override
             public void sendResponse(RestResponse response) {
                 try {