Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch distribution to new style Requests #30595

Merged
merged 5 commits into from
Jul 18, 2018
Merged

Switch distribution to new style Requests #30595

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3d15677
Switch distribution to new style Requests
nik9000 May 14, 2018
6fa7b84
Merge branch 'master' into rest_request_distribution
nik9000 May 18, 2018
3db267c
Don't reuse requests
nik9000 May 18, 2018
ced27a6
Merge branch 'master' into rest_request_distribution
nik9000 Jul 10, 2018
f7dabfa
Remove unused imports
nik9000 Jul 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions ...ves/integ-test-zip/src/test/java/org/elasticsearch/test/rest/CreatedLocationHeaderIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,26 +50,31 @@ public void testIndexWithoutId() throws IOException {
}

public void testUpsert() throws IOException {
locationTestCase(client().performRequest("POST", "test/test/1/_update", emptyMap(), new StringEntity("{"
+ "\"doc\": {\"test\": \"test\"},"
+ "\"doc_as_upsert\": true}", ContentType.APPLICATION_JSON)));
Request request = new Request("POST", "test/test/1/_update");
request.setJsonEntity("{"
+ "\"doc\": {\"test\": \"test\"},"
+ "\"doc_as_upsert\": true}");
locationTestCase(client().performRequest(request));
}

private void locationTestCase(String method, String url) throws IOException {
locationTestCase(client().performRequest(method, url, emptyMap(),
new StringEntity("{\"test\": \"test\"}", ContentType.APPLICATION_JSON)));
Request request = new Request(method, url);
request.setJsonEntity("{\"test\": \"test\"}");
locationTestCase(client().performRequest(request));
// we have to delete the index otherwise the second indexing request will route to the single shard and not produce a 201
final Response response = client().performRequest(new Request("DELETE", "test"));
assertThat(response.getStatusLine().getStatusCode(), equalTo(200));
locationTestCase(client().performRequest(method, url + "?routing=cat", emptyMap(),
new StringEntity("{\"test\": \"test\"}", ContentType.APPLICATION_JSON)));
request = new Request(method, url);
request.addParameter("routing", "cat");
request.setJsonEntity("{\"test\": \"test\"}");
locationTestCase(client().performRequest(request));
}

private void locationTestCase(Response response) throws IOException {
assertEquals(201, response.getStatusLine().getStatusCode());
String location = response.getHeader("Location");
assertThat(location, startsWith("/test/test/"));
Response getResponse = client().performRequest("GET", location);
Response getResponse = client().performRequest(new Request("GET", location));
assertEquals(singletonMap("test", "test"), entityAsMap(getResponse).get("_source"));
}

Expand Down
45 changes: 25 additions & 20 deletions ...on/archives/integ-test-zip/src/test/java/org/elasticsearch/test/rest/NodeRestUsageIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.http.entity.StringEntity;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.Request;

import java.io.IOException;
import java.util.Collections;
Expand All @@ -39,8 +40,8 @@ public class NodeRestUsageIT extends ESRestTestCase {
@SuppressWarnings("unchecked")
public void testWithRestUsage() throws IOException {
// First get the current usage figures
Response beforeResponse = client().performRequest("GET",
randomFrom("_nodes/usage", "_nodes/usage/rest_actions", "_nodes/usage/_all"));
String path = randomFrom("_nodes/usage", "_nodes/usage/rest_actions", "_nodes/usage/_all");
Response beforeResponse = client().performRequest(new Request("GET", path));
Map<String, Object> beforeResponseBodyMap = entityAsMap(beforeResponse);
assertThat(beforeResponseBodyMap, notNullValue());
Map<String, Object> before_nodesMap = (Map<String, Object>) beforeResponseBodyMap.get("_nodes");
Expand Down Expand Up @@ -80,24 +81,28 @@ public void testWithRestUsage() throws IOException {
}

// Do some requests to get some rest usage stats
client().performRequest("PUT", "/test");
client().performRequest("POST", "/test/doc/1", Collections.emptyMap(),
new StringEntity("{ \"foo\": \"bar\"}", ContentType.APPLICATION_JSON));
client().performRequest("POST", "/test/doc/2", Collections.emptyMap(),
new StringEntity("{ \"foo\": \"bar\"}", ContentType.APPLICATION_JSON));
client().performRequest("POST", "/test/doc/3", Collections.emptyMap(),
new StringEntity("{ \"foo\": \"bar\"}", ContentType.APPLICATION_JSON));
client().performRequest("GET", "/test/_search");
client().performRequest("POST", "/test/doc/4", Collections.emptyMap(),
new StringEntity("{ \"foo\": \"bar\"}", ContentType.APPLICATION_JSON));
client().performRequest("POST", "/test/_refresh");
client().performRequest("GET", "/_cat/indices");
client().performRequest("GET", "/_nodes");
client().performRequest("GET", "/test/_search");
client().performRequest("GET", "/_nodes/stats");
client().performRequest("DELETE", "/test");
client().performRequest(new Request("PUT", "/test"));
Request request = new Request("POST", "/test/doc/1");
request.setJsonEntity("{\"foo\": \"bar\"}");
client().performRequest(request);
request = new Request("POST", "/test/doc/2");
request.setJsonEntity("{\"foo\": \"bar\"}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really dislike this style of variable reuse in our tests. If I use my IDE to navigate to the definition of this variable I end up on a line assigning a value to this variable that is removed from its current value. This hinders readability, especially in longer tests. Let’s avoid introducing it here, we should be moving away from it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you prefer extra blocks or different names to stop the reuse?

client().performRequest(request);
request = new Request("POST", "/test/doc/3");
request.setJsonEntity("{\"foo\": \"bar\"}");
client().performRequest(request);
client().performRequest(new Request("GET", "/test/_search"));
request = new Request("POST", "/test/doc/4");
request.setJsonEntity("{\"foo\": \"bar\"}");
client().performRequest(request);
client().performRequest(new Request("POST", "/test/_refresh"));
client().performRequest(new Request("GET", "/_cat/indices"));
client().performRequest(new Request("GET", "/_nodes"));
client().performRequest(new Request("GET", "/test/_search"));
client().performRequest(new Request("GET", "/_nodes/stats"));
client().performRequest(new Request("DELETE", "/test"));

Response response = client().performRequest("GET", "_nodes/usage");
Response response = client().performRequest(new Request("GET", "_nodes/usage"));
Map<String, Object> responseBodyMap = entityAsMap(response);
assertThat(responseBodyMap, notNullValue());
Map<String, Object> _nodesMap = (Map<String, Object>) responseBodyMap.get("_nodes");
Expand Down Expand Up @@ -139,7 +144,7 @@ public void testWithRestUsage() throws IOException {

public void testMetricsWithAll() throws IOException {
ResponseException exception = expectThrows(ResponseException.class,
() -> client().performRequest("GET", "_nodes/usage/_all,rest_actions"));
() -> client().performRequest(new Request("GET", "_nodes/usage/_all,rest_actions")));
assertNotNull(exception);
assertThat(exception.getMessage(), containsString("\"type\":\"illegal_argument_exception\","
+ "\"reason\":\"request [_nodes/usage/_all,rest_actions] contains _all and individual metrics [_all,rest_actions]\""));
Expand Down
37 changes: 19 additions & 18 deletions ...es/integ-test-zip/src/test/java/org/elasticsearch/test/rest/RequestsWithoutContentIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.test.rest;

import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.Request;

import java.io.IOException;

Expand All @@ -28,56 +29,56 @@
public class RequestsWithoutContentIT extends ESRestTestCase {

public void testIndexMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
randomBoolean() ? "POST" : "PUT", "/idx/type/123"));
ResponseException responseException = expectThrows(ResponseException.class, () ->
client().performRequest(new Request(randomBoolean() ? "POST" : "PUT", "/idx/type/123")));
assertResponseException(responseException, "request body is required");
}

public void testBulkMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
randomBoolean() ? "POST" : "PUT", "/_bulk"));
ResponseException responseException = expectThrows(ResponseException.class, () ->
client().performRequest(new Request(randomBoolean() ? "POST" : "PUT", "/_bulk")));
assertResponseException(responseException, "request body is required");
}

public void testPutSettingsMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
"PUT", "/_settings"));
ResponseException responseException = expectThrows(ResponseException.class, () ->
client().performRequest(new Request("PUT", "/_settings")));
assertResponseException(responseException, "request body is required");
}

public void testPutMappingsMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
randomBoolean() ? "POST" : "PUT", "/test_index/test_type/_mapping"));
ResponseException responseException = expectThrows(ResponseException.class, () ->
client().performRequest(new Request(randomBoolean() ? "POST" : "PUT", "/test_index/test_type/_mapping")));
assertResponseException(responseException, "request body is required");
}

public void testPutIndexTemplateMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
randomBoolean() ? "PUT" : "POST", "/_template/my_template"));
ResponseException responseException = expectThrows(ResponseException.class, () ->
client().performRequest(new Request(randomBoolean() ? "PUT" : "POST", "/_template/my_template")));
assertResponseException(responseException, "request body is required");
}

public void testMultiSearchMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
randomBoolean() ? "POST" : "GET", "/_msearch"));
ResponseException responseException = expectThrows(ResponseException.class, () ->
client().performRequest(new Request(randomBoolean() ? "POST" : "GET", "/_msearch")));
assertResponseException(responseException, "request body or source parameter is required");
}

public void testPutPipelineMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
"PUT", "/_ingest/pipeline/my_pipeline"));
ResponseException responseException = expectThrows(ResponseException.class, () ->
client().performRequest(new Request("PUT", "/_ingest/pipeline/my_pipeline")));
assertResponseException(responseException, "request body or source parameter is required");
}

public void testSimulatePipelineMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
randomBoolean() ? "POST" : "GET", "/_ingest/pipeline/my_pipeline/_simulate"));
ResponseException responseException = expectThrows(ResponseException.class, () ->
client().performRequest(new Request(randomBoolean() ? "POST" : "GET", "/_ingest/pipeline/my_pipeline/_simulate")));
assertResponseException(responseException, "request body or source parameter is required");
}

public void testPutScriptMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
randomBoolean() ? "POST" : "PUT", "/_scripts/lang"));
ResponseException responseException = expectThrows(ResponseException.class, () ->
client().performRequest(new Request(randomBoolean() ? "POST" : "PUT", "/_scripts/lang")));
assertResponseException(responseException, "request body is required");
}

Expand Down
36 changes: 21 additions & 15 deletions ...integ-test-zip/src/test/java/org/elasticsearch/test/rest/WaitForRefreshAndCloseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.ResponseListener;
import org.elasticsearch.client.Request;
import org.junit.After;
import org.junit.Before;

Expand All @@ -46,13 +47,14 @@ public class WaitForRefreshAndCloseTests extends ESRestTestCase {
@Before
public void setupIndex() throws IOException {
try {
client().performRequest("DELETE", indexName());
client().performRequest(new Request("DELETE", indexName()));
} catch (ResponseException e) {
// If we get an error, it should be because the index doesn't exist
assertEquals(404, e.getResponse().getStatusLine().getStatusCode());
}
client().performRequest("PUT", indexName(), emptyMap(),
new StringEntity("{\"settings\":{\"refresh_interval\":-1}}", ContentType.APPLICATION_JSON));
Request request = new Request("PUT", indexName());
request.setJsonEntity("{\"settings\":{\"refresh_interval\":-1}}");
client().performRequest(request);
}

@After
Expand All @@ -69,17 +71,20 @@ private String docPath() {
}

public void testIndexAndThenClose() throws Exception {
closeWhileListenerEngaged(start("PUT", "", new StringEntity("{\"test\":\"test\"}", ContentType.APPLICATION_JSON)));
closeWhileListenerEngaged(start("PUT", "", "{\"test\":\"test\"}"));
}

public void testUpdateAndThenClose() throws Exception {
client().performRequest("PUT", docPath(), emptyMap(), new StringEntity("{\"test\":\"test\"}", ContentType.APPLICATION_JSON));
closeWhileListenerEngaged(start("POST", "/_update",
new StringEntity("{\"doc\":{\"name\":\"test\"}}", ContentType.APPLICATION_JSON)));
Request request = new Request("PUT", docPath());
request.setJsonEntity("{\"test\":\"test\"}");
client().performRequest(request);
closeWhileListenerEngaged(start("POST", "/_update", "{\"doc\":{\"name\":\"test\"}}"));
}

public void testDeleteAndThenClose() throws Exception {
client().performRequest("PUT", docPath(), emptyMap(), new StringEntity("{\"test\":\"test\"}", ContentType.APPLICATION_JSON));
Request request = new Request("PUT", docPath());
request.setJsonEntity("{\"test\":\"test\"}");
client().performRequest(request);
closeWhileListenerEngaged(start("DELETE", "", null));
}

Expand All @@ -88,7 +93,7 @@ private void closeWhileListenerEngaged(ActionFuture<String> future) throws Excep
assertBusy(() -> {
Map<String, Object> stats;
try {
stats = entityAsMap(client().performRequest("GET", indexName() + "/_stats/refresh"));
stats = entityAsMap(client().performRequest(new Request("GET", indexName() + "/_stats/refresh")));
} catch (IOException e) {
throw new RuntimeException(e);
}
Expand All @@ -105,18 +110,19 @@ private void closeWhileListenerEngaged(ActionFuture<String> future) throws Excep
});

// Close the index. That should flush the listener.
client().performRequest("POST", indexName() + "/_close");
client().performRequest(new Request("POST", indexName() + "/_close"));

// The request shouldn't fail. It certainly shouldn't hang.
future.get();
}

private ActionFuture<String> start(String method, String path, HttpEntity body) {
private ActionFuture<String> start(String method, String path, String body) {
PlainActionFuture<String> future = new PlainActionFuture<>();
Map<String, String> params = new HashMap<>();
params.put("refresh", "wait_for");
params.put("error_trace", "");
client().performRequestAsync(method, docPath() + path, params, body, new ResponseListener() {
Request request = new Request(method, docPath() + path);
request.addParameter("refresh", "wait_for");
request.addParameter("error_trace", "");
request.setJsonEntity(body);
client().performRequestAsync(request, new ResponseListener() {
@Override
public void onSuccess(Response response) {
try {
Expand Down