More tests and fixes

Author: edsavage

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAlias.java

@@ -47,6 +47,7 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Comparator;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Predicate;
 import java.util.regex.Pattern;
@@ -79,7 +80,7 @@ public final class MlIndexAndAlias {
     private static final Predicate<String> IS_ANOMALIES_SHARED_INDEX = Pattern.compile(
         AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX + AnomalyDetectorsIndexFields.RESULTS_INDEX_DEFAULT + "-\\d{6}"
     ).asMatchPredicate();
-    private static final String ROLLOVER_ALIAS_SUFFIX = ".rollover_alias";
+    public static final String ROLLOVER_ALIAS_SUFFIX = ".rollover_alias";
 
     static final Comparator<String> INDEX_NAME_COMPARATOR = (index1, index2) -> {
         String[] index1Parts = index1.split("-");
@@ -577,18 +578,20 @@ public static void rollover(Client client, RolloverRequest rolloverRequest, Acti
     }
 
     public static Tuple<String, String> createRolloverAliasAndNewIndexName(String index) {
+        String indexName = Objects.requireNonNull(index);
+
         // Create an alias specifically for rolling over.
         // The ml-anomalies index has aliases for each job, any
         // of which could be used but that means one alias is
         // treated differently.
-        // Using a `.` in the alias name avoids any conflicts
+        // ROLLOVER_ALIAS_SUFFIX puts a `.` in the alias name to avoid any conflicts
         // as AD job Ids cannot start with `.`
-        String rolloverAlias = index + ROLLOVER_ALIAS_SUFFIX;
+        String rolloverAlias = indexName + ROLLOVER_ALIAS_SUFFIX;
 
         // If the index does not end in a digit then rollover does not know
         // what to name the new index so it must be specified in the request.
         // Otherwise leave null and rollover will calculate the new name
-        String newIndexName = MlIndexAndAlias.has6DigitSuffix(index) ? null : index + MlIndexAndAlias.FIRST_INDEX_SIX_DIGIT_SUFFIX;
+        String newIndexName = MlIndexAndAlias.has6DigitSuffix(index) ? null : indexName + MlIndexAndAlias.FIRST_INDEX_SIX_DIGIT_SUFFIX;
 
         return new Tuple<>(rolloverAlias, newIndexName);
     }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAliasTests.java

@@ -404,6 +404,37 @@ public void testHas6DigitSuffix() {
         assertFalse(MlIndexAndAlias.has6DigitSuffix("index000001"));
     }
 
+    public void testIsAnomaliesSharedIndex() {
+        assertTrue(MlIndexAndAlias.isAnomaliesSharedIndex(".ml-anomalies-shared-000001"));
+        assertTrue(MlIndexAndAlias.isAnomaliesSharedIndex(".ml-anomalies-shared-000007"));
+        assertTrue(MlIndexAndAlias.isAnomaliesSharedIndex(".ml-anomalies-shared-100000"));
+        assertTrue(MlIndexAndAlias.isAnomaliesSharedIndex(".ml-anomalies-shared-999999"));
+        assertFalse(MlIndexAndAlias.isAnomaliesSharedIndex(".ml-anomalies-shared-1000000"));
+        assertFalse(MlIndexAndAlias.isAnomaliesSharedIndex(".ml-anomalies-shared-00001"));
+        assertFalse(MlIndexAndAlias.isAnomaliesSharedIndex(".ml-anomalies-custom-fred-000007"));
+        assertFalse(MlIndexAndAlias.isAnomaliesSharedIndex("shared-000007"));
+        assertFalse(MlIndexAndAlias.isAnomaliesSharedIndex(".ml-anomalies-shared"));
+        assertFalse(MlIndexAndAlias.isAnomaliesSharedIndex(".ml-anomalies-shared000007"));
+        assertFalse(MlIndexAndAlias.isAnomaliesSharedIndex(".ml-anomalies-state-000007"));
+        assertFalse(MlIndexAndAlias.isAnomaliesSharedIndex(".ml-anomalies-annotations-000007"));
+        assertFalse(MlIndexAndAlias.isAnomaliesSharedIndex(".ml-anomalies-stats-000007"));
+    }
+
+    public void testCreateRolloverAliasAndNewIndexName() {
+        var alias_index1 = MlIndexAndAlias.createRolloverAliasAndNewIndexName("fred");
+        assertThat(alias_index1.v1(), equalTo("fred" + MlIndexAndAlias.ROLLOVER_ALIAS_SUFFIX));
+        assertThat(alias_index1.v2(), equalTo("fred" + MlIndexAndAlias.FIRST_INDEX_SIX_DIGIT_SUFFIX));
+
+        var alias_index2 = MlIndexAndAlias.createRolloverAliasAndNewIndexName("derf" + MlIndexAndAlias.FIRST_INDEX_SIX_DIGIT_SUFFIX);
+        assertThat(
+            alias_index2.v1(),
+            equalTo("derf" + MlIndexAndAlias.FIRST_INDEX_SIX_DIGIT_SUFFIX + MlIndexAndAlias.ROLLOVER_ALIAS_SUFFIX)
+        );
+        assertThat(alias_index2.v2(), equalTo(null));
+
+        assertThrows(NullPointerException.class, () -> MlIndexAndAlias.createRolloverAliasAndNewIndexName(null));
+    }
+
     public void testLatestIndexMatchingBaseName_isLatest() {
         Metadata.Builder metadata = Metadata.builder();
         metadata.put(createSharedResultsIndex(".ml-anomalies-custom-foo", IndexVersion.current(), List.of("job1")));

x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/JobResultsProviderIT.java

@@ -155,7 +155,8 @@ public void testPutJob_CreatesResultsIndex() {
         // Put fist job. This should create the results index as it's the first job.
         client().execute(PutJobAction.INSTANCE, new PutJobAction.Request(job1)).actionGet();
 
-        String sharedResultsIndex = AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX + AnomalyDetectorsIndexFields.RESULTS_INDEX_DEFAULT;
+        String sharedResultsIndex = AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX + AnomalyDetectorsIndexFields.RESULTS_INDEX_DEFAULT
+            + MlIndexAndAlias.FIRST_INDEX_SIX_DIGIT_SUFFIX;
         Map<String, Object> mappingProperties = getIndexMappingProperties(sharedResultsIndex);
 
         // Assert mappings have a few fields from the template