496: Removed unused map property in metada alias rollover tests

Signed-off-by: Stevan Buzejic <[email protected]>
opensearch-project · Sep 20, 2022 · 4f1adea · 4f1adea
1 parent d22c702
commit 4f1adea
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -70,6 +70,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Fixed flaky test `ResourceAwareTasksTests.testTaskIdPersistsInThreadContext` ([#4484](https://github.com/opensearch-project/OpenSearch/pull/4484))
 - Fixed the ignore_malformed setting to also ignore objects ([#4494](https://github.com/opensearch-project/OpenSearch/pull/4494))
 - Updated jackson to 2.13.4 and snakeyml to 1.32 ([#4556](https://github.com/opensearch-project/OpenSearch/pull/4556))
+- [Bug]: Alias filter lost after rollover ([#4499](https://github.com/opensearch-project/OpenSearch/pull/4499))
 
 ### Security
 - CVE-2022-25857 org.yaml:snakeyaml DOS vulnerability ([#4341](https://github.com/opensearch-project/OpenSearch/pull/4341))
@@ -98,4 +99,4 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 
 [Unreleased]: https://github.com/opensearch-project/OpenSearch/compare/2.2.0...HEAD
-[2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.2.0...2.x
+[2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.2.0...2.x
diff --git a/...r/src/main/java/org/opensearch/action/admin/indices/rollover/MetadataRolloverService.java b/...r/src/main/java/org/opensearch/action/admin/indices/rollover/MetadataRolloverService.java
@@ -312,17 +312,43 @@ static List<AliasAction> rolloverAliasToNewIndex(
         AliasMetadata aliasMetada,
         String alias
     ) {
+        String filterAsString = aliasMetada.getFilter() != null ? aliasMetada.getFilter().string() : null;
+
         if (explicitWriteIndex) {
             return Collections.unmodifiableList(
                 Arrays.asList(
-                    new AliasAction.Add(newIndex, alias, aliasMetada.getFilterAsString(), aliasMetada.getIndexRouting(), aliasMetada.getSearchRouting(), true, aliasMetada.isHidden()),
-                    new AliasAction.Add(oldIndex, alias, aliasMetada.getFilterAsString(), aliasMetada.getIndexRouting(), aliasMetada.getSearchRouting(), false, aliasMetada.isHidden())
+                    new AliasAction.Add(
+                        newIndex,
+                        alias,
+                        filterAsString,
+                        aliasMetada.getIndexRouting(),
+                        aliasMetada.getSearchRouting(),
+                        true,
+                        aliasMetada.isHidden()
+                    ),
+                    new AliasAction.Add(
+                        oldIndex,
+                        alias,
+                        filterAsString,
+                        aliasMetada.getIndexRouting(),
+                        aliasMetada.getSearchRouting(),
+                        false,
+                        aliasMetada.isHidden()
+                    )
                 )
             );
         } else {
             return Collections.unmodifiableList(
                 Arrays.asList(
-                    new AliasAction.Add(newIndex, alias, aliasMetada.getFilterAsString(), aliasMetada.getIndexRouting(), aliasMetada.getSearchRouting(), null, aliasMetada.isHidden()),
+                    new AliasAction.Add(
+                        newIndex,
+                        alias,
+                        filterAsString,
+                        aliasMetada.getIndexRouting(),
+                        aliasMetada.getSearchRouting(),
+                        null,
+                        aliasMetada.isHidden()
+                    ),
                     new AliasAction.Remove(oldIndex, alias, null)
                 )
             );
@@ -385,8 +411,8 @@ static void validate(Metadata metadata, String rolloverTarget, String newIndexNa
                     + indexAbstraction.getType().getDisplayName()
                     + "] but one of ["
                     + Strings.collectionToCommaDelimitedString(
-                    VALID_ROLLOVER_TARGETS.stream().map(IndexAbstraction.Type::getDisplayName).collect(Collectors.toList())
-                )
+                        VALID_ROLLOVER_TARGETS.stream().map(IndexAbstraction.Type::getDisplayName).collect(Collectors.toList())
+                    )
                     + "] was expected"
             );
         }

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/AliasAction.java b/server/src/main/java/org/opensearch/cluster/metadata/AliasAction.java
@@ -138,11 +138,17 @@ public String getAlias() {
             return alias;
         }
 
-        public String getFilter(){ return filter; }
+        public String getFilter() {
+            return filter;
+        }
 
-        public String getSearchRouting() { return searchRouting; }
+        public String getSearchRouting() {
+            return searchRouting;
+        }
 
-        public String getIndexRouting() { return indexRouting; }
+        public String getIndexRouting() {
+            return indexRouting;
+        }
 
         public Boolean writeIndex() {
             return writeIndex;

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/AliasMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/AliasMetadata.java
@@ -129,13 +129,6 @@ public CompressedXContent getFilter() {
         return filter();
     }
 
-    public String getFilterAsString() {
-        if(filter == null){
-            return null;
-        }
-        return filter.string();
-    }
-
     public boolean filteringRequired() {
         return filter != null;
     }

diff --git a/.../test/java/org/opensearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java b/.../test/java/org/opensearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java
@@ -43,7 +43,6 @@
 import org.opensearch.cluster.DataStreamTestHelper;
 import org.opensearch.cluster.metadata.AliasAction;
 import org.opensearch.cluster.metadata.AliasMetadata;
-import org.opensearch.cluster.metadata.AliasMetadata.Builder;
 import org.opensearch.cluster.metadata.AliasValidator;
 import org.opensearch.cluster.metadata.ComponentTemplate;
 import org.opensearch.cluster.metadata.ComposableIndexTemplate;
@@ -101,7 +100,6 @@
 import java.util.Map;
 
 import static java.util.Collections.emptyMap;
-import static org.hamcrest.Matchers.in;
 import static org.opensearch.cluster.DataStreamTestHelper.generateMapping;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
@@ -128,7 +126,13 @@ public void testRolloverAliasActions() {
         String sourceIndex = randomAlphaOfLength(10);
         String targetIndex = randomAlphaOfLength(10);
 
-        List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, false, createDefaultAliasMetadata(sourceAlias, null), sourceAlias);
+        List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(
+            sourceIndex,
+            targetIndex,
+            false,
+            createDefaultAliasMetadata(sourceAlias, null),
+            sourceAlias
+        );
         assertThat(actions, hasSize(2));
         boolean foundAdd = false;
         boolean foundRemove = false;
@@ -151,7 +155,13 @@ public void testRolloverAliasActionsWithExplicitWriteIndex() {
         String sourceAlias = randomAlphaOfLength(10);
         String sourceIndex = randomAlphaOfLength(10);
         String targetIndex = randomAlphaOfLength(10);
-        List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, true, createDefaultAliasMetadata(sourceAlias, null), sourceAlias);
+        List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(
+            sourceIndex,
+            targetIndex,
+            true,
+            createDefaultAliasMetadata(sourceAlias, null),
+            sourceAlias
+        );
 
         assertThat(actions, hasSize(2));
         boolean foundAddWrite = false;
@@ -178,12 +188,23 @@ public void testRolloverAliasActionsWithFilterAndExplicitWriteIndex() {
         String sourceAlias = randomAlphaOfLength(10);
         String sourceIndex = randomAlphaOfLength(10);
         String targetIndex = randomAlphaOfLength(10);
-        Map filter = Collections.singletonMap(randomAlphaOfLength(2), randomAlphaOfLength(2));
         String indexRouting = randomAlphaOfLength(10);
         String sourceRouting = randomAlphaOfLength(10);
-        AliasMetadata aliasMetadata = createAliasMetadata(sourceAlias, filter, indexRouting, sourceRouting, true);
+        AliasMetadata aliasMetadata = createAliasMetadata(
+            sourceAlias,
+            Collections.singletonMap(randomAlphaOfLength(2), randomAlphaOfLength(2)),
+            indexRouting,
+            sourceRouting,
+            true
+        );
 
-        List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, true, aliasMetadata, sourceAlias);
+        List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(
+            sourceIndex,
+            targetIndex,
+            true,
+            aliasMetadata,
+            sourceAlias
+        );
 
         assertThat(actions, hasSize(2));
         boolean foundAddWrite = false;
@@ -192,7 +213,7 @@ public void testRolloverAliasActionsWithFilterAndExplicitWriteIndex() {
             AliasAction.Add addAction = (AliasAction.Add) action;
             if (action.getIndex().equals(targetIndex)) {
                 assertEquals(sourceAlias, addAction.getAlias());
-                assertEquals(aliasMetadata.getFilterAsString(), addAction.getFilter());
+                assertEquals(aliasMetadata.filter().string(), addAction.getFilter());
                 assertEquals(indexRouting, addAction.getIndexRouting());
                 assertEquals(sourceRouting, addAction.getSearchRouting());
 
@@ -214,7 +235,13 @@ public void testRolloverAliasActionsWithHiddenAliasAndExplicitWriteIndex() {
         String sourceAlias = randomAlphaOfLength(10);
         String sourceIndex = randomAlphaOfLength(10);
         String targetIndex = randomAlphaOfLength(10);
-        List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, true, createDefaultAliasMetadata(sourceAlias,true), sourceAlias);
+        List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(
+            sourceIndex,
+            targetIndex,
+            true,
+            createDefaultAliasMetadata(sourceAlias, true),
+            sourceAlias
+        );
 
         assertThat(actions, hasSize(2));
         boolean foundAddWrite = false;
@@ -244,12 +271,23 @@ public void testRolloverAliasActionsWithFilterAndHiddenAliasAndImplicitWriteInde
         String sourceAlias = randomAlphaOfLength(10);
         String sourceIndex = randomAlphaOfLength(10);
         String targetIndex = randomAlphaOfLength(10);
-        Map filter = Collections.singletonMap(randomAlphaOfLength(2), randomAlphaOfLength(2));
         String indexRouting = randomAlphaOfLength(10);
         String sourceRouting = randomAlphaOfLength(10);
-        AliasMetadata aliasMetadata = createAliasMetadata(sourceAlias, filter, indexRouting, sourceRouting, true);
+        AliasMetadata aliasMetadata = createAliasMetadata(
+            sourceAlias,
+            Collections.singletonMap(randomAlphaOfLength(2), randomAlphaOfLength(2)),
+            indexRouting,
+            sourceRouting,
+            true
+        );
 
-        List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, false,  aliasMetadata, sourceAlias);
+        List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(
+            sourceIndex,
+            targetIndex,
+            false,
+            aliasMetadata,
+            sourceAlias
+        );
 
         assertThat(actions, hasSize(2));
         boolean foundAddWrite = false;
@@ -261,7 +299,7 @@ public void testRolloverAliasActionsWithFilterAndHiddenAliasAndImplicitWriteInde
                 assertEquals(sourceAlias, addAction.getAlias());
                 assertThat(addAction.writeIndex(), nullValue());
                 assertTrue(addAction.isHidden());
-                assertEquals(aliasMetadata.getFilterAsString(), addAction.getFilter());
+                assertEquals(aliasMetadata.filter().string(), addAction.getFilter());
                 assertEquals(indexRouting, addAction.getIndexRouting());
                 assertEquals(sourceRouting, addAction.getSearchRouting());
                 foundAddWrite = true;
@@ -282,7 +320,13 @@ public void testRolloverAliasActionsWithHiddenAliasAndImplicitWriteIndex() {
         String sourceAlias = randomAlphaOfLength(10);
         String sourceIndex = randomAlphaOfLength(10);
         String targetIndex = randomAlphaOfLength(10);
-        List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(sourceIndex, targetIndex, false,  createDefaultAliasMetadata(sourceAlias,true), sourceAlias);
+        List<AliasAction> actions = MetadataRolloverService.rolloverAliasToNewIndex(
+            sourceIndex,
+            targetIndex,
+            false,
+            createDefaultAliasMetadata(sourceAlias, true),
+            sourceAlias
+        );
 
         assertThat(actions, hasSize(2));
         boolean foundAddWrite = false;
@@ -1091,9 +1135,14 @@ private static AliasMetadata createDefaultAliasMetadata(String alias, Boolean is
         return AliasMetadata.builder(alias).isHidden(isHidden).build();
     }
 
-    private static AliasMetadata createAliasMetadata(String alias, Map filter, String indexRouting, String searchRouting, Boolean isHidden) {
-        return AliasMetadata
-            .builder(alias)
+    private static AliasMetadata createAliasMetadata(
+        String alias,
+        Map filter,
+        String indexRouting,
+        String searchRouting,
+        Boolean isHidden
+    ) {
+        return AliasMetadata.builder(alias)
             .isHidden(isHidden)
             .filter(filter)
             .indexRouting(indexRouting)