[Metadata Immutability] Change different indices lookup objects from …

…array type to lists Changed the arrays to immutable List instances, added new versions of the getters which returns List instances. Resolves opensearch-project#8647 Signed-off-by: Abdul Muneer Kolarkunnu <[email protected]>
akolarkunnu · Jul 9, 2024 · 88cd3df · 88cd3df
1 parent f53e220
commit 88cd3df
Show file tree

Hide file tree

Showing 8 changed files with 24 additions and 72 deletions.
diff --git a/CHANGELOG-3.0.md b/CHANGELOG-3.0.md
@@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add task completion count in search backpressure stats API ([#10028](https://github.com/opensearch-project/OpenSearch/pull/10028/))
 - Deprecate CamelCase `PathHierarchy` tokenizer name in favor to lowercase `path_hierarchy` ([#10894](https://github.com/opensearch-project/OpenSearch/pull/10894))
 - Breaking change: Do not request "search_pipelines" metrics by default in NodesInfoRequest ([#12497](https://github.com/opensearch-project/OpenSearch/pull/12497))
+- Modify the utility APIs in the Metadata to get different indices ([#14557](https://github.com/opensearch-project/OpenSearch/pull/14557))
 
 ### Deprecated
 

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,7 +13,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add batching supported processor base type AbstractBatchingProcessor ([#14554](https://github.com/opensearch-project/OpenSearch/pull/14554))
 - Fix race condition while parsing derived fields from search definition ([14445](https://github.com/opensearch-project/OpenSearch/pull/14445))
 - Add allowlist setting for ingest-common processors ([#14439](https://github.com/opensearch-project/OpenSearch/issues/14439))
-- Add new APIs in the Metadata to get different indices in the form of List ([#14557](https://github.com/opensearch-project/OpenSearch/pull/14557))
 
 ### Dependencies
 - Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442))

diff --git a/...rc/main/java/org/opensearch/action/admin/cluster/health/TransportClusterHealthAction.java b/...rc/main/java/org/opensearch/action/admin/cluster/health/TransportClusterHealthAction.java
@@ -493,7 +493,7 @@ private ClusterHealthResponse clusterHealth(
         String[] concreteIndices;
         if (request.level().equals(ClusterHealthRequest.Level.AWARENESS_ATTRIBUTES)) {
             String awarenessAttribute = request.getAwarenessAttribute();
-            concreteIndices = clusterState.getMetadata().getConcreteAllIndices();
+            concreteIndices = clusterState.getMetadata().getConcreteAllIndices().toArray(String[]::new);
             return new ClusterHealthResponse(
                 clusterState.getClusterName().value(),
                 clusterState,

diff --git a/server/src/main/java/org/opensearch/cluster/health/ClusterStateHealth.java b/server/src/main/java/org/opensearch/cluster/health/ClusterStateHealth.java
@@ -73,7 +73,7 @@ public final class ClusterStateHealth implements Iterable<ClusterIndexHealth>, W
      * @param clusterState The current cluster state. Must not be null.
      */
     public ClusterStateHealth(final ClusterState clusterState) {
-        this(clusterState, clusterState.metadata().getConcreteAllIndices());
+        this(clusterState, clusterState.metadata().getConcreteAllIndices().toArray(String[]::new));
     }
 
     /**

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java
@@ -706,7 +706,7 @@ public Map<String, Set<String>> resolveSearchRoutingAllIndices(Metadata metadata
         if (routing != null) {
             Set<String> r = Sets.newHashSet(Strings.splitStringByCommaToArray(routing));
             Map<String, Set<String>> routings = new HashMap<>();
-            Set<String> concreteIndices = metadata.getConcreteAllIndicesList();
+            Set<String> concreteIndices = metadata.getConcreteAllIndices();
             for (String index : concreteIndices) {
                 routings.put(index, r);
             }
@@ -746,7 +746,7 @@ static boolean isExplicitAllPattern(Collection<String> aliasesOrIndices) {
      */
     boolean isPatternMatchingAllIndices(Metadata metadata, String[] indicesOrAliases, String[] concreteIndices) {
         // if we end up matching on all indices, check, if its a wildcard parameter, or a "-something" structure
-        if (concreteIndices.length == metadata.getConcreteAllIndicesList().size() && indicesOrAliases.length > 0) {
+        if (concreteIndices.length == metadata.getConcreteAllIndices().size() && indicesOrAliases.length > 0) {
 
             // we might have something like /-test1,+test1 that would identify all indices
             // or something like /-test1 with test1 index missing and IndicesOptions.lenient()
@@ -1182,17 +1182,17 @@ private boolean isEmptyOrTrivialWildcard(List<String> expressions) {
 
         private static List<String> resolveEmptyOrTrivialWildcard(IndicesOptions options, Metadata metadata) {
             if (options.expandWildcardsOpen() && options.expandWildcardsClosed() && options.expandWildcardsHidden()) {
-                return new ArrayList<>(metadata.getConcreteAllIndicesList());
+                return List.copyOf(metadata.getConcreteAllIndices());
             } else if (options.expandWildcardsOpen() && options.expandWildcardsClosed()) {
-                return metadata.getConcreteVisibleIndicesList();
+                return metadata.getConcreteVisibleIndices();
             } else if (options.expandWildcardsOpen() && options.expandWildcardsHidden()) {
-                return metadata.getConcreteAllOpenIndicesList();
+                return metadata.getConcreteAllOpenIndices();
             } else if (options.expandWildcardsOpen()) {
-                return metadata.getConcreteVisibleOpenIndicesList();
+                return metadata.getConcreteVisibleOpenIndices();
             } else if (options.expandWildcardsClosed() && options.expandWildcardsHidden()) {
-                return metadata.getConcreteAllClosedIndicesList();
+                return metadata.getConcreteAllClosedIndices();
             } else if (options.expandWildcardsClosed()) {
-                return metadata.getConcreteVisibleClosedIndicesList();
+                return metadata.getConcreteVisibleClosedIndices();
             } else {
                 return Collections.emptyList();
             }

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java
@@ -601,86 +601,44 @@ private static String mergePaths(String path, String field) {
     }
 
     /**
-     * Returns all the concrete indices in the format of array of strings.
+     * Returns all the concrete indices.
      */
-    public String[] getConcreteAllIndices() {
-        return allIndices.toArray(String[]::new);
-    }
-
-    /**
-     * Returns all the concrete indices in the format of set of strings.
-     */
-    public Set<String> getConcreteAllIndicesList() {
+    public Set<String> getConcreteAllIndices() {
         return allIndices;
     }
 
     /**
-     * Returns all the concrete indices that are not hidden in the format of array of strings.
-     */
-    public String[] getConcreteVisibleIndices() {
-        return visibleIndices.toArray(String[]::new);
-    }
-
-    /**
-     * Returns all the concrete indices that are not hidden in the format of list of strings.
+     * Returns all the concrete indices that are not hidden.
      */
-    public List<String> getConcreteVisibleIndicesList() {
+    public List<String> getConcreteVisibleIndices() {
         return visibleIndices;
     }
 
     /**
-     * Returns all of the concrete indices that are open in the format of array of strings.
-     */
-    public String[] getConcreteAllOpenIndices() {
-        return allOpenIndices.toArray(String[]::new);
-    }
-
-    /**
-     * Returns all of the concrete indices that are open in the format of list of strings.
+     * Returns all of the concrete indices that are open.
      */
-    public List<String> getConcreteAllOpenIndicesList() {
+    public List<String> getConcreteAllOpenIndices() {
         return allOpenIndices;
     }
 
     /**
-     * Returns all of the concrete indices that are open and not hidden in the format of array of strings.
+     * Returns all of the concrete indices that are open and not hidden.
      */
-    public String[] getConcreteVisibleOpenIndices() {
-        return visibleOpenIndices.toArray(String[]::new);
-    }
-
-    /**
-     * Returns all of the concrete indices that are open and not hidden in the format of list of strings.
-     */
-    public List<String> getConcreteVisibleOpenIndicesList() {
+    public List<String> getConcreteVisibleOpenIndices() {
         return visibleOpenIndices;
     }
 
     /**
-     * Returns all of the concrete indices that are closed in the format of array of strings.
-     */
-    public String[] getConcreteAllClosedIndices() {
-        return allClosedIndices.toArray(String[]::new);
-    }
-
-    /**
-     * Returns all of the concrete indices that are closed in the format of list of strings.
+     * Returns all of the concrete indices that are closed.
      */
-    public List<String> getConcreteAllClosedIndicesList() {
+    public List<String> getConcreteAllClosedIndices() {
         return allClosedIndices;
     }
 
     /**
-     * Returns all of the concrete indices that are closed and not hidden in the format of array of strings.
-     */
-    public String[] getConcreteVisibleClosedIndices() {
-        return visibleClosedIndices.toArray(String[]::new);
-    }
-
-    /**
-     * Returns all of the concrete indices that are closed and not hidden in the format of list of strings.
+     * Returns all of the concrete indices that are closed and not hidden.
      */
-    public List<String> getConcreteVisibleClosedIndicesList() {
+    public List<String> getConcreteVisibleClosedIndices() {
         return visibleClosedIndices;
     }
 

diff --git a/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java b/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java
@@ -165,7 +165,7 @@ public RemoteRestoreResult restore(
             }
         } else {
             List<String> filteredIndices = filterIndices(
-                List.of(currentState.metadata().getConcreteAllIndices()),
+                List.copyOf(currentState.metadata().getConcreteAllIndices()),
                 indexNames,
                 IndicesOptions.fromOptions(true, true, true, true)
             );

diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java
@@ -1599,17 +1599,11 @@ private static void compareMetadata(
         if (compareIndicesLookups == true) {
             assertEquals(previousMetadata.indices(), newMetadata.indices());
             assertEquals(previousMetadata.getConcreteAllIndices(), newMetadata.getConcreteAllIndices());
-            assertEquals(previousMetadata.getConcreteAllIndicesList(), newMetadata.getConcreteAllIndicesList());
             assertEquals(previousMetadata.getConcreteAllClosedIndices(), newMetadata.getConcreteAllClosedIndices());
-            assertEquals(previousMetadata.getConcreteAllClosedIndicesList(), newMetadata.getConcreteAllClosedIndicesList());
             assertEquals(previousMetadata.getConcreteAllOpenIndices(), newMetadata.getConcreteAllOpenIndices());
-            assertEquals(previousMetadata.getConcreteAllOpenIndicesList(), newMetadata.getConcreteAllOpenIndicesList());
             assertEquals(previousMetadata.getConcreteVisibleIndices(), newMetadata.getConcreteVisibleIndices());
-            assertEquals(previousMetadata.getConcreteVisibleIndicesList(), newMetadata.getConcreteVisibleIndicesList());
             assertEquals(previousMetadata.getConcreteVisibleClosedIndices(), newMetadata.getConcreteVisibleClosedIndices());
-            assertEquals(previousMetadata.getConcreteVisibleClosedIndicesList(), newMetadata.getConcreteVisibleClosedIndicesList());
             assertEquals(previousMetadata.getConcreteVisibleOpenIndices(), newMetadata.getConcreteVisibleOpenIndices());
-            assertEquals(previousMetadata.getConcreteVisibleOpenIndicesList(), newMetadata.getConcreteVisibleOpenIndicesList());
             assertEquals(previousMetadata.getIndicesLookup(), newMetadata.getIndicesLookup());
             assertEquals(previousMetadata.getCustoms().get(DataStreamMetadata.TYPE), newMetadata.getCustoms().get(DataStreamMetadata.TYPE));
         }