From 0a5917d18259314dda5eb0b6c463f1d286f5d8d0 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 15 Feb 2017 13:07:29 -0500 Subject: [PATCH] Fix get HEAD requests Get HEAD requests incorrectly return a content-length header of 0. This commit addresses this by removing the special handling for get HEAD requests, and just relying on the general mechanism that exists for handling HEAD requests in the REST layer. Relates #23186 --- .../elasticsearch/action/ActionModule.java | 2 - .../rest/action/document/RestGetAction.java | 20 +++-- .../rest/action/document/RestHeadAction.java | 90 ------------------- .../rest/Netty4HeadBodyIsEmptyIT.java | 14 +-- 4 files changed, 19 insertions(+), 107 deletions(-) delete mode 100644 core/src/main/java/org/elasticsearch/rest/action/document/RestHeadAction.java diff --git a/core/src/main/java/org/elasticsearch/action/ActionModule.java b/core/src/main/java/org/elasticsearch/action/ActionModule.java index 25b500615fe..94db7d20308 100644 --- a/core/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/core/src/main/java/org/elasticsearch/action/ActionModule.java @@ -288,7 +288,6 @@ import org.elasticsearch.rest.action.document.RestBulkAction; import org.elasticsearch.rest.action.document.RestDeleteAction; import org.elasticsearch.rest.action.document.RestGetAction; import org.elasticsearch.rest.action.document.RestGetSourceAction; -import org.elasticsearch.rest.action.document.RestHeadAction; import org.elasticsearch.rest.action.document.RestIndexAction; import org.elasticsearch.rest.action.document.RestMultiGetAction; import org.elasticsearch.rest.action.document.RestMultiTermVectorsAction; @@ -563,7 +562,6 @@ public class ActionModule extends AbstractModule { registerHandler.accept(new RestIndexAction(settings, restController)); registerHandler.accept(new RestGetAction(settings, restController)); registerHandler.accept(new RestGetSourceAction(settings, restController)); - registerHandler.accept(new RestHeadAction.Document(settings, restController)); registerHandler.accept(new RestMultiGetAction(settings, restController)); registerHandler.accept(new RestDeleteAction(settings, restController)); registerHandler.accept(new org.elasticsearch.rest.action.document.RestCountAction(settings, restController)); diff --git a/core/src/main/java/org/elasticsearch/rest/action/document/RestGetAction.java b/core/src/main/java/org/elasticsearch/rest/action/document/RestGetAction.java index fc131aee3f6..c1a7c586809 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/document/RestGetAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/document/RestGetAction.java @@ -36,13 +36,16 @@ import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import java.io.IOException; import static org.elasticsearch.rest.RestRequest.Method.GET; +import static org.elasticsearch.rest.RestRequest.Method.HEAD; import static org.elasticsearch.rest.RestStatus.NOT_FOUND; import static org.elasticsearch.rest.RestStatus.OK; public class RestGetAction extends BaseRestHandler { - public RestGetAction(Settings settings, RestController controller) { + + public RestGetAction(final Settings settings, final RestController controller) { super(settings); controller.registerHandler(GET, "/{index}/{type}/{id}", this); + controller.registerHandler(HEAD, "/{index}/{type}/{id}", this); } @Override @@ -55,14 +58,14 @@ public class RestGetAction extends BaseRestHandler { getRequest.preference(request.param("preference")); getRequest.realtime(request.paramAsBoolean("realtime", getRequest.realtime())); if (request.param("fields") != null) { - throw new IllegalArgumentException("The parameter [fields] is no longer supported, " + + throw new IllegalArgumentException("the parameter [fields] is no longer supported, " + "please use [stored_fields] to retrieve stored fields or [_source] to load the field from _source"); } - String sField = request.param("stored_fields"); - if (sField != null) { - String[] sFields = Strings.splitStringByCommaToArray(sField); - if (sFields != null) { - getRequest.storedFields(sFields); + final String fieldsParam = request.param("stored_fields"); + if (fieldsParam != null) { + final String[] fields = Strings.splitStringByCommaToArray(fieldsParam); + if (fields != null) { + getRequest.storedFields(fields); } } @@ -73,9 +76,10 @@ public class RestGetAction extends BaseRestHandler { return channel -> client.get(getRequest, new RestToXContentListener(channel) { @Override - protected RestStatus getStatus(GetResponse response) { + protected RestStatus getStatus(final GetResponse response) { return response.isExists() ? OK : NOT_FOUND; } }); } + } diff --git a/core/src/main/java/org/elasticsearch/rest/action/document/RestHeadAction.java b/core/src/main/java/org/elasticsearch/rest/action/document/RestHeadAction.java deleted file mode 100644 index 14e412aebe1..00000000000 --- a/core/src/main/java/org/elasticsearch/rest/action/document/RestHeadAction.java +++ /dev/null @@ -1,90 +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.document; - -import org.elasticsearch.action.get.GetRequest; -import org.elasticsearch.action.get.GetResponse; -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; - -/** - * Base class for {@code HEAD} request handlers for a single document. - */ -public abstract class RestHeadAction extends BaseRestHandler { - - /** - * Handler to check for document existence. - */ - public static class Document extends RestHeadAction { - public Document(Settings settings, RestController controller) { - super(settings); - controller.registerHandler(HEAD, "/{index}/{type}/{id}", this); - } - } - - /** - * All subclasses must be registered in {@link org.elasticsearch.common.network.NetworkModule}. - * @param settings injected settings - * - */ - public RestHeadAction(Settings settings) { - super(settings); - } - - @Override - public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { - final GetRequest getRequest = new GetRequest(request.param("index"), request.param("type"), request.param("id")); - getRequest.operationThreaded(true); - getRequest.refresh(request.paramAsBoolean("refresh", getRequest.refresh())); - getRequest.routing(request.param("routing")); // order is important, set it after routing, so it will set the routing - getRequest.parent(request.param("parent")); - getRequest.preference(request.param("preference")); - getRequest.realtime(request.paramAsBoolean("realtime", getRequest.realtime())); - // don't get any fields back... - getRequest.storedFields(Strings.EMPTY_ARRAY); - // TODO we can also just return the document size as Content-Length - - return channel -> client.get(getRequest, new RestResponseListener(channel) { - @Override - public RestResponse buildResponse(GetResponse response) { - if (!response.isExists()) { - return new BytesRestResponse(NOT_FOUND, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY); - } else { - return new BytesRestResponse(OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY); - } - } - }); - } - -} 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 ac8e8ee0d0a..dab33fc070d 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 @@ -33,7 +33,6 @@ import static java.util.Collections.singletonMap; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.rest.RestStatus.NOT_FOUND; import static org.elasticsearch.rest.RestStatus.OK; -import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; @@ -62,20 +61,21 @@ public class Netty4HeadBodyIsEmptyIT extends ESRestTestCase { public void testDocumentExists() throws IOException { createTestDoc(); - headTestCase("test/test/1", emptyMap(), equalTo(0)); - headTestCase("test/test/1", singletonMap("pretty", "true"), equalTo(0)); + headTestCase("/test/test/1", emptyMap(), greaterThan(0)); + headTestCase("/test/test/1", singletonMap("pretty", "true"), greaterThan(0)); + headTestCase("/test/test/2", emptyMap(), NOT_FOUND.getStatus(), greaterThan(0)); } public void testIndexExists() throws IOException { createTestDoc(); - headTestCase("test", emptyMap(), greaterThan(0)); - headTestCase("test", singletonMap("pretty", "true"), greaterThan(0)); + headTestCase("/test", emptyMap(), greaterThan(0)); + headTestCase("/test", singletonMap("pretty", "true"), greaterThan(0)); } public void testTypeExists() throws IOException { createTestDoc(); - headTestCase("test/test", emptyMap(), equalTo(0)); - headTestCase("test/test", singletonMap("pretty", "true"), equalTo(0)); + headTestCase("/test/test", emptyMap(), equalTo(0)); + headTestCase("/test/test", singletonMap("pretty", "true"), equalTo(0)); } public void testAliasExists() throws IOException {