diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeParser.java index 62a3f3fd915d0..ce310a5b3f8e4 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeParser.java @@ -22,18 +22,33 @@ import java.util.HashMap; import java.util.Locale; import java.util.Map; -import java.util.regex.Pattern; public class MediaTypeParser { private final Map formatToMediaType; private final Map typeWithSubtypeToMediaType; - private final Map> parametersMap; - public MediaTypeParser(Map formatToMediaType, Map typeWithSubtypeToMediaType, - Map> parametersMap) { - this.formatToMediaType = Map.copyOf(formatToMediaType); - this.typeWithSubtypeToMediaType = Map.copyOf(typeWithSubtypeToMediaType); - this.parametersMap = Map.copyOf(parametersMap); + public MediaTypeParser(T[] acceptedMediaTypes) { + this(acceptedMediaTypes, Map.of()); + } + + public MediaTypeParser(T[] acceptedMediaTypes, Map additionalMediaTypes) { + final int size = acceptedMediaTypes.length + additionalMediaTypes.size(); + Map formatMap = new HashMap<>(size); + Map typeMap = new HashMap<>(size); + for (T mediaType : acceptedMediaTypes) { + typeMap.put(mediaType.typeWithSubtype(), mediaType); + formatMap.put(mediaType.format(), mediaType); + } + for (Map.Entry entry : additionalMediaTypes.entrySet()) { + String typeWithSubtype = entry.getKey(); + T mediaType = entry.getValue(); + + typeMap.put(typeWithSubtype.toLowerCase(Locale.ROOT), mediaType); + formatMap.put(mediaType.format(), mediaType); + } + + this.formatToMediaType = Map.copyOf(formatMap); + this.typeWithSubtypeToMediaType = Map.copyOf(typeMap); } public T fromMediaType(String mediaType) { @@ -50,7 +65,6 @@ public T fromFormat(String format) { /** * parsing media type that follows https://tools.ietf.org/html/rfc7231#section-3.1.1.1 - * * @param headerValue a header value from Accept or Content-Type * @return a parsed media-type */ @@ -61,11 +75,9 @@ public ParsedMediaType parseMediaType(String headerValue) { String[] typeSubtype = split[0].trim().toLowerCase(Locale.ROOT) .split("/"); if (typeSubtype.length == 2) { - String type = typeSubtype[0]; String subtype = typeSubtype[1]; - String typeWithSubtype = type + "/" + subtype; - T xContentType = typeWithSubtypeToMediaType.get(typeWithSubtype); + T xContentType = typeWithSubtypeToMediaType.get(type + "/" + subtype); if (xContentType != null) { Map parameters = new HashMap<>(); for (int i = 1; i < split.length; i++) { @@ -74,12 +86,7 @@ public ParsedMediaType parseMediaType(String headerValue) { if (keyValueParam.length != 2 || hasSpaces(keyValueParam[0]) || hasSpaces(keyValueParam[1])) { return null; } - String parameterName = keyValueParam[0].toLowerCase(Locale.ROOT); - String parameterValue = keyValueParam[1].toLowerCase(Locale.ROOT); - if (isValidParameter(typeWithSubtype, parameterName, parameterValue) == false) { - return null; - } - parameters.put(parameterName, parameterValue); + parameters.put(keyValueParam[0].toLowerCase(Locale.ROOT), keyValueParam[1].toLowerCase(Locale.ROOT)); } return new ParsedMediaType(xContentType, parameters); } @@ -89,17 +96,6 @@ public ParsedMediaType parseMediaType(String headerValue) { return null; } - private boolean isValidParameter(String typeWithSubtype, String parameterName, String parameterValue) { - if (parametersMap.containsKey(typeWithSubtype)) { - Map parameters = parametersMap.get(typeWithSubtype); - if (parameters.containsKey(parameterName)) { - Pattern regex = parameters.get(parameterName); - return regex.matcher(parameterValue).matches(); - } - } - return false; - } - private boolean hasSpaces(String s) { return s.trim().equals(s) == false; } @@ -124,37 +120,4 @@ public Map getParameters() { return parameters; } } - - public static class Builder { - private final Map formatToMediaType = new HashMap<>(); - private final Map typeWithSubtypeToMediaType = new HashMap<>(); - private final Map> parametersMap = new HashMap<>(); - - public Builder withMediaTypeAndParams(String alternativeMediaType, T mediaType, Map paramNameAndValueRegex) { - typeWithSubtypeToMediaType.put(alternativeMediaType.toLowerCase(Locale.ROOT), mediaType); - formatToMediaType.put(mediaType.format(), mediaType); - - Map parametersForMediaType = new HashMap<>(paramNameAndValueRegex.size()); - for (Map.Entry params : paramNameAndValueRegex.entrySet()) { - String parameterName = params.getKey().toLowerCase(Locale.ROOT); - String parameterRegex = params.getValue(); - Pattern pattern = Pattern.compile(parameterRegex, Pattern.CASE_INSENSITIVE); - parametersForMediaType.put(parameterName, pattern); - } - parametersMap.put(alternativeMediaType, parametersForMediaType); - - return this; - } - - public Builder copyFromMediaTypeParser(MediaTypeParser mediaTypeParser) { - formatToMediaType.putAll(mediaTypeParser.formatToMediaType); - typeWithSubtypeToMediaType.putAll(mediaTypeParser.typeWithSubtypeToMediaType); - parametersMap.putAll(mediaTypeParser.parametersMap); - return this; - } - - public MediaTypeParser build() { - return new MediaTypeParser<>(formatToMediaType, typeWithSubtypeToMediaType, parametersMap); - } - } } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentType.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentType.java index 076a20bad006a..5801a3cd76def 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentType.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentType.java @@ -24,7 +24,6 @@ import org.elasticsearch.common.xcontent.smile.SmileXContent; import org.elasticsearch.common.xcontent.yaml.YamlXContent; -import java.util.Collections; import java.util.Map; /** @@ -114,26 +113,9 @@ public XContent xContent() { } }; - private static final String COMPATIBLE_WITH_PARAMETER_NAME = "compatible-with"; - private static final String VERSION_PATTERN = "\\d+"; - public static final MediaTypeParser mediaTypeParser = new MediaTypeParser.Builder() - .withMediaTypeAndParams("application/smile", SMILE, Collections.emptyMap()) - .withMediaTypeAndParams("application/cbor", CBOR, Collections.emptyMap()) - .withMediaTypeAndParams("application/json", JSON, Map.of("charset", "UTF-8")) - .withMediaTypeAndParams("application/yaml", YAML, Map.of("charset", "UTF-8")) - .withMediaTypeAndParams("application/*", JSON, Map.of("charset", "UTF-8")) - .withMediaTypeAndParams("application/x-ndjson", JSON, Map.of("charset", "UTF-8")) - .withMediaTypeAndParams("application/vnd.elasticsearch+json", JSON, - Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) - .withMediaTypeAndParams("application/vnd.elasticsearch+smile", SMILE, - Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) - .withMediaTypeAndParams("application/vnd.elasticsearch+yaml", YAML, - Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) - .withMediaTypeAndParams("application/vnd.elasticsearch+cbor", CBOR, - Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) - .withMediaTypeAndParams("application/vnd.elasticsearch+x-ndjson", JSON, - Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) - .build(); + public static final MediaTypeParser mediaTypeParser = new MediaTypeParser<>(XContentType.values(), + Map.of("application/*", JSON, "application/x-ndjson", JSON)); + /** * Accepts a format string, which is most of the time is equivalent to {@link XContentType#subtype()} @@ -155,23 +137,13 @@ public static XContentType fromMediaType(String mediaTypeHeaderValue) { return mediaTypeParser.fromMediaType(mediaTypeHeaderValue); } + private int index; XContentType(int index) { this.index = index; } - public static Byte parseVersion(String mediaType) { - MediaTypeParser.ParsedMediaType parsedMediaType = mediaTypeParser.parseMediaType(mediaType); - if (parsedMediaType != null) { - String version = parsedMediaType - .getParameters() - .get(COMPATIBLE_WITH_PARAMETER_NAME); - return version != null ? Byte.parseByte(version) : null; - } - return null; - } - public int index() { return index; } diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/MediaTypeParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/MediaTypeParserTests.java index 08ca08a3d2240..6514c7ec73306 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/MediaTypeParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/MediaTypeParserTests.java @@ -29,34 +29,29 @@ import static org.hamcrest.Matchers.nullValue; public class MediaTypeParserTests extends ESTestCase { - - MediaTypeParser mediaTypeParser = new MediaTypeParser.Builder() - .withMediaTypeAndParams("application/vnd.elasticsearch+json", - XContentType.JSON, Map.of("compatible-with", "\\d+", - "charset", "UTF-8")) - .build(); + MediaTypeParser mediaTypeParser = XContentType.mediaTypeParser; public void testJsonWithParameters() throws Exception { - String mediaType = "application/vnd.elasticsearch+json"; + String mediaType = "application/json"; assertThat(mediaTypeParser.parseMediaType(mediaType).getParameters(), equalTo(Collections.emptyMap())); assertThat(mediaTypeParser.parseMediaType(mediaType + ";").getParameters(), equalTo(Collections.emptyMap())); assertThat(mediaTypeParser.parseMediaType(mediaType + "; charset=UTF-8").getParameters(), equalTo(Map.of("charset", "utf-8"))); - assertThat(mediaTypeParser.parseMediaType(mediaType + "; compatible-with=123;charset=UTF-8").getParameters(), - equalTo(Map.of("charset", "utf-8", "compatible-with", "123"))); + assertThat(mediaTypeParser.parseMediaType(mediaType + "; custom=123;charset=UTF-8").getParameters(), + equalTo(Map.of("charset", "utf-8", "custom", "123"))); } public void testWhiteSpaceInTypeSubtype() { - String mediaType = " application/vnd.elasticsearch+json "; + String mediaType = " application/json "; assertThat(mediaTypeParser.parseMediaType(mediaType).getMediaType(), equalTo(XContentType.JSON)); - assertThat(mediaTypeParser.parseMediaType(mediaType + "; compatible-with=123; charset=UTF-8").getParameters(), - equalTo(Map.of("charset", "utf-8", "compatible-with", "123"))); - assertThat(mediaTypeParser.parseMediaType(mediaType + "; compatible-with=123;\n charset=UTF-8").getParameters(), - equalTo(Map.of("charset", "utf-8", "compatible-with", "123"))); + assertThat(mediaTypeParser.parseMediaType(mediaType + "; custom=123; charset=UTF-8").getParameters(), + equalTo(Map.of("charset", "utf-8", "custom", "123"))); + assertThat(mediaTypeParser.parseMediaType(mediaType + "; custom=123;\n charset=UTF-8").getParameters(), + equalTo(Map.of("charset", "utf-8", "custom", "123"))); mediaType = " application / json "; assertThat(mediaTypeParser.parseMediaType(mediaType), @@ -64,11 +59,10 @@ public void testWhiteSpaceInTypeSubtype() { } public void testInvalidParameters() { - String mediaType = "application/vnd.elasticsearch+json"; - assertThat(mediaTypeParser.parseMediaType(mediaType + "; charset=unknown") , - is(nullValue())); + String mediaType = "application/json"; assertThat(mediaTypeParser.parseMediaType(mediaType + "; keyvalueNoEqualsSign"), is(nullValue())); + assertThat(mediaTypeParser.parseMediaType(mediaType + "; key = value"), is(nullValue())); assertThat(mediaTypeParser.parseMediaType(mediaType + "; key=") , diff --git a/server/src/main/java/org/elasticsearch/ElasticsearchException.java b/server/src/main/java/org/elasticsearch/ElasticsearchException.java index b8a04cccdf13b..a73b05e5550f2 100644 --- a/server/src/main/java/org/elasticsearch/ElasticsearchException.java +++ b/server/src/main/java/org/elasticsearch/ElasticsearchException.java @@ -631,7 +631,7 @@ public static ElasticsearchException[] guessRootCauses(Throwable t) { * parsing exception because that is generally the most interesting * exception to return to the user. If that exception is caused by * an ElasticsearchException we'd like to keep unwrapping because - * ElasticsearchExceptions tend to contain useful information for + * ElasticserachExceptions tend to contain useful information for * the user. */ Throwable cause = ex.getCause(); diff --git a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java index 6f5aa618ae4d0..c5bf1efa2cdb1 100644 --- a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java +++ b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java @@ -18,8 +18,6 @@ */ package org.elasticsearch.rest; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.Streams; @@ -38,7 +36,6 @@ public abstract class AbstractRestChannel implements RestChannel { - private static final Logger logger = LogManager.getLogger(AbstractRestChannel.class); private static final Predicate INCLUDE_FILTER = f -> f.charAt(0) != '-'; private static final Predicate EXCLUDE_FILTER = INCLUDE_FILTER.negate(); @@ -101,9 +98,8 @@ public XContentBuilder newBuilder(@Nullable XContentType requestContentType, boo public XContentBuilder newBuilder(@Nullable XContentType requestContentType, @Nullable XContentType responseContentType, boolean useFiltering) throws IOException { if (responseContentType == null) { - if (Strings.hasText(format)) { - responseContentType = XContentType.fromFormat(format); - } + //TODO PG shoudld format vs acceptHeader be always the same, do we allow overriding? + responseContentType = XContentType.fromFormat(format); if (responseContentType == null) { responseContentType = XContentType.fromMediaType(acceptHeader); } diff --git a/server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java b/server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java index 028b3a5f8e693..ba762f57c7ba1 100644 --- a/server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java +++ b/server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java @@ -50,7 +50,7 @@ public class BytesRestResponse extends RestResponse { private final RestStatus status; private final BytesReference content; - private final String responseMediaType; + private final String contentType; /** * Creates a new response based on {@link XContentBuilder}. @@ -69,24 +69,24 @@ public BytesRestResponse(RestStatus status, String content) { /** * Creates a new plain text response. */ - public BytesRestResponse(RestStatus status, String responseMediaType, String content) { - this(status, responseMediaType, new BytesArray(content)); + public BytesRestResponse(RestStatus status, String contentType, String content) { + this(status, contentType, new BytesArray(content)); } /** * Creates a binary response. */ - public BytesRestResponse(RestStatus status, String responseMediaType, byte[] content) { - this(status, responseMediaType, new BytesArray(content)); + public BytesRestResponse(RestStatus status, String contentType, byte[] content) { + this(status, contentType, new BytesArray(content)); } /** * Creates a binary response. */ - public BytesRestResponse(RestStatus status, String responseMediaType, BytesReference content) { + public BytesRestResponse(RestStatus status, String contentType, BytesReference content) { this.status = status; this.content = content; - this.responseMediaType = responseMediaType; + this.contentType = contentType; } public BytesRestResponse(RestChannel channel, Exception e) throws IOException { @@ -109,7 +109,7 @@ public BytesRestResponse(RestChannel channel, RestStatus status, Exception e) th try (XContentBuilder builder = channel.newErrorBuilder()) { build(builder, params, status, channel.detailedErrorsEnabled(), e); this.content = BytesReference.bytes(builder); - this.responseMediaType = builder.contentType().mediaType(); + this.contentType = builder.contentType().mediaType(); } if (e instanceof ElasticsearchException) { copyHeaders(((ElasticsearchException) e)); @@ -118,7 +118,7 @@ public BytesRestResponse(RestChannel channel, RestStatus status, Exception e) th @Override public String contentType() { - return this.responseMediaType; + return this.contentType; } @Override diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestTable.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestTable.java index bbb746baafe92..fa4bd586a4723 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestTable.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestTable.java @@ -51,14 +51,14 @@ public class RestTable { public static RestResponse buildResponse(Table table, RestChannel channel) throws Exception { RestRequest request = channel.request(); - XContentType xContentType = getXContentType(request); + XContentType xContentType = getxContentType(request); if (xContentType != null) { return buildXContentBuilder(table, channel); } return buildTextPlainResponse(table, channel); } - private static XContentType getXContentType(RestRequest request) { + private static XContentType getxContentType(RestRequest request) { if (request.hasParam("format")) { return XContentType.fromFormat(request.param("format")); } diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/XContentTypeTests.java b/server/src/test/java/org/elasticsearch/common/xcontent/XContentTypeTests.java index 7b18eaa2bad38..d6f9fb5e9ad5e 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/XContentTypeTests.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/XContentTypeTests.java @@ -23,7 +23,6 @@ import java.util.Locale; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; public class XContentTypeTests extends ESTestCase { @@ -34,7 +33,6 @@ public void testFromJson() throws Exception { assertThat(XContentType.fromMediaType(mediaType), equalTo(expectedXContentType)); assertThat(XContentType.fromMediaType(mediaType + ";"), equalTo(expectedXContentType)); assertThat(XContentType.fromMediaType(mediaType + "; charset=UTF-8"), equalTo(expectedXContentType)); - assertThat(XContentType.fromMediaType(mediaType + "; charset=utf-8"), equalTo(expectedXContentType)); } public void testFromNdJson() throws Exception { @@ -95,65 +93,4 @@ public void testFromRubbish() throws Exception { assertThat(XContentType.fromMediaType("text/plain"), nullValue()); assertThat(XContentType.fromMediaType("gobbly;goop"), nullValue()); } - - public void testVersionedMediaType() { - String version = String.valueOf(randomNonNegativeByte()); - assertThat(XContentType.fromMediaType("application/vnd.elasticsearch+json;compatible-with=" + version), - equalTo(XContentType.JSON)); - assertThat(XContentType.fromMediaType("application/vnd.elasticsearch+cbor;compatible-with=" + version), - equalTo(XContentType.CBOR)); - assertThat(XContentType.fromMediaType("application/vnd.elasticsearch+smile;compatible-with=" + version), - equalTo(XContentType.SMILE)); - assertThat(XContentType.fromMediaType("application/vnd.elasticsearch+yaml;compatible-with=" + version), - equalTo(XContentType.YAML)); - assertThat(XContentType.fromMediaType("application/json"), - equalTo(XContentType.JSON)); - assertThat(XContentType.fromMediaType("application/vnd.elasticsearch+x-ndjson;compatible-with=" + version), - equalTo(XContentType.JSON)); - - - assertThat(XContentType.fromMediaType("APPLICATION/VND.ELASTICSEARCH+JSON;COMPATIBLE-WITH=" + version), - equalTo(XContentType.JSON)); - assertThat(XContentType.fromMediaType("APPLICATION/JSON"), - equalTo(XContentType.JSON)); - } - - public void testVersionParsing() { - byte version = randomNonNegativeByte(); - assertThat(XContentType.parseVersion("application/vnd.elasticsearch+json;compatible-with=" + version), - equalTo(version)); - assertThat(XContentType.parseVersion("application/vnd.elasticsearch+cbor;compatible-with=" + version), - equalTo(version)); - assertThat(XContentType.parseVersion("application/vnd.elasticsearch+smile;compatible-with=" + version), - equalTo(version)); - assertThat(XContentType.parseVersion("application/vnd.elasticsearch+x-ndjson;compatible-with=" + version), - equalTo(version)); - assertThat(XContentType.parseVersion("application/json"), - nullValue()); - - - assertThat(XContentType.parseVersion("APPLICATION/VND.ELASTICSEARCH+JSON;COMPATIBLE-WITH=" + version), - equalTo(version)); - assertThat(XContentType.parseVersion("APPLICATION/JSON"), - nullValue()); - - assertThat(XContentType.parseVersion("application/json;compatible-with=" + version + ".0"), - is(nullValue())); - } - - public void testUnrecognizedParameter() { - assertThat(XContentType.parseVersion("application/json; sth=123"), - is(nullValue())); } - - public void testMediaTypeWithoutESSubtype() { - String version = String.valueOf(randomNonNegativeByte()); - assertThat(XContentType.fromMediaType("application/json;compatible-with=" + version), nullValue()); - } - - public void testAnchoring() { - String version = String.valueOf(randomNonNegativeByte()); - assertThat(XContentType.fromMediaType("sth_application/json;compatible-with=" + version + ".0"), nullValue()); - assertThat(XContentType.fromMediaType("sth_application/json;compatible-with=" + version + "_sth"), nullValue()); - assertThat(XContentType.fromMediaType("application/json;compatible-with=" + version + "_sth"), nullValue()); - } } 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 763e460170cf0..4ade958b60e85 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 @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.sql.plugin; import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.common.xcontent.MediaType; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; @@ -20,6 +19,7 @@ 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 java.io.IOException; @@ -31,11 +31,11 @@ import static org.elasticsearch.rest.RestRequest.Method.GET; import static org.elasticsearch.rest.RestRequest.Method.POST; import static org.elasticsearch.xpack.sql.proto.Protocol.URL_PARAM_DELIMITER; +import static org.elasticsearch.xpack.sql.proto.Protocol.URL_PARAM_FORMAT; public class RestSqlQueryAction extends BaseRestHandler { - private final SqlMediaTypeParser sqlMediaTypeParser = new SqlMediaTypeParser(); - MediaType responseMediaType; + TextFormat textFormat; @Override public List routes() { @@ -52,7 +52,53 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli sqlRequest = SqlQueryRequest.fromXContent(parser); } - responseMediaType = sqlMediaTypeParser.getMediaType(request, sqlRequest); + /* + * Since we support {@link TextFormat} and + * {@link XContent} outputs we can't use {@link RestToXContentListener} + * like everything else. We want to stick as closely as possible to + * Elasticsearch's defaults though, while still layering in ways to + * control the output more easily. + * + * First we find the string that the user used to specify the response + * format. If there is a {@code format} parameter we use that. If there + * isn't but there is a {@code Accept} header then we use that. If there + * isn't then we use the {@code Content-Type} header which is required. + */ + String accept; + + if (Mode.isDedicatedClient(sqlRequest.requestInfo().mode()) + && (sqlRequest.binaryCommunication() == null || sqlRequest.binaryCommunication())) { + // enforce CBOR response for drivers and CLI (unless instructed differently through the config param) + accept = XContentType.CBOR.name(); + } else { + accept = request.param(URL_PARAM_FORMAT); + } + if (accept == null) { + accept = request.header("Accept"); + if ("*/*".equals(accept)) { + // */* means "I don't care" which we should treat like not specifying the header + accept = null; + } + } + if (accept == null) { + accept = request.header("Content-Type"); + } + assert accept != null : "The Content-Type header is required"; + + /* + * Second, we pick the actual content type to use by first parsing the + * string from the previous step as an {@linkplain XContent} value. If + * that doesn't parse we parse it as a {@linkplain TextFormat} value. If + * that doesn't parse it'll throw an {@link IllegalArgumentException} + * which we turn into a 400 error. + */ + XContentType xContentType = getXContentType(accept); + textFormat = xContentType == null ? TextFormat.fromMediaTypeOrFormat(accept) : null; + + if (xContentType == null && sqlRequest.columnar()) { + throw new IllegalArgumentException("Invalid use of [columnar] argument: cannot be used in combination with " + + "txt, csv or tsv formats"); + } long startNanos = System.nanoTime(); return channel -> client.execute(SqlQueryAction.INSTANCE, sqlRequest, new RestResponseListener(channel) { @@ -61,16 +107,16 @@ public RestResponse buildResponse(SqlQueryResponse response) throws Exception { RestResponse restResponse; // XContent branch - if (responseMediaType != null && responseMediaType instanceof XContentType) { - XContentType type = (XContentType) responseMediaType; - XContentBuilder builder = channel.newBuilder(request.getXContentType(), type, true); + if (xContentType != null) { + XContentBuilder builder = channel.newBuilder(request.getXContentType(), xContentType, true); response.toXContent(builder, request); restResponse = new BytesRestResponse(RestStatus.OK, builder); - } else { // TextFormat - TextFormat type = (TextFormat)responseMediaType; - final String data = type.format(request, response); + } + // TextFormat + else { + final String data = textFormat.format(request, response); - restResponse = new BytesRestResponse(RestStatus.OK, type.contentType(request), + restResponse = new BytesRestResponse(RestStatus.OK, textFormat.contentType(request), data.getBytes(StandardCharsets.UTF_8)); if (response.hasCursor()) { @@ -84,12 +130,17 @@ public RestResponse buildResponse(SqlQueryResponse response) throws Exception { }); } - - + private XContentType getXContentType(String accept) { + if (accept == null) { + return XContentType.JSON; + } + XContentType xContentType = XContentType.fromFormat(accept); + return xContentType != null ? xContentType : XContentType.fromMediaType(accept); + } @Override protected Set responseParams() { - return responseMediaType == TextFormat.CSV ? Collections.singleton(URL_PARAM_DELIMITER) : Collections.emptySet(); + return textFormat == TextFormat.CSV ? Collections.singleton(URL_PARAM_DELIMITER) : Collections.emptySet(); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParser.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParser.java deleted file mode 100644 index 189dc137b654c..0000000000000 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParser.java +++ /dev/null @@ -1,74 +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.xcontent.MediaType; -import org.elasticsearch.common.xcontent.MediaTypeParser; -import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.xpack.sql.action.SqlQueryRequest; -import org.elasticsearch.xpack.sql.proto.Mode; - -import java.util.Map; - -import static org.elasticsearch.xpack.sql.proto.Protocol.URL_PARAM_FORMAT; - -public class SqlMediaTypeParser { - private static final MediaTypeParser parser = new MediaTypeParser.Builder<>() - .copyFromMediaTypeParser(XContentType.mediaTypeParser) - .withMediaTypeAndParams(TextFormat.PLAIN_TEXT.typeWithSubtype(), TextFormat.PLAIN_TEXT, - Map.of("header", "present|absent", "charset", "utf-8")) - .withMediaTypeAndParams(TextFormat.CSV.typeWithSubtype(), TextFormat.CSV, - Map.of("header", "present|absent", "charset", "utf-8", - "delimiter", ".+"))// more detailed parsing is in TextFormat.CSV#delimiter - .withMediaTypeAndParams(TextFormat.TSV.typeWithSubtype(), TextFormat.TSV, - Map.of("header", "present|absent", "charset", "utf-8")) - .build(); - - /* - * Since we support {@link TextFormat} and - * {@link XContent} outputs we can't use {@link RestToXContentListener} - * like everything else. We want to stick as closely as possible to - * Elasticsearch's defaults though, while still layering in ways to - * control the output more easily. - * - * First we find the string that the user used to specify the response - * format. If there is a {@code format} parameter we use that. If there - * isn't but there is a {@code Accept} header then we use that. If there - * isn't then we use the {@code Content-Type} header which is required. - */ - public MediaType getMediaType(RestRequest request, SqlQueryRequest sqlRequest) { - - if (Mode.isDedicatedClient(sqlRequest.requestInfo().mode()) - && (sqlRequest.binaryCommunication() == null || sqlRequest.binaryCommunication())) { - // enforce CBOR response for drivers and CLI (unless instructed differently through the config param) - return XContentType.CBOR; - } else if (request.hasParam(URL_PARAM_FORMAT)) { - return validateColumnarRequest(sqlRequest.columnar(), parser.fromFormat(request.param(URL_PARAM_FORMAT))); - } - if (request.getHeaders().containsKey("Accept")) { - String accept = request.header("Accept"); - // */* means "I don't care" which we should treat like not specifying the header - if ("*/*".equals(accept) == false) { - return validateColumnarRequest(sqlRequest.columnar(), parser.fromMediaType(accept)); - } - } - - String contentType = request.header("Content-Type"); - assert contentType != null : "The Content-Type header is required"; - return validateColumnarRequest(sqlRequest.columnar(), parser.fromMediaType(contentType)); - } - - private static MediaType validateColumnarRequest(boolean requestIsColumnar, MediaType fromMediaType) { - if(requestIsColumnar && fromMediaType instanceof TextFormat){ - throw new IllegalArgumentException("Invalid use of [columnar] argument: cannot be used in combination with " - + "txt, csv or tsv formats"); - } - return fromMediaType; - } - -} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java index d397d2316959c..1f50c5fa7ea28 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java @@ -8,6 +8,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.xcontent.MediaType; +import org.elasticsearch.common.xcontent.MediaTypeParser; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.xpack.ql.util.StringUtils; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; @@ -293,6 +294,8 @@ public String subtype() { private static final String PARAM_HEADER_ABSENT = "absent"; private static final String PARAM_HEADER_PRESENT = "present"; + private static final MediaTypeParser parser = new MediaTypeParser<>(TextFormat.values()); + String format(RestRequest request, SqlQueryResponse response) { StringBuilder sb = new StringBuilder(); @@ -313,6 +316,18 @@ boolean hasHeader(RestRequest request) { return true; } + static TextFormat fromMediaTypeOrFormat(String accept) { + TextFormat textFormat = parser.fromFormat(accept); + if (textFormat != null) { + return textFormat; + } + textFormat = parser.fromMediaType(accept); + if (textFormat != null) { + return textFormat; + } + throw new IllegalArgumentException("invalid format [" + accept + "]"); + } + /** * Formal IANA mime type. */ diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParserTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParserTests.java deleted file mode 100644 index 0459b777b15f9..0000000000000 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParserTests.java +++ /dev/null @@ -1,93 +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.unit.TimeValue; -import org.elasticsearch.common.xcontent.MediaType; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.rest.FakeRestRequest; -import org.elasticsearch.xpack.sql.action.SqlQueryRequest; -import org.elasticsearch.xpack.sql.proto.Mode; -import org.elasticsearch.xpack.sql.proto.RequestInfo; - -import java.util.Collections; -import java.util.Map; - -import static org.elasticsearch.xpack.sql.plugin.TextFormat.CSV; -import static org.elasticsearch.xpack.sql.plugin.TextFormat.PLAIN_TEXT; -import static org.elasticsearch.xpack.sql.plugin.TextFormat.TSV; -import static org.elasticsearch.xpack.sql.proto.RequestInfo.CLIENT_IDS; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.CoreMatchers.nullValue; - -public class SqlMediaTypeParserTests extends ESTestCase { - SqlMediaTypeParser parser = new SqlMediaTypeParser(); - - public void testPlainTextDetection() { - MediaType text = parser.getMediaType(reqWithAccept("text/plain"), createTestInstance(false, Mode.PLAIN, false)); - assertThat(text, is(PLAIN_TEXT)); - } - - public void testCsvDetection() { - MediaType text = parser.getMediaType(reqWithAccept("text/csv"), createTestInstance(false, Mode.PLAIN, false)); - assertThat(text, is(CSV)); - - text = parser.getMediaType(reqWithAccept("text/csv; delimiter=x"), createTestInstance(false, Mode.PLAIN, false)); - assertThat(text, is(CSV)); - } - - public void testTsvDetection() { - MediaType text = parser.getMediaType(reqWithAccept("text/tab-separated-values"), createTestInstance(false, Mode.PLAIN, false)); - assertThat(text, is(TSV)); - } - - public void testMediaTypeDetectionWithParameters() { - assertThat(parser.getMediaType(reqWithAccept("text/plain; charset=utf-8"), - createTestInstance(false, Mode.PLAIN, false)), is(PLAIN_TEXT)); - assertThat(parser.getMediaType(reqWithAccept("text/plain; header=present"), - createTestInstance(false, Mode.PLAIN, false)), is(PLAIN_TEXT)); - assertThat(parser.getMediaType(reqWithAccept("text/plain; charset=utf-8; header=present"), - createTestInstance(false, Mode.PLAIN, false)), is(PLAIN_TEXT)); - - assertThat(parser.getMediaType(reqWithAccept("text/csv; charset=utf-8"), - createTestInstance(false, Mode.PLAIN, false)), is(CSV)); - assertThat(parser.getMediaType(reqWithAccept("text/csv; header=present"), - createTestInstance(false, Mode.PLAIN, false)), is(CSV)); - assertThat(parser.getMediaType(reqWithAccept("text/csv; charset=utf-8; header=present"), - createTestInstance(false, Mode.PLAIN, false)), is(CSV)); - - assertThat(parser.getMediaType(reqWithAccept("text/tab-separated-values; charset=utf-8"), - createTestInstance(false, Mode.PLAIN, false)), is(TSV)); - assertThat(parser.getMediaType(reqWithAccept("text/tab-separated-values; header=present"), - createTestInstance(false, Mode.PLAIN, false)), is(TSV)); - assertThat(parser.getMediaType(reqWithAccept("text/tab-separated-values; charset=utf-8; header=present"), - createTestInstance(false, Mode.PLAIN, false)), is(TSV)); - } - - public void testInvalidFormat() { - MediaType mediaType = parser.getMediaType(reqWithAccept("text/garbage"), createTestInstance(false, Mode.PLAIN, false)); - assertThat(mediaType, is(nullValue())); - } - - private static RestRequest reqWithAccept(String acceptHeader) { - - return new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) - .withHeaders(Map.of("Content-Type", Collections.singletonList("application/json"), - "Accept", Collections.singletonList(acceptHeader))) - .build(); - } - - protected SqlQueryRequest createTestInstance(boolean binaryCommunication, Mode mode, boolean columnar) { - return new SqlQueryRequest(randomAlphaOfLength(10), Collections.emptyList(), null, - randomZone(), between(1, Integer.MAX_VALUE), TimeValue.parseTimeValue(randomTimeValue(), null, "test"), - TimeValue.parseTimeValue(randomTimeValue(), null, "test"), columnar, randomAlphaOfLength(10), - new RequestInfo(mode, randomFrom(randomFrom(CLIENT_IDS), randomAlphaOfLengthBetween(10, 20))), - randomBoolean(), randomBoolean()).binaryCommunication(binaryCommunication); - } -} diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/TextFormatTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/TextFormatTests.java index 09d916af5cbc2..a53fd8f94ad59 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/TextFormatTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plugin/TextFormatTests.java @@ -26,9 +26,30 @@ import static java.util.Collections.singletonMap; import static org.elasticsearch.xpack.sql.plugin.TextFormat.CSV; import static org.elasticsearch.xpack.sql.plugin.TextFormat.TSV; +import static org.hamcrest.CoreMatchers.is; public class TextFormatTests extends ESTestCase { + public void testPlainTextDetection() { + TextFormat text = TextFormat.fromMediaTypeOrFormat("text/plain"); + assertThat(text, is(TextFormat.PLAIN_TEXT)); + } + + public void testCsvDetection() { + TextFormat text = TextFormat.fromMediaTypeOrFormat("text/csv"); + assertThat(text, is(CSV)); + } + + public void testTsvDetection() { + TextFormat text = TextFormat.fromMediaTypeOrFormat("text/tab-separated-values"); + assertThat(text, is(TSV)); + } + + public void testInvalidFormat() { + Exception e = expectThrows(IllegalArgumentException.class, () -> TextFormat.fromMediaTypeOrFormat("text/garbage")); + assertEquals("invalid format [text/garbage]", e.getMessage()); + } + public void testCsvContentType() { assertEquals("text/csv; charset=utf-8; header=present", CSV.contentType(req())); }