Fix loading templates in config/ directory

The fixes introduced in elastic#4235 and elastic#4411 do not take into account, that a template JSON in the config/ directory includes a template name, as opposed when calling the Put Template API. This PR allows to put both formats (either specifying a template name or not) into files. However you template name/id may not be one of the template element names like "template", "settings", "order" or "mapping". Closes elastic#4511
mute · Dec 22, 2013 · 54385c1 · 54385c1
1 parent 33103b8
commit 54385c1
Show file tree

Hide file tree

Showing 10 changed files with 103 additions and 30 deletions.
diff --git a/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateMetaData.java b/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateMetaData.java
@@ -20,6 +20,7 @@
 package org.elasticsearch.cluster.metadata;
 
 import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
+import com.google.common.collect.Sets;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.collect.MapBuilder;
 import org.elasticsearch.common.compress.CompressedString;
@@ -35,6 +36,7 @@
 
 import java.io.IOException;
 import java.util.Map;
+import java.util.Set;
 
 /**
  *
@@ -147,6 +149,11 @@ public int hashCode() {
 
     public static class Builder {
 
+        private static final Set<String> VALID_FIELDS = Sets.newHashSet("template", "order", "mappings", "settings");
+        static {
+            VALID_FIELDS.addAll(IndexMetaData.customFactories.keySet());
+        }
+
         private String name;
 
         private int order;
@@ -296,8 +303,8 @@ public static IndexTemplateMetaData fromXContentStandalone(XContentParser parser
         public static IndexTemplateMetaData fromXContent(XContentParser parser) throws IOException {
             Builder builder = new Builder(parser.currentName());
 
-            String currentFieldName = null;
-            XContentParser.Token token = parser.nextToken();
+            String currentFieldName = skipTemplateName(parser);
+            XContentParser.Token token;
             while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
                 if (token == XContentParser.Token.FIELD_NAME) {
                     currentFieldName = parser.currentName();
@@ -359,6 +366,24 @@ public static IndexTemplateMetaData fromXContent(XContentParser parser) throws I
             return builder.build();
         }
 
+        private static String skipTemplateName(XContentParser parser) throws IOException {
+            XContentParser.Token token = parser.nextToken();
+            if (token != null && token == XContentParser.Token.START_OBJECT) {
+                token = parser.nextToken();
+                if (token == XContentParser.Token.FIELD_NAME) {
+                    String currentFieldName = parser.currentName();
+                    if (VALID_FIELDS.contains(currentFieldName)) {
+                        return currentFieldName;
+                    } else {
+                        // we just hit the template name, which should be ignored and we move on
+                        parser.nextToken();
+                    }
+                }
+            }
+
+            return null;
+        }
+
         public static IndexTemplateMetaData readFrom(StreamInput in) throws IOException {
             Builder builder = new Builder(in.readString());
             builder.order(in.readInt());

diff --git a/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java
@@ -484,7 +484,7 @@ private List<IndexTemplateMetaData> findTemplates(Request request, ClusterState
                     try {
                         byte[] templatesData = Streams.copyToByteArray(templatesFile);
                         parser = XContentHelper.createParser(templatesData, 0, templatesData.length);
-                        IndexTemplateMetaData template = IndexTemplateMetaData.Builder.fromXContentStandalone(parser);
+                        IndexTemplateMetaData template = IndexTemplateMetaData.Builder.fromXContent(parser);
                         if (Regex.simpleMatch(template.template(), request.index)) {
                             templates.add(template);
                         }

diff --git a/src/test/java/org/elasticsearch/cluster/metadata/ToAndFromJsonMetaDataTests.java b/src/test/java/org/elasticsearch/cluster/metadata/ToAndFromJsonMetaDataTests.java
@@ -29,6 +29,7 @@
 import static org.elasticsearch.cluster.metadata.AliasMetaData.newAliasMetaDataBuilder;
 import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
 
 /**
@@ -89,6 +90,11 @@ public void testSimpleJsonFromAndTo() throws IOException {
                         .putAlias(newAliasMetaDataBuilder("alias1").filter(ALIAS_FILTER1))
                         .putAlias(newAliasMetaDataBuilder("alias2"))
                         .putAlias(newAliasMetaDataBuilder("alias4").filter(ALIAS_FILTER2)))
+                        .put(IndexTemplateMetaData.builder("foo")
+                                .template("bar")
+                                .order(1).settings(settingsBuilder()
+                                        .put("setting1", "value1")
+                                        .put("setting2", "value2")))
                 .build();
 
         String metaDataSource = MetaData.Builder.toXContent(metaData);
@@ -172,6 +178,12 @@ public void testSimpleJsonFromAndTo() throws IOException {
         assertThat(indexMetaData.aliases().get("alias3").filter(), nullValue());
         assertThat(indexMetaData.aliases().get("alias4").alias(), equalTo("alias4"));
         assertThat(indexMetaData.aliases().get("alias4").filter().string(), equalTo(ALIAS_FILTER2));
+
+        // templates
+        assertThat(parsedMetaData.templates().get("foo").name(), is("foo"));
+        assertThat(parsedMetaData.templates().get("foo").template(), is("bar"));
+        assertThat(parsedMetaData.templates().get("foo").settings().get("index.setting1"), is("value1"));
+        assertThat(parsedMetaData.templates().get("foo").settings().getByPrefix("index.").get("setting2"), is("value2"));
     }
 
     private static final String MAPPING_SOURCE1 = "{\"mapping1\":{\"text1\":{\"type\":\"string\"}}}";

diff --git a/src/test/java/org/elasticsearch/indices/template/IndexTemplateFileLoadingTests.java b/src/test/java/org/elasticsearch/indices/template/IndexTemplateFileLoadingTests.java
@@ -32,13 +32,15 @@
 import org.junit.rules.TemporaryFolder;
 
 import java.io.File;
+import java.util.HashSet;
+import java.util.Set;
 
 import static org.hamcrest.Matchers.is;
 
 /**
  *
  */
-@ClusterScope(scope=Scope.SUITE, numNodes=1)
+@ClusterScope(scope=Scope.TEST, numNodes=1)
 public class IndexTemplateFileLoadingTests extends ElasticsearchIntegrationTest {
 
     @Rule
@@ -57,8 +59,10 @@ protected Settings nodeSettings(int nodeOrdinal) {
             templatesDir.mkdir();
 
             File dst = new File(templatesDir, "template.json");
+            String templatePath = "/org/elasticsearch/indices/template/template" + randomInt(5) + ".json";
+            logger.info("Picking template path [{}]", templatePath);
             // random template, one uses the 'setting.index.number_of_shards', the other 'settings.number_of_shards'
-            String template = Streams.copyToStringFromClasspath("/org/elasticsearch/indices/template/template" + randomInt(2) + ".json");
+            String template = Streams.copyToStringFromClasspath(templatePath);
             Files.write(template, dst, Charsets.UTF_8);
         } catch (Exception e) {
             throw new RuntimeException(e);
@@ -69,11 +73,20 @@ protected Settings nodeSettings(int nodeOrdinal) {
 
     @Test
     public void testThatLoadingTemplateFromFileWorks() throws Exception {
-        createIndex("foobar");
-        ensureYellow(); // ensuring yellow so the test fails faster if the template cannot be loaded
+        final int iters = atLeast(5);
+        Set<String> indices = new HashSet<String>();
+        for (int i = 0; i < iters; i++) {
+            String indexName = "foo" + randomRealisticUnicodeOfLengthBetween(0, 5);
+            if (indices.contains(indexName)) {
+                continue;
+            }
+            indices.add(indexName);
+            createIndex(indexName);
+            ensureYellow(); // ensuring yellow so the test fails faster if the template cannot be loaded
 
-        ClusterStateResponse stateResponse = client().admin().cluster().prepareState().setFilterIndices("foobar").get();
-        assertThat(stateResponse.getState().getMetaData().indices().get("foobar").getNumberOfShards(), is(10));
-        assertThat(stateResponse.getState().getMetaData().indices().get("foobar").getNumberOfReplicas(), is(0));
+            ClusterStateResponse stateResponse = client().admin().cluster().prepareState().setFilterIndices(indexName).get();
+            assertThat(stateResponse.getState().getMetaData().indices().get(indexName).getNumberOfShards(), is(10));
+            assertThat(stateResponse.getState().getMetaData().indices().get(indexName).getNumberOfReplicas(), is(0));
+        }
     }
 }
diff --git a/src/test/java/org/elasticsearch/indices/template/template0.json b/src/test/java/org/elasticsearch/indices/template/template0.json
@@ -1,9 +1,7 @@
 {
-    "template_1" : {
-        "template" : "foo*",
-        "settings" : {
-            "index.number_of_shards": 10,
-            "index.number_of_replicas": 0
-        }
-    }
+    "template" : "foo*",
+    "settings" : {
+        "index.number_of_shards": 10,
+        "index.number_of_replicas": 0
+   }
 }
diff --git a/src/test/java/org/elasticsearch/indices/template/template1.json b/src/test/java/org/elasticsearch/indices/template/template1.json
@@ -1,9 +1,7 @@
 {
-    "template_1" : {
-        "template" : "foo*",
-        "settings" : {
-            "number_of_shards": 10,
-            "number_of_replicas": 0
-        }
+    "template" : "foo*",
+    "settings" : {
+        "number_of_shards": 10,
+        "number_of_replicas": 0
     }
 }
diff --git a/src/test/java/org/elasticsearch/indices/template/template2.json b/src/test/java/org/elasticsearch/indices/template/template2.json
@@ -1,11 +1,9 @@
 {
-    "template_1" : {
-        "template" : "foo*",
-        "settings" : {
-            "index" : {
-                "number_of_shards": 10,
-                "number_of_replicas": 0
-            }
+    "template" : "foo*",
+    "settings" : {
+        "index" : {
+            "number_of_shards": 10,
+            "number_of_replicas": 0
         }
     }
 }
diff --git a/src/test/java/org/elasticsearch/indices/template/template3.json b/src/test/java/org/elasticsearch/indices/template/template3.json
@@ -0,0 +1,9 @@
+{
+    "mytemplate" : {
+        "template" : "foo*",
+        "settings" : {
+            "index.number_of_shards": 10,
+            "index.number_of_replicas": 0
+        }
+    }
+}
diff --git a/src/test/java/org/elasticsearch/indices/template/template4.json b/src/test/java/org/elasticsearch/indices/template/template4.json
@@ -0,0 +1,9 @@
+{
+    "mytemplate" : {
+        "template" : "foo*",
+        "settings" : {
+            "number_of_shards": 10,
+            "number_of_replicas": 0
+        }
+    }
+}
diff --git a/src/test/java/org/elasticsearch/indices/template/template5.json b/src/test/java/org/elasticsearch/indices/template/template5.json
@@ -0,0 +1,11 @@
+{
+    "mytemplate" : {
+        "template" : "foo*",
+        "settings" : {
+            "index" : {
+                "number_of_shards": 10,
+                "number_of_replicas": 0
+            }
+        }
+    }
+}