From 8a0e5f7b87a4930f0077478350a0ab9b2c56a4f1 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Thu, 25 Apr 2019 12:45:18 +0200 Subject: [PATCH] Deprecate support for first line empty in msearch API (#41442) In order to support empty action metadata in the first msearch item, we need to remove support for prepending msearch request body with an empty line, which prevents us from parsing the empty line as action metadata for the first search item. Relates to #41011 --- .../client/RequestConvertersTests.java | 8 ++++- .../action/search/MultiSearchRequest.java | 6 +++- .../action/search/RestMultiSearchAction.java | 2 +- .../search/MultiSearchRequestTests.java | 31 ++++++++++++++++++- .../search/msearch-empty-first-line1.json | 9 ++++++ .../search/msearch-empty-first-line2.json | 9 ++++++ 6 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 server/src/test/resources/org/elasticsearch/action/search/msearch-empty-first-line1.json create mode 100644 server/src/test/resources/org/elasticsearch/action/search/msearch-empty-first-line2.json diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java index 6be59b687d31d..774a404827e5f 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java @@ -27,6 +27,7 @@ import org.apache.http.client.methods.HttpPut; import org.apache.http.nio.entity.NByteArrayEntity; import org.apache.http.util.EntityUtils; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.admin.cluster.storedscripts.DeleteStoredScriptRequest; import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptRequest; @@ -62,6 +63,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.io.Streams; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.lucene.uid.Versions; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ToXContent; @@ -132,6 +134,10 @@ import static org.hamcrest.Matchers.nullValue; public class RequestConvertersTests extends ESTestCase { + + private static final DeprecationLogger deprecationLogger = new DeprecationLogger( + LogManager.getLogger(RequestConvertersTests.class)); + public void testPing() { Request request = RequestConverters.ping(); assertEquals("/", request.getEndpoint()); @@ -1256,7 +1262,7 @@ public void testMultiSearch() throws IOException { }; MultiSearchRequest.readMultiLineFormat(new BytesArray(EntityUtils.toByteArray(request.getEntity())), REQUEST_BODY_CONTENT_TYPE.xContent(), consumer, null, multiSearchRequest.indicesOptions(), null, null, null, null, - xContentRegistry(), true); + xContentRegistry(), true, deprecationLogger); assertEquals(requests, multiSearchRequest.requests()); } diff --git a/server/src/main/java/org/elasticsearch/action/search/MultiSearchRequest.java b/server/src/main/java/org/elasticsearch/action/search/MultiSearchRequest.java index 195e146ab7ca1..071f5108bdded 100644 --- a/server/src/main/java/org/elasticsearch/action/search/MultiSearchRequest.java +++ b/server/src/main/java/org/elasticsearch/action/search/MultiSearchRequest.java @@ -27,6 +27,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ToXContent; @@ -175,7 +176,8 @@ public static void readMultiLineFormat(BytesReference data, String searchType, Boolean ccsMinimizeRoundtrips, NamedXContentRegistry registry, - boolean allowExplicitIndex) throws IOException { + boolean allowExplicitIndex, + DeprecationLogger deprecationLogger) throws IOException { int from = 0; byte marker = xContent.streamSeparator(); while (true) { @@ -186,6 +188,8 @@ public static void readMultiLineFormat(BytesReference data, // support first line with \n if (nextMarker == 0) { from = nextMarker + 1; + deprecationLogger.deprecated("support for empty first line before any action metadata in msearch API is deprecated and " + + "will be removed in the next major version"); continue; } diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java b/server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java index 05a20a0cc06b9..5aec63ccb9ff7 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java @@ -154,7 +154,7 @@ public static void parseMultiLineRequest(RestRequest request, IndicesOptions ind final XContent xContent = sourceTuple.v1().xContent(); final BytesReference data = sourceTuple.v2(); MultiSearchRequest.readMultiLineFormat(data, xContent, consumer, indices, indicesOptions, types, routing, - searchType, ccsMinimizeRoundtrips, request.getXContentRegistry(), allowExplicitIndex); + searchType, ccsMinimizeRoundtrips, request.getXContentRegistry(), allowExplicitIndex, deprecationLogger); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java b/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java index da22ce4c96c1a..60d94c269a960 100644 --- a/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java @@ -19,12 +19,14 @@ package org.elasticsearch.action.search; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.common.CheckedBiConsumer; import org.elasticsearch.common.CheckedRunnable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; @@ -48,10 +50,15 @@ import static org.elasticsearch.search.RandomSearchRequestGenerator.randomSearchRequest; import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; public class MultiSearchRequestTests extends ESTestCase { + + private static final DeprecationLogger deprecationLogger = new DeprecationLogger( + LogManager.getLogger(MultiSearchRequestTests.class)); + public void testSimpleAdd() throws Exception { MultiSearchRequest request = parseMultiSearchRequest("/org/elasticsearch/action/search/simple-msearch1.json"); assertThat(request.requests().size(), @@ -180,6 +187,28 @@ public void testSimpleAdd4() throws Exception { assertThat(request.requests().get(2).routing(), equalTo("123")); } + public void testEmptyFirstLine1() throws Exception { + MultiSearchRequest request = parseMultiSearchRequest("/org/elasticsearch/action/search/msearch-empty-first-line1.json"); + assertThat(request.requests().size(), equalTo(4)); + for (SearchRequest searchRequest : request.requests()) { + assertThat(searchRequest.indices().length, equalTo(0)); + assertThat(searchRequest.source().query(), instanceOf(MatchAllQueryBuilder.class)); + } + assertWarnings("support for empty first line before any action metadata in msearch API is deprecated and will be removed " + + "in the next major version"); + } + + public void testEmptyFirstLine2() throws Exception { + MultiSearchRequest request = parseMultiSearchRequest("/org/elasticsearch/action/search/msearch-empty-first-line2.json"); + assertThat(request.requests().size(), equalTo(4)); + for (SearchRequest searchRequest : request.requests()) { + assertThat(searchRequest.indices().length, equalTo(0)); + assertThat(searchRequest.source().query(), instanceOf(MatchAllQueryBuilder.class)); + } + assertWarnings("support for empty first line before any action metadata in msearch API is deprecated and will be removed " + + "in the next major version"); + } + public void testResponseErrorToXContent() { long tookInMillis = randomIntBetween(1, 1000); MultiSearchResponse response = new MultiSearchResponse( @@ -262,7 +291,7 @@ public void testMultiLineSerialization() throws IOException { parsedRequest.add(r); }; MultiSearchRequest.readMultiLineFormat(new BytesArray(originalBytes), xContentType.xContent(), - consumer, null, null, null, null, null, null, xContentRegistry(), true); + consumer, null, null, null, null, null, null, xContentRegistry(), true, deprecationLogger); assertEquals(originalRequest, parsedRequest); } } diff --git a/server/src/test/resources/org/elasticsearch/action/search/msearch-empty-first-line1.json b/server/src/test/resources/org/elasticsearch/action/search/msearch-empty-first-line1.json new file mode 100644 index 0000000000000..b417d3adc9b53 --- /dev/null +++ b/server/src/test/resources/org/elasticsearch/action/search/msearch-empty-first-line1.json @@ -0,0 +1,9 @@ + + +{ "query": {"match_all": {}}} +{} +{ "query": {"match_all": {}}} + +{ "query": {"match_all": {}}} +{} +{ "query": {"match_all": {}}} diff --git a/server/src/test/resources/org/elasticsearch/action/search/msearch-empty-first-line2.json b/server/src/test/resources/org/elasticsearch/action/search/msearch-empty-first-line2.json new file mode 100644 index 0000000000000..c1c29b4170111 --- /dev/null +++ b/server/src/test/resources/org/elasticsearch/action/search/msearch-empty-first-line2.json @@ -0,0 +1,9 @@ + +{} +{ "query": {"match_all": {}}} + +{ "query": {"match_all": {}}} +{} +{ "query": {"match_all": {}}} + +{ "query": {"match_all": {}}}