Skip to content

Commit

Permalink
REST high-level client: move to POST when calling API to retrieve whi…
Browse files Browse the repository at this point in the history
…ch support request body (#28342)

It has been pointed out that GET with body may cause problems to some proxies. We are then switching to POST the API that retrieve info and support a request body.

Closes #28326
  • Loading branch information
javanna authored Jan 25, 2018
1 parent 9db23e4 commit 45e1fe8
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ static Request multiGet(MultiGetRequest multiGetRequest) throws IOException {
parameters.withRealtime(multiGetRequest.realtime());
parameters.withRefresh(multiGetRequest.refresh());
HttpEntity entity = createEntity(multiGetRequest, REQUEST_BODY_CONTENT_TYPE);
return new Request(HttpGet.METHOD_NAME, "/_mget", parameters.getParams(), entity);
return new Request(HttpPost.METHOD_NAME, "/_mget", parameters.getParams(), entity);
}

static Request index(IndexRequest indexRequest) {
Expand Down Expand Up @@ -447,17 +447,17 @@ static Request search(SearchRequest searchRequest) throws IOException {
if (searchRequest.source() != null) {
entity = createEntity(searchRequest.source(), REQUEST_BODY_CONTENT_TYPE);
}
return new Request(HttpGet.METHOD_NAME, endpoint, params.getParams(), entity);
return new Request(HttpPost.METHOD_NAME, endpoint, params.getParams(), entity);
}

static Request searchScroll(SearchScrollRequest searchScrollRequest) throws IOException {
HttpEntity entity = createEntity(searchScrollRequest, REQUEST_BODY_CONTENT_TYPE);
return new Request("GET", "/_search/scroll", Collections.emptyMap(), entity);
return new Request(HttpPost.METHOD_NAME, "/_search/scroll", Collections.emptyMap(), entity);
}

static Request clearScroll(ClearScrollRequest clearScrollRequest) throws IOException {
HttpEntity entity = createEntity(clearScrollRequest, REQUEST_BODY_CONTENT_TYPE);
return new Request("DELETE", "/_search/scroll", Collections.emptyMap(), entity);
return new Request(HttpDelete.METHOD_NAME, "/_search/scroll", Collections.emptyMap(), entity);
}

static Request multiSearch(MultiSearchRequest multiSearchRequest) throws IOException {
Expand All @@ -469,7 +469,7 @@ static Request multiSearch(MultiSearchRequest multiSearchRequest) throws IOExcep
XContent xContent = REQUEST_BODY_CONTENT_TYPE.xContent();
byte[] source = MultiSearchRequest.writeMultiLineFormat(multiSearchRequest, xContent);
HttpEntity entity = new ByteArrayEntity(source, createContentType(xContent.type()));
return new Request("GET", "/_msearch", params.getParams(), entity);
return new Request(HttpPost.METHOD_NAME, "/_msearch", params.getParams(), entity);
}

private static HttpEntity createEntity(ToXContent toXContent, XContentType xContentType) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
package org.elasticsearch.client;

import org.apache.http.HttpEntity;
import org.apache.http.client.methods.HttpDelete;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpHead;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpPut;
import org.apache.http.entity.ByteArrayEntity;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
Expand Down Expand Up @@ -138,19 +143,19 @@ public void testPing() {
assertEquals("/", request.getEndpoint());
assertEquals(0, request.getParameters().size());
assertNull(request.getEntity());
assertEquals("HEAD", request.getMethod());
assertEquals(HttpHead.METHOD_NAME, request.getMethod());
}

public void testInfo() {
Request request = Request.info();
assertEquals("/", request.getEndpoint());
assertEquals(0, request.getParameters().size());
assertNull(request.getEntity());
assertEquals("GET", request.getMethod());
assertEquals(HttpGet.METHOD_NAME, request.getMethod());
}

public void testGet() {
getAndExistsTest(Request::get, "GET");
getAndExistsTest(Request::get, HttpGet.METHOD_NAME);
}

public void testMultiGet() throws IOException {
Expand Down Expand Up @@ -200,7 +205,7 @@ public void testMultiGet() throws IOException {
}

Request request = Request.multiGet(multiGetRequest);
assertEquals("GET", request.getMethod());
assertEquals(HttpPost.METHOD_NAME, request.getMethod());
assertEquals("/_mget", request.getEndpoint());
assertEquals(expectedParams, request.getParameters());
assertToXContentBody(multiGetRequest, request.getEntity());
Expand Down Expand Up @@ -240,7 +245,7 @@ public void testDelete() {
}

public void testExists() {
getAndExistsTest(Request::exists, "HEAD");
getAndExistsTest(Request::exists, HttpHead.METHOD_NAME);
}

private static void getAndExistsTest(Function<GetRequest, Request> requestConverter, String method) {
Expand Down Expand Up @@ -317,7 +322,7 @@ public void testCreateIndex() throws IOException {
Request request = Request.createIndex(createIndexRequest);
assertEquals("/" + indexName, request.getEndpoint());
assertEquals(expectedParams, request.getParameters());
assertEquals("PUT", request.getMethod());
assertEquals(HttpPut.METHOD_NAME, request.getMethod());
assertToXContentBody(createIndexRequest, request.getEntity());
}

Expand Down Expand Up @@ -365,7 +370,7 @@ public void testPutMapping() throws IOException {
assertEquals(endpoint.toString(), request.getEndpoint());

assertEquals(expectedParams, request.getParameters());
assertEquals("PUT", request.getMethod());
assertEquals(HttpPut.METHOD_NAME, request.getMethod());
assertToXContentBody(putMappingRequest, request.getEntity());
}

Expand All @@ -382,7 +387,7 @@ public void testDeleteIndex() {
Request request = Request.deleteIndex(deleteIndexRequest);
assertEquals("/" + String.join(",", indices), request.getEndpoint());
assertEquals(expectedParams, request.getParameters());
assertEquals("DELETE", request.getMethod());
assertEquals(HttpDelete.METHOD_NAME, request.getMethod());
assertNull(request.getEntity());
}

Expand All @@ -401,7 +406,7 @@ public void testOpenIndex() {
StringJoiner endpoint = new StringJoiner("/", "/", "").add(String.join(",", indices)).add("_open");
assertThat(endpoint.toString(), equalTo(request.getEndpoint()));
assertThat(expectedParams, equalTo(request.getParameters()));
assertThat(request.getMethod(), equalTo("POST"));
assertThat(request.getMethod(), equalTo(HttpPost.METHOD_NAME));
assertThat(request.getEntity(), nullValue());
}

Expand All @@ -418,7 +423,7 @@ public void testCloseIndex() {
StringJoiner endpoint = new StringJoiner("/", "/", "").add(String.join(",", indices)).add("_close");
assertThat(endpoint.toString(), equalTo(request.getEndpoint()));
assertThat(expectedParams, equalTo(request.getParameters()));
assertThat(request.getMethod(), equalTo("POST"));
assertThat(request.getMethod(), equalTo(HttpPost.METHOD_NAME));
assertThat(request.getEntity(), nullValue());
}

Expand All @@ -432,9 +437,9 @@ public void testIndex() throws IOException {

Map<String, String> expectedParams = new HashMap<>();

String method = "POST";
String method = HttpPost.METHOD_NAME;
if (id != null) {
method = "PUT";
method = HttpPut.METHOD_NAME;
if (randomBoolean()) {
indexRequest.opType(DocWriteRequest.OpType.CREATE);
}
Expand Down Expand Up @@ -569,7 +574,7 @@ public void testUpdate() throws IOException {
Request request = Request.update(updateRequest);
assertEquals("/" + index + "/" + type + "/" + id + "/_update", request.getEndpoint());
assertEquals(expectedParams, request.getParameters());
assertEquals("POST", request.getMethod());
assertEquals(HttpPost.METHOD_NAME, request.getMethod());

HttpEntity entity = request.getEntity();
assertTrue(entity instanceof ByteArrayEntity);
Expand Down Expand Up @@ -683,7 +688,7 @@ public void testBulk() throws IOException {
Request request = Request.bulk(bulkRequest);
assertEquals("/_bulk", request.getEndpoint());
assertEquals(expectedParams, request.getParameters());
assertEquals("POST", request.getMethod());
assertEquals(HttpPost.METHOD_NAME, request.getMethod());
assertEquals(xContentType.mediaTypeWithoutParameters(), request.getEntity().getContentType().getValue());
byte[] content = new byte[(int) request.getEntity().getContentLength()];
try (InputStream inputStream = request.getEntity().getContent()) {
Expand Down Expand Up @@ -894,6 +899,7 @@ public void testSearch() throws Exception {
endpoint.add(type);
}
endpoint.add("_search");
assertEquals(HttpPost.METHOD_NAME, request.getMethod());
assertEquals(endpoint.toString(), request.getEndpoint());
assertEquals(expectedParams, request.getParameters());
assertToXContentBody(searchSourceBuilder, request.getEntity());
Expand Down Expand Up @@ -932,6 +938,7 @@ public void testMultiSearch() throws IOException {

Request request = Request.multiSearch(multiSearchRequest);
assertEquals("/_msearch", request.getEndpoint());
assertEquals(HttpPost.METHOD_NAME, request.getMethod());
assertEquals(expectedParams, request.getParameters());

List<SearchRequest> requests = new ArrayList<>();
Expand All @@ -955,7 +962,7 @@ public void testSearchScroll() throws IOException {
searchScrollRequest.scroll(randomPositiveTimeValue());
}
Request request = Request.searchScroll(searchScrollRequest);
assertEquals("GET", request.getMethod());
assertEquals(HttpPost.METHOD_NAME, request.getMethod());
assertEquals("/_search/scroll", request.getEndpoint());
assertEquals(0, request.getParameters().size());
assertToXContentBody(searchScrollRequest, request.getEntity());
Expand All @@ -969,7 +976,7 @@ public void testClearScroll() throws IOException {
clearScrollRequest.addScrollId(randomAlphaOfLengthBetween(5, 10));
}
Request request = Request.clearScroll(clearScrollRequest);
assertEquals("DELETE", request.getMethod());
assertEquals(HttpDelete.METHOD_NAME, request.getMethod());
assertEquals("/_search/scroll", request.getEndpoint());
assertEquals(0, request.getParameters().size());
assertToXContentBody(clearScrollRequest, request.getEntity());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public void testSearchScroll() throws IOException {
assertEquals(5, searchResponse.getTotalShards());
assertEquals(5, searchResponse.getSuccessfulShards());
assertEquals(100, searchResponse.getTook().getMillis());
verify(restClient).performRequest(eq("GET"), eq("/_search/scroll"), eq(Collections.emptyMap()),
verify(restClient).performRequest(eq("POST"), eq("/_search/scroll"), eq(Collections.emptyMap()),
isNotNull(HttpEntity.class), argThat(new HeadersVarargMatcher(headers)));
}

Expand Down

0 comments on commit 45e1fe8

Please sign in to comment.