Remove the single argument Environment constructor (#27235)

Only tests should use the single argument Environment constructor.  To
enforce this the single arg Environment constructor has been replaced with
a test framework factory method.

Production code (beyond initial Bootstrap) should always use the same
Environment object that Node.getEnvironment() returns.  This Environment
is also available via dependency injection.

Author: David Roberts

core/src/main/java/org/elasticsearch/env/Environment.java

@@ -85,10 +85,6 @@ public class Environment {
     /** Path to the temporary file directory used by the JDK */
     private final Path tmpFile = PathUtils.get(System.getProperty("java.io.tmpdir"));
 
-    public Environment(Settings settings) {
-        this(settings, null);
-    }
-
     public Environment(final Settings settings, final Path configPath) {
         final Path homeFile;
         if (PATH_HOME_SETTING.exists(settings)) {

core/src/test/java/org/elasticsearch/action/admin/indices/TransportAnalyzeActionTests.java

@@ -28,6 +28,7 @@
 import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.analysis.AbstractCharFilterFactory;
 import org.elasticsearch.index.analysis.AbstractTokenFilterFactory;
@@ -74,7 +75,7 @@ public void setUp() throws Exception {
                 .put("index.analysis.normalizer.my_normalizer.type", "custom")
                 .putList("index.analysis.normalizer.my_normalizer.filter", "lowercase").build();
         IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("index", indexSettings);
-        environment = new Environment(settings);
+        environment = TestEnvironment.newEnvironment(settings);
         AnalysisPlugin plugin = new AnalysisPlugin() {
             class MockFactory extends AbstractTokenFilterFactory {
                 MockFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) {

core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java

@@ -22,7 +22,6 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.FileSystem;
-import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -35,6 +34,7 @@
 import org.elasticsearch.cli.CommandTestCase;
 import org.elasticsearch.common.io.PathUtilsForTesting;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.env.TestEnvironment;
 import org.junit.After;
 import org.junit.Before;
 
@@ -70,7 +70,7 @@ public static Environment setupEnv(boolean posix, List<FileSystem> fileSystems)
         PathUtilsForTesting.installMock(fs); // restored by restoreFileSystem in ESTestCase
         Path home = fs.getPath("/", "test-home");
         Files.createDirectories(home.resolve("config"));
-        return new Environment(Settings.builder().put("path.home", home).build());
+        return TestEnvironment.newEnvironment(Settings.builder().put("path.home", home).build());
     }
 
     KeyStoreWrapper createKeystore(String password, String... settings) throws Exception {

core/src/test/java/org/elasticsearch/env/EnvironmentTests.java

@@ -43,7 +43,7 @@ public Environment newEnvironment(Settings settings) throws IOException {
                 .put(settings)
                 .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath())
                 .putList(Environment.PATH_DATA_SETTING.getKey(), tmpPaths()).build();
-        return new Environment(build);
+        return new Environment(build, null);
     }
 
     public void testRepositoryResolution() throws IOException {
@@ -76,21 +76,21 @@ public void testRepositoryResolution() throws IOException {
     public void testPathDataWhenNotSet() {
         final Path pathHome = createTempDir().toAbsolutePath();
         final Settings settings = Settings.builder().put("path.home", pathHome).build();
-        final Environment environment = new Environment(settings);
+        final Environment environment = new Environment(settings, null);
         assertThat(environment.dataFiles(), equalTo(new Path[]{pathHome.resolve("data")}));
     }
 
     public void testPathDataNotSetInEnvironmentIfNotSet() {
         final Settings settings = Settings.builder().put("path.home", createTempDir().toAbsolutePath()).build();
         assertFalse(Environment.PATH_DATA_SETTING.exists(settings));
-        final Environment environment = new Environment(settings);
+        final Environment environment = new Environment(settings, null);
         assertFalse(Environment.PATH_DATA_SETTING.exists(environment.settings()));
     }
 
     public void testPathLogsWhenNotSet() {
         final Path pathHome = createTempDir().toAbsolutePath();
         final Settings settings = Settings.builder().put("path.home", pathHome).build();
-        final Environment environment = new Environment(settings);
+        final Environment environment = new Environment(settings, null);
         assertThat(environment.logsFile(), equalTo(pathHome.resolve("logs")));
     }
 
@@ -111,7 +111,7 @@ public void testConfigPath() {
     public void testConfigPathWhenNotSet() {
         final Path pathHome = createTempDir().toAbsolutePath();
         final Settings settings = Settings.builder().put("path.home", pathHome).build();
-        final Environment environment = new Environment(settings);
+        final Environment environment = new Environment(settings, null);
         assertThat(environment.configFile(), equalTo(pathHome.resolve("config")));
     }
 

core/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java

@@ -80,12 +80,12 @@ public void testNodeLockSingleEnvironment() throws IOException {
 
         // Reuse the same location and attempt to lock again
         IllegalStateException ex =
-            expectThrows(IllegalStateException.class, () -> new NodeEnvironment(settings, new Environment(settings)));
+            expectThrows(IllegalStateException.class, () -> new NodeEnvironment(settings, TestEnvironment.newEnvironment(settings)));
         assertThat(ex.getMessage(), containsString("failed to obtain node lock"));
 
         // Close the environment that holds the lock and make sure we can get the lock after release
         env.close();
-        env = new NodeEnvironment(settings, new Environment(settings));
+        env = new NodeEnvironment(settings, TestEnvironment.newEnvironment(settings));
         assertThat(env.nodeDataPaths(), arrayWithSize(dataPaths.size()));
 
         for (int i = 0; i < dataPaths.size(); i++) {
@@ -120,7 +120,7 @@ public void testNodeLockMultipleEnvironment() throws IOException {
         final Settings settings = buildEnvSettings(Settings.builder().put("node.max_local_storage_nodes", 2).build());
         final NodeEnvironment first = newNodeEnvironment(settings);
         List<String> dataPaths = Environment.PATH_DATA_SETTING.get(settings);
-        NodeEnvironment second = new NodeEnvironment(settings, new Environment(settings));
+        NodeEnvironment second = new NodeEnvironment(settings, TestEnvironment.newEnvironment(settings));
         assertEquals(first.nodeDataPaths().length, dataPaths.size());
         assertEquals(second.nodeDataPaths().length, dataPaths.size());
         for (int i = 0; i < dataPaths.size(); i++) {
@@ -477,7 +477,7 @@ public NodeEnvironment newNodeEnvironment() throws IOException {
     @Override
     public NodeEnvironment newNodeEnvironment(Settings settings) throws IOException {
         Settings build = buildEnvSettings(settings);
-        return new NodeEnvironment(build, new Environment(build));
+        return new NodeEnvironment(build, TestEnvironment.newEnvironment(build));
     }
 
     public Settings buildEnvSettings(Settings settings) {
@@ -492,7 +492,7 @@ public NodeEnvironment newNodeEnvironment(String[] dataPaths, Settings settings)
                 .put(settings)
                 .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString())
                 .putList(Environment.PATH_DATA_SETTING.getKey(), dataPaths).build();
-        return new NodeEnvironment(build, new Environment(build));
+        return new NodeEnvironment(build, TestEnvironment.newEnvironment(build));
     }
 
     public NodeEnvironment newNodeEnvironment(String[] dataPaths, String sharedDataPath, Settings settings) throws IOException {
@@ -501,6 +501,6 @@ public NodeEnvironment newNodeEnvironment(String[] dataPaths, String sharedDataP
                 .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString())
                 .put(Environment.PATH_SHARED_DATA_SETTING.getKey(), sharedDataPath)
                 .putList(Environment.PATH_DATA_SETTING.getKey(), dataPaths).build();
-        return new NodeEnvironment(build, new Environment(build));
+        return new NodeEnvironment(build, TestEnvironment.newEnvironment(build));
     }
 }

core/src/test/java/org/elasticsearch/index/IndexModuleTests.java

@@ -42,6 +42,7 @@
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.NodeEnvironment;
 import org.elasticsearch.env.ShardLock;
+import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.index.analysis.AnalysisRegistry;
 import org.elasticsearch.index.cache.query.DisabledQueryCache;
 import org.elasticsearch.index.cache.query.IndexQueryCache;
@@ -118,7 +119,7 @@ public void setUp() throws Exception {
         indicesQueryCache = new IndicesQueryCache(settings);
         indexSettings = IndexSettingsModule.newIndexSettings("foo", settings);
         index = indexSettings.getIndex();
-        environment = new Environment(settings);
+        environment = TestEnvironment.newEnvironment(settings);
         emptyAnalysisRegistry = new AnalysisRegistry(environment, emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap(),
                 emptyMap(), emptyMap(), emptyMap());
         threadPool = new TestThreadPool("test");

core/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java

@@ -30,6 +30,7 @@
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.indices.analysis.AnalysisModule;
 import org.elasticsearch.indices.analysis.AnalysisModule.AnalysisProvider;
@@ -56,8 +57,8 @@ private static AnalyzerProvider<?> analyzerProvider(final String name) {
     }
 
     private static AnalysisRegistry emptyAnalysisRegistry(Settings settings) {
-        return new AnalysisRegistry(new Environment(settings), emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap(),
-                emptyMap(), emptyMap());
+        return new AnalysisRegistry(TestEnvironment.newEnvironment(settings), emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap(),
+                emptyMap(), emptyMap(), emptyMap());
     }
 
     private static IndexSettings indexSettingsOfCurrentVersion(Settings.Builder settings) {
@@ -157,8 +158,8 @@ public Map<String, AnalysisProvider<TokenFilterFactory>> getTokenFilters() {
                 return singletonMap("mock", MockFactory::new);
             }
         };
-        IndexAnalyzers indexAnalyzers = new AnalysisModule(new Environment(settings), singletonList(plugin)).getAnalysisRegistry()
-                .build(idxSettings);
+        IndexAnalyzers indexAnalyzers = new AnalysisModule(TestEnvironment.newEnvironment(settings),
+                singletonList(plugin)).getAnalysisRegistry().build(idxSettings);
 
         // This shouldn't contain English stopwords
         try (NamedAnalyzer custom_analyser = indexAnalyzers.get("custom_analyzer_with_camel_case")) {

core/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java

@@ -22,6 +22,7 @@
 import org.apache.lucene.analysis.CharArraySet;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.test.ESTestCase;
 
 import java.io.BufferedWriter;
@@ -61,7 +62,7 @@ public void testParseNonExistingFile() {
         Settings nodeSettings = Settings.builder()
             .put("foo.bar_path", tempDir.resolve("foo.dict"))
             .put(Environment.PATH_HOME_SETTING.getKey(), tempDir).build();
-        Environment env = new Environment(nodeSettings);
+        Environment env = TestEnvironment.newEnvironment(nodeSettings);
         IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
             () -> Analysis.getWordList(env, nodeSettings, "foo.bar"));
         assertEquals("IOException while reading foo.bar_path: " +  tempDir.resolve("foo.dict").toString(), ex.getMessage());
@@ -80,7 +81,7 @@ public void testParseFalseEncodedFile() throws IOException {
             writer.write(new byte[]{(byte) 0xff, 0x00, 0x00}); // some invalid UTF-8
             writer.write('\n');
         }
-        Environment env = new Environment(nodeSettings);
+        Environment env = TestEnvironment.newEnvironment(nodeSettings);
         IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
             () -> Analysis.getWordList(env, nodeSettings, "foo.bar"));
         assertEquals("Unsupported character encoding detected while reading foo.bar_path: " + tempDir.resolve("foo.dict").toString()
@@ -101,7 +102,7 @@ public void testParseWordList() throws IOException {
             writer.write("world");
             writer.write('\n');
         }
-        Environment env = new Environment(nodeSettings);
+        Environment env = TestEnvironment.newEnvironment(nodeSettings);
         List<String> wordList = Analysis.getWordList(env, nodeSettings, "foo.bar");
         assertEquals(Arrays.asList("hello", "world"), wordList);
 

core/src/test/java/org/elasticsearch/index/shard/NewPathForShardTests.java

@@ -27,14 +27,14 @@
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.NodeEnvironment;
 import org.elasticsearch.env.NodeEnvironment.NodePath;
+import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.IndexSettingsModule;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 
 import java.io.IOException;
-import java.math.BigInteger;
 import java.nio.file.FileStore;
 import java.nio.file.FileSystem;
 import java.nio.file.Files;
@@ -178,7 +178,7 @@ public void testSelectNewPathForShard() throws Exception {
         Settings settings = Settings.builder()
             .put(Environment.PATH_HOME_SETTING.getKey(), path)
             .putList(Environment.PATH_DATA_SETTING.getKey(), paths).build();
-        NodeEnvironment nodeEnv = new NodeEnvironment(settings, new Environment(settings));
+        NodeEnvironment nodeEnv = new NodeEnvironment(settings, TestEnvironment.newEnvironment(settings));
 
         // Make sure all our mocking above actually worked:
         NodePath[] nodePaths = nodeEnv.nodePaths();
@@ -233,7 +233,7 @@ public void testSelectNewPathForShardEvenly() throws Exception {
         Settings settings = Settings.builder()
             .put(Environment.PATH_HOME_SETTING.getKey(), path)
             .putList(Environment.PATH_DATA_SETTING.getKey(), paths).build();
-        NodeEnvironment nodeEnv = new NodeEnvironment(settings, new Environment(settings));
+        NodeEnvironment nodeEnv = new NodeEnvironment(settings, TestEnvironment.newEnvironment(settings));
 
         // Make sure all our mocking above actually worked:
         NodePath[] nodePaths = nodeEnv.nodePaths();
@@ -290,7 +290,7 @@ public void testGettingPathWithMostFreeSpace() throws Exception {
         Settings settings = Settings.builder()
             .put(Environment.PATH_HOME_SETTING.getKey(), path)
             .putList(Environment.PATH_DATA_SETTING.getKey(), paths).build();
-        NodeEnvironment nodeEnv = new NodeEnvironment(settings, new Environment(settings));
+        NodeEnvironment nodeEnv = new NodeEnvironment(settings, TestEnvironment.newEnvironment(settings));
 
         aFileStore.usableSpace = 100000;
         bFileStore.usableSpace = 1000;
@@ -315,7 +315,7 @@ public void testTieBreakWithMostShards() throws Exception {
         Settings settings = Settings.builder()
             .put(Environment.PATH_HOME_SETTING.getKey(), path)
             .putList(Environment.PATH_DATA_SETTING.getKey(), paths).build();
-        NodeEnvironment nodeEnv = new NodeEnvironment(settings, new Environment(settings));
+        NodeEnvironment nodeEnv = new NodeEnvironment(settings, TestEnvironment.newEnvironment(settings));
 
         // Make sure all our mocking above actually worked:
         NodePath[] nodePaths = nodeEnv.nodePaths();