From 7a7f112e890aa55a3b5e180a762ce47cfb7f14da Mon Sep 17 00:00:00 2001 From: David Pilato Date: Sun, 28 Feb 2016 11:21:20 +0100 Subject: [PATCH] Check mutually exclusive scopes We want to make sure that a developer does not put more than one scope on a given setting. --- .../common/settings/Setting.java | 12 +++++ .../common/settings/SettingTests.java | 52 +++++++++++++++++++ 2 files changed, 64 insertions(+) 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 ce20c521932..1469d4679cb 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -125,6 +125,18 @@ public class Setting extends ToXContentToBytes { } else { this.properties = EnumSet.copyOf(Arrays.asList(properties)); } + // We validate scope settings. They are mutually exclusive + int numScopes = 0; + for (SettingsProperty property : properties) { + if (property == SettingsProperty.ClusterScope || + property == SettingsProperty.IndexScope || + property == SettingsProperty.NodeScope) { + numScopes++; + } + } + if (numScopes > 1) { + throw new IllegalArgumentException("More than one scope has been added to the setting [" + key + "]"); + } } /** 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 b916783b316..f2c76931729 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -30,6 +30,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; public class SettingTests extends ESTestCase { @@ -400,4 +401,55 @@ public class SettingTests extends ESTestCase { assertEquals(5, integerSetting.get(Settings.builder().put("foo.bar", 5).build()).intValue()); assertEquals(1, integerSetting.get(Settings.EMPTY).intValue()); } + + /** + * Only one single scope can be added to any setting + */ + public void testMutuallyExclusiveScopes() { + // Those should pass + Setting setting = Setting.simpleString("foo.bar", SettingsProperty.ClusterScope); + assertThat(setting.hasClusterScope(), is(true)); + assertThat(setting.hasNodeScope(), is(false)); + assertThat(setting.hasIndexScope(), is(false)); + setting = Setting.simpleString("foo.bar", SettingsProperty.NodeScope); + assertThat(setting.hasNodeScope(), is(true)); + assertThat(setting.hasIndexScope(), is(false)); + assertThat(setting.hasClusterScope(), is(false)); + setting = Setting.simpleString("foo.bar", SettingsProperty.IndexScope); + assertThat(setting.hasIndexScope(), is(true)); + assertThat(setting.hasNodeScope(), is(false)); + assertThat(setting.hasClusterScope(), is(false)); + + // We test the default scope + setting = Setting.simpleString("foo.bar"); + assertThat(setting.hasNodeScope(), is(true)); + assertThat(setting.hasIndexScope(), is(false)); + assertThat(setting.hasClusterScope(), is(false)); + + // Those should fail + try { + Setting.simpleString("foo.bar", SettingsProperty.IndexScope, SettingsProperty.ClusterScope); + fail("Multiple scopes should fail"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("More than one scope has been added to the setting")); + } + try { + Setting.simpleString("foo.bar", SettingsProperty.IndexScope, SettingsProperty.NodeScope); + fail("Multiple scopes should fail"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("More than one scope has been added to the setting")); + } + try { + Setting.simpleString("foo.bar", SettingsProperty.ClusterScope, SettingsProperty.NodeScope); + fail("Multiple scopes should fail"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("More than one scope has been added to the setting")); + } + try { + Setting.simpleString("foo.bar", SettingsProperty.IndexScope, SettingsProperty.ClusterScope, SettingsProperty.NodeScope); + fail("Multiple scopes should fail"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("More than one scope has been added to the setting")); + } + } }