From f28f4545b24e7912a8623f5f3da9147f28e7d7ad Mon Sep 17 00:00:00 2001 From: likzn <1020193211@qq.com> Date: Fri, 5 Aug 2022 01:44:50 +0800 Subject: [PATCH] In the field capabilities API, re-add support for `fields` in the request body (#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 #86875 --- CHANGELOG.md | 2 +- docs/changelog/88972.yaml | 6 +++ .../test/field_caps/10_basic.yml | 16 +++++- .../action/RestFieldCapabilitiesAction.java | 20 +++++-- .../FieldCapabilitiesRequestTests.java | 17 ++++++ .../RestFieldCapabilitiesActionTests.java | 54 +++++++++++++++++++ 6 files changed, 109 insertions(+), 6 deletions(-) create mode 100644 docs/changelog/88972.yaml create mode 100644 server/src/test/java/org/elasticsearch/rest/action/RestFieldCapabilitiesActionTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a965ee4b6eb..769855531e35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/docs/changelog/88972.yaml b/docs/changelog/88972.yaml new file mode 100644 index 000000000000..9f50a933ec45 --- /dev/null +++ b/docs/changelog/88972.yaml @@ -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 diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/10_basic.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/10_basic.yml index e1817c9c607e..ad641f83b47a 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/10_basic.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/10_basic.yml @@ -121,7 +121,6 @@ setup: --- "Get simple field caps": - - do: field_caps: index: 'test1,test2,test3' @@ -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": diff --git a/server/src/main/java/org/elasticsearch/rest/action/RestFieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/rest/action/RestFieldCapabilitiesAction.java index 879faff87e0b..2ddc1a106dbc 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/RestFieldCapabilitiesAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/RestFieldCapabilitiesAction.java @@ -23,6 +23,7 @@ import java.util.List; 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 { @@ -43,10 +44,9 @@ public class RestFieldCapabilitiesAction extends BaseRestHandler { @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)); @@ -57,16 +57,28 @@ public class RestFieldCapabilitiesAction extends BaseRestHandler { 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 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); } } diff --git a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequestTests.java b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequestTests.java index 2f94d8cd8e3f..3af2639538f0 100644 --- a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequestTests.java @@ -19,10 +19,14 @@ import org.elasticsearch.common.util.ArrayUtils; 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; @@ -31,6 +35,7 @@ import java.util.List; 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; @@ -146,6 +151,18 @@ public class FieldCapabilitiesRequestTests extends AbstractWireSerializingTestCa }""").replaceAll("\\s+", ""), xContent); } + public void testFromXContent() throws IOException { + XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"fields\" : [\"FOO\"] }"); + FieldCapabilitiesRequest request = new FieldCapabilitiesRequest(); + ObjectParser 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(); diff --git a/server/src/test/java/org/elasticsearch/rest/action/RestFieldCapabilitiesActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/RestFieldCapabilitiesActionTests.java new file mode 100644 index 000000000000..0b98a68c1dbc --- /dev/null +++ b/server/src/test/java/org/elasticsearch/rest/action/RestFieldCapabilitiesActionTests.java @@ -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 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" + ); + } + + } +}