Make RestHighLevelClient's Request class public (#26627)

Request class is currently package protected, making it difficult for
the users to extend the RestHighLevelClient and to use its protected
methods to execute requests. This commit makes the Request class public
and changes few methods of RestHighLevelClient to be protected.

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

@@ -63,30 +63,47 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.StringJoiner;
 
-final class Request {
+public final class Request {
 
     static final XContentType REQUEST_BODY_CONTENT_TYPE = XContentType.JSON;
 
-    final String method;
-    final String endpoint;
-    final Map<String, String> params;
-    final HttpEntity entity;
+    private final String method;
+    private final String endpoint;
+    private final Map<String, String> parameters;
+    private final HttpEntity entity;
 
-    Request(String method, String endpoint, Map<String, String> params, HttpEntity entity) {
-        this.method = method;
-        this.endpoint = endpoint;
-        this.params = params;
+    public Request(String method, String endpoint, Map<String, String> parameters, HttpEntity entity) {
+        this.method = Objects.requireNonNull(method, "method cannot be null");
+        this.endpoint = Objects.requireNonNull(endpoint, "endpoint cannot be null");
+        this.parameters = Objects.requireNonNull(parameters, "parameters cannot be null");
         this.entity = entity;
     }
 
+    public String getMethod() {
+        return method;
+    }
+
+    public String getEndpoint() {
+        return endpoint;
+    }
+
+    public Map<String, String> getParameters() {
+        return parameters;
+    }
+
+    public HttpEntity getEntity() {
+        return entity;
+    }
+
     @Override
     public String toString() {
         return "Request{" +
                 "method='" + method + '\'' +
                 ", endpoint='" + endpoint + '\'' +
-                ", params=" + params +
+                ", params=" + parameters +
                 ", hasBody=" + (entity != null) +
                 '}';
     }
@@ -233,7 +250,7 @@ final class Request {
 
     static Request exists(GetRequest getRequest) {
         Request request = get(getRequest);
-        return new Request(HttpHead.METHOD_NAME, request.endpoint, request.params, null);
+        return new Request(HttpHead.METHOD_NAME, request.endpoint, request.parameters, null);
     }
 
     static Request get(GetRequest getRequest) {
@@ -381,7 +398,7 @@ final class Request {
      * @return the {@link ContentType}
      */
     @SuppressForbidden(reason = "Only allowed place to convert a XContentType to a ContentType")
-    static ContentType createContentType(final XContentType xContentType) {
+    public static ContentType createContentType(final XContentType xContentType) {
         return ContentType.create(xContentType.mediaTypeWithoutParameters(), (Charset) null);
     }
 

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

@@ -425,7 +425,7 @@ public class RestHighLevelClient implements Closeable {
         Request req = requestConverter.apply(request);
         Response response;
         try {
-            response = client.performRequest(req.method, req.endpoint, req.params, req.entity, headers);
+            response = client.performRequest(req.getMethod(), req.getEndpoint(), req.getParameters(), req.getEntity(), headers);
         } catch (ResponseException e) {
             if (ignores.contains(e.getResponse().getStatusLine().getStatusCode())) {
                 try {
@@ -474,7 +474,7 @@ public class RestHighLevelClient implements Closeable {
         }
 
         ResponseListener responseListener = wrapResponseListener(responseConverter, listener, ignores);
-        client.performRequestAsync(req.method, req.endpoint, req.params, req.entity, responseListener, headers);
+        client.performRequestAsync(req.getMethod(), req.getEndpoint(), req.getParameters(), req.getEntity(), responseListener, headers);
     }
 
     <Resp> ResponseListener wrapResponseListener(CheckedFunction<Response, Resp, IOException> responseConverter,
@@ -522,7 +522,7 @@ public class RestHighLevelClient implements Closeable {
      * that wraps the original {@link ResponseException}. The potential exception obtained while parsing is added to the returned
      * exception as a suppressed exception. This method is guaranteed to not throw any exception eventually thrown while parsing.
      */
-    ElasticsearchStatusException parseResponseException(ResponseException responseException) {
+    protected ElasticsearchStatusException parseResponseException(ResponseException responseException) {
         Response response = responseException.getResponse();
         HttpEntity entity = response.getEntity();
         ElasticsearchStatusException elasticsearchException;
@@ -542,8 +542,8 @@ public class RestHighLevelClient implements Closeable {
         return elasticsearchException;
     }
 
-    <Resp> Resp parseEntity(
-            HttpEntity entity, CheckedFunction<XContentParser, Resp, IOException> entityParser) throws IOException {
+    protected <Resp> Resp parseEntity(final HttpEntity entity,
+                                      final CheckedFunction<XContentParser, Resp, IOException> entityParser) throws IOException {
         if (entity == null) {
             throw new IllegalStateException("Response body expected but not returned");
         }

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

@@ -22,14 +22,12 @@ package org.elasticsearch.client;
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpHost;
-import org.apache.http.HttpResponse;
 import org.apache.http.ProtocolVersion;
 import org.apache.http.RequestLine;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.entity.ByteArrayEntity;
 import org.apache.http.entity.ContentType;
 import org.apache.http.message.BasicHeader;
-import org.apache.http.message.BasicHttpResponse;
 import org.apache.http.message.BasicRequestLine;
 import org.apache.http.message.BasicStatusLine;
 import org.apache.lucene.util.BytesRef;
@@ -38,6 +36,12 @@ import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.main.MainRequest;
 import org.elasticsearch.action.main.MainResponse;
+import org.elasticsearch.action.support.PlainActionFuture;
+import org.elasticsearch.client.Request;
+import org.elasticsearch.client.Response;
+import org.elasticsearch.client.ResponseListener;
+import org.elasticsearch.client.RestClient;
+import org.elasticsearch.client.RestHighLevelClient;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.xcontent.XContentHelper;
@@ -48,11 +52,14 @@ import org.junit.Before;
 import java.io.IOException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
-import static org.elasticsearch.client.ESRestHighLevelClientTestCase.execute;
+import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyMapOf;
 import static org.mockito.Matchers.anyObject;
@@ -60,6 +67,7 @@ import static org.mockito.Matchers.anyVararg;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 /**
  * Test and demonstrates how {@link RestHighLevelClient} can be extended to support custom endpoints.
@@ -92,31 +100,45 @@ public class CustomRestHighLevelClientTests extends ESTestCase {
         final MainRequest request = new MainRequest();
         final Header header = new BasicHeader("node_name", randomAlphaOfLengthBetween(1, 10));
 
-        MainResponse response = execute(request, restHighLevelClient::custom, restHighLevelClient::customAsync, header);
+        MainResponse response = restHighLevelClient.custom(request, header);
         assertEquals(header.getValue(), response.getNodeName());
 
-        response = execute(request, restHighLevelClient::customAndParse, restHighLevelClient::customAndParseAsync, header);
+        response = restHighLevelClient.customAndParse(request, header);
         assertEquals(header.getValue(), response.getNodeName());
     }
 
+    public void testCustomEndpointAsync() throws Exception {
+        final MainRequest request = new MainRequest();
+        final Header header = new BasicHeader("node_name", randomAlphaOfLengthBetween(1, 10));
+
+        PlainActionFuture<MainResponse> future = PlainActionFuture.newFuture();
+        restHighLevelClient.customAsync(request, future, header);
+        assertEquals(header.getValue(), future.get().getNodeName());
+
+        future = PlainActionFuture.newFuture();
+        restHighLevelClient.customAndParseAsync(request, future, header);
+        assertEquals(header.getValue(), future.get().getNodeName());
+    }
+
     /**
      * The {@link RestHighLevelClient} must declare the following execution methods using the <code>protected</code> modifier
      * so that they can be used by subclasses to implement custom logic.
      */
     @SuppressForbidden(reason = "We're forced to uses Class#getDeclaredMethods() here because this test checks protected methods")
     public void testMethodsVisibility() throws ClassNotFoundException {
-        String[] methodNames = new String[]{"performRequest", "performRequestAndParseEntity", "performRequestAsync",
-                "performRequestAsyncAndParseEntity"};
-        for (String methodName : methodNames) {
-            boolean found = false;
-            for (Method method : RestHighLevelClient.class.getDeclaredMethods()) {
-                if (method.getName().equals(methodName)) {
-                    assertTrue("Method " + methodName + " must be protected", Modifier.isProtected(method.getModifiers()));
-                    found = true;
-                }
-            }
-            assertTrue("Failed to find method " + methodName, found);
-        }
+        final String[] methodNames = new String[]{"performRequest",
+                                                  "performRequestAsync",
+                                                  "performRequestAndParseEntity",
+                                                  "performRequestAsyncAndParseEntity",
+                                                  "parseEntity",
+                                                  "parseResponseException"};
+
+        final List<String> protectedMethods =  Arrays.stream(RestHighLevelClient.class.getDeclaredMethods())
+                                                     .filter(method -> Modifier.isProtected(method.getModifiers()))
+                                                     .map(Method::getName)
+                                                     .collect(Collectors.toList());
+
+        assertThat(protectedMethods, containsInAnyOrder(methodNames));
     }
 
     /**
@@ -135,15 +157,20 @@ public class CustomRestHighLevelClientTests extends ESTestCase {
      * Mocks the synchronous request execution like if it was executed by Elasticsearch.
      */
     private Response mockPerformRequest(Header httpHeader) throws IOException {
+        final Response mockResponse = mock(Response.class);
+        when(mockResponse.getHost()).thenReturn(new HttpHost("localhost", 9200));
+
         ProtocolVersion protocol = new ProtocolVersion("HTTP", 1, 1);
-        HttpResponse httpResponse = new BasicHttpResponse(new BasicStatusLine(protocol, 200, "OK"));
+        when(mockResponse.getStatusLine()).thenReturn(new BasicStatusLine(protocol, 200, "OK"));
 
         MainResponse response = new MainResponse(httpHeader.getValue(), Version.CURRENT, ClusterName.DEFAULT, "_na", Build.CURRENT, true);
         BytesRef bytesRef = XContentHelper.toXContent(response, XContentType.JSON, false).toBytesRef();
-        httpResponse.setEntity(new ByteArrayEntity(bytesRef.bytes, ContentType.APPLICATION_JSON));
+        when(mockResponse.getEntity()).thenReturn(new ByteArrayEntity(bytesRef.bytes, ContentType.APPLICATION_JSON));
 
         RequestLine requestLine = new BasicRequestLine(HttpGet.METHOD_NAME, ENDPOINT, protocol);
-        return new Response(requestLine, new HttpHost("localhost", 9200), httpResponse);
+        when(mockResponse.getRequestLine()).thenReturn(requestLine);
+
+        return mockResponse;
     }
 
     /**

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

@@ -21,6 +21,8 @@ package org.elasticsearch.client;
 
 import org.apache.http.HttpEntity;
 import org.apache.http.entity.ByteArrayEntity;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
 import org.apache.http.util.EntityUtils;
 import org.elasticsearch.action.DocWriteRequest;
 import org.elasticsearch.action.bulk.BulkRequest;
@@ -64,6 +66,8 @@ import org.elasticsearch.test.RandomObjects;
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Modifier;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
@@ -77,20 +81,50 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXC
 
 public class RequestTests extends ESTestCase {
 
+    public void testConstructor() throws Exception {
+        final String method = randomFrom("GET", "PUT", "POST", "HEAD", "DELETE");
+        final String endpoint = randomAlphaOfLengthBetween(1, 10);
+        final Map<String, String> parameters = singletonMap(randomAlphaOfLength(5), randomAlphaOfLength(5));
+        final HttpEntity entity = randomBoolean() ? new StringEntity(randomAlphaOfLengthBetween(1, 100), ContentType.TEXT_PLAIN) : null;
+
+        NullPointerException e = expectThrows(NullPointerException.class, () -> new Request(null, endpoint, parameters, entity));
+        assertEquals("method cannot be null", e.getMessage());
+
+        e = expectThrows(NullPointerException.class, () -> new Request(method, null, parameters, entity));
+        assertEquals("endpoint cannot be null", e.getMessage());
+
+        e = expectThrows(NullPointerException.class, () -> new Request(method, endpoint, null, entity));
+        assertEquals("parameters cannot be null", e.getMessage());
+
+        final Request request = new Request(method, endpoint, parameters, entity);
+        assertEquals(method, request.getMethod());
+        assertEquals(endpoint, request.getEndpoint());
+        assertEquals(parameters, request.getParameters());
+        assertEquals(entity, request.getEntity());
+
+        final Constructor<?>[] constructors = Request.class.getConstructors();
+        assertEquals("Expected only 1 constructor", 1, constructors.length);
+        assertTrue("Request constructor is not public", Modifier.isPublic(constructors[0].getModifiers()));
+    }
+
+    public void testClassVisibility() throws Exception {
+        assertTrue("Request class is not public", Modifier.isPublic(Request.class.getModifiers()));
+    }
+
     public void testPing() {
         Request request = Request.ping();
-        assertEquals("/", request.endpoint);
-        assertEquals(0, request.params.size());
-        assertNull(request.entity);
-        assertEquals("HEAD", request.method);
+        assertEquals("/", request.getEndpoint());
+        assertEquals(0, request.getParameters().size());
+        assertNull(request.getEntity());
+        assertEquals("HEAD", request.getMethod());
     }
 
     public void testInfo() {
         Request request = Request.info();
-        assertEquals("/", request.endpoint);
-        assertEquals(0, request.params.size());
-        assertNull(request.entity);
-        assertEquals("GET", request.method);
+        assertEquals("/", request.getEndpoint());
+        assertEquals(0, request.getParameters().size());
+        assertNull(request.getEntity());
+        assertEquals("GET", request.getMethod());
     }
 
     public void testGet() {
@@ -124,10 +158,10 @@ public class RequestTests extends ESTestCase {
         }
 
         Request request = Request.delete(deleteRequest);
-        assertEquals("/" + index + "/" + type + "/" + id, request.endpoint);
-        assertEquals(expectedParams, request.params);
-        assertEquals("DELETE", request.method);
-        assertNull(request.entity);
+        assertEquals("/" + index + "/" + type + "/" + id, request.getEndpoint());
+        assertEquals(expectedParams, request.getParameters());
+        assertEquals("DELETE", request.getMethod());
+        assertNull(request.getEntity());
     }
 
     public void testExists() {
@@ -200,10 +234,10 @@ public class RequestTests extends ESTestCase {
             }
         }
         Request request = requestConverter.apply(getRequest);
-        assertEquals("/" + index + "/" + type + "/" + id, request.endpoint);
-        assertEquals(expectedParams, request.params);
-        assertNull(request.entity);
-        assertEquals(method, request.method);
+        assertEquals("/" + index + "/" + type + "/" + id, request.getEndpoint());
+        assertEquals(expectedParams, request.getParameters());
+        assertNull(request.getEntity());
+        assertEquals(method, request.getMethod());
     }
 
     public void testIndex() throws IOException {
@@ -267,16 +301,16 @@ public class RequestTests extends ESTestCase {
 
         Request request = Request.index(indexRequest);
         if (indexRequest.opType() == DocWriteRequest.OpType.CREATE) {
-            assertEquals("/" + index + "/" + type + "/" + id + "/_create", request.endpoint);
+            assertEquals("/" + index + "/" + type + "/" + id + "/_create", request.getEndpoint());
         } else if (id != null) {
-            assertEquals("/" + index + "/" + type + "/" + id, request.endpoint);
+            assertEquals("/" + index + "/" + type + "/" + id, request.getEndpoint());
         } else {
-            assertEquals("/" + index + "/" + type, request.endpoint);
+            assertEquals("/" + index + "/" + type, request.getEndpoint());
         }
-        assertEquals(expectedParams, request.params);
-        assertEquals(method, request.method);
+        assertEquals(expectedParams, request.getParameters());
+        assertEquals(method, request.getMethod());
 
-        HttpEntity entity = request.entity;
+        HttpEntity entity = request.getEntity();
         assertTrue(entity instanceof ByteArrayEntity);
         assertEquals(indexRequest.getContentType().mediaTypeWithoutParameters(), entity.getContentType().getValue());
         try (XContentParser parser = createParser(xContentType.xContent(), entity.getContent())) {
@@ -367,11 +401,11 @@ public class RequestTests extends ESTestCase {
         }
 
         Request request = Request.update(updateRequest);
-        assertEquals("/" + index + "/" + type + "/" + id + "/_update", request.endpoint);
-        assertEquals(expectedParams, request.params);
-        assertEquals("POST", request.method);
+        assertEquals("/" + index + "/" + type + "/" + id + "/_update", request.getEndpoint());
+        assertEquals(expectedParams, request.getParameters());
+        assertEquals("POST", request.getMethod());
 
-        HttpEntity entity = request.entity;
+        HttpEntity entity = request.getEntity();
         assertTrue(entity instanceof ByteArrayEntity);
 
         UpdateRequest parsedUpdateRequest = new UpdateRequest();
@@ -485,12 +519,12 @@ public class RequestTests extends ESTestCase {
         }
 
         Request request = Request.bulk(bulkRequest);
-        assertEquals("/_bulk", request.endpoint);
-        assertEquals(expectedParams, request.params);
-        assertEquals("POST", request.method);
-        assertEquals(xContentType.mediaTypeWithoutParameters(), request.entity.getContentType().getValue());
-        byte[] content = new byte[(int) request.entity.getContentLength()];
-        try (InputStream inputStream = request.entity.getContent()) {
+        assertEquals("/_bulk", request.getEndpoint());
+        assertEquals(expectedParams, request.getParameters());
+        assertEquals("POST", request.getMethod());
+        assertEquals(xContentType.mediaTypeWithoutParameters(), request.getEntity().getContentType().getValue());
+        byte[] content = new byte[(int) request.getEntity().getContentLength()];
+        try (InputStream inputStream = request.getEntity().getContent()) {
             Streams.readFully(inputStream, content);
         }
 
@@ -541,7 +575,7 @@ public class RequestTests extends ESTestCase {
             bulkRequest.add(new DeleteRequest("index", "type", "2"));
 
             Request request = Request.bulk(bulkRequest);
-            assertEquals(XContentType.JSON.mediaTypeWithoutParameters(), request.entity.getContentType().getValue());
+            assertEquals(XContentType.JSON.mediaTypeWithoutParameters(), request.getEntity().getContentType().getValue());
         }
         {
             XContentType xContentType = randomFrom(XContentType.JSON, XContentType.SMILE);
@@ -551,7 +585,7 @@ public class RequestTests extends ESTestCase {
             bulkRequest.add(new DeleteRequest("index", "type", "2"));
 
             Request request = Request.bulk(bulkRequest);
-            assertEquals(xContentType.mediaTypeWithoutParameters(), request.entity.getContentType().getValue());
+            assertEquals(xContentType.mediaTypeWithoutParameters(), request.getEntity().getContentType().getValue());
         }
         {
             XContentType xContentType = randomFrom(XContentType.JSON, XContentType.SMILE);
@@ -563,7 +597,7 @@ public class RequestTests extends ESTestCase {
             }
 
             Request request = Request.bulk(new BulkRequest().add(updateRequest));
-            assertEquals(xContentType.mediaTypeWithoutParameters(), request.entity.getContentType().getValue());
+            assertEquals(xContentType.mediaTypeWithoutParameters(), request.getEntity().getContentType().getValue());
         }
         {
             BulkRequest bulkRequest = new BulkRequest();
@@ -712,12 +746,12 @@ public class RequestTests extends ESTestCase {
             endpoint.add(type);
         }
         endpoint.add("_search");
-        assertEquals(endpoint.toString(), request.endpoint);
-        assertEquals(expectedParams, request.params);
+        assertEquals(endpoint.toString(), request.getEndpoint());
+        assertEquals(expectedParams, request.getParameters());
         if (searchSourceBuilder == null) {
-            assertNull(request.entity);
+            assertNull(request.getEntity());
         } else {
-            assertToXContentBody(searchSourceBuilder, request.entity);
+            assertToXContentBody(searchSourceBuilder, request.getEntity());
         }
     }
 
@@ -728,11 +762,11 @@ public class RequestTests extends ESTestCase {
             searchScrollRequest.scroll(randomPositiveTimeValue());
         }
         Request request = Request.searchScroll(searchScrollRequest);
-        assertEquals("GET", request.method);
-        assertEquals("/_search/scroll", request.endpoint);
-        assertEquals(0, request.params.size());
-        assertToXContentBody(searchScrollRequest, request.entity);
-        assertEquals(Request.REQUEST_BODY_CONTENT_TYPE.mediaTypeWithoutParameters(), request.entity.getContentType().getValue());
+        assertEquals("GET", request.getMethod());
+        assertEquals("/_search/scroll", request.getEndpoint());
+        assertEquals(0, request.getParameters().size());
+        assertToXContentBody(searchScrollRequest, request.getEntity());
+        assertEquals(Request.REQUEST_BODY_CONTENT_TYPE.mediaTypeWithoutParameters(), request.getEntity().getContentType().getValue());
     }
 
     public void testClearScroll() throws IOException {
@@ -742,11 +776,11 @@ public class RequestTests extends ESTestCase {
             clearScrollRequest.addScrollId(randomAlphaOfLengthBetween(5, 10));
         }
         Request request = Request.clearScroll(clearScrollRequest);
-        assertEquals("DELETE", request.method);
-        assertEquals("/_search/scroll", request.endpoint);
-        assertEquals(0, request.params.size());
-        assertToXContentBody(clearScrollRequest, request.entity);
-        assertEquals(Request.REQUEST_BODY_CONTENT_TYPE.mediaTypeWithoutParameters(), request.entity.getContentType().getValue());
+        assertEquals("DELETE", request.getMethod());
+        assertEquals("/_search/scroll", request.getEndpoint());
+        assertEquals(0, request.getParameters().size());
+        assertToXContentBody(clearScrollRequest, request.getEntity());
+        assertEquals(Request.REQUEST_BODY_CONTENT_TYPE.mediaTypeWithoutParameters(), request.getEntity().getContentType().getValue());
     }
 
     private static void assertToXContentBody(ToXContent expectedBody, HttpEntity actualEntity) throws IOException {

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

@@ -36,7 +36,7 @@ import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.mockito.Mockito.mock;
 
 /**
- * This test works against a {@link RestHighLevelClient} subclass that simulats how custom response sections returned by
+ * This test works against a {@link RestHighLevelClient} subclass that simulates how custom response sections returned by
  * Elasticsearch plugins can be parsed using the high level client.
  */
 public class RestHighLevelClientExtTests extends ESTestCase {