From 375caf4e4be31131153880d97fb298c148f5a284 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 4 Dec 2018 04:34:14 +0200 Subject: [PATCH] Adressing review comments --- .../sql/action/AbstractSqlQueryRequest.java | 20 ++-- .../sql/action/SqlClearCursorRequest.java | 9 +- .../xpack/sql/action/SqlQueryRequest.java | 11 +-- .../xpack/sql/action/SqlQueryResponse.java | 3 +- .../sql/action/SqlQueryResponseTests.java | 3 +- .../sql/action/SqlRequestParsersTests.java | 93 +++++++++---------- .../xpack/sql/plugin/AbstractSqlAction.java | 64 +++++++++++++ .../sql/plugin/RestSqlClearCursorAction.java | 32 +++---- .../xpack/sql/plugin/RestSqlQueryAction.java | 32 ++----- .../sql/plugin/RestSqlTranslateAction.java | 34 +++---- .../xpack/sql/plugin/SqlField.java | 16 ---- 11 files changed, 160 insertions(+), 157 deletions(-) create mode 100644 x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/AbstractSqlAction.java delete mode 100644 x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlField.java 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 e03221623e46e..56bd6910ea1d4 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 @@ -60,7 +60,7 @@ public AbstractSqlQueryRequest(String query, List params, Qu protected static ObjectParser objectParser(Supplier supplier) { // TODO: convert this into ConstructingObjectParser ObjectParser parser = new ObjectParser<>("sql/query", false, supplier); - parser.declareString(AbstractSqlQueryRequest::query, Field.QUERY); + parser.declareString(AbstractSqlQueryRequest::query, SqlRequestField.QUERY); parser.declareString((request, mode) -> { Mode m = Mode.fromString(mode); if (request.requestInfo() != null) { @@ -68,24 +68,24 @@ protected static ObjectParser objec } else { request.requestInfo(new RequestInfo(m)); } - }, Field.MODE); + }, SqlRequestField.MODE); parser.declareString((request, clientId) -> { if (request.requestInfo() != null) { request.requestInfo().clientId(clientId); } else { request.requestInfo(new RequestInfo(Mode.PLAIN, clientId)); } - }, Field.CLIENT_ID); - parser.declareObjectArray(AbstractSqlQueryRequest::params, (p, c) -> SqlTypedParamValue.fromXContent(p), Field.PARAMS); - parser.declareString((request, zoneId) -> request.timeZone(TimeZone.getTimeZone(zoneId)), Field.TIME_ZONE); - parser.declareInt(AbstractSqlQueryRequest::fetchSize, Field.FETCH_SIZE); + }, SqlRequestField.CLIENT_ID); + parser.declareObjectArray(AbstractSqlQueryRequest::params, (p, c) -> SqlTypedParamValue.fromXContent(p), SqlRequestField.PARAMS); + parser.declareString((request, zoneId) -> request.timeZone(TimeZone.getTimeZone(zoneId)), SqlRequestField.TIME_ZONE); + parser.declareInt(AbstractSqlQueryRequest::fetchSize, SqlRequestField.FETCH_SIZE); parser.declareString((request, timeout) -> request.requestTimeout(TimeValue.parseTimeValue(timeout, Protocol.REQUEST_TIMEOUT, - "request_timeout")), Field.REQUEST_TIMEOUT); + "request_timeout")), SqlRequestField.REQUEST_TIMEOUT); parser.declareString( (request, timeout) -> request.pageTimeout(TimeValue.parseTimeValue(timeout, Protocol.PAGE_TIMEOUT, "page_timeout")), - Field.PAGE_TIMEOUT); + SqlRequestField.PAGE_TIMEOUT); parser.declareObject(AbstractSqlQueryRequest::filter, - (p, c) -> AbstractQueryBuilder.parseInnerQueryBuilder(p), Field.FILTER); + (p, c) -> AbstractQueryBuilder.parseInnerQueryBuilder(p), SqlRequestField.FILTER); return parser; } @@ -252,7 +252,7 @@ public int hashCode() { return Objects.hash(super.hashCode(), query, timeZone, fetchSize, requestTimeout, pageTimeout, filter); } - public interface Field { + public interface SqlRequestField { ParseField QUERY = new ParseField("query"); ParseField CURSOR = new ParseField("cursor"); ParseField PARAMS = new ParseField("params"); 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 d6122d2b6633e..84b69ef4691ca 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 @@ -19,6 +19,9 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; +import static org.elasticsearch.xpack.sql.action.AbstractSqlQueryRequest.SqlRequestField.CURSOR; +import static org.elasticsearch.xpack.sql.action.AbstractSqlQueryRequest.SqlRequestField.MODE; +import static org.elasticsearch.xpack.sql.action.AbstractSqlQueryRequest.SqlRequestField.CLIENT_ID; /** * Request to clean all SQL resources associated with the cursor @@ -35,9 +38,9 @@ public class SqlClearCursorRequest extends AbstractSqlRequest { static { // "cursor" is required constructor parameter - PARSER.declareString(constructorArg(), AbstractSqlQueryRequest.Field.CURSOR); - PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), AbstractSqlQueryRequest.Field.MODE); - PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), AbstractSqlQueryRequest.Field.CLIENT_ID); + PARSER.declareString(constructorArg(), CURSOR); + PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), MODE); + PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), CLIENT_ID); } private String cursor; 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 88e4a22c50bdc..ed40f0110f482 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.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; @@ -25,6 +23,7 @@ import java.util.TimeZone; import static org.elasticsearch.action.ValidateActions.addValidationError; +import static org.elasticsearch.xpack.sql.action.AbstractSqlQueryRequest.SqlRequestField.CURSOR; /** * Request to perform an sql query @@ -32,13 +31,8 @@ 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 = ""; @@ -115,7 +109,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } public static SqlQueryRequest fromXContent(XContentParser parser) { - SqlQueryRequest request = PARSER.apply(parser, null); - return request; + 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 8555776dc8964..d88b0c6640c69 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.Objects; import static java.util.Collections.unmodifiableList; +import static org.elasticsearch.xpack.sql.action.AbstractSqlQueryRequest.SqlRequestField.CURSOR; /** * Response to perform an sql query @@ -158,7 +159,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endArray(); if (cursor.equals("") == false) { - builder.field(AbstractSqlQueryRequest.Field.CURSOR.getPreferredName(), cursor); + builder.field(CURSOR.getPreferredName(), cursor); } } return builder.endObject(); diff --git a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlQueryResponseTests.java b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlQueryResponseTests.java index 2833b4b1b38a8..1c4e5f42b32c0 100644 --- a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlQueryResponseTests.java +++ b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlQueryResponseTests.java @@ -26,6 +26,7 @@ import static org.elasticsearch.common.xcontent.ToXContent.EMPTY_PARAMS; import static org.hamcrest.Matchers.hasSize; +import static org.elasticsearch.xpack.sql.action.AbstractSqlQueryRequest.SqlRequestField.CURSOR; public class SqlQueryResponseTests extends AbstractStreamableXContentTestCase { @@ -109,7 +110,7 @@ public void testToXContent() throws IOException { } if (testInstance.cursor().equals("") == false) { - assertEquals(rootMap.get(AbstractSqlQueryRequest.Field.CURSOR.getPreferredName()), testInstance.cursor()); + assertEquals(rootMap.get(CURSOR.getPreferredName()), testInstance.cursor()); } } diff --git a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlRequestParsersTests.java b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlRequestParsersTests.java index 8f05be83b9820..defda7e9a1574 100644 --- a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlRequestParsersTests.java +++ b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlRequestParsersTests.java @@ -17,82 +17,62 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.function.Consumer; +import java.util.function.Function; import static org.hamcrest.Matchers.containsString; public class SqlRequestParsersTests extends ESTestCase { public void testUnknownFieldParsingErrors() throws IOException { - XContentParser parser = parser("{\"key\" : \"value\"}"); - Exception e = expectThrows(IllegalArgumentException.class, () -> SqlClearCursorRequest.fromXContent(parser)); - assertThat(e.getMessage(), containsString("unknown field [key]")); - - XContentParser parser2 = parser("{\"key\" : \"value\"}"); - e = expectThrows(IllegalArgumentException.class, () -> SqlQueryRequest.fromXContent(parser2)); - assertThat(e.getMessage(), containsString("unknown field [key]")); - - XContentParser parser3 = parser("{\"key\" : \"value\"}"); - e = expectThrows(IllegalArgumentException.class, () -> SqlTranslateRequest.fromXContent(parser3)); - assertThat(e.getMessage(), containsString("unknown field [key]")); + assertParsingErrorMessage("{\"key\" : \"value\"}", "unknown field [key]", SqlClearCursorRequest::fromXContent); + assertParsingErrorMessage("{\"key\" : \"value\"}", "unknown field [key]", SqlQueryRequest::fromXContent); + assertParsingErrorMessage("{\"key\" : \"value\"}", "unknown field [key]", SqlTranslateRequest::fromXContent); } public void testClearCursorRequestParser() throws IOException { - XContentParser parser = parser("{\"mode\" : \"jdbc\"}"); - Exception e = expectThrows(IllegalArgumentException.class, () -> SqlClearCursorRequest.fromXContent(parser)); - assertThat(e.getMessage(), containsString("Required [cursor]")); + assertParsingErrorMessage("{\"mode\" : \"jdbc\"}", "Required [cursor]", SqlClearCursorRequest::fromXContent); + assertParsingErrorMessage("{\"cursor\" : \"whatever\", \"fetch_size\":123}", "unknown field [fetch_size]", + SqlClearCursorRequest::fromXContent); - XContentParser parser2 = parser("{\"cursor\" : \"whatever\", \"fetch_size\":123}"); - e = expectThrows(IllegalArgumentException.class, () -> SqlClearCursorRequest.fromXContent(parser2)); - assertThat(e.getMessage(), containsString("unknown field [fetch_size]")); - - XContentParser parser3 = parser("{\"cursor\" : \"whatever\", \"mode\" : \"jdbc\", \"client.id\" : \"bla\"}"); - SqlClearCursorRequest request = SqlClearCursorRequest.fromXContent(parser3); + SqlClearCursorRequest request = generateRequest("{\"cursor\" : \"whatever\", \"mode\" : \"jdbc\", \"client.id\" : \"bla\"}", + SqlClearCursorRequest::fromXContent); assertEquals("bla", request.clientId()); assertEquals("jdbc", request.mode().toString()); assertEquals("whatever", request.getCursor()); - XContentParser parser4 = parser("{\"cursor\" : \"whatever\", \"mode\" : \"some foo mode\", \"client.id\" : \"bla\"}"); - request = SqlClearCursorRequest.fromXContent(parser4); + request = generateRequest("{\"cursor\" : \"whatever\", \"mode\" : \"some foo mode\", \"client.id\" : \"bla\"}", + SqlClearCursorRequest::fromXContent); assertEquals("bla", request.clientId()); assertEquals("plain", request.mode().toString()); assertEquals("whatever", request.getCursor()); + + request = generateRequest("{\"cursor\" : \"whatever\"}", SqlClearCursorRequest::fromXContent); + assertNull(request.clientId()); + assertEquals("plain", request.mode().toString()); + assertEquals("whatever", request.getCursor()); } public void testTranslateRequestParser() throws IOException { - XContentParser parser = parser("{\"qquery\" : \"select * from bla\"}"); - Exception e = expectThrows(IllegalArgumentException.class, () -> SqlTranslateRequest.fromXContent(parser)); - assertThat(e.getMessage(), containsString("unknown field [qquery]")); + assertParsingErrorMessage("{\"qquery\" : \"select * from bla\"}", "unknown field [qquery]", SqlTranslateRequest::fromXContent); - XContentParser parser3 = parser("{\"query\" : \"select * from foo\"}"); - SqlTranslateRequest request = SqlTranslateRequest.fromXContent(parser3); + SqlTranslateRequest request = generateRequest("{\"query\" : \"select * from foo\"}", SqlTranslateRequest::fromXContent); assertEquals("select * from foo", request.query()); } public void testQueryRequestParser() throws IOException { - XContentParser parser = parser("{\"mode\" : 123}"); - Exception e = expectThrows(IllegalArgumentException.class, () -> SqlQueryRequest.fromXContent(parser)); - assertThat(e.getMessage(), containsString("mode doesn't support values of type: VALUE_NUMBER")); - - XContentParser parser2 = parser("{\"cursor\" : \"whatever\", \"fetch_size\":\"abc\"}"); - e = expectThrows(IllegalArgumentException.class, () -> SqlQueryRequest.fromXContent(parser2)); - assertThat(e.getMessage(), containsString("failed to parse field [fetch_size]")); - - XContentParser parser3 = parser("{\"client.id\":123}"); - e = expectThrows(IllegalArgumentException.class, () -> SqlQueryRequest.fromXContent(parser3)); - assertThat(e.getMessage(), containsString("client.id doesn't support values of type: VALUE_NUMBER")); + assertParsingErrorMessage("{\"mode\" : 123}", "mode doesn't support values of type: VALUE_NUMBER", SqlQueryRequest::fromXContent); + assertParsingErrorMessage("{\"cursor\" : \"whatever\", \"fetch_size\":\"abc\"}", "failed to parse field [fetch_size]", + SqlQueryRequest::fromXContent); + assertParsingErrorMessage("{\"client.id\":123}", "client.id doesn't support values of type: VALUE_NUMBER", + SqlQueryRequest::fromXContent); + assertParsingErrorMessage("{\"params\":[{\"value\":123}]}", "failed to parse field [params]", SqlQueryRequest::fromXContent); + assertParsingErrorMessage("{\"time_zone\":12}", "time_zone doesn't support values of type: VALUE_NUMBER", + SqlQueryRequest::fromXContent); - XContentParser parser4 = parser("{\"params\":[{\"value\":123}]}"); - e = expectThrows(IllegalArgumentException.class, () -> SqlQueryRequest.fromXContent(parser4)); - assertThat(e.getMessage(), containsString("failed to parse field [params]")); - - XContentParser parser5 = parser("{\"time_zone\":12}"); - e = expectThrows(IllegalArgumentException.class, () -> SqlQueryRequest.fromXContent(parser5)); - assertThat(e.getMessage(), containsString("time_zone doesn't support values of type: VALUE_NUMBER")); - - XContentParser parser6 = parser("{\"cursor\" : \"whatever\", \"mode\" : \"jdbc\", \"client.id\" : \"bla\", \"query\":\"select\"," - + "\"params\":[{\"value\":123, \"type\":\"whatever\"}], \"time_zone\":\"UTC\", \"request_timeout\":\"5s\"," - + "\"page_timeout\":\"10s\"}"); - SqlQueryRequest request = SqlQueryRequest.fromXContent(parser6); + SqlQueryRequest request = generateRequest("{\"cursor\" : \"whatever\", \"mode\" : \"jdbc\", \"client.id\" : \"bla\"," + + "\"query\":\"select\",\"params\":[{\"value\":123, \"type\":\"whatever\"}], \"time_zone\":\"UTC\"," + + "\"request_timeout\":\"5s\",\"page_timeout\":\"10s\"}", SqlQueryRequest::fromXContent); assertEquals("bla", request.clientId()); assertEquals("jdbc", request.mode().toString()); assertEquals("whatever", request.cursor()); @@ -101,12 +81,23 @@ public void testQueryRequestParser() throws IOException { List 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 XContentParser parser(String content) throws IOException { XContentType xContentType = XContentType.JSON; return xContentType.xContent().createParser( diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/AbstractSqlAction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/AbstractSqlAction.java new file mode 100644 index 0000000000000..7c0e4e139c3e8 --- /dev/null +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/AbstractSqlAction.java @@ -0,0 +1,64 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.sql.plugin; + +import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.rest.BaseRestHandler; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.xpack.sql.action.AbstractSqlRequest; +import org.elasticsearch.xpack.sql.proto.Mode; +import org.elasticsearch.xpack.sql.proto.RequestInfo; + +import java.io.IOException; +import java.util.Locale; + +import static org.elasticsearch.xpack.sql.proto.RequestInfo.CANVAS; +import static org.elasticsearch.xpack.sql.proto.RequestInfo.CLI; + +public abstract class AbstractSqlAction extends BaseRestHandler { + + protected AbstractSqlAction(Settings settings) { + super(settings); + } + + @Override + public abstract String getName(); + + /* + * Parse the rest request and create a sql request out of it. + */ + protected abstract AbstractSqlRequest initializeSqlRequest(RestRequest request) throws IOException; + + /* + * Perform additional steps after the default initialization. + */ + protected abstract RestChannelConsumer doPrepareRequest(AbstractSqlRequest sqlRequest, RestRequest request, + NodeClient client) throws IOException; + + @Override + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + AbstractSqlRequest sqlRequest = initializeSqlRequest(request); + + // no mode specified, default to "PLAIN" + if (sqlRequest.requestInfo() == null) { + sqlRequest.requestInfo(new RequestInfo(Mode.PLAIN)); + } + + // default to no client id, unless it's CLI or CANVAS + if (sqlRequest.requestInfo().clientId() != null) { + String clientId = sqlRequest.requestInfo().clientId().toLowerCase(Locale.ROOT); + if (!clientId.equals(CLI) && !clientId.equals(CANVAS)) { + clientId = null; + } + sqlRequest.requestInfo().clientId(clientId); + } + + return doPrepareRequest(sqlRequest, request, client); + } + +} 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 b6a8592af185f..4525b7378551b 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 @@ -11,24 +11,19 @@ import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.RestToXContentListener; +import org.elasticsearch.xpack.sql.action.AbstractSqlRequest; 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 org.elasticsearch.xpack.sql.proto.RequestInfo; import java.io.IOException; -import java.util.Locale; 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 RestSqlClearCursorAction extends BaseRestHandler { +public class RestSqlClearCursorAction extends AbstractSqlAction { private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestSqlClearCursorAction.class)); @@ -39,25 +34,20 @@ public class RestSqlClearCursorAction extends BaseRestHandler { POST, Protocol.CLEAR_CURSOR_REST_ENDPOINT, this, POST, Protocol.CLEAR_CURSOR_DEPRECATED_REST_ENDPOINT, deprecationLogger); } - + @Override - protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + protected AbstractSqlRequest initializeSqlRequest(RestRequest request) throws IOException { SqlClearCursorRequest sqlRequest; - try (XContentParser parser = request.contentOrSourceParamParser()) { + try (XContentParser parser = request.contentParser()) { sqlRequest = SqlClearCursorRequest.fromXContent(parser); } - // no mode specified, default to "PLAIN" - if (sqlRequest.requestInfo() == null) { - sqlRequest.requestInfo(new RequestInfo(Mode.PLAIN)); - } - if (sqlRequest.requestInfo().clientId() != null) { - String clientId = sqlRequest.requestInfo().clientId().toLowerCase(Locale.ROOT); - if (!clientId.equals(CLI) && !clientId.equals(CANVAS)) { - clientId = null; - } - sqlRequest.requestInfo().clientId(clientId); - } + return sqlRequest; + } + + @Override + protected RestChannelConsumer doPrepareRequest(AbstractSqlRequest sqlRequest, RestRequest request, NodeClient client) + throws IOException { 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 7dcf1df08e9f0..898c74660d615 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 @@ -14,32 +14,27 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.BytesRestResponse; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestResponse; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.rest.action.RestResponseListener; +import org.elasticsearch.xpack.sql.action.AbstractSqlRequest; 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 { +public class RestSqlQueryAction extends AbstractSqlAction { private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestSqlQueryAction.class)); @@ -54,27 +49,20 @@ public class RestSqlQueryAction extends BaseRestHandler { POST, Protocol.SQL_QUERY_REST_ENDPOINT, this, POST, Protocol.SQL_QUERY_DEPRECATED_REST_ENDPOINT, deprecationLogger); } - + @Override - protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + protected AbstractSqlRequest initializeSqlRequest(RestRequest request) throws IOException { SqlQueryRequest sqlRequest; try (XContentParser parser = request.contentParser()) { sqlRequest = SqlQueryRequest.fromXContent(parser); } - // no mode specified, default to "PLAIN" - if (sqlRequest.requestInfo() == null) { - sqlRequest.requestInfo(new RequestInfo(Mode.PLAIN)); - } - - if (sqlRequest.requestInfo().clientId() != null) { - String clientId = sqlRequest.requestInfo().clientId().toLowerCase(Locale.ROOT); - if (!clientId.equals(CLI) && !clientId.equals(CANVAS)) { - clientId = null; - } - sqlRequest.requestInfo().clientId(clientId); - } + return sqlRequest; + } + @Override + protected RestChannelConsumer doPrepareRequest(AbstractSqlRequest sqlRequest, RestRequest request, NodeClient client) + throws IOException { /* * Since we support {@link TextFormat} and * {@link XContent} outputs we can't use {@link RestToXContentListener} @@ -125,7 +113,7 @@ public RestResponse buildResponse(SqlQueryResponse response) throws Exception { return channel -> client.execute(SqlQueryAction.INSTANCE, sqlRequest, new RestResponseListener(channel) { @Override public RestResponse buildResponse(SqlQueryResponse response) throws Exception { - Cursor cursor = Cursors.decodeFromString(sqlRequest.cursor()); + Cursor cursor = Cursors.decodeFromString(((SqlQueryRequest) sqlRequest).cursor()); final String data = textFormat.format(cursor, request, response); RestResponse restResponse = new BytesRestResponse(RestStatus.OK, textFormat.contentType(request), 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 ec055cc9a0509..7abbd88d8ed41 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 @@ -10,28 +10,23 @@ import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.RestToXContentListener; +import org.elasticsearch.xpack.sql.action.AbstractSqlRequest; 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 org.elasticsearch.xpack.sql.proto.RequestInfo; import java.io.IOException; -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; /** * REST action for translating SQL queries into ES requests */ -public class RestSqlTranslateAction extends BaseRestHandler { +public class RestSqlTranslateAction extends AbstractSqlAction { private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestSqlTranslateAction.class)); @@ -46,27 +41,20 @@ public RestSqlTranslateAction(Settings settings, RestController controller) { POST, Protocol.SQL_TRANSLATE_REST_ENDPOINT, this, POST, Protocol.SQL_TRANSLATE_DEPRECATED_REST_ENDPOINT, deprecationLogger); } - + @Override - protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + protected AbstractSqlRequest initializeSqlRequest(RestRequest request) throws IOException { SqlTranslateRequest sqlRequest; - try (XContentParser parser = request.contentOrSourceParamParser()) { + try (XContentParser parser = request.contentParser()) { sqlRequest = SqlTranslateRequest.fromXContent(parser); } - // no mode specified, default to "PLAIN" - if (sqlRequest.requestInfo() == null) { - sqlRequest.requestInfo(new RequestInfo(Mode.PLAIN)); - } - - if (sqlRequest.requestInfo().clientId() != null) { - String clientId = sqlRequest.requestInfo().clientId().toLowerCase(Locale.ROOT); - if (!clientId.equals(CLI) && !clientId.equals(CANVAS)) { - clientId = null; - } - sqlRequest.requestInfo().clientId(clientId); - } - + return sqlRequest; + } + + @Override + protected RestChannelConsumer doPrepareRequest(AbstractSqlRequest sqlRequest, RestRequest request, NodeClient client) + throws IOException { 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/SqlField.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlField.java deleted file mode 100644 index 58c25c101aebe..0000000000000 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlField.java +++ /dev/null @@ -1,16 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -package org.elasticsearch.xpack.sql.plugin; - -import org.elasticsearch.common.ParseField; - -final class SqlField { - public static final ParseField MODE = new ParseField("mode"); - public static final ParseField CLIENT_ID = new ParseField("client.id"); - - private SqlField() {} -}