Deprecate support for first line empty in msearch API

In order to support for 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 elastic#41011
javanna · Apr 23, 2019 · fcd06ee · fcd06ee
1 parent 31c609d
commit fcd06ee
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 3 deletions.
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,7 @@ public static void readMultiLineFormat(BytesReference data,
             // support first line with \n
             if (nextMarker == 0) {
                 from = nextMarker + 1;
+                deprecationLogger.deprecated("support for empty first line in msearch API is deprecated and will be removed");
                 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,26 @@ 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 in msearch API is deprecated and will be removed");
+    }
+
+    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 in msearch API is deprecated and will be removed");
+    }
+
     public void testResponseErrorToXContent() {
         long tookInMillis = randomIntBetween(1, 1000);
         MultiSearchResponse response = new MultiSearchResponse(
@@ -262,7 +289,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": {}}}