Minor Cleanup in org.elasticsearch.common.collect (elastic#64955)

* Return empty map singleton from builder
* Remove dead methods
* Cleanup duplicate iterator

server/src/main/java/org/elasticsearch/common/collect/HppcMaps.java

@@ -43,17 +43,10 @@ public static <K, V> ObjectObjectHashMap<K, V> newMap(int expectedElements) {
     }
 
     /**
-     * Returns a new map with a default initial capacity.
-     */
-    public static <K, V> ObjectObjectHashMap<K, V> newMap() {
-        return newMap(16);
-    }
-
-    /**
-     * Returns a map like {@link #newMap()} that does not accept <code>null</code> keys
+     * Returns a map like {@link #newMap} that does not accept <code>null</code> keys
      */
     public static <K, V> ObjectObjectHashMap<K, V> newNoNullKeysMap() {
-        return ensureNoNullKeys(16);
+        return newNoNullKeysMap(16);
     }
 
     /**
@@ -64,17 +57,6 @@ public static <K, V> ObjectObjectHashMap<K, V> newNoNullKeysMap() {
      *          expansion (inclusive).
      */
     public static <K, V> ObjectObjectHashMap<K, V> newNoNullKeysMap(int expectedElements) {
-        return ensureNoNullKeys(expectedElements);
-    }
-
-    /**
-     * Wraps the given map and prevent adding of <code>null</code> keys.
-     * 
-     * @param expectedElements
-     *          The expected number of elements guaranteed not to cause buffer
-     *          expansion (inclusive).
-     */
-    public static <K, V> ObjectObjectHashMap<K, V> ensureNoNullKeys(int expectedElements) {
         return new ObjectObjectHashMap<K, V>(expectedElements) {
             @Override
             public V put(K key, V value) {

server/src/main/java/org/elasticsearch/common/collect/ImmutableOpenIntMap.java

@@ -28,7 +28,6 @@
 import com.carrotsearch.hppc.ObjectContainer;
 import com.carrotsearch.hppc.cursors.IntCursor;
 import com.carrotsearch.hppc.cursors.IntObjectCursor;
-import com.carrotsearch.hppc.cursors.ObjectCursor;
 import com.carrotsearch.hppc.predicates.IntObjectPredicate;
 import com.carrotsearch.hppc.predicates.IntPredicate;
 import com.carrotsearch.hppc.procedures.IntObjectProcedure;
@@ -149,23 +148,7 @@ public ObjectContainer<VType> values() {
      * Returns a direct iterator over the keys.
      */
     public Iterator<VType> valuesIt() {
-        final Iterator<ObjectCursor<VType>> iterator = map.values().iterator();
-        return new Iterator<VType>() {
-            @Override
-            public boolean hasNext() {
-                return iterator.hasNext();
-            }
-
-            @Override
-            public VType next() {
-                return iterator.next().value;
-            }
-
-            @Override
-            public void remove() {
-                throw new UnsupportedOperationException();
-            }
-        };
+        return ImmutableOpenMap.iterator(map.values());
     }
 
     @Override
@@ -234,7 +217,7 @@ public Builder(ImmutableOpenIntMap<VType> map) {
         public ImmutableOpenIntMap<VType> build() {
             IntObjectHashMap<VType> map = this.map;
             this.map = null; // nullify the map, so any operation post build will fail! (hackish, but safest)
-            return new ImmutableOpenIntMap<>(map);
+            return map.isEmpty() ? of() : new ImmutableOpenIntMap<>(map);
         }
 
         /**

server/src/main/java/org/elasticsearch/common/collect/ImmutableOpenMap.java

@@ -153,13 +153,17 @@ public ObjectContainer<VType> values() {
      * Returns a direct iterator over the keys.
      */
     public Iterator<VType> valuesIt() {
-        final Iterator<ObjectCursor<VType>> iterator = map.values().iterator();
-        return new Iterator<VType>() {
+        return iterator(map.values());
+    }
+
+    static <T> Iterator<T> iterator(ObjectCollection<T> collection) {
+        final Iterator<ObjectCursor<T>> iterator = collection.iterator();
+        return new Iterator<>() {
             @Override
             public boolean hasNext() { return iterator.hasNext(); }
 
             @Override
-            public VType next() {
+            public T next() {
                 return iterator.next().value;
             }
 
@@ -244,11 +248,9 @@ public Builder(ImmutableOpenMap<KType, VType> map) {
         public ImmutableOpenMap<KType, VType> build() {
             ObjectObjectHashMap<KType, VType> map = this.map;
             this.map = null; // nullify the map, so any operation post build will fail! (hackish, but safest)
-            return new ImmutableOpenMap<>(map);
+            return map.isEmpty() ? of() : new ImmutableOpenMap<>(map);
         }
 
-
-
         /**
          * Puts all the entries in the map to the builder.
          */

server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java

@@ -600,8 +600,9 @@ public void testFindDefaultPipelineFromTemplateMatch(){
         Metadata metadata = mock(Metadata.class);
         when(state.metadata()).thenReturn(metadata);
         when(state.getMetadata()).thenReturn(metadata);
-        when(metadata.templates()).thenReturn(templateMetadataBuilder.build());
-        when(metadata.getTemplates()).thenReturn(templateMetadataBuilder.build());
+        final ImmutableOpenMap<String, IndexTemplateMetadata> templateMetadata = templateMetadataBuilder.build();
+        when(metadata.templates()).thenReturn(templateMetadata);
+        when(metadata.getTemplates()).thenReturn(templateMetadata);
         when(metadata.indices()).thenReturn(ImmutableOpenMap.of());
 
         IndexRequest indexRequest = new IndexRequest("missing_index").id("id");

x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinatorTests.java

@@ -2043,9 +2043,7 @@ private static ClusterState createRemoteClusterState(String indexName, boolean e
         ShardRouting shardRouting =
             TestShardRouting.newShardRouting(indexName, 0, "1", true, ShardRoutingState.INITIALIZING).moveToStarted();
         IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).addShard(shardRouting).build();
-        csBuilder.routingTable(RoutingTable.builder().add(indexRoutingTable).build()).build();
-
-        return csBuilder.build();
+        return csBuilder.routingTable(RoutingTable.builder().add(indexRoutingTable).build()).build();
     }
 
     private static ClusterState createRemoteClusterState(final ClusterState previous, final String... indices) {
@@ -2118,9 +2116,7 @@ private static ClusterState createRemoteClusterStateWithDataStream(String dataSt
         ShardRouting shardRouting =
             TestShardRouting.newShardRouting(dataStreamName, 0, "1", true, ShardRoutingState.INITIALIZING).moveToStarted();
         IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).addShard(shardRouting).build();
-        csBuilder.routingTable(RoutingTable.builder().add(indexRoutingTable).build()).build();
-
-        return csBuilder.build();
+        return csBuilder.routingTable(RoutingTable.builder().add(indexRoutingTable).build()).build();
     }
 
 }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CopySettingsStepTests.java

@@ -65,14 +65,15 @@ public void testPerformAction() {
         IndexMetadata.Builder targetIndexMetadataBuilder = IndexMetadata.builder(targetIndex).settings(settings(Version.CURRENT))
             .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5));
 
+        final IndexMetadata sourceIndexMetadata = sourceIndexMetadataBuilder.build();
         ClusterState clusterState = ClusterState.builder(emptyClusterState()).metadata(
-            Metadata.builder().put(sourceIndexMetadataBuilder).put(targetIndexMetadataBuilder).build()
+            Metadata.builder().put(sourceIndexMetadata, false).put(targetIndexMetadataBuilder).build()
         ).build();
 
         CopySettingsStep copySettingsStep = new CopySettingsStep(randomStepKey(), randomStepKey(), indexPrefix,
             LifecycleSettings.LIFECYCLE_NAME);
 
-        ClusterState newClusterState = copySettingsStep.performAction(sourceIndexMetadataBuilder.build().getIndex(), clusterState);
+        ClusterState newClusterState = copySettingsStep.performAction(sourceIndexMetadata.getIndex(), clusterState);
         IndexMetadata newTargetIndexMetadata = newClusterState.metadata().index(targetIndex);
         assertThat(newTargetIndexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME), is(policyName));
     }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStepTests.java

@@ -66,11 +66,12 @@ public void testPerformAction() {
             IndexMetadata.builder(indexName).settings(settings(Version.CURRENT).put(LifecycleSettings.LIFECYCLE_NAME, policyName))
                 .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5));
 
-        ClusterState clusterState =
-            ClusterState.builder(emptyClusterState()).metadata(Metadata.builder().put(indexMetadataBuilder).build()).build();
+        final IndexMetadata indexMetadata = indexMetadataBuilder.build();
+        ClusterState clusterState = ClusterState.builder(emptyClusterState())
+                .metadata(Metadata.builder().put(indexMetadata, false).build()).build();
 
         GenerateSnapshotNameStep generateSnapshotNameStep = createRandomInstance();
-        ClusterState newClusterState = generateSnapshotNameStep.performAction(indexMetadataBuilder.build().getIndex(), clusterState);
+        ClusterState newClusterState = generateSnapshotNameStep.performAction(indexMetadata.getIndex(), clusterState);
 
         LifecycleExecutionState executionState = LifecycleExecutionState.fromIndexMetadata(newClusterState.metadata().index(indexName));
         assertThat("the " + GenerateSnapshotNameStep.NAME + " step must generate a snapshot name", executionState.getSnapshotName(),

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ReplaceDataStreamBackingIndexStepTests.java

@@ -61,12 +61,13 @@ public void testPerformActionThrowsExceptionIfIndexIsNotPartOfDataStream() {
             IndexMetadata.builder(indexName).settings(settings(Version.CURRENT).put(LifecycleSettings.LIFECYCLE_NAME, policyName))
                 .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5));
 
+        final IndexMetadata sourceIndexMetadata = sourceIndexMetadataBuilder.build();
         ClusterState clusterState = ClusterState.builder(emptyClusterState()).metadata(
-            Metadata.builder().put(sourceIndexMetadataBuilder).build()
+            Metadata.builder().put(sourceIndexMetadata, false).build()
         ).build();
 
         expectThrows(IllegalStateException.class,
-            () -> createRandomInstance().performAction(sourceIndexMetadataBuilder.build().getIndex(), clusterState));
+            () -> createRandomInstance().performAction(sourceIndexMetadata.getIndex(), clusterState));
     }
 
     public void testPerformActionThrowsExceptionIfIndexIsTheDataStreamWriteIndex() {

x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedConfigAutoUpdaterTests.java

@@ -175,10 +175,11 @@ public void testIsAbleToRun() {
         csBuilder.metadata(metadata);
 
         DatafeedConfigAutoUpdater updater = new DatafeedConfigAutoUpdater(provider, indexNameExpressionResolver);
-        assertThat(updater.isAbleToRun(csBuilder.build()), is(true));
+        final ClusterState clusterState = csBuilder.build();
+        assertThat(updater.isAbleToRun(clusterState), is(true));
 
-        metadata = new Metadata.Builder(csBuilder.build().metadata());
-        routingTable = new RoutingTable.Builder(csBuilder.build().routingTable());
+        metadata = new Metadata.Builder(clusterState.metadata());
+        routingTable = new RoutingTable.Builder(clusterState.routingTable());
         if (randomBoolean()) {
             routingTable.remove(MlConfigIndex.indexName());
         } else {
@@ -191,12 +192,13 @@ public void testIsAbleToRun() {
                 .addIndexShard(new IndexShardRoutingTable.Builder(shardId).addShard(shardRouting).build()));
         }
 
+        csBuilder = ClusterState.builder(clusterState);
         csBuilder.routingTable(routingTable.build());
         csBuilder.metadata(metadata);
-        assertThat(updater.isAbleToRun(csBuilder.build()), is(false));
+        final ClusterState csUpdated = csBuilder.build();
+        assertThat(updater.isAbleToRun(csUpdated), is(false));
 
-        csBuilder.metadata(Metadata.EMPTY_METADATA);
-        assertThat(updater.isAbleToRun(csBuilder.build()), is(true));
+        assertThat(updater.isAbleToRun(ClusterState.builder(csUpdated).metadata(Metadata.EMPTY_METADATA).build()), is(true));
     }
 
     private void withDatafeed(String datafeedId, boolean aggsRewritten) {