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() {