Skip to content

Commit

Permalink
Remove the single argument Environment constructor (#27235)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
droberts195 committed Nov 4, 2017
1 parent 964016e commit 749c3ec
Show file tree
Hide file tree
Showing 28 changed files with 129 additions and 72 deletions.
4 changes: 0 additions & 4 deletions core/src/main/java/org/elasticsearch/env/Environment.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions core/src/test/java/org/elasticsearch/env/EnvironmentTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")));
}

Expand All @@ -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")));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand Down Expand Up @@ -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++) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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 {
Expand All @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand All @@ -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()
Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand All @@ -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();
Expand Down
Loading

0 comments on commit 749c3ec

Please sign in to comment.