honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix get source HEAD requests 

Get source HEAD requests incorrectly return a content-length header of
0. This commit addresses this by removing the special handling for get
source HEAD requests, and just relying on the general mechanism that
exists for handling HEAD requests in the REST layer.

Relates #23151
This commit is contained in:
Jason Tedor 2017-02-14 16:37:22 -05:00 committed by GitHub
parent cab43707dc
commit 673754b1d5
4 changed files with 71 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
core/src/main/java/org/elasticsearch/action/ActionModule.java
View File

@ -564,7 +564,6 @@ public class ActionModule extends AbstractModule {
registerHandler.accept(new RestGetAction(settings, restController));
registerHandler.accept(new RestGetSourceAction(settings, restController));
registerHandler.accept(new RestHeadAction.Document(settings, restController));
registerHandler.accept(new RestHeadAction.Source(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));

20
core/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java
View File

@ -36,13 +36,19 @@ 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;
/**
* The REST handler for get source and head source APIs.
*/
public class RestGetSourceAction extends BaseRestHandler {
public RestGetSourceAction(Settings settings, RestController controller) {
public RestGetSourceAction(final Settings settings, final RestController controller) {
super(settings);
controller.registerHandler(GET, "/{index}/{type}/{id}/_source", this);
controller.registerHandler(HEAD, "/{index}/{type}/{id}/_source", this);
}
@Override
@ -50,7 +56,7 @@ public class RestGetSourceAction extends BaseRestHandler {
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.routing(request.param("routing"));
getRequest.parent(request.param("parent"));
getRequest.preference(request.param("preference"));
getRequest.realtime(request.paramAsBoolean("realtime", getRequest.realtime()));
@ -59,15 +65,16 @@ public class RestGetSourceAction extends BaseRestHandler {
return channel -> {
if (getRequest.fetchSourceContext() != null && !getRequest.fetchSourceContext().fetchSource()) {
ActionRequestValidationException validationError = new ActionRequestValidationException();
final ActionRequestValidationException validationError = new ActionRequestValidationException();
validationError.addValidationError("fetching source can not be disabled");
channel.sendResponse(new BytesRestResponse(channel, validationError));
} else {
client.get(getRequest, new RestResponseListener<GetResponse>(channel) {
@Override
public RestResponse buildResponse(GetResponse response) throws Exception {
XContentBuilder builder = channel.newBuilder(request.getXContentType(), false);
if (response.isSourceEmpty()) { // check if doc source (or doc itself) is missing
public RestResponse buildResponse(final GetResponse response) throws Exception {
final XContentBuilder builder = channel.newBuilder(request.getXContentType(), false);
// check if doc source (or doc itself) is missing
if (response.isSourceEmpty()) {
return new BytesRestResponse(NOT_FOUND, builder);
} else {
builder.rawValue(response.getSourceInternal());
@ -78,4 +85,5 @@ public class RestGetSourceAction extends BaseRestHandler {
}
};
}
}

23
core/src/main/java/org/elasticsearch/rest/action/document/RestHeadAction.java
View File

@ -48,33 +48,18 @@ public abstract class RestHeadAction extends BaseRestHandler {
*/
public static class Document extends RestHeadAction {
public Document(Settings settings, RestController controller) {
super(settings, false);
super(settings);
controller.registerHandler(HEAD, "/{index}/{type}/{id}", this);
}
}
/**
* Handler to check for document source existence (may be disabled in the mapping).
*/
public static class Source extends RestHeadAction {
public Source(Settings settings, RestController controller) {
super(settings, true);
controller.registerHandler(HEAD, "/{index}/{type}/{id}/_source", this);
}
}
private final boolean source;
/**
* All subclasses must be registered in {@link org.elasticsearch.common.network.NetworkModule}.
* @param settings injected settings
*
* @param settings injected settings
* @param source {@code false} to check for {@link GetResponse#isExists()}.
* {@code true} to also check for {@link GetResponse#isSourceEmpty()}.
*/
public RestHeadAction(Settings settings, boolean source) {
public RestHeadAction(Settings settings) {
super(settings);
this.source = source;
}
@Override
@ -95,8 +80,6 @@ public abstract class RestHeadAction extends BaseRestHandler {
public RestResponse buildResponse(GetResponse response) {
if (!response.isExists()) {
return new BytesRestResponse(NOT_FOUND, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY);
} else if (source && response.isSourceEmpty()) { // doc exists, but source might not (disabled in the mapping)
return new BytesRestResponse(NOT_FOUND, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY);
} else {
return new BytesRestResponse(OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY);
}

57
modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java
View File

@ -31,6 +31,9 @@ import java.util.Map;
import static java.util.Collections.emptyMap;
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;
@ -43,7 +46,18 @@ public class Netty4HeadBodyIsEmptyIT extends ESRestTestCase {
}
private void createTestDoc() throws IOException {
client().performRequest("PUT", "test/test/1", emptyMap(), new StringEntity("{\"test\": \"test\"}"));
createTestDoc("test", "test");
}
private void createTestDoc(final String indexName, final String typeName) throws IOException {
try (XContentBuilder builder = jsonBuilder()) {
builder.startObject();
{
builder.field("test", "test");
}
builder.endObject();
client().performRequest("PUT", "/" + indexName + "/" + typeName + "/" + "1", emptyMap(), new StringEntity(builder.string()));
}
}
public void testDocumentExists() throws IOException {
@ -110,9 +124,46 @@ public class Netty4HeadBodyIsEmptyIT extends ESRestTestCase {
}
}
private void headTestCase(String url, Map<String, String> params, Matcher<Integer> matcher) throws IOException {
public void testGetSourceAction() throws IOException {
createTestDoc();
headTestCase("/test/test/1/_source", emptyMap(), greaterThan(0));
headTestCase("/test/test/2/_source", emptyMap(), NOT_FOUND.getStatus(), equalTo(0));
try (XContentBuilder builder = jsonBuilder()) {
builder.startObject();
{
builder.startObject("mappings");
{
builder.startObject("test-no-source");
{
builder.startObject("_source");
{
builder.field("enabled", false);
}
builder.endObject();
}
builder.endObject();
}
builder.endObject();
}
builder.endObject();
client().performRequest("PUT", "/test-no-source", emptyMap(), new StringEntity(builder.string()));
createTestDoc("test-no-source", "test-no-source");
headTestCase("/test-no-source/test-no-source/1/_source", emptyMap(), NOT_FOUND.getStatus(), equalTo(0));
}
}
private void headTestCase(final String url, final Map<String, String> params, final Matcher<Integer> matcher) throws IOException {
headTestCase(url, params, OK.getStatus(), matcher);
}
private void headTestCase(
final String url,
final Map<String, String> params,
final int expectedStatusCode,
final Matcher<Integer> matcher) throws IOException {
Response response = client().performRequest("HEAD", url, params);
assertEquals(200, response.getStatusLine().getStatusCode());
assertEquals(expectedStatusCode, response.getStatusLine().getStatusCode());
assertThat(Integer.valueOf(response.getHeader("Content-Length")), matcher);
assertNull("HEAD requests shouldn't have a response body but " + url + " did", response.getEntity());
}