This is an automated email from the ASF dual-hosted git repository.
richardstartin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new aadd0cddcf make ConfigUtils testable without illegal reflective access
(#8530)
aadd0cddcf is described below
commit aadd0cddcf5fb59286d4205fa55a9e1ff2879bc8
Author: Richard Startin <[email protected]>
AuthorDate: Thu Apr 14 08:48:03 2022 +0100
make ConfigUtils testable without illegal reflective access (#8530)
---
.../org/apache/pinot/spi/config/ConfigUtils.java | 24 ++++++++-----
.../org/apache/pinot/spi/config/Environment.java | 30 ++++++++++++++++
.../apache/pinot/spi/config/ConfigUtilsTest.java | 41 +++-------------------
3 files changed, 51 insertions(+), 44 deletions(-)
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/config/ConfigUtils.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/config/ConfigUtils.java
index dcee78aff6..a4f34a49fe 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/config/ConfigUtils.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/config/ConfigUtils.java
@@ -32,17 +32,24 @@ public class ConfigUtils {
private ConfigUtils() {
}
- private static final Map<String, String> ENVIRONMENT_VARIABLES =
System.getenv();
-
/**
* Apply environment variables to any given BaseJsonConfig.
*
* @return Config with environment variable applied.
*/
public static <T extends BaseJsonConfig> T applyConfigWithEnvVariables(T
config) {
+ return applyConfigWithEnvVariables(System::getenv, config);
+ }
+
+ /**
+ * Apply environment variables to any given BaseJsonConfig.
+ *
+ * @return Config with environment variable applied.
+ */
+ public static <T extends BaseJsonConfig> T
applyConfigWithEnvVariables(Environment environment, T config) {
JsonNode jsonNode;
try {
- jsonNode = applyConfigWithEnvVariables(config.toJsonNode());
+ jsonNode = applyConfigWithEnvVariables(environment, config.toJsonNode());
} catch (RuntimeException e) {
throw new RuntimeException(String
.format("Unable to apply environment variables on json config class
[%s].", config.getClass().getName()), e);
@@ -56,7 +63,7 @@ public class ConfigUtils {
}
}
- private static JsonNode applyConfigWithEnvVariables(JsonNode jsonNode) {
+ private static JsonNode applyConfigWithEnvVariables(Environment environment,
JsonNode jsonNode) {
final JsonNodeType nodeType = jsonNode.getNodeType();
switch (nodeType) {
case OBJECT:
@@ -64,7 +71,7 @@ public class ConfigUtils {
Iterator<Map.Entry<String, JsonNode>> iterator = jsonNode.fields();
while (iterator.hasNext()) {
final Map.Entry<String, JsonNode> next = iterator.next();
- next.setValue(applyConfigWithEnvVariables(next.getValue()));
+ next.setValue(applyConfigWithEnvVariables(environment,
next.getValue()));
}
}
break;
@@ -73,7 +80,7 @@ public class ConfigUtils {
ArrayNode arrayNode = (ArrayNode) jsonNode;
for (int i = 0; i < arrayNode.size(); i++) {
JsonNode arrayElement = arrayNode.get(i);
- arrayNode.set(i, applyConfigWithEnvVariables(arrayElement));
+ arrayNode.set(i, applyConfigWithEnvVariables(environment,
arrayElement));
}
}
break;
@@ -82,8 +89,9 @@ public class ConfigUtils {
if (field.startsWith("${") && field.endsWith("}")) {
String[] envVarSplits = field.substring(2, field.length() -
1).split(":", 2);
String envVarKey = envVarSplits[0];
- if (ENVIRONMENT_VARIABLES.containsKey(envVarKey)) {
- return
JsonNodeFactory.instance.textNode(ENVIRONMENT_VARIABLES.get(envVarKey));
+ String value = environment.getVariable(envVarKey);
+ if (value != null) {
+ return JsonNodeFactory.instance.textNode(value);
} else if (envVarSplits.length > 1) {
return JsonNodeFactory.instance.textNode(envVarSplits[1]);
}
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/config/Environment.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/config/Environment.java
new file mode 100644
index 0000000000..efd9eb1908
--- /dev/null
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/config/Environment.java
@@ -0,0 +1,30 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.spi.config;
+
+/**
+ * Encapsulates environment variables so they can be overridden for testing
purposes.
+ */
+public interface Environment {
+ /**
+ * @param name the name of the environment variable
+ * @return an environment variable or null if not set
+ */
+ String getVariable(String name);
+}
diff --git
a/pinot-spi/src/test/java/org/apache/pinot/spi/config/ConfigUtilsTest.java
b/pinot-spi/src/test/java/org/apache/pinot/spi/config/ConfigUtilsTest.java
index 61d9021f39..9220cfd425 100644
--- a/pinot-spi/src/test/java/org/apache/pinot/spi/config/ConfigUtilsTest.java
+++ b/pinot-spi/src/test/java/org/apache/pinot/spi/config/ConfigUtilsTest.java
@@ -19,9 +19,7 @@
package org.apache.pinot.spi.config;
import com.google.common.collect.ImmutableMap;
-import java.lang.reflect.Field;
import java.util.Arrays;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -40,8 +38,7 @@ import static org.testng.Assert.assertTrue;
public class ConfigUtilsTest {
@Test
- public void testIndexing()
- throws Exception {
+ public void testIndexing() {
IndexingConfig indexingConfig = new IndexingConfig();
indexingConfig.setLoadMode("${LOAD_MODE}");
indexingConfig.setAggregateMetrics(true);
@@ -90,10 +87,11 @@ public class ConfigUtilsTest {
.put(StreamConfigProperties.constructStreamProperty(streamType,
"aws.secretKey"), "${AWS_SECRET_KEY}");
indexingConfig.setStreamConfigs(streamConfigMap);
- setEnv(ImmutableMap.of("LOAD_MODE", "MMAP", "AWS_ACCESS_KEY",
"default_aws_access_key", "AWS_SECRET_KEY",
- "default_aws_secret_key"));
+ Map<String, String> environment =
+ ImmutableMap.of("LOAD_MODE", "MMAP", "AWS_ACCESS_KEY",
"default_aws_access_key", "AWS_SECRET_KEY",
+ "default_aws_secret_key");
- indexingConfig = ConfigUtils.applyConfigWithEnvVariables(indexingConfig);
+ indexingConfig = ConfigUtils.applyConfigWithEnvVariables(environment::get,
indexingConfig);
assertEquals(indexingConfig.getLoadMode(), "MMAP");
assertTrue(indexingConfig.isAggregateMetrics());
assertEquals(indexingConfig.getInvertedIndexColumns(),
invertedIndexColumns);
@@ -126,35 +124,6 @@ public class ConfigUtilsTest {
StreamConfig.DEFAULT_FLUSH_THRESHOLD_SEGMENT_SIZE_BYTES);
}
- private static void setEnv(Map<String, String> newEnvVariablsMap)
- throws Exception {
- try {
- Class<?> processEnvironmentClass =
Class.forName("java.lang.ProcessEnvironment");
- Field theEnvironmentField =
processEnvironmentClass.getDeclaredField("theEnvironment");
- theEnvironmentField.setAccessible(true);
- Map<String, String> env = (Map<String, String>)
theEnvironmentField.get(null);
- env.putAll(newEnvVariablsMap);
- Field theCaseInsensitiveEnvironmentField =
-
processEnvironmentClass.getDeclaredField("theCaseInsensitiveEnvironment");
- theCaseInsensitiveEnvironmentField.setAccessible(true);
- Map<String, String> cienv = (Map<String, String>)
theCaseInsensitiveEnvironmentField.get(null);
- cienv.putAll(newEnvVariablsMap);
- } catch (NoSuchFieldException e) {
- Class[] classes = Collections.class.getDeclaredClasses();
- Map<String, String> env = System.getenv();
- for (Class cl : classes) {
- if ("java.util.Collections$UnmodifiableMap".equals(cl.getName())) {
- Field field = cl.getDeclaredField("m");
- field.setAccessible(true);
- Object obj = field.get(env);
- Map<String, String> map = (Map<String, String>) obj;
- map.clear();
- map.putAll(newEnvVariablsMap);
- }
- }
- }
- }
-
@Test
public void testDefaultObfuscation() {
Map<String, Object> map = new HashMap<>();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
