Browse Source

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.
Martijn van Groningen 7 years ago
parent
commit
34379887b4

+ 2 - 33
server/src/main/java/org/elasticsearch/cluster/metadata/DiffableStringMap.java

@@ -42,17 +42,12 @@ public class DiffableStringMap extends AbstractMap<String, String> implements Di
     private final Map<String, String> innerMap;
 
     DiffableStringMap(final Map<String, String> map) {
-        this.innerMap = map;
+        this.innerMap = Collections.unmodifiableMap(map);
     }
 
     @SuppressWarnings("unchecked")
     DiffableStringMap(final StreamInput in) throws IOException {
-        this.innerMap = (Map<String, String>) (Map) in.readMap();
-    }
-
-    @Override
-    public String put(String key, String value) {
-        return innerMap.put(key, value);
+        this((Map<String, String>) (Map) in.readMap());
     }
 
     @Override
@@ -75,32 +70,6 @@ public class DiffableStringMap extends AbstractMap<String, String> implements Di
         return new DiffableStringMapDiff(in);
     }
 
-    @Override
-    public boolean equals(Object obj) {
-        if (obj == null) {
-            return false;
-        }
-        if (obj instanceof DiffableStringMap) {
-            DiffableStringMap other = (DiffableStringMap) obj;
-            return innerMap.equals(other.innerMap);
-        } else if (obj instanceof Map) {
-            Map other = (Map) obj;
-            return innerMap.equals(other);
-        } else {
-            return false;
-        }
-    }
-
-    @Override
-    public int hashCode() {
-        return innerMap.hashCode();
-    }
-
-    @Override
-    public String toString() {
-        return "DiffableStringMap[" + innerMap.toString() + "]";
-    }
-
     /**
      * Represents differences between two DiffableStringMaps.
      */

+ 1 - 1
server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java

@@ -466,7 +466,7 @@ public class IndexMetaData implements Diffable<IndexMetaData>, ToXContentFragmen
     }
 
     public Map<String, String> getCustomData(final String key) {
-        return Collections.unmodifiableMap(this.customData.get(key));
+        return this.customData.get(key);
     }
 
     public ImmutableOpenIntMap<Set<String>> getInSyncAllocationIds() {

+ 2 - 2
server/src/test/java/org/elasticsearch/cluster/metadata/DiffableStringMapTests.java

@@ -70,7 +70,7 @@ public class DiffableStringMapTests extends ESTestCase {
         m.put("2", "2");
         m.put("3", "3");
         DiffableStringMap dsm = new DiffableStringMap(m);
-        DiffableStringMap expected = new DiffableStringMap(m);
+        Map<String, String> expected = new HashMap<>(m);
 
         for (int i = 0; i < randomIntBetween(5, 50); i++) {
             if (randomBoolean() && expected.size() > 1) {
@@ -80,7 +80,7 @@ public class DiffableStringMapTests extends ESTestCase {
             } else {
                 expected.put(randomAlphaOfLength(2), randomAlphaOfLength(4));
             }
-            dsm = expected.diff(dsm).apply(dsm);
+            dsm = new DiffableStringMap(expected).diff(dsm).apply(dsm);
         }
         assertThat(expected, equalTo(dsm));
     }