From 9cf2f42d15675e09ed03613c17356d89502b5623 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 16 Dec 2015 11:52:30 +0100 Subject: [PATCH] fix list/array settings --- .../org/elasticsearch/common/Booleans.java | 3 +- .../common/settings/Setting.java | 56 ++++++++++++++++++- .../common/settings/Settings.java | 2 + .../transport/TransportService.java | 23 ++++---- .../common/settings/SettingTests.java | 56 ++++++++++++++++++- 5 files changed, 122 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/Booleans.java b/core/src/main/java/org/elasticsearch/common/Booleans.java index 6b1b9b016a7..9c5f5746633 100644 --- a/core/src/main/java/org/elasticsearch/common/Booleans.java +++ b/core/src/main/java/org/elasticsearch/common/Booleans.java @@ -84,7 +84,6 @@ public class Booleans { * throws exception if string cannot be parsed to boolean */ public static Boolean parseBooleanExact(String value) { - boolean isFalse = isExplicitFalse(value); if (isFalse) { return false; @@ -94,7 +93,7 @@ public class Booleans { return true; } - throw new IllegalArgumentException("value cannot be parsed to boolean [ true/1/on/yes OR false/0/off/no ] "); + throw new IllegalArgumentException("Failed to parse value [" + value + "] cannot be parsed to boolean [ true/1/on/yes OR false/0/off/no ]"); } public static Boolean parseBoolean(String value, Boolean defaultValue) { 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 ba9573e0bbf..758bff5c6cc 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -18,9 +18,11 @@ */ package org.elasticsearch.common.settings; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.support.ToXContentToBytes; import org.elasticsearch.common.Booleans; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.regex.Regex; @@ -30,6 +32,8 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.*; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Function; @@ -38,7 +42,7 @@ import java.util.function.Function; */ public class Setting extends ToXContentToBytes { private final String key; - private final Function defaultValue; + protected final Function defaultValue; private final Function parser; private final boolean dynamic; private final Scope scope; @@ -127,7 +131,7 @@ public class Setting extends ToXContentToBytes { * Returns the raw (string) settings value. If the setting is not present in the given settings object the default value is returned * instead. This is useful if the value can't be parsed due to an invalid value to access the actual value. */ - public final String getRaw(Settings settings) { + public String getRaw(Settings settings) { return settings.get(key, defaultValue.apply(settings)); } @@ -300,6 +304,54 @@ public class Setting extends ToXContentToBytes { return timeSetting(key, defaultValue, TimeValue.timeValueMillis(0), dynamic, scope); } + public static Setting> listSetting(String key, List defaultStringValue, Function singleValueParser, boolean dynamic, Scope scope) { + Function> parser = (s) -> { + try (XContentParser xContentParser = XContentType.JSON.xContent().createParser(s)){ + XContentParser.Token token = xContentParser.nextToken(); + if (token != XContentParser.Token.START_ARRAY) { + throw new IllegalArgumentException("expected START_ARRAY but got " + token); + } + ArrayList list = new ArrayList<>(); + while ((token = xContentParser.nextToken()) !=XContentParser.Token.END_ARRAY) { + if (token != XContentParser.Token.VALUE_STRING) { + throw new IllegalArgumentException("expected VALUE_STRING but got " + token); + } + list.add(singleValueParser.apply(xContentParser.text())); + } + return list; + } catch (IOException e) { + throw new IllegalArgumentException("failed to parse array", e); + } + }; + return new Setting>(key, arrayToParsableString(defaultStringValue.toArray(Strings.EMPTY_ARRAY)), parser, dynamic, scope) { + + @Override + public String getRaw(Settings settings) { + String[] array = settings.getAsArray(key, null); + + return array == null ? defaultValue.apply(settings) : arrayToParsableString(array); + } + + + }; + } + + private static String arrayToParsableString(String[] array) { + try { + XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent()); + builder.startArray(); + for (String element : array) { + builder.value(element); + } + builder.endArray(); + return builder.string(); + } catch (IOException ex) { + throw new ElasticsearchException(ex); + } + } + + + public static Setting groupSetting(String key, boolean dynamic, Scope scope) { if (key.endsWith(".") == false) { throw new IllegalArgumentException("key must end with a '.'"); diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java index 05f3cb1ff0b..f8dd5d4f1f6 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -597,6 +597,8 @@ public final class Settings implements ToXContent { return result.toArray(new String[result.size()]); } + + /** * Returns group settings for the given setting prefix. */ diff --git a/core/src/main/java/org/elasticsearch/transport/TransportService.java b/core/src/main/java/org/elasticsearch/transport/TransportService.java index 05d5242ac82..916c4863e07 100644 --- a/core/src/main/java/org/elasticsearch/transport/TransportService.java +++ b/core/src/main/java/org/elasticsearch/transport/TransportService.java @@ -42,10 +42,7 @@ import org.elasticsearch.common.util.concurrent.FutureUtils; import org.elasticsearch.threadpool.ThreadPool; import java.io.IOException; -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.atomic.AtomicBoolean; @@ -87,8 +84,8 @@ public class TransportService extends AbstractLifecycleComponent TRACE_LOG_INCLUDE_SETTING = new Setting<>("transport.tracer.include", "", Strings::splitStringByCommaToArray , true, Setting.Scope.CLUSTER); - public static final Setting TRACE_LOG_EXCLUDE_SETTING = new Setting<>("transport.tracer.exclude", "internal:discovery/zen/fd*," + TransportLivenessAction.NAME, Strings::splitStringByCommaToArray , true, Setting.Scope.CLUSTER);; + public static final Setting> TRACE_LOG_INCLUDE_SETTING = Setting.listSetting("transport.tracer.include", Collections.emptyList(), (s) -> s, true, Setting.Scope.CLUSTER); + public static final Setting> TRACE_LOG_EXCLUDE_SETTING = Setting.listSetting("transport.tracer.exclude", Arrays.asList("internal:discovery/zen/fd*", TransportLivenessAction.NAME), (s) -> s, true, Setting.Scope.CLUSTER); private final ESLogger tracerLog; @@ -107,8 +104,8 @@ public class TransportService extends AbstractLifecycleComponent tracerLogInclude) { + this.tracerLogInclude = tracerLogInclude.toArray(Strings.EMPTY_ARRAY); } - void setTracelLogExclude(String[] tracelLogExclude) { - this.tracelLogExclude = tracelLogExclude; + void setTracerLogExclude(List tracelLogExclude) { + this.tracelLogExclude = tracelLogExclude.toArray(Strings.EMPTY_ARRAY); } @Override protected void doStart() { 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 1640cfdd3b5..1895d3ee326 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -24,6 +24,9 @@ import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.test.ESTestCase; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; @@ -72,7 +75,7 @@ public class SettingTests extends ESTestCase { settingUpdater.apply(build, Settings.EMPTY); fail("not a boolean"); } catch (IllegalArgumentException ex) { - assertEquals("Failed to parse value [I am not a boolean] for setting [foo.bar]", ex.getMessage()); + assertEquals("Failed to parse value [I am not a boolean] cannot be parsed to boolean [ true/1/on/yes OR false/0/off/no ]", ex.getMessage()); } } @@ -248,4 +251,55 @@ public class SettingTests extends ESTestCase { assertEquals(1, c.b.intValue()); } + + public void testListSettings() { + Setting> listSetting = Setting.listSetting("foo.bar", Arrays.asList("foo,bar"), (s) -> s.toString(), true, Setting.Scope.CLUSTER); + List value = listSetting.get(Settings.EMPTY); + assertEquals(1, value.size()); + assertEquals("foo,bar", value.get(0)); + + List input = Arrays.asList("test", "test1, test2", "test", ",,,,"); + Settings.Builder builder = Settings.builder().putArray("foo.bar", input.toArray(new String[0])); + value = listSetting.get(builder.build()); + assertEquals(input.size(), value.size()); + assertArrayEquals(value.toArray(new String[0]), input.toArray(new String[0])); + + // try to parse this really annoying format + builder = Settings.builder(); + for (int i = 0; i < input.size(); i++) { + builder.put("foo.bar." + i, input.get(i)); + } + value = listSetting.get(builder.build()); + assertEquals(input.size(), value.size()); + assertArrayEquals(value.toArray(new String[0]), input.toArray(new String[0])); + + AtomicReference> ref = new AtomicReference<>(); + AbstractScopedSettings.SettingUpdater settingUpdater = listSetting.newUpdater(ref::set, logger); + assertTrue(settingUpdater.hasChanged(builder.build(), Settings.EMPTY)); + settingUpdater.apply(builder.build(), Settings.EMPTY); + assertEquals(input.size(), ref.get().size()); + assertArrayEquals(ref.get().toArray(new String[0]), input.toArray(new String[0])); + + settingUpdater.apply(Settings.builder().putArray("foo.bar", "123").build(), builder.build()); + assertEquals(1, ref.get().size()); + assertArrayEquals(ref.get().toArray(new String[0]), new String[] {"123"}); + + settingUpdater.apply(Settings.builder().put("foo.bar", "1,2,3").build(), Settings.builder().putArray("foo.bar", "123").build()); + assertEquals(3, ref.get().size()); + assertArrayEquals(ref.get().toArray(new String[0]), new String[] {"1", "2", "3"}); + + settingUpdater.apply(Settings.EMPTY, Settings.builder().put("foo.bar", "1,2,3").build()); + assertEquals(1, ref.get().size()); + assertEquals("foo,bar", ref.get().get(0)); + + Setting> otherSettings = Setting.listSetting("foo.bar", Collections.emptyList(), Integer::parseInt, true, Setting.Scope.CLUSTER); + List defaultValue = otherSettings.get(Settings.EMPTY); + assertEquals(0, defaultValue.size()); + List intValues = otherSettings.get(Settings.builder().put("foo.bar", "0,1,2,3").build()); + assertEquals(4, intValues.size()); + for (int i = 0; i < intValues.size(); i++) { + assertEquals(i, intValues.get(i).intValue()); + } + + } }