honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #17324 from jasontedor/settings-cleanup 

Settings loader cleanup
This commit is contained in:
Jason Tedor 2016-03-24 10:44:12 -04:00
parent 6ede670524 f522fa1df3
commit 4b17a5c73f
5 changed files with 90 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
buildSrc/src/main/resources/checkstyle_suppressions.xml
View File

@ -380,7 +380,6 @@
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]IndexScopedSettings.java" checks="LineLength" /> <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]IndexScopedSettings.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]Setting.java" checks="LineLength" /> <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]Setting.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]Settings.java" checks="LineLength" /> <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]Settings.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]loader[/\\]PropertiesSettingsLoader.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]loader[/\\]XContentSettingsLoader.java" checks="LineLength" /> <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]loader[/\\]XContentSettingsLoader.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]unit[/\\]ByteSizeValue.java" checks="LineLength" /> <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]unit[/\\]ByteSizeValue.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]unit[/\\]TimeValue.java" checks="LineLength" /> <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]unit[/\\]TimeValue.java" checks="LineLength" />
@ -1078,8 +1077,6 @@
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]rounding[/\\]TimeZoneRoundingTests.java" checks="LineLength" /> <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]rounding[/\\]TimeZoneRoundingTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]ScopedSettingsTests.java" checks="LineLength" /> <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]ScopedSettingsTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]SettingTests.java" checks="LineLength" /> <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]SettingTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]loader[/\\]JsonSettingsLoaderTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]settings[/\\]loader[/\\]YamlSettingsLoaderTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]transport[/\\]BoundTransportAddressTests.java" checks="LineLength" /> <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]transport[/\\]BoundTransportAddressTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]unit[/\\]DistanceUnitTests.java" checks="LineLength" /> <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]unit[/\\]DistanceUnitTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]unit[/\\]FuzzinessTests.java" checks="LineLength" /> <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]unit[/\\]FuzzinessTests.java" checks="LineLength" />

47
core/src/main/java/org/elasticsearch/common/settings/loader/PropertiesSettingsLoader.java
View File

@ -24,10 +24,12 @@ import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.io.FastStringReader; import org.elasticsearch.common.io.FastStringReader;
import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamInput;
import java.io.Closeable;
import java.io.IOException; import java.io.IOException;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.Properties; import java.util.Properties;
import java.util.function.Supplier;
/** /**
* Settings loader that loads (parses) the settings in a properties format. * Settings loader that loads (parses) the settings in a properties format.
@ -36,42 +38,49 @@ public class PropertiesSettingsLoader implements SettingsLoader {
@Override @Override
public Map<String, String> load(String source) throws IOException { public Map<String, String> load(String source) throws IOException {
Properties props = new NoDuplicatesProperties(); return load(() -> new FastStringReader(source), (reader, props) -> props.load(reader));
FastStringReader reader = new FastStringReader(source);
try {
props.load(reader);
Map<String, String> result = new HashMap<>();
for (Map.Entry entry : props.entrySet()) {
result.put((String) entry.getKey(), (String) entry.getValue());
}
return result;
} finally {
IOUtils.closeWhileHandlingException(reader);
}
} }
@Override @Override
public Map<String, String> load(byte[] source) throws IOException { public Map<String, String> load(byte[] source) throws IOException {
Properties props = new NoDuplicatesProperties(); return load(() -> StreamInput.wrap(source), (inStream, props) -> props.load(inStream));
StreamInput stream = StreamInput.wrap(source); }
private final <T extends Closeable> Map<String, String> load(
Supplier<T> supplier,
IOExceptionThrowingBiConsumer<T, Properties> properties
) throws IOException {
T t = null;
try { try {
props.load(stream); t = supplier.get();
Map<String, String> result = new HashMap<>(); final Properties props = new NoDuplicatesProperties();
properties.accept(t, props);
final Map<String, String> result = new HashMap<>();
for (Map.Entry entry : props.entrySet()) { for (Map.Entry entry : props.entrySet()) {
result.put((String) entry.getKey(), (String) entry.getValue()); result.put((String) entry.getKey(), (String) entry.getValue());
} }
return result; return result;
} finally { } finally {
IOUtils.closeWhileHandlingException(stream); IOUtils.closeWhileHandlingException(t);
} }
} }
@FunctionalInterface
private interface IOExceptionThrowingBiConsumer<T, U> {
void accept(T t, U u) throws IOException;
}
class NoDuplicatesProperties extends Properties { class NoDuplicatesProperties extends Properties {
@Override @Override
public synchronized Object put(Object key, Object value) { public synchronized Object put(Object key, Object value) {
Object previousValue = super.put(key, value); final Object previousValue = super.put(key, value);
if (previousValue != null) { if (previousValue != null) {
throw new ElasticsearchParseException("duplicate settings key [{}] found, previous value [{}], current value [{}]", key, previousValue, value); throw new ElasticsearchParseException(
"duplicate settings key [{}] found, previous value [{}], current value [{}]",
key,
previousValue,
value
);
} }
return previousValue; return previousValue;
} }

32
core/src/test/java/org/elasticsearch/common/settings/loader/JsonSettingsLoaderTests.java
View File

@ -28,13 +28,11 @@ import static org.elasticsearch.common.settings.Settings.settingsBuilder;
import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
/**
*
*/
public class JsonSettingsLoaderTests extends ESTestCase { public class JsonSettingsLoaderTests extends ESTestCase {
public void testSimpleJsonSettings() throws Exception { public void testSimpleJsonSettings() throws Exception {
String json = "/org/elasticsearch/common/settings/loader/test-settings.json"; final String json = "/org/elasticsearch/common/settings/loader/test-settings.json";
Settings settings = settingsBuilder() final Settings settings = settingsBuilder()
.loadFromStream(json, getClass().getResourceAsStream(json)) .loadFromStream(json, getClass().getResourceAsStream(json))
.build(); .build();
@ -51,21 +49,23 @@ public class JsonSettingsLoaderTests extends ESTestCase {
} }
public void testDuplicateKeysThrowsException() { public void testDuplicateKeysThrowsException() {
String json = "{\"foo\":\"bar\",\"foo\":\"baz\"}"; final String json = "{\"foo\":\"bar\",\"foo\":\"baz\"}";
try { final SettingsException e = expectThrows(SettingsException.class, () -> settingsBuilder().loadFromSource(json).build());
settingsBuilder() assertEquals(e.getCause().getClass(), ElasticsearchParseException.class);
.loadFromSource(json) assertThat(
.build(); e.toString(),
fail("expected exception"); containsString("duplicate settings key [foo] " +
} catch (SettingsException e) { "found at line number [1], " +
assertEquals(e.getCause().getClass(), ElasticsearchParseException.class); "column number [20], " +
assertTrue(e.toString().contains("duplicate settings key [foo] found at line number [1], column number [20], previous value [bar], current value [baz]")); "previous value [bar], " +
} "current value [baz]"));
} }
public void testNullValuedSettingThrowsException() { public void testNullValuedSettingThrowsException() {
final String json = "{\"foo\":null}"; final String json = "{\"foo\":null}";
final ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () -> new JsonSettingsLoader(false).load(json)); final ElasticsearchParseException e =
expectThrows(ElasticsearchParseException.class, () -> new JsonSettingsLoader(false).load(json));
assertThat(e.toString(), containsString("null-valued setting found for key [foo] found at line number [1], column number [8]")); assertThat(e.toString(), containsString("null-valued setting found for key [foo] found at line number [1], column number [8]"));
} }
} }

32
core/src/test/java/org/elasticsearch/common/settings/loader/PropertiesSettingsLoaderTests.java
View File

@ -21,33 +21,37 @@ package org.elasticsearch.common.settings.loader;
import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.ESTestCase;
import org.junit.Before;
import java.io.IOException; import java.io.IOException;
import java.nio.charset.Charset; import java.nio.charset.Charset;
public class PropertiesSettingsLoaderTests extends ESTestCase { public class PropertiesSettingsLoaderTests extends ESTestCase {
private PropertiesSettingsLoader loader;
@Before
public void setUp() throws Exception {
super.setUp();
loader = new PropertiesSettingsLoader();
}
public void testDuplicateKeyFromStringThrowsException() throws IOException { public void testDuplicateKeyFromStringThrowsException() throws IOException {
PropertiesSettingsLoader loader = new PropertiesSettingsLoader(); final ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () -> loader.load("foo=bar\nfoo=baz"));
try { assertEquals(e.getMessage(), "duplicate settings key [foo] found, previous value [bar], current value [baz]");
loader.load("foo=bar\nfoo=baz");
fail("expected exception");
} catch (ElasticsearchParseException e) {
assertEquals(e.getMessage(), "duplicate settings key [foo] found, previous value [bar], current value [baz]");
}
} }
public void testDuplicateKeysFromBytesThrowsException() throws IOException { public void testDuplicateKeysFromBytesThrowsException() throws IOException {
PropertiesSettingsLoader loader = new PropertiesSettingsLoader(); final ElasticsearchParseException e = expectThrows(
try { ElasticsearchParseException.class,
loader.load("foo=bar\nfoo=baz".getBytes(Charset.defaultCharset())); () -> loader.load("foo=bar\nfoo=baz".getBytes(Charset.defaultCharset()))
} catch (ElasticsearchParseException e) { );
assertEquals(e.getMessage(), "duplicate settings key [foo] found, previous value [bar], current value [baz]"); assertEquals(e.getMessage(), "duplicate settings key [foo] found, previous value [bar], current value [baz]");
}
} }
public void testThatNoDuplicatesPropertiesDoesNotAcceptNullValues() { public void testThatNoDuplicatesPropertiesDoesNotAcceptNullValues() {
final PropertiesSettingsLoader loader = new PropertiesSettingsLoader();
final PropertiesSettingsLoader.NoDuplicatesProperties properties = loader.new NoDuplicatesProperties(); final PropertiesSettingsLoader.NoDuplicatesProperties properties = loader.new NoDuplicatesProperties();
expectThrows(NullPointerException.class, () -> properties.put("key", null)); expectThrows(NullPointerException.class, () -> properties.put("key", null));
} }
} }

62
core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java
View File

@ -28,13 +28,11 @@ import static org.elasticsearch.common.settings.Settings.settingsBuilder;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
/**
*
*/
public class YamlSettingsLoaderTests extends ESTestCase { public class YamlSettingsLoaderTests extends ESTestCase {
public void testSimpleYamlSettings() throws Exception { public void testSimpleYamlSettings() throws Exception {
String yaml = "/org/elasticsearch/common/settings/loader/test-settings.yml"; final String yaml = "/org/elasticsearch/common/settings/loader/test-settings.yml";
Settings settings = settingsBuilder() final Settings settings = settingsBuilder()
.loadFromStream(yaml, getClass().getResourceAsStream(yaml)) .loadFromStream(yaml, getClass().getResourceAsStream(yaml))
.build(); .build();
@ -51,45 +49,41 @@ public class YamlSettingsLoaderTests extends ESTestCase {
} }
public void testIndentation() { public void testIndentation() {
String yaml = "/org/elasticsearch/common/settings/loader/indentation-settings.yml"; final String yaml = "/org/elasticsearch/common/settings/loader/indentation-settings.yml";
try { final SettingsException e =
settingsBuilder() expectThrows(
.loadFromStream(yaml, getClass().getResourceAsStream(yaml)) SettingsException.class,
.build(); () -> settingsBuilder().loadFromStream(yaml, getClass().getResourceAsStream(yaml)).build());
fail("Expected SettingsException"); assertThat(e.getMessage(), containsString("Failed to load settings"));
} catch(SettingsException e ) {
assertThat(e.getMessage(), containsString("Failed to load settings"));
}
} }
public void testIndentationWithExplicitDocumentStart() { public void testIndentationWithExplicitDocumentStart() {
String yaml = "/org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml"; final String yaml = "/org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml";
try { final SettingsException e =
settingsBuilder() expectThrows(
.loadFromStream(yaml, getClass().getResourceAsStream(yaml)) SettingsException.class,
.build(); () -> settingsBuilder().loadFromStream(yaml, getClass().getResourceAsStream(yaml)).build());
fail("Expected SettingsException"); assertThat(e.getMessage(), containsString("Failed to load settings"));
} catch (SettingsException e) {
assertThat(e.getMessage(), containsString("Failed to load settings"));
}
} }
public void testDuplicateKeysThrowsException() { public void testDuplicateKeysThrowsException() {
String yaml = "foo: bar\nfoo: baz"; final String yaml = "foo: bar\nfoo: baz";
try { final SettingsException e = expectThrows(SettingsException.class, () -> settingsBuilder().loadFromSource(yaml).build());
settingsBuilder() assertEquals(e.getCause().getClass(), ElasticsearchParseException.class);
.loadFromSource(yaml) assertThat(
.build(); e.toString(),
fail("expected exception"); containsString("duplicate settings key [foo] " +
} catch (SettingsException e) { "found at line number [2], " +
assertEquals(e.getCause().getClass(), ElasticsearchParseException.class); "column number [6], " +
assertTrue(e.toString().contains("duplicate settings key [foo] found at line number [2], column number [6], previous value [bar], current value [baz]")); "previous value [bar], " +
} "current value [baz]"));
} }
public void testNullValuedSettingThrowsException() { public void testNullValuedSettingThrowsException() {
final String yaml = "foo:"; final String yaml = "foo:";
final ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () -> new YamlSettingsLoader(false).load(yaml)); final ElasticsearchParseException e =
expectThrows(ElasticsearchParseException.class, () -> new YamlSettingsLoader(false).load(yaml));
assertThat(e.toString(), containsString("null-valued setting found for key [foo] found at line number [1], column number [5]")); assertThat(e.toString(), containsString("null-valued setting found for key [foo] found at line number [1], column number [5]"));
} }
} }