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
elastic · Apr 25, 2019 · 8a0e5f7 · 8a0e5f7
1 parent 906f880
commit 8a0e5f7
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 4 deletions.
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,9 @@
+
+{}
+{ "query": {"match_all": {}}}
+
+{ "query": {"match_all": {}}}
+{}
+{ "query": {"match_all": {}}}
+
+{ "query": {"match_all": {}}}