honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Settings: Ensure fields are overriden and not merged when using arrays 

In the case you try to merge two settings, one being an array and one being
a field, together, the settings were merged instead of being overridden.

First config
my.value: 1

Second config
my.value: [ 2, 3 ]

If you execute

settingsBuilder().put(settings1).put(settings2).build()

now only values 2,3 will be in the final settings

Closes #8381
This commit is contained in:
Alexander Reelsen 2014-11-07 09:57:05 +01:00
parent a5127d2ffd
commit 8626c18ad9
2 changed files with 162 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java
View File

@ -20,7 +20,9 @@
package org.elasticsearch.common.settings;
import com.google.common.base.Charsets;
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import org.elasticsearch.ElasticsearchIllegalArgumentException;
@ -45,6 +47,8 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.util.*;
import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import static org.elasticsearch.common.Strings.toCamelCase;
import static org.elasticsearch.common.unit.ByteSizeValue.parseBytesSizeValue;
@ -57,6 +61,7 @@ import static org.elasticsearch.common.unit.TimeValue.parseTimeValue;
public class ImmutableSettings implements Settings {
public static final Settings EMPTY = new Builder().build();
private final static Pattern ARRAY_PATTERN = Pattern.compile("(.*)\\.\\d+$");
private ImmutableMap<String, String> settings;
private final ImmutableMap<String, String> forcedUnderscoreSettings;
@ -873,6 +878,7 @@ public class ImmutableSettings implements Settings {
* Sets all the provided settings.
*/
public Builder put(Settings settings) {
removeNonArraysFieldsIfNewSettingsContainsFieldAsArray(settings.getAsMap());
map.putAll(settings.getAsMap());
classLoader = settings.getClassLoaderIfSet();
return this;
@ -882,10 +888,42 @@ public class ImmutableSettings implements Settings {
* Sets all the provided settings.
*/
public Builder put(Map<String, String> settings) {
removeNonArraysFieldsIfNewSettingsContainsFieldAsArray(settings);
map.putAll(settings);
return this;
}
/**
* Removes non array values from the existing map, if settings contains an array value instead
*
* Example:
* Existing map contains: {key:value}
* New map contains: {key:[value1,value2]} (which has been flattened to {}key.0:value1,key.1:value2})
*
* This ensure that that the 'key' field gets removed from the map in order to override all the
* data instead of merging
*/
private void removeNonArraysFieldsIfNewSettingsContainsFieldAsArray(Map<String, String> settings) {
List<String> prefixesToRemove = new ArrayList<>();
for (final Map.Entry<String, String> entry : settings.entrySet()) {
final Matcher matcher = ARRAY_PATTERN.matcher(entry.getKey());
if (matcher.matches()) {
prefixesToRemove.add(matcher.group(1));
} else if (Iterables.any(map.keySet(), startsWith(entry.getKey() + "."))) {
prefixesToRemove.add(entry.getKey());
}
}
for (String prefix : prefixesToRemove) {
Iterator<Map.Entry<String, String>> iterator = map.entrySet().iterator();
while (iterator.hasNext()) {
Map.Entry<String, String> entry = iterator.next();
if (entry.getKey().startsWith(prefix + ".") || entry.getKey().equals(prefix)) {
iterator.remove();
}
}
}
}
/**
* Sets all the provided settings.
*/
@ -1090,4 +1128,22 @@ public class ImmutableSettings implements Settings {
return new ImmutableSettings(Collections.unmodifiableMap(map), classLoader);
}
}
private static StartsWithPredicate startsWith(String prefix) {
return new StartsWithPredicate(prefix);
}
private static final class StartsWithPredicate implements Predicate<String> {
private String prefix;
public StartsWithPredicate(String prefix) {
this.prefix = prefix;
}
@Override
public boolean apply(String input) {
return input.startsWith(prefix);
}
}
}

106
src/test/java/org/elasticsearch/common/settings/ImmutableSettingsTests.java
View File

@ -21,10 +21,12 @@ package org.elasticsearch.common.settings;
import org.elasticsearch.common.settings.bar.BarTestClass;
import org.elasticsearch.common.settings.foo.FooTestClass;
import org.elasticsearch.common.settings.loader.YamlSettingsLoader;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.hamcrest.Matchers;
import org.junit.Test;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -216,6 +218,110 @@ public class ImmutableSettingsTests extends ElasticsearchTestCase {
assertThat(names.size(), equalTo(2));
assertTrue(names.contains("bar"));
assertTrue(names.contains("baz"));
}
@Test
public void testThatArraysAreOverriddenCorrectly() throws IOException {
// overriding a single value with an array
Settings settings = settingsBuilder()
.put(settingsBuilder().putArray("value", "1").build())
.put(settingsBuilder().putArray("value", "2", "3").build())
.build();
assertThat(settings.getAsArray("value"), arrayContaining("2", "3"));
settings = settingsBuilder()
.put(settingsBuilder().put("value", "1").build())
.put(settingsBuilder().putArray("value", "2", "3").build())
.build();
assertThat(settings.getAsArray("value"), arrayContaining("2", "3"));
settings = settingsBuilder()
.put(new YamlSettingsLoader().load("value: 1"))
.put(new YamlSettingsLoader().load("value: [ 2, 3 ]"))
.build();
assertThat(settings.getAsArray("value"), arrayContaining("2", "3"));
settings = settingsBuilder()
.put(settingsBuilder().put("value.with.deep.key", "1").build())
.put(settingsBuilder().putArray("value.with.deep.key", "2", "3").build())
.build();
assertThat(settings.getAsArray("value.with.deep.key"), arrayContaining("2", "3"));
// overriding an array with a shorter array
settings = settingsBuilder()
.put(settingsBuilder().putArray("value", "1", "2").build())
.put(settingsBuilder().putArray("value", "3").build())
.build();
assertThat(settings.getAsArray("value"), arrayContaining("3"));
settings = settingsBuilder()
.put(settingsBuilder().putArray("value", "1", "2", "3").build())
.put(settingsBuilder().putArray("value", "4", "5").build())
.build();
assertThat(settings.getAsArray("value"), arrayContaining("4", "5"));
settings = settingsBuilder()
.put(settingsBuilder().putArray("value.deep.key", "1", "2", "3").build())
.put(settingsBuilder().putArray("value.deep.key", "4", "5").build())
.build();
assertThat(settings.getAsArray("value.deep.key"), arrayContaining("4", "5"));
// overriding an array with a longer array
settings = settingsBuilder()
.put(settingsBuilder().putArray("value", "1", "2").build())
.put(settingsBuilder().putArray("value", "3", "4", "5").build())
.build();
assertThat(settings.getAsArray("value"), arrayContaining("3", "4", "5"));
settings = settingsBuilder()
.put(settingsBuilder().putArray("value.deep.key", "1", "2", "3").build())
.put(settingsBuilder().putArray("value.deep.key", "4", "5").build())
.build();
assertThat(settings.getAsArray("value.deep.key"), arrayContaining("4", "5"));
// overriding an array with a single value
settings = settingsBuilder()
.put(settingsBuilder().putArray("value", "1", "2").build())
.put(settingsBuilder().put("value", "3").build())
.build();
assertThat(settings.getAsArray("value"), arrayContaining("3"));
settings = settingsBuilder()
.put(settingsBuilder().putArray("value.deep.key", "1", "2").build())
.put(settingsBuilder().put("value.deep.key", "3").build())
.build();
assertThat(settings.getAsArray("value.deep.key"), arrayContaining("3"));
// test that other arrays are not overridden
settings = settingsBuilder()
.put(settingsBuilder().putArray("value", "1", "2", "3").putArray("a", "b", "c").build())
.put(settingsBuilder().putArray("value", "4", "5").putArray("d", "e", "f").build())
.build();
assertThat(settings.getAsArray("value"), arrayContaining("4", "5"));
assertThat(settings.getAsArray("a"), arrayContaining("b", "c"));
assertThat(settings.getAsArray("d"), arrayContaining("e", "f"));
settings = settingsBuilder()
.put(settingsBuilder().putArray("value.deep.key", "1", "2", "3").putArray("a", "b", "c").build())
.put(settingsBuilder().putArray("value.deep.key", "4", "5").putArray("d", "e", "f").build())
.build();
assertThat(settings.getAsArray("value.deep.key"), arrayContaining("4", "5"));
assertThat(settings.getAsArray("a"), notNullValue());
assertThat(settings.getAsArray("d"), notNullValue());
// overriding a deeper structure with an array
settings = settingsBuilder()
.put(settingsBuilder().put("value.data", "1").build())
.put(settingsBuilder().putArray("value", "4", "5").build())
.build();
assertThat(settings.getAsArray("value"), arrayContaining("4", "5"));
// overriding an array with a deeper structure
settings = settingsBuilder()
.put(settingsBuilder().putArray("value", "4", "5").build())
.put(settingsBuilder().put("value.data", "1").build())
.build();
assertThat(settings.get("value.data"), is("1"));
assertThat(settings.get("value"), is(nullValue()));
}
}