Merge remote-tracking branch 'danielmitterdorfer/simplify-azure-settings'

core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java

@@ -32,7 +32,7 @@ public abstract class ESLoggerFactory {
     public static final Setting<LogLevel> LOG_DEFAULT_LEVEL_SETTING =
             new Setting<>("logger.level", LogLevel.INFO.name(), LogLevel::parse, false, Setting.Scope.CLUSTER);
     public static final Setting<LogLevel> LOG_LEVEL_SETTING =
-            Setting.dynamicKeySetting("logger.", LogLevel.INFO.name(), LogLevel::parse, true, Setting.Scope.CLUSTER);
+            Setting.prefixKeySetting("logger.", LogLevel.INFO.name(), LogLevel::parse, true, Setting.Scope.CLUSTER);
 
     public static ESLogger getLogger(String prefix, String name) {
         prefix = prefix == null ? null : prefix.intern();

core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

@@ -296,12 +296,25 @@ public abstract class AbstractScopedSettings extends AbstractComponent {
         }
         for (Map.Entry<String, Setting<?>> entry : complexMatchers.entrySet()) {
             if (entry.getValue().match(key)) {
+                assert assertMatcher(key, 1);
                 return entry.getValue().getConcreteSetting(key);
             }
         }
         return null;
     }
 
+    private boolean assertMatcher(String key, int numComplexMatchers) {
+        List<Setting<?>> list = new ArrayList<>();
+        for (Map.Entry<String, Setting<?>> entry : complexMatchers.entrySet()) {
+            if (entry.getValue().match(key)) {
+                list.add(entry.getValue().getConcreteSetting(key));
+            }
+        }
+        assert list.size() == numComplexMatchers : "Expected " + numComplexMatchers + " complex matchers to match key [" +
+            key + "] but got: "  + list.toString();
+        return true;
+    }
+
     /**
      * Returns <code>true</code> if the setting for the given key is dynamically updateable. Otherwise <code>false</code>.
      */

core/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -66,7 +66,7 @@ import java.util.stream.Collectors;
  * </pre>
  */
 public class Setting<T> extends ToXContentToBytes {
-    private final String key;
+    private final Key key;
     protected final Function<Settings, String> defaultValue;
     private final Function<String, T> parser;
     private final boolean dynamic;
@@ -80,7 +80,7 @@ public class Setting<T> extends ToXContentToBytes {
      * @param dynamic true iff this setting can be dynamically updateable
      * @param scope the scope of this setting
      */
-    public Setting(String key, Function<Settings, String> defaultValue, Function<String, T> parser, boolean dynamic, Scope scope) {
+    public Setting(Key key, Function<Settings, String> defaultValue, Function<String, T> parser, boolean dynamic, Scope scope) {
         assert parser.apply(defaultValue.apply(Settings.EMPTY)) != null || this.isGroupSetting(): "parser returned null";
         this.key = key;
         this.defaultValue = defaultValue;
@@ -89,6 +89,18 @@ public class Setting<T> extends ToXContentToBytes {
         this.scope = scope;
     }
 
+    /**
+     * Creates a new Setting instance
+     * @param key the settings key for this setting.
+     * @param defaultValue a default value function that returns the default values string representation.
+     * @param parser a parser that parses the string rep into a complex datatype.
+     * @param dynamic true iff this setting can be dynamically updateable
+     * @param scope the scope of this setting
+     */
+    public Setting(String key, Function<Settings, String> defaultValue, Function<String, T> parser, boolean dynamic, Scope scope) {
+        this(new SimpleKey(key), defaultValue, parser, dynamic, scope);
+    }
+
     /**
      * Creates a new Setting instance
      * @param key the settings key for this setting.
@@ -109,6 +121,13 @@ public class Setting<T> extends ToXContentToBytes {
      * @see #isGroupSetting()
      */
     public final String getKey() {
+        return key.toString();
+    }
+
+    /**
+     * Returns the original representation of a setting key.
+     */
+    public final Key getRawKey() {
         return key;
     }
 
@@ -159,7 +178,7 @@ public class Setting<T> extends ToXContentToBytes {
      * Returns <code>true</code> iff this setting is present in the given settings object. Otherwise <code>false</code>
      */
     public final boolean exists(Settings settings) {
-        return settings.get(key) != null;
+        return settings.get(getKey()) != null;
     }
 
     /**
@@ -186,7 +205,7 @@ public class Setting<T> extends ToXContentToBytes {
      * instead. This is useful if the value can't be parsed due to an invalid value to access the actual value.
      */
     public String getRaw(Settings settings) {
-        return settings.get(key, defaultValue.apply(settings));
+        return settings.get(getKey(), defaultValue.apply(settings));
     }
 
     /**
@@ -194,14 +213,14 @@ public class Setting<T> extends ToXContentToBytes {
      * given key is part of the settings group.
      * @see #isGroupSetting()
      */
-    public boolean match(String toTest) {
+    public final boolean match(String toTest) {
-        return key.equals(toTest);
+        return key.match(toTest);
     }
 
     @Override
     public final XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
-        builder.field("key", key);
+        builder.field("key", key.toString());
         builder.field("type", scope.name());
         builder.field("dynamic", dynamic);
         builder.field("is_group_setting", isGroupSetting());
@@ -387,6 +406,14 @@ public class Setting<T> extends ToXContentToBytes {
         return value;
     }
 
+    public static TimeValue parseTimeValue(String s, TimeValue minValue, String key) {
+        TimeValue timeValue = TimeValue.parseTimeValue(s, null, key);
+        if (timeValue.millis() < minValue.millis()) {
+            throw new IllegalArgumentException("Failed to parse value [" + s + "] for setting [" + key + "] must be >= " + minValue);
+        }
+        return timeValue;
+    }
+
     public static Setting<Integer> intSetting(String key, int defaultValue, boolean dynamic, Scope scope) {
         return intSetting(key, defaultValue, Integer.MIN_VALUE, dynamic, scope);
     }
@@ -431,19 +458,13 @@ public class Setting<T> extends ToXContentToBytes {
         Function<String, List<T>> parser = (s) ->
                 parseableStringToList(s).stream().map(singleValueParser).collect(Collectors.toList());
 
-        return new Setting<List<T>>(key, (s) -> arrayToParsableString(defaultStringValue.apply(s).toArray(Strings.EMPTY_ARRAY)), parser, dynamic, scope) {
+        return new Setting<List<T>>(new ListKey(key), (s) -> arrayToParsableString(defaultStringValue.apply(s).toArray(Strings.EMPTY_ARRAY)), parser, dynamic, scope) {
-            private final Pattern pattern = Pattern.compile(Pattern.quote(key)+"(\\.\\d+)?");
             @Override
             public String getRaw(Settings settings) {
-                String[] array = settings.getAsArray(key, null);
+                String[] array = settings.getAsArray(getKey(), null);
                 return array == null ? defaultValue.apply(settings) : arrayToParsableString(array);
             }
 
-            @Override
-            public boolean match(String toTest) {
-                return pattern.matcher(toTest).matches();
-            }
-
             @Override
             boolean hasComplexMatcher() {
                 return true;
@@ -486,11 +507,7 @@ public class Setting<T> extends ToXContentToBytes {
     }
 
     public static Setting<Settings> groupSetting(String key, boolean dynamic, Scope scope) {
-        if (key.endsWith(".") == false) {
+        return new Setting<Settings>(new GroupKey(key), (s) -> "", (s) -> null, dynamic, scope) {
-            throw new IllegalArgumentException("key must end with a '.'");
-        }
-        return new Setting<Settings>(key, "", (s) -> null, dynamic, scope) {
-
             @Override
             public boolean isGroupSetting() {
                 return true;
@@ -498,12 +515,7 @@ public class Setting<T> extends ToXContentToBytes {
 
             @Override
             public Settings get(Settings settings) {
-                return settings.getByPrefix(key);
+                return settings.getByPrefix(getKey());
-            }
-
-            @Override
-            public boolean match(String toTest) {
-                return Regex.simpleMatch(key + "*", toTest);
             }
 
             @Override
@@ -549,13 +561,7 @@ public class Setting<T> extends ToXContentToBytes {
     }
 
     public static Setting<TimeValue> timeSetting(String key, Function<Settings, String> defaultValue, TimeValue minValue, boolean dynamic, Scope scope) {
-        return new Setting<>(key, defaultValue, (s) -> {
+        return new Setting<>(key, defaultValue, (s) -> parseTimeValue(s, minValue, key), dynamic, scope);
-            TimeValue timeValue = TimeValue.parseTimeValue(s, null, key);
-            if (timeValue.millis() < minValue.millis()) {
-                throw new IllegalArgumentException("Failed to parse value [" + s + "] for setting [" + key + "] must be >= " + minValue);
-            }
-            return timeValue;
-        }, dynamic, scope);
     }
 
     public static Setting<TimeValue> timeSetting(String key, TimeValue defaultValue, TimeValue minValue, boolean dynamic, Scope scope) {
@@ -595,10 +601,27 @@ public class Setting<T> extends ToXContentToBytes {
 
     /**
      * 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)}
+     * can easily be added with this setting. Yet, prefix key settings don't support updaters out of the box unless
-     * is used to pull the updater.
+     * {@link #getConcreteSetting(String)} is used to pull the updater.
      */
-    public static <T> Setting<T> dynamicKeySetting(String key, String defaultValue, Function<String, T> parser, boolean dynamic, Scope scope) {
+    public static <T> Setting<T> prefixKeySetting(String prefix, String defaultValue, Function<String, T> parser, boolean dynamic, Scope scope) {
+        return affixKeySetting(AffixKey.withPrefix(prefix), (s) -> defaultValue, parser, dynamic, scope);
+    }
+
+    /**
+     * This setting type allows to validate settings that have the same type and a common prefix and suffix. For instance
+     * storage.${backend}.enable=[true|false] can easily be added with this setting. Yet, adfix key settings don't support updaters
+     * out of the box unless {@link #getConcreteSetting(String)} is used to pull the updater.
+     */
+    public static <T> Setting<T> adfixKeySetting(String prefix, String suffix, Function<Settings, String> defaultValue, Function<String, T> parser, boolean dynamic, Scope scope) {
+        return affixKeySetting(AffixKey.withAdfix(prefix, suffix), defaultValue, parser, dynamic, scope);
+    }
+
+    public static <T> Setting<T> adfixKeySetting(String prefix, String suffix, String defaultValue, Function<String, T> parser, boolean dynamic, Scope scope) {
+        return adfixKeySetting(prefix, suffix, (s) -> defaultValue, parser, dynamic, scope);
+    }
+
+    public static <T> Setting<T> affixKeySetting(AffixKey key, Function<Settings, String> defaultValue, Function<String, T> parser, boolean dynamic, Scope scope) {
         return new Setting<T>(key, defaultValue, parser, dynamic, scope) {
 
             @Override
@@ -606,14 +629,9 @@ public class Setting<T> extends ToXContentToBytes {
                 return true;
             }
 
-            @Override
-            public boolean match(String toTest) {
-                return toTest.startsWith(getKey());
-            }
-
             @Override
             AbstractScopedSettings.SettingUpdater<T> newUpdater(Consumer<T> consumer, ESLogger logger, Consumer<T> validator) {
-                throw new UnsupportedOperationException("dynamic settings can't be updated use #getConcreteSetting for updating");
+                throw new UnsupportedOperationException("Affix settings can't be updated. Use #getConcreteSetting for updating.");
             }
 
             @Override
@@ -621,9 +639,145 @@ public class Setting<T> extends ToXContentToBytes {
                 if (match(key)) {
                     return new Setting<>(key, defaultValue, parser, dynamic, scope);
                 } else {
-                    throw new IllegalArgumentException("key must match setting but didn't ["+key +"]");
+                    throw new IllegalArgumentException("key [" + key + "] must match [" + getKey() + "] but didn't.");
                 }
             }
         };
     }
+
+
+    public interface Key {
+        boolean match(String key);
+    }
+
+    public static class SimpleKey implements Key {
+        protected final String key;
+
+        public SimpleKey(String key) {
+            this.key = key;
+        }
+
+        @Override
+        public boolean match(String key) {
+            return this.key.equals(key);
+        }
+
+        @Override
+        public String toString() {
+            return key;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+            SimpleKey simpleKey = (SimpleKey) o;
+            return Objects.equals(key, simpleKey.key);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(key);
+        }
+    }
+
+    public static final class GroupKey extends SimpleKey {
+        public GroupKey(String key) {
+            super(key);
+            if (key.endsWith(".") == false) {
+                throw new IllegalArgumentException("key must end with a '.'");
+            }
+        }
+
+        @Override
+        public boolean match(String toTest) {
+            return Regex.simpleMatch(key + "*", toTest);
+        }
+    }
+
+    public static final class ListKey extends SimpleKey {
+        private final Pattern pattern;
+
+        public ListKey(String key) {
+            super(key);
+            this.pattern = Pattern.compile(Pattern.quote(key) + "(\\.\\d+)?");
+        }
+
+        @Override
+        public boolean match(String toTest) {
+            return pattern.matcher(toTest).matches();
+        }
+    }
+
+    public static final class AffixKey implements Key {
+        public static AffixKey withPrefix(String prefix) {
+            return new AffixKey(prefix, null);
+        }
+
+        public static AffixKey withAdfix(String prefix, String suffix) {
+            return new AffixKey(prefix, suffix);
+        }
+
+        private final String prefix;
+        private final String suffix;
+
+        public AffixKey(String prefix, String suffix) {
+            assert prefix != null || suffix != null: "Either prefix or suffix must be non-null";
+            this.prefix = prefix;
+            this.suffix = suffix;
+        }
+
+        @Override
+        public boolean match(String key) {
+            boolean match = true;
+            if (prefix != null) {
+                match = key.startsWith(prefix);
+            }
+            if (suffix != null) {
+                match = match && key.endsWith(suffix);
+            }
+            return match;
+        }
+
+        public SimpleKey toConcreteKey(String missingPart) {
+            StringBuilder key = new StringBuilder();
+            if (prefix != null) {
+                key.append(prefix);
+            }
+            key.append(missingPart);
+            if (suffix != null) {
+                key.append(".");
+                key.append(suffix);
+            }
+            return new SimpleKey(key.toString());
+        }
+
+        @Override
+        public String toString() {
+            StringBuilder sb = new StringBuilder();
+            if (prefix != null) {
+                sb.append(prefix);
+            }
+            if (suffix != null) {
+                sb.append("*");
+                sb.append(suffix);
+                sb.append(".");
+            }
+            return sb.toString();
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+            AffixKey that = (AffixKey) o;
+            return Objects.equals(prefix, that.prefix) &&
+                Objects.equals(suffix, that.suffix);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(prefix, suffix);
+        }
+    }
 }

core/src/test/java/org/elasticsearch/common/settings/SettingTests.java

@@ -27,7 +27,6 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 
 public class SettingTests extends ESTestCase {
@@ -349,8 +348,8 @@ public class SettingTests extends ESTestCase {
         assertTrue(listSetting.match("foo.bar." + randomIntBetween(0,10000)));
     }
 
-    public void testDynamicKeySetting() {
+    public void testPrefixKeySetting() {
-        Setting<Boolean> setting = Setting.dynamicKeySetting("foo.", "false", Boolean::parseBoolean, false, Setting.Scope.CLUSTER);
+        Setting<Boolean> setting = Setting.prefixKeySetting("foo.", "false", Boolean::parseBoolean, false, Setting.Scope.CLUSTER);
         assertTrue(setting.hasComplexMatcher());
         assertTrue(setting.match("foo.bar"));
         assertFalse(setting.match("foo"));
@@ -362,7 +361,28 @@ public class SettingTests extends ESTestCase {
             setting.getConcreteSetting("foo");
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals("key must match setting but didn't [foo]", ex.getMessage());
+            assertEquals("key [foo] must match [foo.] but didn't.", ex.getMessage());
+        }
+    }
+
+    public void testAdfixKeySetting() {
+        Setting<Boolean> setting = Setting.adfixKeySetting("foo", "enable", "false", Boolean::parseBoolean, false, Setting.Scope.CLUSTER);
+        assertTrue(setting.hasComplexMatcher());
+        assertTrue(setting.match("foo.bar.enable"));
+        assertTrue(setting.match("foo.baz.enable"));
+        assertTrue(setting.match("foo.bar.baz.enable"));
+        assertFalse(setting.match("foo.bar"));
+        assertFalse(setting.match("foo.bar.baz.enabled"));
+        assertFalse(setting.match("foo"));
+        Setting<Boolean> concreteSetting = setting.getConcreteSetting("foo.bar.enable");
+        assertTrue(concreteSetting.get(Settings.builder().put("foo.bar.enable", "true").build()));
+        assertFalse(concreteSetting.get(Settings.builder().put("foo.baz.enable", "true").build()));
+
+        try {
+            setting.getConcreteSetting("foo");
+            fail();
+        } catch (IllegalArgumentException ex) {
+            assertEquals("key [foo] must match [foo*enable.] but didn't.", ex.getMessage());
         }
     }
 

plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageSettings.java

@@ -21,29 +21,50 @@ package org.elasticsearch.cloud.azure.storage;
 
 import org.elasticsearch.cloud.azure.storage.AzureStorageService.Storage;
 import org.elasticsearch.common.collect.Tuple;
-import org.elasticsearch.common.logging.ESLogger;
-import org.elasticsearch.common.logging.ESLoggerFactory;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.settings.SettingsException;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.repositories.RepositorySettings;
 
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.function.Function;
+
+public final class AzureStorageSettings {
+    private static final String TIMEOUT_SUFFIX = "timeout";
+    private static final String ACCOUNT_SUFFIX = "account";
+    private static final String KEY_SUFFIX = "key";
+    private static final String DEFAULT_SUFFIX = "default";
+
+    private static final Setting.AffixKey TIMEOUT_KEY = Setting.AffixKey.withAdfix(Storage.PREFIX, TIMEOUT_SUFFIX);
+
+    private static final Setting<TimeValue> TIMEOUT_SETTING = Setting.affixKeySetting(
+        TIMEOUT_KEY,
+        (s) -> Storage.TIMEOUT_SETTING.get(s).toString(),
+        (s) -> Setting.parseTimeValue(s, TimeValue.timeValueSeconds(-1), TIMEOUT_KEY.toString()),
+        false,
+        Setting.Scope.CLUSTER);
+    private static final Setting<String> ACCOUNT_SETTING = Setting.adfixKeySetting(Storage.PREFIX, ACCOUNT_SUFFIX, "", Function.identity(), false, Setting.Scope.CLUSTER);
+    private static final Setting<String> KEY_SETTING = Setting.adfixKeySetting(Storage.PREFIX, KEY_SUFFIX, "", Function.identity(), false, Setting.Scope.CLUSTER);
+    private static final Setting<Boolean> DEFAULT_SETTING = Setting.adfixKeySetting(Storage.PREFIX, DEFAULT_SUFFIX, "false", Boolean::valueOf, false, Setting.Scope.CLUSTER);
 
-public class AzureStorageSettings {
-    private static ESLogger logger = ESLoggerFactory.getLogger(AzureStorageSettings.class.getName());
 
     private final String name;
     private final String account;
     private final String key;
     private final TimeValue timeout;
+    private final boolean activeByDefault;
 
-    public AzureStorageSettings(String name, String account, String key, TimeValue timeout) {
+    public AzureStorageSettings(String name, String account, String key, TimeValue timeout, boolean activeByDefault) {
         this.name = name;
         this.account = account;
         this.key = key;
         this.timeout = timeout;
+        this.activeByDefault = activeByDefault;
     }
 
     public String getName() {
@@ -62,12 +83,17 @@ public class AzureStorageSettings {
         return timeout;
     }
 
+    public boolean isActiveByDefault() {
+        return activeByDefault;
+    }
+
     @Override
     public String toString() {
         final StringBuilder sb = new StringBuilder("AzureStorageSettings{");
         sb.append("name='").append(name).append('\'');
         sb.append(", account='").append(account).append('\'');
         sb.append(", key='").append(key).append('\'');
+        sb.append(", activeByDefault='").append(activeByDefault).append('\'');
         sb.append(", timeout=").append(timeout);
         sb.append('}');
         return sb.toString();
@@ -79,49 +105,70 @@ public class AzureStorageSettings {
      * @return A tuple with v1 = primary storage and v2 = secondary storage
      */
     public static Tuple<AzureStorageSettings, Map<String, AzureStorageSettings>> parse(Settings settings) {
-        AzureStorageSettings primaryStorage = null;
+        List<AzureStorageSettings> storageSettings = createStorageSettings(settings);
-        Map<String, AzureStorageSettings> secondaryStorage = new HashMap<>();
+        return Tuple.tuple(getPrimary(storageSettings), getSecondaries(storageSettings));
+    }
 
-        TimeValue globalTimeout = Storage.TIMEOUT_SETTING.get(settings);
+    private static List<AzureStorageSettings> createStorageSettings(Settings settings) {
+        Setting<Settings> storageGroupSetting = Setting.groupSetting(Storage.PREFIX, false, Setting.Scope.CLUSTER);
+        // ignore global timeout which has the same prefix but does not belong to any group
+        Settings groups = storageGroupSetting.get(settings.filter((k) -> k.equals(Storage.TIMEOUT_SETTING.getKey()) == false));
+        List<AzureStorageSettings> storageSettings = new ArrayList<>();
+        for (String groupName : groups.getAsGroups().keySet()) {
+            storageSettings.add(
+                new AzureStorageSettings(
+                    groupName,
+                    getValue(settings, groupName, ACCOUNT_SETTING),
+                    getValue(settings, groupName, KEY_SETTING),
+                    getValue(settings, groupName, TIMEOUT_SETTING),
+                    getValue(settings, groupName, DEFAULT_SETTING))
+            );
+        }
+        return storageSettings;
+    }
 
-        Settings storageSettings = settings.getByPrefix(Storage.PREFIX);
+    private static <T> T getValue(Settings settings, String groupName, Setting<T> setting) {
-        if (storageSettings != null) {
+        Setting.AffixKey k = (Setting.AffixKey) setting.getRawKey();
-            Map<String, Object> asMap = storageSettings.getAsStructuredMap();
+        String fullKey = k.toConcreteKey(groupName).toString();
-            for (Map.Entry<String, Object> storage : asMap.entrySet()) {
+        return setting.getConcreteSetting(fullKey).get(settings);
-                if (storage.getValue() instanceof Map) {
+    }
-                    @SuppressWarnings("unchecked")
+
-                    Map<String, String> map = (Map) storage.getValue();
+    private static AzureStorageSettings getPrimary(List<AzureStorageSettings> settings) {
-                    TimeValue timeout = TimeValue.parseTimeValue(map.get("timeout"), globalTimeout, Storage.PREFIX + storage.getKey() + ".timeout");
+        if (settings.isEmpty()) {
-                    AzureStorageSettings current = new AzureStorageSettings(storage.getKey(), map.get("account"), map.get("key"), timeout);
+            return null;
-                    boolean activeByDefault = Boolean.parseBoolean(map.getOrDefault("default", "false"));
+        } else if (settings.size() == 1) {
-                    if (activeByDefault) {
+            // the only storage settings belong (implicitly) to the default primary storage
-                        if (primaryStorage == null) {
+            AzureStorageSettings storage = settings.get(0);
-                            primaryStorage = current;
+            return new AzureStorageSettings(storage.getName(), storage.getAccount(), storage.getKey(), storage.getTimeout(), true);
-                        } else {
+        } else {
-                            logger.warn("default storage settings has already been defined. You can not define it to [{}]", storage.getKey());
+            AzureStorageSettings primary = null;
-                            secondaryStorage.put(storage.getKey(), current);
+            for (AzureStorageSettings setting : settings) {
-                        }
+                if (setting.isActiveByDefault()) {
+                    if (primary == null) {
+                        primary = setting;
                     } else {
-                        secondaryStorage.put(storage.getKey(), current);
+                        throw new SettingsException("Multiple default Azure data stores configured: [" + primary.getName() + "] and [" + setting.getName() + "]");
                     }
                 }
             }
-            // If we did not set any default storage, we should complain and define it
+            if (primary == null) {
-            if (primaryStorage == null && secondaryStorage.isEmpty() == false) {
+                throw new SettingsException("No default Azure data store configured");
-                Map.Entry<String, AzureStorageSettings> fallback = secondaryStorage.entrySet().iterator().next();
+            }
-                // We only warn if the number of secondary storage if > to 1
+            return primary;
-                // If the user defined only one storage account, that's fine. We know it's the default one.
+        }
-                if (secondaryStorage.size() > 1) {
+    }
-                    logger.warn("no default storage settings has been defined. " +
+
-                            "Add \"default\": true to the settings you want to activate by default. " +
+    private static Map<String, AzureStorageSettings> getSecondaries(List<AzureStorageSettings> settings) {
-                            "Forcing default to [{}].", fallback.getKey());
+        Map<String, AzureStorageSettings> secondaries = new HashMap<>();
+        // when only one setting is defined, we don't have secondaries
+        if (settings.size() > 1) {
+            for (AzureStorageSettings setting : settings) {
+                if (setting.isActiveByDefault() == false) {
+                    secondaries.put(setting.getName(), setting);
                 }
-                primaryStorage = fallback.getValue();
-                secondaryStorage.remove(fallback.getKey());
             }
         }
-
+        return Collections.unmodifiableMap(secondaries);
-        return Tuple.tuple(primaryStorage, secondaryStorage);
     }
 
     public static <T> T getValue(RepositorySettings repositorySettings,

plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureSettingsParserTests.java

@@ -23,6 +23,7 @@ import org.apache.lucene.util.LuceneTestCase;
 import org.elasticsearch.cloud.azure.storage.AzureStorageSettings;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.settings.SettingsException;
 
 import java.util.Map;
 
@@ -73,14 +74,12 @@ public class AzureSettingsParserTests extends LuceneTestCase {
                 .put("cloud.azure.storage.azure2.key", "mykey2")
                 .build();
 
-        Tuple<AzureStorageSettings, Map<String, AzureStorageSettings>> tuple = AzureStorageSettings.parse(settings);
+        try {
-        assertThat(tuple.v1(), notNullValue());
+            AzureStorageSettings.parse(settings);
-        assertThat(tuple.v1().getAccount(), is("myaccount1"));
+            fail("Should have failed with a SettingsException (no default data store)");
-        assertThat(tuple.v1().getKey(), is("mykey1"));
+        } catch (SettingsException ex) {
-        assertThat(tuple.v2().keySet(), hasSize(1));
+            assertEquals(ex.getMessage(), "No default Azure data store configured");
-        assertThat(tuple.v2().get("azure2"), notNullValue());
+        }
-        assertThat(tuple.v2().get("azure2").getAccount(), is("myaccount2"));
-        assertThat(tuple.v2().get("azure2").getKey(), is("mykey2"));
     }
 
     public void testParseTwoSettingsTooManyDefaultSet() {
@@ -93,14 +92,13 @@ public class AzureSettingsParserTests extends LuceneTestCase {
                 .put("cloud.azure.storage.azure2.default", true)
                 .build();
 
-        Tuple<AzureStorageSettings, Map<String, AzureStorageSettings>> tuple = AzureStorageSettings.parse(settings);
+        try {
-        assertThat(tuple.v1(), notNullValue());
+            AzureStorageSettings.parse(settings);
-        assertThat(tuple.v1().getAccount(), is("myaccount1"));
+            fail("Should have failed with a SettingsException (multiple default data stores)");
-        assertThat(tuple.v1().getKey(), is("mykey1"));
+        } catch (SettingsException ex) {
-        assertThat(tuple.v2().keySet(), hasSize(1));
+            assertEquals(ex.getMessage(), "Multiple default Azure data stores configured: [azure1] and [azure2]");
-        assertThat(tuple.v2().get("azure2"), notNullValue());
+        }
-        assertThat(tuple.v2().get("azure2").getAccount(), is("myaccount2"));
+
-        assertThat(tuple.v2().get("azure2").getKey(), is("mykey2"));
     }
 
     public void testParseEmptySettings() {