From f5403ca4e7d4add086f62a1e4c262bb163ba89d6 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 5 Feb 2019 17:27:44 -0800 Subject: [PATCH 1/2] Make sure to reject mappings with type _doc when include_type_name is false. `CreateIndexRequest#source(Map, ... )`, which is used when deserializing index creation requests, accidentally accepts mappings that are nested twice under the type key (as described in the bug report #38266). This in turn causes us to be too lenient in parsing typeless mappings. In particular, we accept the following index creation request, even though it should not contain the type key `_doc`: ``` PUT index?include_type_name=false { "mappings": { "_doc": { "properties": { ... } } } } ``` There is a similar issue for both 'put templates' and 'put mappings' requests as well. This PR makes the minimal changes to detect and reject these typed mappings in requests. It does not address #38266 generally, or attempt a larger refactor around types in these server-side requests, as I think this should be done at a later time. --- .../test/indices.create/10_basic.yml | 20 +++++ .../test/indices.put_mapping/10_basic.yml | 25 ++++++ .../test/indices.put_template/10_basic.yml | 22 ++++++ .../test/indices.rollover/40_mapping.yml | 30 ++++++++ .../indices/rollover/RolloverRequest.java | 11 ++- .../metadata/MetaDataMappingService.java | 16 +--- .../index/mapper/MapperService.java | 14 ++++ .../admin/indices/RestCreateIndexAction.java | 39 +++++++--- .../indices/RestPutIndexTemplateAction.java | 24 ++---- .../admin/indices/RestPutMappingAction.java | 17 ++-- .../indices/RestCreateIndexActionTests.java | 77 +++++++++++++++++++ .../RestPutIndexTemplateActionTests.java | 73 ------------------ 12 files changed, 243 insertions(+), 125 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.create/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.create/10_basic.yml index 6f1b77a36b8b3..8e9d394126374 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.create/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.create/10_basic.yml @@ -113,3 +113,23 @@ properties: "": type: keyword + +--- +"Create index with explicit _doc type": + - skip: + version: " - 6.6.99" + reason: include_type_name was introduced in 6.7.0 + - do: + catch: bad_request + indices.create: + include_type_name: false + index: test_index + body: + mappings: + _doc: + properties: + field: + type: keyword + + - match: { error.type: "illegal_argument_exception" } + - match: { error.reason: "The mapping definition cannot be nested under a type [_doc] unless include_type_name is set to true." } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml index 80a8a04d29256..f17f5bcf01258 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml @@ -74,3 +74,28 @@ properties: "": type: keyword + +--- +"Put mappings with explicit _doc type": + - skip: + version: " - 6.6.99" + reason: include_type_name was introduced in 6.7.0 + + - do: + indices.create: + include_type_name: false + index: test_index + + - do: + catch: bad_request + indices.put_mapping: + include_type_name: false + index: test_index + body: + _doc: + properties: + field: + type: keyword + + - match: { error.type: "illegal_argument_exception" } + - match: { error.reason: "Types cannot be provided in put mapping requests, unless the include_type_name parameter is set to true." } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_template/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_template/10_basic.yml index b6cada313cc73..83e3bc9039cd1 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_template/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_template/10_basic.yml @@ -260,3 +260,25 @@ indices.put_template: name: test body: {} + +--- +"Put template with explicit _doc type": + - skip: + version: " - 6.6.99" + reason: include_type_name was introduced in 6.7.0 + + - do: + catch: bad_request + indices.put_template: + include_type_name: false + name: test + body: + index_patterns: test-* + mappings: + _doc: + properties: + field: + type: keyword + + - match: { error.type: "illegal_argument_exception" } + - match: { error.reason: "The mapping definition cannot be nested under a type [_doc] unless include_type_name is set to true." } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/40_mapping.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/40_mapping.yml index 7ed78c6e3159a..628fafb9965ea 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/40_mapping.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/40_mapping.yml @@ -45,3 +45,33 @@ - match: { conditions: { "[max_docs: 2]": true } } - match: { rolled_over: true } + +--- +"Mappings with explicit _doc type": + - skip: + version: " - 6.6.99" + reason: include_type_name was introduced in 6.7.0 + + - do: + indices.create: + index: logs-1 + body: + aliases: + logs_search: {} + + - do: + catch: bad_request + indices.rollover: + include_type_name: false + alias: "logs_search" + body: + conditions: + max_docs: 2 + mappings: + _doc: + properties: + field: + type: keyword + + - match: { error.caused_by.type: "illegal_argument_exception" } + - match: { error.caused_by.reason: "The mapping definition cannot be nested under a type [_doc] unless include_type_name is set to true." } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequest.java index 49db34aa5386e..f0165af2a66f5 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequest.java @@ -70,14 +70,17 @@ public class RolloverRequest extends AcknowledgedRequest implem CONDITIONS, ObjectParser.ValueType.OBJECT); PARSER.declareField((parser, request, context) -> request.createIndexRequest.settings(parser.map()), CreateIndexRequest.SETTINGS, ObjectParser.ValueType.OBJECT); - PARSER.declareField((parser, request, isTypeIncluded) -> { - if (isTypeIncluded) { + PARSER.declareField((parser, request, includeTypeName) -> { + if (includeTypeName) { for (Map.Entry mappingsEntry : parser.map().entrySet()) { request.createIndexRequest.mapping(mappingsEntry.getKey(), (Map) mappingsEntry.getValue()); } } else { - // a type is not included, add a dummy _doc type - request.createIndexRequest.mapping(MapperService.SINGLE_MAPPING_NAME, parser.map()); + Map mappings = parser.map(); + if (MapperService.isMappingSourceTyped(MapperService.SINGLE_MAPPING_NAME, mappings)) { + throw new IllegalArgumentException("The mapping definition cannot be nested under a type " + + "[" + MapperService.SINGLE_MAPPING_NAME + "] unless include_type_name is set to true."); + } } }, CreateIndexRequest.MAPPINGS, ObjectParser.ValueType.OBJECT); PARSER.declareField((parser, request, context) -> request.createIndexRequest.aliases(parser.map()), diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java index 0c04daf0e4bd7..b9e8a40e32c1d 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java @@ -38,8 +38,6 @@ import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.unit.TimeValue; -import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexService; @@ -57,6 +55,7 @@ import java.util.List; import java.util.Map; +import static org.elasticsearch.index.mapper.MapperService.isMappingSourceTyped; import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.NO_LONGER_ASSIGNED; /** @@ -276,7 +275,7 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt // try and parse it (no need to add it here) so we can bail early in case of parsing exception DocumentMapper newMapper; DocumentMapper existingMapper = mapperService.documentMapper(mappingType); - if (existingMapper == null && isMappingSourceTyped(mapperService, mappingUpdateSource, request.type()) == false) { + if (existingMapper == null && isMappingSourceTyped(request.type(), mappingUpdateSource) == false) { existingMapper = getMapperForUpdate(mapperService, mappingType); } String typeForUpdate = existingMapper == null ? mappingType : existingMapper.type(); @@ -337,7 +336,7 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt String typeForUpdate = mappingType; CompressedXContent existingSource = null; DocumentMapper existingMapper = mapperService.documentMapper(mappingType); - if (existingMapper == null && isMappingSourceTyped(mapperService, mappingUpdateSource, request.type()) == false) { + if (existingMapper == null && isMappingSourceTyped(request.type(), mappingUpdateSource) == false) { existingMapper = getMapperForUpdate(mapperService, mappingType); } if (existingMapper != null) { @@ -400,15 +399,6 @@ public String describeTasks(List tasks) { } } - /** - * Returns {@code true} if the given {@code mappingSource} includes a type - * as a top-level object. - */ - private static boolean isMappingSourceTyped(MapperService mapperService, CompressedXContent mappingSource, String type) { - Map root = XContentHelper.convertToMap(mappingSource.compressedReference(), true, XContentType.JSON).v2(); - return root.size() == 1 && root.keySet().iterator().next().equals(type); - } - public void putMapping(final PutMappingClusterStateUpdateRequest request, final ActionListener listener) { clusterService.submitStateUpdateTask("put-mapping", request, diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index ee6c1c9bc1a66..0695bb341b710 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -40,6 +40,7 @@ import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.AbstractIndexComponent; @@ -725,6 +726,19 @@ public DocumentMapperForType documentMapperWithAutoCreate(String type) { return new DocumentMapperForType(mapper, mapper.mapping()); } + /** + * Returns {@code true} if the given {@code mappingSource} includes a type + * as a top-level object. + */ + public static boolean isMappingSourceTyped(String type, Map mapping) { + return mapping.size() == 1 && mapping.keySet().iterator().next().equals(type); + } + + public static boolean isMappingSourceTyped(String type, CompressedXContent mappingSource) { + Map root = XContentHelper.convertToMap(mappingSource.compressedReference(), true, XContentType.JSON).v2(); + return isMappingSourceTyped(type, root); + } + /** * Returns the {@link MappedFieldType} for the give fullName. * diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestCreateIndexAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestCreateIndexAction.java index 064137edb4ceb..e5217cf4c6322 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestCreateIndexAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestCreateIndexAction.java @@ -57,22 +57,16 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { - final boolean includeTypeName = request.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, - DEFAULT_INCLUDE_TYPE_NAME_POLICY); - CreateIndexRequest createIndexRequest = new CreateIndexRequest(request.param("index")); + + boolean includeTypeName = request.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, + DEFAULT_INCLUDE_TYPE_NAME_POLICY); if (request.hasContent()) { Map sourceAsMap = XContentHelper.convertToMap(request.content(), false, request.getXContentType()).v2(); - if (sourceAsMap.containsKey("mappings")) { - if (includeTypeName == false) { - Map newSourceAsMap = new HashMap<>(sourceAsMap); - newSourceAsMap.put("mappings", Collections.singletonMap( - MapperService.SINGLE_MAPPING_NAME, sourceAsMap.get("mappings"))); - sourceAsMap = newSourceAsMap; - } else { - deprecationLogger.deprecatedAndMaybeLog("create_index_with_types", TYPES_DEPRECATION_MESSAGE); - } + if (includeTypeName && sourceAsMap.containsKey("mappings")) { + deprecationLogger.deprecatedAndMaybeLog("create_index_with_types", TYPES_DEPRECATION_MESSAGE); } + sourceAsMap = prepareMappings(sourceAsMap, includeTypeName); createIndexRequest.source(sourceAsMap, LoggingDeprecationHandler.INSTANCE); } @@ -85,4 +79,25 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC createIndexRequest.waitForActiveShards(ActiveShardCount.parseString(request.param("wait_for_active_shards"))); return channel -> client.admin().indices().create(createIndexRequest, new RestToXContentListener<>(channel)); } + + static Map prepareMappings(Map source, boolean includeTypeName) { + if (includeTypeName + || source.containsKey("mappings") == false + || (source.get("mappings") instanceof Map) == false) { + return source; + } + + Map newSource = new HashMap<>(source); + + @SuppressWarnings("unchecked") + Map mappings = (Map) source.get("mappings"); + + if (MapperService.isMappingSourceTyped(MapperService.SINGLE_MAPPING_NAME, mappings)) { + throw new IllegalArgumentException("The mapping definition cannot be nested under a type " + + "[" + MapperService.SINGLE_MAPPING_NAME + "] unless include_type_name is set to true."); + } + + newSource.put("mappings", Collections.singletonMap(MapperService.SINGLE_MAPPING_NAME, mappings)); + return newSource; + } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateAction.java index b1dd13324c85c..94f9efb316142 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateAction.java @@ -26,7 +26,6 @@ import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; @@ -35,7 +34,6 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.Map; public class RestPutIndexTemplateAction extends BaseRestHandler { @@ -72,24 +70,14 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC putRequest.cause(request.param("cause", "")); boolean includeTypeName = request.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, DEFAULT_INCLUDE_TYPE_NAME_POLICY); - Map sourceAsMap = prepareRequestSource(request, includeTypeName); - putRequest.source(sourceAsMap); - - return channel -> client.admin().indices().putTemplate(putRequest, new RestToXContentListener<>(channel)); - } - - Map prepareRequestSource(RestRequest request, boolean includeTypeName) { Map sourceAsMap = XContentHelper.convertToMap(request.requiredContent(), false, request.getXContentType()).v2(); - if (includeTypeName == false && sourceAsMap.containsKey("mappings")) { - Map newSourceAsMap = new HashMap<>(sourceAsMap); - newSourceAsMap.put("mappings", Collections.singletonMap(MapperService.SINGLE_MAPPING_NAME, sourceAsMap.get("mappings"))); - return newSourceAsMap; - } else { - if(includeTypeName && sourceAsMap.containsKey("mappings") ) { - DEPRECATION_LOGGER.deprecatedAndMaybeLog("put_index_template_with_types", TYPES_DEPRECATION_MESSAGE); - } - return sourceAsMap; + if (includeTypeName && sourceAsMap.containsKey("mappings")) { + DEPRECATION_LOGGER.deprecatedAndMaybeLog("put_index_template_with_types", TYPES_DEPRECATION_MESSAGE); } + sourceAsMap = RestCreateIndexAction.prepareMappings(sourceAsMap, includeTypeName); + putRequest.source(sourceAsMap); + + return channel -> client.admin().indices().putTemplate(putRequest, new RestToXContentListener<>(channel)); } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java index f5c171fe70765..26107b17e6080 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java @@ -26,6 +26,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestController; @@ -33,8 +34,10 @@ import org.elasticsearch.rest.action.RestToXContentListener; import java.io.IOException; +import java.util.Map; import static org.elasticsearch.client.Requests.putMappingRequest; +import static org.elasticsearch.index.mapper.MapperService.isMappingSourceTyped; import static org.elasticsearch.rest.RestRequest.Method.POST; import static org.elasticsearch.rest.RestRequest.Method.PUT; @@ -76,20 +79,24 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { - final boolean includeTypeName = request.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, + PutMappingRequest putMappingRequest = putMappingRequest(Strings.splitStringByCommaToArray(request.param("index"))); + + boolean includeTypeName = request.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, DEFAULT_INCLUDE_TYPE_NAME_POLICY); - final String type = request.param("type"); + String type = request.param("type"); + Map sourceAsMap = XContentHelper.convertToMap(request.requiredContent(), false, + request.getXContentType()).v2(); if (includeTypeName) { deprecationLogger.deprecatedAndMaybeLog("put_mapping_with_types", TYPES_DEPRECATION_MESSAGE); - } else if (type != null) { + } else if (type != null || isMappingSourceTyped(MapperService.SINGLE_MAPPING_NAME, sourceAsMap)) { throw new IllegalArgumentException("Types cannot be provided in put mapping requests, unless " + "the include_type_name parameter is set to true."); } - PutMappingRequest putMappingRequest = putMappingRequest(Strings.splitStringByCommaToArray(request.param("index"))); putMappingRequest.type(includeTypeName ? type : MapperService.SINGLE_MAPPING_NAME); - putMappingRequest.source(request.requiredContent(), request.getXContentType()); + putMappingRequest.source(sourceAsMap); + if (request.hasParam("update_all_types")) { deprecationLogger.deprecated("[update_all_types] is deprecated since indices may not have more than one type anymore"); } diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestCreateIndexActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestCreateIndexActionTests.java index 28c8ee7b99c8a..3e14acd801edc 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestCreateIndexActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestCreateIndexActionTests.java @@ -24,6 +24,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.rest.FakeRestRequest; @@ -71,4 +72,80 @@ public void testIncludeTypeName() throws IOException { action.prepareRequest(validRequest, mock(NodeClient.class)); } + + public void testPrepareTypelessRequest() throws IOException { + XContentBuilder content = XContentFactory.jsonBuilder().startObject() + .startObject("mappings") + .startObject("properties") + .startObject("field1").field("type", "keyword").endObject() + .startObject("field2").field("type", "text").endObject() + .endObject() + .endObject() + .startObject("aliases") + .startObject("read_alias").endObject() + .endObject() + .endObject(); + + Map contentAsMap = XContentHelper.convertToMap( + BytesReference.bytes(content), true, content.contentType()).v2(); + boolean includeTypeName = false; + Map source = RestCreateIndexAction.prepareMappings(contentAsMap, includeTypeName); + + XContentBuilder expectedContent = XContentFactory.jsonBuilder().startObject() + .startObject("mappings") + .startObject("_doc") + .startObject("properties") + .startObject("field1").field("type", "keyword").endObject() + .startObject("field2").field("type", "text").endObject() + .endObject() + .endObject() + .endObject() + .startObject("aliases") + .startObject("read_alias").endObject() + .endObject() + .endObject(); + Map expectedContentAsMap = XContentHelper.convertToMap( + BytesReference.bytes(expectedContent), true, expectedContent.contentType()).v2(); + + assertEquals(expectedContentAsMap, source); + } + + public void testPrepareTypedRequest() throws IOException { + XContentBuilder content = XContentFactory.jsonBuilder().startObject() + .startObject("mappings") + .startObject("type") + .startObject("properties") + .startObject("field1").field("type", "keyword").endObject() + .startObject("field2").field("type", "text").endObject() + .endObject() + .endObject() + .endObject() + .startObject("aliases") + .startObject("read_alias").endObject() + .endObject() + .endObject(); + + Map contentAsMap = XContentHelper.convertToMap( + BytesReference.bytes(content), true, content.contentType()).v2(); + boolean includeTypeName = true; + Map source = RestCreateIndexAction.prepareMappings(contentAsMap, includeTypeName); + + assertEquals(contentAsMap, source); + } + + public void testMalformedMappings() throws IOException { + XContentBuilder content = XContentFactory.jsonBuilder().startObject() + .field("mappings", "some string") + .startObject("aliases") + .startObject("read_alias").endObject() + .endObject() + .endObject(); + + Map contentAsMap = XContentHelper.convertToMap( + BytesReference.bytes(content), true, content.contentType()).v2(); + + boolean includeTypeName = false; + Map source = RestCreateIndexAction.prepareMappings(contentAsMap, includeTypeName); + assertEquals(contentAsMap, source); + } } diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateActionTests.java index 5da864207b621..8f7b26dd1207b 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateActionTests.java @@ -24,7 +24,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.rest.FakeRestRequest; @@ -32,10 +31,7 @@ import org.junit.Before; import java.io.IOException; -import java.util.HashMap; -import java.util.Map; -import static org.elasticsearch.rest.BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER; import static org.mockito.Mockito.mock; public class RestPutIndexTemplateActionTests extends RestActionTestCase { @@ -46,54 +42,6 @@ public void setUpAction() { action = new RestPutIndexTemplateAction(Settings.EMPTY, controller()); } - public void testPrepareTypelessRequest() throws IOException { - XContentBuilder content = XContentFactory.jsonBuilder().startObject() - .startObject("mappings") - .startObject("properties") - .startObject("field1").field("type", "keyword").endObject() - .startObject("field2").field("type", "text").endObject() - .endObject() - .endObject() - .startObject("aliases") - .startObject("read_alias").endObject() - .endObject() - .endObject(); - - Map params = new HashMap<>(); - params.put(INCLUDE_TYPE_NAME_PARAMETER, "false"); - RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) - .withMethod(RestRequest.Method.PUT) - .withParams(params) - .withPath("/_template/_some_template") - .withContent(BytesReference.bytes(content), XContentType.JSON) - .build(); - action.prepareRequest(request, mock(NodeClient.class)); - - // Internally the above prepareRequest method calls prepareRequestSource to inject a - // default type into the mapping. Here we test that this does what is expected by - // explicitly calling that same helper function - boolean includeTypeName = false; - Map source = action.prepareRequestSource(request, includeTypeName); - - XContentBuilder expectedContent = XContentFactory.jsonBuilder().startObject() - .startObject("mappings") - .startObject("_doc") - .startObject("properties") - .startObject("field1").field("type", "keyword").endObject() - .startObject("field2").field("type", "text").endObject() - .endObject() - .endObject() - .endObject() - .startObject("aliases") - .startObject("read_alias").endObject() - .endObject() - .endObject(); - Map expectedContentAsMap = XContentHelper.convertToMap( - BytesReference.bytes(expectedContent), true, expectedContent.contentType()).v2(); - - assertEquals(expectedContentAsMap, source); - } - public void testIncludeTypeName() throws IOException { XContentBuilder typedContent = XContentFactory.jsonBuilder().startObject() .startObject("mappings") @@ -116,26 +64,5 @@ public void testIncludeTypeName() throws IOException { .build(); action.prepareRequest(request, mock(NodeClient.class)); assertWarnings(RestPutIndexTemplateAction.TYPES_DEPRECATION_MESSAGE); - boolean includeTypeName = true; - Map source = action.prepareRequestSource(request, includeTypeName); - assertWarnings(RestPutIndexTemplateAction.TYPES_DEPRECATION_MESSAGE); - - XContentBuilder expectedContent = XContentFactory.jsonBuilder().startObject() - .startObject("mappings") - .startObject("my_doc") - .startObject("properties") - .startObject("field1").field("type", "keyword").endObject() - .startObject("field2").field("type", "text").endObject() - .endObject() - .endObject() - .endObject() - .startObject("aliases") - .startObject("read_alias").endObject() - .endObject() - .endObject(); - Map expectedContentAsMap = XContentHelper.convertToMap( - BytesReference.bytes(expectedContent), true, expectedContent.contentType()).v2(); - - assertEquals(expectedContentAsMap, source); } } From f72ac83d5f8dd19f4caa1445609db7028d326e07 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 5 Feb 2019 19:26:57 -0800 Subject: [PATCH 2/2] Fix RestPutMappingActionTests. --- .../IndicesClientDocumentationIT.java | 20 +++++----------- .../admin/indices/RestPutMappingAction.java | 2 +- .../indices/RestPutMappingActionTests.java | 24 ++++++++++++++++--- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java index 3400b9e3480e3..77f2e7aed133e 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java @@ -313,15 +313,13 @@ public void testCreateIndex() throws IOException { { request = new CreateIndexRequest("twitter2"); //tag::create-index-mappings-map - Map jsonMap = new HashMap<>(); Map message = new HashMap<>(); message.put("type", "text"); Map properties = new HashMap<>(); properties.put("message", message); Map mapping = new HashMap<>(); mapping.put("properties", properties); - jsonMap.put("_doc", mapping); - request.mapping(jsonMap); // <1> + request.mapping(mapping); // <1> //end::create-index-mappings-map CreateIndexResponse createIndexResponse = client.indices().create(request, RequestOptions.DEFAULT); assertTrue(createIndexResponse.isAcknowledged()); @@ -332,15 +330,11 @@ public void testCreateIndex() throws IOException { XContentBuilder builder = XContentFactory.jsonBuilder(); builder.startObject(); { - builder.startObject("_doc"); + builder.startObject("properties"); { - builder.startObject("properties"); + builder.startObject("message"); { - builder.startObject("message"); - { - builder.field("type", "text"); - } - builder.endObject(); + builder.field("type", "text"); } builder.endObject(); } @@ -381,10 +375,8 @@ public void testCreateIndex() throws IOException { " \"number_of_replicas\" : 0\n" + " },\n" + " \"mappings\" : {\n" + - " \"_doc\" : {\n" + - " \"properties\" : {\n" + - " \"message\" : { \"type\" : \"text\" }\n" + - " }\n" + + " \"properties\" : {\n" + + " \"message\" : { \"type\" : \"text\" }\n" + " }\n" + " },\n" + " \"aliases\" : {\n" + diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java index 26107b17e6080..24b0cd44fadc3 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java @@ -44,7 +44,7 @@ public class RestPutMappingAction extends BaseRestHandler { private static final DeprecationLogger deprecationLogger = new DeprecationLogger( LogManager.getLogger(RestPutMappingAction.class)); - public static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in create index " + + public static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in put mapping " + "requests is deprecated. To be compatible with 7.0, the mapping definition should not be nested under " + "the type name, and the parameter include_type_name must be provided and set to false."; diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingActionTests.java index d5d7a69151378..548531684a0c9 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingActionTests.java @@ -19,8 +19,11 @@ package org.elasticsearch.rest.action.admin.indices; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.rest.FakeRestChannel; @@ -40,10 +43,17 @@ public void setUpAction() { new RestPutMappingAction(Settings.EMPTY, controller()); } - public void testIncludeTypeName() { + public void testIncludeTypeName() throws Exception { + XContentBuilder content = XContentFactory.jsonBuilder().startObject() + .startObject("mappings") + .startObject("some_type").endObject() + .endObject() + .endObject(); + RestRequest deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry()) .withMethod(RestRequest.Method.PUT) - .withPath("/some_index/_mapping/") + .withPath("/some_index/_mapping") + .withContent(BytesReference.bytes(content), content.contentType()) .build(); dispatchRequest(deprecatedRequest); @@ -55,20 +65,28 @@ public void testIncludeTypeName() { .withMethod(RestRequest.Method.PUT) .withPath("/some_index/_mapping") .withParams(params) + .withContent(BytesReference.bytes(content), content.contentType()) .build(); dispatchRequest(validRequest); } - public void testTypeInPath() { + public void testTypeInPath() throws Exception { // Test that specifying a type while include_type_name is false // results in an illegal argument exception. Map params = new HashMap<>(); params.put(INCLUDE_TYPE_NAME_PARAMETER, "false"); + XContentBuilder content = XContentFactory.jsonBuilder().startObject() + .startObject("mappings") + .startObject("some_type").endObject() + .endObject() + .endObject(); + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) .withMethod(RestRequest.Method.PUT) .withPath("/some_index/_mapping/some_type") .withParams(params) + .withContent(BytesReference.bytes(content), content.contentType()) .build(); FakeRestChannel channel = new FakeRestChannel(request, false, 1);