moving merge methods out of DataStream

Author: masseyke

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

@@ -18,6 +18,7 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.index.mapper.DataStreamTimestampFieldMapper;
 import org.elasticsearch.index.mapper.MapperService;
@@ -309,6 +310,33 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params, @Nulla
         return builder;
     }
 
+    /*
+     * Merges the given settings into the settings in this ComposableIndexTemplate. Any null values in the
+     * given settings are removed from the settings in the returned ComposableIndexTemplate. If this
+     * ComposableIndexTemplate has no settings, the given settings are the only ones in the returned template
+     * (with any null values removed). If this ComposableIndexTemplate has no template, an empty template with
+     * those settings is created. If the given settings are null or empty, this ComposableIndexTemplate is just
+     * returned unchanged. This method never changes this object.
+     */
+    public ComposableIndexTemplate mergeSettings(Settings settings) {
+        if (settings == null || Settings.EMPTY.equals(settings)) {
+            return this;
+        }
+        ComposableIndexTemplate.Builder mergedIndexTemplateBuilder = this.toBuilder();
+        Template.Builder mergedTemplateBuilder;
+        Settings templateSettings;
+        if (this.template() == null) {
+            mergedTemplateBuilder = Template.builder();
+            templateSettings = null;
+        } else {
+            mergedTemplateBuilder = Template.builder(this.template());
+            templateSettings = this.template().settings();
+        }
+        mergedTemplateBuilder.settings(templateSettings == null ? settings : templateSettings.merge(settings));
+        mergedIndexTemplateBuilder.template(mergedTemplateBuilder);
+        return mergedIndexTemplateBuilder.build();
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(

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

@@ -58,7 +58,6 @@
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -377,52 +376,19 @@ public boolean rolloverOnWrite() {
     }
 
     public ComposableIndexTemplate getEffectiveIndexTemplate(ProjectMetadata projectMetadata) {
-        return mergeSettingsIntoTemplate(getMatchingIndexTemplate(projectMetadata), settings);
+        return getMatchingIndexTemplate(projectMetadata).mergeSettings(settings);
     }
 
     public Settings getEffectiveSettings(ProjectMetadata projectMetadata) {
         ComposableIndexTemplate template = getMatchingIndexTemplate(projectMetadata);
-        return mergeSettings(template.template() == null ? Settings.EMPTY : template.template().settings(), settings);
+        Settings templateSettings = template.template() == null ? Settings.EMPTY : template.template().settings();
+        return templateSettings.merge(settings);
     }
 
     private ComposableIndexTemplate getMatchingIndexTemplate(ProjectMetadata projectMetadata) {
         return lookupTemplateForDataStream(name, projectMetadata);
     }
 
-    public static ComposableIndexTemplate mergeSettingsIntoTemplate(ComposableIndexTemplate template, Settings settings) {
-        if (Settings.EMPTY.equals(settings)) {
-            return template;
-        }
-        ComposableIndexTemplate.Builder mergedIndexTemplateBuilder = template.toBuilder();
-        Template.Builder mergedTemplateBuilder;
-        Settings templateSettings;
-        if (template.template() == null) {
-            mergedTemplateBuilder = Template.builder();
-            templateSettings = null;
-        } else {
-            mergedTemplateBuilder = Template.builder(template.template());
-            templateSettings = template.template().settings();
-        }
-        mergedTemplateBuilder.settings(mergeSettings(templateSettings, settings));
-        mergedIndexTemplateBuilder.template(mergedTemplateBuilder);
-        return mergedIndexTemplateBuilder.build();
-    }
-
-    private static Settings mergeSettings(Settings originalSettings, Settings newSettings) {
-        if (newSettings == null || Settings.EMPTY.equals(newSettings)) {
-            return Objects.requireNonNullElse(originalSettings, Settings.EMPTY);
-        } else if (originalSettings == null || Settings.EMPTY.equals(originalSettings)) {
-            return newSettings;
-        }
-        Settings.Builder settingsBuilder = Settings.builder().put(originalSettings).put(newSettings);
-        for (String settingName : new HashSet<>(settingsBuilder.keys())) {
-            if (settingsBuilder.get(settingName) == null) {
-                settingsBuilder.remove(settingName);
-            }
-        }
-        return settingsBuilder.build();
-    }
-
     /**
      * We define that a data stream is considered internal either if it is a system index or if
      * its name starts with a dot.

server/src/main/java/org/elasticsearch/common/settings/Settings.java

@@ -876,6 +876,24 @@ public Set<String> keySet() {
         return newKeySet;
     }
 
+    /*
+     * This method merges the given newSettings into this Settings, returning either a new Settings object or
+     * this if the newSettings are null or empty. If any values are null in newSettings, those keys are removed
+     * from the returned object.
+     */
+    public Settings merge(Settings newSettings) {
+        if (newSettings == null || Settings.EMPTY.equals(newSettings)) {
+            return this;
+        }
+        Settings.Builder settingsBuilder = Settings.builder().put(this).put(newSettings);
+        for (String settingName : new HashSet<>(settingsBuilder.keys())) {
+            if (settingsBuilder.get(settingName) == null) {
+                settingsBuilder.remove(settingName);
+            }
+        }
+        return settingsBuilder.build();
+    }
+
     /**
      * A builder allowing to put different settings and then {@link #build()} an immutable
      * settings implementation. Use {@link Settings#builder()} in order to

server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java

@@ -26,6 +26,7 @@
 import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 
 import static org.elasticsearch.cluster.metadata.DataStream.TIMESTAMP_FIELD_NAME;
@@ -277,4 +278,67 @@ public void testBuilderRoundtrip() {
             assertEquals(template.template(), Template.builder(template.template()).build());
         }
     }
+
+    public void testMergeSettings() {
+        {
+            // We only have settings from the template, so the effective template will just be the original template
+            Settings templateSettings = randomSettings();
+            Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(randomMappings(null));
+            ComposableIndexTemplate indexTemplate = randomInstance();
+            assertThat(indexTemplate.mergeSettings(Settings.EMPTY), equalTo(indexTemplate));
+        }
+        {
+            // We only have settings from the data stream, so we expect to get only those back in the effective template
+            Settings dataStreamSettings = randomSettings();
+            String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
+            Settings templateSettings = Settings.EMPTY;
+            CompressedXContent templateMappings = randomMappings(randomDataStreamTemplate());
+            Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings);
+            ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder()
+                .indexPatterns(List.of(dataStreamName))
+                .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
+                .template(templateBuilder)
+                .build();
+            Template.Builder expectedTemplateBuilder = Template.builder().settings(dataStreamSettings).mappings(templateMappings);
+            ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder()
+                .indexPatterns(List.of(dataStreamName))
+                .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
+                .template(expectedTemplateBuilder)
+                .build();
+            assertThat(indexTemplate.mergeSettings(dataStreamSettings), equalTo(expectedEffectiveTemplate));
+        }
+        {
+            // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence
+            Settings dataStreamSettings = Settings.builder()
+                .put("index.setting1", "dataStreamValue")
+                .put("index.setting2", "dataStreamValue")
+                .put("index.setting3", (String) null) // This one gets removed from the effective settings
+                .build();
+            String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
+            Settings templateSettings = Settings.builder()
+                .put("index.setting1", "templateValue")
+                .put("index.setting3", "templateValue")
+                .put("index.setting4", "templateValue")
+                .build();
+            CompressedXContent templateMappings = randomMappings(randomDataStreamTemplate());
+            Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings);
+            ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder()
+                .indexPatterns(List.of(dataStreamName))
+                .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
+                .template(templateBuilder)
+                .build();
+            Settings mergedSettings = Settings.builder()
+                .put("index.setting1", "dataStreamValue")
+                .put("index.setting2", "dataStreamValue")
+                .put("index.setting4", "templateValue")
+                .build();
+            Template.Builder expectedTemplateBuilder = Template.builder().settings(mergedSettings).mappings(templateMappings);
+            ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder()
+                .indexPatterns(List.of(dataStreamName))
+                .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
+                .template(expectedTemplateBuilder)
+                .build();
+            assertThat(indexTemplate.mergeSettings(dataStreamSettings), equalTo(expectedEffectiveTemplate));
+        }
+    }
 }

server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java

@@ -2778,74 +2778,6 @@ public void testGetEffectiveIndexTemplate() {
         }
     }
 
-    public void testMergeSettingsIntoTemplate() {
-        {
-            // We only have settings from the template, so the effective template will just be the original template
-            DataStream dataStream = createDataStream(Settings.EMPTY);
-            Settings templateSettings = randomSettings();
-            Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(randomMappings());
-            ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder()
-                .indexPatterns(List.of(dataStream.getName()))
-                .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
-                .template(templateBuilder)
-                .build();
-            assertThat(DataStream.mergeSettingsIntoTemplate(indexTemplate, Settings.EMPTY), equalTo(indexTemplate));
-        }
-        {
-            // We only have settings from the data stream, so we expect to get only those back in the effective template
-            Settings dataStreamSettings = randomSettings();
-            String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
-            Settings templateSettings = Settings.EMPTY;
-            CompressedXContent templateMappings = randomMappings();
-            Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings);
-            ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder()
-                .indexPatterns(List.of(dataStreamName))
-                .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
-                .template(templateBuilder)
-                .build();
-            Template.Builder expectedTemplateBuilder = Template.builder().settings(dataStreamSettings).mappings(templateMappings);
-            ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder()
-                .indexPatterns(List.of(dataStreamName))
-                .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
-                .template(expectedTemplateBuilder)
-                .build();
-            assertThat(DataStream.mergeSettingsIntoTemplate(indexTemplate, dataStreamSettings), equalTo(expectedEffectiveTemplate));
-        }
-        {
-            // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence
-            Settings dataStreamSettings = Settings.builder()
-                .put("index.setting1", "dataStreamValue")
-                .put("index.setting2", "dataStreamValue")
-                .put("index.setting3", (String) null) // This one gets removed from the effective settings
-                .build();
-            String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
-            Settings templateSettings = Settings.builder()
-                .put("index.setting1", "templateValue")
-                .put("index.setting3", "templateValue")
-                .put("index.setting4", "templateValue")
-                .build();
-            CompressedXContent templateMappings = randomMappings();
-            Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings);
-            ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder()
-                .indexPatterns(List.of(dataStreamName))
-                .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
-                .template(templateBuilder)
-                .build();
-            Settings mergedSettings = Settings.builder()
-                .put("index.setting1", "dataStreamValue")
-                .put("index.setting2", "dataStreamValue")
-                .put("index.setting4", "templateValue")
-                .build();
-            Template.Builder expectedTemplateBuilder = Template.builder().settings(mergedSettings).mappings(templateMappings);
-            ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder()
-                .indexPatterns(List.of(dataStreamName))
-                .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
-                .template(expectedTemplateBuilder)
-                .build();
-            assertThat(DataStream.mergeSettingsIntoTemplate(indexTemplate, dataStreamSettings), equalTo(expectedEffectiveTemplate));
-        }
-    }
-
     private DataStream createDataStream(Settings settings) {
         DataStream dataStream = createTestInstance();
         return dataStream.copy().setSettings(settings).build();

server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java

@@ -722,4 +722,58 @@ public void testGlobValues() throws IOException {
         assertThat(values, contains("1"));
     }
 
+    public void testMerge() {
+        {
+            assertThat(Settings.EMPTY.merge(null), equalTo(Settings.EMPTY));
+            assertThat(Settings.EMPTY.merge(Settings.EMPTY), equalTo(Settings.EMPTY));
+        }
+        {
+            Settings.Builder builder = Settings.builder();
+            for (int i = 1; i < randomInt(100); i++) {
+                builder.put(randomAlphanumericOfLength(20), randomAlphanumericOfLength(50));
+            }
+            Settings settings = builder.build();
+            assertThat(settings.merge(null), equalTo(settings));
+            assertThat(settings.merge(Settings.EMPTY), equalTo(settings));
+        }
+        {
+            Settings.Builder builder = Settings.builder();
+            for (int i = 1; i < randomInt(100); i++) {
+                builder.put(randomAlphanumericOfLength(20), randomAlphanumericOfLength(50));
+            }
+            Settings newSettings = builder.build();
+            assertThat(Settings.EMPTY.merge(newSettings), equalTo(newSettings));
+        }
+        {
+            Settings settings = Settings.builder()
+                .put("index.setting1", "templateValue")
+                .put("index.setting3", "templateValue")
+                .put("index.setting4", "templateValue")
+                .build();
+            Settings newSettings = Settings.builder()
+                .put("index.setting1", "dataStreamValue")
+                .put("index.setting2", "dataStreamValue")
+                .put("index.setting3", (String) null) // This one gets removed from the effective settings
+                .build();
+            Settings mergedSettings = Settings.builder()
+                .put("index.setting1", "dataStreamValue")
+                .put("index.setting2", "dataStreamValue")
+                .put("index.setting4", "templateValue")
+                .build();
+            assertThat(settings.merge(newSettings), equalTo(mergedSettings));
+        }
+        {
+            Settings newSettings = Settings.builder()
+                .put("index.setting1", "dataStreamValue")
+                .put("index.setting2", "dataStreamValue")
+                .put("index.setting3", (String) null) // This one gets removed from the effective settings
+                .build();
+            Settings mergedSettings = Settings.builder()
+                .put("index.setting1", "dataStreamValue")
+                .put("index.setting2", "dataStreamValue")
+                .build();
+            assertThat(Settings.EMPTY.merge(newSettings), equalTo(mergedSettings));
+        }
+    }
+
 }