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

Move validation logic to `SettingsModule.registerSetting`

This commit is contained in:
David Pilato 2016-03-04 21:44:01 +01:00
parent 5c2ca3c9f5
commit bde97e1d9c
4 changed files with 43 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
core/src/main/java/org/elasticsearch/common/settings/Setting.java
View File

@ -38,7 +38,9 @@ import org.elasticsearch.common.xcontent.XContentType;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.EnumSet;
import java.util.Enumeration;
import java.util.List;
import java.util.Objects;
import java.util.function.BiConsumer;
@ -119,18 +121,11 @@ public class Setting<T> extends ToXContentToBytes {
this.key = key;
this.defaultValue = defaultValue;
this.parser = parser;
// We validate scope settings. We should have one and only one.
int numScopes = 0;
for (Property property : properties) {
if (property == Property.NodeScope ||
property == Property.IndexScope) {
numScopes++;
}
if (properties.length == 0) {
this.properties = EnumSet.noneOf(Property.class);
} else {
this.properties = EnumSet.copyOf(Arrays.asList(properties));
}
if (numScopes != 1) {
throw new IllegalArgumentException("Zero or more than one scope has been added to the setting [" + key + "]");
}
this.properties = EnumSet.copyOf(Arrays.asList(properties));
}
/**

5
core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java
View File

@ -76,6 +76,11 @@ public class SettingsModule extends AbstractModule {
registerSettingsFilter(setting.getKey());
}
}
// We validate scope settings. We should have one and only one scope.
if (setting.hasNodeScope() && setting.hasIndexScope()) {
throw new IllegalArgumentException("More than one scope has been added to the setting [" + setting.getKey() + "]");
}
if (setting.hasNodeScope()) {
if (nodeSettings.containsKey(setting.getKey())) {
throw new IllegalArgumentException("Cannot register setting [" + setting.getKey() + "] twice");

22
core/src/test/java/org/elasticsearch/common/settings/SettingTests.java
View File

@ -435,18 +435,14 @@ public class SettingTests extends ESTestCase {
assertThat(setting.hasIndexScope(), is(true));
assertThat(setting.hasNodeScope(), is(false));
// Those should fail
try {
Setting.simpleString("foo.bar");
fail("Zero scope should fail");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage(), containsString("Zero or more than one scope has been added to the setting"));
}
try {
Setting.simpleString("foo.bar", Property.IndexScope, Property.NodeScope);
fail("Multiple scopes should fail");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage(), containsString("Zero or more than one scope has been added to the setting"));
}
// We accept settings with no scope but they will be rejected when we register with SettingsModule.registerSetting
setting = Setting.simpleString("foo.bar");
assertThat(setting.hasIndexScope(), is(false));
assertThat(setting.hasNodeScope(), is(false));
// We accept settings with multiple scopes but they will be rejected when we register with SettingsModule.registerSetting
setting = Setting.simpleString("foo.bar", Property.IndexScope, Property.NodeScope);
assertThat(setting.hasIndexScope(), is(true));
assertThat(setting.hasNodeScope(), is(true));
}
}

23
core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java
View File

@ -22,6 +22,9 @@ package org.elasticsearch.common.settings;
import org.elasticsearch.common.inject.ModuleTestCase;
import org.elasticsearch.common.settings.Setting.Property;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
public class SettingsModuleTests extends ModuleTestCase {
public void testValidate() {
@ -149,4 +152,24 @@ public class SettingsModuleTests extends ModuleTestCase {
assertInstanceBinding(module, SettingsFilter.class, (s) -> s.filter(settings).getAsMap().get("bar.baz").equals("false"));
}
public void testMutuallyExclusiveScopes() {
new SettingsModule(Settings.EMPTY).registerSetting(Setting.simpleString("foo.bar", Property.NodeScope));
new SettingsModule(Settings.EMPTY).registerSetting(Setting.simpleString("foo.bar", Property.IndexScope));
// Those should fail
try {
new SettingsModule(Settings.EMPTY).registerSetting(Setting.simpleString("foo.bar"));
fail("No scope should fail");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage(), containsString("No scope found for setting"));
}
// Those should fail
try {
new SettingsModule(Settings.EMPTY).registerSetting(Setting.simpleString("foo.bar", Property.IndexScope, Property.NodeScope));
fail("Multiple scopes should fail");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage(), containsString("More than one scope has been added to the setting"));
}
}
}