Skip to content

Commit 3437988

Browse files
authored
Make custom index metadata completely immutable (#33735)
Currently `IndexMetadata#getCustomData(...)` wraps the custom metadata in an unmodifiable map, but in case there is no entry for the specified key then a NPE is thrown by Collections.unmodifiableMap(...). This is not ideal in case callers like to throw an exception with a specific message. (like in the case for ccr to indicate that the follow index was not created by the create_and_follow api and therefor incompatible as follow index) I think making `DiffableStringMap` itself immutable is better then just wrapping custom metadata with `Collections.unmodifiableMap(...)` in all methods that access it. Also removed the `equals()`, `hashcode()` and to `toString()` methods of `DiffableStringMap`, because `AbstractMap` already implements these methods.
1 parent 481f8a9 commit 3437988

File tree

3 files changed

+5
-36
lines changed

3 files changed

+5
-36
lines changed

server/src/main/java/org/elasticsearch/cluster/metadata/DiffableStringMap.java

Lines changed: 2 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,12 @@ public class DiffableStringMap extends AbstractMap<String, String> implements Di
4242
private final Map<String, String> innerMap;
4343

4444
DiffableStringMap(final Map<String, String> map) {
45-
this.innerMap = map;
45+
this.innerMap = Collections.unmodifiableMap(map);
4646
}
4747

4848
@SuppressWarnings("unchecked")
4949
DiffableStringMap(final StreamInput in) throws IOException {
50-
this.innerMap = (Map<String, String>) (Map) in.readMap();
51-
}
52-
53-
@Override
54-
public String put(String key, String value) {
55-
return innerMap.put(key, value);
50+
this((Map<String, String>) (Map) in.readMap());
5651
}
5752

5853
@Override
@@ -75,32 +70,6 @@ public static Diff<DiffableStringMap> readDiffFrom(StreamInput in) throws IOExce
7570
return new DiffableStringMapDiff(in);
7671
}
7772

78-
@Override
79-
public boolean equals(Object obj) {
80-
if (obj == null) {
81-
return false;
82-
}
83-
if (obj instanceof DiffableStringMap) {
84-
DiffableStringMap other = (DiffableStringMap) obj;
85-
return innerMap.equals(other.innerMap);
86-
} else if (obj instanceof Map) {
87-
Map other = (Map) obj;
88-
return innerMap.equals(other);
89-
} else {
90-
return false;
91-
}
92-
}
93-
94-
@Override
95-
public int hashCode() {
96-
return innerMap.hashCode();
97-
}
98-
99-
@Override
100-
public String toString() {
101-
return "DiffableStringMap[" + innerMap.toString() + "]";
102-
}
103-
10473
/**
10574
* Represents differences between two DiffableStringMaps.
10675
*/

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ ImmutableOpenMap<String, DiffableStringMap> getCustomData() {
466466
}
467467

468468
public Map<String, String> getCustomData(final String key) {
469-
return Collections.unmodifiableMap(this.customData.get(key));
469+
return this.customData.get(key);
470470
}
471471

472472
public ImmutableOpenIntMap<Set<String>> getInSyncAllocationIds() {

server/src/test/java/org/elasticsearch/cluster/metadata/DiffableStringMapTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public void testRandomDiffing() {
7070
m.put("2", "2");
7171
m.put("3", "3");
7272
DiffableStringMap dsm = new DiffableStringMap(m);
73-
DiffableStringMap expected = new DiffableStringMap(m);
73+
Map<String, String> expected = new HashMap<>(m);
7474

7575
for (int i = 0; i < randomIntBetween(5, 50); i++) {
7676
if (randomBoolean() && expected.size() > 1) {
@@ -80,7 +80,7 @@ public void testRandomDiffing() {
8080
} else {
8181
expected.put(randomAlphaOfLength(2), randomAlphaOfLength(4));
8282
}
83-
dsm = expected.diff(dsm).apply(dsm);
83+
dsm = new DiffableStringMap(expected).diff(dsm).apply(dsm);
8484
}
8585
assertThat(expected, equalTo(dsm));
8686
}

0 commit comments

Comments
 (0)