From dfa6164fba849879d945fd9f43195f96d26090ad Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Fri, 18 Jan 2019 12:28:31 -0800 Subject: [PATCH 1/4] Deprecate types in the put mapping API. From previous PRs, the `include_type_name` parameter was already present and defaulted to true. This PR makes the following updates: - Add deprecation warnings to `RestPutMappingAction`, plus tests in `RestPutMappingActionTests`. - Add a typeless 'put mappings' method to the Java HLRC, and deprecate the old typed version. To do this cleanly, I opted to create a new `PutMappingRequest` object that differs from the existing server one. --- .../elasticsearch/client/IndicesClient.java | 59 ++++++- .../client/IndicesRequestConverters.java | 18 +- .../client/indices/PutMappingRequest.java | 156 ++++++++++++++++++ .../elasticsearch/client/IndicesClientIT.java | 41 ++++- .../client/IndicesRequestConvertersTests.java | 28 +++- .../client/RestHighLevelClientTests.java | 82 ++++----- .../IndicesClientDocumentationIT.java | 37 ++--- .../indices/PutMappingRequestTests.java | 66 ++++++++ .../high-level/indices/put_mapping.asciidoc | 12 +- .../test/indices.put_mapping/10_basic.yml | 21 --- .../admin/indices/RestPutMappingAction.java | 21 ++- .../mapping/put/PutMappingRequestTests.java | 7 +- .../indices/RestPutMappingActionTests.java | 77 +++++++++ .../index/RandomCreateIndexGenerator.java | 22 ++- .../test/rest/ESRestTestCase.java | 11 ++ 15 files changed, 531 insertions(+), 127 deletions(-) create mode 100644 client/rest-high-level/src/main/java/org/elasticsearch/client/indices/PutMappingRequest.java create mode 100644 client/rest-high-level/src/test/java/org/elasticsearch/client/indices/PutMappingRequestTests.java create mode 100644 server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingActionTests.java diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java index d6d8efad46769..961e8f1699a54 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java @@ -42,7 +42,6 @@ import org.elasticsearch.action.admin.indices.mapping.get.GetFieldMappingsResponse; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsRequest; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; -import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.admin.indices.open.OpenIndexRequest; import org.elasticsearch.action.admin.indices.open.OpenIndexResponse; import org.elasticsearch.action.admin.indices.refresh.RefreshRequest; @@ -64,6 +63,7 @@ import org.elasticsearch.client.indices.FreezeIndexRequest; import org.elasticsearch.client.indices.GetIndexTemplatesRequest; import org.elasticsearch.client.indices.IndexTemplatesExistRequest; +import org.elasticsearch.client.indices.PutMappingRequest; import org.elasticsearch.client.indices.UnfreezeIndexRequest; import org.elasticsearch.rest.RestStatus; @@ -210,14 +210,37 @@ public AcknowledgedResponse putMapping(PutMappingRequest putMappingRequest, Requ *

* See * Put Mapping API on elastic.co - * @deprecated Prefer {@link #putMapping(PutMappingRequest, RequestOptions)} + * + * @deprecated This method uses an old request object which still refers to types, a deprecated feature. The + * method {@link #putMapping(PutMappingRequest, RequestOptions)} should be used instead, which accepts a new + * request object, and also uses request options instead of headers. */ @Deprecated - public AcknowledgedResponse putMapping(PutMappingRequest putMappingRequest, Header... headers) throws IOException { + public AcknowledgedResponse putMapping(org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest putMappingRequest, + Header... headers) throws IOException { return restHighLevelClient.performRequestAndParseEntity(putMappingRequest, IndicesRequestConverters::putMapping, AcknowledgedResponse::fromXContent, emptySet(), headers); } + /** + * Updates the mappings on an index using the Put Mapping API. + * See + * Put Mapping API on elastic.co + * @param putMappingRequest the request + * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized + * @return the response + * @throws IOException in case there is a problem sending the request or parsing back the response + * + * @deprecated This method uses an old request object which still refers to types, a deprecated feature. The method + * {@link #putMapping(PutMappingRequest, RequestOptions)} should be used instead, which accepts a new request object. + */ + @Deprecated + public AcknowledgedResponse putMapping(org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest putMappingRequest, + RequestOptions options) throws IOException { + return restHighLevelClient.performRequestAndParseEntity(putMappingRequest, IndicesRequestConverters::putMapping, options, + AcknowledgedResponse::fromXContent, emptySet()); + } + /** * Asynchronously updates the mappings on an index using the Put Mapping API. * See @@ -237,13 +260,37 @@ public void putMappingAsync(PutMappingRequest putMappingRequest, RequestOptions *

* See * Put Mapping API on elastic.co - * @deprecated Prefer {@link #putMappingAsync(PutMappingRequest, RequestOptions, ActionListener)} + * + * @deprecated This method uses an old request object which still refers to types, a deprecated feature. The + * method {@link #putMappingAsync(PutMappingRequest, RequestOptions, ActionListener)} should be used instead, + * which accepts a new request object, and also uses request options instead of headers. */ @Deprecated - public void putMappingAsync(PutMappingRequest putMappingRequest, ActionListener listener, + public void putMappingAsync(org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest putMappingRequest, + ActionListener listener, Header... headers) { restHighLevelClient.performRequestAsyncAndParseEntity(putMappingRequest, IndicesRequestConverters::putMapping, - AcknowledgedResponse::fromXContent, listener, emptySet(), headers); + AcknowledgedResponse::fromXContent, listener, emptySet(), headers); + } + + /** + * Asynchronously updates the mappings on an index using the Put Mapping API. + * See + * Put Mapping API on elastic.co + * @param putMappingRequest the request + * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized + * @param listener the listener to be notified upon request completion + * + * @deprecated This method uses an old request object which still refers to types, a deprecated feature. The + * method {@link #putMappingAsync(PutMappingRequest, RequestOptions, ActionListener)} should be used instead, + * which accepts a new request object. + */ + @Deprecated + public void putMappingAsync(org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest putMappingRequest, + RequestOptions options, + ActionListener listener) { + restHighLevelClient.performRequestAsyncAndParseEntity(putMappingRequest, IndicesRequestConverters::putMapping, options, + AcknowledgedResponse::fromXContent, listener, emptySet()); } /** diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesRequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesRequestConverters.java index 458416f02caff..6ba8fb077ca35 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesRequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesRequestConverters.java @@ -37,7 +37,6 @@ import org.elasticsearch.action.admin.indices.get.GetIndexRequest; import org.elasticsearch.action.admin.indices.mapping.get.GetFieldMappingsRequest; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsRequest; -import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.admin.indices.open.OpenIndexRequest; import org.elasticsearch.action.admin.indices.refresh.RefreshRequest; import org.elasticsearch.action.admin.indices.rollover.RolloverRequest; @@ -52,6 +51,7 @@ import org.elasticsearch.client.indices.FreezeIndexRequest; import org.elasticsearch.client.indices.GetIndexTemplatesRequest; import org.elasticsearch.client.indices.IndexTemplatesExistRequest; +import org.elasticsearch.client.indices.PutMappingRequest; import org.elasticsearch.client.indices.UnfreezeIndexRequest; import org.elasticsearch.common.Strings; @@ -120,14 +120,26 @@ static Request updateAliases(IndicesAliasesRequest indicesAliasesRequest) throws return request; } + static Request putMapping(PutMappingRequest putMappingRequest) throws IOException { + Request request = new Request(HttpPut.METHOD_NAME, RequestConverters.endpoint(putMappingRequest.indices(), "_mapping")); + + RequestConverters.Params parameters = new RequestConverters.Params(request); + parameters.withTimeout(putMappingRequest.timeout()); + parameters.withMasterTimeout(putMappingRequest.masterNodeTimeout()); + + request.setEntity(RequestConverters.createEntity(putMappingRequest, RequestConverters.REQUEST_BODY_CONTENT_TYPE)); + return request; + } + + static Request putMapping(org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest putMappingRequest) throws IOException { // The concreteIndex is an internal concept, not applicable to requests made over the REST API. if (putMappingRequest.getConcreteIndex() != null) { throw new IllegalArgumentException("concreteIndex cannot be set on PutMapping requests made over the REST API"); } - Request request = new Request(HttpPut.METHOD_NAME, RequestConverters.endpoint(putMappingRequest.indices(), "_mapping", - putMappingRequest.type())); + Request request = new Request(HttpPut.METHOD_NAME, RequestConverters.endpoint(putMappingRequest.indices(), + "_mapping", putMappingRequest.type())); RequestConverters.Params parameters = new RequestConverters.Params(request); parameters.withTimeout(putMappingRequest.timeout()); diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/indices/PutMappingRequest.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/indices/PutMappingRequest.java new file mode 100644 index 0000000000000..4607e7a5589ce --- /dev/null +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/indices/PutMappingRequest.java @@ -0,0 +1,156 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.client.indices; + +import com.carrotsearch.hppc.ObjectHashSet; +import org.elasticsearch.ElasticsearchGenerationException; +import org.elasticsearch.action.IndicesRequest; +import org.elasticsearch.action.support.IndicesOptions; +import org.elasticsearch.client.TimedRequest; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentType; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Map; + +/** + * Put a mapping definition into one or more indices. If an index already contains mappings, + * the new mappings will be merged with the existing one. If there are elements that cannot + * be merged, the request will be rejected. + */ +public class PutMappingRequest extends TimedRequest implements IndicesRequest, ToXContentObject { + + private static ObjectHashSet RESERVED_FIELDS = ObjectHashSet.from( + "_uid", "_id", "_type", "_source", "_all", "_analyzer", "_parent", "_routing", "_index", + "_size", "_timestamp", "_ttl", "_field_names" + ); + + private final String[] indices; + private IndicesOptions indicesOptions = IndicesOptions.fromOptions(false, false, true, true); + + private BytesReference source; + private XContentType xContentType; + + /** + * Constructs a new put mapping request against one or more indices. If no indices + * are provided then it will be executed against all indices. + */ + public PutMappingRequest(String... indices) { + this.indices = indices; + } + + /** + * The indices into which the mappings will be put. + */ + @Override + public String[] indices() { + return indices; + } + + @Override + public IndicesOptions indicesOptions() { + return indicesOptions; + } + + public PutMappingRequest indicesOptions(IndicesOptions indicesOptions) { + this.indicesOptions = indicesOptions; + return this; + } + + /** + * The mapping source definition. + */ + public BytesReference source() { + return source; + } + + /** + * The {@link XContentType} of the mapping source. + */ + public XContentType xContentType() { + return xContentType; + } + + /** + * The mapping source definition. + * + * Note that the definition should *not* be nested under a type name. + */ + public PutMappingRequest source(Map mappingSource) { + try { + XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON); + builder.map(mappingSource); + return source(builder); + } catch (IOException e) { + throw new ElasticsearchGenerationException("Failed to generate [" + mappingSource + "]", e); + } + } + + /** + * The mapping source definition. + * + * Note that the definition should *not* be nested under a type name. + */ + public PutMappingRequest source(String mappingSource, XContentType xContentType) { + this.source = new BytesArray(mappingSource); + this.xContentType = xContentType; + return this; + } + + /** + * The mapping source definition. + * + * Note that the definition should *not* be nested under a type name. + */ + public PutMappingRequest source(XContentBuilder builder) { + this.source = BytesReference.bytes(builder); + this.xContentType = builder.contentType(); + return this; + } + + /** + * The mapping source definition. + * + * Note that the definition should *not* be nested under a type name. + */ + public PutMappingRequest source(BytesReference source, XContentType xContentType) { + this.source = source; + this.xContentType = xContentType; + return this; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { + if (source != null) { + try (InputStream stream = source.streamInput()) { + builder.rawValue(stream, xContentType); + } + } else { + builder.startObject().endObject(); + } + return builder; + } +} diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java index 23c279fff0d88..b6e2b8381d382 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java @@ -47,7 +47,6 @@ import org.elasticsearch.action.admin.indices.mapping.get.GetFieldMappingsResponse; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsRequest; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; -import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.admin.indices.open.OpenIndexRequest; import org.elasticsearch.action.admin.indices.open.OpenIndexResponse; import org.elasticsearch.action.admin.indices.refresh.RefreshRequest; @@ -75,6 +74,7 @@ import org.elasticsearch.client.indices.FreezeIndexRequest; import org.elasticsearch.client.indices.GetIndexTemplatesRequest; import org.elasticsearch.client.indices.IndexTemplatesExistRequest; +import org.elasticsearch.client.indices.PutMappingRequest; import org.elasticsearch.client.indices.UnfreezeIndexRequest; import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -94,6 +94,7 @@ import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.rest.action.admin.indices.RestPutMappingAction; import java.io.IOException; import java.util.Arrays; @@ -413,25 +414,49 @@ public void testGetIndexNonExistentIndex() throws IOException { } public void testPutMapping() throws IOException { - // Add mappings to index String indexName = "mapping_index"; createIndex(indexName, Settings.EMPTY); PutMappingRequest putMappingRequest = new PutMappingRequest(indexName); - putMappingRequest.type("_doc"); XContentBuilder mappingBuilder = JsonXContent.contentBuilder(); mappingBuilder.startObject().startObject("properties").startObject("field"); mappingBuilder.field("type", "text"); mappingBuilder.endObject().endObject().endObject(); putMappingRequest.source(mappingBuilder); - AcknowledgedResponse putMappingResponse = - execute(putMappingRequest, highLevelClient().indices()::putMapping, highLevelClient().indices()::putMappingAsync, - highLevelClient().indices()::putMapping, highLevelClient().indices()::putMappingAsync); + AcknowledgedResponse putMappingResponse = execute(putMappingRequest, + highLevelClient().indices()::putMapping, + highLevelClient().indices()::putMappingAsync); assertTrue(putMappingResponse.isAcknowledged()); Map getIndexResponse = getAsMap(indexName); - assertEquals("text", XContentMapValues.extractValue(indexName + ".mappings._doc.properties.field.type", getIndexResponse)); + assertEquals("text", XContentMapValues.extractValue(indexName + ".mappings.properties.field.type", + getIndexResponse)); + } + + public void testPutMappingWithTypes() throws IOException { + String indexName = "mapping_index"; + createIndex(indexName, Settings.EMPTY); + + org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest putMappingRequest = + new org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest(indexName); + putMappingRequest.type("some_type"); + + XContentBuilder mappingBuilder = JsonXContent.contentBuilder(); + mappingBuilder.startObject().startObject("properties").startObject("field"); + mappingBuilder.field("type", "text"); + mappingBuilder.endObject().endObject().endObject(); + putMappingRequest.source(mappingBuilder); + + AcknowledgedResponse putMappingResponse = execute(putMappingRequest, + highLevelClient().indices()::putMapping, + highLevelClient().indices()::putMappingAsync, + expectWarnings(RestPutMappingAction.TYPES_DEPRECATION_MESSAGE)); + assertTrue(putMappingResponse.isAcknowledged()); + + Map getIndexResponse = getAsMap(indexName); + assertEquals("text", XContentMapValues.extractValue(indexName + ".mappings.properties.field.type", + getIndexResponse)); } public void testGetMapping() throws IOException { @@ -439,7 +464,6 @@ public void testGetMapping() throws IOException { createIndex(indexName, Settings.EMPTY); PutMappingRequest putMappingRequest = new PutMappingRequest(indexName); - putMappingRequest.type("_doc"); XContentBuilder mappingBuilder = JsonXContent.contentBuilder(); mappingBuilder.startObject().startObject("properties").startObject("field"); mappingBuilder.field("type", "text"); @@ -475,7 +499,6 @@ public void testGetFieldMapping() throws IOException { createIndex(indexName, Settings.EMPTY); PutMappingRequest putMappingRequest = new PutMappingRequest(indexName); - putMappingRequest.type("_doc"); XContentBuilder mappingBuilder = JsonXContent.contentBuilder(); mappingBuilder.startObject().startObject("properties").startObject("field"); mappingBuilder.field("type", "text"); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesRequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesRequestConvertersTests.java index b9cd4a4e709f6..db05241fd922d 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesRequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesRequestConvertersTests.java @@ -40,7 +40,6 @@ import org.elasticsearch.action.admin.indices.get.GetIndexRequest; import org.elasticsearch.action.admin.indices.mapping.get.GetFieldMappingsRequest; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsRequest; -import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.admin.indices.open.OpenIndexRequest; import org.elasticsearch.action.admin.indices.refresh.RefreshRequest; import org.elasticsearch.action.admin.indices.rollover.RolloverRequest; @@ -55,6 +54,7 @@ import org.elasticsearch.action.support.master.AcknowledgedRequest; import org.elasticsearch.client.indices.GetIndexTemplatesRequest; import org.elasticsearch.client.indices.IndexTemplatesExistRequest; +import org.elasticsearch.client.indices.PutMappingRequest; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; @@ -163,7 +163,31 @@ public void testUpdateAliases() throws IOException { } public void testPutMapping() throws IOException { - PutMappingRequest putMappingRequest = new PutMappingRequest(); + String[] indices = RequestConvertersTests.randomIndicesNames(0, 5); + PutMappingRequest putMappingRequest = new PutMappingRequest(indices); + + Map expectedParams = new HashMap<>(); + RequestConvertersTests.setRandomTimeout(putMappingRequest, AcknowledgedRequest.DEFAULT_ACK_TIMEOUT, expectedParams); + RequestConvertersTests.setRandomMasterTimeout(putMappingRequest, expectedParams); + + Request request = IndicesRequestConverters.putMapping(putMappingRequest); + + StringJoiner endpoint = new StringJoiner("/", "/", ""); + String index = String.join(",", indices); + if (Strings.hasLength(index)) { + endpoint.add(index); + } + endpoint.add("_mapping"); + + Assert.assertEquals(endpoint.toString(), request.getEndpoint()); + Assert.assertEquals(expectedParams, request.getParameters()); + Assert.assertEquals(HttpPut.METHOD_NAME, request.getMethod()); + RequestConvertersTests.assertToXContentBody(putMappingRequest, request.getEntity()); + } + + public void testPutMappingWithTypes() throws IOException { + org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest putMappingRequest = + new org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest(); String[] indices = RequestConvertersTests.randomIndicesNames(0, 5); putMappingRequest.indices(indices); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java index e894d60c8cda9..69e515db45d96 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java @@ -737,12 +737,14 @@ public void testApiNamingConventions() throws Exception { ClientYamlSuiteRestSpec restSpec = ClientYamlSuiteRestSpec.load("/rest-api-spec/api"); Set apiSpec = restSpec.getApis().stream().map(ClientYamlSuiteRestApi::getName).collect(Collectors.toSet()); + Set apiUnsupported = new HashSet<>(apiSpec); + Set apiNotFound = new HashSet<>(); Set topLevelMethodsExclusions = new HashSet<>(); topLevelMethodsExclusions.add("getLowLevelClient"); topLevelMethodsExclusions.add("close"); - Map methods = Arrays.stream(RestHighLevelClient.class.getMethods()) + Map> methods = Arrays.stream(RestHighLevelClient.class.getMethods()) .filter(method -> method.getDeclaringClass().equals(RestHighLevelClient.class) && topLevelMethodsExclusions.contains(method.getName()) == false && (method.getParameterCount() == 0 @@ -750,44 +752,44 @@ public void testApiNamingConventions() throws Exception { .map(method -> Tuple.tuple(toSnakeCase(method.getName()), method)) .flatMap(tuple -> tuple.v2().getReturnType().getName().endsWith("Client") ? getSubClientMethods(tuple.v1(), tuple.v2().getReturnType()) : Stream.of(tuple)) - .collect(Collectors.toMap(Tuple::v1, Tuple::v2)); - - Set apiNotFound = new HashSet<>(); + .collect(Collectors.groupingBy(Tuple::v1, + Collectors.mapping(Tuple::v2, Collectors.toSet()))); - for (Map.Entry entry : methods.entrySet()) { - Method method = entry.getValue(); + for (Map.Entry> entry : methods.entrySet()) { String apiName = entry.getKey(); - assertTrue("method [" + apiName + "] is not final", + for (Method method : entry.getValue()) { + assertTrue("method [" + apiName + "] is not final", Modifier.isFinal(method.getClass().getModifiers()) || Modifier.isFinal(method.getModifiers())); - assertTrue("method [" + method + "] should be public", Modifier.isPublic(method.getModifiers())); - - //we convert all the method names to snake case, hence we need to look for the '_async' suffix rather than 'Async' - if (apiName.endsWith("_async")) { - assertAsyncMethod(methods, method, apiName); - } else if (isSubmitTaskMethod(apiName)) { - assertSubmitTaskMethod(methods, method, apiName, restSpec); - } else { - assertSyncMethod(method, apiName); - boolean remove = apiSpec.remove(apiName); - if (remove == false) { - if (deprecatedMethods.contains(apiName)) { - assertTrue("method [" + method.getName() + "], api [" + apiName + "] should be deprecated", - method.isAnnotationPresent(Deprecated.class)); - } else { - //TODO xpack api are currently ignored, we need to load xpack yaml spec too - if (apiName.startsWith("xpack.") == false && - apiName.startsWith("license.") == false && - apiName.startsWith("machine_learning.") == false && - apiName.startsWith("rollup.") == false && - apiName.startsWith("watcher.") == false && - apiName.startsWith("graph.") == false && - apiName.startsWith("migration.") == false && - apiName.startsWith("security.") == false && - apiName.startsWith("index_lifecycle.") == false && - apiName.startsWith("ccr.") == false && - apiName.endsWith("freeze") == false) { - apiNotFound.add(apiName); + assertTrue("method [" + method + "] should be public", Modifier.isPublic(method.getModifiers())); + + //we convert all the method names to snake case, hence we need to look for the '_async' suffix rather than 'Async' + if (apiName.endsWith("_async")) { + assertAsyncMethod(methods, method, apiName); + } else if (isSubmitTaskMethod(apiName)) { + assertSubmitTaskMethod(methods, method, apiName, restSpec); + } else { + assertSyncMethod(method, apiName); + apiUnsupported.remove(apiName); + if (apiSpec.contains(apiName) == false) { + if (deprecatedMethods.contains(apiName)) { + assertTrue("method [" + method.getName() + "], api [" + apiName + "] should be deprecated", + method.isAnnotationPresent(Deprecated.class)); + } else { + //TODO xpack api are currently ignored, we need to load xpack yaml spec too + if (apiName.startsWith("xpack.") == false && + apiName.startsWith("license.") == false && + apiName.startsWith("machine_learning.") == false && + apiName.startsWith("rollup.") == false && + apiName.startsWith("watcher.") == false && + apiName.startsWith("graph.") == false && + apiName.startsWith("migration.") == false && + apiName.startsWith("security.") == false && + apiName.startsWith("index_lifecycle.") == false && + apiName.startsWith("ccr.") == false && + apiName.endsWith("freeze") == false) { + apiNotFound.add(apiName); + } } } } @@ -797,11 +799,11 @@ public void testApiNamingConventions() throws Exception { apiNotFound.size(), equalTo(0)); //we decided not to support cat API in the high-level REST client, they are supposed to be used from a low-level client - apiSpec.removeIf(api -> api.startsWith("cat.")); + apiUnsupported.removeIf(api -> api.startsWith("cat.")); Stream.concat(Arrays.stream(notYetSupportedApi), Arrays.stream(notRequiredApi)).forEach( api -> assertTrue(api + " API is either not defined in the spec or already supported by the high-level client", - apiSpec.remove(api))); - assertThat("Some API are not supported but they should be: " + apiSpec, apiSpec.size(), equalTo(0)); + apiUnsupported.remove(api))); + assertThat("Some API are not supported but they should be: " + apiUnsupported, apiUnsupported.size(), equalTo(0)); } private static void assertSyncMethod(Method method, String apiName) { @@ -832,7 +834,7 @@ private static void assertSyncMethod(Method method, String apiName) { } } - private static void assertAsyncMethod(Map methods, Method method, String apiName) { + private static void assertAsyncMethod(Map> methods, Method method, String apiName) { assertTrue("async method [" + method.getName() + "] doesn't have corresponding sync method", methods.containsKey(apiName.substring(0, apiName.length() - 6))); assertThat("async method [" + method + "] should return void", method.getReturnType(), equalTo(Void.TYPE)); @@ -852,7 +854,7 @@ private static void assertAsyncMethod(Map methods, Method method } } - private static void assertSubmitTaskMethod(Map methods, Method method, String apiName, + private static void assertSubmitTaskMethod(Map> methods, Method method, String apiName, ClientYamlSuiteRestSpec restSpec) { String methodName = extractMethodName(apiName); assertTrue("submit task method [" + method.getName() + "] doesn't have corresponding sync method", 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 02381800d6833..94bb0a085e7a3 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 @@ -46,7 +46,6 @@ import org.elasticsearch.action.admin.indices.mapping.get.GetFieldMappingsResponse; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsRequest; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; -import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.admin.indices.open.OpenIndexRequest; import org.elasticsearch.action.admin.indices.open.OpenIndexResponse; import org.elasticsearch.action.admin.indices.refresh.RefreshRequest; @@ -78,6 +77,7 @@ import org.elasticsearch.client.indices.FreezeIndexRequest; import org.elasticsearch.client.indices.GetIndexTemplatesRequest; import org.elasticsearch.client.indices.IndexTemplatesExistRequest; +import org.elasticsearch.client.indices.PutMappingRequest; import org.elasticsearch.client.indices.UnfreezeIndexRequest; import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.IndexTemplateMetaData; @@ -466,7 +466,6 @@ public void testPutMapping() throws IOException { { // tag::put-mapping-request PutMappingRequest request = new PutMappingRequest("twitter"); // <1> - request.type("_doc"); // <2> // end::put-mapping-request { @@ -519,21 +518,13 @@ public void testPutMapping() throws IOException { AcknowledgedResponse putMappingResponse = client.indices().putMapping(request, RequestOptions.DEFAULT); assertTrue(putMappingResponse.isAcknowledged()); } - { - //tag::put-mapping-shortcut - request.source("message", "type=text"); // <1> - //end::put-mapping-shortcut - AcknowledgedResponse putMappingResponse = client.indices().putMapping(request, RequestOptions.DEFAULT); - assertTrue(putMappingResponse.isAcknowledged()); - } // tag::put-mapping-request-timeout - request.timeout(TimeValue.timeValueMinutes(2)); // <1> - request.timeout("2m"); // <2> + request.setTimeout(TimeValue.timeValueMinutes(2)); // <1> // end::put-mapping-request-timeout + // tag::put-mapping-request-masterTimeout - request.masterNodeTimeout(TimeValue.timeValueMinutes(1)); // <1> - request.masterNodeTimeout("1m"); // <2> + request.setMasterTimeout(TimeValue.timeValueMinutes(1)); // <1> // end::put-mapping-request-masterTimeout // tag::put-mapping-execute @@ -556,7 +547,7 @@ public void testPutMappingAsync() throws Exception { } { - PutMappingRequest request = new PutMappingRequest("twitter").type("_doc"); + PutMappingRequest request = new PutMappingRequest("twitter"); // tag::put-mapping-execute-listener ActionListener listener = @@ -592,7 +583,6 @@ public void testGetMapping() throws IOException { CreateIndexResponse createIndexResponse = client.indices().create(new CreateIndexRequest("twitter"), RequestOptions.DEFAULT); assertTrue(createIndexResponse.isAcknowledged()); PutMappingRequest request = new PutMappingRequest("twitter"); - request.type("_doc"); request.source( "{\n" + " \"properties\": {\n" + @@ -649,7 +639,6 @@ public void testGetMappingAsync() throws Exception { CreateIndexResponse createIndexResponse = client.indices().create(new CreateIndexRequest("twitter"), RequestOptions.DEFAULT); assertTrue(createIndexResponse.isAcknowledged()); PutMappingRequest request = new PutMappingRequest("twitter"); - request.type("_doc"); request.source( "{\n" + " \"properties\": {\n" + @@ -659,7 +648,7 @@ public void testGetMappingAsync() throws Exception { " }\n" + "}", // <1> XContentType.JSON); - AcknowledgedResponse putMappingResponse = client.indices().putMapping(request); + AcknowledgedResponse putMappingResponse = client.indices().putMapping(request, RequestOptions.DEFAULT); assertTrue(putMappingResponse.isAcknowledged()); } @@ -719,7 +708,6 @@ public void testGetFieldMapping() throws IOException, InterruptedException { CreateIndexResponse createIndexResponse = client.indices().create(new CreateIndexRequest("twitter"), RequestOptions.DEFAULT); assertTrue(createIndexResponse.isAcknowledged()); PutMappingRequest request = new PutMappingRequest("twitter"); - request.type("_doc"); request.source( "{\n" + " \"properties\": {\n" + @@ -2509,10 +2497,15 @@ public void testAnalyze() throws IOException, InterruptedException { CreateIndexResponse resp = client.indices().create(req, RequestOptions.DEFAULT); assertTrue(resp.isAcknowledged()); - PutMappingRequest pmReq = new PutMappingRequest() - .indices("my_index") - .type("_doc") - .source("my_field", "type=text,analyzer=english"); + PutMappingRequest pmReq = new PutMappingRequest("my_index") + .source(XContentFactory.jsonBuilder().startObject() + .startObject("properties") + .startObject("my_field") + .field("type", "text") + .field("analyzer", "english") + .endObject() + .endObject() + .endObject()); AcknowledgedResponse pmResp = client.indices().putMapping(pmReq, RequestOptions.DEFAULT); assertTrue(pmResp.isAcknowledged()); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/indices/PutMappingRequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/indices/PutMappingRequestTests.java new file mode 100644 index 0000000000000..40b38e40b2647 --- /dev/null +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/indices/PutMappingRequestTests.java @@ -0,0 +1,66 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.client.indices; + +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.index.RandomCreateIndexGenerator; +import org.elasticsearch.test.AbstractXContentTestCase; + +import java.io.IOException; + +public class PutMappingRequestTests extends AbstractXContentTestCase { + + @Override + protected PutMappingRequest createTestInstance() { + PutMappingRequest request = new PutMappingRequest(); + if (frequently()) { + try { + XContentBuilder builder = RandomCreateIndexGenerator.randomMapping(); + request.source(builder); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + return request; + } + + @Override + protected PutMappingRequest doParseInstance(XContentParser parser) throws IOException { + PutMappingRequest request = new PutMappingRequest(); + request.source(parser.map()); + return request; + } + + @Override + protected boolean supportsUnknownFields() { + return false; + } + + @Override + protected void assertEqualInstances(PutMappingRequest expected, PutMappingRequest actual) { + try (XContentParser expectedJson = createParser(expected.xContentType().xContent(), expected.source()); + XContentParser actualJson = createParser(actual.xContentType().xContent(), actual.source())) { + assertEquals(expectedJson.mapOrdered(), actualJson.mapOrdered()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } +} diff --git a/docs/java-rest/high-level/indices/put_mapping.asciidoc b/docs/java-rest/high-level/indices/put_mapping.asciidoc index d1b9c6ad8c6fd..971ad52d62b78 100644 --- a/docs/java-rest/high-level/indices/put_mapping.asciidoc +++ b/docs/java-rest/high-level/indices/put_mapping.asciidoc @@ -10,14 +10,13 @@ [id="{upid}-{api}-request"] ==== Put Mapping Request -A +{request}+ requires an `index` argument, and a type: +A +{request}+ requires an `index` argument: ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- include-tagged::{doc-tests-file}[{api}-request] -------------------------------------------------- <1> The index to add the mapping to -<2> The type to create (or update) ==== Mapping source A description of the fields to create on the mapping; if not defined, the mapping will default to empty. @@ -46,13 +45,6 @@ include-tagged::{doc-tests-file}[{api}-xcontent] <1> Mapping source provided as an `XContentBuilder` object, the Elasticsearch built-in helpers to generate JSON content -["source","java",subs="attributes,callouts,macros"] --------------------------------------------------- -include-tagged::{doc-tests-file}[{api}-shortcut] --------------------------------------------------- -<1> Mapping source provided as `Object` key-pairs, which gets converted to -JSON format - ==== Optional arguments The following arguments can optionally be provided: @@ -61,14 +53,12 @@ The following arguments can optionally be provided: include-tagged::{doc-tests-file}[{api}-request-timeout] -------------------------------------------------- <1> Timeout to wait for the all the nodes to acknowledge the index creation as a `TimeValue` -<2> Timeout to wait for the all the nodes to acknowledge the index creation as a `String` ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- include-tagged::{doc-tests-file}[{api}-request-masterTimeout] -------------------------------------------------- <1> Timeout to connect to the master node as a `TimeValue` -<2> Timeout to connect to the master node as a `String` include::../execution.asciidoc[] 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 eda0a07e734e0..80a8a04d29256 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,24 +74,3 @@ properties: "": type: keyword - ---- -"PUT mapping with a type and include_type_name: false": - - skip: - version: " - 6.6.99" - reason: include_type_name was introduced in 6.7.0 - - do: - indices.create: - index: index - include_type_name: false - - - do: - catch: /illegal_argument_exception/ - indices.put_mapping: - index: index - type: _doc - include_type_name: false - body: - properties: - bar: - type: float 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 6881642649f54..9f69551fbcb52 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 @@ -39,8 +39,10 @@ import static org.elasticsearch.rest.RestRequest.Method.PUT; public class RestPutMappingAction extends BaseRestHandler { - - private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(LogManager.getLogger(RestPutMappingAction.class)); + private static final DeprecationLogger deprecationLogger = new DeprecationLogger( + LogManager.getLogger(RestPutMappingAction.class)); + public static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Using include_type_name in put " + + "mapping requests is deprecated. The parameter will be removed in the next major version."; public RestPutMappingAction(Settings settings, RestController controller) { super(settings); @@ -73,16 +75,23 @@ 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); - PutMappingRequest putMappingRequest = putMappingRequest(Strings.splitStringByCommaToArray(request.param("index"))); + final boolean includeTypeName = request.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, + DEFAULT_INCLUDE_TYPE_NAME_POLICY); + if (request.hasParam(INCLUDE_TYPE_NAME_PARAMETER)) { + deprecationLogger.deprecatedAndMaybeLog("put_mapping_with_types", TYPES_DEPRECATION_MESSAGE); + } + final String type = request.param("type"); if (type != null && includeTypeName == false) { - throw new IllegalArgumentException("Cannot set include_type_name=false and provide a type at the same time"); + 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()); if (request.hasParam("update_all_types")) { - DEPRECATION_LOGGER.deprecated("[update_all_types] is deprecated since indices may not have more than one type anymore"); + deprecationLogger.deprecated("[update_all_types] is deprecated since indices may not have more than one type anymore"); } putMappingRequest.updateAllTypes(request.paramAsBoolean("update_all_types", false)); putMappingRequest.timeout(request.paramAsTime("timeout", putMappingRequest.timeout())); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java index eac9fb53fddde..a59cfc39d1272 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java @@ -44,14 +44,9 @@ public class PutMappingRequestTests extends ESTestCase { public void testValidation() { - PutMappingRequest r = new PutMappingRequest("myindex"); + PutMappingRequest r = new PutMappingRequest("myindex").type(""); ActionRequestValidationException ex = r.validate(); assertNotNull("type validation should fail", ex); - assertTrue(ex.getMessage().contains("type is missing")); - - r.type(""); - ex = r.validate(); - assertNotNull("type validation should fail", ex); assertTrue(ex.getMessage().contains("type is empty")); r.type("mytype"); 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 new file mode 100644 index 0000000000000..daa69c20007f1 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingActionTests.java @@ -0,0 +1,77 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.rest.action.admin.indices; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.test.rest.FakeRestChannel; +import org.elasticsearch.test.rest.FakeRestRequest; +import org.elasticsearch.test.rest.RestActionTestCase; +import org.junit.Before; + +import java.util.HashMap; +import java.util.Map; + +import static org.elasticsearch.rest.BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER; + +public class RestPutMappingActionTests extends RestActionTestCase { + + @Before + public void setUpAction() { + new RestPutMappingAction(Settings.EMPTY, controller()); + } + + public void testIncludeTypeName() { + Map params = new HashMap<>(); + params.put(INCLUDE_TYPE_NAME_PARAMETER, randomFrom("true", "false")); + RestRequest deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry()) + .withMethod(RestRequest.Method.PUT) + .withPath("/some_index/_mapping/") + .withParams(params) + .build(); + + dispatchRequest(deprecatedRequest); + assertWarnings(RestPutMappingAction.TYPES_DEPRECATION_MESSAGE); + + RestRequest validRequest = new FakeRestRequest.Builder(xContentRegistry()) + .withMethod(RestRequest.Method.PUT) + .withPath("/some_index/_mapping") + .build(); + dispatchRequest(validRequest); + } + + public void testTypeInPath() { + // Test that specifying a type while include_type_name is false + // results in an illegal argument exception. + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) + .withMethod(RestRequest.Method.PUT) + .withPath("/some_index/_mapping/some_type") + .build(); + + FakeRestChannel channel = new FakeRestChannel(request, false, 1); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + controller().dispatchRequest(request, channel, threadContext); + + assertEquals(1, channel.errors().get()); + assertEquals(RestStatus.BAD_REQUEST, channel.capturedResponse().status()); + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/index/RandomCreateIndexGenerator.java b/test/framework/src/main/java/org/elasticsearch/index/RandomCreateIndexGenerator.java index e88a9f0a38d2c..e4836150c6e86 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/RandomCreateIndexGenerator.java +++ b/test/framework/src/main/java/org/elasticsearch/index/RandomCreateIndexGenerator.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.XContentType; import java.io.IOException; @@ -31,6 +32,7 @@ import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; import static org.elasticsearch.test.ESTestCase.randomAlphaOfLength; import static org.elasticsearch.test.ESTestCase.randomBoolean; +import static org.elasticsearch.test.ESTestCase.randomFrom; import static org.elasticsearch.test.ESTestCase.randomIntBetween; public final class RandomCreateIndexGenerator { @@ -76,8 +78,26 @@ public static Settings randomIndexSettings() { return builder.build(); } + + /** + * Creates a random mapping, with no mention of types. + */ + public static XContentBuilder randomMapping() throws IOException { + XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); + builder.startObject(); + + randomMappingFields(builder, true); + + builder.endObject(); + return builder; + } + + /** + * Creates a random mapping, with the mapping definition nested + * under the given type name. + */ public static XContentBuilder randomMapping(String type) throws IOException { - XContentBuilder builder = XContentFactory.jsonBuilder(); + XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); builder.startObject().startObject(type); randomMappingFields(builder, true); diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java index fdacf2aa50f17..06890a9d5a669 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java @@ -238,6 +238,17 @@ private boolean isExclusivelyTargetingCurrentVersionCluster() { } } + + /** + * Creates request options designed to be used when making a call that can return warnings, for example a + * deprecated request. The options will ensure that the given warnings are returned if all nodes are on + * {@link Version#CURRENT} and will allow (but not require) the warnings if any node is running an older version. + * + * @param warnings The expected warnings. + */ + public static RequestOptions expectWarnings(String... warnings) { + return expectVersionSpecificWarnings(consumer -> consumer.current(warnings)); + } public static RequestOptions expectVersionSpecificWarnings(Consumer expectationsSetter) { Builder builder = RequestOptions.DEFAULT.toBuilder(); From c2637695184506c5f641b92f5edb9bd37bdee48c Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Fri, 18 Jan 2019 17:38:15 -0800 Subject: [PATCH 2/4] Updates to account for different default for include_type_name. --- .../elasticsearch/client/IndicesRequestConverters.java | 4 +++- .../java/org/elasticsearch/client/IndicesClientIT.java | 4 ++-- .../client/IndicesRequestConvertersTests.java | 5 +++-- .../action/admin/indices/RestPutMappingAction.java | 9 ++++----- .../admin/indices/RestPutMappingActionTests.java | 10 +++++++--- 5 files changed, 19 insertions(+), 13 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesRequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesRequestConverters.java index 6ba8fb077ca35..798b1d9582155 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesRequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesRequestConverters.java @@ -58,6 +58,8 @@ import java.io.IOException; import java.util.Locale; +import static org.elasticsearch.rest.BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER; + final class IndicesRequestConverters { private IndicesRequestConverters() {} @@ -120,13 +122,13 @@ static Request updateAliases(IndicesAliasesRequest indicesAliasesRequest) throws return request; } - static Request putMapping(PutMappingRequest putMappingRequest) throws IOException { Request request = new Request(HttpPut.METHOD_NAME, RequestConverters.endpoint(putMappingRequest.indices(), "_mapping")); RequestConverters.Params parameters = new RequestConverters.Params(request); parameters.withTimeout(putMappingRequest.timeout()); parameters.withMasterTimeout(putMappingRequest.masterNodeTimeout()); + parameters.putParam(INCLUDE_TYPE_NAME_PARAMETER, "false"); request.setEntity(RequestConverters.createEntity(putMappingRequest, RequestConverters.REQUEST_BODY_CONTENT_TYPE)); return request; diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java index b6e2b8381d382..0e7c8be2c3407 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java @@ -430,7 +430,7 @@ public void testPutMapping() throws IOException { assertTrue(putMappingResponse.isAcknowledged()); Map getIndexResponse = getAsMap(indexName); - assertEquals("text", XContentMapValues.extractValue(indexName + ".mappings.properties.field.type", + assertEquals("text", XContentMapValues.extractValue(indexName + ".mappings._doc.properties.field.type", getIndexResponse)); } @@ -455,7 +455,7 @@ public void testPutMappingWithTypes() throws IOException { assertTrue(putMappingResponse.isAcknowledged()); Map getIndexResponse = getAsMap(indexName); - assertEquals("text", XContentMapValues.extractValue(indexName + ".mappings.properties.field.type", + assertEquals("text", XContentMapValues.extractValue(indexName + ".mappings.some_type.properties.field.type", getIndexResponse)); } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesRequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesRequestConvertersTests.java index db05241fd922d..16690aa2fc8e7 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesRequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesRequestConvertersTests.java @@ -61,7 +61,6 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.index.RandomCreateIndexGenerator; -import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.test.ESTestCase; import org.junit.Assert; @@ -80,6 +79,7 @@ import static org.elasticsearch.index.RandomCreateIndexGenerator.randomCreateIndexRequest; import static org.elasticsearch.index.RandomCreateIndexGenerator.randomIndexSettings; import static org.elasticsearch.index.alias.RandomAliasActionsGenerator.randomAliasAction; +import static org.elasticsearch.rest.BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.Matchers.nullValue; @@ -169,6 +169,7 @@ public void testPutMapping() throws IOException { Map expectedParams = new HashMap<>(); RequestConvertersTests.setRandomTimeout(putMappingRequest, AcknowledgedRequest.DEFAULT_ACK_TIMEOUT, expectedParams); RequestConvertersTests.setRandomMasterTimeout(putMappingRequest, expectedParams); + expectedParams.put(INCLUDE_TYPE_NAME_PARAMETER, "false"); Request request = IndicesRequestConverters.putMapping(putMappingRequest); @@ -390,7 +391,7 @@ public void testGetIndex() throws IOException { RequestConvertersTests.setRandomLocal(getIndexRequest, expectedParams); RequestConvertersTests.setRandomHumanReadable(getIndexRequest, expectedParams); // Force "include_type_name" parameter since responses need to be compatible when coming from 7.0 nodes - expectedParams.put(BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER, "true"); + expectedParams.put(INCLUDE_TYPE_NAME_PARAMETER, "true"); if (ESTestCase.randomBoolean()) { // the request object will not have include_defaults present unless it is set to 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 9f69551fbcb52..9a9b95687964e 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 @@ -77,12 +77,11 @@ public String getName() { 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); - if (request.hasParam(INCLUDE_TYPE_NAME_PARAMETER)) { - deprecationLogger.deprecatedAndMaybeLog("put_mapping_with_types", TYPES_DEPRECATION_MESSAGE); - } - final String type = request.param("type"); - if (type != null && includeTypeName == false) { + + if (includeTypeName) { + deprecationLogger.deprecatedAndMaybeLog("put_mapping_with_types", TYPES_DEPRECATION_MESSAGE); + } else if (type != null) { throw new IllegalArgumentException("Types cannot be provided in put mapping requests, unless " + "the include_type_name parameter is set to true."); } 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 daa69c20007f1..d5d7a69151378 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 @@ -41,20 +41,20 @@ public void setUpAction() { } public void testIncludeTypeName() { - Map params = new HashMap<>(); - params.put(INCLUDE_TYPE_NAME_PARAMETER, randomFrom("true", "false")); RestRequest deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry()) .withMethod(RestRequest.Method.PUT) .withPath("/some_index/_mapping/") - .withParams(params) .build(); dispatchRequest(deprecatedRequest); assertWarnings(RestPutMappingAction.TYPES_DEPRECATION_MESSAGE); + Map params = new HashMap<>(); + params.put(INCLUDE_TYPE_NAME_PARAMETER, "false"); RestRequest validRequest = new FakeRestRequest.Builder(xContentRegistry()) .withMethod(RestRequest.Method.PUT) .withPath("/some_index/_mapping") + .withParams(params) .build(); dispatchRequest(validRequest); } @@ -62,9 +62,13 @@ public void testIncludeTypeName() { public void testTypeInPath() { // 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"); + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) .withMethod(RestRequest.Method.PUT) .withPath("/some_index/_mapping/some_type") + .withParams(params) .build(); FakeRestChannel channel = new FakeRestChannel(request, false, 1); From cf736fd5ad3096724e9b2d6438889413df3b55a4 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Fri, 18 Jan 2019 17:49:36 -0800 Subject: [PATCH 3/4] Remove an unused constant in PutMappingRequest. --- .../org/elasticsearch/client/indices/PutMappingRequest.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/indices/PutMappingRequest.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/indices/PutMappingRequest.java index 4607e7a5589ce..16a885796a9ad 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/indices/PutMappingRequest.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/indices/PutMappingRequest.java @@ -19,7 +19,6 @@ package org.elasticsearch.client.indices; -import com.carrotsearch.hppc.ObjectHashSet; import org.elasticsearch.ElasticsearchGenerationException; import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.support.IndicesOptions; @@ -43,11 +42,6 @@ */ public class PutMappingRequest extends TimedRequest implements IndicesRequest, ToXContentObject { - private static ObjectHashSet RESERVED_FIELDS = ObjectHashSet.from( - "_uid", "_id", "_type", "_source", "_all", "_analyzer", "_parent", "_routing", "_index", - "_size", "_timestamp", "_ttl", "_field_names" - ); - private final String[] indices; private IndicesOptions indicesOptions = IndicesOptions.fromOptions(false, false, true, true); From 6cfd122bbf7aef1573481fee1593f8b1fae11767 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 21 Jan 2019 16:29:25 +0100 Subject: [PATCH 4/4] Fix edge case in PutMappingRequestTests The recently introduced client side PutMappingRequestTests has an edge case that leads to failing tests. When the "source" or the original request is `null` it gets rendered to xContent as an empty object, which by the test class itself is parsed back as an empty map. For this reason the original and the parsed request differ (one has an empty source, one an empty map). This fixes this edge case by assuming an empty map means a null source in the request. In practice the distinction doesn't matter because all the client side request does is write itself to xContent, which gives the same result regardless of whether `source` is null or empty. Closes #37654 --- .../indices/PutMappingRequestTests.java | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/indices/PutMappingRequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/indices/PutMappingRequestTests.java index 40b38e40b2647..50224aa1b9ad7 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/indices/PutMappingRequestTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/indices/PutMappingRequestTests.java @@ -25,6 +25,7 @@ import org.elasticsearch.test.AbstractXContentTestCase; import java.io.IOException; +import java.util.Map; public class PutMappingRequestTests extends AbstractXContentTestCase { @@ -45,7 +46,10 @@ protected PutMappingRequest createTestInstance() { @Override protected PutMappingRequest doParseInstance(XContentParser parser) throws IOException { PutMappingRequest request = new PutMappingRequest(); - request.source(parser.map()); + Map map = parser.map(); + if (map.isEmpty() == false) { + request.source(map); + } return request; } @@ -56,11 +60,16 @@ protected boolean supportsUnknownFields() { @Override protected void assertEqualInstances(PutMappingRequest expected, PutMappingRequest actual) { - try (XContentParser expectedJson = createParser(expected.xContentType().xContent(), expected.source()); - XContentParser actualJson = createParser(actual.xContentType().xContent(), actual.source())) { - assertEquals(expectedJson.mapOrdered(), actualJson.mapOrdered()); - } catch (IOException e) { - throw new RuntimeException(e); + if (actual.source() != null) { + try (XContentParser expectedJson = createParser(expected.xContentType().xContent(), expected.source()); + XContentParser actualJson = createParser(actual.xContentType().xContent(), actual.source())) { + assertEquals(expectedJson.mapOrdered(), actualJson.mapOrdered()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } else { + // if the original `source` is null, the parsed source should be so too + assertNull(expected.source()); } } }