From 55a2d3e0dd6e3f1ac6d37790c8ee5342a0fec764 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 23 Jul 2018 16:50:15 -0400 Subject: [PATCH] Switch monitoring to new style Requests (#32255) 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. --- .../http/PublishableHttpResource.java | 33 +++- .../exporter/http/VersionHttpResource.java | 11 +- ...stractPublishableHttpResourceTestCase.java | 43 ++++- .../http/HttpExporterResourceTests.java | 171 +++++++++--------- .../http/PublishableHttpResourceTests.java | 63 +++++-- .../http/VersionHttpResourceTests.java | 14 +- 6 files changed, 198 insertions(+), 137 deletions(-) diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/PublishableHttpResource.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/PublishableHttpResource.java index 168ccff44c4..a804703a530 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/PublishableHttpResource.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/PublishableHttpResource.java @@ -9,6 +9,7 @@ import org.apache.http.HttpEntity; 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; @@ -311,13 +312,15 @@ public abstract class PublishableHttpResource extends HttpResource { final Set exists, final Set doesNotExist) { logger.trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, resourceOwnerName, resourceOwnerType); - final Set 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 getParameters = new HashMap<>(parameters); - getParameters.put("ignore", expectedResponseCodes.stream().map(i -> i.toString()).collect(Collectors.joining(","))); + final Set 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 @@ -385,8 +388,12 @@ public abstract class PublishableHttpResource extends HttpResource { 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 @@ -431,12 +438,15 @@ public abstract class PublishableHttpResource extends HttpResource { boolean success = false; - // avoid 404 being an exception by default - final Map 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!) @@ -498,4 +508,9 @@ public abstract class PublishableHttpResource extends HttpResource { return true; } + private void addParameters(Request request) { + for (Map.Entry param : parameters.entrySet()) { + request.addParameter(param.getKey(), param.getValue()); + } + } } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/VersionHttpResource.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/VersionHttpResource.java index bbc86d37774..eec9162e7ed 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/VersionHttpResource.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/VersionHttpResource.java @@ -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.Version; +import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; import org.elasticsearch.common.logging.Loggers; @@ -16,7 +17,6 @@ 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; @@ -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 PARAMETERS = Collections.singletonMap("filter_path", "version.number"); - /** * The minimum supported version of Elasticsearch. */ @@ -59,7 +54,9 @@ public class VersionHttpResource extends HttpResource { 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)() -> diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/AbstractPublishableHttpResourceTestCase.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/AbstractPublishableHttpResourceTestCase.java index 208f878543f..ef365042bd5 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/AbstractPublishableHttpResourceTestCase.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/AbstractPublishableHttpResourceTestCase.java @@ -11,6 +11,7 @@ import org.apache.http.RequestLine; 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; @@ -20,6 +21,8 @@ import org.elasticsearch.common.util.set.Sets; 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; @@ -30,10 +33,10 @@ import java.util.stream.Collectors; 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}. @@ -87,7 +90,9 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase 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)); } @@ -123,7 +128,9 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase 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)); } @@ -173,9 +180,15 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase 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 = 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) { @@ -244,7 +257,9 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase 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)); } @@ -257,9 +272,14 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase 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 = 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, @@ -277,7 +297,9 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase 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)); } @@ -427,4 +449,9 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase return entity; } + protected void addParameters(Request request, Map parameters) { + for (Map.Entry param : parameters.entrySet()) { + request.addParameter(param.getKey(), param.getValue()); + } + } } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterResourceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterResourceTests.java index 355a5644bdd..fdf67602af6 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterResourceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterResourceTests.java @@ -10,6 +10,7 @@ import org.apache.http.StatusLine; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.elasticsearch.Version; +import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.RestClient; @@ -23,6 +24,9 @@ import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils; import org.elasticsearch.xpack.monitoring.exporter.ClusterAlertsUtil; import org.elasticsearch.xpack.monitoring.exporter.Exporter; +import org.hamcrest.Description; +import org.hamcrest.Matcher; +import org.hamcrest.TypeSafeMatcher; import org.junit.Before; import java.io.IOException; @@ -37,10 +41,9 @@ import static org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplat import static org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse.DOES_NOT_EXIST; import static org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse.EXISTS; import static org.hamcrest.Matchers.hasSize; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyMapOf; -import static org.mockito.Matchers.eq; -import static org.mockito.Matchers.startsWith; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.startsWith; +import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -101,7 +104,8 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe final HttpEntity entity = new StringEntity("{\"version\":{\"number\":\"unknown\"}}", ContentType.APPLICATION_JSON); when(versionResponse.getEntity()).thenReturn(entity); - when(client.performRequest(eq("GET"), eq("/"), anyMapOf(String.class, String.class))).thenReturn(versionResponse); + when(client.performRequest(argThat(new RequestMatcher(is("GET"), is("/"))))) + .thenReturn(versionResponse); assertTrue(resources.isDirty()); assertFalse(resources.checkAndPublish(client)); @@ -140,15 +144,15 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe final List otherResponses = getTemplateResponses(1, successful, unsuccessful); // last check fails implies that N - 2 publishes succeeded! - when(client.performRequest(eq("GET"), startsWith("/_template/"), anyMapOf(String.class, String.class))) - .thenReturn(first, otherResponses.toArray(new Response[otherResponses.size()])) - .thenThrow(exception); + when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_template/"))))) + .thenReturn(first, otherResponses.toArray(new Response[otherResponses.size()])) + .thenThrow(exception); whenSuccessfulPutTemplates(otherResponses.size() + 1); expectedGets += 1 + successful + unsuccessful; expectedPuts = (successfulFirst ? 0 : 1) + unsuccessful; } else { - when(client.performRequest(eq("GET"), startsWith("/_template/"), anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_template/"))))) .thenThrow(exception); } @@ -185,7 +189,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe whenGetTemplates(successful, unsuccessful + 2); // previous publishes must have succeeded - when(client.performRequest(eq("PUT"), startsWith("/_template/"), anyMapOf(String.class, String.class), any(HttpEntity.class))) + when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_template/"))))) .thenReturn(firstSuccess, otherResponses.toArray(new Response[otherResponses.size()])) .thenThrow(exception); @@ -197,7 +201,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe // fail the check so that it has to attempt the PUT whenGetTemplates(0, 1); - when(client.performRequest(eq("PUT"), startsWith("/_template/"), anyMapOf(String.class, String.class), any(HttpEntity.class))) + when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_template/"))))) .thenThrow(exception); } @@ -238,7 +242,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe } // last check fails - when(client.performRequest(eq("GET"), startsWith("/_ingest/pipeline/"), anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/"))))) .thenReturn(first) .thenThrow(exception); if (successfulFirst == false) { @@ -248,7 +252,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe expectedGets = EXPECTED_PIPELINES; expectedPuts = successfulFirst ? 0 : 1; } else { - when(client.performRequest(eq("GET"), startsWith("/_ingest/pipeline/"), anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/"))))) .thenThrow(exception); } @@ -285,10 +289,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe whenGetPipelines(0, 2); // previous publishes must have succeeded - when(client.performRequest(eq("PUT"), - startsWith("/_ingest/pipeline/"), - anyMapOf(String.class, String.class), - any(HttpEntity.class))) + when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/"))))) .thenReturn(firstSuccess) .thenThrow(exception); @@ -300,10 +301,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe // fail the check so that it has to attempt the PUT whenGetPipelines(0, 1); - when(client.performRequest(eq("PUT"), - startsWith("/_ingest/pipeline/"), - anyMapOf(String.class, String.class), - any(HttpEntity.class))) + when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/"))))) .thenThrow(exception); } @@ -334,7 +332,8 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe whenSuccessfulPutPipelines(unsuccessfulGetPipelines); // there's only one check - when(client.performRequest(eq("GET"), eq("/_xpack"), anyMapOf(String.class, String.class))).thenThrow(exception); + when(client.performRequest(argThat(new RequestMatcher(is("GET"), is("/_xpack"))))) + .thenThrow(exception); assertTrue(resources.isDirty()); assertFalse(resources.checkAndPublish(client)); @@ -382,7 +381,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe final List otherResponses = getWatcherResponses(1, successful, unsuccessful); // last check fails implies that N - 2 publishes succeeded! - when(client.performRequest(eq("GET"), startsWith("/_xpack/watcher/watch/"), anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_xpack/watcher/watch/"))))) .thenReturn(first, otherResponses.toArray(new Response[otherResponses.size()])) .thenThrow(exception); whenSuccessfulPutWatches(otherResponses.size() + 1); @@ -398,7 +397,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe // there is no form of an unsuccessful delete; only success or error final List responses = successfulDeleteResponses(successful); - when(client.performRequest(eq("DELETE"), startsWith("/_xpack/watcher/watch/"), anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("DELETE"), startsWith("/_xpack/watcher/watch/"))))) .thenReturn(responses.get(0), responses.subList(1, successful).toArray(new Response[successful - 1])) .thenThrow(exception); @@ -407,7 +406,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe } else { final String method = validLicense ? "GET" : "DELETE"; - when(client.performRequest(eq(method), startsWith("/_xpack/watcher/watch/"), anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is(method), startsWith("/_xpack/watcher/watch/"))))) .thenThrow(exception); } @@ -463,10 +462,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe whenGetWatches(successful, unsuccessful + 2); // previous publishes must have succeeded - when(client.performRequest(eq("PUT"), - startsWith("/_xpack/watcher/watch/"), - anyMapOf(String.class, String.class), - any(HttpEntity.class))) + when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/"))))) .thenReturn(firstSuccess, otherResponses.toArray(new Response[otherResponses.size()])) .thenThrow(exception); @@ -478,10 +474,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe // fail the check so that it has to attempt the PUT whenGetWatches(0, 1); - when(client.performRequest(eq("PUT"), - startsWith("/_xpack/watcher/watch/"), - anyMapOf(String.class, String.class), - any(HttpEntity.class))) + when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/"))))) .thenThrow(exception); } @@ -715,17 +708,18 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe final HttpEntity entity = new StringEntity("{\"version\":{\"number\":\"" + Version.CURRENT + "\"}}", ContentType.APPLICATION_JSON); when(versionResponse.getEntity()).thenReturn(entity); - when(client.performRequest(eq("GET"), eq("/"), anyMapOf(String.class, String.class))).thenReturn(versionResponse); + when(client.performRequest(argThat(new RequestMatcher(is("GET"), is("/"))))) + .thenReturn(versionResponse); } private void whenGetTemplates(final int successful, final int unsuccessful) throws IOException { final List gets = getTemplateResponses(0, successful, unsuccessful); if (gets.size() == 1) { - when(client.performRequest(eq("GET"), startsWith("/_template/"), anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_template/"))))) .thenReturn(gets.get(0)); } else { - when(client.performRequest(eq("GET"), startsWith("/_template/"), anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_template/"))))) .thenReturn(gets.get(0), gets.subList(1, gets.size()).toArray(new Response[gets.size() - 1])); } } @@ -735,10 +729,10 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe // empty is possible if they all exist if (successful == 1) { - when(client.performRequest(eq("PUT"), startsWith("/_template/"), anyMapOf(String.class, String.class), any(HttpEntity.class))) + when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_template/"))))) .thenReturn(successfulPuts.get(0)); } else if (successful > 1) { - when(client.performRequest(eq("PUT"), startsWith("/_template/"), anyMapOf(String.class, String.class), any(HttpEntity.class))) + when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_template/"))))) .thenReturn(successfulPuts.get(0), successfulPuts.subList(1, successful).toArray(new Response[successful - 1])); } } @@ -747,10 +741,10 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe final List gets = getPipelineResponses(0, successful, unsuccessful); if (gets.size() == 1) { - when(client.performRequest(eq("GET"), startsWith("/_ingest/pipeline/"), anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/"))))) .thenReturn(gets.get(0)); } else { - when(client.performRequest(eq("GET"), startsWith("/_ingest/pipeline/"), anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/"))))) .thenReturn(gets.get(0), gets.subList(1, gets.size()).toArray(new Response[gets.size() - 1])); } } @@ -760,16 +754,10 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe // empty is possible if they all exist if (successful == 1) { - when(client.performRequest(eq("PUT"), - startsWith("/_ingest/pipeline/"), - anyMapOf(String.class, String.class), - any(HttpEntity.class))) + when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/"))))) .thenReturn(successfulPuts.get(0)); } else if (successful > 1) { - when(client.performRequest(eq("PUT"), - startsWith("/_ingest/pipeline/"), - anyMapOf(String.class, String.class), - any(HttpEntity.class))) + when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/"))))) .thenReturn(successfulPuts.get(0), successfulPuts.subList(1, successful).toArray(new Response[successful - 1])); } } @@ -787,7 +775,8 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe final Response successfulGet = response("GET", "_xpack", successfulCheckStatus(), entity); // empty is possible if they all exist - when(client.performRequest(eq("GET"), eq("/_xpack"), anyMapOf(String.class, String.class))).thenReturn(successfulGet); + when(client.performRequest(argThat(new RequestMatcher(is("GET"), is("/_xpack"))))) + .thenReturn(successfulGet); } private void whenWatcherCannotBeUsed() throws IOException { @@ -805,17 +794,18 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe } // empty is possible if they all exist - when(client.performRequest(eq("GET"), eq("/_xpack"), anyMapOf(String.class, String.class))).thenReturn(response); + when(client.performRequest(argThat(new RequestMatcher(is("GET"), is("/_xpack"))))) + .thenReturn(response); } private void whenGetWatches(final int successful, final int unsuccessful) throws IOException { final List gets = getWatcherResponses(0, successful, unsuccessful); if (gets.size() == 1) { - when(client.performRequest(eq("GET"), startsWith("/_xpack/watcher/watch/"), anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_xpack/watcher/watch/"))))) .thenReturn(gets.get(0)); } else { - when(client.performRequest(eq("GET"), startsWith("/_xpack/watcher/watch/"), anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_xpack/watcher/watch/"))))) .thenReturn(gets.get(0), gets.subList(1, gets.size()).toArray(new Response[gets.size() - 1])); } } @@ -825,16 +815,10 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe // empty is possible if they all exist if (successful == 1) { - when(client.performRequest(eq("PUT"), - startsWith("/_xpack/watcher/watch/"), - anyMapOf(String.class, String.class), - any(HttpEntity.class))) + when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/"))))) .thenReturn(successfulPuts.get(0)); } else if (successful > 1) { - when(client.performRequest(eq("PUT"), - startsWith("/_xpack/watcher/watch/"), - anyMapOf(String.class, String.class), - any(HttpEntity.class))) + when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/"))))) .thenReturn(successfulPuts.get(0), successfulPuts.subList(1, successful).toArray(new Response[successful - 1])); } } @@ -844,64 +828,55 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe // empty is possible if they all exist if (successful == 1) { - when(client.performRequest(eq("DELETE"), - startsWith("/_xpack/watcher/watch/"), - anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("DELETE"), startsWith("/_xpack/watcher/watch/"))))) .thenReturn(successfulDeletes.get(0)); } else if (successful > 1) { - when(client.performRequest(eq("DELETE"), - startsWith("/_xpack/watcher/watch/"), - anyMapOf(String.class, String.class))) + when(client.performRequest(argThat(new RequestMatcher(is("DELETE"), startsWith("/_xpack/watcher/watch/"))))) .thenReturn(successfulDeletes.get(0), successfulDeletes.subList(1, successful).toArray(new Response[successful - 1])); } } + private void verifyVersionCheck() throws IOException { - verify(client).performRequest(eq("GET"), eq("/"), anyMapOf(String.class, String.class)); + verify(client).performRequest(argThat(new RequestMatcher(is("GET"), is("/")))); } private void verifyGetTemplates(final int called) throws IOException { - verify(client, times(called)).performRequest(eq("GET"), startsWith("/_template/"), anyMapOf(String.class, String.class)); + verify(client, times(called)) + .performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_template/")))); } private void verifyPutTemplates(final int called) throws IOException { - verify(client, times(called)).performRequest(eq("PUT"), // method - startsWith("/_template/"), // endpoint - anyMapOf(String.class, String.class), // parameters (e.g., timeout) - any(HttpEntity.class)); // raw template + verify(client, times(called)) + .performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_template/")))); } private void verifyGetPipelines(final int called) throws IOException { - verify(client, times(called)).performRequest(eq("GET"), startsWith("/_ingest/pipeline/"), anyMapOf(String.class, String.class)); + verify(client, times(called)) + .performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/")))); } private void verifyPutPipelines(final int called) throws IOException { - verify(client, times(called)).performRequest(eq("PUT"), // method - startsWith("/_ingest/pipeline/"), // endpoint - anyMapOf(String.class, String.class), // parameters (e.g., timeout) - any(HttpEntity.class)); // raw template + verify(client, times(called)) + .performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/")))); } private void verifyWatcherCheck() throws IOException { - verify(client).performRequest(eq("GET"), eq("/_xpack"), anyMapOf(String.class, String.class)); + verify(client).performRequest(argThat(new RequestMatcher(is("GET"), is("/_xpack")))); } private void verifyDeleteWatches(final int called) throws IOException { - verify(client, times(called)).performRequest(eq("DELETE"), // method - startsWith("/_xpack/watcher/watch/"), // endpoint - anyMapOf(String.class, String.class));// parameters (e.g., timeout) + verify(client, times(called)) + .performRequest(argThat(new RequestMatcher(is("DELETE"), startsWith("/_xpack/watcher/watch/")))); } private void verifyGetWatches(final int called) throws IOException { - verify(client, times(called)).performRequest(eq("GET"), - startsWith("/_xpack/watcher/watch/"), - anyMapOf(String.class, String.class)); + verify(client, times(called)) + .performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_xpack/watcher/watch/")))); } private void verifyPutWatches(final int called) throws IOException { - verify(client, times(called)).performRequest(eq("PUT"), // method - startsWith("/_xpack/watcher/watch/"), // endpoint - anyMapOf(String.class, String.class), // parameters (e.g., timeout) - any(HttpEntity.class)); // raw template + verify(client, times(called)) + .performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/")))); } private ClusterService mockClusterService(final ClusterState state) { @@ -922,4 +897,24 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe return state; } + private static class RequestMatcher extends TypeSafeMatcher { + private final Matcher method; + private final Matcher endpoint; + + private RequestMatcher(Matcher method, Matcher endpoint) { + this.method = method; + this.endpoint = endpoint; + } + + @Override + protected boolean matchesSafely(Request item) { + return method.matches(item.getMethod()) && endpoint.matches(item.getEndpoint()); + } + + @Override + public void describeTo(Description description) { + description.appendText("method is ").appendDescriptionOf(method); + description.appendText(" and endpoint is ").appendDescriptionOf(endpoint); + } + } } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/PublishableHttpResourceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/PublishableHttpResourceTests.java index 9c217612263..e32effc85f9 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/PublishableHttpResourceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/PublishableHttpResourceTests.java @@ -10,6 +10,7 @@ import org.apache.http.HttpEntity; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.apache.logging.log4j.Logger; +import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.RestClient; @@ -61,13 +62,15 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final RestStatus failedStatus = failedCheckStatus(); final Response response = response("GET", endpoint, failedStatus); + final Request request = new Request("GET", endpoint); + addParameters(request, getParameters(resource.getParameters())); - when(client.performRequest("GET", endpoint, getParameters(resource.getParameters()))).thenReturn(response); + when(client.performRequest(request)).thenReturn(response); sometimesAssertSimpleCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, CheckResponse.ERROR, response); verify(logger).trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest("GET", endpoint, getParameters(resource.getParameters())); + verify(client).performRequest(request); verify(logger).error(any(org.apache.logging.log4j.util.Supplier.class), any(ResponseException.class)); verifyNoMoreInteractions(client, logger); @@ -95,8 +98,10 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc final Response response = response("GET", endpoint, failedStatus); final XContent xContent = mock(XContent.class); final int minimumVersion = randomInt(); + final Request request = new Request("GET", endpoint); + addParameters(request, getParameters(resource.getParameters())); - when(client.performRequest("GET", endpoint, getParameters(resource.getParameters()))).thenReturn(response); + when(client.performRequest(request)).thenReturn(response); assertThat(resource.versionCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, owner, ownerType, @@ -104,7 +109,7 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc is(CheckResponse.ERROR)); verify(logger).trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest("GET", endpoint, getParameters(resource.getParameters())); + verify(client).performRequest(request); verify(logger).error(any(org.apache.logging.log4j.util.Supplier.class), any(ResponseException.class)); verifyNoMoreInteractions(client, logger); @@ -117,8 +122,10 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc final HttpEntity entity = entityForResource(CheckResponse.ERROR, resourceName, minimumVersion); final Response response = response("GET", endpoint, okStatus, entity); final XContent xContent = mock(XContent.class); + final Request request = new Request("GET", endpoint); + addParameters(request, getParameters(resource.getParameters())); - when(client.performRequest("GET", endpoint, getParameters(resource.getParameters()))).thenReturn(response); + when(client.performRequest(request)).thenReturn(response); assertThat(resource.versionCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, owner, ownerType, @@ -127,7 +134,7 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc verify(logger).trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, owner, ownerType); verify(logger).debug("{} [{}] found on the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest("GET", endpoint, getParameters(resource.getParameters())); + verify(client).performRequest(request); verify(logger).error(any(org.apache.logging.log4j.util.Supplier.class), any(ResponseException.class)); verifyNoMoreInteractions(client, logger); @@ -140,12 +147,14 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc final Exception e = randomFrom(new IOException("expected"), new RuntimeException("expected"), responseException); final Response response = e == responseException ? responseException.getResponse() : null; - 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); sometimesAssertSimpleCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, CheckResponse.ERROR, response); verify(logger).trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest("GET", endpoint, getParameters(resource.getParameters())); + verify(client).performRequest(request); verify(logger).error(any(org.apache.logging.log4j.util.Supplier.class), eq(e)); verifyNoMoreInteractions(client, logger); @@ -162,13 +171,16 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc public void testPutResourceFalseWithException() throws IOException { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final Exception e = randomFrom(new IOException("expected"), new RuntimeException("expected")); + final Request request = new Request("PUT", endpoint); + addParameters(request, resource.getParameters()); + request.setEntity(entity); - when(client.performRequest("PUT", endpoint, resource.getParameters(), entity)).thenThrow(e); + when(client.performRequest(request)).thenThrow(e); assertThat(resource.putResource(client, logger, resourceBasePath, resourceName, body, resourceType, owner, ownerType), is(false)); verify(logger).trace("uploading {} [{}] to the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest("PUT", endpoint, resource.getParameters(), entity); + verify(client).performRequest(request); verify(logger).error(any(org.apache.logging.log4j.util.Supplier.class), eq(e)); verifyNoMoreInteractions(client, logger); @@ -190,13 +202,15 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc final ResponseException responseException = responseException("DELETE", endpoint, failedStatus); final Exception e = randomFrom(new IOException("expected"), new RuntimeException("expected"), responseException); final Map deleteParameters = deleteParameters(resource.getParameters()); + final Request request = new Request("DELETE", endpoint); + addParameters(request, deleteParameters); - when(client.performRequest("DELETE", endpoint, deleteParameters)).thenThrow(e); + when(client.performRequest(request)).thenThrow(e); assertThat(resource.deleteResource(client, logger, resourceBasePath, resourceName, resourceType, owner, ownerType), is(false)); verify(logger).trace("deleting {} [{}] from the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest("DELETE", endpoint, deleteParameters); + verify(client).performRequest(request); verify(logger).error(any(org.apache.logging.log4j.util.Supplier.class), eq(e)); verifyNoMoreInteractions(client, logger); @@ -277,13 +291,15 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc throws IOException { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final Response response = response("GET", endpoint, status); + final Request request = new Request("GET", endpoint); + addParameters(request, getParameters(resource.getParameters())); - when(client.performRequest("GET", endpoint, getParameters(resource.getParameters()))).thenReturn(response); + when(client.performRequest(request)).thenReturn(response); sometimesAssertSimpleCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, expected, response); verify(logger).trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest("GET", endpoint, getParameters(resource.getParameters())); + verify(client).performRequest(request); if (expected == CheckResponse.EXISTS || expected == CheckResponse.DOES_NOT_EXIST) { verify(response).getStatusLine(); @@ -310,8 +326,10 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc final HttpEntity entity = status == RestStatus.OK ? entityForResource(expected, resourceName, minimumVersion) : null; final Response response = response("GET", endpoint, status, entity); final XContent xContent = XContentType.JSON.xContent(); + final Request request = new Request("GET", endpoint); + addParameters(request, getParameters(resource.getParameters())); - when(client.performRequest("GET", endpoint, getParameters(resource.getParameters()))).thenReturn(response); + when(client.performRequest(request)).thenReturn(response); assertThat(resource.versionCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, owner, ownerType, @@ -319,7 +337,7 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc is(expected)); verify(logger).trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest("GET", endpoint, getParameters(resource.getParameters())); + verify(client).performRequest(request); if (shouldReplace || expected == CheckResponse.EXISTS) { verify(response).getStatusLine(); @@ -341,13 +359,16 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc private void assertPutResource(final RestStatus status, final boolean expected) throws IOException { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final Response response = response("PUT", endpoint, status); + final Request request = new Request("PUT", endpoint); + addParameters(request, resource.getParameters()); + request.setEntity(entity); - when(client.performRequest("PUT", endpoint, resource.getParameters(), entity)).thenReturn(response); + when(client.performRequest(request)).thenReturn(response); assertThat(resource.putResource(client, logger, resourceBasePath, resourceName, body, resourceType, owner, ownerType), is(expected)); - verify(client).performRequest("PUT", endpoint, resource.getParameters(), entity); + verify(client).performRequest(request); verify(response).getStatusLine(); verify(logger).trace("uploading {} [{}] to the [{}] {}", resourceType, resourceName, owner, ownerType); @@ -388,12 +409,14 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final Response response = response("DELETE", endpoint, status); final Map deleteParameters = deleteParameters(resource.getParameters()); + final Request request = new Request("DELETE", endpoint); + addParameters(request, deleteParameters); - when(client.performRequest("DELETE", endpoint, deleteParameters)).thenReturn(response); + when(client.performRequest(request)).thenReturn(response); assertThat(resource.deleteResource(client, logger, resourceBasePath, resourceName, resourceType, owner, ownerType), is(expected)); - verify(client).performRequest("DELETE", endpoint, deleteParameters); + verify(client).performRequest(request); verify(response).getStatusLine(); verify(logger).trace("deleting {} [{}] from the [{}] {}", resourceType, resourceName, owner, ownerType); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/VersionHttpResourceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/VersionHttpResourceTests.java index d6005b57046..27de4b28cee 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/VersionHttpResourceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/VersionHttpResourceTests.java @@ -6,8 +6,9 @@ package org.elasticsearch.xpack.monitoring.exporter.http; import org.apache.http.entity.ContentType; -import org.apache.http.entity.StringEntity; +import org.apache.http.nio.entity.NStringEntity; import org.elasticsearch.Version; +import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; import org.elasticsearch.test.ESTestCase; @@ -73,8 +74,9 @@ public class VersionHttpResourceTests extends ESTestCase { } public void testDoCheckAndPublishFailedWithIOException() throws IOException { - // request fails for some reason - when(client.performRequest("GET", "/", VersionHttpResource.PARAMETERS)).thenThrow(new IOException("expected")); + Request request = new Request("GET", "/"); + request.addParameter("filter_path", "version.number"); + when(client.performRequest(request)).thenThrow(new IOException("expected")); final VersionHttpResource resource = new VersionHttpResource(owner, Version.CURRENT); @@ -82,12 +84,14 @@ public class VersionHttpResourceTests extends ESTestCase { } private Response responseForJSON(final String json) throws IOException { - final StringEntity entity = new StringEntity(json, ContentType.APPLICATION_JSON); + final NStringEntity entity = new NStringEntity(json, ContentType.APPLICATION_JSON); final Response response = mock(Response.class); when(response.getEntity()).thenReturn(entity); - when(client.performRequest("GET", "/", VersionHttpResource.PARAMETERS)).thenReturn(response); + Request request = new Request("GET", "/"); + request.addParameter("filter_path", "version.number"); + when(client.performRequest(request)).thenReturn(response); return response; }