From 93b8e14824ac94ba77c3b915ef5690d977424cab Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Fri, 21 Dec 2018 18:08:13 -0800 Subject: [PATCH] Make sure to accept empty unnested mappings in create index requests. --- .../index/mapper/DocumentMapperParser.java | 21 +++++++- .../admin/indices/create/CreateIndexIT.java | 53 +++++++++++++++++++ .../MetaDataIndexTemplateServiceTests.java | 22 -------- .../index/mapper/MapperServiceTests.java | 22 +------- 4 files changed, 73 insertions(+), 45 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java index 15faa70456c2a..e63d5a279f3cd 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java @@ -172,12 +172,29 @@ private Tuple> extractMapping(String type, String so return extractMapping(type, root); } + /** + * Given an optional type name and mapping definition, returns the type and a normalized form of the mappings. + * + * The provided mapping definition may or may not contain the type name as the root key in the map. This method + * attempts to unwrap the mappings, so that they no longer contain a type name at the root. If no type name can + * be found, through either the 'type' parameter or by examining the provided mappings, then an exception will be + * thrown. + * + * @param type An optional type name. + * @param root The mapping definition. + * + * @return A tuple of the form (type, normalized mappings). + */ @SuppressWarnings({"unchecked"}) private Tuple> extractMapping(String type, Map root) throws MapperParsingException { if (root.size() == 0) { - // if we don't have any keys throw an exception - throw new MapperParsingException("malformed mapping no root object found"); + if (type != null) { + return new Tuple<>(type, root); + } else { + throw new MapperParsingException("malformed mapping, no type name found"); + } } + String rootName = root.keySet().iterator().next(); Tuple> mapping; if (type == null || type.equals(rootName)) { 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 index 3253eb1dc1c88..05da57cc5da45 100644 --- 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 @@ -24,17 +24,20 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.UnavailableShardsException; import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; +import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.support.ActiveShardCount; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.cluster.metadata.MappingMetaData; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.gateway.MetaStateService; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.query.RangeQueryBuilder; @@ -120,6 +123,56 @@ public void testDoubleAddMapping() throws Exception { } } + public void testNonNestedMappings() throws Exception { + assertAcked(prepareCreate("test") + .addMapping("_doc", XContentFactory.jsonBuilder().startObject() + .startObject("properties") + .startObject("date") + .field("type", "date") + .endObject() + .endObject() + .endObject())); + + GetMappingsResponse response = client().admin().indices().prepareGetMappings("test").get(); + + ImmutableOpenMap mappings = response.mappings().get("test"); + assertNotNull(mappings); + + MappingMetaData metadata = mappings.get("_doc"); + assertNotNull(metadata); + assertFalse(metadata.sourceAsMap().isEmpty()); + } + + public void testEmptyNestedMappings() throws Exception { + assertAcked(prepareCreate("test") + .addMapping("_doc", XContentFactory.jsonBuilder().startObject().endObject())); + + GetMappingsResponse response = client().admin().indices().prepareGetMappings("test").get(); + + ImmutableOpenMap mappings = response.mappings().get("test"); + assertNotNull(mappings); + + MappingMetaData metadata = mappings.get("_doc"); + assertNotNull(metadata); + assertTrue(metadata.sourceAsMap().isEmpty()); + } + + public void testEmptyMappings() throws Exception { + assertAcked(prepareCreate("test") + .addMapping("_doc", XContentFactory.jsonBuilder().startObject() + .startObject("_doc").endObject() + .endObject())); + + GetMappingsResponse response = client().admin().indices().prepareGetMappings("test").get(); + + ImmutableOpenMap mappings = response.mappings().get("test"); + assertNotNull(mappings); + + MappingMetaData metadata = mappings.get("_doc"); + assertNotNull(metadata); + assertTrue(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/template/put/MetaDataIndexTemplateServiceTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java index 6302766be9017..a929ee63c576e 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java @@ -97,17 +97,6 @@ public void testIndexTemplateWithAliasNameEqualToTemplatePattern() { assertThat(errors.get(0).getMessage(), equalTo("Alias [foobar] cannot be the same as any pattern in [foo, foobar]")); } - public void testIndexTemplateWithValidateEmptyMapping() throws Exception { - PutRequest request = new PutRequest("api", "validate_template"); - request.patterns(Collections.singletonList("validate_template")); - request.putMapping("type1", "{}"); - - List errors = putTemplateDetail(request); - assertThat(errors.size(), equalTo(1)); - assertThat(errors.get(0), instanceOf(MapperParsingException.class)); - assertThat(errors.get(0).getMessage(), containsString("malformed mapping no root object found")); - } - public void testIndexTemplateWithValidateMapping() throws Exception { PutRequest request = new PutRequest("api", "validate_template"); request.patterns(Collections.singletonList("te*")); @@ -132,17 +121,6 @@ public void testBrokenMapping() throws Exception { assertThat(errors.get(0).getMessage(), containsString("Failed to parse mapping ")); } - public void testBlankMapping() throws Exception { - PutRequest request = new PutRequest("api", "blank_mapping"); - request.patterns(Collections.singletonList("te*")); - request.putMapping("type1", "{}"); - - List errors = putTemplateDetail(request); - assertThat(errors.size(), equalTo(1)); - assertThat(errors.get(0), instanceOf(MapperParsingException.class)); - assertThat(errors.get(0).getMessage(), containsString("malformed mapping no root object found")); - } - public void testAliasInvalidFilterInvalidJson() throws Exception { //invalid json: put index template fails PutRequest request = new PutRequest("api", "blank_mapping"); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java b/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java index b5eeef0fa2847..2c1a75b40d4c0 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java @@ -39,8 +39,6 @@ import java.io.IOException; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; -import java.util.Map; import java.util.concurrent.ExecutionException; import static org.hamcrest.CoreMatchers.containsString; @@ -179,25 +177,7 @@ public void testUnmappedFieldType() { assertWarnings("[unmapped_type:string] should be replaced with [unmapped_type:keyword]"); } - public void testMergeWithMap() throws Throwable { - IndexService indexService1 = createIndex("index1"); - MapperService mapperService = indexService1.mapperService(); - Map> mappings = new HashMap<>(); - - mappings.put(MapperService.DEFAULT_MAPPING, MapperService.parseMapping(xContentRegistry(), "{}")); - MapperException e = expectThrows(MapperParsingException.class, - () -> mapperService.merge(mappings, MergeReason.MAPPING_UPDATE)); - assertThat(e.getMessage(), startsWith("Failed to parse mapping [" + MapperService.DEFAULT_MAPPING + "]: ")); - - mappings.clear(); - mappings.put("type1", MapperService.parseMapping(xContentRegistry(), "{}")); - - e = expectThrows( MapperParsingException.class, - () -> mapperService.merge(mappings, MergeReason.MAPPING_UPDATE)); - assertThat(e.getMessage(), startsWith("Failed to parse mapping [type1]: ")); - } - - public void testPartitionedConstraints() { + public void testPartitionedConstraints() { // partitioned index must have routing IllegalArgumentException noRoutingException = expectThrows(IllegalArgumentException.class, () -> { client().admin().indices().prepareCreate("test-index")