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

Reject null customs at build time (#41782) 

Today you can add a null `Custom` to the cluster state or its metadata, but
attempting to publish such a cluster state will fail. Unfortunately, the
publication-time failure gives very little information about the source of the
problem. This change causes the failure to manifest earlier and adds
information about which `Custom` was null in order to simplify the
investigation.

Relates #41090.
This commit is contained in:
David Turner 2019-05-03 14:49:23 +02:00
parent 00af42fefe
commit 873d0020a5
4 changed files with 41 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
server/src/main/java/org/elasticsearch/cluster/ClusterState.java
View File

@ -66,8 +66,10 @@ import java.util.EnumSet;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.StreamSupport;
import static org.elasticsearch.cluster.coordination.Coordinator.ZEN1_BWC_TERM;
@ -736,7 +738,7 @@ public class ClusterState implements ToXContentFragment, Diffable<ClusterState>
}
public Builder putCustom(String type, Custom custom) {
customs.put(type, custom);
customs.put(type, Objects.requireNonNull(custom, type));
return this;
}
@ -746,6 +748,7 @@ public class ClusterState implements ToXContentFragment, Diffable<ClusterState>
}
public Builder customs(ImmutableOpenMap<String, Custom> customs) {
StreamSupport.stream(customs.spliterator(), false).forEach(cursor -> Objects.requireNonNull(cursor.value, cursor.key));
this.customs.putAll(customs);
return this;
}

5
server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java
View File

@ -74,11 +74,13 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.StreamSupport;
import static org.elasticsearch.common.settings.Settings.readSettingsFromStream;
import static org.elasticsearch.common.settings.Settings.writeSettingsToStream;
@ -1069,7 +1071,7 @@ public class MetaData implements Iterable<IndexMetaData>, Diffable<MetaData>, To
}
public Builder putCustom(String type, Custom custom) {
customs.put(type, custom);
customs.put(type, Objects.requireNonNull(custom, type));
return this;
}
@ -1079,6 +1081,7 @@ public class MetaData implements Iterable<IndexMetaData>, Diffable<MetaData>, To
}
public Builder customs(ImmutableOpenMap<String, Custom> customs) {
StreamSupport.stream(customs.spliterator(), false).forEach(cursor -> Objects.requireNonNull(cursor.value, cursor.key));
this.customs.putAll(customs);
return this;
}

17
server/src/test/java/org/elasticsearch/cluster/ClusterStateTests.java
View File

@ -21,11 +21,13 @@ package org.elasticsearch.cluster;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.ESTestCase;
import static java.util.Collections.emptyMap;
import static java.util.Collections.emptySet;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
public class ClusterStateTests extends ESTestCase {
@ -56,4 +58,19 @@ public class ClusterStateTests extends ESTestCase {
// state from the same master compare by version
assertThat(withMaster1a.supersedes(withMaster1b), equalTo(withMaster1a.version() > withMaster1b.version()));
}
public void testBuilderRejectsNullCustom() {
final ClusterState.Builder builder = ClusterState.builder(ClusterName.DEFAULT);
final String key = randomAlphaOfLength(10);
assertThat(expectThrows(NullPointerException.class, () -> builder.putCustom(key, null)).getMessage(), containsString(key));
}
public void testBuilderRejectsNullInCustoms() {
final ClusterState.Builder builder = ClusterState.builder(ClusterName.DEFAULT);
final String key = randomAlphaOfLength(10);
final ImmutableOpenMap.Builder<String, ClusterState.Custom> mapBuilder = ImmutableOpenMap.builder();
mapBuilder.put(key, null);
final ImmutableOpenMap<String, ClusterState.Custom> map = mapBuilder.build();
assertThat(expectThrows(NullPointerException.class, () -> builder.customs(map)).getMessage(), containsString(key));
}
}

16
server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java
View File

@ -50,6 +50,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.startsWith;
@ -891,4 +892,19 @@ public class MetaDataTests extends ESTestCase {
.transientSettings(Settings.builder().put(setting.getKey(), "transient-value").build()).build();
assertThat(setting.get(metaData.settings()), equalTo("transient-value"));
}
public void testBuilderRejectsNullCustom() {
final MetaData.Builder builder = MetaData.builder();
final String key = randomAlphaOfLength(10);
assertThat(expectThrows(NullPointerException.class, () -> builder.putCustom(key, null)).getMessage(), containsString(key));
}
public void testBuilderRejectsNullInCustoms() {
final MetaData.Builder builder = MetaData.builder();
final String key = randomAlphaOfLength(10);
final ImmutableOpenMap.Builder<String, MetaData.Custom> mapBuilder = ImmutableOpenMap.builder();
mapBuilder.put(key, null);
final ImmutableOpenMap<String, MetaData.Custom> map = mapBuilder.build();
assertThat(expectThrows(NullPointerException.class, () -> builder.customs(map)).getMessage(), containsString(key));
}
}