Repository: searchbox-io/Jest
Pull request #550 (2017-09-14 09:08:22)

Ensure valid Multi Search requests
The [Multi Search](https://www.elastic.co/guide/en/elasticsearch/reference/5.6/search-multi-search.html) API expects queries to consist of a single line but the `MultiSearch` class didn't check and ensure this until now.

Additionally, this PR sets the `Content-Type` HTTP request header to the recommended media type `application/x-ndjson` for Multi Search requests.


Patch:
diff --git a/jest-common/src/main/java/io/searchbox/core/MultiSearch.java b/jest-common/src/main/java/io/searchbox/core/MultiSearch.java
index 831fbd51..a3e14d5a 100644
--- a/jest-common/src/main/java/io/searchbox/core/MultiSearch.java
+++ b/jest-common/src/main/java/io/searchbox/core/MultiSearch.java
@@ -1,5 +1,6 @@
 package io.searchbox.core;
 
+import com.google.common.base.CharMatcher;
 import com.google.gson.Gson;
 import io.searchbox.action.AbstractAction;
 import io.searchbox.action.GenericResultAbstractAction;
@@ -15,6 +16,7 @@ import java.util.Objects;
  * @author cihat keser
  */
 public class MultiSearch extends AbstractAction<MultiSearchResult> {
+    private static final CharMatcher NEWLINE_MATCHER = CharMatcher.anyOf("\r\n").precomputed();
 
     private Collection<Search> searches;
 
@@ -54,23 +56,25 @@ public class MultiSearch extends AbstractAction<MultiSearchResult> {
             sb.append(getParameter(search, "ignore_unavailable"));
             sb.append(getParameter(search, "allow_no_indices"));
             sb.append(getParameter(search, "expand_wildcards"));
+            final String query = NEWLINE_MATCHER.removeFrom(search.getData(gson));
             sb.append("\"}\n")
-                    .append(search.getData(gson))
+                    .append(query)
                     .append("\n");
         }
         return sb.toString();
     }
 
     private String getParameter(Search search, String parameter) {
-        Collection<Object> searchParameter = search.getParameter(parameter);
-        if (searchParameter != null)
-            if (searchParameter != null) {
-                if (searchParameter.size() == 1) {
-                    return "\", \"" + parameter + "\" : \"" + searchParameter.iterator().next();
-                } else if (searchParameter.size() > 1) {
-                    throw new IllegalArgumentException("Expecting a single value for '" + parameter + "' parameter, you provided: " + searchParameter.size());
-                }
+        final Collection<Object> searchParameter = search.getParameter(parameter);
+        if (searchParameter != null) {
+            final int parameters = searchParameter.size();
+            if (parameters == 1) {
+                return "\", \"" + parameter + "\" : \"" + searchParameter.iterator().next();
+            } else if (parameters > 1) {
+                throw new IllegalArgumentException("Expecting a single value for '" + parameter + "' parameter, you provided: " + parameters);
             }
+        }
+
         return "";
     }
 
@@ -101,13 +105,15 @@ public class MultiSearch extends AbstractAction<MultiSearchResult> {
     }
 
     public static class Builder extends GenericResultAbstractAction.Builder<MultiSearch, Builder> {
-        private List<Search> searchList = new LinkedList<Search>();
+        private List<Search> searchList = new LinkedList<>();
 
         public Builder(Search search) {
+            setHeader("Content-Type", "application/x-ndjson");
             searchList.add(search);
         }
 
         public Builder(Collection<? extends Search> searches) {
+            setHeader("Content-Type", "application/x-ndjson");
             searchList.addAll(searches);
         }
 
diff --git a/jest-common/src/main/java/io/searchbox/core/MultiSearchResult.java b/jest-common/src/main/java/io/searchbox/core/MultiSearchResult.java
index af6d0aa9..1c972b7e 100644
--- a/jest-common/src/main/java/io/searchbox/core/MultiSearchResult.java
+++ b/jest-common/src/main/java/io/searchbox/core/MultiSearchResult.java
@@ -3,6 +3,7 @@ package io.searchbox.core;
 import com.google.gson.Gson;
 import com.google.gson.JsonArray;
 import com.google.gson.JsonElement;
+import com.google.gson.JsonNull;
 import com.google.gson.JsonObject;
 import io.searchbox.client.JestResult;
 
@@ -41,29 +42,33 @@ public class MultiSearchResult extends JestResult {
 
         public final boolean isError;
         public final String errorMessage;
+        public final JsonElement error;
         public final SearchResult searchResult;
 
         public MultiSearchResponse(JsonObject jsonObject) {
             final JsonElement error = jsonObject.get(ERROR_KEY);
             if(error != null) {
-                isError = true;
+                this.isError = true;
+                this.error = error;
                 if (error.isJsonPrimitive()) {
-                    errorMessage = error.getAsString();
+                    this.errorMessage = error.getAsString();
+                } else if (error.isJsonObject()){
+                    this.errorMessage = error.getAsJsonObject().get("reason").getAsString();
+                } else {
+                    this.errorMessage = error.toString();
                 }
-                else {
-                    errorMessage = error.toString();
-                }
-                searchResult = null;
+                this.searchResult = null;
             } else {
-                isError = false;
-                errorMessage = null;
+                this.isError = false;
+                this.errorMessage = null;
+                this.error = JsonNull.INSTANCE;
 
-                searchResult = new SearchResult(gson);
-                searchResult.setSucceeded(true);
-                searchResult.setResponseCode(responseCode);
-                searchResult.setJsonObject(jsonObject);
-                searchResult.setJsonString(jsonObject.toString());
-                searchResult.setPathToResult("hits/hits/_source");
+                this.searchResult = new SearchResult(gson);
+                this.searchResult.setSucceeded(true);
+                this.searchResult.setResponseCode(responseCode);
+                this.searchResult.setJsonObject(jsonObject);
+                this.searchResult.setJsonString(jsonObject.toString());
+                this.searchResult.setPathToResult("hits/hits/_source");
             }
         }
     }


Test patch:
diff --git a/jest-common/src/test/java/io/searchbox/core/MultiSearchTest.java b/jest-common/src/test/java/io/searchbox/core/MultiSearchTest.java
index e5db3da9..74035cd0 100644
--- a/jest-common/src/test/java/io/searchbox/core/MultiSearchTest.java
+++ b/jest-common/src/test/java/io/searchbox/core/MultiSearchTest.java
@@ -16,6 +16,16 @@ import static org.junit.Assert.*;
  */
 public class MultiSearchTest {
 
+    @Test
+    public void multiSearchHasCorrectContentType() throws JSONException {
+        Search search = new Search.Builder("").build();
+        MultiSearch multiSearch = new MultiSearch.Builder(search).build();
+
+        assertEquals("POST", multiSearch.getRestMethodName());
+        assertEquals("/_msearch", multiSearch.getURI());
+        assertEquals("application/x-ndjson", multiSearch.getHeader("Content-Type"));
+    }
+
     @Test
     public void singleMultiSearchWithoutIndex() throws JSONException {
         String expectedData = " {\"index\" : \"_all\"}\n" +
@@ -129,9 +139,12 @@ public class MultiSearchTest {
                 "          }\n" +
                 "      },\n" +
                 "      {\n" +
-                "         \"error\": \"There was a \\\"test\\\" error\"\n" +
+                "        \"error\" : {\n" +
+                "          \"type\" : \"search_phase_execution_exception\",\n" +
+                "          \"reason\" : \"There was a \\\"test\\\" error\"\n" +
+                "        }\n" +
                 "      }\n" +
-                "   ]\n" +
+                "  ]\n" +
                 "}";
 
         MultiSearchResult multiSearchResult = new MultiSearchResult(new Gson());
diff --git a/jest/src/test/java/io/searchbox/core/MultiSearchIntegrationTest.java b/jest/src/test/java/io/searchbox/core/MultiSearchIntegrationTest.java
index a68cce92..ba6d59b0 100644
--- a/jest/src/test/java/io/searchbox/core/MultiSearchIntegrationTest.java
+++ b/jest/src/test/java/io/searchbox/core/MultiSearchIntegrationTest.java
@@ -82,6 +82,7 @@ public class MultiSearchIntegrationTest extends AbstractIntegrationTest {
         assertFalse(complexSearchResponse.isError);
         assertNull(complexSearchResponse.errorMessage);
         SearchResult complexSearchResult = complexSearchResponse.searchResult;
+        assertNotNull(complexSearchResult);
         assertTrue(complexSearchResult.isSucceeded());
         assertNull(complexSearchResult.getErrorMessage());
         assertEquals(Long.valueOf(2L), complexSearchResult.getTotal());
@@ -92,6 +93,7 @@ public class MultiSearchIntegrationTest extends AbstractIntegrationTest {
         assertFalse(simpleSearchResponse.isError);
         assertNull(simpleSearchResponse.errorMessage);
         SearchResult simpleSearchResult = simpleSearchResponse.searchResult;
+        assertNotNull(simpleSearchResult);
         assertTrue(simpleSearchResult.isSucceeded());
         assertNull(simpleSearchResult.getErrorMessage());
         assertEquals(Long.valueOf(3L), simpleSearchResult.getTotal());