This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 07ff1338525 SOLR-17347: Remove ENV storage/retrieval in EnvUtils
(#2534)
07ff1338525 is described below
commit 07ff133852545054d4fe13473588343e3e4d33b5
Author: David Smiley <[email protected]>
AuthorDate: Wed Jul 3 16:02:48 2024 +0200
SOLR-17347: Remove ENV storage/retrieval in EnvUtils (#2534)
Removed "env" methods because Solr code should almost always access global
configuration via system properties. Properties are augmented with the "env"
nonetheless.
---
solr/CHANGES.txt | 3 +
.../core/src/java/org/apache/solr/cli/SolrCLI.java | 7 +-
.../cloud/api/collections/CreateCollectionCmd.java | 2 +-
.../src/test/org/apache/solr/cli/PostToolTest.java | 4 +-
.../solr/opentelemetry/OtelTracerConfigurator.java | 2 +-
.../java/org/apache/solr/common/util/EnvUtils.java | 87 +++-------------------
.../org/apache/solr/common/util/EnvUtilsTest.java | 63 ++++++----------
.../org/apache/solr/cloud/SolrCloudTestCase.java | 4 +-
8 files changed, 44 insertions(+), 128 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index b4c7dd8301a..568b13051b6 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -206,6 +206,9 @@ Other Changes
* PR#2524: QueryResult refactoring so that it's only returned from
SolrIndexSearcher instead of
being provided to it. Deprecated APIs in 9x; should be removed later.
(David Smiley)
+* SOLR-17347: EnvUtils: Removed "env" methods, as they were problematic. Code
should almost never
+ refer to env vars. (David Smiley)
+
================== 9.6.1 ==================
Bug Fixes
---------------------
diff --git a/solr/core/src/java/org/apache/solr/cli/SolrCLI.java
b/solr/core/src/java/org/apache/solr/cli/SolrCLI.java
index f422586aaee..06e0714b722 100755
--- a/solr/core/src/java/org/apache/solr/cli/SolrCLI.java
+++ b/solr/core/src/java/org/apache/solr/cli/SolrCLI.java
@@ -245,9 +245,10 @@ public class SolrCLI implements CLIO {
}
public static String getDefaultSolrUrl() {
- String scheme = EnvUtils.getEnv("SOLR_URL_SCHEME", "http");
- String host = EnvUtils.getEnv("SOLR_TOOL_HOST", "localhost");
- String port = EnvUtils.getEnv("SOLR_PORT", "8983");
+ // note that ENV_VAR syntax (and the env vars too) are mapped to env.var
sys props
+ String scheme = EnvUtils.getProperty("solr.url.scheme", "http");
+ String host = EnvUtils.getProperty("solr.tool.host", "localhost");
+ String port = EnvUtils.getProperty("jetty.port", "8983"); // from
SOLR_PORT env
return String.format(Locale.ROOT, "%s://%s:%s",
scheme.toLowerCase(Locale.ROOT), host, port);
}
diff --git
a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
index 46c9b4e5a42..ccfd4a1e5e2 100644
---
a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
+++
b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
@@ -112,7 +112,7 @@ public class CreateCollectionCmd implements
CollApiCmds.CollectionApiCommand {
final boolean waitForFinalState = message.getBool(WAIT_FOR_FINAL_STATE,
false);
final String alias = message.getStr(ALIAS, collectionName);
log.info("Create collection {}", collectionName);
- boolean prsDefault = EnvUtils.getEnvAsBool(PRS_DEFAULT_PROP, false);
+ boolean prsDefault = EnvUtils.getPropertyAsBool(PRS_DEFAULT_PROP, false);
final boolean isPRS =
message.getBool(CollectionStateProps.PER_REPLICA_STATE, prsDefault);
if (log.isInfoEnabled()) {
log.info(
diff --git a/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
b/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
index a6e60f84bed..98686c277e2 100644
--- a/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
+++ b/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
@@ -114,8 +114,8 @@ public class PostToolTest extends SolrCloudTestCase {
public void testRunWithCollectionParam() throws Exception {
final String collection = "testRunWithCollectionParam";
- // Provide the port as an environment variable for the PostTool to look up.
- EnvUtils.setEnv("SOLR_PORT", cluster.getJettySolrRunner(0).getLocalPort()
+ "");
+ // Provide the port for the PostTool to look up.
+ EnvUtils.setProperty("jetty.port",
cluster.getJettySolrRunner(0).getLocalPort() + "");
withBasicAuth(CollectionAdminRequest.createCollection(collection, "conf1",
1, 1, 0, 0))
.processAndWait(cluster.getSolrClient(), 10);
diff --git
a/solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java
b/solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java
index f4a5384e633..0d2dd5df9de 100644
---
a/solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java
+++
b/solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java
@@ -41,7 +41,7 @@ public class OtelTracerConfigurator extends
TracerConfigurator {
private final Map<String, String> currentEnv;
public OtelTracerConfigurator() {
- this(EnvUtils.getEnvs());
+ this(System.getenv());
}
OtelTracerConfigurator(Map<String, String> currentEnv) {
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java
b/solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java
index 29c9586ca96..40bb7c0fdb3 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java
@@ -34,13 +34,14 @@ import java.util.stream.Collectors;
import org.apache.solr.common.SolrException;
/**
- * This class is a unified provider of environment variables and system
properties. It exposes a
- * mutable copy of the environment variables. It also converts 'SOLR_FOO'
variables to system
- * properties 'solr.foo' and provide various convenience accessors for them.
+ * Provides convenient access to System Properties for Solr. It also converts
'SOLR_FOO' env vars to
+ * system properties 'solr.foo', which is done on first access of this class.
All Solr code should
+ * use this in lieu of JDK equivalents.
*/
public class EnvUtils {
- private static final SortedMap<String, String> ENV = new
TreeMap<>(System.getenv());
+ /** Maps ENV keys to sys prop keys for special/custom mappings */
private static final Map<String, String> CUSTOM_MAPPINGS = new HashMap<>();
+
private static final Map<String, String> camelCaseToDotsMap = new
ConcurrentHashMap<>();
static {
@@ -52,7 +53,7 @@ public class EnvUtils {
for (String key : props.stringPropertyNames()) {
CUSTOM_MAPPINGS.put(key, props.getProperty(key));
}
- init(false);
+ init(false, System.getenv());
}
} catch (IOException e) {
throw new SolrException(
@@ -60,74 +61,6 @@ public class EnvUtils {
}
}
- /**
- * Get Solr's mutable copy of all environment variables.
- *
- * @return sorted map of environment variables
- */
- public static SortedMap<String, String> getEnvs() {
- return ENV;
- }
-
- /** Get a single environment variable as string */
- public static String getEnv(String key) {
- return ENV.get(key);
- }
-
- /** Get a single environment variable as string, or default */
- public static String getEnv(String key, String defaultValue) {
- return ENV.getOrDefault(key, defaultValue);
- }
-
- /** Get an environment variable as long */
- public static long getEnvAsLong(String key) {
- return Long.parseLong(ENV.get(key));
- }
-
- /** Get an environment variable as long, or default value */
- public static long getEnvAsLong(String key, long defaultValue) {
- String value = ENV.get(key);
- if (value == null) {
- return defaultValue;
- }
- return Long.parseLong(value);
- }
-
- /** Get an env var as boolean */
- public static boolean getEnvAsBool(String key) {
- return StrUtils.parseBool(ENV.get(key));
- }
-
- /** Get an env var as boolean, or default value */
- public static boolean getEnvAsBool(String key, boolean defaultValue) {
- String value = ENV.get(key);
- if (value == null) {
- return defaultValue;
- }
- return StrUtils.parseBool(value);
- }
-
- /** Get comma separated strings from env as List */
- public static List<String> getEnvAsList(String key) {
- return getEnv(key) != null ? stringValueToList(getEnv(key)) : null;
- }
-
- /** Get comma separated strings from env as List */
- public static List<String> getEnvAsList(String key, List<String>
defaultValue) {
- return ENV.get(key) != null ? getEnvAsList(key) : defaultValue;
- }
-
- /** Set an environment variable */
- public static void setEnv(String key, String value) {
- ENV.put(key, value);
- }
-
- /** Set all environment variables */
- public static synchronized void setEnvs(Map<String, String> env) {
- ENV.clear();
- ENV.putAll(env);
- }
-
/** Get all Solr system properties as a sorted map */
public static SortedMap<String, String> getProperties() {
return System.getProperties().entrySet().stream()
@@ -231,17 +164,15 @@ public class EnvUtils {
/**
* Re-reads environment variables and updates the internal map. Mainly for
internal and test use.
- *
- * @param overwrite if true, overwrite existing system properties with
environment variables
*/
- static synchronized void init(boolean overwrite) {
+ static synchronized void init(boolean overwrite, Map<String, String> env) {
// Convert eligible environment variables to system properties
- for (String key : ENV.keySet()) {
+ for (String key : env.keySet()) {
if (key.startsWith("SOLR_") || CUSTOM_MAPPINGS.containsKey(key)) {
String sysPropKey = envNameToSyspropName(key);
// Existing system properties take precedence
if (!sysPropKey.isBlank() && (overwrite || getProperty(sysPropKey,
null) == null)) {
- setProperty(sysPropKey, ENV.get(key));
+ setProperty(sysPropKey, env.get(key));
}
}
}
diff --git a/solr/solrj/src/test/org/apache/solr/common/util/EnvUtilsTest.java
b/solr/solrj/src/test/org/apache/solr/common/util/EnvUtilsTest.java
index b6af3f56d40..d2cd63795f9 100644
--- a/solr/solrj/src/test/org/apache/solr/common/util/EnvUtilsTest.java
+++ b/solr/solrj/src/test/org/apache/solr/common/util/EnvUtilsTest.java
@@ -24,43 +24,24 @@ import org.junit.BeforeClass;
import org.junit.Test;
public class EnvUtilsTest extends SolrTestCase {
+
+ private static final Map<String, String> ENV =
+ Map.of(
+ "SOLR_HOME", "/home/solr",
+ "SOLR_PORT", "8983",
+ "SOLR_HOST", "localhost",
+ "SOLR_LOG_LEVEL", "INFO",
+ "SOLR_BOOLEAN", "true",
+ "SOLR_LONG", "1234567890",
+ "SOLR_COMMASEP", "one,two, three",
+ "SOLR_JSON_LIST", "[\"one\", \"two\", \"three\"]",
+ "SOLR_ALWAYS_ON_TRACE_ID", "true",
+ "SOLR_STR_WITH_NEWLINE", "foo\nbar,baz");
+
@BeforeClass
public static void beforeClass() throws Exception {
// Make a map of some common Solr environment variables for testing, and
initialize EnvUtils
- EnvUtils.setEnvs(
- Map.of(
- "SOLR_HOME", "/home/solr",
- "SOLR_PORT", "8983",
- "SOLR_HOST", "localhost",
- "SOLR_LOG_LEVEL", "INFO",
- "SOLR_BOOLEAN", "true",
- "SOLR_LONG", "1234567890",
- "SOLR_COMMASEP", "one,two, three",
- "SOLR_JSON_LIST", "[\"one\", \"two\", \"three\"]",
- "SOLR_ALWAYS_ON_TRACE_ID", "true",
- "SOLR_STR_WITH_NEWLINE", "foo\nbar,baz"));
- EnvUtils.init(true);
- }
-
- @Test
- public void testGetEnv() {
- assertEquals("INFO", EnvUtils.getEnv("SOLR_LOG_LEVEL"));
-
- assertNull(EnvUtils.getEnv("SOLR_NONEXIST"));
- assertEquals("myString", EnvUtils.getEnv("SOLR_NONEXIST", "myString"));
-
- assertTrue(EnvUtils.getEnvAsBool("SOLR_BOOLEAN"));
- assertFalse(EnvUtils.getEnvAsBool("SOLR_BOOLEAN_NONEXIST", false));
-
- assertEquals("1234567890", EnvUtils.getEnv("SOLR_LONG"));
- assertEquals(1234567890L, EnvUtils.getEnvAsLong("SOLR_LONG"));
- assertEquals(987L, EnvUtils.getEnvAsLong("SOLR_LONG_NONEXIST", 987L));
-
- assertEquals("one,two, three", EnvUtils.getEnv("SOLR_COMMASEP"));
- assertEquals(List.of("one", "two", "three"),
EnvUtils.getEnvAsList("SOLR_COMMASEP"));
- assertEquals(List.of("one", "two", "three"),
EnvUtils.getEnvAsList("SOLR_JSON_LIST"));
- assertEquals(List.of("fallback"), EnvUtils.getEnvAsList("SOLR_MISSING",
List.of("fallback")));
- assertEquals(List.of("foo\nbar", "baz"),
EnvUtils.getEnvAsList("SOLR_STR_WITH_NEWLINE"));
+ EnvUtils.init(true, ENV);
}
@Test
@@ -96,10 +77,10 @@ public class EnvUtilsTest extends SolrTestCase {
@Test
public void testEnvsWithCustomKeyNameMappings() {
// These have different names than the environment variables
- assertEquals(EnvUtils.getEnv("SOLR_HOME"),
EnvUtils.getProperty("solr.solr.home"));
- assertEquals(EnvUtils.getEnv("SOLR_PORT"),
EnvUtils.getProperty("jetty.port"));
- assertEquals(EnvUtils.getEnv("SOLR_HOST"), EnvUtils.getProperty("host"));
- assertEquals(EnvUtils.getEnv("SOLR_LOGS_DIR"),
EnvUtils.getProperty("solr.log.dir"));
+ assertEquals(ENV.get("SOLR_HOME"), EnvUtils.getProperty("solr.solr.home"));
+ assertEquals(ENV.get("SOLR_PORT"), EnvUtils.getProperty("jetty.port"));
+ assertEquals(ENV.get("SOLR_HOST"), EnvUtils.getProperty("host"));
+ assertEquals(ENV.get("SOLR_LOGS_DIR"),
EnvUtils.getProperty("solr.log.dir"));
}
@Test
@@ -111,10 +92,10 @@ public class EnvUtilsTest extends SolrTestCase {
@Test
public void testOverwrite() {
EnvUtils.setProperty("solr.overwrite", "original");
- EnvUtils.setEnv("SOLR_OVERWRITE", "overwritten");
- EnvUtils.init(false);
+ var env2 = Map.of("SOLR_OVERWRITE", "overwritten");
+ EnvUtils.init(false, env2);
assertEquals("original", EnvUtils.getProperty("solr.overwrite"));
- EnvUtils.init(true);
+ EnvUtils.init(true, env2);
assertEquals("overwritten", EnvUtils.getProperty("solr.overwrite"));
}
}
diff --git
a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java
b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java
index 37d200f0f02..d410c3abeb4 100644
--- a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java
+++ b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java
@@ -101,7 +101,7 @@ public class SolrCloudTestCase extends SolrTestCaseJ4 {
* beforeClass method.
*/
public static boolean isPRS() {
- return EnvUtils.getEnvAsBool(PRS_DEFAULT_PROP, false);
+ return EnvUtils.getPropertyAsBool(PRS_DEFAULT_PROP, false);
}
/**
@@ -134,7 +134,7 @@ public class SolrCloudTestCase extends SolrTestCaseJ4 {
if (target != null && target.isAnnotationPresent(NoPrs.class)) {
usePrs = false;
} else {
- usePrs = EnvUtils.getEnvAsBool(PRS_DEFAULT_PROP,
LuceneTestCase.random().nextBoolean());
+ usePrs = EnvUtils.getPropertyAsBool(PRS_DEFAULT_PROP,
LuceneTestCase.random().nextBoolean());
}
System.setProperty(PRS_DEFAULT_PROP, usePrs ? "true" : "false");
}
