diff --git a/src/main/java/com/knowledgepixels/query/TripleStore.java b/src/main/java/com/knowledgepixels/query/TripleStore.java index f167ba7..7c68a80 100644 --- a/src/main/java/com/knowledgepixels/query/TripleStore.java +++ b/src/main/java/com/knowledgepixels/query/TripleStore.java @@ -1,6 +1,5 @@ package com.knowledgepixels.query; -import org.apache.commons.exec.environment.EnvironmentUtils; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpUriRequest; @@ -79,10 +78,14 @@ public static TripleStore get() { } private TripleStore() throws IOException { - Map env = EnvironmentUtils.getProcEnvironment(); - endpointBase = env.get("ENDPOINT_BASE"); + // Read directly from the JVM's in-memory environment block rather than via + // Apache Commons Exec's EnvironmentUtils.getProcEnvironment(), which forks a + // subprocess and can throw under memory pressure / a restart storm — leaving + // the singleton uninitialised. Same fragile mechanism that caused issue #117 + // on the per-nanopub hot path (see Utils.getEnvString); retired here too. + endpointBase = System.getenv("ENDPOINT_BASE"); log.info("Endpoint base: {}", endpointBase); - endpointType = env.get("ENDPOINT_TYPE"); + endpointType = System.getenv("ENDPOINT_TYPE"); getRepository("empty"); // Make sure empty repo exists } diff --git a/src/main/java/com/knowledgepixels/query/Utils.java b/src/main/java/com/knowledgepixels/query/Utils.java index 5b28daf..1781534 100644 --- a/src/main/java/com/knowledgepixels/query/Utils.java +++ b/src/main/java/com/knowledgepixels/query/Utils.java @@ -9,7 +9,6 @@ import java.util.Map; import java.util.Properties; -import org.apache.commons.exec.environment.EnvironmentUtils; import org.apache.commons.lang3.StringUtils; import org.apache.http.client.config.CookieSpecs; import org.apache.http.client.config.RequestConfig; @@ -165,17 +164,35 @@ public static List getObjectsForPattern(RepositoryConnection conn, IRI gr * @return environment variable value */ public static String getEnvString(String envVarName, String defaultValue) { - try { - String s = EnvironmentUtils.getProcEnvironment().get(envVarName); - if (s != null && !s.isEmpty()) { - return s; - } - } catch (Exception ex) { - log.info("Could not get environment variable", ex); + String s = getRawEnv(envVarName); + if (s != null && !s.isEmpty()) { + return s; } return defaultValue; } + /** + * Reads a single environment variable from the JVM's in-memory environment + * block. The previous implementation used Apache Commons Exec's + * {@code EnvironmentUtils.getProcEnvironment()}, which forks a subprocess + * ({@code env}/{@code sh}) and parses its stdout on every call. That is neither + * thread-safe nor robust under load, and since {@link #getEnvString} is on the + * per-nanopub hot path ({@link FeatureFlags#fullRepoEnabled()} and friends, + * called from the 4-thread loading pool) an intermittently mangled read made + * {@code fullRepoEnabled()} evaluate to {@code false} and silently skip the + * full-repo write for individual nanopubs — leaving {@code full} behind + * {@code meta} undetectably (issue #117). + * + *

Extracted as a package-private seam because {@link System} static methods + * cannot be mocked directly with Mockito; tests stub this instead. + * + * @param envVarName environment variable name + * @return the raw value, or {@code null} if unset + */ + static String getRawEnv(String envVarName) { + return System.getenv(envVarName); + } + /** * Returns the system environment variable content as an integer for the given environment variable name. * diff --git a/src/test/java/com/knowledgepixels/query/UtilsTest.java b/src/test/java/com/knowledgepixels/query/UtilsTest.java index dfbaf6f..47047e8 100644 --- a/src/test/java/com/knowledgepixels/query/UtilsTest.java +++ b/src/test/java/com/knowledgepixels/query/UtilsTest.java @@ -1,7 +1,6 @@ package com.knowledgepixels.query; import com.github.jsonldjava.shaded.com.google.common.hash.Hashing; -import org.apache.commons.exec.environment.EnvironmentUtils; import org.apache.http.client.config.RequestConfig; import org.eclipse.rdf4j.model.IRI; import org.eclipse.rdf4j.model.Value; @@ -14,7 +13,6 @@ import org.mockito.Mockito; import org.nanopub.vocabulary.NPA; -import java.io.IOException; import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.util.HashMap; @@ -128,24 +126,20 @@ void getObjectsForPattern() { @Test void getEnvString() { final String defaultValue = "default"; + final String mockValueString = "value"; - Map mockEnv = new HashMap<>(); - String mockValueString = "value"; - mockEnv.put("EXISTING_VAR", mockValueString); - mockEnv.put("EMPTY_VAR", ""); - mockEnv.put("NULL_VAR", null); - - try (MockedStatic mockedEnvUtils = Mockito.mockStatic(EnvironmentUtils.class)) { - mockedEnvUtils.when(EnvironmentUtils::getProcEnvironment).thenReturn(mockEnv); + // getEnvString now reads the env via the Utils.getRawEnv seam (System.getenv + // cannot be mocked directly — see #117). CALLS_REAL_METHODS keeps getEnvString real. + try (MockedStatic mockedUtils = Mockito.mockStatic(Utils.class, Mockito.CALLS_REAL_METHODS)) { + mockedUtils.when(() -> Utils.getRawEnv("EXISTING_VAR")).thenReturn(mockValueString); + mockedUtils.when(() -> Utils.getRawEnv("EMPTY_VAR")).thenReturn(""); + mockedUtils.when(() -> Utils.getRawEnv("NULL_VAR")).thenReturn(null); + mockedUtils.when(() -> Utils.getRawEnv("NON_EXISTING_VAR")).thenReturn(null); assertEquals(mockValueString, Utils.getEnvString("EXISTING_VAR", defaultValue)); assertEquals(defaultValue, Utils.getEnvString("NON_EXISTING_VAR", defaultValue)); assertEquals(defaultValue, Utils.getEnvString("EMPTY_VAR", defaultValue)); assertEquals(defaultValue, Utils.getEnvString("NULL_VAR", defaultValue)); - - mockedEnvUtils.when(EnvironmentUtils::getProcEnvironment).thenThrow(IOException.class); - assertEquals(defaultValue, Utils.getEnvString("EXISTING_VAR", defaultValue)); - } } @@ -156,14 +150,12 @@ void getEnvInt() { final int validInt = Integer.parseInt(validIntValue); final String invalidIntValue = "not_an_int"; - Map mockEnv = new HashMap<>(); - mockEnv.put("VALID_INT", validIntValue); - mockEnv.put("INVALID_INT", invalidIntValue); - mockEnv.put("EMPTY_VAR", ""); - mockEnv.put("NULL_VAR", null); - - try (MockedStatic mockedEnvUtils = Mockito.mockStatic(EnvironmentUtils.class)) { - mockedEnvUtils.when(EnvironmentUtils::getProcEnvironment).thenReturn(mockEnv); + try (MockedStatic mockedUtils = Mockito.mockStatic(Utils.class, Mockito.CALLS_REAL_METHODS)) { + mockedUtils.when(() -> Utils.getRawEnv("VALID_INT")).thenReturn(validIntValue); + mockedUtils.when(() -> Utils.getRawEnv("INVALID_INT")).thenReturn(invalidIntValue); + mockedUtils.when(() -> Utils.getRawEnv("EMPTY_VAR")).thenReturn(""); + mockedUtils.when(() -> Utils.getRawEnv("NULL_VAR")).thenReturn(null); + mockedUtils.when(() -> Utils.getRawEnv("NON_EXISTING_VAR")).thenReturn(null); assertEquals(validInt, Utils.getEnvInt("VALID_INT", defaultValue)); assertEquals(defaultValue, Utils.getEnvInt("NON_EXISTING_VAR", defaultValue));