From db44ce6f409d80ddf3148c55db5484b41544153b Mon Sep 17 00:00:00 2001 From: pgomulka Date: Tue, 8 Jun 2021 15:34:06 +0200 Subject: [PATCH 1/2] explain action --- rest-api-spec/build.gradle | 12 +--- .../action/explain/ExplainResponse.java | 5 ++ .../rest/action/search/RestExplainAction.java | 15 ++++- .../action/search/RestExplainActionTests.java | 55 +++++++++++++++++++ 4 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/rest/action/search/RestExplainActionTests.java diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index 69ef1ed2178e5..d30e803d6da92 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -121,7 +121,9 @@ tasks.named("yamlRestCompatTest").configure { 'search/160_exists_query/Test exists query on _type field', //type information is not stored, hence the the index will be found 'termvectors/50_mix_typeless_typeful/Term vectors with typeless API on an index that has types', - // 85 - 13 = 72 tests won't be fixed + // asserting about type not found won't work as we ignore the type information + 'explain/40_mix_typeless_typeful/Explain with typeless API on an index that has types', + // 87 - 14 = 73 tests won't be fixed 'bulk/11_basic_with_types/Array of objects', 'bulk/11_basic_with_types/Empty _id', 'bulk/11_basic_with_types/Empty _id with op_type create', @@ -138,14 +140,6 @@ tasks.named("yamlRestCompatTest").configure { 'count/11_basic_with_types/count body without query element', 'count/11_basic_with_types/count with body', 'count/11_basic_with_types/count with empty body', - 'explain/10_basic/Basic explain', - 'explain/10_basic/Basic explain with alias', - 'explain/11_basic_with_types/Basic explain', - 'explain/11_basic_with_types/Basic explain with alias', - 'explain/20_source_filtering/Source filtering', - 'explain/21_source_filtering_with_types/Source filtering', - 'explain/31_query_string_with_types/explain with query_string parameters', - 'explain/40_mix_typeless_typeful/Explain with typeless API on an index that has types', 'field_caps/30_filter/Field caps with index filter', 'get_source/11_basic_with_types/Basic with types', 'get_source/16_default_values_with_types/Default values', diff --git a/server/src/main/java/org/elasticsearch/action/explain/ExplainResponse.java b/server/src/main/java/org/elasticsearch/action/explain/ExplainResponse.java index d70240c929e86..f64c096fccef5 100644 --- a/server/src/main/java/org/elasticsearch/action/explain/ExplainResponse.java +++ b/server/src/main/java/org/elasticsearch/action/explain/ExplainResponse.java @@ -12,6 +12,7 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.RestApiVersion; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; @@ -172,6 +173,10 @@ public static ExplainResponse fromXContent(XContentParser parser, boolean exists public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); builder.field(_INDEX.getPreferredName(), index); + if (builder.getRestApiVersion() == RestApiVersion.V_7) { + builder.field(MapperService.TYPE_FIELD_NAME, MapperService.SINGLE_MAPPING_NAME); + } + builder.field(_ID.getPreferredName(), id); builder.field(MATCHED.getPreferredName(), isMatch()); if (hasExplanation()) { diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/RestExplainAction.java b/server/src/main/java/org/elasticsearch/rest/action/search/RestExplainAction.java index 2cd3f8889014d..eb2ca663bf3f6 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/search/RestExplainAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/search/RestExplainAction.java @@ -10,7 +10,9 @@ import org.elasticsearch.action.explain.ExplainRequest; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.RestApiVersion; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; @@ -28,12 +30,20 @@ * Rest action for computing a score explanation for specific documents. */ public class RestExplainAction extends BaseRestHandler { + public static final String TYPES_DEPRECATION_MESSAGE = "[types removal] " + + "Specifying a type in explain requests is deprecated."; @Override public List routes() { return List.of( new Route(GET, "/{index}/_explain/{id}"), - new Route(POST, "/{index}/_explain/{id}")); + new Route(POST, "/{index}/_explain/{id}"), + Route.builder(GET, "/{index}/{type}/{id}/_explain") + .deprecated(TYPES_DEPRECATION_MESSAGE, RestApiVersion.V_7) + .build(), + Route.builder(POST, "/{index}/{type}/{id}/_explain") + .deprecated(TYPES_DEPRECATION_MESSAGE, RestApiVersion.V_7) + .build()); } @Override @@ -43,6 +53,9 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + if(request.getRestApiVersion() == RestApiVersion.V_7 && request.hasParam("type")) { + request.param("type"); + } ExplainRequest explainRequest = new ExplainRequest(request.param("index"), request.param("id")); explainRequest.parent(request.param("parent")); diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/RestExplainActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/RestExplainActionTests.java new file mode 100644 index 0000000000000..dba75b0792d8c --- /dev/null +++ b/server/src/test/java/org/elasticsearch/rest/action/search/RestExplainActionTests.java @@ -0,0 +1,55 @@ +/* + * 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.search; + +import junit.framework.TestCase; +import org.elasticsearch.action.explain.ExplainResponse; +import org.elasticsearch.action.search.MultiSearchResponse; +import org.elasticsearch.common.RestApiVersion; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.test.rest.FakeRestRequest; +import org.elasticsearch.test.rest.RestActionTestCase; +import org.junit.Before; +import org.mockito.Mockito; + +import java.util.Collections; +import java.util.List; +import java.util.Map; + +public class RestExplainActionTests extends RestActionTestCase { + final List contentTypeHeader = Collections.singletonList(compatibleMediaType(XContentType.VND_JSON, RestApiVersion.V_7)); + + @Before + public void setUpAction() { + RestExplainAction action = new RestExplainAction(); + controller().registerHandler(action); + verifyingClient.setExecuteVerifier((actionType, request) -> Mockito.mock(ExplainResponse.class)); + verifyingClient.setExecuteLocallyVerifier((actionType, request) -> Mockito.mock(ExplainResponse.class)); + } + + public void testTypeInPath() { + RestRequest deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry()) + .withHeaders(Map.of("Accept", contentTypeHeader)) + .withMethod(RestRequest.Method.GET) + .withPath("/some_index/some_type/some_id/_explain") + .build(); + dispatchRequest(deprecatedRequest); + assertWarnings(RestExplainAction.TYPES_DEPRECATION_MESSAGE); + + RestRequest validRequest = new FakeRestRequest.Builder(xContentRegistry()) + .withHeaders(Map.of("Accept", contentTypeHeader)) + .withMethod(RestRequest.Method.GET) + .withPath("/some_index/_explain/some_id") + .build(); + dispatchRequest(validRequest); + } + +} From df407bd9f76194b60ff988ed0613cc0e178420fc Mon Sep 17 00:00:00 2001 From: pgomulka Date: Tue, 8 Jun 2021 17:09:23 +0200 Subject: [PATCH 2/2] imports --- .../elasticsearch/rest/action/search/RestExplainAction.java | 1 - .../rest/action/search/RestExplainActionTests.java | 3 --- 2 files changed, 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/RestExplainAction.java b/server/src/main/java/org/elasticsearch/rest/action/search/RestExplainAction.java index eb2ca663bf3f6..ecf35a9a939e5 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/search/RestExplainAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/search/RestExplainAction.java @@ -12,7 +12,6 @@ import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.RestApiVersion; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/RestExplainActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/RestExplainActionTests.java index dba75b0792d8c..31c8b7e608fde 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/RestExplainActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/RestExplainActionTests.java @@ -8,11 +8,8 @@ package org.elasticsearch.rest.action.search; -import junit.framework.TestCase; import org.elasticsearch.action.explain.ExplainResponse; -import org.elasticsearch.action.search.MultiSearchResponse; import org.elasticsearch.common.RestApiVersion; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.rest.FakeRestRequest;