Skip to content

Commit ea19d8f

Browse files
astefanastefan
committed
SQL: move requests' parameters to requests JSON body (#36149)
(cherry picked from commit eead8a1)
1 parent d6e388e commit ea19d8f

File tree

30 files changed

+348
-178
lines changed

30 files changed

+348
-178
lines changed

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import static java.util.Collections.singletonList;
2828
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
29+
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
2930
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
3031

3132
/**
@@ -111,10 +112,7 @@ private void assertCount(RestClient client, int count) throws IOException {
111112
expected.put("rows", singletonList(singletonList(count)));
112113

113114
Request request = new Request("POST", "/_xpack/sql");
114-
if (false == mode.isEmpty()) {
115-
request.addParameter("mode", mode);
116-
}
117-
request.setJsonEntity("{\"query\": \"SELECT COUNT(*) FROM test\"}");
115+
request.setJsonEntity("{\"query\": \"SELECT COUNT(*) FROM test\"" + mode(mode) + "}");
118116
Map<String, Object> actual = responseToMap(client.performRequest(request));
119117

120118
if (false == expected.equals(actual)) {

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

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.util.stream.Collectors;
3131

3232
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
33+
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
3334
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
3435
import static org.hamcrest.Matchers.containsString;
3536
import static org.hamcrest.Matchers.empty;
@@ -66,21 +67,23 @@ public void expectMatchesAdmin(String adminSql, String user, String userSql) thr
6667
@Override
6768
public void expectScrollMatchesAdmin(String adminSql, String user, String userSql) throws Exception {
6869
String mode = randomMode();
69-
Map<String, Object> adminResponse = runSql(null, mode,
70-
new StringEntity("{\"query\": \"" + adminSql + "\", \"fetch_size\": 1}", ContentType.APPLICATION_JSON));
71-
Map<String, Object> otherResponse = runSql(user, mode,
72-
new StringEntity("{\"query\": \"" + adminSql + "\", \"fetch_size\": 1}", ContentType.APPLICATION_JSON));
70+
Map<String, Object> adminResponse = runSql(null,
71+
new StringEntity("{\"query\": \"" + adminSql + "\", \"fetch_size\": 1" + mode(mode) + "}",
72+
ContentType.APPLICATION_JSON));
73+
Map<String, Object> otherResponse = runSql(user,
74+
new StringEntity("{\"query\": \"" + adminSql + "\", \"fetch_size\": 1" + mode(mode) + "}",
75+
ContentType.APPLICATION_JSON));
7376

7477
String adminCursor = (String) adminResponse.remove("cursor");
7578
String otherCursor = (String) otherResponse.remove("cursor");
7679
assertNotNull(adminCursor);
7780
assertNotNull(otherCursor);
7881
assertResponse(adminResponse, otherResponse);
7982
while (true) {
80-
adminResponse = runSql(null, mode,
81-
new StringEntity("{\"cursor\": \"" + adminCursor + "\"}", ContentType.APPLICATION_JSON));
82-
otherResponse = runSql(user, mode,
83-
new StringEntity("{\"cursor\": \"" + otherCursor + "\"}", ContentType.APPLICATION_JSON));
83+
adminResponse = runSql(null,
84+
new StringEntity("{\"cursor\": \"" + adminCursor + "\"" + mode(mode) + "}", ContentType.APPLICATION_JSON));
85+
otherResponse = runSql(user,
86+
new StringEntity("{\"cursor\": \"" + otherCursor + "\"" + mode(mode) + "}", ContentType.APPLICATION_JSON));
8487
adminCursor = (String) adminResponse.remove("cursor");
8588
otherCursor = (String) otherResponse.remove("cursor");
8689
assertResponse(adminResponse, otherResponse);
@@ -173,14 +176,11 @@ public void checkNoMonitorMain(String user) throws Exception {
173176
}
174177

175178
private static Map<String, Object> runSql(@Nullable String asUser, String mode, String sql) throws IOException {
176-
return runSql(asUser, mode, new StringEntity("{\"query\": \"" + sql + "\"}", ContentType.APPLICATION_JSON));
179+
return runSql(asUser, new StringEntity("{\"query\": \"" + sql + "\"" + mode(mode) + "}", ContentType.APPLICATION_JSON));
177180
}
178181

179-
private static Map<String, Object> runSql(@Nullable String asUser, String mode, HttpEntity entity) throws IOException {
182+
private static Map<String, Object> runSql(@Nullable String asUser, HttpEntity entity) throws IOException {
180183
Request request = new Request("POST", "/_xpack/sql");
181-
if (false == mode.isEmpty()) {
182-
request.addParameter("mode", mode);
183-
}
184184
if (asUser != null) {
185185
RequestOptions.Builder options = request.getOptions().toBuilder();
186186
options.addHeader("es-security-runas-user", asUser);
@@ -223,14 +223,15 @@ protected AuditLogAsserter createAuditLogAsserter() {
223223
public void testHijackScrollFails() throws Exception {
224224
createUser("full_access", "rest_minimal");
225225

226-
Map<String, Object> adminResponse = RestActions.runSql(null, randomMode(),
227-
new StringEntity("{\"query\": \"SELECT * FROM test\", \"fetch_size\": 1}", ContentType.APPLICATION_JSON));
226+
Map<String, Object> adminResponse = RestActions.runSql(null,
227+
new StringEntity("{\"query\": \"SELECT * FROM test\", \"fetch_size\": 1" + mode(randomMode()) + "}",
228+
ContentType.APPLICATION_JSON));
228229

229230
String cursor = (String) adminResponse.remove("cursor");
230231
assertNotNull(cursor);
231232

232-
ResponseException e = expectThrows(ResponseException.class, () -> RestActions.runSql("full_access", randomMode(),
233-
new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON)));
233+
ResponseException e = expectThrows(ResponseException.class, () -> RestActions.runSql("full_access",
234+
new StringEntity("{\"cursor\":\"" + cursor + "\"" + mode(randomMode()) + "}", ContentType.APPLICATION_JSON)));
234235
// TODO return a better error message for bad scrolls
235236
assertThat(e.getMessage(), containsString("No search context found for id"));
236237
assertEquals(404, e.getResponse().getStatusLine().getStatusCode());

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
import java.util.Map;
3636

3737
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
38+
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
39+
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
3840

3941
public class UserFunctionIT extends ESRestTestCase {
4042

@@ -172,14 +174,11 @@ private void deleteUser(String name) throws IOException {
172174
}
173175

174176
private Map<String, Object> runSql(String asUser, String mode, String sql) throws IOException {
175-
return runSql(asUser, mode, new StringEntity("{\"query\": \"" + sql + "\"}", ContentType.APPLICATION_JSON));
177+
return runSql(asUser, new StringEntity("{\"query\": \"" + sql + "\"" + mode(mode) + "}", ContentType.APPLICATION_JSON));
176178
}
177179

178-
private Map<String, Object> runSql(String asUser, String mode, HttpEntity entity) throws IOException {
180+
private Map<String, Object> runSql(String asUser, HttpEntity entity) throws IOException {
179181
Request request = new Request("POST", "/_xpack/sql");
180-
if (false == mode.isEmpty()) {
181-
request.addParameter("mode", mode);
182-
}
183182
if (asUser != null) {
184183
RequestOptions.Builder options = request.getOptions().toBuilder();
185184
options.addHeader("es-security-runas-user", asUser);
@@ -203,10 +202,6 @@ private static Map<String, Object> toMap(Response response) throws IOException {
203202
}
204203
}
205204

206-
private String randomMode() {
207-
return randomFrom("plain", "jdbc", "");
208-
}
209-
210205
private void index(String... docs) throws IOException {
211206
Request request = new Request("POST", "/test/test/_bulk");
212207
request.addParameter("refresh", "true");

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

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@
1515
import org.elasticsearch.client.Response;
1616
import org.elasticsearch.client.ResponseException;
1717
import org.elasticsearch.common.CheckedSupplier;
18+
import org.elasticsearch.common.Strings;
1819
import org.elasticsearch.common.collect.Tuple;
1920
import org.elasticsearch.common.io.Streams;
2021
import org.elasticsearch.common.xcontent.XContentHelper;
2122
import org.elasticsearch.common.xcontent.json.JsonXContent;
2223
import org.elasticsearch.test.NotEqualMessageBuilder;
2324
import org.elasticsearch.test.rest.ESRestTestCase;
25+
import org.elasticsearch.xpack.sql.proto.StringUtils;
2426
import org.elasticsearch.xpack.sql.qa.ErrorsTestCase;
2527
import org.hamcrest.Matcher;
2628

@@ -97,10 +99,10 @@ public void testNextPage() throws IOException {
9799
for (int i = 0; i < 20; i += 2) {
98100
Map<String, Object> response;
99101
if (i == 0) {
100-
response = runSql(mode, new StringEntity(sqlRequest, ContentType.APPLICATION_JSON));
102+
response = runSql(new StringEntity(sqlRequest, ContentType.APPLICATION_JSON), "");
101103
} else {
102-
response = runSql(mode, new StringEntity("{\"cursor\":\"" + cursor + "\"}",
103-
ContentType.APPLICATION_JSON));
104+
response = runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"" + mode(mode) + "}",
105+
ContentType.APPLICATION_JSON), StringUtils.EMPTY);
104106
}
105107

106108
Map<String, Object> expected = new HashMap<>();
@@ -120,8 +122,8 @@ public void testNextPage() throws IOException {
120122
}
121123
Map<String, Object> expected = new HashMap<>();
122124
expected.put("rows", emptyList());
123-
assertResponse(expected, runSql(mode, new StringEntity("{ \"cursor\":\"" + cursor + "\"}",
124-
ContentType.APPLICATION_JSON)));
125+
assertResponse(expected, runSql(new StringEntity("{ \"cursor\":\"" + cursor + "\"" + mode(mode) + "}",
126+
ContentType.APPLICATION_JSON), StringUtils.EMPTY));
125127
}
126128

127129
@AwaitsFix(bugUrl = "https://github.com/elastic/x-pack-elasticsearch/issues/2074")
@@ -136,8 +138,7 @@ public void testTimeZone() throws IOException {
136138
expected.put("size", 2);
137139

138140
// Default TimeZone is UTC
139-
assertResponse(expected, runSql(mode, new StringEntity("{\"query\":\"SELECT DAY_OF_YEAR(test), COUNT(*) FROM test\"}",
140-
ContentType.APPLICATION_JSON)));
141+
assertResponse(expected, runSql(mode, "SELECT DAY_OF_YEAR(test), COUNT(*) FROM test"));
141142
}
142143

143144
public void testScoreWithFieldNamedScore() throws IOException {
@@ -302,35 +303,29 @@ private void expectBadRequest(CheckedSupplier<Map<String, Object>, Exception> co
302303
}
303304

304305
private Map<String, Object> runSql(String mode, String sql) throws IOException {
305-
return runSql(mode, sql, "");
306+
return runSql(mode, sql, StringUtils.EMPTY);
306307
}
307308

308309
private Map<String, Object> runSql(String mode, String sql, String suffix) throws IOException {
309-
return runSql(mode, new StringEntity("{\"query\":\"" + sql + "\"}", ContentType.APPLICATION_JSON), suffix);
310+
return runSql(new StringEntity("{\"query\":\"" + sql + "\"" + mode(mode) + "}", ContentType.APPLICATION_JSON), suffix);
310311
}
311312

312-
private Map<String, Object> runSql(String mode, HttpEntity sql) throws IOException {
313-
return runSql(mode, sql, "");
314-
}
315-
316-
private Map<String, Object> runSql(String mode, HttpEntity sql, String suffix) throws IOException {
313+
private Map<String, Object> runSql(HttpEntity sql, String suffix) throws IOException {
317314
Request request = new Request("POST", "/_xpack/sql" + suffix);
318315
request.addParameter("error_trace", "true"); // Helps with debugging in case something crazy happens on the server.
319316
request.addParameter("pretty", "true"); // Improves error reporting readability
320317
if (randomBoolean()) {
321318
// We default to JSON but we force it randomly for extra coverage
322319
request.addParameter("format", "json");
323320
}
324-
if (false == mode.isEmpty()) {
325-
request.addParameter("mode", mode); // JDBC or PLAIN mode
326-
}
327321
if (randomBoolean()) {
328322
// JSON is the default but randomly set it sometime for extra coverage
329323
RequestOptions.Builder options = request.getOptions().toBuilder();
330324
options.addHeader("Accept", randomFrom("*/*", "application/json"));
331325
request.setOptions(options);
332326
}
333327
request.setEntity(sql);
328+
334329
Response response = client().performRequest(request);
335330
try (InputStream content = response.getEntity().getContent()) {
336331
return XContentHelper.convertToMap(JsonXContent.jsonXContent, content, false);
@@ -357,9 +352,9 @@ public void testBasicQueryWithFilter() throws IOException {
357352
Map<String, Object> expected = new HashMap<>();
358353
expected.put("columns", singletonList(columnInfo(mode, "test", "text", JDBCType.VARCHAR, 0)));
359354
expected.put("rows", singletonList(singletonList("foo")));
360-
assertResponse(expected, runSql(mode, new StringEntity("{\"query\":\"SELECT * FROM test\", " +
361-
"\"filter\":{\"match\": {\"test\": \"foo\"}}}",
362-
ContentType.APPLICATION_JSON)));
355+
assertResponse(expected, runSql(new StringEntity("{\"query\":\"SELECT * FROM test\", " +
356+
"\"filter\":{\"match\": {\"test\": \"foo\"}}" + mode(mode) + "}",
357+
ContentType.APPLICATION_JSON), StringUtils.EMPTY));
363358
}
364359

365360
public void testBasicQueryWithParameters() throws IOException {
@@ -373,16 +368,16 @@ public void testBasicQueryWithParameters() throws IOException {
373368
columnInfo(mode, "param", "integer", JDBCType.INTEGER, 11)
374369
));
375370
expected.put("rows", singletonList(Arrays.asList("foo", 10)));
376-
assertResponse(expected, runSql(mode, new StringEntity("{\"query\":\"SELECT test, ? param FROM test WHERE test = ?\", " +
377-
"\"params\":[{\"type\": \"integer\", \"value\": 10}, {\"type\": \"keyword\", \"value\": \"foo\"}]}",
378-
ContentType.APPLICATION_JSON)));
371+
assertResponse(expected, runSql(new StringEntity("{\"query\":\"SELECT test, ? param FROM test WHERE test = ?\", " +
372+
"\"params\":[{\"type\": \"integer\", \"value\": 10}, {\"type\": \"keyword\", \"value\": \"foo\"}]"
373+
+ mode(mode) + "}", ContentType.APPLICATION_JSON), StringUtils.EMPTY));
379374
}
380375

381376
public void testBasicTranslateQueryWithFilter() throws IOException {
382377
index("{\"test\":\"foo\"}",
383378
"{\"test\":\"bar\"}");
384379

385-
Map<String, Object> response = runSql("",
380+
Map<String, Object> response = runSql(
386381
new StringEntity("{\"query\":\"SELECT * FROM test\", \"filter\":{\"match\": {\"test\": \"foo\"}}}",
387382
ContentType.APPLICATION_JSON), "/translate/"
388383
);
@@ -424,7 +419,7 @@ public void testTranslateQueryWithGroupByAndHaving() throws IOException {
424419
index("{\"salary\":100}",
425420
"{\"age\":20}");
426421

427-
Map<String, Object> response = runSql("",
422+
Map<String, Object> response = runSql(
428423
new StringEntity("{\"query\":\"SELECT avg(salary) FROM test GROUP BY abs(age) HAVING avg(salary) > 50 LIMIT 10\"}",
429424
ContentType.APPLICATION_JSON), "/translate/"
430425
);
@@ -551,10 +546,10 @@ public void testNextPageText() throws IOException {
551546
for (int i = 0; i < 20; i += 2) {
552547
Tuple<String, String> response;
553548
if (i == 0) {
554-
response = runSqlAsText("", new StringEntity(request, ContentType.APPLICATION_JSON), "text/plain");
549+
response = runSqlAsText(StringUtils.EMPTY, new StringEntity(request, ContentType.APPLICATION_JSON), "text/plain");
555550
} else {
556-
response = runSqlAsText("", new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON),
557-
"text/plain");
551+
response = runSqlAsText(StringUtils.EMPTY, new StringEntity("{\"cursor\":\"" + cursor + "\"}",
552+
ContentType.APPLICATION_JSON), "text/plain");
558553
}
559554

560555
StringBuilder expected = new StringBuilder();
@@ -570,9 +565,10 @@ public void testNextPageText() throws IOException {
570565
}
571566
Map<String, Object> expected = new HashMap<>();
572567
expected.put("rows", emptyList());
573-
assertResponse(expected, runSql("", new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON)));
568+
assertResponse(expected, runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON),
569+
StringUtils.EMPTY));
574570

575-
Map<String, Object> response = runSql("", new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON),
571+
Map<String, Object> response = runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON),
576572
"/close");
577573
assertEquals(true, response.get("succeeded"));
578574

@@ -638,7 +634,7 @@ public void testQueryInTSV() throws IOException {
638634
}
639635

640636
private Tuple<String, String> runSqlAsText(String sql, String accept) throws IOException {
641-
return runSqlAsText("", new StringEntity("{\"query\":\"" + sql + "\"}", ContentType.APPLICATION_JSON), accept);
637+
return runSqlAsText(StringUtils.EMPTY, new StringEntity("{\"query\":\"" + sql + "\"}", ContentType.APPLICATION_JSON), accept);
642638
}
643639

644640
/**
@@ -716,7 +712,11 @@ private static Map<String, Object> searchStats() throws IOException {
716712
}
717713

718714
public static String randomMode() {
719-
return randomFrom("", "jdbc", "plain");
715+
return randomFrom(StringUtils.EMPTY, "jdbc", "plain");
716+
}
717+
718+
public static String mode(String mode) {
719+
return Strings.isEmpty(mode) ? StringUtils.EMPTY : ",\"mode\":\"" + mode + "\"";
720720
}
721721

722722
private void index(String... docs) throws IOException {

0 commit comments

Comments
 (0)