From 3337fa73511151453580a6ec181efdb3b62de080 Mon Sep 17 00:00:00 2001 From: Chris Earle Date: Tue, 27 Nov 2018 14:28:14 -0500 Subject: [PATCH] [Monitoring] Make Exporters Async (#35765) This changes the exporter code -- most notably the `http` exporter -- to use async operations throughout the resource management and bulk initialization code (the bulk indexing of monitoring documents was already async). As part of this change, this does change one semi-core aspect of the `HttpResource` class in that it will no longer block all concurrent calls until the first call completes with `HttpResource::checkAndPublishIfDirty`. Now, any parallel attempts to check the resources will be skipped until the first call completes (success or failure). While this is a technical change, it has very little practical impact because the existing behavior was either quick success (then every blocked request processed) or each request timed out and failed anyway, thus being effectively skipped (and a burden on the system). --- .../monitoring/MonitoringFeatureSet.java | 2 +- .../xpack/monitoring/MonitoringService.java | 10 +- .../xpack/monitoring/exporter/Exporter.java | 10 +- .../xpack/monitoring/exporter/Exporters.java | 186 +++++--- .../http/ClusterAlertHttpResource.java | 32 +- .../exporter/http/HttpExporter.java | 31 +- .../exporter/http/HttpResource.java | 70 ++- .../exporter/http/MultiHttpResource.java | 36 +- .../exporter/http/PipelineHttpResource.java | 19 +- .../http/PublishableHttpResource.java | 397 ++++++++---------- .../exporter/http/TemplateHttpResource.java | 19 +- .../exporter/http/VersionHttpResource.java | 39 +- .../http/WatcherExistsHttpResource.java | 61 +-- .../exporter/local/LocalExporter.java | 16 +- .../monitoring/MonitoringFeatureSetTests.java | 2 +- .../AbstractIndicesCleanerTestCase.java | 2 +- .../monitoring/exporter/ExportersTests.java | 39 +- ...stractPublishableHttpResourceTestCase.java | 296 +++++++------ .../http/AsyncHttpResourceHelper.java | 117 ++++++ .../http/ClusterAlertHttpResourceTests.java | 28 +- .../exporter/http/HttpExporterIT.java | 59 +-- .../http/HttpExporterResourceTests.java | 246 +++++------ .../exporter/http/HttpExporterTests.java | 61 ++- .../exporter/http/HttpResourceTests.java | 132 ++++-- .../exporter/http/MockHttpResource.java | 40 +- .../exporter/http/MultiHttpResourceTests.java | 30 +- .../http/PipelineHttpResourceTests.java | 29 +- .../http/PublishableHttpResourceTests.java | 213 +++++----- .../http/TemplateHttpResourceTests.java | 29 +- .../http/VersionHttpResourceTests.java | 45 +- .../http/WatcherExistsHttpResourceTests.java | 65 +-- 31 files changed, 1307 insertions(+), 1054 deletions(-) create mode 100644 x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/AsyncHttpResourceHelper.java diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSet.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSet.java index 347a72b4f38..8482bb52de6 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSet.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSet.java @@ -75,7 +75,7 @@ public class MonitoringFeatureSet implements XPackFeatureSet { return null; } Map usage = new HashMap<>(); - for (Exporter exporter : exporters) { + for (Exporter exporter : exporters.getEnabledExporters()) { if (exporter.config().enabled()) { String type = exporter.config().type(); int count = (Integer) usage.getOrDefault(type, 0); diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java index 8f5471cb11f..8759d493c8b 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java @@ -20,7 +20,6 @@ import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; import org.elasticsearch.xpack.monitoring.collector.Collector; -import org.elasticsearch.xpack.monitoring.exporter.Exporter; import org.elasticsearch.xpack.monitoring.exporter.Exporters; import java.io.Closeable; @@ -174,14 +173,7 @@ public class MonitoringService extends AbstractLifecycleComponent { protected void doClose() { logger.debug("monitoring service is closing"); monitor.close(); - - for (Exporter exporter : exporters) { - try { - exporter.close(); - } catch (Exception e) { - logger.error((Supplier) () -> new ParameterizedMessage("failed to close exporter [{}]", exporter.name()), e); - } - } + exporters.close(); logger.debug("monitoring service closed"); } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporter.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporter.java index c33be6dea85..85a6da15177 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporter.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporter.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.monitoring.exporter; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; @@ -93,17 +94,18 @@ public abstract class Exporter implements AutoCloseable { } /** - * Opens up a new export bulk. May return {@code null} indicating this exporter is not ready - * yet to export the docs + * Opens up a new export bulk. + * + * @param listener Returns {@code null} to indicate that this exporter is not ready to export the docs. */ - public abstract ExportBulk openBulk(); + public abstract void openBulk(ActionListener listener); protected final boolean isClosed() { return closed.get(); } @Override - public void close() throws Exception { + public void close() { if (closed.compareAndSet(false, true)) { doClose(); } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporters.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporters.java index ac0d4dcbc06..54d4fde4fbb 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporters.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporters.java @@ -17,7 +17,10 @@ import org.elasticsearch.common.component.Lifecycle; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsException; +import org.elasticsearch.common.util.concurrent.AtomicArray; +import org.elasticsearch.common.util.concurrent.CountDown; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.gateway.GatewayService; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.monitoring.exporter.http.HttpExporter; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; @@ -27,7 +30,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -36,7 +38,7 @@ import java.util.concurrent.atomic.AtomicReference; import static java.util.Collections.emptyMap; -public class Exporters extends AbstractLifecycleComponent implements Iterable { +public class Exporters extends AbstractLifecycleComponent { private static final Logger logger = LogManager.getLogger(Exporters.class); private final Settings settings; @@ -90,9 +92,13 @@ public class Exporters extends AbstractLifecycleComponent implements Iterable iterator() { - return exporters.get().values().iterator(); + /** + * Get all enabled {@linkplain Exporter}s. + * + * @return Never {@code null}. Can be empty if none are enabled. + */ + public Collection getEnabledExporters() { + return exporters.get().values(); } static void closeExporters(Logger logger, Map exporters) { @@ -105,31 +111,6 @@ public class Exporters extends AbstractLifecycleComponent implements Iterable bulks = new ArrayList<>(); - for (Exporter exporter : this) { - try { - ExportBulk bulk = exporter.openBulk(); - if (bulk == null) { - logger.debug("skipping exporter [{}] as it is not ready yet", exporter.name()); - } else { - bulks.add(bulk); - } - } catch (Exception e) { - logger.error( - (Supplier) () -> new ParameterizedMessage("exporter [{}] failed to open exporting bulk", exporter.name()), e); - } - } - return bulks.isEmpty() ? null : new ExportBulk.Compound(bulks, threadContext); - } - Map initExporters(Settings settings) { Set singletons = new HashSet<>(); Map exporters = new HashMap<>(); @@ -180,38 +161,81 @@ public class Exporters extends AbstractLifecycleComponent implements Iterable listener) { + final ClusterState state = clusterService.state(); + + // wait until we have a usable cluster state + if (state.blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK) || + ClusterState.UNKNOWN_UUID.equals(state.metaData().clusterUUID()) || + state.version() == ClusterState.UNKNOWN_VERSION) { + logger.trace("skipping exporters because the cluster state is not loaded"); + + listener.onResponse(null); + return; + } + + final Map exporterMap = exporters.get(); + final AtomicArray accumulatedBulks = new AtomicArray<>(exporterMap.size()); + final CountDown countDown = new CountDown(exporterMap.size()); + + int i = 0; + + // get every exporter's ExportBulk and, when they've all responded, respond with a wrapped version + for (final Exporter exporter : exporterMap.values()) { + exporter.openBulk( + new AccumulatingExportBulkActionListener(exporter.name(), i++, accumulatedBulks, countDown, threadContext, listener)); + } + } + /** * Exports a collection of monitoring documents using the configured exporters */ - public void export(Collection docs, ActionListener listener) throws ExportException { + public void export(final Collection docs, final ActionListener listener) throws ExportException { if (this.lifecycleState() != Lifecycle.State.STARTED) { listener.onFailure(new ExportException("Export service is not started")); } else if (docs != null && docs.size() > 0) { - final ExportBulk bulk = openBulk(); - - if (bulk != null) { - final AtomicReference exceptionRef = new AtomicReference<>(); - try { - bulk.add(docs); - } catch (ExportException e) { - exceptionRef.set(e); - } finally { - bulk.close(lifecycleState() == Lifecycle.State.STARTED, ActionListener.wrap(r -> { - if (exceptionRef.get() == null) { - listener.onResponse(null); - } else { - listener.onFailure(exceptionRef.get()); - } - }, listener::onFailure)); + wrapExportBulk(ActionListener.wrap(bulk -> { + if (bulk != null) { + doExport(bulk, docs, listener); + } else { + listener.onResponse(null); } - } else { - listener.onResponse(null); - } + }, listener::onFailure)); } else { listener.onResponse(null); } } + /** + * Add {@code docs} and send the {@code bulk}, then respond to the {@code listener}. + * + * @param bulk The bulk object to send {@code docs} through. + * @param docs The monitoring documents to send. + * @param listener Returns {@code null} when complete, or failure where relevant. + */ + private void doExport(final ExportBulk bulk, final Collection docs, final ActionListener listener) { + final AtomicReference exceptionRef = new AtomicReference<>(); + + try { + bulk.add(docs); + } catch (ExportException e) { + exceptionRef.set(e); + } finally { + bulk.close(lifecycleState() == Lifecycle.State.STARTED, ActionListener.wrap(r -> { + if (exceptionRef.get() == null) { + listener.onResponse(null); + } else { + listener.onFailure(exceptionRef.get()); + } + }, listener::onFailure)); + } + } + /** * Return all the settings of all the exporters, no matter if HTTP or Local */ @@ -221,4 +245,66 @@ public class Exporters extends AbstractLifecycleComponent implements Iterable { + + private final String name; + private final int indexPosition; + private final AtomicArray accumulatedBulks; + private final CountDown countDown; + private final ActionListener delegate; + private final ThreadContext threadContext; + + AccumulatingExportBulkActionListener(final String name, + final int indexPosition, final AtomicArray accumulatedBulks, + final CountDown countDown, + final ThreadContext threadContext, final ActionListener delegate) { + this.name = name; + this.indexPosition = indexPosition; + this.accumulatedBulks = accumulatedBulks; + this.countDown = countDown; + this.threadContext = threadContext; + this.delegate = delegate; + } + + @Override + public void onResponse(final ExportBulk exportBulk) { + if (exportBulk == null) { + logger.debug("skipping exporter [{}] as it is not ready yet", name); + } else { + accumulatedBulks.set(indexPosition, exportBulk); + } + + delegateIfComplete(); + } + + @Override + public void onFailure(Exception e) { + logger.error((Supplier) () -> new ParameterizedMessage("exporter [{}] failed to open exporting bulk", name), e); + + delegateIfComplete(); + } + + /** + * Once all {@linkplain Exporter}'s have responded, whether it was success or failure, then this responds with all successful + * {@linkplain ExportBulk}s wrapped using an {@linkplain ExportBulk.Compound} wrapper. + */ + void delegateIfComplete() { + if (countDown.countDown()) { + final List bulkList = accumulatedBulks.asList(); + + if (bulkList.isEmpty()) { + delegate.onResponse(null); + } else { + delegate.onResponse(new ExportBulk.Compound(bulkList, threadContext)); + } + } + } + + } + } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/ClusterAlertHttpResource.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/ClusterAlertHttpResource.java index e035668841f..d944c0d549c 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/ClusterAlertHttpResource.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/ClusterAlertHttpResource.java @@ -13,6 +13,7 @@ import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; import org.elasticsearch.common.CheckedFunction; @@ -79,34 +80,33 @@ public class ClusterAlertHttpResource extends PublishableHttpResource { * Determine if the current {@linkplain #watchId Watch} exists. */ @Override - protected CheckResponse doCheck(final RestClient client) { + protected void doCheck(final RestClient client, final ActionListener listener) { // if we should be adding, then we need to check for existence if (isWatchDefined() && licenseState.isMonitoringClusterAlertsAllowed()) { final CheckedFunction watchChecker = (response) -> shouldReplaceClusterAlert(response, XContentType.JSON.xContent(), LAST_UPDATED_VERSION); - return versionCheckForResource(client, logger, - "/_xpack/watcher/watch", watchId.get(), "monitoring cluster alert", - resourceOwnerName, "monitoring cluster", - watchChecker); + checkForResource(client, listener, logger, + "/_xpack/watcher/watch", watchId.get(), "monitoring cluster alert", + resourceOwnerName, "monitoring cluster", + GET_EXISTS, GET_DOES_NOT_EXIST, + watchChecker, this::alwaysReplaceResource); + } else { + // if we should be deleting, then just try to delete it (same level of effort as checking) + deleteResource(client, listener, logger, "/_xpack/watcher/watch", watchId.get(), + "monitoring cluster alert", + resourceOwnerName, "monitoring cluster"); } - - // if we should be deleting, then just try to delete it (same level of effort as checking) - final boolean deleted = deleteResource(client, logger, "/_xpack/watcher/watch", watchId.get(), - "monitoring cluster alert", - resourceOwnerName, "monitoring cluster"); - - return deleted ? CheckResponse.EXISTS : CheckResponse.ERROR; } /** * Publish the missing {@linkplain #watchId Watch}. */ @Override - protected boolean doPublish(final RestClient client) { - return putResource(client, logger, - "/_xpack/watcher/watch", watchId.get(), this::watchToHttpEntity, "monitoring cluster alert", - resourceOwnerName, "monitoring cluster"); + protected void doPublish(final RestClient client, final ActionListener listener) { + putResource(client, listener, logger, + "/_xpack/watcher/watch", watchId.get(), this::watchToHttpEntity, "monitoring cluster alert", + resourceOwnerName, "monitoring cluster"); } /** diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java index 9f13f58065f..e89f3c3e64d 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java @@ -16,6 +16,7 @@ import org.apache.http.nio.conn.ssl.SSLIOSessionStrategy; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.Version; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.RestClient; import org.elasticsearch.client.RestClientBuilder; import org.elasticsearch.client.sniff.ElasticsearchNodesSniffer; @@ -38,6 +39,7 @@ import org.elasticsearch.xpack.core.ssl.SSLConfiguration; import org.elasticsearch.xpack.core.ssl.SSLConfigurationSettings; import org.elasticsearch.xpack.core.ssl.SSLService; import org.elasticsearch.xpack.monitoring.exporter.ClusterAlertsUtil; +import org.elasticsearch.xpack.monitoring.exporter.ExportBulk; import org.elasticsearch.xpack.monitoring.exporter.Exporter; import org.joda.time.format.DateTimeFormatter; @@ -661,12 +663,8 @@ public class HttpExporter extends Exporter { } } - /** - * Determine if this {@link HttpExporter} is ready to use. - * - * @return {@code true} if it is ready. {@code false} if not. - */ - boolean isExporterReady() { + @Override + public void openBulk(final ActionListener listener) { final boolean canUseClusterAlerts = config.licenseState().isMonitoringClusterAlertsAllowed(); // if this changes between updates, then we need to add OR remove the watches @@ -674,19 +672,16 @@ public class HttpExporter extends Exporter { resource.markDirty(); } - // block until all resources are verified to exist - return resource.checkAndPublishIfDirty(client); - } + resource.checkAndPublishIfDirty(client, ActionListener.wrap((success) -> { + if (success) { + final String name = "xpack.monitoring.exporters." + config.name(); - @Override - public HttpExportBulk openBulk() { - // block until all resources are verified to exist - if (isExporterReady()) { - String name = "xpack.monitoring.exporters." + config.name(); - return new HttpExportBulk(name, client, defaultParams, dateTimeFormatter, threadContext); - } - - return null; + listener.onResponse(new HttpExportBulk(name, client, defaultParams, dateTimeFormatter, threadContext)); + } else { + // we're not ready yet, so keep waiting + listener.onResponse(null); + } + }, listener::onFailure)); } @Override diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpResource.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpResource.java index 66ca68615c6..557ed960399 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpResource.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpResource.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.monitoring.exporter.http; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.RestClient; import java.util.Objects; @@ -83,7 +84,7 @@ public abstract class HttpResource { * Determine if the resource needs to be checked. * * @return {@code true} to indicate that the resource should block follow-on actions that require it. - * @see #checkAndPublish(RestClient) + * @see #checkAndPublish(RestClient, ActionListener) */ public boolean isDirty() { return state.get() != State.CLEAN; @@ -101,35 +102,22 @@ public abstract class HttpResource { *

* Expected usage: * 


-     * if (resource.checkAndPublishIfDirty(client)) {
-     *     // use client with resources having been verified
-     * }
+     * resource.checkAndPublishIfDirty(client, ActionListener.wrap((success) -> {
+     *     if (success) {
+     *         // use client with resources having been verified
+     *     }
+     * }, listener::onFailure);
      *
* * @param client The REST client to make the request(s). - * @return {@code true} if the resource is available for use. {@code false} to stop. + * @param listener Returns {@code true} if the resource is available for use. {@code false} to stop. */ - public final boolean checkAndPublishIfDirty(final RestClient client) { - final State state = this.state.get(); - - // get in line and wait until the check passes or fails if it's checking now, or start checking - return state == State.CLEAN || blockUntilCheckAndPublish(client); - } - - /** - * Invoked by {@link #checkAndPublishIfDirty(RestClient)} to block incase {@link #checkAndPublish(RestClient)} is in the middle of - * {@linkplain State#CHECKING checking}. - *

- * Unlike {@link #isDirty()} and {@link #checkAndPublishIfDirty(RestClient)}, this is {@code synchronized} in order to prevent - * double-execution and it invokes {@link #checkAndPublish(RestClient)} if it's {@linkplain State#DIRTY dirty}. - * - * @param client The REST client to make the request(s). - * @return {@code true} if the resource is available for use. {@code false} to stop. - */ - private synchronized boolean blockUntilCheckAndPublish(final RestClient client) { - final State state = this.state.get(); - - return state == State.CLEAN || (state == State.DIRTY && checkAndPublish(client)); + public final void checkAndPublishIfDirty(final RestClient client, final ActionListener listener) { + if (state.get() == State.CLEAN) { + listener.onResponse(true); + } else { + checkAndPublish(client, listener); + } } /** @@ -143,30 +131,30 @@ public abstract class HttpResource { * still be dirty at the end, but the success of it will still return based on the checks it ran. * * @param client The REST client to make the request(s). - * @return {@code true} if the resource is available for use. {@code false} to stop. + * @param listener Returns {@code true} if the resource is available for use. {@code false} to stop. * @see #isDirty() */ - public final synchronized boolean checkAndPublish(final RestClient client) { - // we always check when asked, regardless of clean or dirty - state.set(State.CHECKING); - - boolean success = false; - - try { - success = doCheckAndPublish(client); - } finally { - state.compareAndSet(State.CHECKING, success ? State.CLEAN : State.DIRTY); + public final void checkAndPublish(final RestClient client, final ActionListener listener) { + // we always check when asked, regardless of clean or dirty, but we do not run parallel checks + if (state.getAndSet(State.CHECKING) != State.CHECKING) { + doCheckAndPublish(client, ActionListener.wrap(success -> { + state.compareAndSet(State.CHECKING, success ? State.CLEAN : State.DIRTY); + listener.onResponse(success); + }, e -> { + state.compareAndSet(State.CHECKING, State.DIRTY); + listener.onFailure(e); + })); + } else { + listener.onResponse(false); } - - return success; } /** * Perform whatever is necessary to check and publish this {@link HttpResource}. * * @param client The REST client to make the request(s). - * @return {@code true} if the resource is available for use. {@code false} to stop. + * @param listener Returns {@code true} if the resource is available for use. {@code false} to stop. */ - protected abstract boolean doCheckAndPublish(RestClient client); + protected abstract void doCheckAndPublish(RestClient client, ActionListener listener); } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/MultiHttpResource.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/MultiHttpResource.java index 12e2f3f3f1a..d902abf7113 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/MultiHttpResource.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/MultiHttpResource.java @@ -5,8 +5,10 @@ */ package org.elasticsearch.xpack.monitoring.exporter.http; +import java.util.Iterator; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.RestClient; import java.util.Collections; @@ -38,6 +40,10 @@ public class MultiHttpResource extends HttpResource { public MultiHttpResource(final String resourceOwnerName, final List resources) { super(resourceOwnerName); + if (resources.isEmpty()) { + throw new IllegalArgumentException("[resources] cannot be empty"); + } + this.resources = Collections.unmodifiableList(resources); } @@ -54,22 +60,34 @@ public class MultiHttpResource extends HttpResource { * Check and publish all {@linkplain #resources sub-resources}. */ @Override - protected boolean doCheckAndPublish(RestClient client) { + protected void doCheckAndPublish(final RestClient client, final ActionListener listener) { logger.trace("checking sub-resources existence and publishing on the [{}]", resourceOwnerName); - boolean exists = true; + final Iterator iterator = resources.iterator(); // short-circuits on the first failure, thus marking the whole thing dirty - for (final HttpResource resource : resources) { - if (resource.checkAndPublish(client) == false) { - exists = false; - break; + iterator.next().checkAndPublish(client, new ActionListener() { + + @Override + public void onResponse(final Boolean success) { + // short-circuit on the first failure + if (success && iterator.hasNext()) { + iterator.next().checkAndPublish(client, this); + } else { + logger.trace("all sub-resources exist [{}] on the [{}]", success, resourceOwnerName); + + listener.onResponse(success); + } } - } - logger.trace("all sub-resources exist [{}] on the [{}]", exists, resourceOwnerName); + @Override + public void onFailure(final Exception e) { + logger.trace("all sub-resources exist [false] on the [{}]", resourceOwnerName); - return exists; + listener.onFailure(e); + } + + }); } } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/PipelineHttpResource.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/PipelineHttpResource.java index 78ca43ff9bb..206ef924c53 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/PipelineHttpResource.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/PipelineHttpResource.java @@ -10,6 +10,7 @@ import org.apache.http.entity.ByteArrayEntity; import org.apache.http.entity.ContentType; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.RestClient; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.unit.TimeValue; @@ -58,21 +59,21 @@ public class PipelineHttpResource extends PublishableHttpResource { * Determine if the current {@linkplain #pipelineName pipeline} exists. */ @Override - protected CheckResponse doCheck(final RestClient client) { - return versionCheckForResource(client, logger, - "/_ingest/pipeline", pipelineName, "monitoring pipeline", - resourceOwnerName, "monitoring cluster", - XContentType.JSON.xContent(), MonitoringTemplateUtils.LAST_UPDATED_VERSION); + protected void doCheck(final RestClient client, final ActionListener listener) { + versionCheckForResource(client, listener, logger, + "/_ingest/pipeline", pipelineName, "monitoring pipeline", + resourceOwnerName, "monitoring cluster", + XContentType.JSON.xContent(), MonitoringTemplateUtils.LAST_UPDATED_VERSION); } /** * Publish the current {@linkplain #pipelineName pipeline}. */ @Override - protected boolean doPublish(final RestClient client) { - return putResource(client, logger, - "/_ingest/pipeline", pipelineName, this::pipelineToHttpEntity, "monitoring pipeline", - resourceOwnerName, "monitoring cluster"); + protected void doPublish(final RestClient client, final ActionListener listener) { + putResource(client, listener, logger, + "/_ingest/pipeline", pipelineName, this::pipelineToHttpEntity, "monitoring pipeline", + resourceOwnerName, "monitoring cluster"); } /** 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 a804703a530..143132b3dd0 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,13 +9,14 @@ 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.action.ActionListener; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; +import org.elasticsearch.client.ResponseListener; import org.elasticsearch.client.RestClient; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.common.xcontent.XContent; @@ -33,33 +34,11 @@ import java.util.stream.Collectors; * {@code PublishableHttpResource} represents an {@link HttpResource} that is a single file or object that can be checked and * published in the event that the check does not pass. * - * @see #doCheck(RestClient) - * @see #doPublish(RestClient) + * @see #doCheck(RestClient, ActionListener) + * @see #doPublish(RestClient, ActionListener) */ public abstract class PublishableHttpResource extends HttpResource { - /** - * {@code CheckResponse} provides a ternary state for {@link #doCheck(RestClient)}. - */ - public enum CheckResponse { - - /** - * The check found the resource, so nothing needs to be published. - *

- * This can also be used to skip the publishing portion if desired. - */ - EXISTS, - /** - * The check did not find the resource, so we need to attempt to publish it. - */ - DOES_NOT_EXIST, - /** - * The check hit an unexpected exception that should block publishing attempts until it can check again. - */ - ERROR - - } - /** * A value that will never match anything in the JSON response body, thus limiting it to "{}". */ @@ -142,56 +121,27 @@ public abstract class PublishableHttpResource extends HttpResource { * Perform whatever is necessary to check and publish this {@link PublishableHttpResource}. * * @param client The REST client to make the request(s). - * @return {@code true} if the resource is available for use. {@code false} to stop. + * @param listener Returns {@code true} if the resource is available for use. {@code false} to stop. */ @Override - protected final boolean doCheckAndPublish(final RestClient client) { - final CheckResponse check = doCheck(client); - - // errors cause a dead-stop - return check != CheckResponse.ERROR && (check == CheckResponse.EXISTS || doPublish(client)); + protected final void doCheckAndPublish(final RestClient client, final ActionListener listener) { + doCheck(client, ActionListener.wrap(exists -> { + if (exists) { + // it already exists, so we can skip publishing it + listener.onResponse(true); + } else { + doPublish(client, listener); + } + }, listener::onFailure)); } /** * Determine if the current resource exists. - *

    - *
  • - * {@link CheckResponse#EXISTS EXISTS} will not run {@link #doPublish(RestClient)} and mark this as not dirty. - *
    • - *
  • - * {@link CheckResponse#DOES_NOT_EXIST DOES_NOT_EXIST} will run {@link #doPublish(RestClient)}, which determines the dirtiness. - *
    • - *
  • {@link CheckResponse#ERROR ERROR} will not run {@link #doPublish(RestClient)} and mark this as dirty.
    • - *
* * @param client The REST client to make the request(s). - * @return Never {@code null}. + * @param listener Returns {@code true} if the resource already available to use. {@code false} otherwise. */ - protected abstract CheckResponse doCheck(RestClient client); - - /** - * Determine if the current {@code resourceName} exists at the {@code resourceBasePath} endpoint. - *

- * This provides the base-level check for any resource that does not need to care about its response beyond existence (and likely does - * not need to inspect its contents). - * - * @param client The REST client to make the request(s). - * @param logger The logger to use for status messages. - * @param resourceBasePath The base path/endpoint to check for the resource (e.g., "/_template"). - * @param resourceName The name of the resource (e.g., "template123"). - * @param resourceType The type of resource (e.g., "monitoring template"). - * @param resourceOwnerName The user-recognizeable resource owner. - * @param resourceOwnerType The type of resource owner being dealt with (e.g., "monitoring cluster"). - * @return Never {@code null}. - */ - protected CheckResponse simpleCheckForResource(final RestClient client, final Logger logger, - final String resourceBasePath, - final String resourceName, final String resourceType, - final String resourceOwnerName, final String resourceOwnerType) { - return checkForResource(client, logger, resourceBasePath, resourceName, resourceType, resourceOwnerName, resourceOwnerType, - GET_EXISTS, GET_DOES_NOT_EXIST) - .v1(); - } + protected abstract void doCheck(RestClient client, ActionListener listener); /** * Determine if the current {@code resourceName} exists at the {@code resourceBasePath} endpoint with a version greater than or equal @@ -210,6 +160,7 @@ public abstract class PublishableHttpResource extends HttpResource { * * * @param client The REST client to make the request(s). + * @param listener Returns {@code true} if the resource was successfully published. {@code false} otherwise. * @param logger The logger to use for status messages. * @param resourceBasePath The base path/endpoint to check for the resource (e.g., "/_template"). * @param resourceName The name of the resource (e.g., "template123"). @@ -218,73 +169,23 @@ public abstract class PublishableHttpResource extends HttpResource { * @param resourceOwnerType The type of resource owner being dealt with (e.g., "monitoring cluster"). * @param xContent The XContent used to parse the response. * @param minimumVersion The minimum version allowed without being replaced (expected to be the last updated version). - * @return Never {@code null}. */ - protected CheckResponse versionCheckForResource(final RestClient client, final Logger logger, - final String resourceBasePath, - final String resourceName, final String resourceType, - final String resourceOwnerName, final String resourceOwnerType, - final XContent xContent, final int minimumVersion) { + protected void versionCheckForResource(final RestClient client, + final ActionListener listener, + final Logger logger, + final String resourceBasePath, + final String resourceName, + final String resourceType, + final String resourceOwnerName, + final String resourceOwnerType, + final XContent xContent, + final int minimumVersion) { final CheckedFunction responseChecker = - (response) -> shouldReplaceResource(response, xContent, resourceName, minimumVersion); + (response) -> shouldReplaceResource(response, xContent, resourceName, minimumVersion); - return versionCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, resourceOwnerName, resourceOwnerType, - responseChecker); - } - - /** - * Determine if the current {@code resourceName} exists at the {@code resourceBasePath} endpoint with a version greater than or equal - * to the expected version. - *

- * This provides the base-level check for any resource that does not need to care about its response beyond existence (and likely does - * not need to inspect its contents). - *

- * This expects responses in the form of: - * 


-     * {
-     *   "resourceName": {
-     *     "version": 6000002
-     *   }
-     * }
-     *
- * - * @param client The REST client to make the request(s). - * @param logger The logger to use for status messages. - * @param resourceBasePath The base path/endpoint to check for the resource (e.g., "/_template"). - * @param resourceName The name of the resource (e.g., "template123"). - * @param resourceType The type of resource (e.g., "monitoring template"). - * @param resourceOwnerName The user-recognizeable resource owner. - * @param resourceOwnerType The type of resource owner being dealt with (e.g., "monitoring cluster"). - * @param responseChecker Determine if the resource should be replaced given the response. - * @return Never {@code null}. - */ - protected CheckResponse versionCheckForResource(final RestClient client, final Logger logger, - final String resourceBasePath, - final String resourceName, final String resourceType, - final String resourceOwnerName, final String resourceOwnerType, - final CheckedFunction responseChecker) { - final Tuple response = - checkForResource(client, logger, resourceBasePath, resourceName, resourceType, resourceOwnerName, resourceOwnerType, - GET_EXISTS, GET_DOES_NOT_EXIST); - - final CheckResponse checkResponse = response.v1(); - - // if the template exists, then we also need to check its version - if (checkResponse == CheckResponse.EXISTS) { - try { - // replace the resource because the version is below what was required - if (responseChecker.apply(response.v2())) { - return CheckResponse.DOES_NOT_EXIST; - } - } catch (IOException | RuntimeException e) { - logger.error((Supplier) () -> new ParameterizedMessage("failed to parse [{}/{}] on the [{}]", - resourceBasePath, resourceName, resourceOwnerName), e); - - return CheckResponse.ERROR; - } - } - - return checkResponse; + checkForResource(client, listener, logger, + resourceBasePath, resourceName, resourceType, resourceOwnerName, resourceOwnerType, + GET_EXISTS, GET_DOES_NOT_EXIST, responseChecker, this::alwaysReplaceResource); } /** @@ -293,6 +194,7 @@ public abstract class PublishableHttpResource extends HttpResource { * This provides the base-level check for any resource that cares about existence and also its contents. * * @param client The REST client to make the request(s). + * @param listener Returns {@code true} if the resource was successfully published. {@code false} otherwise. * @param logger The logger to use for status messages. * @param resourceBasePath The base path/endpoint to check for the resource (e.g., "/_template"), if any. * @param resourceName The name of the resource (e.g., "template123"). @@ -301,76 +203,99 @@ public abstract class PublishableHttpResource extends HttpResource { * @param resourceOwnerType The type of resource owner being dealt with (e.g., "monitoring cluster"). * @param exists Response codes that represent {@code EXISTS}. * @param doesNotExist Response codes that represent {@code DOES_NOT_EXIST}. - * @return Never {@code null} pair containing the checked response and the returned response. - * The response will only ever be {@code null} if none was returned. - * @see #simpleCheckForResource(RestClient, Logger, String, String, String, String, String) + * @param responseChecker Returns {@code true} if the resource should be replaced. + * @param doesNotExistResponseChecker Returns {@code true} if the resource should be replaced. */ - protected Tuple checkForResource(final RestClient client, final Logger logger, - final String resourceBasePath, - final String resourceName, final String resourceType, - final String resourceOwnerName, final String resourceOwnerType, - final Set exists, final Set doesNotExist) { + protected void checkForResource(final RestClient client, + final ActionListener listener, + final Logger logger, + final String resourceBasePath, + final String resourceName, + final String resourceType, + final String resourceOwnerName, + final String resourceOwnerType, + final Set exists, + final Set doesNotExist, + final CheckedFunction responseChecker, + final CheckedFunction doesNotExistResponseChecker) { logger.trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, resourceOwnerName, resourceOwnerType); - final Request request = new Request("GET", resourceBasePath + "/" + resourceName); addParameters(request); // avoid exists and DNE parameters from being an exception by default 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(request); - final int statusCode = response.getStatusLine().getStatusCode(); + client.performRequestAsync(request, new ResponseListener() { - // checking the content is the job of whoever called this function by checking the tuple's response - if (exists.contains(statusCode)) { - logger.debug("{} [{}] found on the [{}] {}", resourceType, resourceName, resourceOwnerName, resourceOwnerType); + @Override + public void onSuccess(final Response response) { + try { + final int statusCode = response.getStatusLine().getStatusCode(); - return new Tuple<>(CheckResponse.EXISTS, response); - } else if (doesNotExist.contains(statusCode)) { - logger.debug("{} [{}] does not exist on the [{}] {}", resourceType, resourceName, resourceOwnerName, resourceOwnerType); + // checking the content is the job of whoever called this function by checking the tuple's response + if (exists.contains(statusCode)) { + logger.debug("{} [{}] found on the [{}] {}", resourceType, resourceName, resourceOwnerName, resourceOwnerType); - return new Tuple<>(CheckResponse.DOES_NOT_EXIST, response); - } else { - throw new ResponseException(response); + // if we should replace it -- true -- then the resource "does not exist" as far as the caller is concerned + listener.onResponse(false == responseChecker.apply(response)); + + } else if (doesNotExist.contains(statusCode)) { + logger.debug("{} [{}] does not exist on the [{}] {}", + resourceType, resourceName, resourceOwnerName, resourceOwnerType); + + // if we should replace it -- true -- then the resource "does not exist" as far as the caller is concerned + listener.onResponse(false == doesNotExistResponseChecker.apply(response)); + } else { + onFailure(new ResponseException(response)); + } + } catch (Exception e) { + logger.error((Supplier) () -> new ParameterizedMessage("failed to parse [{}/{}] on the [{}]", + resourceBasePath, resourceName, resourceOwnerName), + e); + + onFailure(e); + } } - } catch (final ResponseException e) { - final Response response = e.getResponse(); - final int statusCode = response.getStatusLine().getStatusCode(); - logger.error((Supplier) () -> - new ParameterizedMessage("failed to verify {} [{}] on the [{}] {} with status code [{}]", - resourceType, resourceName, resourceOwnerName, resourceOwnerType, statusCode), - e); + @Override + public void onFailure(final Exception exception) { + if (exception instanceof ResponseException) { + final Response response = ((ResponseException)exception).getResponse(); + final int statusCode = response.getStatusLine().getStatusCode(); - // weirder failure than below; block responses just like other unexpected failures - return new Tuple<>(CheckResponse.ERROR, response); - } catch (IOException | RuntimeException e) { - logger.error((Supplier) () -> - new ParameterizedMessage("failed to verify {} [{}] on the [{}] {}", - resourceType, resourceName, resourceOwnerName, resourceOwnerType), - e); + logger.error((Supplier) () -> + new ParameterizedMessage("failed to verify {} [{}] on the [{}] {} with status code [{}]", + resourceType, resourceName, resourceOwnerName, resourceOwnerType, statusCode), + exception); + } else { + logger.error((Supplier) () -> + new ParameterizedMessage("failed to verify {} [{}] on the [{}] {}", + resourceType, resourceName, resourceOwnerName, resourceOwnerType), + exception); + } - // do not attempt to publish the resource because we're in a broken state - return new Tuple<>(CheckResponse.ERROR, null); - } + listener.onFailure(exception); + } + + }); } /** * Publish the current resource. *

- * This is only invoked if {@linkplain #doCheck(RestClient) the check} fails. + * This is only invoked if {@linkplain #doCheck(RestClient, ActionListener) the check} fails. * * @param client The REST client to make the request(s). - * @return {@code true} if it exists. + * @param listener Returns {@code true} if the resource is available to use. Otherwise {@code false}. */ - protected abstract boolean doPublish(RestClient client); + protected abstract void doPublish(RestClient client, ActionListener listener); /** * Upload the {@code resourceName} to the {@code resourceBasePath} endpoint. * * @param client The REST client to make the request(s). + * @param listener Returns {@code true} if the resource was successfully published. {@code false} otherwise. * @param logger The logger to use for status messages. * @param resourceBasePath The base path/endpoint to check for the resource (e.g., "/_template"). * @param resourceName The name of the resource (e.g., "template123"). @@ -379,39 +304,49 @@ public abstract class PublishableHttpResource extends HttpResource { * @param resourceOwnerName The user-recognizeable resource owner. * @param resourceOwnerType The type of resource owner being dealt with (e.g., "monitoring cluster"). */ - protected boolean putResource(final RestClient client, final Logger logger, - final String resourceBasePath, - final String resourceName, final java.util.function.Supplier body, - final String resourceType, - final String resourceOwnerName, final String resourceOwnerType) { + protected void putResource(final RestClient client, + final ActionListener listener, + final Logger logger, + final String resourceBasePath, + final String resourceName, + final java.util.function.Supplier body, + final String resourceType, + final String resourceOwnerName, + final String resourceOwnerType) { logger.trace("uploading {} [{}] to the [{}] {}", resourceType, resourceName, resourceOwnerName, resourceOwnerType); - boolean success = false; final Request request = new Request("PUT", resourceBasePath + "/" + resourceName); addParameters(request); request.setEntity(body.get()); - try { - final Response response = client.performRequest(request); - final int statusCode = response.getStatusLine().getStatusCode(); + client.performRequestAsync(request, new ResponseListener() { - // 200 or 201 - if (statusCode == RestStatus.OK.getStatus() || statusCode == RestStatus.CREATED.getStatus()) { - logger.debug("{} [{}] uploaded to the [{}] {}", resourceType, resourceName, resourceOwnerName, resourceOwnerType); + @Override + public void onSuccess(final Response response) { + final int statusCode = response.getStatusLine().getStatusCode(); - success = true; - } else { - throw new RuntimeException("[" + resourceBasePath + "/" + resourceName + "] responded with [" + statusCode + "]"); + // 200 or 201 + if (statusCode == RestStatus.OK.getStatus() || statusCode == RestStatus.CREATED.getStatus()) { + logger.debug("{} [{}] uploaded to the [{}] {}", resourceType, resourceName, resourceOwnerName, resourceOwnerType); + + listener.onResponse(true); + } else { + onFailure(new RuntimeException("[" + resourceBasePath + "/" + resourceName + "] responded with [" + statusCode + "]")); + } } - } catch (IOException | RuntimeException e) { - logger.error((Supplier) () -> - new ParameterizedMessage("failed to upload {} [{}] on the [{}] {}", - resourceType, resourceName, resourceOwnerName, resourceOwnerType), - e); - } - return success; + @Override + public void onFailure(final Exception exception) { + logger.error((Supplier) () -> + new ParameterizedMessage("failed to upload {} [{}] on the [{}] {}", + resourceType, resourceName, resourceOwnerName, resourceOwnerType), + exception); + + listener.onFailure(exception); + } + + }); } /** @@ -422,48 +357,59 @@ public abstract class PublishableHttpResource extends HttpResource { * responses will result in {@code false} and logged failure. * * @param client The REST client to make the request(s). + * @param listener Returns {@code true} if it successfully deleted the item; never {@code false}. * @param logger The logger to use for status messages. * @param resourceBasePath The base path/endpoint to check for the resource (e.g., "/_template"). * @param resourceName The name of the resource (e.g., "template123"). * @param resourceType The type of resource (e.g., "monitoring template"). * @param resourceOwnerName The user-recognizeable resource owner. * @param resourceOwnerType The type of resource owner being dealt with (e.g., "monitoring cluster"). - * @return {@code true} if it successfully deleted the item; otherwise {@code false}. */ - protected boolean deleteResource(final RestClient client, final Logger logger, - final String resourceBasePath, final String resourceName, - final String resourceType, - final String resourceOwnerName, final String resourceOwnerType) { + protected void deleteResource(final RestClient client, + final ActionListener listener, + final Logger logger, + final String resourceBasePath, + final String resourceName, + final String resourceType, + final String resourceOwnerName, + final String resourceOwnerType) { logger.trace("deleting {} [{}] from the [{}] {}", resourceType, resourceName, resourceOwnerName, resourceOwnerType); - boolean success = false; - - Request request = new Request("DELETE", resourceBasePath + "/" + resourceName); + final 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(request); - final int statusCode = response.getStatusLine().getStatusCode(); + client.performRequestAsync(request, new ResponseListener() { - // 200 or 404 (not found is just as good as deleting it!) - if (statusCode == RestStatus.OK.getStatus() || statusCode == RestStatus.NOT_FOUND.getStatus()) { - logger.debug("{} [{}] deleted from the [{}] {}", resourceType, resourceName, resourceOwnerName, resourceOwnerType); + @Override + public void onSuccess(Response response) { + final int statusCode = response.getStatusLine().getStatusCode(); - success = true; - } else { - throw new RuntimeException("[" + resourceBasePath + "/" + resourceName + "] responded with [" + statusCode + "]"); + // 200 or 404 (not found is just as good as deleting it!) + if (statusCode == RestStatus.OK.getStatus() || statusCode == RestStatus.NOT_FOUND.getStatus()) { + logger.debug("{} [{}] deleted from the [{}] {}", resourceType, resourceName, resourceOwnerName, resourceOwnerType); + + listener.onResponse(true); + } else { + onFailure(new RuntimeException("[" + resourceBasePath + "/" + resourceName + "] responded with [" + statusCode + "]")); + } } - } catch (IOException | RuntimeException e) { - logger.error((Supplier) () -> new ParameterizedMessage("failed to delete {} [{}] on the [{}] {}", - resourceType, resourceName, resourceOwnerName, resourceOwnerType), - e); - } - return success; + @Override + public void onFailure(Exception exception) { + logger.error((Supplier) () -> + new ParameterizedMessage("failed to delete {} [{}] on the [{}] {}", + resourceType, resourceName, resourceOwnerName, resourceOwnerType), + exception); + + listener.onFailure(exception); + } + + }); } /** @@ -498,18 +444,27 @@ public abstract class PublishableHttpResource extends HttpResource { final Map resource = (Map) resources.get(resourceName); final Object version = resource != null ? resource.get("version") : null; - // if we don't have it (perhaps more fields were returned), then we need to replace it + // the version in the template is expected to include the alpha/beta/rc codes as well if (version instanceof Number) { - // the version in the template is expected to include the alpha/beta/rc codes as well - return ((Number)version).intValue() < minimumVersion; + return ((Number) version).intValue() < minimumVersion; } } return true; } - private void addParameters(Request request) { - for (Map.Entry param : parameters.entrySet()) { + /** + * A useful placeholder for {@link CheckedFunction}s that want to always return {@code true}. + * + * @param response Unused. + * @return Always {@code true}. + */ + protected boolean alwaysReplaceResource(final Response response) { + return true; + } + + private void addParameters(final Request request) { + for (final 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/TemplateHttpResource.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/TemplateHttpResource.java index 5e6b056959c..5e6ed790b1a 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/TemplateHttpResource.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/TemplateHttpResource.java @@ -10,6 +10,7 @@ import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.RestClient; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.unit.TimeValue; @@ -61,21 +62,21 @@ public class TemplateHttpResource extends PublishableHttpResource { * @see MonitoringTemplateUtils#LAST_UPDATED_VERSION */ @Override - protected CheckResponse doCheck(final RestClient client) { - return versionCheckForResource(client, logger, - "/_template", templateName, "monitoring template", - resourceOwnerName, "monitoring cluster", - XContentType.JSON.xContent(), MonitoringTemplateUtils.LAST_UPDATED_VERSION); + protected void doCheck(final RestClient client, final ActionListener listener) { + versionCheckForResource(client, listener, logger, + "/_template", templateName, "monitoring template", + resourceOwnerName, "monitoring cluster", + XContentType.JSON.xContent(), MonitoringTemplateUtils.LAST_UPDATED_VERSION); } /** * Publish the missing {@linkplain #templateName template}. */ @Override - protected boolean doPublish(final RestClient client) { - return putResource(client, logger, - "/_template", templateName, this::templateToHttpEntity, "monitoring template", - resourceOwnerName, "monitoring cluster"); + protected void doPublish(final RestClient client, final ActionListener listener) { + putResource(client, listener, logger, + "/_template", templateName, this::templateToHttpEntity, "monitoring template", + resourceOwnerName, "monitoring cluster"); } /** 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 252d9072685..52ac37ef9b7 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 @@ -10,8 +10,10 @@ 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.action.ActionListener; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseListener; import org.elasticsearch.client.RestClient; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.json.JsonXContent; @@ -50,22 +52,33 @@ public class VersionHttpResource extends HttpResource { * If it does not, then there is nothing that can be done except wait until it does. There is no publishing aspect to this operation. */ @Override - protected boolean doCheckAndPublish(final RestClient client) { + protected void doCheckAndPublish(final RestClient client, final ActionListener listener) { logger.trace("checking [{}] to ensure that it supports the minimum version [{}]", resourceOwnerName, minimumVersion); - try { - Request request = new Request("GET", "/"); - request.addParameter("filter_path", "version.number"); - return validateVersion(client.performRequest(request)); - } catch (IOException | RuntimeException e) { - logger.error( - (Supplier)() -> - new ParameterizedMessage("failed to verify minimum version [{}] on the [{}] monitoring cluster", - minimumVersion, resourceOwnerName), - e); - } + final Request request = new Request("GET", "/"); + request.addParameter("filter_path", "version.number"); - return false; + client.performRequestAsync(request, new ResponseListener() { + @Override + public void onSuccess(final Response response) { + try { + // malformed responses can cause exceptions during validation + listener.onResponse(validateVersion(response)); + } catch (Exception e) { + onFailure(e); + } + } + + @Override + public void onFailure(final Exception exception) { + logger.error((Supplier) () -> + new ParameterizedMessage("failed to verify minimum version [{}] on the [{}] monitoring cluster", + minimumVersion, resourceOwnerName), + exception); + + listener.onFailure(exception); + } + }); } /** diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/WatcherExistsHttpResource.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/WatcherExistsHttpResource.java index 93eed61f4c5..87aae0367a5 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/WatcherExistsHttpResource.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/WatcherExistsHttpResource.java @@ -7,12 +7,11 @@ package org.elasticsearch.xpack.monitoring.exporter.http; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.message.ParameterizedMessage; -import org.apache.logging.log4j.util.Supplier; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.collect.Tuple; +import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentHelper; @@ -82,49 +81,33 @@ public class WatcherExistsHttpResource extends PublishableHttpResource { * Watcher. We do the same thing if the current node is not the elected master node. */ @Override - protected CheckResponse doCheck(final RestClient client) { + protected void doCheck(final RestClient client, final ActionListener listener) { // only the master manages watches if (clusterService.state().nodes().isLocalNodeElectedMaster()) { - return checkXPackForWatcher(client); + checkXPackForWatcher(client, listener); + } else { + // not the elected master + listener.onResponse(true); } - - // not the elected master - return CheckResponse.EXISTS; } /** * Reach out to the remote cluster to determine the usability of Watcher. * * @param client The REST client to make the request(s). - * @return Never {@code null}. + * @param listener Returns {@code true} to skip cluster alert creation. {@code false} to check/create them. */ - private CheckResponse checkXPackForWatcher(final RestClient client) { - final Tuple response = - checkForResource(client, logger, - "", "_xpack", "watcher check", - resourceOwnerName, "monitoring cluster", - GET_EXISTS, - Sets.newHashSet(RestStatus.NOT_FOUND.getStatus(), RestStatus.BAD_REQUEST.getStatus())); + private void checkXPackForWatcher(final RestClient client, final ActionListener listener) { + final CheckedFunction responseChecker = + (response) -> canUseWatcher(response, XContentType.JSON.xContent()); + // use DNE to pretend that we're all set; it means that Watcher is unusable + final CheckedFunction doesNotExistChecker = (response) -> false; - final CheckResponse checkResponse = response.v1(); - - // if the response succeeds overall, then we have X-Pack, but we need to inspect to verify Watcher existence - if (checkResponse == CheckResponse.EXISTS) { - try { - if (canUseWatcher(response.v2(), XContentType.JSON.xContent())) { - return CheckResponse.DOES_NOT_EXIST; - } - } catch (final IOException | RuntimeException e) { - logger.error((Supplier) () -> new ParameterizedMessage("failed to parse [_xpack] on the [{}]", resourceOwnerName), e); - - return CheckResponse.ERROR; - } - } else if (checkResponse == CheckResponse.ERROR) { - return CheckResponse.ERROR; - } - - // we return _exists_ to SKIP the work of putting Watches because WATCHER does not exist, so follow-on work cannot succeed - return CheckResponse.EXISTS; + checkForResource(client, listener, logger, + "", "_xpack", "watcher check", + resourceOwnerName, "monitoring cluster", + GET_EXISTS, Sets.newHashSet(RestStatus.NOT_FOUND.getStatus(), RestStatus.BAD_REQUEST.getStatus()), + responseChecker, doesNotExistChecker); } /** @@ -148,9 +131,7 @@ public class WatcherExistsHttpResource extends PublishableHttpResource { final Map watcher = (Map) features.get("watcher"); // if Watcher is both available _and_ enabled, then we can use it; either being true is not sufficient - if (Boolean.TRUE == watcher.get("available") && Boolean.TRUE == watcher.get("enabled")) { - return true; - } + return Boolean.TRUE == watcher.get("available") && Boolean.TRUE == watcher.get("enabled"); } return false; @@ -160,7 +141,7 @@ public class WatcherExistsHttpResource extends PublishableHttpResource { * Add Watches to the remote cluster. */ @Override - protected boolean doPublish(final RestClient client) { - return watches.checkAndPublish(client); + protected void doPublish(final RestClient client, final ActionListener listener) { + watches.checkAndPublish(client, listener); } } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporter.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporter.java index e1475669f2d..908919ed2ce 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporter.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporter.java @@ -31,7 +31,6 @@ import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.gateway.GatewayService; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.ingest.IngestMetadata; import org.elasticsearch.ingest.PipelineConfiguration; @@ -49,6 +48,7 @@ import org.elasticsearch.xpack.core.watcher.transport.actions.get.GetWatchRespon import org.elasticsearch.xpack.core.watcher.watch.Watch; import org.elasticsearch.xpack.monitoring.cleaner.CleanerService; import org.elasticsearch.xpack.monitoring.exporter.ClusterAlertsUtil; +import org.elasticsearch.xpack.monitoring.exporter.ExportBulk; import org.elasticsearch.xpack.monitoring.exporter.Exporter; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; @@ -138,11 +138,12 @@ public class LocalExporter extends Exporter implements ClusterStateListener, Cle } @Override - public LocalBulk openBulk() { + public void openBulk(final ActionListener listener) { if (state.get() != State.RUNNING) { - return null; + listener.onResponse(null); + } else { + listener.onResponse(resolveBulk(clusterService.state(), false)); } - return resolveBulk(clusterService.state(), false); } @Override @@ -161,13 +162,6 @@ public class LocalExporter extends Exporter implements ClusterStateListener, Cle return null; } - if (clusterState.blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK)) { - // wait until the gateway has recovered from disk, otherwise we think may not have .monitoring-es- - // indices but they may not have been restored from the cluster state on disk - logger.debug("waiting until gateway has recovered from disk"); - return null; - } - // List of templates final Map templates = Arrays.stream(MonitoringTemplateUtils.TEMPLATE_IDS) .collect(Collectors.toMap(MonitoringTemplateUtils::templateName, MonitoringTemplateUtils::loadTemplate)); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSetTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSetTests.java index c5866217240..1a06a9a4037 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSetTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSetTests.java @@ -94,7 +94,7 @@ public class MonitoringFeatureSetTests extends ESTestCase { exporterList.add(exporter); } } - when(exporters.iterator()).thenReturn(exporterList.iterator()); + when(exporters.getEnabledExporters()).thenReturn(exporterList); when(monitoring.isMonitoringActive()).thenReturn(collectionEnabled); MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, monitoring, licenseState, exporters); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/cleaner/AbstractIndicesCleanerTestCase.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/cleaner/AbstractIndicesCleanerTestCase.java index c7a25772955..23bb21a55ed 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/cleaner/AbstractIndicesCleanerTestCase.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/cleaner/AbstractIndicesCleanerTestCase.java @@ -162,7 +162,7 @@ public abstract class AbstractIndicesCleanerTestCase extends MonitoringIntegTest protected CleanerService.Listener getListener() { Exporters exporters = internalCluster().getInstance(Exporters.class, internalCluster().getMasterName()); - for (Exporter exporter : exporters) { + for (Exporter exporter : exporters.getEnabledExporters()) { if (exporter instanceof CleanerService.Listener) { return (CleanerService.Listener) exporter; } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/ExportersTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/ExportersTests.java index 5df8df74f7b..e349b19b8d2 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/ExportersTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/ExportersTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.monitoring.exporter; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.block.ClusterBlocks; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.ClusterSettings; @@ -17,6 +18,7 @@ import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.gateway.GatewayService; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; @@ -50,6 +52,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class ExportersTests extends ESTestCase { @@ -57,6 +60,7 @@ public class ExportersTests extends ESTestCase { private Map factories; private ClusterService clusterService; private ClusterState state; + private final ClusterBlocks blocks = mock(ClusterBlocks.class); private final MetaData metadata = mock(MetaData.class); private final XPackLicenseState licenseState = mock(XPackLicenseState.class); private ClusterSettings clusterSettings; @@ -79,6 +83,7 @@ public class ExportersTests extends ESTestCase { clusterSettings = new ClusterSettings(Settings.EMPTY, settingsSet); when(clusterService.getClusterSettings()).thenReturn(clusterSettings); when(clusterService.state()).thenReturn(state); + when(state.blocks()).thenReturn(blocks); when(state.metaData()).thenReturn(metadata); // we always need to have the local exporter as it serves as the default one @@ -210,6 +215,8 @@ public class ExportersTests extends ESTestCase { public void testExporterBlocksOnClusterState() { if (rarely()) { when(metadata.clusterUUID()).thenReturn(ClusterState.UNKNOWN_UUID); + } else if (rarely()) { + when(blocks.hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK)).thenReturn(true); } else { when(state.version()).thenReturn(ClusterState.UNKNOWN_VERSION); } @@ -223,7 +230,13 @@ public class ExportersTests extends ESTestCase { final Exporters exporters = new Exporters(settings.build(), factories, clusterService, licenseState, threadContext); - assertThat(exporters.openBulk(), nullValue()); + // synchronously checks the cluster state + exporters.wrapExportBulk(ActionListener.wrap( + bulk -> assertThat(bulk, is(nullValue())), + e -> fail(e.getMessage()) + )); + + verify(state).blocks(); } /** @@ -284,7 +297,7 @@ public class ExportersTests extends ESTestCase { } assertThat(exceptions, empty()); - for (Exporter exporter : exporters) { + for (Exporter exporter : exporters.getEnabledExporters()) { assertThat(exporter, instanceOf(CountingExporter.class)); assertThat(((CountingExporter) exporter).getExportedCount(), equalTo(total)); } @@ -298,8 +311,8 @@ public class ExportersTests extends ESTestCase { } @Override - public ExportBulk openBulk() { - return mock(ExportBulk.class); + public void openBulk(final ActionListener listener) { + listener.onResponse(mock(ExportBulk.class)); } @Override @@ -318,19 +331,6 @@ public class ExportersTests extends ESTestCase { } } - - static class MockFactory implements Exporter.Factory { - - @Override - public Exporter create(Exporter.Config config) { - Exporter exporter = mock(Exporter.class); - when(exporter.name()).thenReturn(config.name()); - when(exporter.openBulk()).thenReturn(mock(ExportBulk.class)); - return exporter; - } - - } - static class CountingExporter extends Exporter { private static final AtomicInteger count = new AtomicInteger(0); @@ -343,10 +343,11 @@ public class ExportersTests extends ESTestCase { } @Override - public ExportBulk openBulk() { + public void openBulk(final ActionListener listener) { CountingBulk bulk = new CountingBulk(config.type() + "#" + count.getAndIncrement(), threadContext); bulks.add(bulk); - return bulk; + + listener.onResponse(bulk); } @Override 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 ef365042bd5..39f6c573fc4 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,18 +11,18 @@ 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.action.ActionListener; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; +import org.elasticsearch.client.ResponseListener; import org.elasticsearch.client.RestClient; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.unit.TimeValue; 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,11 +30,17 @@ import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; +import static org.elasticsearch.xpack.monitoring.exporter.http.AsyncHttpResourceHelper.mockBooleanActionListener; +import static org.elasticsearch.xpack.monitoring.exporter.http.AsyncHttpResourceHelper.whenPerformRequestAsyncWith; 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.hamcrest.Matchers.instanceOf; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.when; import static org.mockito.Mockito.verify; @@ -48,96 +54,102 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase protected final TimeValue masterTimeout = randomFrom(TimeValue.timeValueMinutes(5), TimeValue.MINUS_ONE, null); protected final RestClient client = mock(RestClient.class); + protected final ActionListener listener = mockBooleanActionListener(); /** - * Perform {@link PublishableHttpResource#doCheck(RestClient) doCheck} against the {@code resource} and assert that it returns - * {@code EXISTS} given a {@link RestStatus} that is {@link RestStatus#OK}. + * Perform {@link PublishableHttpResource#doCheck(RestClient, ActionListener) doCheck} against the {@code resource} and assert that it + * returns {@code onResponse(false)} given a {@link RestStatus} that is not {@link RestStatus#OK}. * * @param resource The resource to execute. * @param resourceBasePath The base endpoint (e.g., "/_template") * @param resourceName The resource name (e.g., the template or pipeline name). */ - protected void assertCheckExists(final PublishableHttpResource resource, final String resourceBasePath, final String resourceName) - throws IOException { - doCheckWithStatusCode(resource, resourceBasePath, resourceName, successfulCheckStatus(), CheckResponse.EXISTS); + protected void assertCheckDoesNotExist(final PublishableHttpResource resource, + final String resourceBasePath, + final String resourceName) { + doCheckWithStatusCode(resource, resourceBasePath, resourceName, notFoundCheckStatus(), false); } /** - * Perform {@link PublishableHttpResource#doCheck(RestClient) doCheck} against the {@code resource} and assert that it returns - * {@code DOES_NOT_EXIST} given a {@link RestStatus} that is not {@link RestStatus#OK}. - * - * @param resource The resource to execute. - * @param resourceBasePath The base endpoint (e.g., "/_template") - * @param resourceName The resource name (e.g., the template or pipeline name). - */ - protected void assertCheckDoesNotExist(final PublishableHttpResource resource, final String resourceBasePath, final String resourceName) - throws IOException { - doCheckWithStatusCode(resource, resourceBasePath, resourceName, notFoundCheckStatus(), CheckResponse.DOES_NOT_EXIST); - } - - /** - * Perform {@link PublishableHttpResource#doCheck(RestClient) doCheck} against the {@code resource} that throws an exception and assert - * that it returns {@code ERROR}. + * Perform {@link PublishableHttpResource#doCheck(RestClient, ActionListener) doCheck} against the {@code resource} that throws an + * exception and assert that it returns {@code onFailure}. * * @param resource The resource to execute. * @param resourceBasePath The base endpoint (e.g., "/_template") * @param resourceName The resource name (e.g., the template or pipeline name). */ protected void assertCheckWithException(final PublishableHttpResource resource, - final String resourceBasePath, final String resourceName) - throws IOException { + final String resourceBasePath, final String resourceName) { + assertCheckWithException(resource, getParameters(resource.getParameters()), resourceBasePath, resourceName); + } + + /** + * Perform {@link PublishableHttpResource#doCheck(RestClient, ActionListener) doCheck} against the {@code resource} that throws an + * exception and assert that it returns {@code onFailure}. + * + * @param resource The resource to execute. + * @param expectedParameters The test-supplied parameters for the {@code Request}. + * @param resourceBasePath The base endpoint (e.g., "/_template") + * @param resourceName The resource name (e.g., the template or pipeline name). + */ + protected void assertCheckWithException(final PublishableHttpResource resource, + final Map expectedParameters, + final String resourceBasePath, final String resourceName) { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final ResponseException responseException = responseException("GET", endpoint, failedCheckStatus()); final Exception e = randomFrom(new IOException("expected"), new RuntimeException("expected"), responseException); - Request request = new Request("GET", endpoint); - addParameters(request, getParameters(resource.getParameters())); - when(client.performRequest(request)).thenThrow(e); + final Request request = new Request("GET", endpoint); + addParameters(request, expectedParameters); + whenPerformRequestAsyncWith(client, request, e); - assertThat(resource.doCheck(client), is(CheckResponse.ERROR)); + resource.doCheck(client, listener); + + verifyListener(null); } /** - * Perform {@link PublishableHttpResource#doCheck(RestClient) doCheck} against the {@code resource}, expecting a {@code DELETE}, and - * assert that it returns {@code EXISTS} given a {@link RestStatus} that is {@link RestStatus#OK} or {@link RestStatus#NOT_FOUND}. + * Perform {@link PublishableHttpResource#doCheck(RestClient, ActionListener) doCheck} against the {@code resource}, expecting a + * {@code DELETE}, and assert that it returns {@code onResponse(true)} given a {@link RestStatus} that is {@link RestStatus#OK} or + * {@link RestStatus#NOT_FOUND}. * * @param resource The resource to execute. * @param resourceBasePath The base endpoint (e.g., "/_template") * @param resourceName The resource name (e.g., the template or pipeline name). */ protected void assertCheckAsDeleteExists(final PublishableHttpResource resource, - final String resourceBasePath, final String resourceName) - throws IOException { + final String resourceBasePath, final String resourceName) { final RestStatus status = randomFrom(successfulCheckStatus(), notFoundCheckStatus()); - doCheckAsDeleteWithStatusCode(resource, resourceBasePath, resourceName, status, CheckResponse.EXISTS); + doCheckAsDeleteWithStatusCode(resource, resourceBasePath, resourceName, status, true); } /** - * Perform {@link PublishableHttpResource#doCheck(RestClient) doCheck} against the {@code resource} that throws an exception and assert - * that it returns {@code ERRPR} when performing a {@code DELETE} rather than the more common {@code GET}. + * Perform {@link PublishableHttpResource#doCheck(RestClient, ActionListener) doCheck} against the {@code resource} that throws an + * exception and assert that it returns {@code onFailure()} when performing a {@code DELETE} rather than the more common {@code GET}. * * @param resource The resource to execute. * @param resourceBasePath The base endpoint (e.g., "/_template") * @param resourceName The resource name (e.g., the template or pipeline name). */ protected void assertCheckAsDeleteWithException(final PublishableHttpResource resource, - final String resourceBasePath, final String resourceName) - throws IOException { + final String resourceBasePath, final String resourceName) { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final ResponseException responseException = responseException("DELETE", endpoint, failedCheckStatus()); final Exception e = randomFrom(new IOException("expected"), new RuntimeException("expected"), responseException); - Request request = new Request("DELETE", endpoint); + final Request request = new Request("DELETE", endpoint); addParameters(request, deleteParameters(resource.getParameters())); - when(client.performRequest(request)).thenThrow(e); + whenPerformRequestAsyncWith(client, request, e); - assertThat(resource.doCheck(client), is(CheckResponse.ERROR)); + resource.doCheck(client, listener); + + verifyListener(null); } /** - * Perform {@link PublishableHttpResource#doPublish(RestClient) doPublish} against the {@code resource} and assert that it returns - * {@code true} given a {@link RestStatus} that is {@link RestStatus#OK} or {@link RestStatus#CREATED}. + * Perform {@link PublishableHttpResource#doPublish(RestClient, ActionListener) doPublish} against the {@code resource} and assert that + * it returns {@code onResponse(true)} given a {@link RestStatus} that is {@link RestStatus#OK} or {@link RestStatus#CREATED}. * * @param resource The resource to execute. * @param resourceBasePath The base endpoint (e.g., "/_template") @@ -145,29 +157,13 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase * @param bodyType The request body provider's type. */ protected void assertPublishSucceeds(final PublishableHttpResource resource, final String resourceBasePath, final String resourceName, - final Class bodyType) - throws IOException { + final Class bodyType) { doPublishWithStatusCode(resource, resourceBasePath, resourceName, bodyType, successfulPublishStatus(), true); } /** - * Perform {@link PublishableHttpResource#doPublish(RestClient) doPublish} against the {@code resource} and assert that it returns - * {@code false} given a {@link RestStatus} that is neither {@link RestStatus#OK} or {@link RestStatus#CREATED}. - * - * @param resource The resource to execute. - * @param resourceBasePath The base endpoint (e.g., "/_template") - * @param resourceName The resource name (e.g., the template or pipeline name). - * @param bodyType The request body provider's type. - */ - protected void assertPublishFails(final PublishableHttpResource resource, final String resourceBasePath, final String resourceName, - final Class bodyType) - throws IOException { - doPublishWithStatusCode(resource, resourceBasePath, resourceName, bodyType, failedPublishStatus(), false); - } - - /** - * Perform {@link PublishableHttpResource#doPublish(RestClient) doPublish} against the {@code resource} that throws an exception and - * assert that it returns {@code false}. + * Perform {@link PublishableHttpResource#doPublish(RestClient, ActionListener) doPublish} against the {@code resource} that throws an + * exception and assert that it returns {@code onResponse(false)}. * * @param resource The resource to execute. * @param resourceBasePath The base endpoint (e.g., "/_template") @@ -175,16 +171,18 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase */ protected void assertPublishWithException(final PublishableHttpResource resource, final String resourceBasePath, final String resourceName, - final Class bodyType) - throws IOException { + final Class bodyType) { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final Exception e = randomFrom(new IOException("expected"), new RuntimeException("expected")); - when(client.performRequest(Mockito.any(Request.class))).thenThrow(e); + whenPerformRequestAsyncWith(client, e); - assertThat(resource.doPublish(client), is(false)); - ArgumentCaptor request = ArgumentCaptor.forClass(Request.class); - verify(client).performRequest(request.capture()); + resource.doPublish(client, listener); + + verifyListener(null); + + final ArgumentCaptor request = ArgumentCaptor.forClass(Request.class); + verify(client).performRequestAsync(request.capture(), any(ResponseListener.class)); assertThat(request.getValue().getMethod(), is("PUT")); assertThat(request.getValue().getEndpoint(), is(endpoint)); assertThat(request.getValue().getParameters(), is(resource.getParameters())); @@ -219,63 +217,60 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase protected void doCheckWithStatusCode(final PublishableHttpResource resource, final String resourceBasePath, final String resourceName, final RestStatus status, - final CheckResponse expected) - throws IOException { + final Boolean expected) { doCheckWithStatusCode(resource, resourceBasePath, resourceName, status, expected, null); } protected void doCheckWithStatusCode(final PublishableHttpResource resource, final String resourceBasePath, final String resourceName, - final RestStatus status, final CheckResponse expected, final HttpEntity entity) - throws IOException { + final RestStatus status, final Boolean expected, final HttpEntity entity) { doCheckWithStatusCode(resource, resourceBasePath, resourceName, status, GET_EXISTS, GET_DOES_NOT_EXIST, expected, entity); } protected void doCheckWithStatusCode(final PublishableHttpResource resource, final String resourceBasePath, final String resourceName, final RestStatus status, final Set exists, final Set doesNotExist, - final CheckResponse expected) - throws IOException { + final Boolean expected) { doCheckWithStatusCode(resource, resourceBasePath, resourceName, status, exists, doesNotExist, expected, null); } protected void doCheckWithStatusCode(final PublishableHttpResource resource, final String resourceBasePath, final String resourceName, final RestStatus status, final Set exists, final Set doesNotExist, - final CheckResponse expected, final HttpEntity entity) - throws IOException { + final Boolean expected, final HttpEntity entity) { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final Response response = response("GET", endpoint, status, entity); doCheckWithStatusCode(resource, getParameters(resource.getParameters(), exists, doesNotExist), endpoint, expected, response); } - protected void doCheckWithStatusCode(final PublishableHttpResource resource, final String endpoint, final CheckResponse expected, - final Response response) - throws IOException { - doCheckWithStatusCode(resource, getParameters(resource.getParameters()), endpoint, expected, response); - } - protected void doCheckWithStatusCode(final PublishableHttpResource resource, final Map expectedParameters, - final String endpoint, final CheckResponse expected, - final Response response) - throws IOException { - Request request = new Request("GET", endpoint); + final String endpoint, final Boolean expected, + final Response response) { + final Request request = new Request("GET", endpoint); addParameters(request, expectedParameters); - when(client.performRequest(request)).thenReturn(response); - assertThat(resource.doCheck(client), is(expected)); + whenPerformRequestAsyncWith(client, request, response); + + resource.doCheck(client, listener); + + verify(client).performRequestAsync(eq(request), any(ResponseListener.class)); + verifyListener(expected); } private void doPublishWithStatusCode(final PublishableHttpResource resource, final String resourceBasePath, final String resourceName, final Class bodyType, final RestStatus status, - final boolean expected) - throws IOException { + final boolean errorFree) { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final Response response = response("GET", endpoint, status); - ArgumentCaptor request = ArgumentCaptor.forClass(Request.class); - when(client.performRequest(request.capture())).thenReturn(response); + whenPerformRequestAsyncWith(client, response); + + resource.doPublish(client, listener); + + verifyListener(errorFree ? true : null); + + final ArgumentCaptor request = ArgumentCaptor.forClass(Request.class); + verify(client).performRequestAsync(request.capture(), any(ResponseListener.class)); - 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())); @@ -285,8 +280,7 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase protected void doCheckAsDeleteWithStatusCode(final PublishableHttpResource resource, final String resourceBasePath, final String resourceName, final RestStatus status, - final CheckResponse expected) - throws IOException { + final Boolean expected) { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final Response response = response("DELETE", endpoint, status); @@ -294,14 +288,15 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase } protected void doCheckAsDeleteWithStatusCode(final PublishableHttpResource resource, - final String endpoint, final CheckResponse expected, - final Response response) - throws IOException { - Request request = new Request("DELETE", endpoint); + final String endpoint, final Boolean expected, + final Response response) { + final Request request = new Request("DELETE", endpoint); addParameters(request, deleteParameters(resource.getParameters())); - when(client.performRequest(request)).thenReturn(response); + whenPerformRequestAsyncWith(client, request, response); - assertThat(resource.doCheck(client), is(expected)); + resource.doCheck(client, listener); + + verifyListener(expected); } protected RestStatus successfulCheckStatus() { @@ -381,77 +376,68 @@ public abstract class AbstractPublishableHttpResourceTestCase extends ESTestCase return parametersWithIgnore; } - protected HttpEntity entityForResource(final CheckResponse expected, final String resourceName, final int minimumVersion) { - HttpEntity entity = null; - - switch (expected) { + protected HttpEntity entityForResource(final Boolean expected, final String resourceName, final int minimumVersion) { + if (expected == Boolean.FALSE) { // the version check is what is expected to cause it to be replaced - case DOES_NOT_EXIST: - final int olderVersion = minimumVersion - randomIntBetween(1, 10000); + final int olderVersion = minimumVersion - randomIntBetween(1, 10000); - entity = randomFrom( - new StringEntity("{}", ContentType.APPLICATION_JSON), - new StringEntity("{\"" + resourceName + "\":{}}", ContentType.APPLICATION_JSON), - new StringEntity("{\"" + resourceName + "\":{\"version\":\"123\"}}", ContentType.APPLICATION_JSON), - new StringEntity("{\"" + resourceName + "\":{\"version\":" + olderVersion + "}}", ContentType.APPLICATION_JSON) - ); - break; + return randomFrom( + new StringEntity("{}", ContentType.APPLICATION_JSON), + new StringEntity("{\"" + resourceName + "\":{}}", ContentType.APPLICATION_JSON), + new StringEntity("{\"" + resourceName + "\":{\"version\":\"123\"}}", ContentType.APPLICATION_JSON), + new StringEntity("{\"" + resourceName + "\":{\"version\":" + olderVersion + "}}", ContentType.APPLICATION_JSON) + ); + } else if (expected == Boolean.TRUE) { // the version is there and it's exactly what we specify - case EXISTS: - entity = new StringEntity("{\"" + resourceName + "\":{\"version\":" + minimumVersion + "}}", ContentType.APPLICATION_JSON); - break; + return new StringEntity("{\"" + resourceName + "\":{\"version\":" + minimumVersion + "}}", ContentType.APPLICATION_JSON); + } else { // expected = null, which is for malformed/failure // malformed - case ERROR: - entity = randomFrom( - new StringEntity("{", ContentType.APPLICATION_JSON), - new StringEntity("{\"" + resourceName + "\":\"not an object\"}", ContentType.APPLICATION_JSON) - ); - break; - default: - fail("Unhandled/unknown CheckResponse"); + return randomFrom( + new StringEntity("{", ContentType.APPLICATION_JSON), + new StringEntity("{\"" + resourceName + "\":\"not an object\"}", ContentType.APPLICATION_JSON) + ); } - - return entity; } - protected HttpEntity entityForClusterAlert(final CheckResponse expected, final int minimumVersion) { - HttpEntity entity = null; - - switch (expected) { + protected HttpEntity entityForClusterAlert(final Boolean expected, final int minimumVersion) { + if (expected == Boolean.FALSE) { // the version check is what is expected to cause it to be replaced - case DOES_NOT_EXIST: - final int olderVersion = minimumVersion - randomIntBetween(1, 10000); + final int olderVersion = minimumVersion - randomIntBetween(1, 10000); - entity = randomFrom( - new StringEntity("{}", ContentType.APPLICATION_JSON), - new StringEntity("{\"metadata\":{}}", ContentType.APPLICATION_JSON), - new StringEntity("{\"metadata\":{\"xpack\":{\"version_created\":\"123\"}}}", ContentType.APPLICATION_JSON), - new StringEntity("{\"metadata\":{\"xpack\":{\"version_created\":" + olderVersion + "}}}}", ContentType.APPLICATION_JSON) - ); - break; + return randomFrom( + new StringEntity("{}", ContentType.APPLICATION_JSON), + new StringEntity("{\"metadata\":{}}", ContentType.APPLICATION_JSON), + new StringEntity("{\"metadata\":{\"xpack\":{\"version_created\":\"123\"}}}", ContentType.APPLICATION_JSON), + new StringEntity("{\"metadata\":{\"xpack\":{\"version_created\":" + olderVersion + "}}}}", ContentType.APPLICATION_JSON) + ); + } else if (expected == Boolean.TRUE) { // the version is there and it's exactly what we specify - case EXISTS: - entity = new StringEntity("{\"metadata\":{\"xpack\":{\"version_created\":" + - minimumVersion + "}}}", ContentType.APPLICATION_JSON); - break; + return new StringEntity("{\"metadata\":{\"xpack\":{\"version_created\":" + + minimumVersion + "}}}", ContentType.APPLICATION_JSON); + } else { // expected == null, which is for malformed/failure // malformed - case ERROR: - entity = randomFrom( - new StringEntity("{", ContentType.APPLICATION_JSON), - new StringEntity("{\"\"metadata\":\"not an object\"}", ContentType.APPLICATION_JSON), - new StringEntity("{\"\"metadata\":{\"xpack\":\"not an object\"}}", ContentType.APPLICATION_JSON) - ); - break; - default: - fail("Unhandled/unknown CheckResponse"); + return randomFrom( + new StringEntity("{", ContentType.APPLICATION_JSON), + new StringEntity("{\"\"metadata\":\"not an object\"}", ContentType.APPLICATION_JSON), + new StringEntity("{\"\"metadata\":{\"xpack\":\"not an object\"}}", ContentType.APPLICATION_JSON) + ); } - - return entity; } - protected void addParameters(Request request, Map parameters) { + protected void addParameters(final Request request, final Map parameters) { for (Map.Entry param : parameters.entrySet()) { request.addParameter(param.getKey(), param.getValue()); } } + + protected void verifyListener(final Boolean expected) { + if (expected == null) { + verify(listener, never()).onResponse(anyBoolean()); + verify(listener).onFailure(any(Exception.class)); + } else { + verify(listener).onResponse(expected); + verify(listener, never()).onFailure(any(Exception.class)); + } + } + } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/AsyncHttpResourceHelper.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/AsyncHttpResourceHelper.java new file mode 100644 index 00000000000..d973bc19686 --- /dev/null +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/AsyncHttpResourceHelper.java @@ -0,0 +1,117 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.monitoring.exporter.http; + +import java.util.List; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseListener; +import org.elasticsearch.client.RestClient; +import org.hamcrest.Matcher; +import org.mockito.stubbing.Stubber; + +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.argThat; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; + +class AsyncHttpResourceHelper { + + @SuppressWarnings("unchecked") + static ActionListener mockBooleanActionListener() { + return mock(ActionListener.class); + } + + static void whenPerformRequestAsyncWith(final RestClient client, final Response response) { + doAnswer(invocation -> { + ((ResponseListener)invocation.getArguments()[1]).onSuccess(response); + return null; + }).when(client).performRequestAsync(any(Request.class), any(ResponseListener.class)); + } + + static void whenPerformRequestAsyncWith(final RestClient client, final Matcher request, final Response response) { + doAnswer(invocation -> { + ((ResponseListener)invocation.getArguments()[1]).onSuccess(response); + return null; + }).when(client).performRequestAsync(argThat(request), any(ResponseListener.class)); + } + + static void whenPerformRequestAsyncWith(final RestClient client, final Matcher request, final List responses) { + if (responses.size() == 1) { + whenPerformRequestAsyncWith(client, request, responses.get(0)); + } else if (responses.size() > 1) { + whenPerformRequestAsyncWith(client, request, responses.get(0), responses.subList(1, responses.size()), null); + } + } + + static void whenPerformRequestAsyncWith(final RestClient client, + final Matcher request, + final Response response, + final Exception exception) { + whenPerformRequestAsyncWith(client, request, response, null, exception); + } + + static void whenPerformRequestAsyncWith(final RestClient client, + final Matcher request, + final Response first, + final List responses, + final Exception exception) { + Stubber stub = doAnswer(invocation -> { + ((ResponseListener)invocation.getArguments()[1]).onSuccess(first); + return null; + }); + + if (responses != null) { + for (final Response response : responses) { + stub.doAnswer(invocation -> { + ((ResponseListener) invocation.getArguments()[1]).onSuccess(response); + return null; + }); + } + } + + if (exception != null) { + stub.doAnswer(invocation -> { + ((ResponseListener) invocation.getArguments()[1]).onFailure(exception); + return null; + }); + } + + stub.when(client).performRequestAsync(argThat(request), any(ResponseListener.class)); + } + + static void whenPerformRequestAsyncWith(final RestClient client, final Request request, final Response response) { + doAnswer(invocation -> { + ((ResponseListener)invocation.getArguments()[1]).onSuccess(response); + return null; + }).when(client).performRequestAsync(eq(request), any(ResponseListener.class)); + } + + static void whenPerformRequestAsyncWith(final RestClient client, final Exception exception) { + doAnswer(invocation -> { + ((ResponseListener)invocation.getArguments()[1]).onFailure(exception); + return null; + }).when(client).performRequestAsync(any(Request.class), any(ResponseListener.class)); + } + + static void whenPerformRequestAsyncWith(final RestClient client, final Matcher request, final Exception exception) { + doAnswer(invocation -> { + ((ResponseListener)invocation.getArguments()[1]).onFailure(exception); + return null; + }).when(client).performRequestAsync(argThat(request), any(ResponseListener.class)); + } + + static void whenPerformRequestAsyncWith(final RestClient client, final Request request, final Exception exception) { + doAnswer(invocation -> { + ((ResponseListener)invocation.getArguments()[1]).onFailure(exception); + return null; + }).when(client).performRequestAsync(eq(request), any(ResponseListener.class)); + } + +} diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/ClusterAlertHttpResourceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/ClusterAlertHttpResourceTests.java index 72d749780af..d86e901e38a 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/ClusterAlertHttpResourceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/ClusterAlertHttpResourceTests.java @@ -18,7 +18,6 @@ import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.monitoring.exporter.ClusterAlertsUtil; -import org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse; import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.mock; @@ -62,10 +61,9 @@ public class ClusterAlertHttpResourceTests extends AbstractPublishableHttpResour public void testDoCheckGetWatchExists() throws IOException { when(licenseState.isMonitoringClusterAlertsAllowed()).thenReturn(true); - final HttpEntity entity = entityForClusterAlert(CheckResponse.EXISTS, minimumVersion); + final HttpEntity entity = entityForClusterAlert(true, minimumVersion); - doCheckWithStatusCode(resource, "/_xpack/watcher/watch", watchId, successfulCheckStatus(), - CheckResponse.EXISTS, entity); + doCheckWithStatusCode(resource, "/_xpack/watcher/watch", watchId, successfulCheckStatus(), true, entity); } public void testDoCheckGetWatchDoesNotExist() throws IOException { @@ -76,10 +74,9 @@ public class ClusterAlertHttpResourceTests extends AbstractPublishableHttpResour assertCheckDoesNotExist(resource, "/_xpack/watcher/watch", watchId); } else { // it does not exist because we need to replace it - final HttpEntity entity = entityForClusterAlert(CheckResponse.DOES_NOT_EXIST, minimumVersion); + final HttpEntity entity = entityForClusterAlert(false, minimumVersion); - doCheckWithStatusCode(resource, "/_xpack/watcher/watch", watchId, successfulCheckStatus(), - CheckResponse.DOES_NOT_EXIST, entity); + doCheckWithStatusCode(resource, "/_xpack/watcher/watch", watchId, successfulCheckStatus(), false, entity); } } @@ -91,10 +88,9 @@ public class ClusterAlertHttpResourceTests extends AbstractPublishableHttpResour assertCheckWithException(resource, "/_xpack/watcher/watch", watchId); } else { // error because of a malformed response - final HttpEntity entity = entityForClusterAlert(CheckResponse.ERROR, minimumVersion); + final HttpEntity entity = entityForClusterAlert(null, minimumVersion); - doCheckWithStatusCode(resource, "/_xpack/watcher/watch", watchId, successfulCheckStatus(), - CheckResponse.ERROR, entity); + doCheckWithStatusCode(resource, "/_xpack/watcher/watch", watchId, successfulCheckStatus(), null, entity); } } @@ -134,10 +130,6 @@ public class ClusterAlertHttpResourceTests extends AbstractPublishableHttpResour assertPublishSucceeds(resource, "/_xpack/watcher/watch", watchId, StringEntity.class); } - public void testDoPublishFalse() throws IOException { - assertPublishFails(resource, "/_xpack/watcher/watch", watchId, StringEntity.class); - } - public void testDoPublishFalseWithException() throws IOException { assertPublishWithException(resource, "/_xpack/watcher/watch", watchId, StringEntity.class); } @@ -153,9 +145,9 @@ public class ClusterAlertHttpResourceTests extends AbstractPublishableHttpResour expectThrows(IOException.class, () -> resource.shouldReplaceClusterAlert(response, xContent, randomInt())); } - public void testShouldReplaceClusterAlertThrowsExceptionForMalformedResponse() throws IOException { + public void testShouldReplaceClusterAlertThrowsExceptionForMalformedResponse() { final Response response = mock(Response.class); - final HttpEntity entity = entityForClusterAlert(CheckResponse.ERROR, randomInt()); + final HttpEntity entity = entityForClusterAlert(null, randomInt()); final XContent xContent = XContentType.JSON.xContent(); when(response.getEntity()).thenReturn(entity); @@ -166,7 +158,7 @@ public class ClusterAlertHttpResourceTests extends AbstractPublishableHttpResour public void testShouldReplaceClusterAlertReturnsTrueVersionIsNotExpected() throws IOException { final int minimumVersion = randomInt(); final Response response = mock(Response.class); - final HttpEntity entity = entityForClusterAlert(CheckResponse.DOES_NOT_EXIST, minimumVersion); + final HttpEntity entity = entityForClusterAlert(false, minimumVersion); final XContent xContent = XContentType.JSON.xContent(); when(response.getEntity()).thenReturn(entity); @@ -180,7 +172,7 @@ public class ClusterAlertHttpResourceTests extends AbstractPublishableHttpResour final boolean shouldReplace = version < minimumVersion; final Response response = mock(Response.class); - final HttpEntity entity = entityForClusterAlert(CheckResponse.EXISTS, version); + final HttpEntity entity = entityForClusterAlert(true, version); final XContent xContent = XContentType.JSON.xContent(); when(response.getEntity()).thenReturn(entity); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java index ff90e399e9d..46a0a353d09 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java @@ -36,6 +36,7 @@ import org.elasticsearch.xpack.core.ssl.SSLService; import org.elasticsearch.xpack.monitoring.MonitoringTestUtils; import org.elasticsearch.xpack.monitoring.collector.indices.IndexRecoveryMonitoringDoc; import org.elasticsearch.xpack.monitoring.exporter.ClusterAlertsUtil; +import org.elasticsearch.xpack.monitoring.exporter.ExportBulk; import org.elasticsearch.xpack.monitoring.exporter.Exporter; import org.elasticsearch.xpack.monitoring.test.MonitoringIntegTestCase; import org.joda.time.format.DateTimeFormat; @@ -66,7 +67,6 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; @@ -116,7 +116,7 @@ public class HttpExporterIT extends MonitoringIntegTestCase { .build(); } - protected Settings.Builder baseSettings() { + private Settings.Builder baseSettings() { return Settings.builder() .put("xpack.monitoring.exporters._http.type", "http") .put("xpack.monitoring.exporters._http.host", getFormattedAddress(webServer)) @@ -295,7 +295,7 @@ public class HttpExporterIT extends MonitoringIntegTestCase { } public void testUnsupportedClusterVersion() throws Exception { - Settings settings = Settings.builder() + final Settings settings = Settings.builder() .put("xpack.monitoring.exporters._http.type", "http") .put("xpack.monitoring.exporters._http.host", getFormattedAddress(webServer)) .build(); @@ -311,7 +311,21 @@ public class HttpExporterIT extends MonitoringIntegTestCase { // ensure that the exporter is not able to be used try (HttpExporter exporter = createHttpExporter(settings)) { - assertThat(exporter.isExporterReady(), is(false)); + final CountDownLatch awaitResponseAndClose = new CountDownLatch(1); + + final ActionListener listener = ActionListener.wrap( + bulk -> { + assertNull(bulk); + + awaitResponseAndClose.countDown(); + }, + e -> fail(e.getMessage()) + ); + + exporter.openBulk(listener); + + // wait for it to actually respond + assertTrue(awaitResponseAndClose.await(15, TimeUnit.SECONDS)); } assertThat(webServer.requests(), hasSize(1)); @@ -549,7 +563,7 @@ public class HttpExporterIT extends MonitoringIntegTestCase { } } - private HttpExporter createHttpExporter(final Settings settings) throws Exception { + private HttpExporter createHttpExporter(final Settings settings) { final Exporter.Config config = new Exporter.Config("_http", "http", settings, clusterService(), new XPackLicenseState(Settings.EMPTY)); @@ -561,34 +575,25 @@ public class HttpExporterIT extends MonitoringIntegTestCase { assertBusy(() -> assertThat(clusterService().state().version(), not(ClusterState.UNKNOWN_VERSION))); try (HttpExporter exporter = createHttpExporter(settings)) { - // the readiness check happens synchronously, so we don't need to busy-wait for it - assertThat("Exporter is not ready", exporter.isExporterReady(), is(true)); - - final HttpExportBulk bulk = exporter.openBulk(); - - assertThat("Bulk should never be null after the exporter is ready", bulk, notNullValue()); - final CountDownLatch awaitResponseAndClose = new CountDownLatch(2); - final ActionListener listener = new ActionListener() { - @Override - public void onResponse(Void response) { - awaitResponseAndClose.countDown(); - } - @Override - public void onFailure(Exception e) { - fail(e.getMessage()); + exporter.openBulk(ActionListener.wrap(exportBulk -> { + final HttpExportBulk bulk = (HttpExportBulk)exportBulk; - awaitResponseAndClose.countDown(); - } - }; + assertThat("Bulk should never be null after the exporter is ready", bulk, notNullValue()); - bulk.doAdd(docs); - bulk.doFlush(listener); - bulk.doClose(listener); + final ActionListener listener = ActionListener.wrap( + ignored -> awaitResponseAndClose.countDown(), + e -> fail(e.getMessage()) + ); + + bulk.doAdd(docs); + bulk.doFlush(listener); + bulk.doClose(listener); // reusing the same listener, which is why we expect countDown x2 + }, e -> fail("Failed to create HttpExportBulk"))); // block until the bulk responds - awaitResponseAndClose.await(15, TimeUnit.SECONDS); + assertTrue(awaitResponseAndClose.await(15, TimeUnit.SECONDS)); } } 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 fdf67602af6..bcb4181f651 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 @@ -13,6 +13,7 @@ import org.elasticsearch.Version; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; +import org.elasticsearch.client.ResponseListener; import org.elasticsearch.client.RestClient; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.MetaData; @@ -38,11 +39,11 @@ import java.util.stream.Collectors; import static org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils.OLD_TEMPLATE_IDS; import static org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils.PIPELINE_IDS; import static org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils.TEMPLATE_IDS; -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.elasticsearch.xpack.monitoring.exporter.http.AsyncHttpResourceHelper.whenPerformRequestAsyncWith; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.startsWith; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -100,15 +101,20 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe assertThat("Not all watches are supplied", watchNames, hasSize(EXPECTED_WATCHES)); } - public void testInvalidVersionBlocks() throws IOException { - final HttpEntity entity = new StringEntity("{\"version\":{\"number\":\"unknown\"}}", ContentType.APPLICATION_JSON); + public void awaitCheckAndPublish(final Boolean expected) { + resources.checkAndPublish(client, listener); + + verifyListener(expected); + } + + public void testInvalidVersionBlocks() { + final HttpEntity entity = new StringEntity("{\"version\":{\"number\":\"3.0.0\"}}", ContentType.APPLICATION_JSON); when(versionResponse.getEntity()).thenReturn(entity); - when(client.performRequest(argThat(new RequestMatcher(is("GET"), is("/"))))) - .thenReturn(versionResponse); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), is("/")), versionResponse); assertTrue(resources.isDirty()); - assertFalse(resources.checkAndPublish(client)); + awaitCheckAndPublish(false); // ensure it didn't magically become clean assertTrue(resources.isDirty()); @@ -116,7 +122,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe verifyNoMoreInteractions(client); } - public void testTemplateCheckBlocksAfterSuccessfulVersion() throws IOException { + public void testTemplateCheckBlocksAfterSuccessfulVersion() { final Exception exception = failureGetException(); final boolean firstSucceeds = randomBoolean(); int expectedGets = 1; @@ -144,20 +150,18 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe final List otherResponses = getTemplateResponses(1, successful, unsuccessful); // last check fails implies that N - 2 publishes succeeded! - when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_template/"))))) - .thenReturn(first, otherResponses.toArray(new Response[otherResponses.size()])) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), startsWith("/_template/")), + first, otherResponses, exception); whenSuccessfulPutTemplates(otherResponses.size() + 1); expectedGets += 1 + successful + unsuccessful; expectedPuts = (successfulFirst ? 0 : 1) + unsuccessful; } else { - when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_template/"))))) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), startsWith("/_template/")), exception); } assertTrue(resources.isDirty()); - assertFalse(resources.checkAndPublish(client)); + awaitCheckAndPublish(null); // ensure it didn't magically become not-dirty assertTrue(resources.isDirty()); @@ -167,7 +171,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe verifyNoMoreInteractions(client); } - public void testTemplatePublishBlocksAfterSuccessfulVersion() throws IOException { + public void testTemplatePublishBlocksAfterSuccessfulVersion() { final Exception exception = failurePutException(); final boolean firstSucceeds = randomBoolean(); int expectedGets = 1; @@ -189,9 +193,8 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe whenGetTemplates(successful, unsuccessful + 2); // previous publishes must have succeeded - when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_template/"))))) - .thenReturn(firstSuccess, otherResponses.toArray(new Response[otherResponses.size()])) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("PUT"), startsWith("/_template/")), + firstSuccess, otherResponses, exception); // GETs required for each PUT attempt (first is guaranteed "unsuccessful") expectedGets += successful + unsuccessful + 1; @@ -201,12 +204,11 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe // fail the check so that it has to attempt the PUT whenGetTemplates(0, 1); - when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_template/"))))) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("PUT"), startsWith("/_template/")), exception); } assertTrue(resources.isDirty()); - assertFalse(resources.checkAndPublish(client)); + awaitCheckAndPublish(null); // ensure it didn't magically become not-dirty assertTrue(resources.isDirty()); @@ -216,7 +218,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe verifyNoMoreInteractions(client); } - public void testPipelineCheckBlocksAfterSuccessfulTemplates() throws IOException { + public void testPipelineCheckBlocksAfterSuccessfulTemplates() { final int successfulGetTemplates = randomIntBetween(0, EXPECTED_TEMPLATES); final int unsuccessfulGetTemplates = EXPECTED_TEMPLATES - successfulGetTemplates; final Exception exception = failureGetException(); @@ -242,9 +244,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe } // last check fails - when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/"))))) - .thenReturn(first) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/")), first, exception); if (successfulFirst == false) { whenSuccessfulPutPipelines(1); } @@ -252,12 +252,11 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe expectedGets = EXPECTED_PIPELINES; expectedPuts = successfulFirst ? 0 : 1; } else { - when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/"))))) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/")), exception); } assertTrue(resources.isDirty()); - assertFalse(resources.checkAndPublish(client)); + awaitCheckAndPublish(null); // ensure it didn't magically become not-dirty assertTrue(resources.isDirty()); @@ -269,7 +268,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe verifyNoMoreInteractions(client); } - public void testPipelinePublishBlocksAfterSuccessfulTemplates() throws IOException { + public void testPipelinePublishBlocksAfterSuccessfulTemplates() { final int successfulGetTemplates = randomIntBetween(0, EXPECTED_TEMPLATES); final int unsuccessfulGetTemplates = EXPECTED_TEMPLATES - successfulGetTemplates; final Exception exception = failurePutException(); @@ -289,9 +288,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe whenGetPipelines(0, 2); // previous publishes must have succeeded - when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/"))))) - .thenReturn(firstSuccess) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/")), firstSuccess, exception); // GETs required for each PUT attempt (first is guaranteed "unsuccessful") expectedGets += 1; @@ -301,12 +298,11 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe // fail the check so that it has to attempt the PUT whenGetPipelines(0, 1); - when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/"))))) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/")), exception); } assertTrue(resources.isDirty()); - assertFalse(resources.checkAndPublish(client)); + awaitCheckAndPublish(null); // ensure it didn't magically become not-dirty assertTrue(resources.isDirty()); @@ -318,7 +314,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe verifyNoMoreInteractions(client); } - public void testWatcherCheckBlocksAfterSuccessfulPipelines() throws IOException { + public void testWatcherCheckBlocksAfterSuccessfulPipelines() { final int successfulGetTemplates = randomIntBetween(0, EXPECTED_TEMPLATES); final int unsuccessfulGetTemplates = EXPECTED_TEMPLATES - successfulGetTemplates; final int successfulGetPipelines = randomIntBetween(0, EXPECTED_PIPELINES); @@ -332,11 +328,10 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe whenSuccessfulPutPipelines(unsuccessfulGetPipelines); // there's only one check - when(client.performRequest(argThat(new RequestMatcher(is("GET"), is("/_xpack"))))) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), is("/_xpack")), exception); assertTrue(resources.isDirty()); - assertFalse(resources.checkAndPublish(client)); + awaitCheckAndPublish(null); // ensure it didn't magically become not-dirty assertTrue(resources.isDirty()); @@ -349,7 +344,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe verifyNoMoreInteractions(client); } - public void testWatchCheckBlocksAfterSuccessfulWatcherCheck() throws IOException { + public void testWatchCheckBlocksAfterSuccessfulWatcherCheck() { final int successfulGetTemplates = randomIntBetween(0, EXPECTED_TEMPLATES); final int unsuccessfulGetTemplates = EXPECTED_TEMPLATES - successfulGetTemplates; final int successfulGetPipelines = randomIntBetween(0, EXPECTED_PIPELINES); @@ -381,9 +376,8 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe final List otherResponses = getWatcherResponses(1, successful, unsuccessful); // last check fails implies that N - 2 publishes succeeded! - when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_xpack/watcher/watch/"))))) - .thenReturn(first, otherResponses.toArray(new Response[otherResponses.size()])) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), startsWith("/_xpack/watcher/watch/")), + first, otherResponses, exception); whenSuccessfulPutWatches(otherResponses.size() + 1); // +1 for the "first" @@ -397,21 +391,19 @@ 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(argThat(new RequestMatcher(is("DELETE"), startsWith("/_xpack/watcher/watch/"))))) - .thenReturn(responses.get(0), responses.subList(1, successful).toArray(new Response[successful - 1])) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("DELETE"), startsWith("/_xpack/watcher/watch/")), + responses.get(0), responses.subList(1, responses.size()), exception); expectedGets += successful; } } else { final String method = validLicense ? "GET" : "DELETE"; - when(client.performRequest(argThat(new RequestMatcher(is(method), startsWith("/_xpack/watcher/watch/"))))) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is(method), startsWith("/_xpack/watcher/watch/")), exception); } assertTrue(resources.isDirty()); - assertFalse(resources.checkAndPublish(client)); + awaitCheckAndPublish(null); // ensure it didn't magically become not-dirty assertTrue(resources.isDirty()); @@ -430,7 +422,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe verifyNoMoreInteractions(client); } - public void testWatchPublishBlocksAfterSuccessfulWatcherCheck() throws IOException { + public void testWatchPublishBlocksAfterSuccessfulWatcherCheck() { final int successfulGetTemplates = randomIntBetween(0, EXPECTED_TEMPLATES); final int unsuccessfulGetTemplates = EXPECTED_TEMPLATES - successfulGetTemplates; final int successfulGetPipelines = randomIntBetween(0, EXPECTED_PIPELINES); @@ -462,9 +454,8 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe whenGetWatches(successful, unsuccessful + 2); // previous publishes must have succeeded - when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/"))))) - .thenReturn(firstSuccess, otherResponses.toArray(new Response[otherResponses.size()])) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/")), + firstSuccess, otherResponses, exception); // GETs required for each PUT attempt (first is guaranteed "unsuccessful") expectedGets += successful + unsuccessful + 1; @@ -474,12 +465,11 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe // fail the check so that it has to attempt the PUT whenGetWatches(0, 1); - when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/"))))) - .thenThrow(exception); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/")), exception); } assertTrue(resources.isDirty()); - assertFalse(resources.checkAndPublish(client)); + awaitCheckAndPublish(null); // ensure it didn't magically become not-dirty assertTrue(resources.isDirty()); @@ -494,7 +484,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe verifyNoMoreInteractions(client); } - public void testSuccessfulChecksOnElectedMasterNode() throws IOException { + public void testSuccessfulChecksOnElectedMasterNode() { final int successfulGetTemplates = randomIntBetween(0, EXPECTED_TEMPLATES); final int unsuccessfulGetTemplates = EXPECTED_TEMPLATES - successfulGetTemplates; final int successfulGetPipelines = randomIntBetween(0, EXPECTED_PIPELINES); @@ -522,7 +512,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe assertTrue(resources.isDirty()); // it should be able to proceed! - assertTrue(resources.checkAndPublish(client)); + awaitCheckAndPublish(true); assertFalse(resources.isDirty()); verifyVersionCheck(); @@ -545,7 +535,7 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe /** * If the node is not the elected master node, then it should never check Watcher or send Watches (Cluster Alerts). */ - public void testSuccessfulChecksIfNotElectedMasterNode() throws IOException { + public void testSuccessfulChecksIfNotElectedMasterNode() { final ClusterState state = mockClusterState(false); final ClusterService clusterService = mockClusterService(state); @@ -566,8 +556,10 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe assertTrue(resources.isDirty()); - // it should be able to proceed! - assertTrue(resources.checkAndPublish(client)); + // it should be able to proceed! (note: we are not using the instance "resources" here) + resources.checkAndPublish(client, listener); + + verifyListener(true); assertFalse(resources.isDirty()); verifyVersionCheck(); @@ -601,13 +593,13 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe } private Response successfulGetWatchResponse(final String watchId) { - final HttpEntity goodEntity = entityForClusterAlert(EXISTS, ClusterAlertsUtil.LAST_UPDATED_VERSION); + final HttpEntity goodEntity = entityForClusterAlert(true, ClusterAlertsUtil.LAST_UPDATED_VERSION); return response("GET", "/_xpack/watcher/watch/" + watchId, successfulCheckStatus(), goodEntity); } private Response unsuccessfulGetWatchResponse(final String watchId) { if (randomBoolean()) { - final HttpEntity badEntity = entityForClusterAlert(DOES_NOT_EXIST, ClusterAlertsUtil.LAST_UPDATED_VERSION); + final HttpEntity badEntity = entityForClusterAlert(false, ClusterAlertsUtil.LAST_UPDATED_VERSION); return response("GET", "/_xpack/watcher/watch/" + watchId, successfulCheckStatus(), badEntity); } @@ -616,14 +608,14 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe } private Response successfulGetResourceResponse(final String resourcePath, final String resourceName) { - final HttpEntity goodEntity = entityForResource(EXISTS, resourceName, MonitoringTemplateUtils.LAST_UPDATED_VERSION); + final HttpEntity goodEntity = entityForResource(true, resourceName, MonitoringTemplateUtils.LAST_UPDATED_VERSION); return response("GET", resourcePath + resourceName, successfulCheckStatus(), goodEntity); } private Response unsuccessfulGetResourceResponse(final String resourcePath, final String resourceName) { if (randomBoolean()) { - final HttpEntity badEntity = entityForResource(DOES_NOT_EXIST, resourceName, MonitoringTemplateUtils.LAST_UPDATED_VERSION); + final HttpEntity badEntity = entityForResource(false, resourceName, MonitoringTemplateUtils.LAST_UPDATED_VERSION); return response("GET", resourcePath + resourceName, successfulCheckStatus(), badEntity); } @@ -704,65 +696,40 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe return responses; } - private void whenValidVersionResponse() throws IOException { + private void whenValidVersionResponse() { final HttpEntity entity = new StringEntity("{\"version\":{\"number\":\"" + Version.CURRENT + "\"}}", ContentType.APPLICATION_JSON); when(versionResponse.getEntity()).thenReturn(entity); - when(client.performRequest(argThat(new RequestMatcher(is("GET"), is("/"))))) - .thenReturn(versionResponse); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), is("/")), versionResponse); } - private void whenGetTemplates(final int successful, final int unsuccessful) throws IOException { + private void whenGetTemplates(final int successful, final int unsuccessful) { final List gets = getTemplateResponses(0, successful, unsuccessful); - if (gets.size() == 1) { - when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_template/"))))) - .thenReturn(gets.get(0)); - } else { - 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])); - } + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), startsWith("/_template/")), gets); } - private void whenSuccessfulPutTemplates(final int successful) throws IOException { + private void whenSuccessfulPutTemplates(final int successful) { final List successfulPuts = successfulPutResponses(successful); // empty is possible if they all exist - if (successful == 1) { - when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_template/"))))) - .thenReturn(successfulPuts.get(0)); - } else if (successful > 1) { - when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_template/"))))) - .thenReturn(successfulPuts.get(0), successfulPuts.subList(1, successful).toArray(new Response[successful - 1])); - } + whenPerformRequestAsyncWith(client, new RequestMatcher(is("PUT"), startsWith("/_template/")), successfulPuts); } - private void whenGetPipelines(final int successful, final int unsuccessful) throws IOException { + private void whenGetPipelines(final int successful, final int unsuccessful) { final List gets = getPipelineResponses(0, successful, unsuccessful); - if (gets.size() == 1) { - when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/"))))) - .thenReturn(gets.get(0)); - } else { - 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])); - } + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/")), gets); } - private void whenSuccessfulPutPipelines(final int successful) throws IOException { + private void whenSuccessfulPutPipelines(final int successful) { final List successfulPuts = successfulPutResponses(successful); // empty is possible if they all exist - if (successful == 1) { - when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/"))))) - .thenReturn(successfulPuts.get(0)); - } else if (successful > 1) { - when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/"))))) - .thenReturn(successfulPuts.get(0), successfulPuts.subList(1, successful).toArray(new Response[successful - 1])); - } + whenPerformRequestAsyncWith(client, new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/")), successfulPuts); } - private void whenWatcherCanBeUsed(final boolean validLicense) throws IOException { + private void whenWatcherCanBeUsed(final boolean validLicense) { final MetaData metaData = mock(MetaData.class); when(state.metaData()).thenReturn(metaData); @@ -774,12 +741,10 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe new StringEntity("{\"features\":{\"watcher\":{\"enabled\":true,\"available\":true}}}", ContentType.APPLICATION_JSON); final Response successfulGet = response("GET", "_xpack", successfulCheckStatus(), entity); - // empty is possible if they all exist - when(client.performRequest(argThat(new RequestMatcher(is("GET"), is("/_xpack"))))) - .thenReturn(successfulGet); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), is("/_xpack")), successfulGet); } - private void whenWatcherCannotBeUsed() throws IOException { + private void whenWatcherCannotBeUsed() { final Response response; if (randomBoolean()) { final HttpEntity entity = randomFrom( @@ -793,90 +758,71 @@ public class HttpExporterResourceTests extends AbstractPublishableHttpResourceTe response = response("GET", "_xpack", notFoundCheckStatus()); } - // empty is possible if they all exist - when(client.performRequest(argThat(new RequestMatcher(is("GET"), is("/_xpack"))))) - .thenReturn(response); + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), is("/_xpack")), response); } - private void whenGetWatches(final int successful, final int unsuccessful) throws IOException { + private void whenGetWatches(final int successful, final int unsuccessful) { final List gets = getWatcherResponses(0, successful, unsuccessful); - if (gets.size() == 1) { - when(client.performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_xpack/watcher/watch/"))))) - .thenReturn(gets.get(0)); - } else { - 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])); - } + whenPerformRequestAsyncWith(client, new RequestMatcher(is("GET"), startsWith("/_xpack/watcher/watch/")), gets); } - private void whenSuccessfulPutWatches(final int successful) throws IOException { + private void whenSuccessfulPutWatches(final int successful) { final List successfulPuts = successfulPutResponses(successful); // empty is possible if they all exist - if (successful == 1) { - when(client.performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/"))))) - .thenReturn(successfulPuts.get(0)); - } else if (successful > 1) { - 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])); - } + whenPerformRequestAsyncWith(client, new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/")), successfulPuts); } - private void whenSuccessfulDeleteWatches(final int successful) throws IOException { + private void whenSuccessfulDeleteWatches(final int successful) { final List successfulDeletes = successfulDeleteResponses(successful); // empty is possible if they all exist - if (successful == 1) { - when(client.performRequest(argThat(new RequestMatcher(is("DELETE"), startsWith("/_xpack/watcher/watch/"))))) - .thenReturn(successfulDeletes.get(0)); - } else if (successful > 1) { - 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])); - } + whenPerformRequestAsyncWith(client, new RequestMatcher(is("DELETE"), startsWith("/_xpack/watcher/watch/")), successfulDeletes); } - private void verifyVersionCheck() throws IOException { - verify(client).performRequest(argThat(new RequestMatcher(is("GET"), is("/")))); + private void verifyVersionCheck() { + verify(client).performRequestAsync(argThat(new RequestMatcher(is("GET"), is("/"))), any(ResponseListener.class)); } - private void verifyGetTemplates(final int called) throws IOException { + private void verifyGetTemplates(final int called) { verify(client, times(called)) - .performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_template/")))); + .performRequestAsync(argThat(new RequestMatcher(is("GET"), startsWith("/_template/"))), any(ResponseListener.class)); } - private void verifyPutTemplates(final int called) throws IOException { + private void verifyPutTemplates(final int called) { verify(client, times(called)) - .performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_template/")))); + .performRequestAsync(argThat(new RequestMatcher(is("PUT"), startsWith("/_template/"))), any(ResponseListener.class)); } - private void verifyGetPipelines(final int called) throws IOException { + private void verifyGetPipelines(final int called) { verify(client, times(called)) - .performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/")))); + .performRequestAsync(argThat(new RequestMatcher(is("GET"), startsWith("/_ingest/pipeline/"))), any(ResponseListener.class)); } - private void verifyPutPipelines(final int called) throws IOException { + private void verifyPutPipelines(final int called) { verify(client, times(called)) - .performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/")))); + .performRequestAsync(argThat(new RequestMatcher(is("PUT"), startsWith("/_ingest/pipeline/"))), any(ResponseListener.class)); } - private void verifyWatcherCheck() throws IOException { - verify(client).performRequest(argThat(new RequestMatcher(is("GET"), is("/_xpack")))); + private void verifyWatcherCheck() { + verify(client).performRequestAsync(argThat(new RequestMatcher(is("GET"), is("/_xpack"))), any(ResponseListener.class)); } - private void verifyDeleteWatches(final int called) throws IOException { + private void verifyDeleteWatches(final int called) { verify(client, times(called)) - .performRequest(argThat(new RequestMatcher(is("DELETE"), startsWith("/_xpack/watcher/watch/")))); + .performRequestAsync(argThat(new RequestMatcher(is("DELETE"), startsWith("/_xpack/watcher/watch/"))), + any(ResponseListener.class)); } - private void verifyGetWatches(final int called) throws IOException { + private void verifyGetWatches(final int called) { verify(client, times(called)) - .performRequest(argThat(new RequestMatcher(is("GET"), startsWith("/_xpack/watcher/watch/")))); + .performRequestAsync(argThat(new RequestMatcher(is("GET"), startsWith("/_xpack/watcher/watch/"))), any(ResponseListener.class)); } - private void verifyPutWatches(final int called) throws IOException { + private void verifyPutWatches(final int called) { verify(client, times(called)) - .performRequest(argThat(new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/")))); + .performRequestAsync(argThat(new RequestMatcher(is("PUT"), startsWith("/_xpack/watcher/watch/"))), any(ResponseListener.class)); } private ClusterService mockClusterService(final ClusterState state) { diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java index a96dc8ebb12..9c036a53f9e 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java @@ -5,9 +5,12 @@ */ package org.elasticsearch.xpack.monitoring.exporter.http; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.apache.http.nio.conn.ssl.SSLIOSessionStrategy; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; @@ -25,6 +28,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils; import org.elasticsearch.xpack.core.ssl.SSLService; import org.elasticsearch.xpack.monitoring.exporter.ClusterAlertsUtil; +import org.elasticsearch.xpack.monitoring.exporter.ExportBulk; import org.elasticsearch.xpack.monitoring.exporter.Exporter.Config; import org.junit.Before; import org.mockito.InOrder; @@ -434,12 +438,50 @@ public class HttpExporterTests extends ESTestCase { final RestClient client = mock(RestClient.class); final Sniffer sniffer = randomFrom(mock(Sniffer.class), null); final NodeFailureListener listener = mock(NodeFailureListener.class); - final HttpResource resource = new MockHttpResource(exporterName(), true, PublishableHttpResource.CheckResponse.ERROR, false); + // this is configured to throw an error when the resource is checked + final HttpResource resource = new MockHttpResource(exporterName(), true, null, false); try (HttpExporter exporter = new HttpExporter(config, client, sniffer, threadContext, listener, resource)) { verify(listener).setResource(resource); - assertThat(exporter.openBulk(), nullValue()); + final CountDownLatch awaitResponseAndClose = new CountDownLatch(1); + final ActionListener bulkListener = ActionListener.wrap( + bulk -> fail("[onFailure] should have been invoked by failed resource check"), + e -> awaitResponseAndClose.countDown() + ); + + exporter.openBulk(bulkListener); + + // wait for it to actually respond + assertTrue(awaitResponseAndClose.await(15, TimeUnit.SECONDS)); + } + } + + public void testHttpExporterReturnsNullForOpenBulkIfNotReady() throws Exception { + final Config config = createConfig(Settings.EMPTY); + final RestClient client = mock(RestClient.class); + final Sniffer sniffer = randomFrom(mock(Sniffer.class), null); + final NodeFailureListener listener = mock(NodeFailureListener.class); + // always has to check, and never succeeds checks but it does not throw an exception (e.g., version check fails) + final HttpResource resource = new MockHttpResource(exporterName(), true, false, false); + + try (HttpExporter exporter = new HttpExporter(config, client, sniffer, threadContext, listener, resource)) { + verify(listener).setResource(resource); + + final CountDownLatch awaitResponseAndClose = new CountDownLatch(1); + final ActionListener bulkListener = ActionListener.wrap( + bulk -> { + assertThat(bulk, nullValue()); + + awaitResponseAndClose.countDown(); + }, + e -> fail(e.getMessage()) + ); + + exporter.openBulk(bulkListener); + + // wait for it to actually respond + assertTrue(awaitResponseAndClose.await(15, TimeUnit.SECONDS)); } } @@ -454,9 +496,20 @@ public class HttpExporterTests extends ESTestCase { try (HttpExporter exporter = new HttpExporter(config, client, sniffer, threadContext, listener, resource)) { verify(listener).setResource(resource); - final HttpExportBulk bulk = exporter.openBulk(); + final CountDownLatch awaitResponseAndClose = new CountDownLatch(1); + final ActionListener bulkListener = ActionListener.wrap( + bulk -> { + assertThat(bulk.getName(), equalTo(exporterName())); - assertThat(bulk.getName(), equalTo(exporterName())); + awaitResponseAndClose.countDown(); + }, + e -> fail(e.getMessage()) + ); + + exporter.openBulk(bulkListener); + + // wait for it to actually respond + assertTrue(awaitResponseAndClose.await(15, TimeUnit.SECONDS)); } } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpResourceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpResourceTests.java index 565600fe41d..dfb71e4e048 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpResourceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpResourceTests.java @@ -5,11 +5,15 @@ */ package org.elasticsearch.xpack.monitoring.exporter.http; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.RestClient; import org.elasticsearch.test.ESTestCase; import java.util.function.Supplier; +import static org.elasticsearch.xpack.monitoring.exporter.http.AsyncHttpResourceHelper.mockBooleanActionListener; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -26,8 +30,8 @@ public class HttpResourceTests extends ESTestCase { public void testConstructorRequiresOwner() { expectThrows(NullPointerException.class, () -> new HttpResource(null) { @Override - protected boolean doCheckAndPublish(RestClient client) { - return false; + protected void doCheckAndPublish(RestClient client, ActionListener listener) { + listener.onResponse(false); } }); } @@ -35,8 +39,8 @@ public class HttpResourceTests extends ESTestCase { public void testConstructor() { final HttpResource resource = new HttpResource(owner) { @Override - protected boolean doCheckAndPublish(RestClient client) { - return false; + protected void doCheckAndPublish(RestClient client, ActionListener listener) { + listener.onResponse(false); } }; @@ -48,8 +52,8 @@ public class HttpResourceTests extends ESTestCase { final boolean dirty = randomBoolean(); final HttpResource resource = new HttpResource(owner, dirty) { @Override - protected boolean doCheckAndPublish(RestClient client) { - return false; + protected void doCheckAndPublish(RestClient client, ActionListener listener) { + listener.onResponse(false); } }; @@ -58,6 +62,7 @@ public class HttpResourceTests extends ESTestCase { } public void testDirtiness() { + final ActionListener listener = mockBooleanActionListener(); // MockHttpResponse always succeeds for checkAndPublish final HttpResource resource = new MockHttpResource(owner); @@ -68,59 +73,120 @@ public class HttpResourceTests extends ESTestCase { assertTrue(resource.isDirty()); // if this fails, then the mocked resource needs to be fixed - assertTrue(resource.checkAndPublish(client)); + resource.checkAndPublish(client, listener); + verify(listener).onResponse(true); assertFalse(resource.isDirty()); } public void testCheckAndPublish() { + final ActionListener listener = mockBooleanActionListener(); final boolean expected = randomBoolean(); // the default dirtiness should be irrelevant; it should always be run! final HttpResource resource = new HttpResource(owner) { @Override - protected boolean doCheckAndPublish(final RestClient client) { - return expected; - } - }; - - assertEquals(expected, resource.checkAndPublish(client)); - } - - public void testCheckAndPublishEvenWhenDirty() { - final Supplier supplier = mock(Supplier.class); - when(supplier.get()).thenReturn(true, false); - - final HttpResource resource = new HttpResource(owner) { - @Override - protected boolean doCheckAndPublish(final RestClient client) { - return supplier.get(); + protected void doCheckAndPublish(RestClient client, ActionListener listener) { + listener.onResponse(expected); } }; - assertTrue(resource.isDirty()); - assertTrue(resource.checkAndPublish(client)); - assertFalse(resource.isDirty()); - assertFalse(resource.checkAndPublish(client)); + resource.checkAndPublish(client, listener); - verify(supplier, times(2)).get(); + verify(listener).onResponse(expected); } - public void testCheckAndPublishIfDirty() { + public void testCheckAndPublishEvenWhenDirty() { + final ActionListener listener1 = mockBooleanActionListener(); + final ActionListener listener2 = mockBooleanActionListener(); @SuppressWarnings("unchecked") final Supplier supplier = mock(Supplier.class); when(supplier.get()).thenReturn(true, false); final HttpResource resource = new HttpResource(owner) { @Override - protected boolean doCheckAndPublish(final RestClient client) { - return supplier.get(); + protected void doCheckAndPublish(RestClient client, ActionListener listener) { + listener.onResponse(supplier.get()); } }; assertTrue(resource.isDirty()); - assertTrue(resource.checkAndPublishIfDirty(client)); + resource.checkAndPublish(client, listener1); + verify(listener1).onResponse(true); assertFalse(resource.isDirty()); - assertTrue(resource.checkAndPublishIfDirty(client)); + resource.checkAndPublish(client, listener2); + verify(listener2).onResponse(false); + + verify(supplier, times(2)).get(); + } + + public void testCheckAndPublishIfDirtyFalseWhileChecking() throws InterruptedException { + final CountDownLatch firstCheck = new CountDownLatch(1); + final CountDownLatch secondCheck = new CountDownLatch(1); + + final boolean response = randomBoolean(); + final ActionListener listener = mockBooleanActionListener(); + // listener used while checking is blocked, and thus should be ignored + final ActionListener checkingListener = ActionListener.wrap( + success -> { + // busy checking, so this should be ignored + assertFalse(success); + secondCheck.countDown(); + }, + e -> { + fail(e.getMessage()); + secondCheck.countDown(); + } + ); + + // the default dirtiness should be irrelevant; it should always be run! + final HttpResource resource = new HttpResource(owner) { + @Override + protected void doCheckAndPublish(RestClient client, ActionListener listener) { + // wait until the second check has had a chance to run to completion, + // then respond here + final Thread thread = new Thread(() -> { + try { + assertTrue(secondCheck.await(15, TimeUnit.SECONDS)); + listener.onResponse(response); + } catch (InterruptedException e) { + listener.onFailure(e); + } + + firstCheck.countDown(); + }); + thread.start(); + } + }; + + resource.checkAndPublishIfDirty(client, listener); + resource.checkAndPublishIfDirty(client, checkingListener); + + assertTrue(firstCheck.await(15, TimeUnit.SECONDS)); + + verify(listener).onResponse(response); + + } + + public void testCheckAndPublishIfDirty() { + final ActionListener listener1 = mockBooleanActionListener(); + final ActionListener listener2 = mockBooleanActionListener(); + @SuppressWarnings("unchecked") + final Supplier supplier = mock(Supplier.class); + when(supplier.get()).thenReturn(true, false); + + final HttpResource resource = new HttpResource(owner) { + @Override + protected void doCheckAndPublish(RestClient client, ActionListener listener) { + listener.onResponse(supplier.get()); + } + }; + + assertTrue(resource.isDirty()); + resource.checkAndPublishIfDirty(client, listener1); + verify(listener1).onResponse(true); + assertFalse(resource.isDirty()); + resource.checkAndPublishIfDirty(client, listener2); + verify(listener2).onResponse(true); // once is the default! verify(supplier).get(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/MockHttpResource.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/MockHttpResource.java index 1824036eb05..57f6f96ca83 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/MockHttpResource.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/MockHttpResource.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.monitoring.exporter.http; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.RestClient; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.unit.TimeValue; @@ -17,8 +18,8 @@ import java.util.Map; */ public class MockHttpResource extends PublishableHttpResource { - public final CheckResponse check; - public final boolean publish; + public final Boolean check; + public final Boolean publish; public int checked = 0; public int published = 0; @@ -29,7 +30,7 @@ public class MockHttpResource extends PublishableHttpResource { * @param resourceOwnerName The user-recognizable name */ public MockHttpResource(final String resourceOwnerName) { - this(resourceOwnerName, true, CheckResponse.EXISTS, true); + this(resourceOwnerName, true, true, true); } /** @@ -39,7 +40,7 @@ public class MockHttpResource extends PublishableHttpResource { * @param dirty The starting dirtiness of the resource. */ public MockHttpResource(final String resourceOwnerName, final boolean dirty) { - this(resourceOwnerName, dirty, CheckResponse.EXISTS, true); + this(resourceOwnerName, dirty, true, true); } /** @@ -50,7 +51,7 @@ public class MockHttpResource extends PublishableHttpResource { * @param parameters The base parameters to specify for the request. */ public MockHttpResource(final String resourceOwnerName, @Nullable final TimeValue masterTimeout, final Map parameters) { - this(resourceOwnerName, masterTimeout, parameters, true, CheckResponse.EXISTS, true); + this(resourceOwnerName, masterTimeout, parameters, true, true, true); } /** @@ -59,9 +60,9 @@ public class MockHttpResource extends PublishableHttpResource { * @param resourceOwnerName The user-recognizable name * @param dirty The starting dirtiness of the resource. * @param check The expected response when checking for the resource. - * @param publish The expected response when publishing the resource (assumes check was {@link CheckResponse#DOES_NOT_EXIST}). + * @param publish The expected response when publishing the resource (assumes check was {@code false}). */ - public MockHttpResource(final String resourceOwnerName, final boolean dirty, final CheckResponse check, final boolean publish) { + public MockHttpResource(final String resourceOwnerName, final boolean dirty, final Boolean check, final Boolean publish) { this(resourceOwnerName, null, Collections.emptyMap(), dirty, check, publish); } @@ -70,12 +71,12 @@ public class MockHttpResource extends PublishableHttpResource { * * @param resourceOwnerName The user-recognizable name * @param check The expected response when checking for the resource. - * @param publish The expected response when publishing the resource (assumes check was {@link CheckResponse#DOES_NOT_EXIST}). + * @param publish The expected response when publishing the resource (assumes check was {@code false}). * @param masterTimeout Master timeout to use with any request. * @param parameters The base parameters to specify for the request. */ public MockHttpResource(final String resourceOwnerName, @Nullable final TimeValue masterTimeout, final Map parameters, - final CheckResponse check, final boolean publish) { + final Boolean check, final Boolean publish) { this(resourceOwnerName, masterTimeout, parameters, true, check, publish); } @@ -85,12 +86,12 @@ public class MockHttpResource extends PublishableHttpResource { * @param resourceOwnerName The user-recognizable name * @param dirty The starting dirtiness of the resource. * @param check The expected response when checking for the resource. - * @param publish The expected response when publishing the resource (assumes check was {@link CheckResponse#DOES_NOT_EXIST}). + * @param publish The expected response when publishing the resource (assumes check was {@code false}). * @param masterTimeout Master timeout to use with any request. * @param parameters The base parameters to specify for the request. */ public MockHttpResource(final String resourceOwnerName, @Nullable final TimeValue masterTimeout, final Map parameters, - final boolean dirty, final CheckResponse check, final boolean publish) { + final boolean dirty, final Boolean check, final Boolean publish) { super(resourceOwnerName, masterTimeout, parameters, dirty); this.check = check; @@ -98,21 +99,30 @@ public class MockHttpResource extends PublishableHttpResource { } @Override - protected CheckResponse doCheck(final RestClient client) { + protected void doCheck(final RestClient client, final ActionListener listener) { assert client != null; ++checked; - return check; + if (check == null) { + listener.onFailure(new RuntimeException("TEST - expected")); + } else { + listener.onResponse(check); + } } @Override - protected boolean doPublish(final RestClient client) { + protected void doPublish(final RestClient client, final ActionListener listener) { assert client != null; ++published; - return publish; + + if (publish == null) { + listener.onFailure(new RuntimeException("TEST - expected")); + } else { + listener.onResponse(publish); + } } } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/MultiHttpResourceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/MultiHttpResourceTests.java index 393ca8e5c21..5a20733a9e9 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/MultiHttpResourceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/MultiHttpResourceTests.java @@ -5,16 +5,19 @@ */ package org.elasticsearch.xpack.monitoring.exporter.http; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.RestClient; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import static org.elasticsearch.xpack.monitoring.exporter.http.AsyncHttpResourceHelper.mockBooleanActionListener; import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; /** * Tests {@link MultiHttpResource}. @@ -23,12 +26,15 @@ public class MultiHttpResourceTests extends ESTestCase { private final String owner = getClass().getSimpleName(); private final RestClient client = mock(RestClient.class); + private final ActionListener listener = mockBooleanActionListener(); public void testDoCheckAndPublish() { final List allResources = successfulResources(); final MultiHttpResource multiResource = new MultiHttpResource(owner, allResources); - assertTrue(multiResource.doCheckAndPublish(client)); + multiResource.doCheckAndPublish(client, listener); + + verify(listener).onResponse(true); for (final MockHttpResource resource : allResources) { assertSuccessfulResource(resource); @@ -37,8 +43,8 @@ public class MultiHttpResourceTests extends ESTestCase { public void testDoCheckAndPublishShortCircuits() { // fail either the check or the publish - final CheckResponse check = randomFrom(CheckResponse.ERROR, CheckResponse.DOES_NOT_EXIST); - final boolean publish = check == CheckResponse.ERROR; + final Boolean check = randomBoolean() ? null : false; + final boolean publish = check == null; final List allResources = successfulResources(); final MockHttpResource failureResource = new MockHttpResource(owner, true, check, publish); @@ -48,7 +54,13 @@ public class MultiHttpResourceTests extends ESTestCase { final MultiHttpResource multiResource = new MultiHttpResource(owner, allResources); - assertFalse(multiResource.doCheckAndPublish(client)); + multiResource.doCheckAndPublish(client, listener); + + if (check == null) { + verify(listener).onFailure(any(Exception.class)); + } else { + verify(listener).onResponse(false); + } boolean found = false; @@ -56,7 +68,7 @@ public class MultiHttpResourceTests extends ESTestCase { // should stop looking at this point if (resource == failureResource) { assertThat(resource.checked, equalTo(1)); - if (resource.check == CheckResponse.ERROR) { + if (resource.check == null) { assertThat(resource.published, equalTo(0)); } else { assertThat(resource.published, equalTo(1)); @@ -85,8 +97,8 @@ public class MultiHttpResourceTests extends ESTestCase { final List resources = new ArrayList<>(successful); for (int i = 0; i < successful; ++i) { - final CheckResponse check = randomFrom(CheckResponse.DOES_NOT_EXIST, CheckResponse.EXISTS); - final MockHttpResource resource = new MockHttpResource(owner, randomBoolean(), check, check == CheckResponse.DOES_NOT_EXIST); + final boolean check = randomBoolean(); + final MockHttpResource resource = new MockHttpResource(owner, randomBoolean(), check, check == false); resources.add(resource); } @@ -96,7 +108,7 @@ public class MultiHttpResourceTests extends ESTestCase { private void assertSuccessfulResource(final MockHttpResource resource) { assertThat(resource.checked, equalTo(1)); - if (resource.check == CheckResponse.DOES_NOT_EXIST) { + if (resource.check == false) { assertThat(resource.published, equalTo(1)); } else { assertThat(resource.published, equalTo(0)); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/PipelineHttpResourceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/PipelineHttpResourceTests.java index c97f5d03c0f..b0ba7442112 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/PipelineHttpResourceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/PipelineHttpResourceTests.java @@ -15,9 +15,6 @@ import java.io.IOException; import java.io.InputStream; import java.util.function.Supplier; -import static org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse.DOES_NOT_EXIST; -import static org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse.ERROR; -import static org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse.EXISTS; import static org.hamcrest.Matchers.is; /** @@ -48,46 +45,42 @@ public class PipelineHttpResourceTests extends AbstractPublishableHttpResourceTe assertThat(byteStream.available(), is(0)); } - public void testDoCheckExists() throws IOException { - final HttpEntity entity = entityForResource(EXISTS, pipelineName, minimumVersion); + public void testDoCheckExists() { + final HttpEntity entity = entityForResource(true, pipelineName, minimumVersion); - doCheckWithStatusCode(resource, "/_ingest/pipeline", pipelineName, successfulCheckStatus(), EXISTS, entity); + doCheckWithStatusCode(resource, "/_ingest/pipeline", pipelineName, successfulCheckStatus(), true, entity); } - public void testDoCheckDoesNotExist() throws IOException { + public void testDoCheckDoesNotExist() { if (randomBoolean()) { // it does not exist because it's literally not there assertCheckDoesNotExist(resource, "/_ingest/pipeline", pipelineName); } else { // it does not exist because we need to replace it - final HttpEntity entity = entityForResource(DOES_NOT_EXIST, pipelineName, minimumVersion); + final HttpEntity entity = entityForResource(false, pipelineName, minimumVersion); doCheckWithStatusCode(resource, "/_ingest/pipeline", pipelineName, - successfulCheckStatus(), DOES_NOT_EXIST, entity); + successfulCheckStatus(), false, entity); } } - public void testDoCheckError() throws IOException { + public void testDoCheckError() { if (randomBoolean()) { // error because of a server error assertCheckWithException(resource, "/_ingest/pipeline", pipelineName); } else { // error because of a malformed response - final HttpEntity entity = entityForResource(ERROR, pipelineName, minimumVersion); + final HttpEntity entity = entityForResource(null, pipelineName, minimumVersion); - doCheckWithStatusCode(resource, "/_ingest/pipeline", pipelineName, successfulCheckStatus(), ERROR, entity); + doCheckWithStatusCode(resource, "/_ingest/pipeline", pipelineName, successfulCheckStatus(), null, entity); } } - public void testDoPublishTrue() throws IOException { + public void testDoPublishTrue() { assertPublishSucceeds(resource, "/_ingest/pipeline", pipelineName, ByteArrayEntity.class); } - public void testDoPublishFalse() throws IOException { - assertPublishFails(resource, "/_ingest/pipeline", pipelineName, ByteArrayEntity.class); - } - - public void testDoPublishFalseWithException() throws IOException { + public void testDoPublishFalseWithException() { assertPublishWithException(resource, "/_ingest/pipeline", pipelineName, ByteArrayEntity.class); } 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 e32effc85f9..f0ab6484f79 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 @@ -13,25 +13,28 @@ 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.ResponseListener; import org.elasticsearch.client.RestClient; +import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.SuppressLoggerChecks; -import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestStatus; -import org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse; import org.mockito.ArgumentCaptor; import java.io.IOException; import java.util.function.Supplier; +import static org.elasticsearch.xpack.monitoring.exporter.http.AsyncHttpResourceHelper.whenPerformRequestAsyncWith; import static org.hamcrest.Matchers.is; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; /** @@ -51,11 +54,11 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc new MockHttpResource(owner, masterTimeout, PublishableHttpResource.NO_BODY_PARAMETERS); public void testCheckForResourceExists() throws IOException { - assertCheckForResource(successfulCheckStatus(), CheckResponse.EXISTS, "{} [{}] found on the [{}] {}"); + assertCheckForResource(successfulCheckStatus(), true, "{} [{}] found on the [{}] {}"); } public void testCheckForResourceDoesNotExist() throws IOException { - assertCheckForResource(notFoundCheckStatus(), CheckResponse.DOES_NOT_EXIST, "{} [{}] does not exist on the [{}] {}"); + assertCheckForResource(notFoundCheckStatus(), false, "{} [{}] does not exist on the [{}] {}"); } public void testCheckForResourceUnexpectedResponse() throws IOException { @@ -65,34 +68,34 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc final Request request = new Request("GET", endpoint); addParameters(request, getParameters(resource.getParameters())); - when(client.performRequest(request)).thenReturn(response); + whenPerformRequestAsyncWith(client, request, response); - sometimesAssertSimpleCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, CheckResponse.ERROR, response); + assertCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, null, response); verify(logger).trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest(request); + verify(client).performRequestAsync(eq(request), any(ResponseListener.class)); verify(logger).error(any(org.apache.logging.log4j.util.Supplier.class), any(ResponseException.class)); verifyNoMoreInteractions(client, logger); } - public void testVersionCheckForResourceExists() throws IOException { - assertVersionCheckForResource(successfulCheckStatus(), CheckResponse.EXISTS, randomInt(), "{} [{}] found on the [{}] {}"); + public void testVersionCheckForResourceExists() { + assertVersionCheckForResource(successfulCheckStatus(), true, randomInt(), "{} [{}] found on the [{}] {}"); } - public void testVersionCheckForResourceDoesNotExist() throws IOException { + public void testVersionCheckForResourceDoesNotExist() { if (randomBoolean()) { // it literally does not exist - assertVersionCheckForResource(notFoundCheckStatus(), CheckResponse.DOES_NOT_EXIST, + assertVersionCheckForResource(notFoundCheckStatus(), false, randomInt(), "{} [{}] does not exist on the [{}] {}"); } else { // it DOES exist, but the version needs to be replaced - assertVersionCheckForResource(successfulCheckStatus(), CheckResponse.DOES_NOT_EXIST, + assertVersionCheckForResource(successfulCheckStatus(), false, randomInt(), "{} [{}] found on the [{}] {}"); } } - public void testVersionCheckForResourceUnexpectedResponse() throws IOException { + public void testVersionCheckForResourceUnexpectedResponse() { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final RestStatus failedStatus = failedCheckStatus(); final Response response = response("GET", endpoint, failedStatus); @@ -101,41 +104,41 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc final Request request = new Request("GET", endpoint); addParameters(request, getParameters(resource.getParameters())); - when(client.performRequest(request)).thenReturn(response); + whenPerformRequestAsyncWith(client, request, response); - assertThat(resource.versionCheckForResource(client, logger, - resourceBasePath, resourceName, resourceType, owner, ownerType, - xContent, minimumVersion), - is(CheckResponse.ERROR)); + resource.versionCheckForResource(client, listener, logger, + resourceBasePath, resourceName, resourceType, owner, ownerType, + xContent, minimumVersion); + verifyListener(null); verify(logger).trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest(request); + verify(client).performRequestAsync(eq(request), any(ResponseListener.class)); verify(logger).error(any(org.apache.logging.log4j.util.Supplier.class), any(ResponseException.class)); verifyNoMoreInteractions(client, logger); } - public void testVersionCheckForResourceMalformedResponse() throws IOException { + public void testVersionCheckForResourceMalformedResponse() { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final RestStatus okStatus = successfulCheckStatus(); final int minimumVersion = randomInt(); - final HttpEntity entity = entityForResource(CheckResponse.ERROR, resourceName, minimumVersion); + final HttpEntity entity = entityForResource(null, 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(request)).thenReturn(response); + whenPerformRequestAsyncWith(client, request, response); - assertThat(resource.versionCheckForResource(client, logger, - resourceBasePath, resourceName, resourceType, owner, ownerType, - xContent, minimumVersion), - is(CheckResponse.ERROR)); + resource.versionCheckForResource(client, listener, logger, + resourceBasePath, resourceName, resourceType, owner, ownerType, + xContent, minimumVersion); + verifyListener(null); 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(request); - verify(logger).error(any(org.apache.logging.log4j.util.Supplier.class), any(ResponseException.class)); + verify(client).performRequestAsync(eq(request), any(ResponseListener.class)); + verify(logger, times(2)).error(any(org.apache.logging.log4j.util.Supplier.class), any(ResponseException.class)); verifyNoMoreInteractions(client, logger); } @@ -147,56 +150,59 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc final Exception e = randomFrom(new IOException("expected"), new RuntimeException("expected"), responseException); final Response response = e == responseException ? responseException.getResponse() : null; - Request request = new Request("GET", endpoint); + final 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); + whenPerformRequestAsyncWith(client, request, e); + + assertCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, null, response); verify(logger).trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest(request); + verify(client).performRequestAsync(eq(request), any(ResponseListener.class)); verify(logger).error(any(org.apache.logging.log4j.util.Supplier.class), eq(e)); verifyNoMoreInteractions(client, logger); } - public void testPutResourceTrue() throws IOException { + public void testPutResourceTrue() { assertPutResource(successfulPublishStatus(), true); } - public void testPutResourceFalse() throws IOException { + public void testPutResourceFalse() { assertPutResource(failedPublishStatus(), false); } - public void testPutResourceFalseWithException() throws IOException { + public void testPutResourceFalseWithException() { 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(request)).thenThrow(e); + whenPerformRequestAsyncWith(client, request, e); - assertThat(resource.putResource(client, logger, resourceBasePath, resourceName, body, resourceType, owner, ownerType), is(false)); + resource.putResource(client, listener, logger, resourceBasePath, resourceName, body, resourceType, owner, ownerType); + + verifyListener(null); verify(logger).trace("uploading {} [{}] to the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest(request); + verify(client).performRequestAsync(eq(request), any(ResponseListener.class)); verify(logger).error(any(org.apache.logging.log4j.util.Supplier.class), eq(e)); verifyNoMoreInteractions(client, logger); } - public void testDeleteResourceTrue() throws IOException { + public void testDeleteResourceTrue() { final RestStatus status = randomFrom(successfulCheckStatus(), notFoundCheckStatus()); assertDeleteResource(status, true); } - public void testDeleteResourceFalse() throws IOException { + public void testDeleteResourceFalse() { assertDeleteResource(failedCheckStatus(), false); } - public void testDeleteResourceErrors() throws IOException { + public void testDeleteResourceErrors() { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); final RestStatus failedStatus = failedCheckStatus(); final ResponseException responseException = responseException("DELETE", endpoint, failedStatus); @@ -205,12 +211,14 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc final Request request = new Request("DELETE", endpoint); addParameters(request, deleteParameters); - when(client.performRequest(request)).thenThrow(e); + whenPerformRequestAsyncWith(client, request, e); - assertThat(resource.deleteResource(client, logger, resourceBasePath, resourceName, resourceType, owner, ownerType), is(false)); + resource.deleteResource(client, listener, logger, resourceBasePath, resourceName, resourceType, owner, ownerType); + + verifyListener(null); verify(logger).trace("deleting {} [{}] from the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest(request); + verify(client).performRequestAsync(eq(request), any(ResponseListener.class)); verify(logger).error(any(org.apache.logging.log4j.util.Supplier.class), eq(e)); verifyNoMoreInteractions(client, logger); @@ -222,20 +230,24 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc public void testDoCheckAndPublishIgnoresPublishWhenCheckErrors() { final PublishableHttpResource resource = - new MockHttpResource(owner, masterTimeout, PublishableHttpResource.NO_BODY_PARAMETERS, CheckResponse.ERROR, true); + new MockHttpResource(owner, masterTimeout, PublishableHttpResource.NO_BODY_PARAMETERS, null, true); - assertThat(resource.doCheckAndPublish(client), is(false)); + resource.doCheckAndPublish(client, listener); + + verifyListener(null); } public void testDoCheckAndPublish() { // not an error (the third state) - final PublishableHttpResource.CheckResponse exists = randomBoolean() ? CheckResponse.EXISTS : CheckResponse.DOES_NOT_EXIST; + final boolean exists = randomBoolean(); final boolean publish = randomBoolean(); final PublishableHttpResource resource = new MockHttpResource(owner, masterTimeout, PublishableHttpResource.NO_BODY_PARAMETERS, exists, publish); - assertThat(resource.doCheckAndPublish(client), is(exists == CheckResponse.EXISTS || publish)); + resource.doCheckAndPublish(client, listener); + + verifyListener(exists || publish); } public void testShouldReplaceResourceRethrowsIOException() throws IOException { @@ -249,9 +261,9 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc expectThrows(IOException.class, () -> resource.shouldReplaceResource(response, xContent, resourceName, randomInt())); } - public void testShouldReplaceResourceThrowsExceptionForMalformedResponse() throws IOException { + public void testShouldReplaceResourceThrowsExceptionForMalformedResponse() { final Response response = mock(Response.class); - final HttpEntity entity = entityForResource(CheckResponse.ERROR, resourceName, randomInt()); + final HttpEntity entity = entityForResource(null, resourceName, randomInt()); final XContent xContent = XContentType.JSON.xContent(); when(response.getEntity()).thenReturn(entity); @@ -262,7 +274,7 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc public void testShouldReplaceResourceReturnsTrueVersionIsNotExpected() throws IOException { final int minimumVersion = randomInt(); final Response response = mock(Response.class); - final HttpEntity entity = entityForResource(CheckResponse.DOES_NOT_EXIST, resourceName, minimumVersion); + final HttpEntity entity = entityForResource(false, resourceName, minimumVersion); final XContent xContent = XContentType.JSON.xContent(); when(response.getEntity()).thenReturn(entity); @@ -287,21 +299,21 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc } @SuppressLoggerChecks(reason = "mock logger used") - private void assertCheckForResource(final RestStatus status, final CheckResponse expected, final String debugLogMessage) + private void assertCheckForResource(final RestStatus status, final Boolean expected, final String debugLogMessage) 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(request)).thenReturn(response); + whenPerformRequestAsyncWith(client, request, response); - sometimesAssertSimpleCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, expected, response); + assertCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, expected, response); verify(logger).trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest(request); + verify(client).performRequestAsync(eq(request), any(ResponseListener.class)); - if (expected == CheckResponse.EXISTS || expected == CheckResponse.DOES_NOT_EXIST) { + if (expected != null) { verify(response).getStatusLine(); } else { verify(response).getStatusLine(); @@ -316,64 +328,62 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc } @SuppressLoggerChecks(reason = "mock logger used") - private void assertVersionCheckForResource(final RestStatus status, final CheckResponse expected, + private void assertVersionCheckForResource(final RestStatus status, final Boolean expected, final int minimumVersion, - final String debugLogMessage) - throws IOException { - + final String debugLogMessage) { final String endpoint = concatenateEndpoint(resourceBasePath, resourceName); - final boolean shouldReplace = status == RestStatus.OK && expected == CheckResponse.DOES_NOT_EXIST; + final boolean shouldReplace = status == RestStatus.OK && expected == Boolean.FALSE; 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(request)).thenReturn(response); + whenPerformRequestAsyncWith(client, request, response); - assertThat(resource.versionCheckForResource(client, logger, - resourceBasePath, resourceName, resourceType, owner, ownerType, - xContent, minimumVersion), - is(expected)); + resource.versionCheckForResource(client, listener, logger, + resourceBasePath, resourceName, resourceType, owner, ownerType, + xContent, minimumVersion); verify(logger).trace("checking if {} [{}] exists on the [{}] {}", resourceType, resourceName, owner, ownerType); - verify(client).performRequest(request); + verify(client).performRequestAsync(eq(request), any(ResponseListener.class)); - if (shouldReplace || expected == CheckResponse.EXISTS) { + if (shouldReplace || expected == true) { verify(response).getStatusLine(); verify(response).getEntity(); - } else if (expected == CheckResponse.DOES_NOT_EXIST) { + } else if (expected == false) { verify(response).getStatusLine(); - } else { + } else { // expected == null verify(response).getStatusLine(); verify(response).getRequestLine(); verify(response).getHost(); verify(response).getEntity(); } + verifyListener(expected); verify(logger).debug(debugLogMessage, resourceType, resourceName, owner, ownerType); verifyNoMoreInteractions(client, response, logger); } - private void assertPutResource(final RestStatus status, final boolean expected) throws IOException { + private void assertPutResource(final RestStatus status, final boolean errorFree) { 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(request)).thenReturn(response); + whenPerformRequestAsyncWith(client, request, response); - assertThat(resource.putResource(client, logger, resourceBasePath, resourceName, body, resourceType, owner, ownerType), - is(expected)); + resource.putResource(client, listener, logger, resourceBasePath, resourceName, body, resourceType, owner, ownerType); - verify(client).performRequest(request); + verifyListener(errorFree ? true : null); + verify(client).performRequestAsync(eq(request), any(ResponseListener.class)); verify(response).getStatusLine(); verify(logger).trace("uploading {} [{}] to the [{}] {}", resourceType, resourceName, owner, ownerType); - if (expected) { + if (errorFree) { verify(logger).debug("{} [{}] uploaded to the [{}] {}", resourceType, resourceName, owner, ownerType); } else { ArgumentCaptor e = ArgumentCaptor.forClass(RuntimeException.class); @@ -387,42 +397,56 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc verifyNoMoreInteractions(client, response, logger, entity); } - private void sometimesAssertSimpleCheckForResource(final RestClient client, final Logger logger, - final String resourceBasePath, - final String resourceName, final String resourceType, - final CheckResponse expected, final Response response) { - // sometimes use the simple check - if (randomBoolean()) { - assertThat(resource.simpleCheckForResource(client, logger, resourceBasePath, resourceName, resourceType, owner, ownerType), - is(expected)); - } else { - final Tuple responseTuple = - resource.checkForResource(client, logger, resourceBasePath, resourceName, resourceType, owner, ownerType, - PublishableHttpResource.GET_EXISTS, PublishableHttpResource.GET_DOES_NOT_EXIST); + @SuppressWarnings("unchecked") + private void assertCheckForResource(final RestClient client, final Logger logger, + final String resourceBasePath, final String resourceName, final String resourceType, + final Boolean expected, final Response response) + throws IOException { + final CheckedFunction responseChecker = mock(CheckedFunction.class); + final CheckedFunction dneResponseChecker = mock(CheckedFunction.class); - assertThat(responseTuple.v1(), is(expected)); - assertThat(responseTuple.v2(), is(response)); + if (expected != null) { + // invert expected to keep the same value + when(responseChecker.apply(response)).thenReturn(false == expected); + when(dneResponseChecker.apply(response)).thenReturn(false == expected); } + + resource.checkForResource(client, listener, logger, resourceBasePath, resourceName, resourceType, owner, ownerType, + PublishableHttpResource.GET_EXISTS, PublishableHttpResource.GET_DOES_NOT_EXIST, + responseChecker, dneResponseChecker); + + if (expected == Boolean.TRUE) { + verify(responseChecker).apply(response); + verifyZeroInteractions(dneResponseChecker); + } else if (expected == Boolean.FALSE) { + verifyZeroInteractions(responseChecker); + verify(dneResponseChecker).apply(response); + } else { + verifyZeroInteractions(responseChecker, dneResponseChecker); + } + + verifyListener(expected); } - private void assertDeleteResource(final RestStatus status, final boolean expected) throws IOException { + private void assertDeleteResource(final RestStatus status, final boolean expected) { 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(request)).thenReturn(response); + whenPerformRequestAsyncWith(client, request, response); - assertThat(resource.deleteResource(client, logger, resourceBasePath, resourceName, resourceType, owner, ownerType), is(expected)); + resource.deleteResource(client, listener, logger, resourceBasePath, resourceName, resourceType, owner, ownerType); - verify(client).performRequest(request); + verify(client).performRequestAsync(eq(request), any(ResponseListener.class)); verify(response).getStatusLine(); verify(logger).trace("deleting {} [{}] from the [{}] {}", resourceType, resourceName, owner, ownerType); if (expected) { verify(logger).debug("{} [{}] deleted from the [{}] {}", resourceType, resourceName, owner, ownerType); + verifyListener(true); } else { ArgumentCaptor e = ArgumentCaptor.forClass(RuntimeException.class); @@ -430,6 +454,7 @@ public class PublishableHttpResourceTests extends AbstractPublishableHttpResourc assertThat(e.getValue().getMessage(), is("[" + resourceBasePath + "/" + resourceName + "] responded with [" + status.getStatus() + "]")); + verifyListener(null); } verifyNoMoreInteractions(client, response, logger, entity); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/TemplateHttpResourceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/TemplateHttpResourceTests.java index 0922e5e3d5c..4be1d2031ac 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/TemplateHttpResourceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/TemplateHttpResourceTests.java @@ -15,9 +15,6 @@ import java.io.IOException; import java.io.InputStream; import java.util.function.Supplier; -import static org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse.DOES_NOT_EXIST; -import static org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse.ERROR; -import static org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse.EXISTS; import static org.hamcrest.Matchers.is; /** @@ -49,45 +46,41 @@ public class TemplateHttpResourceTests extends AbstractPublishableHttpResourceTe assertThat(byteStream.available(), is(0)); } - public void testDoCheckExists() throws IOException { - final HttpEntity entity = entityForResource(EXISTS, templateName, minimumVersion); + public void testDoCheckExists() { + final HttpEntity entity = entityForResource(true, templateName, minimumVersion); - doCheckWithStatusCode(resource, "/_template", templateName, successfulCheckStatus(), EXISTS, entity); + doCheckWithStatusCode(resource, "/_template", templateName, successfulCheckStatus(), true, entity); } - public void testDoCheckDoesNotExist() throws IOException { + public void testDoCheckDoesNotExist() { if (randomBoolean()) { // it does not exist because it's literally not there assertCheckDoesNotExist(resource, "/_template", templateName); } else { // it does not exist because we need to replace it - final HttpEntity entity = entityForResource(DOES_NOT_EXIST, templateName, minimumVersion); + final HttpEntity entity = entityForResource(false, templateName, minimumVersion); - doCheckWithStatusCode(resource, "/_template", templateName, successfulCheckStatus(), DOES_NOT_EXIST, entity); + doCheckWithStatusCode(resource, "/_template", templateName, successfulCheckStatus(), false, entity); } } - public void testDoCheckError() throws IOException { + public void testDoCheckError() { if (randomBoolean()) { // error because of a server error assertCheckWithException(resource, "/_template", templateName); } else { // error because of a malformed response - final HttpEntity entity = entityForResource(ERROR, templateName, minimumVersion); + final HttpEntity entity = entityForResource(null, templateName, minimumVersion); - doCheckWithStatusCode(resource, "/_template", templateName, successfulCheckStatus(), ERROR, entity); + doCheckWithStatusCode(resource, "/_template", templateName, successfulCheckStatus(), null, entity); } } - public void testDoPublishTrue() throws IOException { + public void testDoPublishTrue() { assertPublishSucceeds(resource, "/_template", templateName, StringEntity.class); } - public void testDoPublishFalse() throws IOException { - assertPublishFails(resource, "/_template", templateName, StringEntity.class); - } - - public void testDoPublishFalseWithException() throws IOException { + public void testDoPublishFalseWithException() { assertPublishWithException(resource, "/_template", templateName, StringEntity.class); } 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 27de4b28cee..9fab23c7c40 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 @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.monitoring.exporter.http; import org.apache.http.entity.ContentType; import org.apache.http.nio.entity.NStringEntity; import org.elasticsearch.Version; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; @@ -16,6 +17,9 @@ import org.elasticsearch.test.VersionUtils; import java.io.IOException; +import static org.elasticsearch.xpack.monitoring.exporter.http.AsyncHttpResourceHelper.mockBooleanActionListener; +import static org.elasticsearch.xpack.monitoring.exporter.http.AsyncHttpResourceHelper.whenPerformRequestAsyncWith; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -27,76 +31,85 @@ public class VersionHttpResourceTests extends ESTestCase { private final String owner = getClass().getSimpleName(); private final RestClient client = mock(RestClient.class); + private final ActionListener listener = mockBooleanActionListener(); - public void testDoCheckAndPublishSuccess() throws IOException { + public void testDoCheckAndPublishSuccess() { final Version minimumVersion = VersionUtils.randomVersion(random()); final Version version = randomFrom(minimumVersion, Version.CURRENT); final Response response = responseForVersion(version); final VersionHttpResource resource = new VersionHttpResource(owner, minimumVersion); - assertTrue(resource.doCheckAndPublish(client)); + resource.doCheckAndPublish(client, listener); + verify(listener).onResponse(true); verify(response).getEntity(); } - public void testDoCheckAndPublishFailedParsing() throws IOException { + public void testDoCheckAndPublishFailedParsing() { // malformed JSON final Response response = responseForJSON("{"); final VersionHttpResource resource = new VersionHttpResource(owner, Version.CURRENT); - assertFalse(resource.doCheckAndPublish(client)); + resource.doCheckAndPublish(client, listener); + verify(listener).onFailure(any(Exception.class)); verify(response).getEntity(); } - public void testDoCheckAndPublishFailedFieldMissing() throws IOException { + public void testDoCheckAndPublishFailedFieldMissing() { // malformed response; imagining that we may change it in the future or someone breaks filter_path final Response response = responseForJSON("{\"version.number\":\"" + Version.CURRENT + "\"}"); final VersionHttpResource resource = new VersionHttpResource(owner, Version.CURRENT); - assertFalse(resource.doCheckAndPublish(client)); + resource.doCheckAndPublish(client, listener); + verify(listener).onFailure(any(Exception.class)); verify(response).getEntity(); } - public void testDoCheckAndPublishFailedFieldWrongType() throws IOException { + public void testDoCheckAndPublishFailedFieldWrongType() { // malformed response (should be {version: { number : ... }}) final Response response = responseForJSON("{\"version\":\"" + Version.CURRENT + "\"}"); final VersionHttpResource resource = new VersionHttpResource(owner, Version.CURRENT); - assertFalse(resource.doCheckAndPublish(client)); + resource.doCheckAndPublish(client, listener); + verify(listener).onFailure(any(Exception.class)); verify(response).getEntity(); } - public void testDoCheckAndPublishFailedWithIOException() throws IOException { - Request request = new Request("GET", "/"); + public void testDoCheckAndPublishFailedWithIOException() { + final Request request = new Request("GET", "/"); request.addParameter("filter_path", "version.number"); - when(client.performRequest(request)).thenThrow(new IOException("expected")); + + whenPerformRequestAsyncWith(client, request, new IOException("expected")); final VersionHttpResource resource = new VersionHttpResource(owner, Version.CURRENT); - assertFalse(resource.doCheckAndPublish(client)); + resource.doCheckAndPublish(client, listener); + + verify(listener).onFailure(any(Exception.class)); } - private Response responseForJSON(final String json) throws IOException { + private Response responseForJSON(final String json) { final NStringEntity entity = new NStringEntity(json, ContentType.APPLICATION_JSON); final Response response = mock(Response.class); when(response.getEntity()).thenReturn(entity); - Request request = new Request("GET", "/"); + final Request request = new Request("GET", "/"); request.addParameter("filter_path", "version.number"); - when(client.performRequest(request)).thenReturn(response); + + whenPerformRequestAsyncWith(client, request, response); return response; } - private Response responseForVersion(final Version version) throws IOException { + private Response responseForVersion(final Version version) { return responseForJSON("{\"version\":{\"number\":\"" + version + "\"}}"); } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/WatcherExistsHttpResourceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/WatcherExistsHttpResourceTests.java index ca70e26d769..1559ffb1c68 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/WatcherExistsHttpResourceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/WatcherExistsHttpResourceTests.java @@ -14,9 +14,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.rest.RestStatus; -import org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.CheckResponse; -import java.io.IOException; import java.util.Map; import static org.elasticsearch.xpack.monitoring.exporter.http.PublishableHttpResource.GET_EXISTS; @@ -42,28 +40,29 @@ public class WatcherExistsHttpResourceTests extends AbstractPublishableHttpResou public void testDoCheckIgnoresClientWhenNotElectedMaster() { whenNotElectedMaster(); - assertThat(resource.doCheck(client), is(CheckResponse.EXISTS)); + resource.doCheck(client, listener); + verify(listener).onResponse(true); verifyZeroInteractions(client); } - public void testDoCheckExistsFor404() throws IOException { + public void testDoCheckExistsFor404() { whenElectedMaster(); // /_xpack returning a 404 means ES didn't handle the request properly and X-Pack doesn't exist doCheckWithStatusCode(resource, "", "_xpack", notFoundCheckStatus(), - GET_EXISTS, XPACK_DOES_NOT_EXIST, CheckResponse.EXISTS); + GET_EXISTS, XPACK_DOES_NOT_EXIST, true); } - public void testDoCheckExistsFor400() throws IOException { + public void testDoCheckExistsFor400() { whenElectedMaster(); // /_xpack returning a 400 means X-Pack does not exist doCheckWithStatusCode(resource, "", "_xpack", RestStatus.BAD_REQUEST, - GET_EXISTS, XPACK_DOES_NOT_EXIST, CheckResponse.EXISTS); + GET_EXISTS, XPACK_DOES_NOT_EXIST, true); } - public void testDoCheckExistsAsElectedMaster() throws IOException { + public void testDoCheckExistsAsElectedMaster() { whenElectedMaster(); final String[] noWatcher = { @@ -82,12 +81,12 @@ public class WatcherExistsHttpResourceTests extends AbstractPublishableHttpResou when(response.getEntity()).thenReturn(responseEntity); // returning EXISTS implies that we CANNOT use Watcher to avoid running the publish phase - doCheckWithStatusCode(resource, expectedParameters, endpoint, CheckResponse.EXISTS, response); + doCheckWithStatusCode(resource, expectedParameters, endpoint, true, response); verify(response).getEntity(); } - public void testDoCheckDoesNotExist() throws IOException { + public void testDoCheckDoesNotExist() { whenElectedMaster(); final String[] hasWatcher = { @@ -103,12 +102,12 @@ public class WatcherExistsHttpResourceTests extends AbstractPublishableHttpResou when(response.getEntity()).thenReturn(responseEntity); // returning DOES_NOT_EXIST implies that we CAN use Watcher and need to run the publish phase - doCheckWithStatusCode(resource, expectedParameters, endpoint, CheckResponse.DOES_NOT_EXIST, response); + doCheckWithStatusCode(resource, expectedParameters, endpoint, false, response); verify(response).getEntity(); } - public void testDoCheckErrorWithDataException() throws IOException { + public void testDoCheckErrorWithDataException() { whenElectedMaster(); final String[] errorWatcher = { @@ -124,39 +123,55 @@ public class WatcherExistsHttpResourceTests extends AbstractPublishableHttpResou when(response.getEntity()).thenReturn(responseEntity); - // returning DOES_NOT_EXIST implies that we CAN use Watcher and need to run the publish phase - doCheckWithStatusCode(resource, endpoint, CheckResponse.ERROR, response); + // returning an error implies that we CAN use Watcher and need to run the publish phase + doCheckWithStatusCode(resource, expectedParameters, endpoint, null, response); } - public void testDoCheckErrorWithResponseException() throws IOException { + public void testDoCheckErrorWithResponseException() { whenElectedMaster(); - assertCheckWithException(resource, "", "_xpack"); + assertCheckWithException(resource, expectedParameters, "", "_xpack"); } - public void testDoPublishTrue() throws IOException { - final CheckResponse checkResponse = randomFrom(CheckResponse.EXISTS, CheckResponse.DOES_NOT_EXIST); - final boolean publish = checkResponse == CheckResponse.DOES_NOT_EXIST; + public void testDoPublishTrue() { + final boolean checkResponse = randomBoolean(); + final boolean publish = checkResponse == false; final MockHttpResource mockWatch = new MockHttpResource(owner, randomBoolean(), checkResponse, publish); final MultiHttpResource watches = new MultiHttpResource(owner, Collections.singletonList(mockWatch)); final WatcherExistsHttpResource resource = new WatcherExistsHttpResource(owner, clusterService, watches); - assertTrue(resource.doPublish(client)); + resource.doPublish(client, listener); + + verifyListener(true); assertThat(mockWatch.checked, is(1)); assertThat(mockWatch.published, is(publish ? 1 : 0)); } - public void testDoPublishFalse() throws IOException { - final CheckResponse checkResponse = randomFrom(CheckResponse.DOES_NOT_EXIST, CheckResponse.ERROR); - final MockHttpResource mockWatch = new MockHttpResource(owner, true, checkResponse, false); + public void testDoPublishFalse() { + final MockHttpResource mockWatch = new MockHttpResource(owner, true, false, false); final MultiHttpResource watches = new MultiHttpResource(owner, Collections.singletonList(mockWatch)); final WatcherExistsHttpResource resource = new WatcherExistsHttpResource(owner, clusterService, watches); - assertFalse(resource.doPublish(client)); + resource.doPublish(client, listener); + + verifyListener(false); assertThat(mockWatch.checked, is(1)); - assertThat(mockWatch.published, is(checkResponse == CheckResponse.DOES_NOT_EXIST ? 1 : 0)); + assertThat(mockWatch.published, is(1)); + } + + public void testDoPublishException() { + final MockHttpResource mockWatch = new MockHttpResource(owner, true, false, null); + final MultiHttpResource watches = new MultiHttpResource(owner, Collections.singletonList(mockWatch)); + final WatcherExistsHttpResource resource = new WatcherExistsHttpResource(owner, clusterService, watches); + + resource.doPublish(client, listener); + + verifyListener(null); + + assertThat(mockWatch.checked, is(1)); + assertThat(mockWatch.published, is(1)); } public void testParameters() {