Switch remaining LLREST usage to new style Requests (#33171)

In #29623 we added `Request` object flavored requests to the low level
REST client and in #30315 we deprecated the old `performRequest`s. In a
long series of PRs I've changed all of the old style requests that I
could find with `grep`. In this PR I change all requests that I could
find by *removing* the deprecated methods. Since this is a non-trivial
change I do not include actually removing the deprecated requests. I'll
do that in a follow up. But this should be the last set of usage
removals before the actual deprecated method removal. Yay!
This commit is contained in:
Nik Everett 2018-08-28 14:20:14 -04:00 committed by GitHub
parent 7f5e29ddb2
commit 6c8f568808
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 74 additions and 75 deletions

View File

@ -19,6 +19,7 @@
package org.elasticsearch.script.mustache; package org.elasticsearch.script.mustache;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.ResponseException;
import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.ESRestTestCase;
@ -30,14 +31,14 @@ public class SearchTemplateWithoutContentIT extends ESRestTestCase {
public void testSearchTemplateMissingBody() throws IOException { public void testSearchTemplateMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest( ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
randomBoolean() ? "POST" : "GET", "/_search/template")); new Request(randomBoolean() ? "POST" : "GET", "/_search/template")));
assertEquals(400, responseException.getResponse().getStatusLine().getStatusCode()); assertEquals(400, responseException.getResponse().getStatusLine().getStatusCode());
assertThat(responseException.getMessage(), containsString("request body or source parameter is required")); assertThat(responseException.getMessage(), containsString("request body or source parameter is required"));
} }
public void testMultiSearchTemplateMissingBody() throws IOException { public void testMultiSearchTemplateMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest( ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
randomBoolean() ? "POST" : "GET", "/_msearch/template")); new Request(randomBoolean() ? "POST" : "GET", "/_msearch/template")));
assertEquals(400, responseException.getResponse().getStatusLine().getStatusCode()); assertEquals(400, responseException.getResponse().getStatusLine().getStatusCode());
assertThat(responseException.getMessage(), containsString("request body or source parameter is required")); assertThat(responseException.getMessage(), containsString("request body or source parameter is required"));
} }

View File

@ -19,6 +19,7 @@
package org.elasticsearch.index.reindex; package org.elasticsearch.index.reindex;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.ResponseException;
import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.ESRestTestCase;
@ -30,7 +31,7 @@ public class ReindexWithoutContentIT extends ESRestTestCase {
public void testReindexMissingBody() throws IOException { public void testReindexMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest( ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
"POST", "/_reindex")); new Request("POST", "/_reindex")));
assertEquals(400, responseException.getResponse().getStatusLine().getStatusCode()); assertEquals(400, responseException.getResponse().getStatusLine().getStatusCode());
assertThat(responseException.getMessage(), containsString("request body is required")); assertThat(responseException.getMessage(), containsString("request body is required"));
} }

View File

@ -32,7 +32,6 @@ import org.elasticsearch.test.rest.yaml.ObjectPath;
import java.io.IOException; import java.io.IOException;
import java.nio.charset.Charset; import java.nio.charset.Charset;
import java.util.Collections;
import java.util.Map; import java.util.Map;
import static org.elasticsearch.rest.RestStatus.BAD_REQUEST; import static org.elasticsearch.rest.RestStatus.BAD_REQUEST;
@ -71,7 +70,7 @@ public class Netty4BadRequestIT extends ESRestTestCase {
final ResponseException e = final ResponseException e =
expectThrows( expectThrows(
ResponseException.class, ResponseException.class,
() -> client().performRequest(randomFrom("GET", "POST", "PUT"), path, Collections.emptyMap())); () -> client().performRequest(new Request(randomFrom("GET", "POST", "PUT"), path)));
assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(BAD_REQUEST.getStatus())); assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(BAD_REQUEST.getStatus()));
assertThat(e, hasToString(containsString("too_long_frame_exception"))); assertThat(e, hasToString(containsString("too_long_frame_exception")));
assertThat(e, hasToString(matches("An HTTP line is larger than \\d+ bytes"))); assertThat(e, hasToString(matches("An HTTP line is larger than \\d+ bytes")));

View File

@ -5,14 +5,14 @@
*/ */
package org.elasticsearch.xpack.monitoring.exporter.http; package org.elasticsearch.xpack.monitoring.exporter.http;
import org.apache.http.HttpEntity;
import org.apache.http.entity.ByteArrayEntity;
import org.apache.http.entity.ContentType; import org.apache.http.entity.ContentType;
import org.apache.http.nio.entity.NByteArrayEntity;
import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier; import org.apache.logging.log4j.util.Supplier;
import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.BytesRef;
import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionListener;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response; import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseListener; import org.elasticsearch.client.ResponseListener;
import org.elasticsearch.client.RestClient; import org.elasticsearch.client.RestClient;
@ -94,9 +94,13 @@ class HttpExportBulk extends ExportBulk {
if (payload == null) { if (payload == null) {
listener.onFailure(new ExportException("unable to send documents because none were loaded for export bulk [{}]", name)); listener.onFailure(new ExportException("unable to send documents because none were loaded for export bulk [{}]", name));
} else if (payload.length != 0) { } else if (payload.length != 0) {
final HttpEntity body = new ByteArrayEntity(payload, ContentType.APPLICATION_JSON); final Request request = new Request("POST", "/_bulk");
for (Map.Entry<String, String> param : params.entrySet()) {
request.addParameter(param.getKey(), param.getValue());
}
request.setEntity(new NByteArrayEntity(payload, ContentType.APPLICATION_JSON));
client.performRequestAsync("POST", "/_bulk", params, body, new ResponseListener() { client.performRequestAsync(request, new ResponseListener() {
@Override @Override
public void onSuccess(Response response) { public void onSuccess(Response response) {
try { try {

View File

@ -7,10 +7,9 @@ package org.elasticsearch.integration;
import org.apache.http.HttpEntity; import org.apache.http.HttpEntity;
import org.apache.http.StatusLine; import org.apache.http.StatusLine;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.message.BasicHeader;
import org.apache.http.util.EntityUtils; import org.apache.http.util.EntityUtils;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.Response; import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.ResponseException;
import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.SecureString;
@ -18,9 +17,7 @@ import org.elasticsearch.test.SecuritySingleNodeTestCase;
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken; import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
import java.io.IOException; import java.io.IOException;
import java.util.HashMap;
import java.util.Locale; import java.util.Locale;
import java.util.Map;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo;
@ -28,64 +25,59 @@ import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.not;
/** /**
* a helper class that contains a couple of HTTP helper methods * A helper class that contains a couple of HTTP helper methods.
*/ */
public abstract class AbstractPrivilegeTestCase extends SecuritySingleNodeTestCase { public abstract class AbstractPrivilegeTestCase extends SecuritySingleNodeTestCase {
protected void assertAccessIsAllowed(String user, String method, String uri, String body, protected void assertAccessIsAllowed(String user, Request request) throws IOException {
Map<String, String> params) throws IOException { setUser(request, user);
Response response = getRestClient().performRequest(method, uri, params, entityOrNull(body), Response response = getRestClient().performRequest(request);
new BasicHeader(UsernamePasswordToken.BASIC_AUTH_HEADER,
UsernamePasswordToken.basicAuthHeaderValue(user, new SecureString("passwd".toCharArray()))));
StatusLine statusLine = response.getStatusLine(); StatusLine statusLine = response.getStatusLine();
String message = String.format(Locale.ROOT, "%s %s: Expected no error got %s %s with body %s", method, uri, String message = String.format(Locale.ROOT, "%s %s: Expected no error got %s %s with body %s",
statusLine.getStatusCode(), statusLine.getReasonPhrase(), EntityUtils.toString(response.getEntity())); request.getMethod(), request.getEndpoint(), statusLine.getStatusCode(),
statusLine.getReasonPhrase(), EntityUtils.toString(response.getEntity()));
assertThat(message, statusLine.getStatusCode(), is(not(greaterThanOrEqualTo(400)))); assertThat(message, statusLine.getStatusCode(), is(not(greaterThanOrEqualTo(400))));
} }
protected void assertAccessIsAllowed(String user, String method, String uri, String body) throws IOException { protected void assertAccessIsAllowed(String user, String method, String uri, String body) throws IOException {
assertAccessIsAllowed(user, method, uri, body, new HashMap<>()); Request request = new Request(method, uri);
request.setJsonEntity(body);
assertAccessIsAllowed(user, request);
} }
protected void assertAccessIsAllowed(String user, String method, String uri) throws IOException { protected void assertAccessIsAllowed(String user, String method, String uri) throws IOException {
assertAccessIsAllowed(user, method, uri, null, new HashMap<>()); assertAccessIsAllowed(user, new Request(method, uri));
} }
protected void assertAccessIsDenied(String user, String method, String uri, String body) throws IOException { protected void assertAccessIsDenied(String user, Request request) throws IOException {
assertAccessIsDenied(user, method, uri, body, new HashMap<>()); setUser(request, user);
} ResponseException responseException = expectThrows(ResponseException.class, () -> getRestClient().performRequest(request));
protected void assertAccessIsDenied(String user, String method, String uri) throws IOException {
assertAccessIsDenied(user, method, uri, null, new HashMap<>());
}
protected void assertAccessIsDenied(String user, String method, String uri, String body,
Map<String, String> params) throws IOException {
ResponseException responseException = expectThrows(ResponseException.class,
() -> getRestClient().performRequest(method, uri, params, entityOrNull(body),
new BasicHeader(UsernamePasswordToken.BASIC_AUTH_HEADER,
UsernamePasswordToken.basicAuthHeaderValue(user, new SecureString("passwd".toCharArray())))));
StatusLine statusLine = responseException.getResponse().getStatusLine(); StatusLine statusLine = responseException.getResponse().getStatusLine();
String message = String.format(Locale.ROOT, "%s %s body %s: Expected 403, got %s %s with body %s", method, uri, body, String requestBody = request.getEntity() == null ? "" : "with body " + EntityUtils.toString(request.getEntity());
String message = String.format(Locale.ROOT, "%s %s body %s: Expected 403, got %s %s with body %s",
request.getMethod(), request.getEndpoint(), requestBody,
statusLine.getStatusCode(), statusLine.getReasonPhrase(), statusLine.getStatusCode(), statusLine.getReasonPhrase(),
EntityUtils.toString(responseException.getResponse().getEntity())); EntityUtils.toString(responseException.getResponse().getEntity()));
assertThat(message, statusLine.getStatusCode(), is(403)); assertThat(message, statusLine.getStatusCode(), is(403));
} }
protected void assertAccessIsDenied(String user, String method, String uri, String body) throws IOException {
Request request = new Request(method, uri);
request.setJsonEntity(body);
assertAccessIsDenied(user, request);
}
protected void assertBodyHasAccessIsDenied(String user, String method, String uri, String body) throws IOException { protected void assertAccessIsDenied(String user, String method, String uri) throws IOException {
assertBodyHasAccessIsDenied(user, method, uri, body, new HashMap<>()); assertAccessIsDenied(user, new Request(method, uri));
} }
/** /**
* Like {@code assertAcessIsDenied}, but for _bulk requests since the entire * Like {@code assertAcessIsDenied}, but for _bulk requests since the entire
* request will not be failed, just the individual ones * request will not be failed, just the individual ones
*/ */
protected void assertBodyHasAccessIsDenied(String user, String method, String uri, String body, protected void assertBodyHasAccessIsDenied(String user, Request request) throws IOException {
Map<String, String> params) throws IOException { setUser(request, user);
Response resp = getRestClient().performRequest(method, uri, params, entityOrNull(body), Response resp = getRestClient().performRequest(request);
new BasicHeader(UsernamePasswordToken.BASIC_AUTH_HEADER,
UsernamePasswordToken.basicAuthHeaderValue(user, new SecureString("passwd".toCharArray()))));
StatusLine statusLine = resp.getStatusLine(); StatusLine statusLine = resp.getStatusLine();
assertThat(statusLine.getStatusCode(), is(200)); assertThat(statusLine.getStatusCode(), is(200));
HttpEntity bodyEntity = resp.getEntity(); HttpEntity bodyEntity = resp.getEntity();
@ -93,11 +85,15 @@ public abstract class AbstractPrivilegeTestCase extends SecuritySingleNodeTestCa
assertThat(bodyStr, containsString("unauthorized for user [" + user + "]")); assertThat(bodyStr, containsString("unauthorized for user [" + user + "]"));
} }
private static HttpEntity entityOrNull(String body) { protected void assertBodyHasAccessIsDenied(String user, String method, String uri, String body) throws IOException {
HttpEntity entity = null; Request request = new Request(method, uri);
if (body != null) { request.setJsonEntity(body);
entity = new StringEntity(body, ContentType.APPLICATION_JSON); assertBodyHasAccessIsDenied(user, request);
} }
return entity;
private void setUser(Request request, String user) {
RequestOptions.Builder options = RequestOptions.DEFAULT.toBuilder();
options.addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue(user, new SecureString("passwd".toCharArray())));
request.setOptions(options);
} }
} }

View File

@ -6,6 +6,7 @@
package org.elasticsearch.integration; package org.elasticsearch.integration;
import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse; import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse;
import org.elasticsearch.client.Request;
import org.elasticsearch.cluster.SnapshotsInProgress; import org.elasticsearch.cluster.SnapshotsInProgress;
import org.elasticsearch.common.Strings; import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.SecureString;
@ -15,9 +16,7 @@ import org.junit.AfterClass;
import org.junit.BeforeClass; import org.junit.BeforeClass;
import java.nio.file.Path; import java.nio.file.Path;
import java.util.Map;
import static java.util.Collections.singletonMap;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.is;
@ -132,10 +131,12 @@ public class ClusterPrivilegeTests extends AbstractPrivilegeTestCase {
assertAccessIsDenied("user_c", "PUT", "/_snapshot/my-repo", repoJson); assertAccessIsDenied("user_c", "PUT", "/_snapshot/my-repo", repoJson);
assertAccessIsAllowed("user_a", "PUT", "/_snapshot/my-repo", repoJson); assertAccessIsAllowed("user_a", "PUT", "/_snapshot/my-repo", repoJson);
Map<String, String> params = singletonMap("refresh", "true"); Request createBar = new Request("PUT", "/someindex/bar/1");
assertAccessIsDenied("user_a", "PUT", "/someindex/bar/1", "{ \"name\" : \"elasticsearch\" }", params); createBar.setJsonEntity("{ \"name\" : \"elasticsearch\" }");
assertAccessIsDenied("user_b", "PUT", "/someindex/bar/1", "{ \"name\" : \"elasticsearch\" }", params); createBar.addParameter("refresh", "true");
assertAccessIsAllowed("user_c", "PUT", "/someindex/bar/1", "{ \"name\" : \"elasticsearch\" }", params); assertAccessIsDenied("user_a", createBar);
assertAccessIsDenied("user_b", createBar);
assertAccessIsAllowed("user_c", createBar);
assertAccessIsDenied("user_b", "PUT", "/_snapshot/my-repo/my-snapshot", "{ \"indices\": \"someindex\" }"); assertAccessIsDenied("user_b", "PUT", "/_snapshot/my-repo/my-snapshot", "{ \"indices\": \"someindex\" }");
assertAccessIsDenied("user_c", "PUT", "/_snapshot/my-repo/my-snapshot", "{ \"indices\": \"someindex\" }"); assertAccessIsDenied("user_c", "PUT", "/_snapshot/my-repo/my-snapshot", "{ \"indices\": \"someindex\" }");
@ -152,10 +153,11 @@ public class ClusterPrivilegeTests extends AbstractPrivilegeTestCase {
assertAccessIsDenied("user_b", "DELETE", "/someindex"); assertAccessIsDenied("user_b", "DELETE", "/someindex");
assertAccessIsAllowed("user_c", "DELETE", "/someindex"); assertAccessIsAllowed("user_c", "DELETE", "/someindex");
params = singletonMap("wait_for_completion", "true"); Request restoreSnapshotRequest = new Request("POST", "/_snapshot/my-repo/my-snapshot/_restore");
assertAccessIsDenied("user_b", "POST", "/_snapshot/my-repo/my-snapshot/_restore", null, params); restoreSnapshotRequest.addParameter("wait_for_completion", "true");
assertAccessIsDenied("user_c", "POST", "/_snapshot/my-repo/my-snapshot/_restore", null, params); assertAccessIsDenied("user_b", restoreSnapshotRequest);
assertAccessIsAllowed("user_a", "POST", "/_snapshot/my-repo/my-snapshot/_restore", null, params); assertAccessIsDenied("user_c", restoreSnapshotRequest);
assertAccessIsAllowed("user_a", restoreSnapshotRequest);
assertAccessIsDenied("user_a", "GET", "/someindex/bar/1"); assertAccessIsDenied("user_a", "GET", "/someindex/bar/1");
assertAccessIsDenied("user_b", "GET", "/someindex/bar/1"); assertAccessIsDenied("user_b", "GET", "/someindex/bar/1");

View File

@ -13,11 +13,8 @@ import org.elasticsearch.xpack.core.security.authc.support.Hasher;
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken; import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
import org.junit.Before; import org.junit.Before;
import java.util.Collections;
import java.util.Locale; import java.util.Locale;
import java.util.Map;
import static java.util.Collections.singletonMap;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoTimeout; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoTimeout;
import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.is;
@ -143,11 +140,12 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase {
@Before @Before
public void insertBaseDocumentsAsAdmin() throws Exception { public void insertBaseDocumentsAsAdmin() throws Exception {
// indices: a,b,c,abc // indices: a,b,c,abc
Map<String, String> params = singletonMap("refresh", "true"); for (String index : new String[] {"a", "b", "c", "abc"}) {
assertAccessIsAllowed("admin", "PUT", "/a/foo/1", jsonDoc, params); Request request = new Request("PUT", "/" + index + "/foo/1");
assertAccessIsAllowed("admin", "PUT", "/b/foo/1", jsonDoc, params); request.setJsonEntity(jsonDoc);
assertAccessIsAllowed("admin", "PUT", "/c/foo/1", jsonDoc, params); request.addParameter("refresh", "true");
assertAccessIsAllowed("admin", "PUT", "/abc/foo/1", jsonDoc, params); assertAccessIsAllowed("admin", request);
}
} }
private static String randomIndex() { private static String randomIndex() {
@ -402,8 +400,6 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase {
} }
private void assertUserExecutes(String user, String action, String index, boolean userIsAllowed) throws Exception { private void assertUserExecutes(String user, String action, String index, boolean userIsAllowed) throws Exception {
Map<String, String> refreshParams = Collections.emptyMap();//singletonMap("refresh", "true");
switch (action) { switch (action) {
case "all" : case "all" :
if (userIsAllowed) { if (userIsAllowed) {
@ -438,7 +434,7 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase {
assertAccessIsAllowed(user, "POST", "/" + index + "/_open"); assertAccessIsAllowed(user, "POST", "/" + index + "/_open");
assertAccessIsAllowed(user, "POST", "/" + index + "/_cache/clear"); assertAccessIsAllowed(user, "POST", "/" + index + "/_cache/clear");
// indexing a document to have the mapping available, and wait for green state to make sure index is created // indexing a document to have the mapping available, and wait for green state to make sure index is created
assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/1", jsonDoc, refreshParams); assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/1", jsonDoc);
assertNoTimeout(client().admin().cluster().prepareHealth(index).setWaitForGreenStatus().get()); assertNoTimeout(client().admin().cluster().prepareHealth(index).setWaitForGreenStatus().get());
assertAccessIsAllowed(user, "GET", "/" + index + "/_mapping/foo/field/name"); assertAccessIsAllowed(user, "GET", "/" + index + "/_mapping/foo/field/name");
assertAccessIsAllowed(user, "GET", "/" + index + "/_settings"); assertAccessIsAllowed(user, "GET", "/" + index + "/_settings");
@ -535,8 +531,8 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase {
case "delete" : case "delete" :
String jsonDoc = "{ \"name\" : \"docToDelete\"}"; String jsonDoc = "{ \"name\" : \"docToDelete\"}";
assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/docToDelete", jsonDoc, refreshParams); assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/docToDelete", jsonDoc);
assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/docToDelete2", jsonDoc, refreshParams); assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/docToDelete2", jsonDoc);
if (userIsAllowed) { if (userIsAllowed) {
assertAccessIsAllowed(user, "DELETE", "/" + index + "/foo/docToDelete"); assertAccessIsAllowed(user, "DELETE", "/" + index + "/foo/docToDelete");
} else { } else {