diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequest.java index 7cac3f0c234..06f8d955915 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequest.java @@ -26,7 +26,10 @@ import org.elasticsearch.common.io.stream.StreamOutput; import java.io.IOException; import java.util.Arrays; +import java.util.HashSet; import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; import java.util.stream.Collectors; /** @@ -34,7 +37,7 @@ import java.util.stream.Collectors; */ public class NodesInfoRequest extends BaseNodesRequest { - private Set requestedMetrics = Metrics.allMetrics(); + private Set requestedMetrics = Metric.allMetrics(); /** * Create a new NodeInfoRequest from a {@link StreamInput} object. @@ -48,16 +51,16 @@ public class NodesInfoRequest extends BaseNodesRequest { if (in.getVersion().before(Version.V_7_7_0)){ // prior to version 8.x, a NodesInfoRequest was serialized as a list // of booleans in a fixed order - addOrRemoveMetric(in.readBoolean(), Metrics.SETTINGS.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.OS.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.PROCESS.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.JVM.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.THREAD_POOL.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.TRANSPORT.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.HTTP.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.PLUGINS.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.INGEST.metricName()); - addOrRemoveMetric(in.readBoolean(), Metrics.INDICES.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.SETTINGS.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.OS.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.PROCESS.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.JVM.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.THREAD_POOL.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.TRANSPORT.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.HTTP.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.PLUGINS.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.INGEST.metricName()); + optionallyAddMetric(in.readBoolean(), Metric.INDICES.metricName()); } else { requestedMetrics.addAll(Arrays.asList(in.readStringArray())); } @@ -84,174 +87,63 @@ public class NodesInfoRequest extends BaseNodesRequest { * Sets to return all the data. */ public NodesInfoRequest all() { - requestedMetrics.addAll(Metrics.allMetrics()); + requestedMetrics.addAll(Metric.allMetrics()); return this; } /** - * Should the node settings be returned. + * Get the names of requested metrics */ - public boolean settings() { - return Metrics.SETTINGS.containedIn(requestedMetrics); + public Set requestedMetrics() { + return new HashSet<>(requestedMetrics); } /** - * Should the node settings be returned. + * Add metric */ - public NodesInfoRequest settings(boolean settings) { - addOrRemoveMetric(settings, Metrics.SETTINGS.metricName()); + public NodesInfoRequest addMetric(String metric) { + if (Metric.allMetrics().contains(metric) == false) { + throw new IllegalStateException("Used an illegal metric: " + metric); + } + requestedMetrics.add(metric); return this; } /** - * Should the node OS be returned. + * Add multiple metrics */ - public boolean os() { - return Metrics.OS.containedIn(requestedMetrics); - } - - /** - * Should the node OS be returned. - */ - public NodesInfoRequest os(boolean os) { - addOrRemoveMetric(os, Metrics.OS.metricName()); + public NodesInfoRequest addMetrics(String... metrics) { + SortedSet metricsSet = new TreeSet<>(Arrays.asList(metrics)); + if (Metric.allMetrics().containsAll(metricsSet) == false) { + metricsSet.removeAll(Metric.allMetrics()); + String plural = metricsSet.size() == 1 ? "" : "s"; + throw new IllegalStateException("Used illegal metric" + plural + ": " + metricsSet); + } + requestedMetrics.addAll(metricsSet); return this; } /** - * Should the node Process be returned. + * Remove metric */ - public boolean process() { - return Metrics.PROCESS.containedIn(requestedMetrics); - } - - /** - * Should the node Process be returned. - */ - public NodesInfoRequest process(boolean process) { - addOrRemoveMetric(process, Metrics.PROCESS.metricName()); + public NodesInfoRequest removeMetric(String metric) { + if (Metric.allMetrics().contains(metric) == false) { + throw new IllegalStateException("Used an illegal metric: " + metric); + } + requestedMetrics.remove(metric); return this; } /** - * Should the node JVM be returned. - */ - public boolean jvm() { - return Metrics.JVM.containedIn(requestedMetrics); - } - - /** - * Should the node JVM be returned. - */ - public NodesInfoRequest jvm(boolean jvm) { - addOrRemoveMetric(jvm, Metrics.JVM.metricName()); - return this; - } - - /** - * Should the node Thread Pool info be returned. - */ - public boolean threadPool() { - return Metrics.THREAD_POOL.containedIn(requestedMetrics); - } - - /** - * Should the node Thread Pool info be returned. - */ - public NodesInfoRequest threadPool(boolean threadPool) { - addOrRemoveMetric(threadPool, Metrics.THREAD_POOL.metricName()); - return this; - } - - /** - * Should the node Transport be returned. - */ - public boolean transport() { - return Metrics.TRANSPORT.containedIn(requestedMetrics); - } - - /** - * Should the node Transport be returned. - */ - public NodesInfoRequest transport(boolean transport) { - addOrRemoveMetric(transport, Metrics.TRANSPORT.metricName()); - return this; - } - - /** - * Should the node HTTP be returned. - */ - public boolean http() { - return Metrics.HTTP.containedIn(requestedMetrics); - } - - /** - * Should the node HTTP be returned. - */ - public NodesInfoRequest http(boolean http) { - addOrRemoveMetric(http, Metrics.HTTP.metricName()); - return this; - } - - /** - * Should information about plugins be returned - * @param plugins true if you want info - * @return The request - */ - public NodesInfoRequest plugins(boolean plugins) { - addOrRemoveMetric(plugins, Metrics.PLUGINS.metricName()); - return this; - } - - /** - * @return true if information about plugins is requested - */ - public boolean plugins() { - return Metrics.PLUGINS.containedIn(requestedMetrics); - } - - /** - * Should information about ingest be returned - * @param ingest true if you want info - */ - public NodesInfoRequest ingest(boolean ingest) { - addOrRemoveMetric(ingest, Metrics.INGEST.metricName()); - return this; - } - - /** - * @return true if information about ingest is requested - */ - public boolean ingest() { - return Metrics.INGEST.containedIn(requestedMetrics); - } - - /** - * Should information about indices (currently just indexing buffers) be returned - * @param indices true if you want info - */ - public NodesInfoRequest indices(boolean indices) { - addOrRemoveMetric(indices, Metrics.INDICES.metricName()); - return this; - } - - /** - * @return true if information about indices (currently just indexing buffers) - */ - public boolean indices() { - return Metrics.INDICES.containedIn(requestedMetrics); - } - - /** - * Helper method for adding and removing metrics. - * @param includeMetric Whether or not to include a metric. + * Helper method for adding and removing metrics. Used when deserializing + * a NodesInfoRequest from an ordered list of booleans. + * + * @param addMetric Whether or not to include a metric. * @param metricName Name of the metric to include or remove. */ - private void addOrRemoveMetric(boolean includeMetric, String metricName) { - if (includeMetric) { + private void optionallyAddMetric(boolean addMetric, String metricName) { + if (addMetric) { requestedMetrics.add(metricName); - } else { - requestedMetrics.remove(metricName); } } @@ -261,16 +153,16 @@ public class NodesInfoRequest extends BaseNodesRequest { if (out.getVersion().before(Version.V_7_7_0)){ // prior to version 8.x, a NodesInfoRequest was serialized as a list // of booleans in a fixed order - out.writeBoolean(Metrics.SETTINGS.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.OS.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.PROCESS.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.JVM.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.THREAD_POOL.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.TRANSPORT.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.HTTP.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.PLUGINS.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.INGEST.containedIn(requestedMetrics)); - out.writeBoolean(Metrics.INDICES.containedIn(requestedMetrics)); + out.writeBoolean(Metric.SETTINGS.containedIn(requestedMetrics)); + out.writeBoolean(Metric.OS.containedIn(requestedMetrics)); + out.writeBoolean(Metric.PROCESS.containedIn(requestedMetrics)); + out.writeBoolean(Metric.JVM.containedIn(requestedMetrics)); + out.writeBoolean(Metric.THREAD_POOL.containedIn(requestedMetrics)); + out.writeBoolean(Metric.TRANSPORT.containedIn(requestedMetrics)); + out.writeBoolean(Metric.HTTP.containedIn(requestedMetrics)); + out.writeBoolean(Metric.PLUGINS.containedIn(requestedMetrics)); + out.writeBoolean(Metric.INGEST.containedIn(requestedMetrics)); + out.writeBoolean(Metric.INDICES.containedIn(requestedMetrics)); } else { out.writeStringArray(requestedMetrics.toArray(new String[0])); } @@ -281,7 +173,7 @@ public class NodesInfoRequest extends BaseNodesRequest { * from the nodes information endpoint. Eventually this list list will be * pluggable. */ - enum Metrics { + public enum Metric { SETTINGS("settings"), OS("os"), PROCESS("process"), @@ -295,11 +187,11 @@ public class NodesInfoRequest extends BaseNodesRequest { private String metricName; - Metrics(String name) { + Metric(String name) { this.metricName = name; } - String metricName() { + public String metricName() { return this.metricName; } @@ -307,8 +199,8 @@ public class NodesInfoRequest extends BaseNodesRequest { return metricNames.contains(this.metricName()); } - static Set allMetrics() { - return Arrays.stream(values()).map(Metrics::metricName).collect(Collectors.toSet()); + public static Set allMetrics() { + return Arrays.stream(values()).map(Metric::metricName).collect(Collectors.toSet()); } } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestBuilder.java index 12221016160..97b55b7fd49 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestBuilder.java @@ -22,6 +22,7 @@ package org.elasticsearch.action.admin.cluster.node.info; import org.elasticsearch.action.support.nodes.NodesOperationRequestBuilder; import org.elasticsearch.client.ElasticsearchClient; +// TODO: This class's interface should match that of NodesInfoRequest public class NodesInfoRequestBuilder extends NodesOperationRequestBuilder { public NodesInfoRequestBuilder(ElasticsearchClient client, NodesInfoAction action) { @@ -48,7 +49,7 @@ public class NodesInfoRequestBuilder extends NodesOperationRequestBuilder metrics = request.requestedMetrics(); + return nodeService.info( + metrics.contains(NodesInfoRequest.Metric.SETTINGS.metricName()), + metrics.contains(NodesInfoRequest.Metric.OS.metricName()), + metrics.contains(NodesInfoRequest.Metric.PROCESS.metricName()), + metrics.contains(NodesInfoRequest.Metric.JVM.metricName()), + metrics.contains(NodesInfoRequest.Metric.THREAD_POOL.metricName()), + metrics.contains(NodesInfoRequest.Metric.TRANSPORT.metricName()), + metrics.contains(NodesInfoRequest.Metric.HTTP.metricName()), + metrics.contains(NodesInfoRequest.Metric.PLUGINS.metricName()), + metrics.contains(NodesInfoRequest.Metric.INGEST.metricName()), + metrics.contains(NodesInfoRequest.Metric.INDICES.metricName())); } public static class NodeInfoRequest extends BaseNodeRequest { diff --git a/server/src/main/java/org/elasticsearch/action/ingest/PutPipelineTransportAction.java b/server/src/main/java/org/elasticsearch/action/ingest/PutPipelineTransportAction.java index bc0937d667a..9b70b2e0194 100644 --- a/server/src/main/java/org/elasticsearch/action/ingest/PutPipelineTransportAction.java +++ b/server/src/main/java/org/elasticsearch/action/ingest/PutPipelineTransportAction.java @@ -73,8 +73,8 @@ public class PutPipelineTransportAction extends TransportMasterNodeAction listener) throws Exception { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear(); - nodesInfoRequest.ingest(true); + nodesInfoRequest.clear() + .addMetric(NodesInfoRequest.Metric.INGEST.metricName()); client.admin().cluster().nodesInfo(nodesInfoRequest, ActionListener.wrap(nodeInfos -> { Map ingestInfos = new HashMap<>(); for (NodeInfo nodeInfo : nodeInfos.getNodes()) { diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoAction.java index 4b42516ef06..ec23d722462 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoAction.java @@ -38,17 +38,7 @@ import static java.util.Collections.unmodifiableList; import static org.elasticsearch.rest.RestRequest.Method.GET; public class RestNodesInfoAction extends BaseRestHandler { - static final Set ALLOWED_METRICS = Sets.newHashSet( - "http", - "ingest", - "indices", - "jvm", - "os", - "plugins", - "process", - "settings", - "thread_pool", - "transport"); + static final Set ALLOWED_METRICS = NodesInfoRequest.Metric.allMetrics(); private final SettingsFilter settingsFilter; @@ -110,16 +100,9 @@ public class RestNodesInfoAction extends BaseRestHandler { nodesInfoRequest.all(); } else { nodesInfoRequest.clear(); - nodesInfoRequest.settings(metrics.contains("settings")); - nodesInfoRequest.os(metrics.contains("os")); - nodesInfoRequest.process(metrics.contains("process")); - nodesInfoRequest.jvm(metrics.contains("jvm")); - nodesInfoRequest.threadPool(metrics.contains("thread_pool")); - nodesInfoRequest.transport(metrics.contains("transport")); - nodesInfoRequest.http(metrics.contains("http")); - nodesInfoRequest.plugins(metrics.contains("plugins")); - nodesInfoRequest.ingest(metrics.contains("ingest")); - nodesInfoRequest.indices(metrics.contains("indices")); + // disregard unknown metrics + metrics.retainAll(ALLOWED_METRICS); + nodesInfoRequest.addMetrics(metrics.stream().toArray(String[]::new)); } return nodesInfoRequest; } diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodeAttrsAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodeAttrsAction.java index dcb75b22e64..5231baacb6f 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodeAttrsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodeAttrsAction.java @@ -68,7 +68,8 @@ public class RestNodeAttrsAction extends AbstractCatAction { @Override public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear().jvm(false).os(false).process(true); + nodesInfoRequest.clear() + .addMetric(NodesInfoRequest.Metric.PROCESS.metricName()); client.admin().cluster().nodesInfo(nodesInfoRequest, new RestResponseListener(channel) { @Override public RestResponse buildResponse(NodesInfoResponse nodesInfoResponse) throws Exception { diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java index 7f16476d1c1..dde182d0df7 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java @@ -104,7 +104,11 @@ public class RestNodesAction extends AbstractCatAction { @Override public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear().jvm(true).os(true).process(true).http(true); + nodesInfoRequest.clear().addMetrics( + NodesInfoRequest.Metric.JVM.metricName(), + NodesInfoRequest.Metric.OS.metricName(), + NodesInfoRequest.Metric.PROCESS.metricName(), + NodesInfoRequest.Metric.HTTP.metricName()); client.admin().cluster().nodesInfo(nodesInfoRequest, new RestActionListener(channel) { @Override public void processResponse(final NodesInfoResponse nodesInfoResponse) { diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestPluginsAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestPluginsAction.java index 9eab96a9a6f..f3c71321b9c 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestPluginsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestPluginsAction.java @@ -68,7 +68,8 @@ public class RestPluginsAction extends AbstractCatAction { @Override public void processResponse(final ClusterStateResponse clusterStateResponse) throws Exception { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear().plugins(true); + nodesInfoRequest.clear() + .addMetric(NodesInfoRequest.Metric.PLUGINS.metricName()); client.admin().cluster().nodesInfo(nodesInfoRequest, new RestResponseListener(channel) { @Override public RestResponse buildResponse(final NodesInfoResponse nodesInfoResponse) throws Exception { diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java index caaaeba07b9..368ec15e58c 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java @@ -82,7 +82,9 @@ public class RestThreadPoolAction extends AbstractCatAction { @Override public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); - nodesInfoRequest.clear().process(true).threadPool(true); + nodesInfoRequest.clear().addMetrics( + NodesInfoRequest.Metric.PROCESS.metricName(), + NodesInfoRequest.Metric.THREAD_POOL.metricName()); client.admin().cluster().nodesInfo(nodesInfoRequest, new RestActionListener(channel) { @Override public void processResponse(final NodesInfoResponse nodesInfoResponse) { diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestTests.java index 7e944541024..0817ffc2f8a 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequestTests.java @@ -23,7 +23,13 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.test.ESTestCase; +import java.util.HashSet; +import java.util.Set; + +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.in; +import static org.hamcrest.Matchers.not; /** * Granular tests for the {@link NodesInfoRequest} class. Higher-level tests @@ -34,82 +40,94 @@ public class NodesInfoRequestTests extends ESTestCase { /** * Make sure that we can set, serialize, and deserialize arbitrary sets * of metrics. - * - * TODO: Once we can set values by string, use a collection rather than - * checking each and every setter in the public API */ - public void testMetricsSetters() throws Exception { + public void testAddMetricsSet() throws Exception { NodesInfoRequest request = new NodesInfoRequest(randomAlphaOfLength(8)); - request.settings(randomBoolean()); - request.os(randomBoolean()); - request.process(randomBoolean()); - request.jvm(randomBoolean()); - request.threadPool(randomBoolean()); - request.transport(randomBoolean()); - request.http(randomBoolean()); - request.plugins(randomBoolean()); - request.ingest(randomBoolean()); - request.indices(randomBoolean()); + randomSubsetOf(NodesInfoRequest.Metric.allMetrics()).forEach(request::addMetric); NodesInfoRequest deserializedRequest = roundTripRequest(request); - assertRequestsEqual(request, deserializedRequest); + assertThat(request.requestedMetrics(), equalTo(deserializedRequest.requestedMetrics())); + } + + /** + * Check that we can add a metric. + */ + public void testAddSingleMetric() throws Exception { + NodesInfoRequest request = new NodesInfoRequest(randomAlphaOfLength(8)); + request.addMetric(randomFrom(NodesInfoRequest.Metric.allMetrics())); + NodesInfoRequest deserializedRequest = roundTripRequest(request); + assertThat(request.requestedMetrics(), equalTo(deserializedRequest.requestedMetrics())); + } + + /** + * Check that we can remove a metric. + */ + public void testRemoveSingleMetric() throws Exception { + NodesInfoRequest request = new NodesInfoRequest(randomAlphaOfLength(8)); + request.all(); + String metric = randomFrom(NodesInfoRequest.Metric.allMetrics()); + request.removeMetric(metric); + + NodesInfoRequest deserializedRequest = roundTripRequest(request); + assertThat(request.requestedMetrics(), equalTo(deserializedRequest.requestedMetrics())); + assertThat(metric, not(in(request.requestedMetrics()))); } /** * Test that a newly constructed NodesInfoRequestObject requests all of the - * possible metrics defined in {@link NodesInfoRequest.Metrics}. + * possible metrics defined in {@link NodesInfoRequest.Metric}. */ public void testNodesInfoRequestDefaults() { NodesInfoRequest defaultNodesInfoRequest = new NodesInfoRequest(randomAlphaOfLength(8)); NodesInfoRequest allMetricsNodesInfoRequest = new NodesInfoRequest(randomAlphaOfLength(8)); allMetricsNodesInfoRequest.all(); - assertRequestsEqual(defaultNodesInfoRequest, allMetricsNodesInfoRequest); + assertThat(defaultNodesInfoRequest.requestedMetrics(), equalTo(allMetricsNodesInfoRequest.requestedMetrics())); } /** - * Test that the {@link NodesInfoRequest#all()} method sets all of the - * metrics to {@code true}. - * - * TODO: Once we can check values by string, use a collection rather than - * checking each and every getter in the public API + * Test that the {@link NodesInfoRequest#all()} method enables all metrics. */ public void testNodesInfoRequestAll() throws Exception { NodesInfoRequest request = new NodesInfoRequest("node"); request.all(); - assertTrue(request.settings()); - assertTrue(request.os()); - assertTrue(request.process()); - assertTrue(request.jvm()); - assertTrue(request.threadPool()); - assertTrue(request.transport()); - assertTrue(request.http()); - assertTrue(request.plugins()); - assertTrue(request.ingest()); - assertTrue(request.indices()); + assertThat(request.requestedMetrics(), equalTo(NodesInfoRequest.Metric.allMetrics())); } /** - * Test that the {@link NodesInfoRequest#clear()} method sets all of the - * metrics to {@code false}. - * - * TODO: Once we can check values by string, use a collection rather than - * checking each and every getter in the public API + * Test that the {@link NodesInfoRequest#clear()} method disables all metrics. */ public void testNodesInfoRequestClear() throws Exception { NodesInfoRequest request = new NodesInfoRequest("node"); request.clear(); - assertFalse(request.settings()); - assertFalse(request.os()); - assertFalse(request.process()); - assertFalse(request.jvm()); - assertFalse(request.threadPool()); - assertFalse(request.transport()); - assertFalse(request.http()); - assertFalse(request.plugins()); - assertFalse(request.ingest()); - assertFalse(request.indices()); + assertThat(request.requestedMetrics(), empty()); + } + + /** + * Test that (for now) we can only add metrics from a set of known metrics. + */ + public void testUnknownMetricsRejected() { + String unknownMetric1 = "unknown_metric1"; + String unknownMetric2 = "unknown_metric2"; + Set unknownMetrics = new HashSet<>(); + unknownMetrics.add(unknownMetric1); + unknownMetrics.addAll(randomSubsetOf(NodesInfoRequest.Metric.allMetrics())); + + NodesInfoRequest request = new NodesInfoRequest(); + + IllegalStateException exception = expectThrows(IllegalStateException.class, () -> request.addMetric(unknownMetric1)); + assertThat(exception.getMessage(), equalTo("Used an illegal metric: " + unknownMetric1)); + + exception = expectThrows(IllegalStateException.class, () -> request.removeMetric(unknownMetric1)); + assertThat(exception.getMessage(), equalTo("Used an illegal metric: " + unknownMetric1)); + + exception = expectThrows(IllegalStateException.class, () -> request.addMetrics(unknownMetrics.stream().toArray(String[]::new))); + assertThat(exception.getMessage(), equalTo("Used illegal metric: [" + unknownMetric1 + "]")); + + unknownMetrics.add(unknownMetric2); + exception = expectThrows(IllegalStateException.class, () -> request.addMetrics(unknownMetrics.stream().toArray(String[]::new))); + assertThat(exception.getMessage(), equalTo("Used illegal metrics: [" + unknownMetric1 + ", " + unknownMetric2 + "]")); } /** @@ -125,20 +143,4 @@ public class NodesInfoRequestTests extends ESTestCase { } } } - - private static void assertRequestsEqual(NodesInfoRequest request1, NodesInfoRequest request2) { - - // TODO: Once we can check values by string, use a collection rather than - // checking each and every getter in the public API - assertThat(request1.settings(), equalTo(request2.settings())); - assertThat(request1.os(), equalTo(request2.os())); - assertThat(request1.process(), equalTo(request2.process())); - assertThat(request1.jvm(), equalTo(request2.jvm())); - assertThat(request1.threadPool(), equalTo(request2.threadPool())); - assertThat(request1.transport(), equalTo(request2.transport())); - assertThat(request1.http(), equalTo(request2.http())); - assertThat(request1.plugins(), equalTo(request2.plugins())); - assertThat(request1.ingest(), equalTo(request2.ingest())); - assertThat(request1.indices(), equalTo(request2.indices())); - } } diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoActionTests.java index d757ee095cd..456b371b0eb 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestNodesInfoActionTests.java @@ -20,71 +20,77 @@ package org.elasticsearch.rest.action.admin.cluster; import org.elasticsearch.action.admin.cluster.node.info.NodesInfoRequest; +import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static org.elasticsearch.rest.action.admin.cluster.RestNodesInfoAction.ALLOWED_METRICS; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.not; public class RestNodesInfoActionTests extends ESTestCase { public void testDuplicatedFiltersAreNotRemoved() { Map params = new HashMap<>(); params.put("nodeId", "_all,master:false,_all"); - + RestRequest restRequest = buildRestRequest(params); NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest); assertArrayEquals(new String[] { "_all", "master:false", "_all" }, actual.nodesIds()); } - + public void testOnlyMetrics() { Map params = new HashMap<>(); int metricsCount = randomIntBetween(1, ALLOWED_METRICS.size()); List metrics = new ArrayList<>(); - + for(int i = 0; i < metricsCount; i++) { metrics.add(randomFrom(ALLOWED_METRICS)); } params.put("nodeId", String.join(",", metrics)); - + RestRequest restRequest = buildRestRequest(params); NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest); assertArrayEquals(new String[] { "_all" }, actual.nodesIds()); assertMetrics(metrics, actual); } - + public void testAllMetricsSelectedWhenNodeAndMetricSpecified() { Map params = new HashMap<>(); String nodeId = randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(23)); String metric = randomFrom(ALLOWED_METRICS); - + params.put("nodeId", nodeId + "," + metric); RestRequest restRequest = buildRestRequest(params); - + NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest); assertArrayEquals(new String[] { nodeId, metric }, actual.nodesIds()); assertAllMetricsTrue(actual); } - + public void testSeparateNodeIdsAndMetrics() { Map params = new HashMap<>(); List nodeIds = new ArrayList<>(5); List metrics = new ArrayList<>(5); - + for(int i = 0; i < 5; i++) { nodeIds.add(randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(23))); metrics.add(randomFrom(ALLOWED_METRICS)); } - + params.put("nodeId", String.join(",", nodeIds)); params.put("metrics", String.join(",", metrics)); RestRequest restRequest = buildRestRequest(params); - + NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest); assertArrayEquals(nodeIds.toArray(), actual.nodesIds()); assertMetrics(metrics, actual); @@ -97,16 +103,42 @@ public class RestNodesInfoActionTests extends ESTestCase { for(int i = 0; i < 5; i++) { nodeIds.add(randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(23))); } - + params.put("nodeId", String.join(",", nodeIds)); params.put("metrics", "_all"); RestRequest restRequest = buildRestRequest(params); - + NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest); assertArrayEquals(nodeIds.toArray(), actual.nodesIds()); assertAllMetricsTrue(actual); } + /** + * Test that if a user requests a non-existent metric, it's dropped from the + * request without an error. + */ + public void testNonexistentMetricsDropped() { + Map params = new HashMap<>(); + List nodeIds = new ArrayList<>(5); + List metrics = new ArrayList<>(5); + + for(int i = 0; i < 5; i++) { + nodeIds.add(randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(23))); + metrics.add(randomFrom(ALLOWED_METRICS)); + } + String nonAllowedMetric = randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(5)); + metrics.add(nonAllowedMetric); + + params.put("nodeId", String.join(",", nodeIds)); + params.put("metrics", String.join(",", metrics)); + RestRequest restRequest = buildRestRequest(params); + + NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest); + assertArrayEquals(nodeIds.toArray(), actual.nodesIds()); + assertThat(actual.requestedMetrics(), not(hasItem(nonAllowedMetric))); + assertMetrics(metrics, actual); + } + private FakeRestRequest buildRestRequest(Map params) { return new FakeRestRequest.Builder(xContentRegistry()) .withMethod(RestRequest.Method.GET) @@ -114,30 +146,13 @@ public class RestNodesInfoActionTests extends ESTestCase { .withParams(params) .build(); } - + private void assertMetrics(List metrics, NodesInfoRequest nodesInfoRequest) { - assertTrue((metrics.contains("http") && nodesInfoRequest.http()) || metrics.contains("http") == false); - assertTrue((metrics.contains("ingest") && nodesInfoRequest.ingest()) || metrics.contains("ingest") == false); - assertTrue((metrics.contains("indices") && nodesInfoRequest.indices()) || metrics.contains("indices") == false); - assertTrue((metrics.contains("jvm") && nodesInfoRequest.jvm()) || metrics.contains("jvm") == false); - assertTrue((metrics.contains("os") && nodesInfoRequest.os()) || metrics.contains("os") == false); - assertTrue((metrics.contains("plugins") && nodesInfoRequest.plugins()) || metrics.contains("plugins") == false); - assertTrue((metrics.contains("process") && nodesInfoRequest.process()) || metrics.contains("process") == false); - assertTrue((metrics.contains("settings") && nodesInfoRequest.settings()) || metrics.contains("settings") == false); - assertTrue((metrics.contains("thread_pool") && nodesInfoRequest.threadPool()) || metrics.contains("thread_pool") == false); - assertTrue((metrics.contains("transport") && nodesInfoRequest.transport()) || metrics.contains("transport") == false); + Set validRequestedMetrics = Sets.intersection(new HashSet<>(metrics), ALLOWED_METRICS); + assertThat(nodesInfoRequest.requestedMetrics(), equalTo(validRequestedMetrics)); } - + private void assertAllMetricsTrue(NodesInfoRequest nodesInfoRequest) { - assertTrue(nodesInfoRequest.http()); - assertTrue(nodesInfoRequest.ingest()); - assertTrue(nodesInfoRequest.indices()); - assertTrue(nodesInfoRequest.jvm()); - assertTrue(nodesInfoRequest.os()); - assertTrue(nodesInfoRequest.plugins()); - assertTrue(nodesInfoRequest.process()); - assertTrue(nodesInfoRequest.settings()); - assertTrue(nodesInfoRequest.threadPool()); - assertTrue(nodesInfoRequest.transport()); + assertThat(nodesInfoRequest.requestedMetrics(), equalTo(ALLOWED_METRICS)); } }