diff --git a/core/src/main/java/org/elasticsearch/common/settings/Setting.java b/core/src/main/java/org/elasticsearch/common/settings/Setting.java index 606326dd295..44e440500ff 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -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 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)); } /** diff --git a/core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index ee770f74756..9fc2ee257a0 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -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"); diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java index b2654092067..1c1f06f5914 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -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)); } } diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java index a8b5824f8ec..bc6afda9a01 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java @@ -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")); + } + } }