From 3509ceaa80cd70d9f891f9ef5a09f53651958169 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 28 Jan 2016 16:16:24 +0100 Subject: [PATCH] simplify dynamic key setting and make it generally useful --- .../common/logging/ESLoggerFactory.java | 23 +---------- .../common/settings/Setting.java | 39 +++++++++++++++++-- .../common/settings/SettingTests.java | 16 ++++++++ 3 files changed, 53 insertions(+), 25 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java b/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java index 840c4ac203f..4fdde3db895 100644 --- a/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java +++ b/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java @@ -37,28 +37,7 @@ import java.util.regex.Pattern; public abstract class ESLoggerFactory { public static final Setting LOG_DEFAULT_LEVEL_SETTING = new Setting<>("logger.level", LogLevel.INFO.name(), LogLevel::parse, false, Setting.Scope.CLUSTER); - public static final Setting LOG_LEVEL_SETTING = new Setting("logger.", LogLevel.INFO.name(), LogLevel::parse, true, Setting.Scope.CLUSTER) { - private final Pattern KEY_PATTERN = Pattern.compile("^logger[.](?:[-\\w]+[.])*[-\\w]+$$"); - - @Override - protected boolean isGroupSetting() { - return true; - } - - @Override - public boolean match(String toTest) { - return KEY_PATTERN.matcher(toTest).matches(); - } - - @Override - public Setting getConcreteSetting(String key) { - if (match(key)) { - return new Setting<>(key, LogLevel.WARN.name(), LogLevel::parse, true, Setting.Scope.CLUSTER); - } else { - throw new IllegalArgumentException("key must match setting but didn't ["+key +"]"); - } - } - }; + public static final Setting LOG_LEVEL_SETTING = Setting.dynamicKeySetting("logger.", LogLevel.INFO.name(), LogLevel::parse, true, Setting.Scope.CLUSTER); private static volatile ESLoggerFactory defaultFactory = new JdkESLoggerFactory(); 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 e12d5cc02a1..96b0a238a7f 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -111,7 +111,7 @@ public class Setting extends ToXContentToBytes { * rather than a single value. The key, see {@link #getKey()}, in contrast to non-group settings is a prefix like cluster.store. * that matches all settings with this prefix. */ - protected boolean isGroupSetting() { + boolean isGroupSetting() { return false; } @@ -201,6 +201,7 @@ public class Setting extends ToXContentToBytes { } public Setting getConcreteSetting(String key) { + assert key.startsWith(this.getKey()) : "was " + key + " expected: " + getKey(); // we use startsWith here since the key might be foo.bar.0 if it's an array return this; } @@ -453,8 +454,6 @@ public class Setting extends ToXContentToBytes { } } - - public static Setting groupSetting(String key, boolean dynamic, Scope scope) { if (key.endsWith(".") == false) { throw new IllegalArgumentException("key must end with a '.'"); @@ -562,4 +561,38 @@ public class Setting extends ToXContentToBytes { public int hashCode() { return Objects.hash(key); } + + /** + * This setting type allows to validate settings that have the same type and a common prefix. For instance feature.${type}=[true|false] + * can easily be added with this setting. Yet, dynamic key settings don't support updaters our of the box unless {@link #getConcreteSetting(String)} + * is used to pull the updater. + */ + public static Setting dynamicKeySetting(String key, String defaultValue, Function parser, boolean dynamic, Scope scope) { + return new Setting(key, defaultValue, parser, dynamic, scope) { + + @Override + boolean isGroupSetting() { + return true; + } + + @Override + public boolean match(String toTest) { + return toTest.startsWith(getKey()); + } + + @Override + AbstractScopedSettings.SettingUpdater newUpdater(Consumer consumer, ESLogger logger, Consumer validator) { + throw new UnsupportedOperationException("dynamic settings can't be updated use #getConcreteSetting for updating"); + } + + @Override + public Setting getConcreteSetting(String key) { + if (match(key)) { + return new Setting<>(key, defaultValue, parser, dynamic, scope); + } else { + throw new IllegalArgumentException("key must match setting but didn't ["+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 e5ae9bd6296..c4386179af9 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -346,6 +346,22 @@ public class SettingTests extends ESTestCase { assertFalse(listSetting.match("foo_bar.1")); assertTrue(listSetting.match("foo.bar")); assertTrue(listSetting.match("foo.bar." + randomIntBetween(0,10000))); + } + public void testDynamicKeySetting() { + Setting setting = Setting.dynamicKeySetting("foo.", "false", Boolean::parseBoolean, false, Setting.Scope.CLUSTER); + assertTrue(setting.hasComplexMatcher()); + assertTrue(setting.match("foo.bar")); + assertFalse(setting.match("foo")); + Setting concreteSetting = setting.getConcreteSetting("foo.bar"); + assertTrue(concreteSetting.get(Settings.builder().put("foo.bar", "true").build())); + assertFalse(concreteSetting.get(Settings.builder().put("foo.baz", "true").build())); + + try { + setting.getConcreteSetting("foo"); + fail(); + } catch (IllegalArgumentException ex) { + assertEquals("key must match setting but didn't [foo]", ex.getMessage()); + } } }