Default to not accepting type wrapper in indexing requests

Currently it is possible to index a document as:

```
POST /myindex/mytype/1
{ "foo"...}
```

or as:

```
POST /myindex/mytype/1
{
    "mytype": {
        "foo"...
    }
}
```

This makes indexing non-deterministic and fields can be misinterpreted
as type names.

This changes makes Elasticsearch accept only the first form by default,
ie without the type wrapper. This can be changed by setting
`index.mapping.allow_type_wrapper` to `true`` when creating the index.

Closes elastic#4484

docs/reference/docs/index_.asciidoc

@@ -33,7 +33,7 @@ The result of the above index operation is:
 
 The index operation automatically creates an index if it has not been
 created before (check out the
-<<indices-create-index,create index API>> for manually 
+<<indices-create-index,create index API>> for manually
 creating an index), and also automatically creates a
 dynamic type mapping for the specific type if one has not yet been
 created (check out the <<indices-put-mapping,put mapping>>
@@ -44,12 +44,21 @@ objects will automatically be added to the mapping definition of the
 type specified. Check out the <<mapping,mapping>>
 section for more information on mapping definitions.
 
-Though explained on the <<mapping,mapping>> section,
-it's important to note that the format of the JSON document can also
-include the type (very handy when using JSON mappers), for example:
+Note that the format of the JSON document can also include the type (very handy
+when using JSON mappers) if the `index.mapping.allow_type_wrapper` setting is
+set to true, for example:
 
 [source,js]
 --------------------------------------------------
+$ curl -XPOST 'http://localhost:9200/twitter' -d '{
+  "settings": {
+    "index": {
+      "mapping.allow_type_wrapper": true
+    }
+  }
+}'
+{"acknowledged":true}
+
 $ curl -XPUT 'http://localhost:9200/twitter/tweet/1' -d '{
     "tweet" : {
         "user" : "kimchy",

src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java

@@ -239,6 +239,8 @@ protected ParseContext initialValue() {
         }
     };
 
+    public static final String ALLOW_TYPE_WRAPPER = "index.mapping.allow_type_wrapper";
+
     private final String index;
 
     private final Settings indexSettings;
@@ -494,18 +496,15 @@ public ParsedDocument parse(SourceToParse source, @Nullable ParseListener listen
             } else if (token != XContentParser.Token.FIELD_NAME) {
                 throw new MapperParsingException("Malformed content, after first object, either the type field or the actual properties should exist");
             }
-            if (type.equals(parser.currentName())) {
-                // first field is the same as the type, this might be because the type is provided, and the object exists within it
-                // or because there is a valid field that by chance is named as the type
-
-                // Note, in this case, we only handle plain value types, an object type will be analyzed as if it was the type itself
-                // and other same level fields will be ignored
-                token = parser.nextToken();
+            // first field is the same as the type, this might be because the
+            // type is provided, and the object exists within it or because
+            // there is a valid field that by chance is named as the type.
+            // Because of this, by default wrapping a document in a type is
+            // disabled, but can be enabled by setting
+            // index.mapping.allow_type_wrapper to true
+            if (type.equals(parser.currentName()) && indexSettings.getAsBoolean(ALLOW_TYPE_WRAPPER, false)) {
+                parser.nextToken();
                 countDownTokens++;
-                // commented out, allow for same type with START_OBJECT, we do our best to handle it except for the above corner case
-//                if (token != XContentParser.Token.START_OBJECT) {
-//                    throw new MapperException("Malformed content, a field with the same name as the type must be an object with the properties/fields within it");
-//                }
             }
 
             for (RootMapper rootMapper : rootMappersOrdered) {

src/test/java/org/elasticsearch/aliases/IndexAliasesTests.java

@@ -853,6 +853,6 @@ private void assertHits(SearchHits hits, String... ids) {
     }
 
     private String source(String id, String nameValue) {
-        return "{ type1 : { \"id\" : \"" + id + "\", \"name\" : \"" + nameValue + "\" } }";
+        return "{ \"id\" : \"" + id + "\", \"name\" : \"" + nameValue + "\" }";
     }
 }

src/test/java/org/elasticsearch/index/mapper/all/SimpleAllMapperTests.java

@@ -231,11 +231,11 @@ public void testRandom() throws Exception {
         // reparse it
         DocumentMapper builtDocMapper = MapperTestUtils.newParser().parse(builtMapping);
 
-        byte[] json = jsonBuilder().startObject().startObject("test")
+        byte[] json = jsonBuilder().startObject()
                 .field("foo", "bar")
                 .field("_id", 1)
                 .field("foobar", "foobar")
-                .endObject().endObject().bytes().array();
+                .endObject().bytes().array();
         Document doc = builtDocMapper.parse(new BytesArray(json)).rootDoc();
         AllField field = (AllField) doc.getField("_all");
         if (enabled) {

src/test/java/org/elasticsearch/index/mapper/all/test1.json

@@ -1,20 +1,18 @@
 {
-    "person":{
-        "_boost":3.7,
-        "_id":"1",
-        "name":{
-            "first":"shay",
-            "last":"banon"
+    "_boost":3.7,
+    "_id":"1",
+    "name":{
+        "first":"shay",
+        "last":"banon"
+    },
+    "address":{
+        "first":{
+            "location":"first location"
         },
-        "address":{
-            "first":{
-                "location":"first location"
-            },
-            "last":{
-                "location":"last location"
-            }
-        },
-        "simple1":1,
-        "simple2":2
-    }
-}
+        "last":{
+            "location":"last location"
+        }
+    },
+    "simple1":1,
+    "simple2":2
+}

src/test/java/org/elasticsearch/index/mapper/simple/SimpleMapperTests.java

@@ -22,6 +22,8 @@
 import com.google.common.base.Charsets;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.settings.ImmutableSettings;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.mapper.*;
 import org.elasticsearch.index.mapper.ParseContext.Document;
 import org.elasticsearch.test.ElasticsearchTestCase;
@@ -128,4 +130,18 @@ public void testNoDocumentSent() throws Exception {
             assertThat(e.getMessage(), equalTo("failed to parse, document is empty"));
         }
     }
+
+    @Test
+    public void testTypeWrapperWithSetting() throws Exception {
+        String mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/simple/test-mapping.json");
+        Settings settings = ImmutableSettings.settingsBuilder().put("index.mapping.allow_type_wrapper", true).build();
+        DocumentMapper docMapper = MapperTestUtils.newParser(settings).parse(mapping);
+
+        assertThat((String) docMapper.meta().get("param1"), equalTo("value1"));
+
+        BytesReference json = new BytesArray(copyToBytesFromClasspath("/org/elasticsearch/index/mapper/simple/test1-withtype.json"));
+        Document doc = docMapper.parse(json).rootDoc();
+        assertThat(doc.get(docMapper.uidMapper().names().indexName()), equalTo(Uid.createUid("person", "1")));
+        assertThat(doc.get(docMapper.mappers().name("first").mapper().names().indexName()), equalTo("shay"));
+    }
 }

src/test/java/org/elasticsearch/index/mapper/simple/test1-withtype.json

@@ -0,0 +1,43 @@
+{
+    person:{
+        _boost:3.7,
+        _id:"1",
+        name:{
+            first:"shay",
+            last:"banon"
+        },
+        address:{
+            first:{
+                location:"first location"
+            },
+            last:{
+                location:"last location"
+            }
+        },
+        age:32,
+        birthDate:"1977-11-15",
+        nerd:true,
+        dogs:["buck", "mia"],
+        complex:[
+            {
+                value1:"value1"
+            },
+            {
+                value2:"value2"
+            }
+        ],
+        complex2:[
+            [
+                {
+                    value1:"value1"
+                }
+            ],
+            [
+                {
+                    value2:"value2"
+                }
+            ]
+        ],
+        nullValue:null
+    }
+}

src/test/java/org/elasticsearch/index/mapper/simple/test1.json

@@ -1,43 +1,41 @@
 {
-    person:{
-        _boost:3.7,
-        _id:"1",
-        name:{
-            first:"shay",
-            last:"banon"
+    _boost:3.7,
+    _id:"1",
+    name:{
+        first:"shay",
+        last:"banon"
+    },
+    address:{
+        first:{
+            location:"first location"
         },
-        address:{
-            first:{
-                location:"first location"
-            },
-            last:{
-                location:"last location"
-            }
+        last:{
+            location:"last location"
+        }
+    },
+    age:32,
+    birthDate:"1977-11-15",
+    nerd:true,
+    dogs:["buck", "mia"],
+    complex:[
+        {
+            value1:"value1"
         },
-        age:32,
-        birthDate:"1977-11-15",
-        nerd:true,
-        dogs:["buck", "mia"],
-        complex:[
+        {
+            value2:"value2"
+        }
+    ],
+    complex2:[
+        [
             {
                 value1:"value1"
-            },
+            }
+        ],
+        [
             {
                 value2:"value2"
             }
-        ],
-        complex2:[
-            [
-                {
-                    value1:"value1"
-                }
-            ],
-            [
-                {
-                    value2:"value2"
-                }
-            ]
-        ],
-        nullValue:null
-    }
-}
+        ]
+    ],
+    nullValue:null
+}

src/test/java/org/elasticsearch/index/mapper/typelevels/ParseDocumentTypeLevelsTests.java

@@ -26,9 +26,7 @@
 import org.elasticsearch.test.ElasticsearchTestCase;
 import org.junit.Test;
 
-import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.nullValue;
 
 /**
  *
@@ -68,9 +66,9 @@ public void testTypeLevel() throws Exception {
                 .endObject().endObject()
                 .bytes());
 
-        assertThat(doc.rootDoc().get("test1"), equalTo("value1"));
-        assertThat(doc.rootDoc().get("test2"), equalTo("value2"));
-        assertThat(doc.rootDoc().get("inner.inner_field"), equalTo("inner_value"));
+        assertThat(doc.rootDoc().get("type.test1"), equalTo("value1"));
+        assertThat(doc.rootDoc().get("type.test2"), equalTo("value2"));
+        assertThat(doc.rootDoc().get("type.inner.inner_field"), equalTo("inner_value"));
     }
 
     @Test
@@ -109,10 +107,10 @@ public void testTypeLevelWithFieldTypeAsValue() throws Exception {
                 .endObject().endObject()
                 .bytes());
 
-        assertThat(doc.rootDoc().get("type"), equalTo("value_type"));
-        assertThat(doc.rootDoc().get("test1"), equalTo("value1"));
-        assertThat(doc.rootDoc().get("test2"), equalTo("value2"));
-        assertThat(doc.rootDoc().get("inner.inner_field"), equalTo("inner_value"));
+        assertThat(doc.rootDoc().get("type.type"), equalTo("value_type"));
+        assertThat(doc.rootDoc().get("type.test1"), equalTo("value1"));
+        assertThat(doc.rootDoc().get("type.test2"), equalTo("value2"));
+        assertThat(doc.rootDoc().get("type.inner.inner_field"), equalTo("inner_value"));
     }
 
     @Test
@@ -131,9 +129,9 @@ public void testNoLevelWithFieldTypeAsObject() throws Exception {
                 .bytes());
 
         // in this case, we analyze the type object as the actual document, and ignore the other same level fields
-        assertThat(doc.rootDoc().get("type_field"), equalTo("type_value"));
-        assertThat(doc.rootDoc().get("test1"), nullValue());
-        assertThat(doc.rootDoc().get("test2"), nullValue());
+        assertThat(doc.rootDoc().get("type.type_field"), equalTo("type_value"));
+        assertThat(doc.rootDoc().get("test1"), equalTo("value1"));
+        assertThat(doc.rootDoc().get("test2"), equalTo("value2"));
     }
 
     @Test
@@ -151,10 +149,10 @@ public void testTypeLevelWithFieldTypeAsObject() throws Exception {
                 .endObject().endObject()
                 .bytes());
 
-        assertThat(doc.rootDoc().get("type.type_field"), equalTo("type_value"));
-        assertThat(doc.rootDoc().get("test1"), equalTo("value1"));
-        assertThat(doc.rootDoc().get("test2"), equalTo("value2"));
-        assertThat(doc.rootDoc().get("inner.inner_field"), equalTo("inner_value"));
+        assertThat(doc.rootDoc().get("type.type.type_field"), equalTo("type_value"));
+        assertThat(doc.rootDoc().get("type.test1"), equalTo("value1"));
+        assertThat(doc.rootDoc().get("type.test2"), equalTo("value2"));
+        assertThat(doc.rootDoc().get("type.inner.inner_field"), equalTo("inner_value"));
     }
 
     @Test
@@ -172,10 +170,10 @@ public void testNoLevelWithFieldTypeAsValueNotFirst() throws Exception {
                 .endObject().endObject()
                 .bytes());
 
-        assertThat(doc.rootDoc().get("type"), equalTo("value_type"));
-        assertThat(doc.rootDoc().get("test1"), equalTo("value1"));
-        assertThat(doc.rootDoc().get("test2"), equalTo("value2"));
-        assertThat(doc.rootDoc().get("inner.inner_field"), equalTo("inner_value"));
+        assertThat(doc.rootDoc().get("type.type"), equalTo("value_type"));
+        assertThat(doc.rootDoc().get("type.test1"), equalTo("value1"));
+        assertThat(doc.rootDoc().get("type.test2"), equalTo("value2"));
+        assertThat(doc.rootDoc().get("type.inner.inner_field"), equalTo("inner_value"));
     }
 
     @Test
@@ -193,10 +191,10 @@ public void testTypeLevelWithFieldTypeAsValueNotFirst() throws Exception {
                 .endObject().endObject()
                 .bytes());
 
-        assertThat(doc.rootDoc().get("type"), equalTo("value_type"));
-        assertThat(doc.rootDoc().get("test1"), equalTo("value1"));
-        assertThat(doc.rootDoc().get("test2"), equalTo("value2"));
-        assertThat(doc.rootDoc().get("inner.inner_field"), equalTo("inner_value"));
+        assertThat(doc.rootDoc().get("type.type"), equalTo("value_type"));
+        assertThat(doc.rootDoc().get("type.test1"), equalTo("value1"));
+        assertThat(doc.rootDoc().get("type.test2"), equalTo("value2"));
+        assertThat(doc.rootDoc().get("type.inner.inner_field"), equalTo("inner_value"));
     }
 
     @Test
@@ -236,9 +234,9 @@ public void testTypeLevelWithFieldTypeAsObjectNotFirst() throws Exception {
                 .endObject().endObject()
                 .bytes());
 
-        assertThat(doc.rootDoc().get("type.type_field"), equalTo("type_value"));
-        assertThat(doc.rootDoc().get("test1"), equalTo("value1"));
-        assertThat(doc.rootDoc().get("test2"), equalTo("value2"));
-        assertThat(doc.rootDoc().get("inner.inner_field"), equalTo("inner_value"));
+        assertThat(doc.rootDoc().get("type.type.type_field"), equalTo("type_value"));
+        assertThat(doc.rootDoc().get("type.test1"), equalTo("value1"));
+        assertThat(doc.rootDoc().get("type.test2"), equalTo("value2"));
+        assertThat(doc.rootDoc().get("type.inner.inner_field"), equalTo("inner_value"));
     }
 }