From dcf57f296e8f691e8099ea57ce011c41061e79c8 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 11 Jun 2017 14:58:56 -0400 Subject: [PATCH] Fix get mappings HEAD requests Get mappings HEAD requests incorrectly return a content-length header of 0. This commit addresses this by removing the special handling for get mappings HEAD requests, and just relying on the general mechanism that exists for handling HEAD requests in the REST layer. Relates #23192 --- .../elasticsearch/action/ActionModule.java | 2 - .../admin/indices/RestGetMappingAction.java | 106 +++++++++++++----- .../admin/indices/RestTypesExistsAction.java | 75 ------------- docs/reference/indices/get-mapping.asciidoc | 4 +- .../rest/Netty4HeadBodyIsEmptyIT.java | 10 +- .../indices.get_mapping/20_missing_type.yml | 89 ++++++++++++++- 6 files changed, 176 insertions(+), 110 deletions(-) delete mode 100644 core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestTypesExistsAction.java diff --git a/core/src/main/java/org/elasticsearch/action/ActionModule.java b/core/src/main/java/org/elasticsearch/action/ActionModule.java index 89994dc30f3..302c387cc13 100644 --- a/core/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/core/src/main/java/org/elasticsearch/action/ActionModule.java @@ -275,7 +275,6 @@ import org.elasticsearch.rest.action.admin.indices.RestRefreshAction; import org.elasticsearch.rest.action.admin.indices.RestRolloverIndexAction; import org.elasticsearch.rest.action.admin.indices.RestShrinkIndexAction; import org.elasticsearch.rest.action.admin.indices.RestSyncedFlushAction; -import org.elasticsearch.rest.action.admin.indices.RestTypesExistsAction; import org.elasticsearch.rest.action.admin.indices.RestUpdateSettingsAction; import org.elasticsearch.rest.action.admin.indices.RestUpgradeAction; import org.elasticsearch.rest.action.admin.indices.RestValidateQueryAction; @@ -547,7 +546,6 @@ public class ActionModule extends AbstractModule { registerHandler.accept(new RestGetAllAliasesAction(settings, restController)); registerHandler.accept(new RestGetAllMappingsAction(settings, restController)); registerHandler.accept(new RestGetAllSettingsAction(settings, restController, indexScopedSettings, settingsFilter)); - registerHandler.accept(new RestTypesExistsAction(settings, restController)); registerHandler.accept(new RestGetIndicesAction(settings, restController, indexScopedSettings, settingsFilter)); registerHandler.accept(new RestIndicesStatsAction(settings, restController)); registerHandler.accept(new RestIndicesSegmentsAction(settings, restController)); diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetMappingAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetMappingAction.java index f379f18fe71..99b8215025e 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetMappingAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetMappingAction.java @@ -19,6 +19,7 @@ package org.elasticsearch.rest.action.admin.indices; +import com.carrotsearch.hppc.cursors.ObjectCursor; import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsRequest; @@ -28,7 +29,9 @@ import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.metadata.MappingMetaData; import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.indices.TypeMissingException; @@ -37,21 +40,33 @@ import org.elasticsearch.rest.BytesRestResponse; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestResponse; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.rest.action.RestBuilderListener; import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import java.util.SortedSet; +import java.util.stream.Collectors; import static org.elasticsearch.rest.RestRequest.Method.GET; +import static org.elasticsearch.rest.RestRequest.Method.HEAD; import static org.elasticsearch.rest.RestStatus.OK; public class RestGetMappingAction extends BaseRestHandler { - public RestGetMappingAction(Settings settings, RestController controller) { + + public RestGetMappingAction(final Settings settings, final RestController controller) { super(settings); controller.registerHandler(GET, "/{index}/{type}/_mapping", this); controller.registerHandler(GET, "/{index}/_mappings", this); controller.registerHandler(GET, "/{index}/_mapping", this); controller.registerHandler(GET, "/{index}/_mappings/{type}", this); controller.registerHandler(GET, "/{index}/_mapping/{type}", this); + controller.registerHandler(HEAD, "/{index}/_mapping/{type}", this); controller.registerHandler(GET, "/_mapping/{type}", this); } @@ -64,48 +79,87 @@ public class RestGetMappingAction extends BaseRestHandler { public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { final String[] indices = Strings.splitStringByCommaToArray(request.param("index")); final String[] types = request.paramAsStringArrayOrEmptyIfAll("type"); - GetMappingsRequest getMappingsRequest = new GetMappingsRequest(); + final GetMappingsRequest getMappingsRequest = new GetMappingsRequest(); getMappingsRequest.indices(indices).types(types); getMappingsRequest.indicesOptions(IndicesOptions.fromRequest(request, getMappingsRequest.indicesOptions())); getMappingsRequest.local(request.paramAsBoolean("local", getMappingsRequest.local())); return channel -> client.admin().indices().getMappings(getMappingsRequest, new RestBuilderListener(channel) { @Override - public RestResponse buildResponse(GetMappingsResponse response, XContentBuilder builder) throws Exception { - - ImmutableOpenMap> mappingsByIndex = response.getMappings(); - if (mappingsByIndex.isEmpty()) { - if (indices.length != 0 && types.length != 0) { - return new BytesRestResponse(OK, builder.startObject().endObject()); - } else if (indices.length != 0) { + public RestResponse buildResponse(final GetMappingsResponse response, final XContentBuilder builder) throws Exception { + final ImmutableOpenMap> mappingsByIndex = response.getMappings(); + if (mappingsByIndex.isEmpty() && (indices.length != 0 || types.length != 0)) { + if (indices.length != 0 && types.length == 0) { builder.close(); - return new BytesRestResponse(channel, new IndexNotFoundException(indices[0])); - } else if (types.length != 0) { - builder.close(); - return new BytesRestResponse(channel, new TypeMissingException("_all", types[0])); + return new BytesRestResponse(channel, new IndexNotFoundException(String.join(",", indices))); } else { - return new BytesRestResponse(OK, builder.startObject().endObject()); + builder.close(); + return new BytesRestResponse(channel, new TypeMissingException("_all", String.join(",", types))); } } + final Set typeNames = new HashSet<>(); + for (final ObjectCursor> cursor : mappingsByIndex.values()) { + for (final ObjectCursor inner : cursor.value.keys()) { + typeNames.add(inner.value); + } + } + + final SortedSet difference = Sets.sortedDifference(Arrays.stream(types).collect(Collectors.toSet()), typeNames); + + // now remove requested aliases that contain wildcards that are simple matches + final List matches = new ArrayList<>(); + outer: + for (final String pattern : difference) { + if (pattern.contains("*")) { + for (final String typeName : typeNames) { + if (Regex.simpleMatch(pattern, typeName)) { + matches.add(pattern); + continue outer; + } + } + } + } + difference.removeAll(matches); + + final RestStatus status; builder.startObject(); - for (ObjectObjectCursor> indexEntry : mappingsByIndex) { - builder.startObject(indexEntry.key); - builder.startObject(Fields.MAPPINGS); - for (ObjectObjectCursor typeEntry : indexEntry.value) { - builder.field(typeEntry.key); - builder.map(typeEntry.value.sourceAsMap()); + { + if (difference.isEmpty()) { + status = RestStatus.OK; + } else { + status = RestStatus.NOT_FOUND; + final String message; + if (difference.size() == 1) { + message = String.format(Locale.ROOT, "type [%s] missing", toNamesString(difference.iterator().next())); + } else { + message = String.format(Locale.ROOT, "types [%s] missing", toNamesString(difference.toArray(new String[0]))); + } + builder.field("error", message); + builder.field("status", status.getStatus()); } - builder.endObject(); - builder.endObject(); - } + for (final ObjectObjectCursor> indexEntry : mappingsByIndex) { + builder.startObject(indexEntry.key); + { + builder.startObject("mappings"); + { + for (final ObjectObjectCursor typeEntry : indexEntry.value) { + builder.field(typeEntry.key, typeEntry.value.sourceAsMap()); + } + } + builder.endObject(); + } + builder.endObject(); + } + } builder.endObject(); - return new BytesRestResponse(OK, builder); + return new BytesRestResponse(status, builder); } }); } - static class Fields { - static final String MAPPINGS = "mappings"; + private static String toNamesString(final String... names) { + return Arrays.stream(names).collect(Collectors.joining(",")); } + } diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestTypesExistsAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestTypesExistsAction.java deleted file mode 100644 index b270fed30ce..00000000000 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestTypesExistsAction.java +++ /dev/null @@ -1,75 +0,0 @@ -/* - * 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.action.admin.indices.exists.types.TypesExistsRequest; -import org.elasticsearch.action.admin.indices.exists.types.TypesExistsResponse; -import org.elasticsearch.action.support.IndicesOptions; -import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.rest.BaseRestHandler; -import org.elasticsearch.rest.BytesRestResponse; -import org.elasticsearch.rest.RestController; -import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.RestResponse; -import org.elasticsearch.rest.action.RestResponseListener; - -import java.io.IOException; - -import static org.elasticsearch.rest.RestRequest.Method.HEAD; -import static org.elasticsearch.rest.RestStatus.NOT_FOUND; -import static org.elasticsearch.rest.RestStatus.OK; - -/** - * Rest api for checking if a type exists. - */ -public class RestTypesExistsAction extends BaseRestHandler { - public RestTypesExistsAction(Settings settings, RestController controller) { - super(settings); - controller.registerWithDeprecatedHandler( - HEAD, "/{index}/_mapping/{type}", this, - HEAD, "/{index}/{type}", deprecationLogger); - } - - @Override - public String getName() { - return "types_exists_action"; - } - - @Override - public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { - TypesExistsRequest typesExistsRequest = new TypesExistsRequest( - Strings.splitStringByCommaToArray(request.param("index")), Strings.splitStringByCommaToArray(request.param("type")) - ); - typesExistsRequest.local(request.paramAsBoolean("local", typesExistsRequest.local())); - typesExistsRequest.indicesOptions(IndicesOptions.fromRequest(request, typesExistsRequest.indicesOptions())); - return channel -> client.admin().indices().typesExists(typesExistsRequest, new RestResponseListener(channel) { - @Override - public RestResponse buildResponse(TypesExistsResponse response) throws Exception { - if (response.isExists()) { - return new BytesRestResponse(OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY); - } else { - return new BytesRestResponse(NOT_FOUND, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY); - } - } - }); - } -} diff --git a/docs/reference/indices/get-mapping.asciidoc b/docs/reference/indices/get-mapping.asciidoc index c3580917d9a..d1e45b2dbb0 100644 --- a/docs/reference/indices/get-mapping.asciidoc +++ b/docs/reference/indices/get-mapping.asciidoc @@ -23,9 +23,9 @@ following are some examples: [source,js] -------------------------------------------------- -GET /_mapping/tweet,kimchy +GET /_mapping/tweet -GET /_all/_mapping/tweet,book +GET /_all/_mapping/tweet -------------------------------------------------- // CONSOLE // TEST[setup:twitter] diff --git a/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java b/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java index 6f264fc5e7f..d1a5c15a29c 100644 --- a/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java +++ b/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java @@ -76,8 +76,14 @@ public class Netty4HeadBodyIsEmptyIT extends ESRestTestCase { public void testTypeExists() throws IOException { createTestDoc(); - headTestCase("/test/test", emptyMap(), equalTo(0)); - headTestCase("/test/test", singletonMap("pretty", "true"), equalTo(0)); + headTestCase("/test/_mapping/test", emptyMap(), greaterThan(0)); + headTestCase("/test/_mapping/test", singletonMap("pretty", "true"), greaterThan(0)); + } + + public void testTypeDoesNotExist() throws IOException { + createTestDoc(); + headTestCase("/test/_mapping/does-not-exist", emptyMap(), NOT_FOUND.getStatus(), greaterThan(0)); + headTestCase("/text/_mapping/test,does-not-exist", emptyMap(), NOT_FOUND.getStatus(), greaterThan(0)); } public void testAliasExists() throws IOException { diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_mapping/20_missing_type.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_mapping/20_missing_type.yml index a0f09949789..54e0e5e7557 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_mapping/20_missing_type.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_mapping/20_missing_type.yml @@ -1,5 +1,8 @@ --- -"Return empty response when type doesn't exist": +"Non-existent type returns 404": + - skip: + version: " - 5.99.99" + reason: Previous versions did not 404 on missing types - do: indices.create: index: test_index @@ -12,11 +15,91 @@ analyzer: whitespace - do: + catch: missing indices.get_mapping: index: test_index type: not_test_type - - - match: { '': {}} + + - match: { status: 404 } + - match: { error.reason: 'type[[not_test_type]] missing' } + +--- +"No type matching pattern returns 404": + - skip: + version: " - 5.99.99" + reason: Previous versions did not 404 on missing types + - do: + indices.create: + index: test_index + body: + mappings: + test_type: + properties: + text: + type: text + analyzer: whitespace + + - do: + catch: missing + indices.get_mapping: + index: test_index + type: test*,not* + + - match: { status: 404 } + - match: { error: 'type [not*] missing' } + - is_true: test_index.mappings.test_type + +--- +"Existent and non-existent type returns 404 and the existing type": + - skip: + version: " - 5.99.99" + reason: Previous versions did not 404 on missing types + - do: + indices.create: + index: test_index + body: + mappings: + test_type: + properties: + text: + type: text + analyzer: whitespace + + - do: + catch: missing + indices.get_mapping: + index: test_index + type: test_type,not_test_type + + - match: { status: 404 } + - match: { error: 'type [not_test_type] missing' } + - is_true: test_index.mappings.test_type + +--- +"Existent and non-existent types returns 404 and the existing type": + - skip: + version: " - 5.99.99" + reason: Previous versions did not 404 on missing types + - do: + indices.create: + index: test_index + body: + mappings: + test_type: + properties: + text: + type: text + analyzer: whitespace + + - do: + catch: missing + indices.get_mapping: + index: test_index + type: test_type,not_test_type,another_not_test_type + + - match: { status: 404 } + - match: { error: 'types [another_not_test_type,not_test_type] missing' } + - is_true: test_index.mappings.test_type --- "Type missing when no types exist":