diff --git a/x-pack/plugin/sql/qa/multi-node/src/test/java/org/elasticsearch/xpack/sql/qa/multi_node/RestSqlMultinodeIT.java b/x-pack/plugin/sql/qa/multi-node/src/test/java/org/elasticsearch/xpack/sql/qa/multi_node/RestSqlMultinodeIT.java index 69b33767fa9..2a4dd79001d 100644 --- a/x-pack/plugin/sql/qa/multi-node/src/test/java/org/elasticsearch/xpack/sql/qa/multi_node/RestSqlMultinodeIT.java +++ b/x-pack/plugin/sql/qa/multi-node/src/test/java/org/elasticsearch/xpack/sql/qa/multi_node/RestSqlMultinodeIT.java @@ -26,6 +26,7 @@ import java.util.Map; 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 @@ public class RestSqlMultinodeIT extends ESRestTestCase { 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 actual = responseToMap(client.performRequest(request)); if (false == expected.equals(actual)) { diff --git a/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/RestSqlSecurityIT.java b/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/RestSqlSecurityIT.java index b6dd5efdfef..86772a49372 100644 --- a/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/RestSqlSecurityIT.java +++ b/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/RestSqlSecurityIT.java @@ -30,6 +30,7 @@ import java.util.Map; 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,10 +67,12 @@ public class RestSqlSecurityIT extends SqlSecurityTestCase { @Override public void expectScrollMatchesAdmin(String adminSql, String user, String userSql) throws Exception { String mode = randomMode(); - Map adminResponse = runSql(null, mode, - new StringEntity("{\"query\": \"" + adminSql + "\", \"fetch_size\": 1}", ContentType.APPLICATION_JSON)); - Map otherResponse = runSql(user, mode, - new StringEntity("{\"query\": \"" + adminSql + "\", \"fetch_size\": 1}", ContentType.APPLICATION_JSON)); + Map adminResponse = runSql(null, + new StringEntity("{\"query\": \"" + adminSql + "\", \"fetch_size\": 1" + mode(mode) + "}", + ContentType.APPLICATION_JSON)); + Map 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"); @@ -77,10 +80,10 @@ public class RestSqlSecurityIT extends SqlSecurityTestCase { 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 class RestSqlSecurityIT extends SqlSecurityTestCase { } private static Map 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 runSql(@Nullable String asUser, String mode, HttpEntity entity) throws IOException { + private static Map 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 @@ public class RestSqlSecurityIT extends SqlSecurityTestCase { public void testHijackScrollFails() throws Exception { createUser("full_access", "rest_minimal"); - Map adminResponse = RestActions.runSql(null, randomMode(), - new StringEntity("{\"query\": \"SELECT * FROM test\", \"fetch_size\": 1}", ContentType.APPLICATION_JSON)); + Map 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()); diff --git a/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/UserFunctionIT.java b/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/UserFunctionIT.java index 4ddeb7d4cdc..5d37e38bc4c 100644 --- a/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/UserFunctionIT.java +++ b/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/UserFunctionIT.java @@ -35,6 +35,8 @@ import java.util.List; 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 @@ public class UserFunctionIT extends ESRestTestCase { } private Map 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 runSql(String asUser, String mode, HttpEntity entity) throws IOException { + private Map 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 @@ public class UserFunctionIT extends ESRestTestCase { } } - 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"); diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java index 1b8160146cd..062ab0c81b9 100644 --- a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java +++ b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java @@ -15,12 +15,14 @@ import org.elasticsearch.client.RequestOptions; 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 abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe for (int i = 0; i < 20; i += 2) { Map 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 expected = new HashMap<>(); @@ -120,8 +122,8 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe } Map 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 abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe 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,18 +303,14 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe } private Map runSql(String mode, String sql) throws IOException { - return runSql(mode, sql, ""); + return runSql(mode, sql, StringUtils.EMPTY); } private Map 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 runSql(String mode, HttpEntity sql) throws IOException { - return runSql(mode, sql, ""); - } - - private Map runSql(String mode, HttpEntity sql, String suffix) throws IOException { + private Map 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 @@ -321,9 +318,6 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe // 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(); @@ -331,6 +325,7 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe 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 abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe Map 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 abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe 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 response = runSql("", + Map response = runSql( new StringEntity("{\"query\":\"SELECT * FROM test\", \"filter\":{\"match\": {\"test\": \"foo\"}}}", ContentType.APPLICATION_JSON), "/translate/" ); @@ -424,7 +419,7 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe index("{\"salary\":100}", "{\"age\":20}"); - Map response = runSql("", + Map 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 abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe for (int i = 0; i < 20; i += 2) { Tuple 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 abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe } Map 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 response = runSql("", new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON), + Map response = runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON), "/close"); assertEquals(true, response.get("succeeded")); @@ -638,7 +634,7 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe } private Tuple 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 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe } 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 { diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlUsageTestCase.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlUsageTestCase.java index a874296e155..a8c45fedf2a 100644 --- a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlUsageTestCase.java +++ b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlUsageTestCase.java @@ -14,6 +14,7 @@ import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.xpack.sql.proto.Mode; +import org.elasticsearch.xpack.sql.proto.StringUtils; import org.elasticsearch.xpack.sql.qa.FeatureMetric; import org.junit.Before; @@ -25,6 +26,8 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode; + public abstract class RestSqlUsageTestCase extends ESRestTestCase { private List testData = Arrays.asList( new IndexDocument("used", "Don Quixote", 1072), @@ -64,11 +67,11 @@ public abstract class RestSqlUsageTestCase extends ESRestTestCase { Map baseStats = getStats(); List>> nodesListStats = (List) baseStats.get("stats"); - // used for "client.id" request parameter value, but also for getting the stats from ES + // used for "client_id" request parameter value, but also for getting the stats from ES clientType = randomFrom(ClientType.values()).toString(); ignoreClientType = randomBoolean(); - // "client.id" parameter will not be sent in the requests + // "client_id" parameter will not be sent in the requests // and "clientType" will only be used for getting the stats back from ES if (ignoreClientType) { clientType = ClientType.REST.toString(); @@ -274,20 +277,15 @@ public abstract class RestSqlUsageTestCase extends ESRestTestCase { // 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 - } - // randomly use the "client.id" parameter or not - if (false == ignoreClientType) { - request.addParameter("client.id", restClient); - } 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(new StringEntity("{\"query\":\"" + sql + "\"}", ContentType.APPLICATION_JSON)); + request.setEntity(new StringEntity("{\"query\":\"" + sql + "\"" + mode(mode) + + (ignoreClientType ? StringUtils.EMPTY : ",\"client_id\":\"" + restClient + "\"") + "}", + ContentType.APPLICATION_JSON)); client().performRequest(request); } diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/AbstractSqlQueryRequest.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/AbstractSqlQueryRequest.java index 8892346c1b8..2b90a7d41fa 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/AbstractSqlQueryRequest.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/AbstractSqlQueryRequest.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.index.query.AbstractQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.xpack.sql.proto.Mode; import org.elasticsearch.xpack.sql.proto.Protocol; import org.elasticsearch.xpack.sql.proto.RequestInfo; import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue; @@ -39,6 +40,17 @@ public abstract class AbstractSqlQueryRequest extends AbstractSqlRequest impleme @Nullable private QueryBuilder filter = null; private List params = Collections.emptyList(); + + static final ParseField QUERY = new ParseField("query"); + static final ParseField CURSOR = new ParseField("cursor"); + static final ParseField PARAMS = new ParseField("params"); + static final ParseField TIME_ZONE = new ParseField("time_zone"); + static final ParseField FETCH_SIZE = new ParseField("fetch_size"); + static final ParseField REQUEST_TIMEOUT = new ParseField("request_timeout"); + static final ParseField PAGE_TIMEOUT = new ParseField("page_timeout"); + static final ParseField FILTER = new ParseField("filter"); + static final ParseField MODE = new ParseField("mode"); + static final ParseField CLIENT_ID = new ParseField("client_id"); public AbstractSqlQueryRequest() { super(); @@ -57,19 +69,22 @@ public abstract class AbstractSqlQueryRequest extends AbstractSqlRequest impleme } protected static ObjectParser objectParser(Supplier supplier) { - // TODO: convert this into ConstructingObjectParser - ObjectParser parser = new ObjectParser<>("sql/query", true, supplier); - parser.declareString(AbstractSqlQueryRequest::query, new ParseField("query")); - parser.declareObjectArray(AbstractSqlQueryRequest::params, (p, c) -> SqlTypedParamValue.fromXContent(p), new ParseField("params")); - parser.declareString((request, zoneId) -> request.timeZone(TimeZone.getTimeZone(zoneId)), new ParseField("time_zone")); - parser.declareInt(AbstractSqlQueryRequest::fetchSize, new ParseField("fetch_size")); + // Using an ObjectParser here (vs. ConstructingObjectParser) because the latter needs to instantiate a concrete class + // and we would duplicate the code from this class to its subclasses + ObjectParser parser = new ObjectParser<>("sql/query", false, supplier); + parser.declareString(AbstractSqlQueryRequest::query, QUERY); + parser.declareString((request, mode) -> request.mode(Mode.fromString(mode)), MODE); + parser.declareString((request, clientId) -> request.clientId(clientId), CLIENT_ID); + parser.declareObjectArray(AbstractSqlQueryRequest::params, (p, c) -> SqlTypedParamValue.fromXContent(p), PARAMS); + parser.declareString((request, zoneId) -> request.timeZone(TimeZone.getTimeZone(zoneId)), TIME_ZONE); + parser.declareInt(AbstractSqlQueryRequest::fetchSize, FETCH_SIZE); parser.declareString((request, timeout) -> request.requestTimeout(TimeValue.parseTimeValue(timeout, Protocol.REQUEST_TIMEOUT, - "request_timeout")), new ParseField("request_timeout")); + "request_timeout")), REQUEST_TIMEOUT); parser.declareString( (request, timeout) -> request.pageTimeout(TimeValue.parseTimeValue(timeout, Protocol.PAGE_TIMEOUT, "page_timeout")), - new ParseField("page_timeout")); + PAGE_TIMEOUT); parser.declareObject(AbstractSqlQueryRequest::filter, - (p, c) -> AbstractQueryBuilder.parseInnerQueryBuilder(p), new ParseField("filter")); + (p, c) -> AbstractQueryBuilder.parseInnerQueryBuilder(p), FILTER); return parser; } diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/AbstractSqlRequest.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/AbstractSqlRequest.java index 9dd9bf40bfa..ea5cce74ed3 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/AbstractSqlRequest.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/AbstractSqlRequest.java @@ -28,7 +28,7 @@ public abstract class AbstractSqlRequest extends ActionRequest implements ToXCon private RequestInfo requestInfo; protected AbstractSqlRequest() { - + this.requestInfo = new RequestInfo(Mode.PLAIN); } protected AbstractSqlRequest(RequestInfo requestInfo) { diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlClearCursorRequest.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlClearCursorRequest.java index 778ab4a613a..4d5cc173433 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlClearCursorRequest.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlClearCursorRequest.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.sql.action; import org.elasticsearch.action.ActionRequestValidationException; -import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; @@ -20,30 +19,37 @@ import java.util.Objects; import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; +import static org.elasticsearch.xpack.sql.action.AbstractSqlQueryRequest.CURSOR; +import static org.elasticsearch.xpack.sql.action.AbstractSqlQueryRequest.MODE; +import static org.elasticsearch.xpack.sql.action.AbstractSqlQueryRequest.CLIENT_ID; /** * Request to clean all SQL resources associated with the cursor */ public class SqlClearCursorRequest extends AbstractSqlRequest { - private static final ConstructingObjectParser PARSER = - new ConstructingObjectParser<>(SqlClearCursorAction.NAME, true, (objects, mode) -> new SqlClearCursorRequest( - mode, - (String) objects[0] - )); + private static final ConstructingObjectParser PARSER = + // here the position in "objects" is the same as the fields parser declarations below + new ConstructingObjectParser<>(SqlClearCursorAction.NAME, objects -> { + RequestInfo requestInfo = new RequestInfo(Mode.fromString((String) objects[1]), + (String) objects[2]); + return new SqlClearCursorRequest(requestInfo, (String) objects[0]); + }); static { - PARSER.declareString(constructorArg(), new ParseField("cursor")); + // "cursor" is required constructor parameter + PARSER.declareString(constructorArg(), CURSOR); + PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), MODE); + PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), CLIENT_ID); } private String cursor; public SqlClearCursorRequest() { - } - public SqlClearCursorRequest(Mode mode, String cursor) { - super(new RequestInfo(mode)); + public SqlClearCursorRequest(RequestInfo requestInfo, String cursor) { + super(requestInfo); this.cursor = cursor; } @@ -101,7 +107,7 @@ public class SqlClearCursorRequest extends AbstractSqlRequest { return new org.elasticsearch.xpack.sql.proto.SqlClearCursorRequest(cursor, requestInfo()).toXContent(builder, params); } - public static SqlClearCursorRequest fromXContent(XContentParser parser, Mode mode) { - return PARSER.apply(parser, mode); + public static SqlClearCursorRequest fromXContent(XContentParser parser) { + return PARSER.apply(parser, null); } } diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryRequest.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryRequest.java index eeda0b9d8bf..ec3e2b331f0 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryRequest.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryRequest.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.sql.action; import org.elasticsearch.action.ActionRequestValidationException; -import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -14,7 +13,6 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.index.query.AbstractQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.xpack.sql.proto.RequestInfo; import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue; @@ -32,18 +30,14 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; public class SqlQueryRequest extends AbstractSqlQueryRequest { private static final ObjectParser PARSER = objectParser(SqlQueryRequest::new); - public static final ParseField CURSOR = new ParseField("cursor"); - public static final ParseField FILTER = new ParseField("filter"); - static { PARSER.declareString(SqlQueryRequest::cursor, CURSOR); - PARSER.declareObject(SqlQueryRequest::filter, - (p, c) -> AbstractQueryBuilder.parseInnerQueryBuilder(p), FILTER); } private String cursor = ""; public SqlQueryRequest() { + super(); } public SqlQueryRequest(String query, List params, QueryBuilder filter, TimeZone timeZone, @@ -114,9 +108,7 @@ public class SqlQueryRequest extends AbstractSqlQueryRequest { pageTimeout(), filter(), cursor(), requestInfo()).toXContent(builder, params); } - public static SqlQueryRequest fromXContent(XContentParser parser, RequestInfo requestInfo) { - SqlQueryRequest request = PARSER.apply(parser, null); - request.requestInfo(requestInfo); - return request; + public static SqlQueryRequest fromXContent(XContentParser parser) { + return PARSER.apply(parser, null); } } \ No newline at end of file diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java index bafd4581f1f..6206879487d 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Objects; import static java.util.Collections.unmodifiableList; +import static org.elasticsearch.xpack.sql.action.AbstractSqlQueryRequest.CURSOR; /** * Response to perform an sql query @@ -31,6 +32,7 @@ public class SqlQueryResponse extends ActionResponse implements ToXContentObject // TODO: Simplify cursor handling private String cursor; + private Mode mode; private List columns; // TODO investigate reusing Page here - it probably is much more efficient private List> rows; @@ -38,8 +40,9 @@ public class SqlQueryResponse extends ActionResponse implements ToXContentObject public SqlQueryResponse() { } - public SqlQueryResponse(String cursor, @Nullable List columns, List> rows) { + public SqlQueryResponse(String cursor, Mode mode, @Nullable List columns, List> rows) { this.cursor = cursor; + this.mode = mode; this.columns = columns; this.rows = rows; } @@ -134,7 +137,6 @@ public class SqlQueryResponse extends ActionResponse implements ToXContentObject @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - Mode mode = Mode.fromString(params.param("mode")); builder.startObject(); { if (columns != null) { @@ -157,7 +159,7 @@ public class SqlQueryResponse extends ActionResponse implements ToXContentObject builder.endArray(); if (cursor.equals("") == false) { - builder.field(SqlQueryRequest.CURSOR.getPreferredName(), cursor); + builder.field(CURSOR.getPreferredName(), cursor); } } return builder.endObject(); diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlTranslateRequest.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlTranslateRequest.java index a25a1ca5c95..4522c938f8a 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlTranslateRequest.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlTranslateRequest.java @@ -13,9 +13,8 @@ import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.xpack.sql.proto.Mode; -import org.elasticsearch.xpack.sql.proto.SqlQueryRequest; import org.elasticsearch.xpack.sql.proto.RequestInfo; +import org.elasticsearch.xpack.sql.proto.SqlQueryRequest; import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue; import java.io.IOException; @@ -31,6 +30,7 @@ public class SqlTranslateRequest extends AbstractSqlQueryRequest { private static final ObjectParser PARSER = objectParser(SqlTranslateRequest::new); public SqlTranslateRequest() { + super(); } public SqlTranslateRequest(String query, List params, QueryBuilder filter, TimeZone timeZone, @@ -56,9 +56,8 @@ public class SqlTranslateRequest extends AbstractSqlQueryRequest { return "SQL Translate [" + query() + "][" + filter() + "]"; } - public static SqlTranslateRequest fromXContent(XContentParser parser, Mode mode) { + public static SqlTranslateRequest fromXContent(XContentParser parser) { SqlTranslateRequest request = PARSER.apply(parser, null); - request.requestInfo(new RequestInfo(mode)); return request; } diff --git a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlClearCursorRequestTests.java b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlClearCursorRequestTests.java index e9c7043519b..a77ded7e63a 100644 --- a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlClearCursorRequestTests.java +++ b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlClearCursorRequestTests.java @@ -9,22 +9,27 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.AbstractSerializingTestCase; import org.elasticsearch.xpack.sql.proto.Mode; +import org.elasticsearch.xpack.sql.proto.RequestInfo; import org.junit.Before; import java.io.IOException; import java.util.function.Consumer; +import static org.elasticsearch.xpack.sql.proto.RequestInfo.CLIENT_IDS; + public class SqlClearCursorRequestTests extends AbstractSerializingTestCase { - public Mode testMode; + + public RequestInfo requestInfo; @Before public void setup() { - testMode = randomFrom(Mode.values()); + requestInfo = new RequestInfo(randomFrom(Mode.values()), + randomFrom(randomFrom(CLIENT_IDS), randomAlphaOfLengthBetween(10, 20))); } @Override protected SqlClearCursorRequest createTestInstance() { - return new SqlClearCursorRequest(testMode, randomAlphaOfLength(100)); + return new SqlClearCursorRequest(requestInfo, randomAlphaOfLength(100)); } @Override @@ -34,20 +39,23 @@ public class SqlClearCursorRequestTests extends AbstractSerializingTestCase mutator = randomFrom( - request -> request.mode(randomValueOtherThan(request.mode(), () -> randomFrom(Mode.values()))), + request -> request.requestInfo(randomValueOtherThan(request.requestInfo(), this::randomRequestInfo)), request -> request.setCursor(randomValueOtherThan(request.getCursor(), SqlQueryResponseTests::randomStringCursor)) ); - SqlClearCursorRequest newRequest = new SqlClearCursorRequest(instance.mode(), instance.getCursor()); + SqlClearCursorRequest newRequest = new SqlClearCursorRequest(instance.requestInfo(), instance.getCursor()); mutator.accept(newRequest); return newRequest; - } } diff --git a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlQueryRequestTests.java b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlQueryRequestTests.java index beb206b2dbc..4a5e063df15 100644 --- a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlQueryRequestTests.java +++ b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlQueryRequestTests.java @@ -27,18 +27,16 @@ import java.util.function.Supplier; import static org.elasticsearch.xpack.sql.action.SqlTestUtils.randomFilter; import static org.elasticsearch.xpack.sql.action.SqlTestUtils.randomFilterOrNull; -import static org.elasticsearch.xpack.sql.proto.RequestInfo.CLI; -import static org.elasticsearch.xpack.sql.proto.RequestInfo.CANVAS; +import static org.elasticsearch.xpack.sql.proto.RequestInfo.CLIENT_IDS; public class SqlQueryRequestTests extends AbstractSerializingTestCase { public RequestInfo requestInfo; - public String clientId; @Before public void setup() { - clientId = randomFrom(CLI, CANVAS, randomAlphaOfLengthBetween(10, 20)); - requestInfo = new RequestInfo(randomFrom(Mode.values()), clientId); + requestInfo = new RequestInfo(randomFrom(Mode.values()), + randomFrom(randomFrom(CLIENT_IDS), randomAlphaOfLengthBetween(10, 20))); } @Override @@ -62,7 +60,7 @@ public class SqlQueryRequestTests extends AbstractSerializingTestCase randomParameters() { @@ -96,7 +94,7 @@ public class SqlQueryRequestTests extends AbstractSerializingTestCase { @@ -34,10 +36,10 @@ public class SqlQueryResponseTests extends AbstractStreamableXContentTestCase columns = null; @@ -69,7 +71,7 @@ public class SqlQueryResponseTests extends AbstractStreamableXContentTestCase list = new ArrayList(1); + list.add(new SqlTypedParamValue("whatever", 123)); + assertEquals(list, request.params()); + assertEquals("UTC", request.timeZone().getID()); + assertEquals(TimeValue.parseTimeValue("5s", "request_timeout"), request.requestTimeout()); + assertEquals(TimeValue.parseTimeValue("10s", "page_timeout"), request.pageTimeout()); + } + + private R generateRequest(String json, Function fromXContent) + throws IOException { + XContentParser parser = parser(json); + return fromXContent.apply(parser); + } + + private void assertParsingErrorMessage(String json, String errorMessage, Consumer consumer) throws IOException { + XContentParser parser = parser(json); + Exception e = expectThrows(IllegalArgumentException.class, () -> consumer.accept(parser)); + assertThat(e.getMessage(), containsString(errorMessage)); + } + + private void assertParsingErrorMessageReason(String json, String errorMessage, Consumer consumer) throws IOException { + XContentParser parser = parser(json); + Exception e = expectThrows(IllegalArgumentException.class, () -> consumer.accept(parser)); + assertThat(e.getCause().getMessage(), containsString(errorMessage)); + } + + private XContentParser parser(String content) throws IOException { + XContentType xContentType = XContentType.JSON; + return xContentType.xContent().createParser( + NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, content); + } +} diff --git a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlTranslateRequestTests.java b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlTranslateRequestTests.java index fbc282a0a96..3d48f7fc7a4 100644 --- a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlTranslateRequestTests.java +++ b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlTranslateRequestTests.java @@ -63,7 +63,7 @@ public class SqlTranslateRequestTests extends AbstractSerializingTestCase responseParser) throws SQLException { byte[] requestBytes = toXContent(request); - String query = "error_trace&mode=" + - request.mode() + - (request.clientId() != null ? "&" + CLIENT_ID + "=" + request.clientId() : ""); + String query = "error_trace"; Tuple response = AccessController.doPrivileged((PrivilegedAction>>) () -> JreHttpUrlConnection.http(path, query, cfg, con -> diff --git a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/Mode.java b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/Mode.java index 598c52a9179..a301b41d388 100644 --- a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/Mode.java +++ b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/Mode.java @@ -17,7 +17,7 @@ public enum Mode { ODBC; public static Mode fromString(String mode) { - if (mode == null) { + if (mode == null || mode.isEmpty()) { return PLAIN; } return Mode.valueOf(mode.toUpperCase(Locale.ROOT)); diff --git a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/RequestInfo.java b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/RequestInfo.java index 03a388359d9..be1f1a58adb 100644 --- a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/RequestInfo.java +++ b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/RequestInfo.java @@ -6,11 +6,15 @@ package org.elasticsearch.xpack.sql.proto; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; import java.util.Objects; public class RequestInfo { public static final String CLI = "cli"; - public static final String CANVAS = "canvas"; + private static final String CANVAS = "canvas"; + public static final List CLIENT_IDS = Arrays.asList(CLI, CANVAS); private Mode mode; private String clientId; @@ -20,8 +24,8 @@ public class RequestInfo { } public RequestInfo(Mode mode, String clientId) { - this.mode = mode; - this.clientId = clientId; + mode(mode); + clientId(clientId); } public Mode mode() { @@ -37,6 +41,12 @@ public class RequestInfo { } public void clientId(String clientId) { + if (clientId != null) { + clientId = clientId.toLowerCase(Locale.ROOT); + if (false == CLIENT_IDS.contains(clientId)) { + clientId = null; + } + } this.clientId = clientId; } diff --git a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/SqlClearCursorRequest.java b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/SqlClearCursorRequest.java index 2881c0657aa..b1f374e74a8 100644 --- a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/SqlClearCursorRequest.java +++ b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/SqlClearCursorRequest.java @@ -43,6 +43,10 @@ public class SqlClearCursorRequest extends AbstractSqlRequest { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.field("cursor", cursor); + builder.field("mode", mode().toString()); + if (clientId() != null) { + builder.field("client_id", clientId()); + } return builder; } } diff --git a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/SqlQueryRequest.java b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/SqlQueryRequest.java index 93a8f1f5e71..651dc468bb9 100644 --- a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/SqlQueryRequest.java +++ b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/SqlQueryRequest.java @@ -140,6 +140,10 @@ public class SqlQueryRequest extends AbstractSqlRequest { if (query != null) { builder.field("query", query); } + builder.field("mode", mode().toString()); + if (clientId() != null) { + builder.field("client_id", clientId()); + } if (this.params.isEmpty() == false) { builder.startArray("params"); for (SqlTypedParamValue val : this.params) { diff --git a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/StringUtils.java b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/StringUtils.java index 10ece6f5a00..93c2e2f743c 100644 --- a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/StringUtils.java +++ b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/StringUtils.java @@ -24,6 +24,8 @@ import static java.time.temporal.ChronoField.SECOND_OF_MINUTE; public final class StringUtils { + public static final String EMPTY = ""; + private static final DateTimeFormatter ISO_WITH_MILLIS = new DateTimeFormatterBuilder() .parseCaseInsensitive() .append(ISO_LOCAL_DATE) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlClearCursorAction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlClearCursorAction.java index cb2ffa3e107..00e316ffabe 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlClearCursorAction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlClearCursorAction.java @@ -17,7 +17,6 @@ import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.RestToXContentListener; import org.elasticsearch.xpack.sql.action.SqlClearCursorAction; import org.elasticsearch.xpack.sql.action.SqlClearCursorRequest; -import org.elasticsearch.xpack.sql.proto.Mode; import org.elasticsearch.xpack.sql.proto.Protocol; import java.io.IOException; @@ -37,11 +36,13 @@ public class RestSqlClearCursorAction extends BaseRestHandler { } @Override - protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) + throws IOException { SqlClearCursorRequest sqlRequest; - try (XContentParser parser = request.contentOrSourceParamParser()) { - sqlRequest = SqlClearCursorRequest.fromXContent(parser, Mode.fromString(request.param("mode"))); + try (XContentParser parser = request.contentParser()) { + sqlRequest = SqlClearCursorRequest.fromXContent(parser); } + return channel -> client.executeLocally(SqlClearCursorAction.INSTANCE, sqlRequest, new RestToXContentListener<>(channel)); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java index b8bf7cb8583..2158d0a0037 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java @@ -24,27 +24,20 @@ import org.elasticsearch.rest.action.RestResponseListener; import org.elasticsearch.xpack.sql.action.SqlQueryAction; import org.elasticsearch.xpack.sql.action.SqlQueryRequest; import org.elasticsearch.xpack.sql.action.SqlQueryResponse; -import org.elasticsearch.xpack.sql.proto.Mode; import org.elasticsearch.xpack.sql.proto.Protocol; -import org.elasticsearch.xpack.sql.proto.RequestInfo; import org.elasticsearch.xpack.sql.session.Cursor; import org.elasticsearch.xpack.sql.session.Cursors; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.Locale; import static org.elasticsearch.rest.RestRequest.Method.GET; import static org.elasticsearch.rest.RestRequest.Method.POST; -import static org.elasticsearch.xpack.sql.proto.RequestInfo.CANVAS; -import static org.elasticsearch.xpack.sql.proto.RequestInfo.CLI; public class RestSqlQueryAction extends BaseRestHandler { private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestSqlQueryAction.class)); - private static String CLIENT_ID = "client.id"; - RestSqlQueryAction(Settings settings, RestController controller) { super(settings); // TODO: remove deprecated endpoint in 8.0.0 @@ -58,21 +51,13 @@ public class RestSqlQueryAction extends BaseRestHandler { } @Override - protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) + throws IOException { SqlQueryRequest sqlRequest; try (XContentParser parser = request.contentOrSourceParamParser()) { - String clientId = request.param(CLIENT_ID); - if (clientId != null) { - clientId = clientId.toLowerCase(Locale.ROOT); - if (!clientId.equals(CLI) && !clientId.equals(CANVAS)) { - clientId = null; - } - } - - sqlRequest = SqlQueryRequest.fromXContent(parser, - new RequestInfo(Mode.fromString(request.param("mode")), clientId)); + sqlRequest = SqlQueryRequest.fromXContent(parser); } - + /* * Since we support {@link TextFormat} and * {@link XContent} outputs we can't use {@link RestToXContentListener} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlTranslateAction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlTranslateAction.java index 5929ca5aadd..c066b854593 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlTranslateAction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlTranslateAction.java @@ -16,7 +16,6 @@ import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.RestToXContentListener; import org.elasticsearch.xpack.sql.action.SqlTranslateAction; import org.elasticsearch.xpack.sql.action.SqlTranslateRequest; -import org.elasticsearch.xpack.sql.proto.Mode; import org.elasticsearch.xpack.sql.proto.Protocol; import java.io.IOException; @@ -44,11 +43,13 @@ public class RestSqlTranslateAction extends BaseRestHandler { } @Override - protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) + throws IOException { SqlTranslateRequest sqlRequest; try (XContentParser parser = request.contentOrSourceParamParser()) { - sqlRequest = SqlTranslateRequest.fromXContent(parser, Mode.fromString(request.param("mode"))); + sqlRequest = SqlTranslateRequest.fromXContent(parser); } + return channel -> client.executeLocally(SqlTranslateAction.INSTANCE, sqlRequest, new RestToXContentListener<>(channel)); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java index 94c7965ee95..5a794572b90 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java @@ -88,7 +88,7 @@ public class TransportSqlQueryAction extends HandledTransportAction listener.onResponse(createResponse(rowSet, null)), + ActionListener.wrap(rowSet -> listener.onResponse(createResponse(request.mode(), rowSet, null)), e -> { planExecutor.metrics().failed(metric); listener.onFailure(e); @@ -107,10 +107,10 @@ public class TransportSqlQueryAction extends HandledTransportAction columns) { + static SqlQueryResponse createResponse(Mode mode, RowSet rowSet, List columns) { List> rows = new ArrayList<>(); rowSet.forEachRow(rowView -> { List row = new ArrayList<>(rowView.columnCount()); @@ -120,6 +120,7 @@ public class TransportSqlQueryAction extends HandledTransportAction