Prefer using builder for ComposableIndexTemplate (#101760)

There's a recurring pattern where we make a copy of a
`ComposableIndexTemplate` while changing a single property. We tend to
use the constructor and manually supply all properties.

This isn't very robust when adding properties to the class as you can
easily overlook supplying the new property to the copied instance.

It also leads to a proliferation of overloaded constructors as changing
all callers to use the new constructor is sometimes not feasible as it
would create a very large change set. Also, we'd mostly be supplying
`null` values for newly added properties in existing test cases anyway.
Often, tests are just setting one or two properties and the rest are
null values, which doesn't make the tests very readable.

The `ComposableIndexTemplate` class already has a `Builder` class. This
PR deprecates all constructors of `ComposableIndexTemplate` in favor of
using `ComposableIndexTemplate.Bulider`. Changing all constructor call
to use the builder everywhere is not feasible as there are too many
usages. But the PR uses the builder for all instances where we make a
copy with a minor modification.

We can think about doing the same for other classes but this is the one
where it seems most important based on the large number of constructor
overloads and the bugs or almost committed bugs related to this class.
Also, conveniently, there's already a builder class for it.

modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamGetWriteIndexTests.java

@@ -290,7 +290,8 @@ public void cleanup() {
     }
 
     private ClusterState createInitialState() {
-        ComposableIndexTemplate template = new ComposableIndexTemplate.Builder().indexPatterns(List.of("logs-*"))
+        ComposableIndexTemplate template = ComposableIndexTemplate.builder()
+            .indexPatterns(List.of("logs-*"))
             .template(
                 new Template(Settings.builder().put("index.mode", "time_series").put("index.routing_path", "uid").build(), null, null)
             )

modules/data-streams/src/test/java/org/elasticsearch/datastreams/MetadataDataStreamRolloverServiceTests.java

@@ -71,7 +71,8 @@ public void testRolloverClusterStateForDataStream() throws Exception {
             false,
             IndexMode.TIME_SERIES
         );
-        ComposableIndexTemplate template = new ComposableIndexTemplate.Builder().indexPatterns(List.of(dataStream.getName() + "*"))
+        ComposableIndexTemplate template = ComposableIndexTemplate.builder()
+            .indexPatterns(List.of(dataStream.getName() + "*"))
             .template(
                 new Template(Settings.builder().put("index.mode", "time_series").put("index.routing_path", "uid").build(), null, null)
             )
@@ -176,7 +177,8 @@ public void testRolloverAndMigrateDataStream() throws Exception {
             false,
             dsIndexMode
         );
-        ComposableIndexTemplate template = new ComposableIndexTemplate.Builder().indexPatterns(List.of(dataStream.getName() + "*"))
+        ComposableIndexTemplate template = ComposableIndexTemplate.builder()
+            .indexPatterns(List.of(dataStream.getName() + "*"))
             .template(
                 new Template(Settings.builder().put("index.mode", "time_series").put("index.routing_path", "uid").build(), null, null)
             )
@@ -262,7 +264,8 @@ public void testChangingIndexModeFromTimeSeriesToSomethingElseNoEffectOnExisting
             false,
             IndexMode.TIME_SERIES
         );
-        ComposableIndexTemplate template = new ComposableIndexTemplate.Builder().indexPatterns(List.of(dataStream.getName() + "*"))
+        ComposableIndexTemplate template = ComposableIndexTemplate.builder()
+            .indexPatterns(List.of(dataStream.getName() + "*"))
             .template(
                 new Template(Settings.builder().put("index.mode", "time_series").put("index.routing_path", "uid").build(), null, null)
             )
@@ -477,7 +480,8 @@ private static ClusterState createClusterState(String dataStreamName, int number
             false,
             null
         );
-        ComposableIndexTemplate template = new ComposableIndexTemplate.Builder().indexPatterns(List.of(dataStream.getName() + "*"))
+        ComposableIndexTemplate template = ComposableIndexTemplate.builder()
+            .indexPatterns(List.of(dataStream.getName() + "*"))
             .template(
                 new Template(Settings.builder().put("index.mode", "time_series").put("index.routing_path", "uid").build(), null, null)
             )

server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java

@@ -107,6 +107,14 @@ public static ComposableIndexTemplate parse(XContentParser parser) throws IOExce
         return PARSER.parse(parser, null);
     }
 
+    public static Builder builder() {
+        return new Builder();
+    }
+
+    /**
+     * @deprecated use {@link Builder} instead
+     */
+    @Deprecated(forRemoval = true)
     public ComposableIndexTemplate(
         List<String> indexPatterns,
         @Nullable Template template,
@@ -115,9 +123,13 @@ public ComposableIndexTemplate(
         @Nullable Long version,
         @Nullable Map<String, Object> metadata
     ) {
-        this(indexPatterns, template, componentTemplates, priority, version, metadata, null, null, null);
+        this(indexPatterns, template, componentTemplates, priority, version, metadata, null, null, null, null);
     }
 
+    /**
+     * @deprecated use {@link Builder} instead
+     */
+    @Deprecated(forRemoval = true)
     public ComposableIndexTemplate(
         List<String> indexPatterns,
         @Nullable Template template,
@@ -127,9 +139,13 @@ public ComposableIndexTemplate(
         @Nullable Map<String, Object> metadata,
         @Nullable DataStreamTemplate dataStreamTemplate
     ) {
-        this(indexPatterns, template, componentTemplates, priority, version, metadata, dataStreamTemplate, null, null);
+        this(indexPatterns, template, componentTemplates, priority, version, metadata, dataStreamTemplate, null, null, null);
     }
 
+    /**
+     * @deprecated use {@link Builder} instead
+     */
+    @Deprecated(forRemoval = true)
     public ComposableIndexTemplate(
         List<String> indexPatterns,
         @Nullable Template template,
@@ -140,34 +156,13 @@ public ComposableIndexTemplate(
         @Nullable DataStreamTemplate dataStreamTemplate,
         @Nullable Boolean allowAutoCreate
     ) {
-        this(indexPatterns, template, componentTemplates, priority, version, metadata, dataStreamTemplate, allowAutoCreate, null);
-    }
-
-    ComposableIndexTemplate(
-        List<String> indexPatterns,
-        @Nullable Template template,
-        @Nullable List<String> componentTemplates,
-        @Nullable Long priority,
-        @Nullable Long version,
-        @Nullable Map<String, Object> metadata,
-        @Nullable DataStreamTemplate dataStreamTemplate,
-        @Nullable Boolean allowAutoCreate,
-        @Nullable List<String> ignoreMissingComponentTemplates
-    ) {
-        this(
-            indexPatterns,
-            template,
-            componentTemplates,
-            priority,
-            version,
-            metadata,
-            dataStreamTemplate,
-            allowAutoCreate,
-            ignoreMissingComponentTemplates,
-            null
-        );
+        this(indexPatterns, template, componentTemplates, priority, version, metadata, dataStreamTemplate, allowAutoCreate, null, null);
     }
 
+    /**
+     * @deprecated use {@link Builder} instead
+     */
+    @Deprecated(forRemoval = true)
     public ComposableIndexTemplate(
         List<String> indexPatterns,
         @Nullable Template template,
@@ -287,10 +282,6 @@ public List<String> getIgnoreMissingComponentTemplates() {
         return ignoreMissingComponentTemplates;
     }
 
-    public Boolean deprecated() {
-        return deprecated;
-    }
-
     public boolean isDeprecated() {
         return Boolean.TRUE.equals(deprecated);
     }
@@ -412,6 +403,10 @@ static boolean componentTemplatesEquals(List<String> c1, List<String> c2) {
         return false;
     }
 
+    public Builder toBuilder() {
+        return new Builder(this);
+    }
+
     @Override
     public String toString() {
         return Strings.toString(this);
@@ -535,8 +530,25 @@ public static class Builder {
         private List<String> ignoreMissingComponentTemplates;
         private Boolean deprecated;
 
+        /**
+         * @deprecated use {@link ComposableIndexTemplate#builder()}
+         */
+        @Deprecated(forRemoval = true)
         public Builder() {}
 
+        private Builder(ComposableIndexTemplate template) {
+            this.indexPatterns = template.indexPatterns;
+            this.template = template.template;
+            this.componentTemplates = template.componentTemplates;
+            this.priority = template.priority;
+            this.version = template.version;
+            this.metadata = template.metadata;
+            this.dataStreamTemplate = template.dataStreamTemplate;
+            this.allowAutoCreate = template.allowAutoCreate;
+            this.ignoreMissingComponentTemplates = template.ignoreMissingComponentTemplates;
+            this.deprecated = template.deprecated;
+        }
+
         public Builder indexPatterns(List<String> indexPatterns) {
             this.indexPatterns = indexPatterns;
             return this;

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java

@@ -615,18 +615,7 @@ public ClusterState addIndexTemplateV2(
             CompressedXContent mappings = innerTemplate.mappings();
             CompressedXContent wrappedMappings = wrapMappingsIfNecessary(mappings, xContentRegistry);
             final Template finalTemplate = new Template(finalSettings, wrappedMappings, innerTemplate.aliases(), innerTemplate.lifecycle());
-            finalIndexTemplate = new ComposableIndexTemplate(
-                template.indexPatterns(),
-                finalTemplate,
-                template.composedOf(),
-                template.priority(),
-                template.version(),
-                template.metadata(),
-                template.getDataStreamTemplate(),
-                template.getAllowAutoCreate(),
-                template.getIgnoreMissingComponentTemplates(),
-                template.deprecated()
-            );
+            finalIndexTemplate = template.toBuilder().template(finalTemplate).build();
         }
 
         if (finalIndexTemplate.equals(existing)) {
@@ -713,23 +702,16 @@ private void validateIndexTemplateV2(String name, ComposableIndexTemplate indexT
         // Then apply settings resolved from templates:
         finalSettings.put(finalTemplate.map(Template::settings).orElse(Settings.EMPTY));
 
-        var templateToValidate = new ComposableIndexTemplate(
-            indexTemplate.indexPatterns(),
-            new Template(
-                finalSettings.build(),
-                finalTemplate.map(Template::mappings).orElse(null),
-                finalTemplate.map(Template::aliases).orElse(null),
-                finalTemplate.map(Template::lifecycle).orElse(null)
-            ),
-            indexTemplate.composedOf(),
-            indexTemplate.priority(),
-            indexTemplate.version(),
-            indexTemplate.metadata(),
-            indexTemplate.getDataStreamTemplate(),
-            indexTemplate.getAllowAutoCreate(),
-            indexTemplate.getIgnoreMissingComponentTemplates(),
-            indexTemplate.deprecated()
-        );
+        var templateToValidate = indexTemplate.toBuilder()
+            .template(
+                new Template(
+                    finalSettings.build(),
+                    finalTemplate.map(Template::mappings).orElse(null),
+                    finalTemplate.map(Template::aliases).orElse(null),
+                    finalTemplate.map(Template::lifecycle).orElse(null)
+                )
+            )
+            .build();
 
         validate(name, templateToValidate);
         validateDataStreamsStillReferenced(currentState, name, templateToValidate);

server/src/test/java/org/elasticsearch/action/admin/indices/create/AutoCreateActionTests.java

@@ -25,17 +25,19 @@ public void testResolveTemplates() {
         {
             Metadata.Builder mdBuilder = new Metadata.Builder();
             DataStreamTemplate dataStreamTemplate = new DataStreamTemplate();
-            mdBuilder.put("1", new ComposableIndexTemplate.Builder().indexPatterns(List.of("legacy-logs-*")).priority(10L).build());
+            mdBuilder.put("1", ComposableIndexTemplate.builder().indexPatterns(List.of("legacy-logs-*")).priority(10L).build());
             mdBuilder.put(
                 "2",
-                new ComposableIndexTemplate.Builder().indexPatterns(List.of("logs-*"))
+                ComposableIndexTemplate.builder()
+                    .indexPatterns(List.of("logs-*"))
                     .priority(20L)
                     .dataStreamTemplate(dataStreamTemplate)
                     .build()
             );
             mdBuilder.put(
                 "3",
-                new ComposableIndexTemplate.Builder().indexPatterns(List.of("logs-*"))
+                ComposableIndexTemplate.builder()
+                    .indexPatterns(List.of("logs-*"))
                     .priority(30L)
                     .dataStreamTemplate(dataStreamTemplate)
                     .build()

server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java

@@ -348,7 +348,8 @@ public void testRejectDuplicateAliasV2() {
         Map<String, AliasMetadata> aliases = new HashMap<>();
         aliases.put("foo-write", AliasMetadata.builder("foo-write").build());
         aliases.put("bar-write", AliasMetadata.builder("bar-write").writeIndex(randomBoolean()).build());
-        final ComposableIndexTemplate template = new ComposableIndexTemplate.Builder().indexPatterns(Arrays.asList("foo-*", "bar-*"))
+        final ComposableIndexTemplate template = ComposableIndexTemplate.builder()
+            .indexPatterns(Arrays.asList("foo-*", "bar-*"))
             .template(new Template(null, null, aliases))
             .build();
 
@@ -370,7 +371,8 @@ public void testRejectDuplicateAliasV2UsingComponentTemplates() {
         aliases.put("foo-write", AliasMetadata.builder("foo-write").build());
         aliases.put("bar-write", AliasMetadata.builder("bar-write").writeIndex(randomBoolean()).build());
         final ComponentTemplate ct = new ComponentTemplate(new Template(null, null, aliases), null, null);
-        final ComposableIndexTemplate template = new ComposableIndexTemplate.Builder().indexPatterns(Arrays.asList("foo-*", "bar-*"))
+        final ComposableIndexTemplate template = ComposableIndexTemplate.builder()
+            .indexPatterns(Arrays.asList("foo-*", "bar-*"))
             .componentTemplates(Collections.singletonList("ct"))
             .build();
 
@@ -396,9 +398,10 @@ public void testRolloverDoesntRejectOperationIfValidComposableTemplateOverridesL
             .build();
 
         // v2 template overrides the v1 template and does not define the rollover aliases
-        final ComposableIndexTemplate composableTemplate = new ComposableIndexTemplate.Builder().indexPatterns(
-            Arrays.asList("foo-*", "bar-*")
-        ).template(new Template(null, null, null)).build();
+        final ComposableIndexTemplate composableTemplate = ComposableIndexTemplate.builder()
+            .indexPatterns(Arrays.asList("foo-*", "bar-*"))
+            .template(new Template(null, null, null))
+            .build();
 
         final Metadata metadata = Metadata.builder()
             .put(createMetadata(randomAlphaOfLengthBetween(5, 7)), false)
@@ -441,7 +444,8 @@ public void testHiddenAffectsResolvedV2Templates() {
         Map<String, AliasMetadata> aliases = new HashMap<>();
         aliases.put("foo-write", AliasMetadata.builder("foo-write").build());
         aliases.put("bar-write", AliasMetadata.builder("bar-write").writeIndex(randomBoolean()).build());
-        final ComposableIndexTemplate template = new ComposableIndexTemplate.Builder().indexPatterns(Collections.singletonList("*"))
+        final ComposableIndexTemplate template = ComposableIndexTemplate.builder()
+            .indexPatterns(Collections.singletonList("*"))
             .template(new Template(null, null, aliases))
             .build();
 
@@ -472,7 +476,8 @@ public void testHiddenAffectsResolvedV2ComponentTemplates() {
         aliases.put("foo-write", AliasMetadata.builder("foo-write").build());
         aliases.put("bar-write", AliasMetadata.builder("bar-write").writeIndex(randomBoolean()).build());
         final ComponentTemplate ct = new ComponentTemplate(new Template(null, null, aliases), null, null);
-        final ComposableIndexTemplate template = new ComposableIndexTemplate.Builder().indexPatterns(Collections.singletonList("*"))
+        final ComposableIndexTemplate template = ComposableIndexTemplate.builder()
+            .indexPatterns(Collections.singletonList("*"))
             .componentTemplates(Collections.singletonList("ct"))
             .build();
 
@@ -575,7 +580,8 @@ public void testRolloverClusterStateForDataStream() throws Exception {
         final DataStream dataStream = DataStreamTestHelper.randomInstance()
             // ensure no replicate data stream
             .promoteDataStream();
-        ComposableIndexTemplate template = new ComposableIndexTemplate.Builder().indexPatterns(List.of(dataStream.getName() + "*"))
+        ComposableIndexTemplate template = ComposableIndexTemplate.builder()
+            .indexPatterns(List.of(dataStream.getName() + "*"))
             .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
             .build();
         Metadata.Builder builder = Metadata.builder();
@@ -651,7 +657,8 @@ public void testValidation() throws Exception {
             rolloverTarget = dataStream.getName();
             sourceIndexName = dataStream.getIndices().get(dataStream.getIndices().size() - 1).getName();
             defaultRolloverIndexName = DataStream.getDefaultBackingIndexName(dataStream.getName(), dataStream.getGeneration() + 1);
-            ComposableIndexTemplate template = new ComposableIndexTemplate.Builder().indexPatterns(List.of(dataStream.getName() + "*"))
+            ComposableIndexTemplate template = ComposableIndexTemplate.builder()
+                .indexPatterns(List.of(dataStream.getName() + "*"))
                 .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
                 .build();
             builder.put("template", template);

server/src/test/java/org/elasticsearch/action/admin/indices/template/post/SimulateIndexTemplateRequestTests.java

@@ -52,9 +52,7 @@ public void testIndexNameCannotBeNullOrEmpty() {
 
     public void testAddingGlobalTemplateWithHiddenIndexSettingIsIllegal() {
         Template template = new Template(Settings.builder().put(IndexMetadata.SETTING_INDEX_HIDDEN, true).build(), null, null);
-        ComposableIndexTemplate globalTemplate = new ComposableIndexTemplate.Builder().indexPatterns(List.of("*"))
-            .template(template)
-            .build();
+        ComposableIndexTemplate globalTemplate = ComposableIndexTemplate.builder().indexPatterns(List.of("*")).template(template).build();
 
         PutComposableIndexTemplateAction.Request request = new PutComposableIndexTemplateAction.Request("test");
         request.indexTemplate(globalTemplate);

server/src/test/java/org/elasticsearch/action/admin/indices/template/post/SimulateTemplateRequestTests.java

@@ -55,9 +55,7 @@ public void testIndexNameCannotBeNullOrEmpty() {
 
     public void testAddingGlobalTemplateWithHiddenIndexSettingIsIllegal() {
         Template template = new Template(Settings.builder().put(IndexMetadata.SETTING_INDEX_HIDDEN, true).build(), null, null);
-        ComposableIndexTemplate globalTemplate = new ComposableIndexTemplate.Builder().indexPatterns(List.of("*"))
-            .template(template)
-            .build();
+        ComposableIndexTemplate globalTemplate = ComposableIndexTemplate.builder().indexPatterns(List.of("*")).template(template).build();
 
         PutComposableIndexTemplateAction.Request request = new PutComposableIndexTemplateAction.Request("test");
         request.indexTemplate(globalTemplate);