Stop automatically nesting mappings in index creation requests. (#36924)

jtibshirani · web-flow · commit ac1c6940d20b · 2019-01-03T17:41:28.000-08:00
Now that we unwrap mappings in DocumentMapperParser#extractMappings, it is not necessary for the mapping definition to always be nested under the type. This leniency around the mapping format was added in 2341825.
diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java
@@ -33,7 +33,6 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
-import org.elasticsearch.common.collect.MapBuilder;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.settings.Settings;
@@ -256,10 +255,6 @@ public CreateIndexRequest mapping(String type, Map<String, ?> source) {
         if (mappings.containsKey(type)) {
             throw new IllegalStateException("mappings for type \"" + type + "\" were already defined");
         }
-        // wrap it in a type map if its not
-        if (source.size() != 1 || !source.containsKey(type)) {
-            source = MapBuilder.<String, Object>newMapBuilder().put(type, source).map();
-        }
         try {
             XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON);
             builder.map(source);
@@ -274,7 +269,7 @@ public CreateIndexRequest mapping(String type, Map<String, ?> source) {
      * ("field1", "type=string,store=true").
      */
     public CreateIndexRequest mapping(String type, Object... source) {
-        mapping(type, PutMappingRequest.buildFromSimplifiedDef(type, source));
+        mapping(type, PutMappingRequest.buildFromSimplifiedDef(source));
         return this;
     }
 
diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java
@@ -184,6 +184,18 @@ public PutMappingRequest source(Object... source) {
         return source(buildFromSimplifiedDef(type, source));
     }
 
+    /**
+     * @param source
+     *            consisting of field/properties pairs (e.g. "field1",
+     *            "type=string,store=true")
+     * @throws IllegalArgumentException
+     *             if the number of the source arguments is not divisible by two
+     * @return the mappings definition
+     */
+    public static XContentBuilder buildFromSimplifiedDef(Object... source) {
+        return buildFromSimplifiedDef(null, source);
+    }
+
     /**
      * @param type
      *            the mapping type
diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java
@@ -143,7 +143,7 @@ public void testNonNestedMappings() throws Exception {
         assertFalse(metadata.sourceAsMap().isEmpty());
     }
 
-    public void testEmptyNestedMappings() throws Exception {
+    public void testNonNestedEmptyMappings() throws Exception {
         assertAcked(prepareCreate("test")
             .addMapping("_doc", XContentFactory.jsonBuilder().startObject().endObject()));
 
@@ -173,6 +173,20 @@ public void testEmptyMappings() throws Exception {
         assertTrue(metadata.sourceAsMap().isEmpty());
     }
 
+    public void testFlatMappingFormat() throws Exception {
+        assertAcked(prepareCreate("test")
+            .addMapping("_doc", "field", "type=keyword"));
+
+        GetMappingsResponse response = client().admin().indices().prepareGetMappings("test").get();
+
+        ImmutableOpenMap<String, MappingMetaData> mappings = response.mappings().get("test");
+        assertNotNull(mappings);
+
+        MappingMetaData metadata = mappings.get("_doc");
+        assertNotNull(metadata);
+        assertFalse(metadata.sourceAsMap().isEmpty());
+    }
+
     public void testInvalidShardCountSettings() throws Exception {
         int value = randomIntBetween(-10, 0);
         try {
diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestTests.java
@@ -22,31 +22,86 @@
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.action.admin.indices.alias.Alias;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.index.RandomCreateIndexGenerator;
-import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.test.hamcrest.ElasticsearchAssertions;
+import org.elasticsearch.test.AbstractXContentTestCase;
 
 import java.io.IOException;
 import java.util.Map;
 import java.util.Set;
 
-import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
-import static org.elasticsearch.common.xcontent.ToXContent.EMPTY_PARAMS;
+public class CreateIndexRequestTests extends AbstractXContentTestCase<CreateIndexRequest> {
 
-public class CreateIndexRequestTests extends ESTestCase {
+    @Override
+    protected CreateIndexRequest createTestInstance() {
+        try {
+            return RandomCreateIndexGenerator.randomCreateIndexRequest();
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    @Override
+    protected CreateIndexRequest doParseInstance(XContentParser parser) throws IOException {
+        CreateIndexRequest request = new CreateIndexRequest();
+        request.source(parser.map(), LoggingDeprecationHandler.INSTANCE);
+        return request;
+    }
+
+    @Override
+    protected void assertEqualInstances(CreateIndexRequest expectedInstance, CreateIndexRequest newInstance) {
+        assertEquals(expectedInstance.settings(), newInstance.settings());
+        assertAliasesEqual(expectedInstance.aliases(), newInstance.aliases());
+        assertMappingsEqual(expectedInstance.mappings(), newInstance.mappings());
+    }
+
+    @Override
+    protected boolean supportsUnknownFields() {
+        return false;
+    }
+
+    public static void assertMappingsEqual(Map<String, String> expected, Map<String, String> actual) {
+        assertEquals(expected.keySet(), actual.keySet());
+
+        for (Map.Entry<String, String> expectedEntry : expected.entrySet()) {
+            String expectedValue = expectedEntry.getValue();
+            String actualValue = actual.get(expectedEntry.getKey());
+            try (XContentParser expectedJson = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
+                LoggingDeprecationHandler.INSTANCE, expectedValue);
+                 XContentParser actualJson = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
+                     LoggingDeprecationHandler.INSTANCE, actualValue)) {
+                assertEquals(expectedJson.map(), actualJson.map());
+            } catch (IOException e) {
+                throw new RuntimeException(e);
+            }
+        }
+    }
+
+    public static void assertAliasesEqual(Set<Alias> expected, Set<Alias> actual) {
+        assertEquals(expected, actual);
+
+        for (Alias expectedAlias : expected) {
+            for (Alias actualAlias : actual) {
+                if (expectedAlias.equals(actualAlias)) {
+                    // As Alias#equals only looks at name, we check the equality of the other Alias parameters here.
+                    assertEquals(expectedAlias.filter(), actualAlias.filter());
+                    assertEquals(expectedAlias.indexRouting(), actualAlias.indexRouting());
+                    assertEquals(expectedAlias.searchRouting(), actualAlias.searchRouting());
+                }
+            }
+        }
+    }
 
     public void testSerialization() throws IOException {
         CreateIndexRequest request = new CreateIndexRequest("foo");
-        String mapping = Strings.toString(JsonXContent.contentBuilder().startObject().startObject("type").endObject().endObject());
+        String mapping = Strings.toString(JsonXContent.contentBuilder().startObject()
+            .startObject("type").endObject().endObject());
         request.mapping("my_type", mapping, XContentType.JSON);
 
         try (BytesStreamOutput output = new BytesStreamOutput()) {
@@ -63,7 +118,7 @@ public void testSerialization() throws IOException {
 
     public void testTopLevelKeys() {
         String createIndex =
-                "{\n"
+            "{\n"
                 + "  \"FOO_SHOULD_BE_ILLEGAL_HERE\": {\n"
                 + "    \"BAR_IS_THE_SAME\": 42\n"
                 + "  },\n"
@@ -80,81 +135,7 @@ public void testTopLevelKeys() {
 
         CreateIndexRequest request = new CreateIndexRequest();
         ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class,
-                () -> {request.source(createIndex, XContentType.JSON);});
+            () -> {request.source(createIndex, XContentType.JSON);});
         assertEquals("unknown key [FOO_SHOULD_BE_ILLEGAL_HERE] for create index", e.getMessage());
     }
-
-    public void testToXContent() throws IOException {
-        CreateIndexRequest request = new CreateIndexRequest("foo");
-
-        String mapping = Strings.toString(JsonXContent.contentBuilder().startObject().startObject("type").endObject().endObject());
-        request.mapping("my_type", mapping, XContentType.JSON);
-
-        Alias alias = new Alias("test_alias");
-        alias.routing("1");
-        alias.filter("{\"term\":{\"year\":2016}}");
-        alias.writeIndex(true);
-        request.alias(alias);
-
-        Settings.Builder settings = Settings.builder();
-        settings.put(SETTING_NUMBER_OF_SHARDS, 10);
-        request.settings(settings);
-
-        String actualRequestBody = Strings.toString(request);
-
-        String expectedRequestBody = "{\"settings\":{\"index\":{\"number_of_shards\":\"10\"}}," +
-            "\"mappings\":{\"my_type\":{\"type\":{}}}," +
-            "\"aliases\":{\"test_alias\":{\"filter\":{\"term\":{\"year\":2016}},\"routing\":\"1\",\"is_write_index\":true}}}";
-
-        assertEquals(expectedRequestBody, actualRequestBody);
-    }
-
-    public void testToAndFromXContent() throws IOException {
-
-        final CreateIndexRequest createIndexRequest = RandomCreateIndexGenerator.randomCreateIndexRequest();
-
-        boolean humanReadable = randomBoolean();
-        final XContentType xContentType = randomFrom(XContentType.values());
-        BytesReference originalBytes = toShuffledXContent(createIndexRequest, xContentType, EMPTY_PARAMS, humanReadable);
-
-        CreateIndexRequest parsedCreateIndexRequest = new CreateIndexRequest();
-        parsedCreateIndexRequest.source(originalBytes, xContentType);
-
-        assertMappingsEqual(createIndexRequest.mappings(), parsedCreateIndexRequest.mappings());
-        assertAliasesEqual(createIndexRequest.aliases(), parsedCreateIndexRequest.aliases());
-        assertEquals(createIndexRequest.settings(), parsedCreateIndexRequest.settings());
-
-        BytesReference finalBytes = toShuffledXContent(parsedCreateIndexRequest, xContentType, EMPTY_PARAMS, humanReadable);
-        ElasticsearchAssertions.assertToXContentEquivalent(originalBytes, finalBytes, xContentType);
-    }
-
-    public static void assertMappingsEqual(Map<String, String> expected, Map<String, String> actual) throws IOException {
-        assertEquals(expected.keySet(), actual.keySet());
-
-        for (Map.Entry<String, String> expectedEntry : expected.entrySet()) {
-            String expectedValue = expectedEntry.getValue();
-            String actualValue = actual.get(expectedEntry.getKey());
-            try (XContentParser expectedJson = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
-                    LoggingDeprecationHandler.INSTANCE, expectedValue);
-                 XContentParser actualJson = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
-                    LoggingDeprecationHandler.INSTANCE, actualValue)){
-                assertEquals(expectedJson.map(), actualJson.map());
-            }
-        }
-    }
-
-    public static void assertAliasesEqual(Set<Alias> expected, Set<Alias> actual) throws IOException {
-        assertEquals(expected, actual);
-
-        for (Alias expectedAlias : expected) {
-            for (Alias actualAlias : actual) {
-                if (expectedAlias.equals(actualAlias)) {
-                    // As Alias#equals only looks at name, we check the equality of the other Alias parameters here.
-                    assertEquals(expectedAlias.filter(), actualAlias.filter());
-                    assertEquals(expectedAlias.indexRouting(), actualAlias.indexRouting());
-                    assertEquals(expectedAlias.searchRouting(), actualAlias.searchRouting());
-                }
-            }
-        }
-    }
 }
diff --git a/test/framework/src/main/java/org/elasticsearch/index/RandomCreateIndexGenerator.java b/test/framework/src/main/java/org/elasticsearch/index/RandomCreateIndexGenerator.java
@@ -29,6 +29,7 @@
 
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS;
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
+import static org.elasticsearch.test.ESTestCase.frequently;
 import static org.elasticsearch.test.ESTestCase.randomAlphaOfLength;
 import static org.elasticsearch.test.ESTestCase.randomBoolean;
 import static org.elasticsearch.test.ESTestCase.randomIntBetween;
@@ -45,9 +46,14 @@ public static CreateIndexRequest randomCreateIndexRequest() throws IOException {
         String index = randomAlphaOfLength(5);
         CreateIndexRequest request = new CreateIndexRequest(index);
         randomAliases(request);
-        if (randomBoolean()) {
+        if (frequently()) {
             String type = randomAlphaOfLength(5);
-            request.mapping(type, randomMapping(type));
+            if (randomBoolean()) {
+                request.mapping(type, randomMapping());
+            } else {
+                request.mapping(type, randomMapping(type));
+
+            }
         }
         if (randomBoolean()) {
             request.settings(randomIndexSettings());
@@ -76,6 +82,16 @@ public static Settings randomIndexSettings() {
         return builder.build();
     }
 
+    public static XContentBuilder randomMapping() throws IOException {
+        XContentBuilder builder = XContentFactory.jsonBuilder();
+        builder.startObject();
+
+        randomMappingFields(builder, true);
+
+        builder.endObject();
+        return builder;
+    }
+
     public static XContentBuilder randomMapping(String type) throws IOException {
         XContentBuilder builder = XContentFactory.jsonBuilder();
         builder.startObject().startObject(type);
