Skip to content

Commit

Permalink
Switch monitoring to new style Requests (#32255)
Browse files Browse the repository at this point in the history 
In #29623 we added `Request` object flavored requests to the low level
REST client and in #30315 we deprecated the old `performRequest`s. This
changes all calls in the `x-pack/plugin/monitoring` project to use the new
versions.
  • Loading branch information
@nik9000
nik9000 committed Jul 23, 2018
1 parent 2329b52 commit f45b17d
Show file tree
Hide file tree
Showing 6 changed files with 198 additions and 137 deletions.
33 changes: 24 additions & 9 deletions ...c/main/java/org/elasticsearch/xpack/monitoring/exporter/http/PublishableHttpResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.RestClient;
Expand Down Expand Up @@ -311,13 +312,15 @@ protected Tuple<CheckResponse, Response> checkForResource(final RestClient clien
final Set<Integer> exists, final Set<Integer> doesNotExist) {
logger.trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, resourceOwnerName, resourceOwnerType);

final Set<Integer> expectedResponseCodes = Sets.union(exists, doesNotExist);

final Request request = new Request("GET", resourceBasePath + "/" + resourceName);
addParameters(request);
// avoid exists and DNE parameters from being an exception by default
final Map<String, String> getParameters = new HashMap<>(parameters);
getParameters.put("ignore", expectedResponseCodes.stream().map(i -> i.toString()).collect(Collectors.joining(",")));
final Set<Integer> expectedResponseCodes = Sets.union(exists, doesNotExist);
request.addParameter("ignore", expectedResponseCodes.stream().map(i -> i.toString()).collect(Collectors.joining(",")));

try {
final Response response = client.performRequest("GET", resourceBasePath + "/" + resourceName, getParameters);
final Response response = client.performRequest(request);
final int statusCode = response.getStatusLine().getStatusCode();

// checking the content is the job of whoever called this function by checking the tuple's response
Expand Down Expand Up @@ -385,8 +388,12 @@ protected boolean putResource(final RestClient client, final Logger logger,

boolean success = false;

final Request request = new Request("PUT", resourceBasePath + "/" + resourceName);
addParameters(request);
request.setEntity(body.get());

try {
final Response response = client.performRequest("PUT", resourceBasePath + "/" + resourceName, parameters, body.get());
final Response response = client.performRequest(request);
final int statusCode = response.getStatusLine().getStatusCode();

// 200 or 201
Expand Down Expand Up @@ -431,12 +438,15 @@ protected boolean deleteResource(final RestClient client, final Logger logger,

boolean success = false;

// avoid 404 being an exception by default
final Map<String, String> deleteParameters = new HashMap<>(parameters);
deleteParameters.putIfAbsent("ignore", Integer.toString(RestStatus.NOT_FOUND.getStatus()));
Request request = new Request("DELETE", resourceBasePath + "/" + resourceName);
addParameters(request);
if (false == parameters.containsKey("ignore")) {
// avoid 404 being an exception by default
request.addParameter("ignore", Integer.toString(RestStatus.NOT_FOUND.getStatus()));
}

try {
final Response response = client.performRequest("DELETE", resourceBasePath + "/" + resourceName, deleteParameters);
final Response response = client.performRequest(request);
final int statusCode = response.getStatusLine().getStatusCode();

// 200 or 404 (not found is just as good as deleting it!)
Expand Down Expand Up @@ -498,4 +508,9 @@ protected boolean shouldReplaceResource(final Response response, final XContent
return true;
}

private void addParameters(Request request) {
for (Map.Entry<String, String> param : parameters.entrySet()) {
request.addParameter(param.getKey(), param.getValue());
}
}
}
11 changes: 4 additions & 7 deletions ...g/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/VersionHttpResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.Version;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.json.JsonXContent;

import java.io.IOException;
import java.util.Collections;
import java.util.Map;
import java.util.Objects;

Expand All @@ -27,11 +27,6 @@ public class VersionHttpResource extends HttpResource {

private static final Logger logger = Loggers.getLogger(VersionHttpResource.class);

/**
* The parameters to pass with every version request to limit the output to just the version number.
*/
public static final Map<String, String> PARAMETERS = Collections.singletonMap("filter_path", "version.number");

/**
* The minimum supported version of Elasticsearch.
*/
Expand Down Expand Up @@ -59,7 +54,9 @@ protected boolean doCheckAndPublish(final RestClient client) {
logger.trace("checking [{}] to ensure that it supports the minimum version [{}]", resourceOwnerName, minimumVersion);

try {
return validateVersion(client.performRequest("GET", "/", PARAMETERS));
Request request = new Request("GET", "/");
request.addParameter("filter_path", "version.number");
return validateVersion(client.performRequest(request));
} catch (IOException | RuntimeException e) {
logger.error(
(Supplier<?>)() ->
Expand Down
43 changes: 35 additions & 8 deletions ...elasticsearch/xpack/monitoring/exporter/http/AbstractPublishableHttpResourceTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.apache.http.StatusLine;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.RestClient;
Expand All @@ -20,6 +21,8 @@
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;

import java.io.IOException;
import java.util.Map;
Expand All @@ -30,10 +33,10 @@
import static org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.GET_DOES_NOT_EXIST;
import static org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.GET_EXISTS;
import static org.hamcrest.Matchers.is;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.eq;
import static org.hamcrest.Matchers.instanceOf;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.verify;

/**
* Base test helper for any {@link PublishableHttpResource}.
Expand Down Expand Up @@ -87,7 +90,9 @@ protected void assertCheckWithException(final PublishableHttpResource resource,
final ResponseException responseException = responseException("GET", endpoint, failedCheckStatus());
final Exception e = randomFrom(new IOException("expected"), new RuntimeException("expected"), responseException);

when(client.performRequest("GET", endpoint, getParameters(resource.getParameters()))).thenThrow(e);
Request request = new Request("GET", endpoint);
addParameters(request, getParameters(resource.getParameters()));
when(client.performRequest(request)).thenThrow(e);

assertThat(resource.doCheck(client), is(CheckResponse.ERROR));
}
Expand Down Expand Up @@ -123,7 +128,9 @@ protected void assertCheckAsDeleteWithException(final PublishableHttpResource re
final ResponseException responseException = responseException("DELETE", endpoint, failedCheckStatus());
final Exception e = randomFrom(new IOException("expected"), new RuntimeException("expected"), responseException);

when(client.performRequest("DELETE", endpoint, deleteParameters(resource.getParameters()))).thenThrow(e);
Request request = new Request("DELETE", endpoint);
addParameters(request, deleteParameters(resource.getParameters()));
when(client.performRequest(request)).thenThrow(e);

assertThat(resource.doCheck(client), is(CheckResponse.ERROR));
}
Expand Down Expand Up @@ -173,9 +180,15 @@ protected void assertPublishWithException(final PublishableHttpResource resource
final String endpoint = concatenateEndpoint(resourceBasePath, resourceName);
final Exception e = randomFrom(new IOException("expected"), new RuntimeException("expected"));

when(client.performRequest(eq("PUT"), eq(endpoint), eq(resource.getParameters()), any(bodyType))).thenThrow(e);
when(client.performRequest(Mockito.any(Request.class))).thenThrow(e);

assertThat(resource.doPublish(client), is(false));
ArgumentCaptor<Request> request = ArgumentCaptor.forClass(Request.class);
verify(client).performRequest(request.capture());
assertThat(request.getValue().getMethod(), is("PUT"));
assertThat(request.getValue().getEndpoint(), is(endpoint));
assertThat(request.getValue().getParameters(), is(resource.getParameters()));
assertThat(request.getValue().getEntity(), instanceOf(bodyType));
}

protected void assertParameters(final PublishableHttpResource resource) {
Expand Down Expand Up @@ -244,7 +257,9 @@ protected void doCheckWithStatusCode(final PublishableHttpResource resource, fin
final String endpoint, final CheckResponse expected,
final Response response)
throws IOException {
when(client.performRequest("GET", endpoint, expectedParameters)).thenReturn(response);
Request request = new Request("GET", endpoint);
addParameters(request, expectedParameters);
when(client.performRequest(request)).thenReturn(response);

assertThat(resource.doCheck(client), is(expected));
}
Expand All @@ -257,9 +272,14 @@ private void doPublishWithStatusCode(final PublishableHttpResource resource, fin
final String endpoint = concatenateEndpoint(resourceBasePath, resourceName);
final Response response = response("GET", endpoint, status);

when(client.performRequest(eq("PUT"), eq(endpoint), eq(resource.getParameters()), any(bodyType))).thenReturn(response);
ArgumentCaptor<Request> request = ArgumentCaptor.forClass(Request.class);
when(client.performRequest(request.capture())).thenReturn(response);

assertThat(resource.doPublish(client), is(expected));
assertThat(request.getValue().getMethod(), is("PUT"));
assertThat(request.getValue().getEndpoint(), is(endpoint));
assertThat(request.getValue().getParameters(), is(resource.getParameters()));
assertThat(request.getValue().getEntity(), instanceOf(bodyType));
}

protected void doCheckAsDeleteWithStatusCode(final PublishableHttpResource resource,
Expand All @@ -277,7 +297,9 @@ protected void doCheckAsDeleteWithStatusCode(final PublishableHttpResource resou
final String endpoint, final CheckResponse expected,
final Response response)
throws IOException {
when(client.performRequest("DELETE", endpoint, deleteParameters(resource.getParameters()))).thenReturn(response);
Request request = new Request("DELETE", endpoint);
addParameters(request, deleteParameters(resource.getParameters()));
when(client.performRequest(request)).thenReturn(response);

assertThat(resource.doCheck(client), is(expected));
}
Expand Down Expand Up @@ -427,4 +449,9 @@ protected HttpEntity entityForClusterAlert(final CheckResponse expected, final i
return entity;
}

protected void addParameters(Request request, Map<String, String> parameters) {
for (Map.Entry<String, String> param : parameters.entrySet()) {
request.addParameter(param.getKey(), param.getValue());
}
}
}
Loading

0 comments on commit f45b17d

Please sign in to comment.