From 329f27c032ee4a17906cf1180e155dea9b6fa023 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 26 Dec 2013 11:25:25 -0700 Subject: [PATCH] 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 #4484 --- docs/reference/docs/index_.asciidoc | 17 +++-- .../index/mapper/DocumentMapper.java | 21 +++--- .../aliases/IndexAliasesTests.java | 2 +- .../mapper/all/SimpleAllMapperTests.java | 4 +- .../elasticsearch/index/mapper/all/test1.json | 34 +++++---- .../mapper/simple/SimpleMapperTests.java | 16 +++++ .../index/mapper/simple/test1-withtype.json | 43 +++++++++++ .../index/mapper/simple/test1.json | 68 +++++++++--------- .../ParseDocumentTypeLevelsTests.java | 54 +++++++------- .../org/elasticsearch/index/query/data.json | 72 +++++++++---------- .../percolator/PercolatorTests.java | 22 ++---- 11 files changed, 200 insertions(+), 153 deletions(-) create mode 100644 src/test/java/org/elasticsearch/index/mapper/simple/test1-withtype.json diff --git a/docs/reference/docs/index_.asciidoc b/docs/reference/docs/index_.asciidoc index c193cbdeaaf9a..de062c80ff37c 100644 --- a/docs/reference/docs/index_.asciidoc +++ b/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 -<> for manually +<> 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 <> @@ -44,12 +44,21 @@ objects will automatically be added to the mapping definition of the type specified. Check out the <> section for more information on mapping definitions. -Though explained on the <> 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", diff --git a/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index d2745d5136374..98157f288704c 100644 --- a/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/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) { diff --git a/src/test/java/org/elasticsearch/aliases/IndexAliasesTests.java b/src/test/java/org/elasticsearch/aliases/IndexAliasesTests.java index 34653c9c35ba4..5599857fd1849 100644 --- a/src/test/java/org/elasticsearch/aliases/IndexAliasesTests.java +++ b/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 + "\" }"; } } diff --git a/src/test/java/org/elasticsearch/index/mapper/all/SimpleAllMapperTests.java b/src/test/java/org/elasticsearch/index/mapper/all/SimpleAllMapperTests.java index 0d8d8bebd7634..1f15caf861892 100644 --- a/src/test/java/org/elasticsearch/index/mapper/all/SimpleAllMapperTests.java +++ b/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) { diff --git a/src/test/java/org/elasticsearch/index/mapper/all/test1.json b/src/test/java/org/elasticsearch/index/mapper/all/test1.json index c3e72dfa9a8d4..834400a48cbe8 100644 --- a/src/test/java/org/elasticsearch/index/mapper/all/test1.json +++ b/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 - } -} \ No newline at end of file + "last":{ + "location":"last location" + } + }, + "simple1":1, + "simple2":2 +} diff --git a/src/test/java/org/elasticsearch/index/mapper/simple/SimpleMapperTests.java b/src/test/java/org/elasticsearch/index/mapper/simple/SimpleMapperTests.java index dd7291b22bb43..79951e46a9630 100644 --- a/src/test/java/org/elasticsearch/index/mapper/simple/SimpleMapperTests.java +++ b/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")); + } } diff --git a/src/test/java/org/elasticsearch/index/mapper/simple/test1-withtype.json b/src/test/java/org/elasticsearch/index/mapper/simple/test1-withtype.json new file mode 100644 index 0000000000000..096554abb201b --- /dev/null +++ b/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 + } +} diff --git a/src/test/java/org/elasticsearch/index/mapper/simple/test1.json b/src/test/java/org/elasticsearch/index/mapper/simple/test1.json index 0d6786929995f..93507daffefe8 100644 --- a/src/test/java/org/elasticsearch/index/mapper/simple/test1.json +++ b/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 - } -} \ No newline at end of file + ] + ], + nullValue:null +} diff --git a/src/test/java/org/elasticsearch/index/mapper/typelevels/ParseDocumentTypeLevelsTests.java b/src/test/java/org/elasticsearch/index/mapper/typelevels/ParseDocumentTypeLevelsTests.java index 2bde20e14f2f9..56d1a961c8d6a 100644 --- a/src/test/java/org/elasticsearch/index/mapper/typelevels/ParseDocumentTypeLevelsTests.java +++ b/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")); } } diff --git a/src/test/java/org/elasticsearch/index/query/data.json b/src/test/java/org/elasticsearch/index/query/data.json index ef942d9c55d2c..b3c6db83a07d2 100644 --- a/src/test/java/org/elasticsearch/index/query/data.json +++ b/src/test/java/org/elasticsearch/index/query/data.json @@ -1,47 +1,45 @@ { - 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, - "location":{ - "lat":1.1, - "lon":1.2 - } + ] + ], + nullValue:null, + "location":{ + "lat":1.1, + "lon":1.2 } } \ No newline at end of file diff --git a/src/test/java/org/elasticsearch/percolator/PercolatorTests.java b/src/test/java/org/elasticsearch/percolator/PercolatorTests.java index 6c20a50cfe2de..42859d7591321 100644 --- a/src/test/java/org/elasticsearch/percolator/PercolatorTests.java +++ b/src/test/java/org/elasticsearch/percolator/PercolatorTests.java @@ -164,11 +164,6 @@ public void testSimple2() throws Exception { .field("field2", "value") .endObject().endObject(); - XContentBuilder docWithType = XContentFactory.jsonBuilder().startObject().startObject("doc").startObject("type1") - .field("field1", 1) - .field("field2", "value") - .endObject().endObject().endObject(); - PercolateResponse response = client().preparePercolate().setSource(doc) .setIndices("test").setDocumentType("type1") .execute().actionGet(); @@ -187,13 +182,6 @@ public void testSimple2() throws Exception { assertThat(response.getMatches(), arrayWithSize(1)); assertThat(convertFromTextArray(response.getMatches(), "test"), arrayContaining("test1")); - response = client().preparePercolate() - .setIndices("test").setDocumentType("type1") - .setSource(docWithType).execute().actionGet(); - assertMatchCount(response, 1l); - assertThat(response.getMatches(), arrayWithSize(1)); - assertThat(convertFromTextArray(response.getMatches(), "test"), arrayContaining("test1")); - // add second query... client().prepareIndex("test", PercolatorService.TYPE_NAME, "test2") .setSource(XContentFactory.jsonBuilder().startObject().field("query", termQuery("field1", 1)).endObject()) @@ -430,7 +418,7 @@ public void multiplePercolators() throws Exception { percolate = client().preparePercolate() .setIndices("test").setDocumentType("type1") - .setSource(jsonBuilder().startObject().startObject("doc").startObject("type1").field("field1", "value2").endObject().endObject().endObject()) + .setSource(jsonBuilder().startObject().startObject("doc").field("field1", "value2").endObject().endObject()) .execute().actionGet(); assertMatchCount(percolate, 1l); assertThat(percolate.getMatches(), arrayWithSize(1)); @@ -471,7 +459,7 @@ public void dynamicAddingRemovingQueries() throws Exception { percolate = client().preparePercolate() .setIndices("test").setDocumentType("type1") - .setSource(jsonBuilder().startObject().startObject("doc").startObject("type1").field("field1", "value2").endObject().endObject().endObject()) + .setSource(jsonBuilder().startObject().startObject("doc").field("field1", "value2").endObject().endObject()) .execute().actionGet(); assertMatchCount(percolate, 1l); assertThat(percolate.getMatches(), arrayWithSize(1)); @@ -487,7 +475,7 @@ public void dynamicAddingRemovingQueries() throws Exception { .execute().actionGet(); PercolateSourceBuilder sourceBuilder = new PercolateSourceBuilder() - .setDoc(docBuilder().setDoc(jsonBuilder().startObject().startObject("type1").field("field1", "value2").endObject().endObject())) + .setDoc(docBuilder().setDoc(jsonBuilder().startObject().field("field1", "value2").endObject())) .setQueryBuilder(termQuery("color", "red")); percolate = client().preparePercolate() .setIndices("test").setDocumentType("type1") @@ -533,9 +521,9 @@ public void percolateWithSizeField() throws Exception { logger.info("--> percolate a document"); PercolateResponse percolate = client().preparePercolate().setIndices("test").setDocumentType("type1") .setSource(jsonBuilder().startObject() - .startObject("doc").startObject("type1") + .startObject("doc") .field("field1", "value1") - .endObject().endObject() + .endObject() .endObject()) .execute().actionGet(); assertMatchCount(percolate, 1l);