Skip to content

Commit

Permalink
In the field capabilities API, re-add support for fields in the req…
Browse files Browse the repository at this point in the history
…uest body (elastic#88972)

We previously removed support for `fields` in the request body, to ensure there
was only one way to specify the parameter. We've now decided to undo the
change, since it was disruptive and the request body is actually the best place to
pass variable-length data like `fields`.

This PR restores support for `fields` in the request body. It throws an error
if the parameter is specified both in the URL and the body.

Closes elastic#86875
  • Loading branch information
likzn authored Aug 4, 2022
1 parent b81f418 commit f28f454
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 6 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# Elasticsearch Changlog
# Elasticsearch Changelog

Please see the [release notes](https://www.elastic.co/guide/en/elasticsearch/reference/current/es-release-notes.html) in the reference manual.
6 changes: 6 additions & 0 deletions docs/changelog/88972.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 88972
summary: In the field capabilities API, re-add support for fields in the request body
area: Search
type: enhancement
issues:
- 86875
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ setup:

---
"Get simple field caps":

- do:
field_caps:
index: 'test1,test2,test3'
Expand Down Expand Up @@ -162,6 +161,21 @@ setup:
- match: {fields.geo.keyword.indices: ["test3"]}
- is_false: fields.geo.keyword.non_searchable_indices
- is_false: fields.geo.keyword.on_aggregatable_indices

---
"Get field caps with fields in body":
- skip:
version: " - 8.4.99"
reason: re-added support for fields in the request body in 8.5
- do:
field_caps:
index: 'test1,test2,test3'
body:
fields: [text]

- match: {fields.text.text.searchable: true}
- match: {fields.text.text.aggregatable: false}
- is_false: fields.keyword
---
"Get date_nanos field caps":

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static org.elasticsearch.index.query.AbstractQueryBuilder.parseInnerQueryBuilder;
import static org.elasticsearch.rest.RestRequest.Method.GET;
import static org.elasticsearch.rest.RestRequest.Method.POST;
import static org.elasticsearch.xcontent.ObjectParser.fromList;

public class RestFieldCapabilitiesAction extends BaseRestHandler {

Expand All @@ -43,10 +44,9 @@ public String getName() {

@Override
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
String[] indices = Strings.splitStringByCommaToArray(request.param("index"));
FieldCapabilitiesRequest fieldRequest = new FieldCapabilitiesRequest().fields(
Strings.splitStringByCommaToArray(request.param("fields"))
).indices(indices);
final String[] indices = Strings.splitStringByCommaToArray(request.param("index"));
final FieldCapabilitiesRequest fieldRequest = new FieldCapabilitiesRequest();
fieldRequest.indices(indices);

fieldRequest.indicesOptions(IndicesOptions.fromRequest(request, fieldRequest.indicesOptions()));
fieldRequest.includeUnmapped(request.paramAsBoolean("include_unmapped", false));
Expand All @@ -57,16 +57,28 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
PARSER.parse(parser, fieldRequest, null);
}
});
if (request.hasParam("fields")) {
if (fieldRequest.fields().length > 0) {
throw new IllegalArgumentException(
"can't specify a request body and [fields]"
+ " request parameter, either specify a request body or the"
+ " [fields] request parameter"
);
}
fieldRequest.fields(Strings.splitStringByCommaToArray(request.param("fields")));
}
return channel -> client.fieldCaps(fieldRequest, new RestToXContentListener<>(channel));
}

private static ParseField INDEX_FILTER_FIELD = new ParseField("index_filter");
private static ParseField RUNTIME_MAPPINGS_FIELD = new ParseField("runtime_mappings");
private static ParseField FIELDS_FIELD = new ParseField("fields");

private static final ObjectParser<FieldCapabilitiesRequest, Void> PARSER = new ObjectParser<>("field_caps_request");

static {
PARSER.declareObject(FieldCapabilitiesRequest::indexFilter, (p, c) -> parseInnerQueryBuilder(p), INDEX_FILTER_FIELD);
PARSER.declareObject(FieldCapabilitiesRequest::runtimeFields, (p, c) -> p.map(), RUNTIME_MAPPINGS_FIELD);
PARSER.declareStringArray(fromList(String.class, FieldCapabilitiesRequest::fields), FIELDS_FIELD);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.search.SearchModule;
import org.elasticsearch.test.AbstractWireSerializingTestCase;
import org.elasticsearch.xcontent.ObjectParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ToXContent;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentFactory;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.xcontent.XContentType;
import org.elasticsearch.xcontent.json.JsonXContent;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -31,6 +35,7 @@
import java.util.function.Consumer;

import static java.util.Collections.singletonMap;
import static org.elasticsearch.xcontent.ObjectParser.fromList;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.containsString;
Expand Down Expand Up @@ -146,6 +151,18 @@ public void testToXContent() throws IOException {
}""").replaceAll("\\s+", ""), xContent);
}

public void testFromXContent() throws IOException {
XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"fields\" : [\"FOO\"] }");
FieldCapabilitiesRequest request = new FieldCapabilitiesRequest();
ObjectParser<FieldCapabilitiesRequest, Void> PARSER = new ObjectParser<>("field_caps_request");
PARSER.declareStringArray(fromList(String.class, FieldCapabilitiesRequest::fields), new ParseField("fields"));

PARSER.parse(parser, request, null);

assertArrayEquals(request.fields(), new String[] { "FOO" });

}

public void testValidation() {
FieldCapabilitiesRequest request = new FieldCapabilitiesRequest().indices("index2");
ActionRequestValidationException exception = request.validate();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.rest.action;

import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.elasticsearch.xcontent.XContentType;
import org.junit.Before;

import java.io.IOException;
import java.util.HashMap;

import static org.mockito.Mockito.mock;

public class RestFieldCapabilitiesActionTests extends ESTestCase {

private RestFieldCapabilitiesAction action;

@Before
public void setUpAction() {
action = new RestFieldCapabilitiesAction();
}

public void testRequestBodyAndParamsBothInput() throws IOException {
String content = "{ \"fields\": [\"title\"] }";
HashMap<String, String> paramsMap = new HashMap<>();
paramsMap.put("fields", "title");
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withPath("/_field_caps")
.withParams(paramsMap)
.withContent(new BytesArray(content), XContentType.JSON)
.build();
try {
action.prepareRequest(request, mock(NodeClient.class));
fail("expected failure");
} catch (IllegalArgumentException e) {
assertEquals(
e.getMessage(),
"can't specify a request body and [fields]"
+ " request parameter, either specify a request body or the"
+ " [fields] request parameter"
);
}

}
}

0 comments on commit f28f454

Please sign in to comment.