fix list/array settings

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) {

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<T> extends ToXContentToBytes {
     private final String key;
-    private final Function<Settings, String> defaultValue;
+    protected final Function<Settings, String> defaultValue;
     private final Function<String, T> parser;
     private final boolean dynamic;
     private final Scope scope;
@@ -127,7 +131,7 @@ public class Setting<T> 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<T> extends ToXContentToBytes {
         return timeSetting(key, defaultValue, TimeValue.timeValueMillis(0), dynamic, scope);
     }
 
+    public static <T> Setting<List<T>> listSetting(String key, List<String> defaultStringValue, Function<String, T> singleValueParser, boolean dynamic, Scope scope) {
+        Function<String, List<T>> 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<T> 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<List<T>>(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<Settings> groupSetting(String key, boolean dynamic, Scope scope) {
         if (key.endsWith(".") == false) {
             throw new IllegalArgumentException("key must end with a '.'");

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.
      */

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.*;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
 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<TransportServic
 
     // tracer log
 
-    public static final Setting<String[]> TRACE_LOG_INCLUDE_SETTING = new Setting<>("transport.tracer.include", "", Strings::splitStringByCommaToArray , true, Setting.Scope.CLUSTER);
+    public static final Setting<List<String>> TRACE_LOG_INCLUDE_SETTING = Setting.listSetting("transport.tracer.include", Collections.emptyList(), (s) -> s, true, Setting.Scope.CLUSTER);
-    public static final Setting<String[]> 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<List<String>> 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<TransportServic
         super(settings);
         this.transport = transport;
         this.threadPool = threadPool;
-        this.tracerLogInclude = TRACE_LOG_INCLUDE_SETTING.get(settings);
+        setTracerLogInclude(TRACE_LOG_INCLUDE_SETTING.get(settings));
-        this.tracelLogExclude = TRACE_LOG_EXCLUDE_SETTING.get(settings);
+        setTracerLogExclude(TRACE_LOG_EXCLUDE_SETTING.get(settings));
         tracerLog = Loggers.getLogger(logger, ".tracer");
         adapter = createAdapter();
     }
@@ -134,15 +131,15 @@ public class TransportService extends AbstractLifecycleComponent<TransportServic
     @Inject(optional = true)
     public void setDynamicSettings(ClusterSettings clusterSettings) {
         clusterSettings.addSettingsUpdateConsumer(TRACE_LOG_INCLUDE_SETTING, this::setTracerLogInclude);
-        clusterSettings.addSettingsUpdateConsumer(TRACE_LOG_EXCLUDE_SETTING, this::setTracelLogExclude);
+        clusterSettings.addSettingsUpdateConsumer(TRACE_LOG_EXCLUDE_SETTING, this::setTracerLogExclude);
     }
 
-    void setTracerLogInclude(String[] tracerLogInclude) {
+    void setTracerLogInclude(List<String> tracerLogInclude) {
-        this.tracerLogInclude = tracerLogInclude;
+        this.tracerLogInclude = tracerLogInclude.toArray(Strings.EMPTY_ARRAY);
     }
 
-    void setTracelLogExclude(String[] tracelLogExclude) {
+    void setTracerLogExclude(List<String> tracelLogExclude) {
-        this.tracelLogExclude = tracelLogExclude;
+        this.tracelLogExclude = tracelLogExclude.toArray(Strings.EMPTY_ARRAY);
     }
     @Override
     protected void doStart() {

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<List<String>> listSetting = Setting.listSetting("foo.bar", Arrays.asList("foo,bar"), (s) -> s.toString(), true, Setting.Scope.CLUSTER);
+        List<String> value = listSetting.get(Settings.EMPTY);
+        assertEquals(1, value.size());
+        assertEquals("foo,bar", value.get(0));
+
+        List<String> 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<List<String>> 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<List<Integer>> otherSettings = Setting.listSetting("foo.bar", Collections.emptyList(), Integer::parseInt, true, Setting.Scope.CLUSTER);
+        List<Integer> defaultValue = otherSettings.get(Settings.EMPTY);
+        assertEquals(0, defaultValue.size());
+        List<Integer> 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());
+        }
+
+    }
 }