From c43a734f8f9beba493d7d0adcb0122f996d218f9 Mon Sep 17 00:00:00 2001 From: Russell Gold Date: Wed, 4 Jan 2023 20:21:47 -0500 Subject: [PATCH 1/2] Improve filtering: ignore subtrees with no matches, exclude forbidden fields --- .../oracle/wls/buildhelper/GitTagMojo.java | 16 +- .../wls/buildhelper/GitTagMojoTest.java | 20 +- .../com/oracle/wls/exporter/ExporterCall.java | 19 +- .../oracle/wls/exporter/WebClientCommon.java | 3 +- .../wls/exporter/domain/ExporterConfig.java | 8 +- .../wls/exporter/domain/JsonQuerySpec.java | 14 +- .../wls/exporter/domain/MBeanSelector.java | 80 +++++++- .../com/oracle/wls/exporter/DemoInputs.java | 2 +- .../oracle/wls/exporter/ExporterCallTest.java | 24 +++ .../exporter/domain/MBeanSelectorTest.java | 175 +++++++++++++++++- 10 files changed, 323 insertions(+), 38 deletions(-) rename wls-exporter-core/src/{main => test}/java/com/oracle/wls/exporter/DemoInputs.java (99%) diff --git a/build-helper-mojo/src/main/java/com/oracle/wls/buildhelper/GitTagMojo.java b/build-helper-mojo/src/main/java/com/oracle/wls/buildhelper/GitTagMojo.java index 0fcaf99b..fe157de6 100644 --- a/build-helper-mojo/src/main/java/com/oracle/wls/buildhelper/GitTagMojo.java +++ b/build-helper-mojo/src/main/java/com/oracle/wls/buildhelper/GitTagMojo.java @@ -1,14 +1,8 @@ -// Copyright (c) 2022, Oracle and/or its affiliates. +// Copyright (c) 2022, 2023, Oracle and/or its affiliates. // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl. package com.oracle.wls.buildhelper; -import org.apache.maven.plugin.AbstractMojo; -import org.apache.maven.plugin.MojoExecutionException; -import org.apache.maven.plugins.annotations.LifecyclePhase; -import org.apache.maven.plugins.annotations.Mojo; -import org.apache.maven.plugins.annotations.Parameter; - import java.io.File; import java.io.IOException; import java.nio.file.Files; @@ -17,6 +11,12 @@ import java.util.Collections; import java.util.List; +import org.apache.maven.plugin.AbstractMojo; +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugins.annotations.LifecyclePhase; +import org.apache.maven.plugins.annotations.Mojo; +import org.apache.maven.plugins.annotations.Parameter; + @Mojo(name = "gitVersion", defaultPhase = LifecyclePhase.PREPARE_PACKAGE) public class GitTagMojo extends AbstractMojo { @@ -61,7 +61,7 @@ private String toVersionString(String gitResponse) { } private String formatVersionString(String[] segments) { - final String commit = segments[segments.length - 1]; + final String commit = segments[segments.length - 1].substring(1); final String numCommits = segments[segments.length - 2]; final String versionParts = String.join("-", Arrays.copyOfRange(segments, 0, segments.length - 2)); return formatVersionString(commit, numCommits, versionParts); diff --git a/build-helper-mojo/src/test/java/com/oracle/wls/buildhelper/GitTagMojoTest.java b/build-helper-mojo/src/test/java/com/oracle/wls/buildhelper/GitTagMojoTest.java index d09eb01c..fb45a68f 100644 --- a/build-helper-mojo/src/test/java/com/oracle/wls/buildhelper/GitTagMojoTest.java +++ b/build-helper-mojo/src/test/java/com/oracle/wls/buildhelper/GitTagMojoTest.java @@ -1,21 +1,25 @@ -// Copyright (c) 2022, Oracle and/or its affiliates. +// Copyright (c) 2022, 2023, Oracle and/or its affiliates. // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl. package com.oracle.wls.buildhelper; +import java.io.File; +import java.lang.reflect.Field; +import java.nio.file.Files; +import java.nio.file.Path; + import org.apache.maven.plugin.AbstractMojo; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugins.annotations.LifecyclePhase; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import java.io.File; -import java.lang.reflect.Field; -import java.nio.file.Files; -import java.nio.file.Path; - import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; import static org.junit.jupiter.api.Assertions.assertThrows; class GitTagMojoTest { @@ -88,7 +92,7 @@ void whenGitResponseIsVersionPlusHistory_createVersionProperty() throws Exceptio mojo.execute(); assertThat(inMemoryFileSystem.getContents(outputFile.getAbsolutePath()), - containsString("version=gcb4385f3aa (946 commits since v3.3.5-3)")); + containsString("version=cb4385f3aa (946 commits since v3.3.5-3)")); } @Test diff --git a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/ExporterCall.java b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/ExporterCall.java index 20ec9b76..c30702cf 100644 --- a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/ExporterCall.java +++ b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/ExporterCall.java @@ -1,4 +1,4 @@ -// Copyright (c) 2021, 2022, Oracle and/or its affiliates. +// Copyright (c) 2021, 2023, Oracle and/or its affiliates. // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl. package com.oracle.wls.exporter; @@ -52,9 +52,7 @@ private void displayMetrics(WebClient webClient, MetricsStream metricsStream, MB if (!metrics.isEmpty()) sort(metrics).forEach(metricsStream::printMetric); } catch (RestQueryException e) { - metricsStream.println( - withCommentMarkers("REST service was unable to handle this query and returned a " + HTTP_BAD_REQUEST + "\n" - + selector.getPrintableRequest())); + reportProblem(metricsStream, selector); } catch (AuthenticationChallengeException e) { // don't add a message for this case throw e; } catch (IOException | RuntimeException e) { @@ -63,6 +61,19 @@ private void displayMetrics(WebClient webClient, MetricsStream metricsStream, MB } } + private void reportProblem(MetricsStream metricsStream, MBeanSelector selector) { + metricsStream.println(withCommentMarkers(getProblem(selector) + "\n" + selector.getPrintableRequest())); + } + + private String getProblem(MBeanSelector selector) { + if (selector.isRequestForPrivilegedProperty()) + return "You seem to have encountered a bug in the WebLogic REST API.\n" + + " The JDBCServiceRuntime.JDBCDataSourceRuntimeMBeans.properties property " + + " may only be accessed by a user with administrator privileges."; + else + return "REST service was unable to handle this query and returned a " + HTTP_BAD_REQUEST; + } + private static String withCommentMarkers(String string) { StringBuilder sb = new StringBuilder(); for (String s : string.split("\\r?\\n")) diff --git a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/WebClientCommon.java b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/WebClientCommon.java index 95245a44..8e3e1df3 100644 --- a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/WebClientCommon.java +++ b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/WebClientCommon.java @@ -25,7 +25,6 @@ import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; import static java.net.HttpURLConnection.HTTP_FORBIDDEN; import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED; -import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; /** * A client for sending http requests. Note that it does not do any authentication by itself. @@ -227,7 +226,7 @@ private void processStatusCode() { case HTTP_FORBIDDEN: throw new ForbiddenException(); default: - if (response.getResponseCode() > SC_BAD_REQUEST) + if (response.getResponseCode() > HTTP_BAD_REQUEST) throw createServerErrorException(); } } diff --git a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/ExporterConfig.java b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/ExporterConfig.java index 4b56b4ab..d5008ef7 100644 --- a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/ExporterConfig.java +++ b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/ExporterConfig.java @@ -1,4 +1,4 @@ -// Copyright (c) 2017, 2022, Oracle and/or its affiliates. +// Copyright (c) 2017, 2023, Oracle and/or its affiliates. // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl. package com.oracle.wls.exporter.domain; @@ -24,6 +24,10 @@ * @author Russell Gold */ public class ExporterConfig implements MetricsProcessor { + // Due to a bug in the WLS REST API, a query that includes this field will fail if not run with admin privileges. + // The code thus ensures that it is excluded from all queries. + private static final String[] FORBIDDEN_FIELDS = {"JDBCServiceRuntime:JDBCDataSourceRuntimeMBeans:properties"}; + public static final String DOMAIN_NAME_PROPERTY = "DOMAIN"; private static final String QUERY_SYNC = "query_sync"; @@ -173,7 +177,7 @@ private QuerySyncConfiguration loadQuerySync(Object o) { private void appendQueries(Object queriesYaml) { for (Map selectorSpec : getAsListOfMaps(queriesYaml)) { - appendQuery(MBeanSelector.create(selectorSpec)); + appendQuery(MBeanSelector.create(selectorSpec).withForbiddenFields(FORBIDDEN_FIELDS)); } } diff --git a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/JsonQuerySpec.java b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/JsonQuerySpec.java index e6528582..3612f3c0 100644 --- a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/JsonQuerySpec.java +++ b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/JsonQuerySpec.java @@ -1,4 +1,4 @@ -// Copyright (c) 2017, 2020, Oracle and/or its affiliates. +// Copyright (c) 2017, 2023, Oracle and/or its affiliates. // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl. package com.oracle.wls.exporter.domain; @@ -27,6 +27,12 @@ class JsonQuerySpec { private Map children = null; private String keyName = null; private List selectedKeys = null; + private List excludeFields = null; + + JsonQuerySpec asTopLevel() { + addFields(); + return this; + } /** * Specifies the name of any mbean values which should be retrieved. @@ -62,6 +68,7 @@ JsonObject toJsonObject() { result.add("links", new JsonArray()); if (fields != null) result.add("fields", asStringArray(fields)); + if (excludeFields != null) result.add("excludeFields", asStringArray(excludeFields)); if (keyName != null) result.add(keyName, asStringArray(selectedKeys)); if (children != null) asChildObject(result); @@ -81,4 +88,9 @@ private void asChildObject(JsonObject result) { nesting.add(entry.getKey(), entry.getValue().toJsonObject()); } } + + public void excludeField(String fieldName) { + if (excludeFields == null) excludeFields = new ArrayList<>(); + excludeFields.add(fieldName); + } } diff --git a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/MBeanSelector.java b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/MBeanSelector.java index 7efb7e78..cf2e500d 100644 --- a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/MBeanSelector.java +++ b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/MBeanSelector.java @@ -1,4 +1,4 @@ -// Copyright (c) 2017, 2022, Oracle and/or its affiliates. +// Copyright (c) 2017, 2023, Oracle and/or its affiliates. // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl. package com.oracle.wls.exporter.domain; @@ -6,6 +6,7 @@ import java.time.Clock; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; @@ -15,6 +16,7 @@ import java.util.Optional; import java.util.Set; import java.util.regex.Pattern; +import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.stream.StreamSupport; @@ -61,6 +63,7 @@ public class MBeanSelector { private Map nestedSelectors = new LinkedHashMap<>(); private QueryType queryType = QueryType.RUNTIME; private long lastKeyTime = 0; + private String[] forbiddenFields; private static MBeanSelector createDomainNameSelector() { Map yaml = new HashMap<>(); @@ -228,6 +231,39 @@ public static MBeanSelector create(Map map) { return new MBeanSelector(map); } + MBeanSelector withForbiddenFields(String... forbiddenFields) { + setForbiddenFields(forbiddenFields); + return this; + } + + private void setForbiddenFields(String[] forbiddenFields) { + this.forbiddenFields = forbiddenFields; + nestedSelectors.entrySet().forEach(this::defineNestedForbiddenFields); + } + + private void defineNestedForbiddenFields(Map.Entry entry) { + entry.getValue().setForbiddenFields(getNestedForbiddenFields(entry.getKey())); + } + + private String[] getNestedForbiddenFields(String key) { + return Arrays.stream(forbiddenFields) + .map(f -> withoutMatchingTopLevel(f, key)) + .filter(Objects::nonNull) + .toArray(String[]::new); + } + + private String withoutMatchingTopLevel(String forbiddenField, String key) { + final int index = forbiddenField.indexOf(':'); + if (index < 0) + return null; + else if (!key.equalsIgnoreCase(forbiddenField.substring(0, index))) { + return null; + } else { + return forbiddenField.substring(index+1); + } + } + + /** * Returns the type of mbean to process, from among those captured by this selector. If empty or null, * processes all captured mbeans. @@ -284,6 +320,7 @@ String getExcludedKeys() { String[] getQueryValues() { final List result = new ArrayList<>(values); if (stringValues != null) result.addAll(stringValues.keySet()); + getActiveForbiddenFields().forEach(result::remove); return result.toArray(new String[0]); } @@ -313,13 +350,16 @@ public String getPrintableRequest() { * @return a JSON string */ public String getRequest() { - return toQuerySpec().toJson(new Gson()); + return toQuerySpec().asTopLevel().toJson(new Gson()); } JsonQuerySpec toQuerySpec() { JsonQuerySpec spec = new JsonQuerySpec(); - if (!useAllValues()) + if (useAllValues()) { + getActiveForbiddenFields().forEach(spec::excludeField); + } else { selectQueryFields(spec, getQueryValues()); + } if (currentSelectorHasFilter() && !filter.isEmpty()) spec.setFilter(key, filter); @@ -330,6 +370,17 @@ JsonQuerySpec toQuerySpec() { return spec; } + private List getActiveForbiddenFields() { + if (forbiddenFields == null) + return Collections.emptyList(); + else + return Arrays.stream(forbiddenFields).filter(this::isLeafField).collect(Collectors.toList()); + } + + private boolean isLeafField(String forbiddenField) { + return !forbiddenField.contains(":"); + } + private boolean isEnabled() { return !filter.isEmpty() || !currentSelectorHasFilter(); } @@ -354,10 +405,14 @@ public String getKeyRequest() { JsonQuerySpec toKeyQuerySpec() { JsonQuerySpec spec = new JsonQuerySpec(); - if (currentSelectorHasFilter()) spec.addFields(key); + if (currentSelectorHasFilter()) + spec.addFields(key); + else + spec.addFields(); for (Map.Entry selector : nestedSelectors.entrySet()) - spec.addChild(selector.getKey(), selector.getValue().toKeyQuerySpec()); + if (selector.getValue().hasFilter()) + spec.addChild(selector.getKey(), selector.getValue().toKeyQuerySpec()); return spec; } @@ -552,4 +607,19 @@ private int getIndex(String value, List fieldValues) { void postProcessMetrics(Map metrics, MetricsProcessor processor) { queryType.postProcessMetrics(metrics, processor); } + + public boolean isRequestForPrivilegedProperty() { + if (queryType != QueryType.RUNTIME) return false; + + return Optional.ofNullable(nestedSelectors.get("JDBCServiceRuntime")) + .map(MBeanSelector::isJDBCDataSourceRuntimeMBeansPropertiesRequest) + .orElse(false); + } + + private boolean isJDBCDataSourceRuntimeMBeansPropertiesRequest() { + final MBeanSelector jdbcSourceRuntimeSelector = nestedSelectors.get("JDBCDataSourceRuntimeMBeans"); + if (jdbcSourceRuntimeSelector == null) return false; + return jdbcSourceRuntimeSelector.values.isEmpty() || jdbcSourceRuntimeSelector.values.contains("properties"); + } + } diff --git a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/DemoInputs.java b/wls-exporter-core/src/test/java/com/oracle/wls/exporter/DemoInputs.java similarity index 99% rename from wls-exporter-core/src/main/java/com/oracle/wls/exporter/DemoInputs.java rename to wls-exporter-core/src/test/java/com/oracle/wls/exporter/DemoInputs.java index ecf87a2e..de66b791 100644 --- a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/DemoInputs.java +++ b/wls-exporter-core/src/test/java/com/oracle/wls/exporter/DemoInputs.java @@ -1,4 +1,4 @@ -// Copyright (c) 2017, 2022, Oracle and/or its affiliates. +// Copyright (c) 2017, 2023, Oracle and/or its affiliates. // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl. package com.oracle.wls.exporter; diff --git a/wls-exporter-core/src/test/java/com/oracle/wls/exporter/ExporterCallTest.java b/wls-exporter-core/src/test/java/com/oracle/wls/exporter/ExporterCallTest.java index 7be68ae7..49903564 100644 --- a/wls-exporter-core/src/test/java/com/oracle/wls/exporter/ExporterCallTest.java +++ b/wls-exporter-core/src/test/java/com/oracle/wls/exporter/ExporterCallTest.java @@ -25,6 +25,10 @@ class ExporterCallTest { private static final String ONE_VALUE_CONFIG = "queries:\n- groups:\n key: name\n values: testSample1"; private static final String CONFIG_WITH_FILTER = "queries:" + "\n- groups:\n key: name\n includedKeyValues: abc.*\n values: testSample1"; + private static final String REQUEST_FOR_PRIVILEGED_PROPERTY = "queries:" + + "\n- JDBCServiceRuntime:\n JDBCDataSourceRuntimeMBeans:\n key: name\n values: properties"; + private static final String REQUEST_INCLUDES_PRIVILEGED_PROPERTY = "queries:" + + "\n- JDBCServiceRuntime:\n JDBCDataSourceRuntimeMBeans:\n prefix: ds_\n key: name\n"; private static final String DUAL_QUERY_CONFIG = ONE_VALUE_CONFIG + "\n- clubs:\n key: name\n values: testSample2"; @@ -117,6 +121,26 @@ void whenQuerySentWithFilter_twoRequestsAreMade() throws IOException { assertThat(factory.getNumQueriesSent(), equalTo(2)); } + @Test + void whenBadQueryReceivedAndConfigurationSelectsPrivilegedPropertiesProperty_explainProblem() throws IOException { + factory.reportBadQuery(); + LiveConfiguration.loadFromString(REQUEST_FOR_PRIVILEGED_PROPERTY); + + handleMetricsCall(context.withHttps()); + + assertThat(context.getResponse(), containsString("bug in the WebLogic REST API")); + } + + @Test + void whenBadQueryReceivedAndConfigurationSelectsPropertiesIncludingPrivilegedProperty_explainProblem() throws IOException { + factory.reportBadQuery(); + LiveConfiguration.loadFromString(REQUEST_INCLUDES_PRIVILEGED_PROPERTY); + + handleMetricsCall(context.withHttps()); + + assertThat(context.getResponse(), containsString("bug in the WebLogic REST API")); + } + @Test void whenHaveMultipleQueries_sendServerDefinedCookieOnSecondQuery() throws IOException { factory.forJson(QUERY_RESPONSE1_JSON).withResponseHeader(SET_COOKIE_HEADER, "cookieName=newValue").addResponse(); diff --git a/wls-exporter-core/src/test/java/com/oracle/wls/exporter/domain/MBeanSelectorTest.java b/wls-exporter-core/src/test/java/com/oracle/wls/exporter/domain/MBeanSelectorTest.java index 84605d5d..72b07b9e 100644 --- a/wls-exporter-core/src/test/java/com/oracle/wls/exporter/domain/MBeanSelectorTest.java +++ b/wls-exporter-core/src/test/java/com/oracle/wls/exporter/domain/MBeanSelectorTest.java @@ -1,4 +1,4 @@ -// Copyright (c) 2017, 2022, Oracle and/or its affiliates. +// Copyright (c) 2017, 2023, Oracle and/or its affiliates. // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl. package com.oracle.wls.exporter.domain; @@ -36,6 +36,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -222,6 +223,68 @@ void whenMapHasNestedElements_pathIncludesChildren() { assertThat(querySpec(selector), hasJsonPath("$.children.servlets.fields", containsInAnyOrder("first", "second"))); } + @Test + void whenTopLevelSelectorHasFields_isIncludedInQuery() { + MBeanSelector selector = MBeanSelector.create(ImmutableMap.of(MBeanSelector.VALUES_KEY, "sequence")); + + assertThat(querySpec(selector), hasJsonPath("$.fields", contains("sequence"))); + } + + @Test + void whenTopLevelSelectorHasNoFields_fieldsListIsEmpty() { + MBeanSelector selector = MBeanSelector.create(ImmutableMap.of("servlets", + ImmutableMap.of(MBeanSelector.VALUES_KEY, new String[] {"first", "second"}))); + + assertThat(querySpec(selector), hasJsonPath("$.fields", hasSize(0))); + } + + @Test + void whenTopLevelSelectorHasPrefixAndNoFields_fieldsListIsEmpty() { + MBeanSelector selector = MBeanSelector.create(ImmutableMap.of(MBeanSelector.PREFIX_KEY, "top_", "servlets", + ImmutableMap.of(MBeanSelector.VALUES_KEY, new String[] {"first", "second"}))); + + assertThat(querySpec(selector), hasJsonPath("$.fields", hasSize(0))); + } + + @Test + void whenSelectorHasForbiddenFieldLeaf_querySpecListsItAsExcluded() { + MBeanSelector selector = MBeanSelector.create(ImmutableMap.of(MBeanSelector.PREFIX_KEY, "top_")).withForbiddenFields("bottom"); + + assertThat(querySpec(selector), hasJsonPath("$.excludeFields", contains("bottom"))); + } + + @Test + void whenSelectorExplicitlyIncludesForbiddenField_querySpecExcludesItFromList() { + MBeanSelector selector = MBeanSelector.create( + ImmutableMap.of(MBeanSelector.VALUES_KEY, new String[] {"top", "middle", "bottom"})).withForbiddenFields("bottom"); + + assertThat(querySpec(selector), hasJsonPath("$.fields", containsInAnyOrder("top", "middle"))); + assertThat(querySpec(selector), hasNoJsonPath("$.excludeFields")); + } + + @Test + void whenSelectorHasNestedForbiddenField_querySpecListsItAsExcluded() { + MBeanSelector selector = MBeanSelector.create(DEEP_MAP).withForbiddenFields("groups:middle:subgroup1:bottom"); + + assertThat(querySpec(selector), + hasJsonPath("$.children.groups.children.middle.children.subgroup1.excludeFields", contains("bottom"))); + } + + private static final Map DEEP_MAP = ImmutableMap.of("groups", + ImmutableMap.of(MBeanSelector.QUERY_KEY, "groupName", + "middle", ImmutableMap.of( + "subgroup1", ImmutableMap.of(MBeanSelector.KEY_NAME, "name", MBeanSelector.PREFIX_KEY, "sub1_"), + "subgroup2", ImmutableMap.of(MBeanSelector.PREFIX_KEY, "sub2_", MBeanSelector.VALUES_KEY, "val2" )))); + + + @Test + void whenSelectorHasAttempedNestingDeeperThanForbiddenLeaf_ignoreIt() { + MBeanSelector selector = MBeanSelector.create(DEEP_MAP).withForbiddenFields("groups:middle"); + + assertThat(querySpec(selector), + hasJsonPath("$.children.groups.children.middle.children.subgroup2.fields", contains("val2"))); + } + @Test void whenIncludedKeysSpecifiedWithoutKeyName_report() { final Map BAD_MAP_WITH_INCLUDED_KEYS @@ -250,17 +313,39 @@ void whenMapHasSelectedKeys_AddToKeyQuery() { @Test void whenMapHasSelectedKeysAtMultipleLevels_AddToKeyQuery() { - MBeanSelector selector = MBeanSelector.create(DEEP_MAP_WITH_INCLUDED_KEYS); + MBeanSelector selector = MBeanSelector.create(DEEP_MAP_WITH_NESTED_INCLUDED_KEYS); assertThat(selector.getKeyRequest(), hasJsonPath("$.children.groups.fields", contains("groupName"))); - assertThat(selector.getKeyRequest(), hasJsonPath("$.children.groups.children.subgroup1.fields", contains("name1"))); - assertThat(selector.getKeyRequest(), hasJsonPath("$.children.groups.children.subgroup2.fields", contains("name2"))); + assertThat(selector.getKeyRequest(), hasJsonPath("$.children.groups.children.middle.children.subgroup1.fields", contains("name1"))); + assertThat(selector.getKeyRequest(), hasJsonPath("$.children.groups.children.middle.children.subgroup2.fields", contains("name2"))); } - private static final Map DEEP_MAP_WITH_INCLUDED_KEYS = ImmutableMap.of("groups", - ImmutableMap.of(MBeanSelector.QUERY_KEY, "groupName", MBeanSelector.INCLUDED_KEYS_KEY, "alpha|beta", + private static final Map DEEP_MAP_WITH_NESTED_INCLUDED_KEYS = ImmutableMap.of(MBeanSelector.PREFIX_KEY, "wls_", "groups", + ImmutableMap.of(MBeanSelector.QUERY_KEY, "groupName", MBeanSelector.INCLUDED_KEYS_KEY, "alpha|beta", + "middle", ImmutableMap.of( "subgroup1", ImmutableMap.of(MBeanSelector.QUERY_KEY, "name1", MBeanSelector.INCLUDED_KEYS_KEY, "abc.*", MBeanSelector.VALUES_KEY, "group1Val" ), - "subgroup2", ImmutableMap.of(MBeanSelector.QUERY_KEY, "name2", MBeanSelector.INCLUDED_KEYS_KEY, "def.*", MBeanSelector.VALUES_KEY, "group2Val" ))); + "subgroup2", ImmutableMap.of(MBeanSelector.QUERY_KEY, "name2", MBeanSelector.INCLUDED_KEYS_KEY, "def.*", MBeanSelector.VALUES_KEY, "group2Val" )))); + + @Test + void whenIntermediateLevelLacksSelectedKeys_dontRequestFieldsInKeyQuery() { + MBeanSelector selector = MBeanSelector.create(DEEP_MAP_WITH_NESTED_INCLUDED_KEYS); + + assertThat(selector.getKeyRequest(), hasJsonPath("$.children.groups.children.middle.fields", hasSize(0))); + } + + @Test + void whenNestedEntriesLackFilter_excludeSubTreeFromKeyQuery() { + MBeanSelector selector = MBeanSelector.create(DEEP_MAP_WITHOUT_NESTED_INCLUDED_KEYS); + + assertThat(selector.getKeyRequest(), hasJsonPath("$.children.groups.fields", contains("groupName"))); + assertThat(selector.getKeyRequest(), hasNoJsonPath("$.children.groups.children")); + } + + private static final Map DEEP_MAP_WITHOUT_NESTED_INCLUDED_KEYS = ImmutableMap.of("groups", + ImmutableMap.of(MBeanSelector.QUERY_KEY, "groupName", MBeanSelector.INCLUDED_KEYS_KEY, "alpha|beta", + "middle", ImmutableMap.of( + "subgroup1", ImmutableMap.of(MBeanSelector.QUERY_KEY, "name1", MBeanSelector.VALUES_KEY, "group1Val" ), + "subgroup2", ImmutableMap.of(MBeanSelector.QUERY_KEY, "name2", MBeanSelector.VALUES_KEY, "group2Val" )))); @Test void whenMapLacksKeyFilter_dontNeedNewKeys() { @@ -311,6 +396,31 @@ void afterKeysOffered_selectorHasIncludedKeys() { assertThat(selector.getRequest(), hasJsonPath("$.children.servlets.servletName", containsInAnyOrder("alpha", "beta"))); } + @Test + void afterKeysOfferedThatDoNotMatchFilter_parentSelectorHasNoChildren() { + MBeanSelector selector = MBeanSelector.create(MAP_WITH_INCLUDED_KEYS); + selector.offerKeys(MISMATCHED_KEY_RESPONSE); + + assertThat(selector.getRequest(), hasNoJsonPath("$.children")); + } + + @Test + void afterKeysOfferedThatDoNotMatchFilter_parentSelectorHasEmptyFieldsArray() { + MBeanSelector selector = MBeanSelector.create(MAP_WITH_INCLUDED_KEYS); + selector.offerKeys(MISMATCHED_KEY_RESPONSE); + + assertThat(selector.getRequest(), hasJsonPath("$.fields", hasSize(0))); + } + + private static final String MISMATCHED_KEY_RESPONSE_JSON = "{\"servlets\": {\"items\": [\n" + + " {\"servletName\": \"delta\"},\n" + + " {\"servletName\": \"epsilon\" },\n" + + " {\"servletName\": \"zeta\"}\n" + + "]}}"; + + private static final JsonObject MISMATCHED_KEY_RESPONSE = + JsonParser.parseString(MISMATCHED_KEY_RESPONSE_JSON).getAsJsonObject(); + @Test void whenMapHasSelectedKeysAtMultipleLevels_specifySelectedKeys() { MBeanSelector selector = MBeanSelector.create(DEEP_MAP_WITH_INCLUDED_KEYS); @@ -321,6 +431,11 @@ void whenMapHasSelectedKeysAtMultipleLevels_specifySelectedKeys() { assertThat(selector.getRequest(), hasJsonPath("$.children.groups.children.subgroup2.name2", containsInAnyOrder("defabc", "def123", "def678"))); } + private static final Map DEEP_MAP_WITH_INCLUDED_KEYS = ImmutableMap.of("groups", + ImmutableMap.of(MBeanSelector.QUERY_KEY, "groupName", MBeanSelector.INCLUDED_KEYS_KEY, "alpha|beta", + "subgroup1", ImmutableMap.of(MBeanSelector.QUERY_KEY, "name1", MBeanSelector.INCLUDED_KEYS_KEY, "abc.*", MBeanSelector.VALUES_KEY, "group1Val" ), + "subgroup2", ImmutableMap.of(MBeanSelector.QUERY_KEY, "name2", MBeanSelector.INCLUDED_KEYS_KEY, "def.*", MBeanSelector.VALUES_KEY, "group2Val" ))); + private static final String DEEP_KEY_RESPONSE_JSON = "{'groups': {'items': [\n" + " {'groupName': 'alpha',\n" + " 'subgroup1': {'items': [\n" + @@ -357,6 +472,52 @@ void whenMapHasSelectedKeysAtMultipleLevels_specifySelectedKeys() { private static final JsonObject DEEP_KEY_RESPONSE = JsonParser.parseString(DEEP_KEY_RESPONSE_JSON.replace("'", "\"")).getAsJsonObject(); + @Test + void whenMapHasMismatchedNested_specifySelectedKeys() { + MBeanSelector selector = MBeanSelector.create(DEEP_MAP_WITH_INCLUDED_KEYS); + selector.offerKeys(MISMATCHED_DEEP_KEY_RESPONSE); + + assertThat(selector.getRequest(), hasJsonPath("$.children.groups.groupName", containsInAnyOrder("alpha", "beta"))); + assertThat(selector.getRequest(), hasJsonPath("$.children.groups.children.subgroup1.name1", containsInAnyOrder("abcdef", "abc123", "abc567"))); + assertThat(selector.getRequest(), hasNoJsonPath("$.children.groups.children.subgroup2")); + } + + private static final String MISMATCHED_DEEP_KEY_RESPONSE_JSON = "{'groups': {'items': [\n" + + " {'groupName': 'alpha',\n" + + " 'subgroup1': {'items': [\n" + + " {'name1': 'abcdef'},\n" + + " {'name1': 'abc123'},\n" + + " {'name1': 'ab12_2'}\n" + + " ]},\n" + + " 'subgroup2': {'items': [\n" + + " {'name2': 'abcdef'},\n" + + " {'name2': 'xyzabc'},\n" + + " {'name2': 'jklmn'}\n" + + " ]}\n" + + " },\n" + + " {'groupName': 'beta',\n" + + " 'subgroup1': {'items': [\n" + + " {'name1': 'abcdef'},\n" + + " {'name1': 'abc567'},\n" + + " {'name1': 'abjkl'}\n" + + " ]},\n" + + " 'subgroup2': {'items': [\n" + + " {'name2': 'ghi678'},\n" + + " ]}\n" + + " },\n" + + " {'groupName': 'gamma',\n" + + " 'subgroup1': {'items': [\n" + + " {'name1': 'abcxyz'},\n" + + " ]},\n" + + " 'subgroup2': {'items': [\n" + + " {'name2': 'jkl987'},\n" + + " ]}\n" + + " }\n" + + "]}}"; + + private static final JsonObject MISMATCHED_DEEP_KEY_RESPONSE = + JsonParser.parseString(MISMATCHED_DEEP_KEY_RESPONSE_JSON.replace("'", "\"")).getAsJsonObject(); + @Test void whenMapHasBothIncludedAndExcludedKeys_selectKeysLeft() { MBeanSelector selector = MBeanSelector.create(DEEP_MAP_WITH_INCLUDED_AND_EXCLUDED_KEYS); From 47c6351d55b121a6070f552cedc3465f972cd1fd Mon Sep 17 00:00:00 2001 From: Russell Gold Date: Thu, 5 Jan 2023 11:43:37 -0500 Subject: [PATCH 2/2] reverse removal of unqualified top-level metrics --- .../oracle/wls/exporter/domain/JsonQuerySpec.java | 8 ++------ .../oracle/wls/exporter/domain/MBeanSelector.java | 2 +- .../wls/exporter/domain/MBeanSelectorTest.java | 12 ++---------- 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/JsonQuerySpec.java b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/JsonQuerySpec.java index 3612f3c0..0aa5eab7 100644 --- a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/JsonQuerySpec.java +++ b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/JsonQuerySpec.java @@ -8,6 +8,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import com.google.gson.Gson; @@ -29,18 +30,13 @@ class JsonQuerySpec { private List selectedKeys = null; private List excludeFields = null; - JsonQuerySpec asTopLevel() { - addFields(); - return this; - } - /** * Specifies the name of any mbean values which should be retrieved. * @param newFields the field names to add to any previous defined */ void addFields(String... newFields) { if (fields == null) fields = new ArrayList<>(); - fields.addAll(Arrays.asList(newFields)); + Arrays.stream(newFields).filter(Objects::nonNull).forEach(fields::add); } /** diff --git a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/MBeanSelector.java b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/MBeanSelector.java index cf2e500d..069bda86 100644 --- a/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/MBeanSelector.java +++ b/wls-exporter-core/src/main/java/com/oracle/wls/exporter/domain/MBeanSelector.java @@ -350,7 +350,7 @@ public String getPrintableRequest() { * @return a JSON string */ public String getRequest() { - return toQuerySpec().asTopLevel().toJson(new Gson()); + return toQuerySpec().toJson(new Gson()); } JsonQuerySpec toQuerySpec() { diff --git a/wls-exporter-core/src/test/java/com/oracle/wls/exporter/domain/MBeanSelectorTest.java b/wls-exporter-core/src/test/java/com/oracle/wls/exporter/domain/MBeanSelectorTest.java index 72b07b9e..4c4cb980 100644 --- a/wls-exporter-core/src/test/java/com/oracle/wls/exporter/domain/MBeanSelectorTest.java +++ b/wls-exporter-core/src/test/java/com/oracle/wls/exporter/domain/MBeanSelectorTest.java @@ -231,19 +231,11 @@ void whenTopLevelSelectorHasFields_isIncludedInQuery() { } @Test - void whenTopLevelSelectorHasNoFields_fieldsListIsEmpty() { - MBeanSelector selector = MBeanSelector.create(ImmutableMap.of("servlets", - ImmutableMap.of(MBeanSelector.VALUES_KEY, new String[] {"first", "second"}))); - - assertThat(querySpec(selector), hasJsonPath("$.fields", hasSize(0))); - } - - @Test - void whenTopLevelSelectorHasPrefixAndNoFields_fieldsListIsEmpty() { + void whenTopLevelSelectorHasPrefixAndNoFields_dontRequestFields() { MBeanSelector selector = MBeanSelector.create(ImmutableMap.of(MBeanSelector.PREFIX_KEY, "top_", "servlets", ImmutableMap.of(MBeanSelector.VALUES_KEY, new String[] {"first", "second"}))); - assertThat(querySpec(selector), hasJsonPath("$.fields", hasSize(0))); + assertThat(querySpec(selector), hasNoJsonPath("$.fields")); } @Test