SQL: move requests' parameters to requests JSON body

x-pack/plugin/sql/qa/multi-node/src/test/java/org/elasticsearch/xpack/sql/qa/multi_node/RestSqlMultinodeIT.java

@@ -26,6 +26,7 @@
 
 import static java.util.Collections.singletonList;
 import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
+import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
 import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
 
 /**
@@ -111,10 +112,7 @@ private void assertCount(RestClient client, int count) throws IOException {
         expected.put("rows", singletonList(singletonList(count)));
 
         Request request = new Request("POST", "/_sql");
-        if (false == mode.isEmpty()) {
-            request.addParameter("mode", mode);
-        }
-        request.setJsonEntity("{\"query\": \"SELECT COUNT(*) FROM test\"}");
+        request.setJsonEntity("{\"query\": \"SELECT COUNT(*) FROM test\"" + mode(mode) + "}");
         Map<String, Object> actual = responseToMap(client.performRequest(request));
 
         if (false == expected.equals(actual)) {

x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/RestSqlSecurityIT.java

@@ -30,6 +30,7 @@
 import java.util.stream.Collectors;
 
 import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
+import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
 import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
@@ -66,21 +67,23 @@ public void expectMatchesAdmin(String adminSql, String user, String userSql) thr
         @Override
         public void expectScrollMatchesAdmin(String adminSql, String user, String userSql) throws Exception {
             String mode = randomMode();
-            Map<String, Object> adminResponse = runSql(null, mode,
-                    new StringEntity("{\"query\": \"" + adminSql + "\", \"fetch_size\": 1}", ContentType.APPLICATION_JSON));
-            Map<String, Object> otherResponse = runSql(user, mode,
-                    new StringEntity("{\"query\": \"" + adminSql + "\", \"fetch_size\": 1}", ContentType.APPLICATION_JSON));
+            Map<String, Object> adminResponse = runSql(null,
+                    new StringEntity("{\"query\": \"" + adminSql + "\", \"fetch_size\": 1" + mode(mode) + "}",
+                            ContentType.APPLICATION_JSON));
+            Map<String, Object> otherResponse = runSql(user,
+                    new StringEntity("{\"query\": \"" + adminSql + "\", \"fetch_size\": 1" + mode(mode) + "}",
+                            ContentType.APPLICATION_JSON));
 
             String adminCursor = (String) adminResponse.remove("cursor");
             String otherCursor = (String) otherResponse.remove("cursor");
             assertNotNull(adminCursor);
             assertNotNull(otherCursor);
             assertResponse(adminResponse, otherResponse);
             while (true) {
-                adminResponse = runSql(null, mode,
-                        new StringEntity("{\"cursor\": \"" + adminCursor + "\"}", ContentType.APPLICATION_JSON));
-                otherResponse = runSql(user, mode,
-                        new StringEntity("{\"cursor\": \"" + otherCursor + "\"}", ContentType.APPLICATION_JSON));
+                adminResponse = runSql(null,
+                        new StringEntity("{\"cursor\": \"" + adminCursor + "\"" + mode(mode) + "}", ContentType.APPLICATION_JSON));
+                otherResponse = runSql(user,
+                        new StringEntity("{\"cursor\": \"" + otherCursor + "\"" + mode(mode) + "}", ContentType.APPLICATION_JSON));
                 adminCursor = (String) adminResponse.remove("cursor");
                 otherCursor = (String) otherResponse.remove("cursor");
                 assertResponse(adminResponse, otherResponse);
@@ -173,14 +176,11 @@ public void checkNoMonitorMain(String user) throws Exception {
         }
 
         private static Map<String, Object> runSql(@Nullable String asUser, String mode, String sql) throws IOException {
-            return runSql(asUser, mode, new StringEntity("{\"query\": \"" + sql + "\"}", ContentType.APPLICATION_JSON));
+            return runSql(asUser, new StringEntity("{\"query\": \"" + sql + "\"" + mode(mode) + "}", ContentType.APPLICATION_JSON));
         }
 
-        private static Map<String, Object> runSql(@Nullable String asUser, String mode, HttpEntity entity) throws IOException {
+        private static Map<String, Object> runSql(@Nullable String asUser, HttpEntity entity) throws IOException {
             Request request = new Request("POST", "/_sql");
-            if (false == mode.isEmpty()) {
-                request.addParameter("mode", mode);
-            }
             if (asUser != null) {
                 RequestOptions.Builder options = request.getOptions().toBuilder();
                 options.addHeader("es-security-runas-user", asUser);
@@ -223,14 +223,15 @@ protected AuditLogAsserter createAuditLogAsserter() {
     public void testHijackScrollFails() throws Exception {
         createUser("full_access", "rest_minimal");
 
-        Map<String, Object> adminResponse = RestActions.runSql(null, randomMode(),
-                new StringEntity("{\"query\": \"SELECT * FROM test\", \"fetch_size\": 1}", ContentType.APPLICATION_JSON));
+        Map<String, Object> adminResponse = RestActions.runSql(null,
+                new StringEntity("{\"query\": \"SELECT * FROM test\", \"fetch_size\": 1" + mode(randomMode()) + "}",
+                        ContentType.APPLICATION_JSON));
 
         String cursor = (String) adminResponse.remove("cursor");
         assertNotNull(cursor);
 
-        ResponseException e = expectThrows(ResponseException.class, () -> RestActions.runSql("full_access", randomMode(),
-                new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON)));
+        ResponseException e = expectThrows(ResponseException.class, () -> RestActions.runSql("full_access",
+                new StringEntity("{\"cursor\":\"" + cursor + "\"" + mode(randomMode()) + "}", ContentType.APPLICATION_JSON)));
         // TODO return a better error message for bad scrolls
         assertThat(e.getMessage(), containsString("No search context found for id"));
         assertEquals(404, e.getResponse().getStatusLine().getStatusCode());

x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/UserFunctionIT.java

@@ -35,6 +35,8 @@
 import java.util.Map;
 
 import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
+import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
+import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
 
 public class UserFunctionIT extends ESRestTestCase {
 
@@ -172,14 +174,11 @@ private void deleteUser(String name) throws IOException {
     }
 
     private Map<String, Object> runSql(String asUser, String mode, String sql) throws IOException {
-        return runSql(asUser, mode, new StringEntity("{\"query\": \"" + sql + "\"}", ContentType.APPLICATION_JSON));
+        return runSql(asUser, new StringEntity("{\"query\": \"" + sql + "\"" + mode(mode) + "}", ContentType.APPLICATION_JSON));
     }
 
-    private Map<String, Object> runSql(String asUser, String mode, HttpEntity entity) throws IOException {
+    private Map<String, Object> runSql(String asUser, HttpEntity entity) throws IOException {
         Request request = new Request("POST", "/_sql");
-        if (false == mode.isEmpty()) {
-            request.addParameter("mode", mode);
-        }
         if (asUser != null) {
             RequestOptions.Builder options = request.getOptions().toBuilder();
             options.addHeader("es-security-runas-user", asUser);
@@ -203,10 +202,6 @@ private static Map<String, Object> toMap(Response response) throws IOException {
         }
     }
 
-    private String randomMode() {
-        return randomFrom("plain", "jdbc", "");
-    }
-
     private void index(String... docs) throws IOException {
         Request request = new Request("POST", "/test/test/_bulk");
         request.addParameter("refresh", "true");

x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java

@@ -15,12 +15,14 @@
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.ResponseException;
 import org.elasticsearch.common.CheckedSupplier;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.io.Streams;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.test.NotEqualMessageBuilder;
 import org.elasticsearch.test.rest.ESRestTestCase;
+import org.elasticsearch.xpack.sql.proto.StringUtils;
 import org.elasticsearch.xpack.sql.qa.ErrorsTestCase;
 import org.hamcrest.Matcher;
 
@@ -97,10 +99,10 @@ public void testNextPage() throws IOException {
         for (int i = 0; i < 20; i += 2) {
             Map<String, Object> response;
             if (i == 0) {
-                response = runSql(mode, new StringEntity(sqlRequest, ContentType.APPLICATION_JSON));
+                response = runSql(new StringEntity(sqlRequest, ContentType.APPLICATION_JSON), "");
             } else {
-                response = runSql(mode, new StringEntity("{\"cursor\":\"" + cursor + "\"}",
-                        ContentType.APPLICATION_JSON));
+                response = runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"" + mode(mode) + "}",
+                        ContentType.APPLICATION_JSON), StringUtils.EMPTY);
             }
 
             Map<String, Object> expected = new HashMap<>();
@@ -120,8 +122,8 @@ public void testNextPage() throws IOException {
         }
         Map<String, Object> expected = new HashMap<>();
         expected.put("rows", emptyList());
-        assertResponse(expected, runSql(mode, new StringEntity("{ \"cursor\":\"" + cursor + "\"}",
-                ContentType.APPLICATION_JSON)));
+        assertResponse(expected, runSql(new StringEntity("{ \"cursor\":\"" + cursor + "\"" + mode(mode) + "}",
+                ContentType.APPLICATION_JSON), StringUtils.EMPTY));
     }
 
     @AwaitsFix(bugUrl = "Unclear status, https://github.com/elastic/x-pack-elasticsearch/issues/2074")
@@ -136,8 +138,7 @@ public void testTimeZone() throws IOException {
         expected.put("size", 2);
 
         // Default TimeZone is UTC
-        assertResponse(expected, runSql(mode, new StringEntity("{\"query\":\"SELECT DAY_OF_YEAR(test), COUNT(*) FROM test\"}",
-                        ContentType.APPLICATION_JSON)));
+        assertResponse(expected, runSql(mode, "SELECT DAY_OF_YEAR(test), COUNT(*) FROM test"));
     }
 
     public void testScoreWithFieldNamedScore() throws IOException {
@@ -302,35 +303,29 @@ private void expectBadRequest(CheckedSupplier<Map<String, Object>, Exception> co
     }
 
     private Map<String, Object> runSql(String mode, String sql) throws IOException {
-        return runSql(mode, sql, "");
+        return runSql(mode, sql, StringUtils.EMPTY);
     }
 
     private Map<String, Object> runSql(String mode, String sql, String suffix) throws IOException {
-        return runSql(mode, new StringEntity("{\"query\":\"" + sql + "\"}", ContentType.APPLICATION_JSON), suffix);
+        return runSql(new StringEntity("{\"query\":\"" + sql + "\"" + mode(mode) + "}", ContentType.APPLICATION_JSON), suffix);
     }
 
-    private Map<String, Object> runSql(String mode, HttpEntity sql) throws IOException {
-        return runSql(mode, sql, "");
-    }
-
-    private Map<String, Object> runSql(String mode, HttpEntity sql, String suffix) throws IOException {
+    private Map<String, Object> runSql(HttpEntity sql, String suffix) throws IOException {
         Request request = new Request("POST", "/_sql" + suffix);
         request.addParameter("error_trace", "true");   // Helps with debugging in case something crazy happens on the server.
         request.addParameter("pretty", "true");        // Improves error reporting readability
         if (randomBoolean()) {
             // We default to JSON but we force it randomly for extra coverage
             request.addParameter("format", "json");
         }
-        if (false == mode.isEmpty()) {
-            request.addParameter("mode", mode);        // JDBC or PLAIN mode
-        }
         if (randomBoolean()) {
             // JSON is the default but randomly set it sometime for extra coverage
             RequestOptions.Builder options = request.getOptions().toBuilder();
             options.addHeader("Accept", randomFrom("*/*", "application/json"));
             request.setOptions(options);
         }
         request.setEntity(sql);
+
         Response response = client().performRequest(request);
         try (InputStream content = response.getEntity().getContent()) {
             return XContentHelper.convertToMap(JsonXContent.jsonXContent, content, false);
@@ -357,9 +352,9 @@ public void testBasicQueryWithFilter() throws IOException {
         Map<String, Object> expected = new HashMap<>();
         expected.put("columns", singletonList(columnInfo(mode, "test", "text", JDBCType.VARCHAR, 0)));
         expected.put("rows", singletonList(singletonList("foo")));
-        assertResponse(expected, runSql(mode, new StringEntity("{\"query\":\"SELECT * FROM test\", " +
-                                "\"filter\":{\"match\": {\"test\": \"foo\"}}}",
-                ContentType.APPLICATION_JSON)));
+        assertResponse(expected, runSql(new StringEntity("{\"query\":\"SELECT * FROM test\", " +
+                "\"filter\":{\"match\": {\"test\": \"foo\"}}" + mode(mode) + "}",
+                ContentType.APPLICATION_JSON), StringUtils.EMPTY));
     }
 
     public void testBasicQueryWithParameters() throws IOException {
@@ -373,16 +368,16 @@ public void testBasicQueryWithParameters() throws IOException {
                 columnInfo(mode, "param", "integer", JDBCType.INTEGER, 11)
         ));
         expected.put("rows", singletonList(Arrays.asList("foo", 10)));
-        assertResponse(expected, runSql(mode, new StringEntity("{\"query\":\"SELECT test, ? param FROM test WHERE test = ?\", " +
-                "\"params\":[{\"type\": \"integer\", \"value\": 10}, {\"type\": \"keyword\", \"value\": \"foo\"}]}",
-                ContentType.APPLICATION_JSON)));
+        assertResponse(expected, runSql(new StringEntity("{\"query\":\"SELECT test, ? param FROM test WHERE test = ?\", " +
+                "\"params\":[{\"type\": \"integer\", \"value\": 10}, {\"type\": \"keyword\", \"value\": \"foo\"}]"
+                + mode(mode) + "}", ContentType.APPLICATION_JSON), StringUtils.EMPTY));
     }
 
     public void testBasicTranslateQueryWithFilter() throws IOException {
         index("{\"test\":\"foo\"}",
             "{\"test\":\"bar\"}");
 
-        Map<String, Object> response = runSql("",
+        Map<String, Object> response = runSql(
                 new StringEntity("{\"query\":\"SELECT * FROM test\", \"filter\":{\"match\": {\"test\": \"foo\"}}}",
                 ContentType.APPLICATION_JSON), "/translate/"
         );
@@ -424,7 +419,7 @@ public void testTranslateQueryWithGroupByAndHaving() throws IOException {
         index("{\"salary\":100}",
             "{\"age\":20}");
 
-        Map<String, Object> response = runSql("",
+        Map<String, Object> response = runSql(
             new StringEntity("{\"query\":\"SELECT avg(salary) FROM test GROUP BY abs(age) HAVING avg(salary) > 50 LIMIT 10\"}",
                 ContentType.APPLICATION_JSON), "/translate/"
         );
@@ -551,10 +546,10 @@ public void testNextPageText() throws IOException {
         for (int i = 0; i < 20; i += 2) {
             Tuple<String, String> response;
             if (i == 0) {
-                response = runSqlAsText("", new StringEntity(request, ContentType.APPLICATION_JSON), "text/plain");
+                response = runSqlAsText(StringUtils.EMPTY, new StringEntity(request, ContentType.APPLICATION_JSON), "text/plain");
             } else {
-                response = runSqlAsText("", new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON),
-                        "text/plain");
+                response = runSqlAsText(StringUtils.EMPTY, new StringEntity("{\"cursor\":\"" + cursor + "\"}",
+                        ContentType.APPLICATION_JSON), "text/plain");
             }
 
             StringBuilder expected = new StringBuilder();
@@ -570,9 +565,10 @@ public void testNextPageText() throws IOException {
         }
         Map<String, Object> expected = new HashMap<>();
         expected.put("rows", emptyList());
-        assertResponse(expected, runSql("", new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON)));
+        assertResponse(expected, runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON),
+                StringUtils.EMPTY));
 
-        Map<String, Object> response = runSql("", new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON),
+        Map<String, Object> response = runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON),
                 "/close");
         assertEquals(true, response.get("succeeded"));
 
@@ -638,7 +634,7 @@ public void testQueryInTSV() throws IOException {
     }
 
     private Tuple<String, String> runSqlAsText(String sql, String accept) throws IOException {
-        return runSqlAsText("", new StringEntity("{\"query\":\"" + sql + "\"}", ContentType.APPLICATION_JSON), accept);
+        return runSqlAsText(StringUtils.EMPTY, new StringEntity("{\"query\":\"" + sql + "\"}", ContentType.APPLICATION_JSON), accept);
     }
 
     /**
@@ -716,7 +712,11 @@ private static Map<String, Object> searchStats() throws IOException {
     }
 
     public static String randomMode() {
-        return randomFrom("", "jdbc", "plain");
+        return randomFrom(StringUtils.EMPTY, "jdbc", "plain");
+    }
+
+    public static String mode(String mode) {
+        return Strings.isEmpty(mode) ? StringUtils.EMPTY : ",\"mode\":\"" + mode + "\"";
     }
 
     private void index(String... docs) throws IOException {