From 52acf0e6e17916bdece0a677dfba8e1a5fd064fd Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Tue, 2 Feb 2016 09:16:44 +0100 Subject: [PATCH] Use new settings infra to parse AzureStorageSettings With this commit we simplify the parsing logic in AzureStorageSettings by leveraging the new settings infrastructure. Closes #16363 --- .../common/logging/ESLoggerFactory.java | 2 +- .../settings/AbstractScopedSettings.java | 13 + .../common/settings/Setting.java | 240 ++++++++++++++---- .../common/settings/SettingTests.java | 28 +- .../azure/storage/AzureStorageSettings.java | 125 ++++++--- .../azure/AzureSettingsParserTests.java | 30 +-- 6 files changed, 335 insertions(+), 103 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 1cd3405bde6..94ade9334d7 100644 --- a/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java +++ b/core/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java @@ -32,7 +32,7 @@ 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 = - 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(); diff --git a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java index 8f5373dfe3b..453fc3f9a36 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -296,12 +296,25 @@ public abstract class AbstractScopedSettings extends AbstractComponent { } for (Map.Entry> 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> list = new ArrayList<>(); + for (Map.Entry> 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 true if the setting for the given key is dynamically updateable. Otherwise false. */ 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 7f64c011133..0b4e43744a5 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -66,7 +66,7 @@ import java.util.stream.Collectors; * */ public class Setting extends ToXContentToBytes { - private final String key; + private final Key key; protected final Function defaultValue; private final Function parser; private final boolean dynamic; @@ -80,7 +80,7 @@ public class Setting extends ToXContentToBytes { * @param dynamic true iff this setting can be dynamically updateable * @param scope the scope of this setting */ - public Setting(String key, Function defaultValue, Function parser, boolean dynamic, Scope scope) { + public Setting(Key key, Function defaultValue, Function 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 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 defaultValue, Function 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 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 extends ToXContentToBytes { * Returns true iff this setting is present in the given settings object. Otherwise false */ public final boolean exists(Settings settings) { - return settings.get(key) != null; + return settings.get(getKey()) != null; } /** @@ -186,7 +205,7 @@ public class Setting 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 extends ToXContentToBytes { * given key is part of the settings group. * @see #isGroupSetting() */ - public boolean match(String toTest) { - return key.equals(toTest); + public final boolean match(String 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 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 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 extends ToXContentToBytes { Function> parser = (s) -> parseableStringToList(s).stream().map(singleValueParser).collect(Collectors.toList()); - return new Setting>(key, (s) -> arrayToParsableString(defaultStringValue.apply(s).toArray(Strings.EMPTY_ARRAY)), parser, dynamic, scope) { - private final Pattern pattern = Pattern.compile(Pattern.quote(key)+"(\\.\\d+)?"); + return new Setting>(new ListKey(key), (s) -> arrayToParsableString(defaultStringValue.apply(s).toArray(Strings.EMPTY_ARRAY)), parser, dynamic, scope) { @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 extends ToXContentToBytes { } public static Setting groupSetting(String key, boolean dynamic, Scope scope) { - if (key.endsWith(".") == false) { - throw new IllegalArgumentException("key must end with a '.'"); - } - return new Setting(key, "", (s) -> null, dynamic, scope) { - + return new Setting(new GroupKey(key), (s) -> "", (s) -> null, dynamic, scope) { @Override public boolean isGroupSetting() { return true; @@ -498,12 +515,7 @@ public class Setting extends ToXContentToBytes { @Override public Settings get(Settings settings) { - return settings.getByPrefix(key); - } - - @Override - public boolean match(String toTest) { - return Regex.simpleMatch(key + "*", toTest); + return settings.getByPrefix(getKey()); } @Override @@ -549,13 +561,7 @@ public class Setting extends ToXContentToBytes { } public static Setting timeSetting(String key, Function defaultValue, TimeValue minValue, boolean dynamic, Scope scope) { - return new Setting<>(key, defaultValue, (s) -> { - 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); + return new Setting<>(key, defaultValue, (s) -> parseTimeValue(s, minValue, key), dynamic, scope); } public static Setting timeSetting(String key, TimeValue defaultValue, TimeValue minValue, boolean dynamic, Scope scope) { @@ -595,10 +601,27 @@ public class Setting 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)} - * is used to pull the updater. + * can easily be added with this setting. Yet, prefix key settings don't support updaters out 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) { + public static Setting prefixKeySetting(String prefix, String defaultValue, Function 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 Setting adfixKeySetting(String prefix, String suffix, Function defaultValue, Function parser, boolean dynamic, Scope scope) { + return affixKeySetting(AffixKey.withAdfix(prefix, suffix), defaultValue, parser, dynamic, scope); + } + + public static Setting adfixKeySetting(String prefix, String suffix, String defaultValue, Function parser, boolean dynamic, Scope scope) { + return adfixKeySetting(prefix, suffix, (s) -> defaultValue, parser, dynamic, scope); + } + + public static Setting affixKeySetting(AffixKey key, Function defaultValue, Function parser, boolean dynamic, Scope scope) { return new Setting(key, defaultValue, parser, dynamic, scope) { @Override @@ -606,14 +629,9 @@ public class Setting extends ToXContentToBytes { 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"); + throw new UnsupportedOperationException("Affix settings can't be updated. Use #getConcreteSetting for updating."); } @Override @@ -621,9 +639,145 @@ public class Setting 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); + } + } } 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 f5b84fb366f..df2014f7855 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/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() { - Setting setting = Setting.dynamicKeySetting("foo.", "false", Boolean::parseBoolean, false, Setting.Scope.CLUSTER); + public void testPrefixKeySetting() { + Setting 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 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 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()); } } diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageSettings.java b/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageSettings.java index eed5e14657c..da9151e9504 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageSettings.java +++ b/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 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 ACCOUNT_SETTING = Setting.adfixKeySetting(Storage.PREFIX, ACCOUNT_SUFFIX, "", Function.identity(), false, Setting.Scope.CLUSTER); + private static final Setting KEY_SETTING = Setting.adfixKeySetting(Storage.PREFIX, KEY_SUFFIX, "", Function.identity(), false, Setting.Scope.CLUSTER); + private static final Setting 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> parse(Settings settings) { - AzureStorageSettings primaryStorage = null; - Map secondaryStorage = new HashMap<>(); + List storageSettings = createStorageSettings(settings); + return Tuple.tuple(getPrimary(storageSettings), getSecondaries(storageSettings)); + } - TimeValue globalTimeout = Storage.TIMEOUT_SETTING.get(settings); + private static List createStorageSettings(Settings settings) { + Setting 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 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); - if (storageSettings != null) { - Map asMap = storageSettings.getAsStructuredMap(); - for (Map.Entry storage : asMap.entrySet()) { - if (storage.getValue() instanceof Map) { - @SuppressWarnings("unchecked") - Map map = (Map) storage.getValue(); - TimeValue timeout = TimeValue.parseTimeValue(map.get("timeout"), globalTimeout, Storage.PREFIX + storage.getKey() + ".timeout"); - AzureStorageSettings current = new AzureStorageSettings(storage.getKey(), map.get("account"), map.get("key"), timeout); - boolean activeByDefault = Boolean.parseBoolean(map.getOrDefault("default", "false")); - if (activeByDefault) { - if (primaryStorage == null) { - primaryStorage = current; - } else { - logger.warn("default storage settings has already been defined. You can not define it to [{}]", storage.getKey()); - secondaryStorage.put(storage.getKey(), current); - } + private static T getValue(Settings settings, String groupName, Setting setting) { + Setting.AffixKey k = (Setting.AffixKey) setting.getRawKey(); + String fullKey = k.toConcreteKey(groupName).toString(); + return setting.getConcreteSetting(fullKey).get(settings); + } + + private static AzureStorageSettings getPrimary(List settings) { + if (settings.isEmpty()) { + return null; + } else if (settings.size() == 1) { + // the only storage settings belong (implicitly) to the default primary storage + AzureStorageSettings storage = settings.get(0); + return new AzureStorageSettings(storage.getName(), storage.getAccount(), storage.getKey(), storage.getTimeout(), true); + } else { + AzureStorageSettings primary = null; + 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 (primaryStorage == null && secondaryStorage.isEmpty() == false) { - Map.Entry fallback = secondaryStorage.entrySet().iterator().next(); - // We only warn if the number of secondary storage if > to 1 - // 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. " + - "Forcing default to [{}].", fallback.getKey()); + if (primary == null) { + throw new SettingsException("No default Azure data store configured"); + } + return primary; + } + } + + private static Map getSecondaries(List settings) { + Map 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 Tuple.tuple(primaryStorage, secondaryStorage); + return Collections.unmodifiableMap(secondaries); } public static T getValue(RepositorySettings repositorySettings, diff --git a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureSettingsParserTests.java b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureSettingsParserTests.java index 5347be09da0..ac2c51441f9 100644 --- a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureSettingsParserTests.java +++ b/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> tuple = AzureStorageSettings.parse(settings); - assertThat(tuple.v1(), notNullValue()); - assertThat(tuple.v1().getAccount(), is("myaccount1")); - assertThat(tuple.v1().getKey(), is("mykey1")); - assertThat(tuple.v2().keySet(), hasSize(1)); - assertThat(tuple.v2().get("azure2"), notNullValue()); - assertThat(tuple.v2().get("azure2").getAccount(), is("myaccount2")); - assertThat(tuple.v2().get("azure2").getKey(), is("mykey2")); + try { + AzureStorageSettings.parse(settings); + fail("Should have failed with a SettingsException (no default data store)"); + } catch (SettingsException ex) { + assertEquals(ex.getMessage(), "No default Azure data store configured"); + } } public void testParseTwoSettingsTooManyDefaultSet() { @@ -93,14 +92,13 @@ public class AzureSettingsParserTests extends LuceneTestCase { .put("cloud.azure.storage.azure2.default", true) .build(); - Tuple> tuple = AzureStorageSettings.parse(settings); - assertThat(tuple.v1(), notNullValue()); - assertThat(tuple.v1().getAccount(), is("myaccount1")); - assertThat(tuple.v1().getKey(), is("mykey1")); - assertThat(tuple.v2().keySet(), hasSize(1)); - assertThat(tuple.v2().get("azure2"), notNullValue()); - assertThat(tuple.v2().get("azure2").getAccount(), is("myaccount2")); - assertThat(tuple.v2().get("azure2").getKey(), is("mykey2")); + try { + AzureStorageSettings.parse(settings); + fail("Should have failed with a SettingsException (multiple default data stores)"); + } catch (SettingsException ex) { + assertEquals(ex.getMessage(), "Multiple default Azure data stores configured: [azure1] and [azure2]"); + } + } public void testParseEmptySettings() {