From 1b0c1d2b0e5e0f2604a0e803aee4c0698d15ec4d Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 12 Oct 2020 17:01:31 +0200 Subject: [PATCH 1/5] pull supported media types, but push down registry --- .../common/xcontent/MediaType.java | 10 ++- .../common/xcontent/MediaTypeDefinition.java | 68 +++++++++++++++ .../common/xcontent/MediaTypeParser.java | 79 ++++++----------- .../common/xcontent/MediaTypeRegistry.java | 85 +++++++++++++++++++ .../common/xcontent/XContentType.java | 49 ++++++----- .../java/org/elasticsearch/node/Node.java | 18 ++++ .../elasticsearch/plugins/ActionPlugin.java | 4 + .../plugins/MediaTypeRegistryPlugin.java | 27 ++++++ .../plugins/RestCompatibilityPlugin.java | 2 + .../compat/CompatibleVersionPlugin.java | 18 +++- .../xpack/sql/plugin/RestSqlQueryAction.java | 9 +- .../xpack/sql/plugin/SqlMediaTypeParser.java | 24 +++--- .../xpack/sql/plugin/SqlPlugin.java | 55 +++++++++++- .../xpack/sql/plugin/TextFormat.java | 21 ++++- .../sql/plugin/SqlMediaTypeParserTests.java | 8 +- 15 files changed, 382 insertions(+), 95 deletions(-) create mode 100644 libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeDefinition.java create mode 100644 libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeRegistry.java create mode 100644 server/src/main/java/org/elasticsearch/plugins/MediaTypeRegistryPlugin.java diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaType.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaType.java index 2d61120288706..0d2cc5fccb043 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaType.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaType.java @@ -19,6 +19,10 @@ package org.elasticsearch.common.xcontent; +import java.util.Collections; +import java.util.Map; +import java.util.regex.Pattern; + /** * Abstracts a Media Type and a format parameter. * Media types are used as values on Content-Type and Accept headers @@ -46,7 +50,11 @@ public interface MediaType { /** * returns a string representation of a media type. */ - default String typeWithSubtype(){ + default String typeWithSubtype() { return type() + "/" + subtype(); } + + default Map validatedParameters() { + return Collections.emptyMap(); + } } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeDefinition.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeDefinition.java new file mode 100644 index 0000000000000..6fddf5de1224f --- /dev/null +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeDefinition.java @@ -0,0 +1,68 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.xcontent; + +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; +import java.util.regex.Pattern; + +public class MediaTypeDefinition { + private final String typeWithSubtype; + private final MediaType mediaType; + private final String format; + private final Map parameters; + + public MediaTypeDefinition(String typeWithSubtype, MediaType mediaType, String format, Map parameters) { + + this.typeWithSubtype = typeWithSubtype; + this.mediaType = mediaType; + this.format = format; + Map parametersForMediaType = new HashMap<>(parameters.size()); + for (Map.Entry params : parameters.entrySet()) { + String parameterName = params.getKey().toLowerCase(Locale.ROOT); + String parameterRegex = params.getValue(); + Pattern pattern = Pattern.compile(parameterRegex, Pattern.CASE_INSENSITIVE); + parametersForMediaType.put(parameterName, pattern); + } + this.parameters = parametersForMediaType; + } + + + public static MediaTypeDefinition of(String typeWithSubtype, MediaType mediaType, String format, Map parameters) { + return new MediaTypeDefinition(typeWithSubtype, mediaType, format, parameters); + } + + public String getTypeWithSubtype() { + return typeWithSubtype; + } + + public MediaType getMediaType() { + return mediaType; + } + + public String getFormat() { + return format; + } + + public Map getParameters() { + return parameters; + } +} 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..feee9d10bce7f 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 @@ -1,3 +1,4 @@ + /* * Licensed to Elasticsearch under one or more contributor * license agreements. See the NOTICE file distributed with @@ -25,27 +26,22 @@ 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); - } + private MediaTypeRegistry mediaTypeRegistry; + public MediaTypeParser(MediaTypeRegistry mediaTypeRegistry) { + this.mediaTypeRegistry = mediaTypeRegistry; + } + @SuppressWarnings("unchecked") public T fromMediaType(String mediaType) { ParsedMediaType parsedMediaType = parseMediaType(mediaType); - return parsedMediaType != null ? parsedMediaType.getMediaType() : null; + return parsedMediaType != null ? (T)parsedMediaType.getMediaType() : null; } - + @SuppressWarnings("unchecked") public T fromFormat(String format) { if (format == null) { return null; } - return formatToMediaType.get(format.toLowerCase(Locale.ROOT)); + return (T)mediaTypeRegistry.formatToMediaType(format.toLowerCase(Locale.ROOT)); } /** @@ -65,7 +61,7 @@ public ParsedMediaType parseMediaType(String headerValue) { String type = typeSubtype[0]; String subtype = typeSubtype[1]; String typeWithSubtype = type + "/" + subtype; - T xContentType = typeWithSubtypeToMediaType.get(typeWithSubtype); + MediaType xContentType = mediaTypeRegistry.typeWithSubtypeToMediaType(typeWithSubtype); if (xContentType != null) { Map parameters = new HashMap<>(); for (int i = 1; i < split.length; i++) { @@ -90,8 +86,8 @@ public ParsedMediaType parseMediaType(String headerValue) { } private boolean isValidParameter(String typeWithSubtype, String parameterName, String parameterValue) { - if (parametersMap.containsKey(typeWithSubtype)) { - Map parameters = parametersMap.get(typeWithSubtype); + if (mediaTypeRegistry.parametersFor(typeWithSubtype) != null) { + Map parameters = mediaTypeRegistry.parametersFor(typeWithSubtype); if (parameters.containsKey(parameterName)) { Pattern regex = parameters.get(parameterName); return regex.matcher(parameterValue).matches(); @@ -104,19 +100,32 @@ private boolean hasSpaces(String s) { return s.trim().equals(s) == false; } + private static final String COMPATIBLE_WITH_PARAMETER_NAME = "compatible-with"; + + public Byte parseVersion(String mediaType) { + ParsedMediaType parsedMediaType = parseMediaType(mediaType); + if (parsedMediaType != null) { + String version = parsedMediaType + .getParameters() + .get(COMPATIBLE_WITH_PARAMETER_NAME); + return version != null ? Byte.parseByte(version) : null; + } + return null; + } + /** * A media type object that contains all the information provided on a Content-Type or Accept header */ public class ParsedMediaType { private final Map parameters; - private final T mediaType; + private final MediaType mediaType; - public ParsedMediaType(T mediaType, Map parameters) { + public ParsedMediaType(MediaType mediaType, Map parameters) { this.parameters = parameters; this.mediaType = mediaType; } - public T getMediaType() { + public MediaType getMediaType() { return mediaType; } @@ -125,36 +134,4 @@ public Map getParameters() { } } - 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/MediaTypeRegistry.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeRegistry.java new file mode 100644 index 0000000000000..3750a4a0e11b5 --- /dev/null +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeRegistry.java @@ -0,0 +1,85 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.xcontent; + +import java.util.Collection; +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Pattern; + +public class MediaTypeRegistry { + + private Map formatToMediaType = new ConcurrentHashMap<>(); + private Map typeWithSubtypeToMediaType = new ConcurrentHashMap<>(); + private Map> parametersMap = new ConcurrentHashMap<>(); + + public MediaTypeRegistry register(Map formatToMediaType, Map typeWithSubtypeToMediaType, Map> parametersMap) { + this.formatToMediaType.putAll(formatToMediaType); + this.typeWithSubtypeToMediaType.putAll(typeWithSubtypeToMediaType); + this.parametersMap.putAll(parametersMap); + return this; + } + + public MediaType formatToMediaType(String format) { + return formatToMediaType.get(format); + } + + public MediaType typeWithSubtypeToMediaType(String typeWithSubtype) { + return typeWithSubtypeToMediaType.get(typeWithSubtype); + } + + public Map parametersFor(String typeWithSubtype) { + return parametersMap.get(typeWithSubtype); + } + + public MediaTypeRegistry register(String alternativeMediaType, MediaType 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 MediaTypeRegistry register(Collection mediaTypes) { + for (MediaTypeDefinition mediaTypeDefinition : mediaTypes) { + this.typeWithSubtypeToMediaType.put(mediaTypeDefinition.getTypeWithSubtype(), mediaTypeDefinition.getMediaType()); + this.parametersMap.put(mediaTypeDefinition.getTypeWithSubtype(), mediaTypeDefinition.getParameters()); + if(mediaTypeDefinition.getFormat() != null){ + this.formatToMediaType.put(mediaTypeDefinition.getFormat(), mediaTypeDefinition.getMediaType()); + } + } + return this; + } + + public void add(MediaTypeRegistry xContentTypeRegistry) { + formatToMediaType.putAll(xContentTypeRegistry.formatToMediaType); + typeWithSubtypeToMediaType.putAll(xContentTypeRegistry.typeWithSubtypeToMediaType); + parametersMap.putAll(xContentTypeRegistry.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..2c8a7695c5d38 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 @@ -114,26 +114,35 @@ 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 String COMPATIBLE_WITH_PARAMETER_NAME = "compatible-with"; + public static final String VERSION_PATTERN = "\\d+"; + public static MediaTypeParser mediaTypeParser; + + public static void addMediaTypesToRegistry(MediaTypeRegistry global) { + MediaTypeRegistry xContentTypeRegistry = new MediaTypeRegistry() + .register("application/smile", SMILE, Collections.emptyMap()) + .register("application/cbor", CBOR, Collections.emptyMap()) + .register("application/json", JSON, Map.of("charset", "UTF-8")) + .register("application/yaml", YAML, Map.of("charset", "UTF-8")) + .register("application/*", JSON, Map.of("charset", "UTF-8")) + .register("application/x-ndjson", JSON, Map.of("charset", "UTF-8")) + .register("application/vnd.elasticsearch+json", JSON, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .register("application/vnd.elasticsearch+smile", SMILE, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .register("application/vnd.elasticsearch+yaml", YAML, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .register("application/vnd.elasticsearch+cbor", CBOR, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .register("application/vnd.elasticsearch+x-ndjson", JSON, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")); + global.add(xContentTypeRegistry); + mediaTypeParser = new MediaTypeParser<>(xContentTypeRegistry); + } + + public static MediaTypeParser getMediaTypeParser() { + return mediaTypeParser; + } /** * Accepts a format string, which is most of the time is equivalent to {@link XContentType#subtype()} diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 15ca8e4f66bbd..f3bbcd27dc1f6 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -87,7 +87,10 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.common.util.PageCacheRecycler; +import org.elasticsearch.common.xcontent.MediaTypeDefinition; +import org.elasticsearch.common.xcontent.MediaTypeRegistry; import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.discovery.Discovery; import org.elasticsearch.discovery.DiscoveryModule; @@ -137,6 +140,7 @@ import org.elasticsearch.plugins.IndexStorePlugin; import org.elasticsearch.plugins.IngestPlugin; import org.elasticsearch.plugins.MapperPlugin; +import org.elasticsearch.plugins.MediaTypeRegistryPlugin; import org.elasticsearch.plugins.MetadataUpgrader; import org.elasticsearch.plugins.NetworkPlugin; import org.elasticsearch.plugins.PersistentTaskPlugin; @@ -330,6 +334,20 @@ protected Node(final Environment initialEnvironment, .collect(Collectors.toSet()); DiscoveryNode.setAdditionalRoles(additionalRoles); + Set mediaTypes = pluginsService.filterPlugins(ActionPlugin.class) + .stream() + .map(ActionPlugin::getAdditionalMediaTypes) + .flatMap(Collection::stream) + .collect(Collectors.toSet()); + + MediaTypeRegistry globalMediaTypeRegistry = new MediaTypeRegistry(); + globalMediaTypeRegistry.register(mediaTypes); + XContentType.addMediaTypesToRegistry(globalMediaTypeRegistry); + + // passes down to SQL and CompatibleVersion plugins + pluginsService.filterPlugins(MediaTypeRegistryPlugin.class) + .forEach(plugin -> plugin.setGlobalMediaTypeRegistry(globalMediaTypeRegistry)); + /* * Create the environment based on the finalized view of the settings. This is to ensure that components get the same setting * values, no matter they ask for them from. diff --git a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java index ef9861f266222..c73e35375aade 100644 --- a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java @@ -34,6 +34,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsFilter; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.common.xcontent.MediaTypeDefinition; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestHeaderDefinition; @@ -181,4 +182,7 @@ default Collection> in return Collections.emptyList(); } + default Collection getAdditionalMediaTypes(){ + return Collections.emptyList(); + } } diff --git a/server/src/main/java/org/elasticsearch/plugins/MediaTypeRegistryPlugin.java b/server/src/main/java/org/elasticsearch/plugins/MediaTypeRegistryPlugin.java new file mode 100644 index 0000000000000..62b907375ea23 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/plugins/MediaTypeRegistryPlugin.java @@ -0,0 +1,27 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.plugins; + +import org.elasticsearch.common.xcontent.MediaTypeRegistry; + +public interface MediaTypeRegistryPlugin { + void setGlobalMediaTypeRegistry(MediaTypeRegistry globalMediaTypeRegistry); + +} diff --git a/server/src/main/java/org/elasticsearch/plugins/RestCompatibilityPlugin.java b/server/src/main/java/org/elasticsearch/plugins/RestCompatibilityPlugin.java index 9fd73a24ee87b..0346602af8277 100644 --- a/server/src/main/java/org/elasticsearch/plugins/RestCompatibilityPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/RestCompatibilityPlugin.java @@ -21,6 +21,7 @@ import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.xcontent.MediaTypeRegistry; /** @@ -36,4 +37,5 @@ public interface RestCompatibilityPlugin { * @return a requested Compatible API Version */ Version getCompatibleVersion(@Nullable String acceptHeader, @Nullable String contentTypeHeader, boolean hasContent); + } diff --git a/x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java b/x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java index 2d4be753839eb..108899d6d1575 100644 --- a/x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java +++ b/x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java @@ -8,18 +8,23 @@ import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.common.xcontent.MediaType; +import org.elasticsearch.common.xcontent.MediaTypeParser; +import org.elasticsearch.common.xcontent.MediaTypeRegistry; +import org.elasticsearch.plugins.MediaTypeRegistryPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.RestCompatibilityPlugin; import org.elasticsearch.rest.RestStatus; -public class CompatibleVersionPlugin extends Plugin implements RestCompatibilityPlugin { +public class CompatibleVersionPlugin extends Plugin implements RestCompatibilityPlugin, MediaTypeRegistryPlugin { + + private MediaTypeParser mediaTypeParser; @Override public Version getCompatibleVersion(@Nullable String acceptHeader, @Nullable String contentTypeHeader, boolean hasContent) { - Byte aVersion = XContentType.parseVersion(acceptHeader); + Byte aVersion = mediaTypeParser.parseVersion(acceptHeader); byte acceptVersion = aVersion == null ? Version.CURRENT.major : Integer.valueOf(aVersion).byteValue(); - Byte cVersion = XContentType.parseVersion(contentTypeHeader); + Byte cVersion = mediaTypeParser.parseVersion(contentTypeHeader); byte contentTypeVersion = cVersion == null ? Version.CURRENT.major : Integer.valueOf(cVersion).byteValue(); // accept version must be current or prior @@ -72,4 +77,9 @@ public Version getCompatibleVersion(@Nullable String acceptHeader, @Nullable Str return Version.CURRENT; } + + @Override + public void setGlobalMediaTypeRegistry(MediaTypeRegistry globalMediaTypeRegistry) { + this.mediaTypeParser = new MediaTypeParser<>(globalMediaTypeRegistry); + } } 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..d89a97bbb730a 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 @@ -8,6 +8,7 @@ import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.xcontent.MediaType; +import org.elasticsearch.common.xcontent.MediaTypeParser; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; @@ -34,8 +35,12 @@ public class RestSqlQueryAction extends BaseRestHandler { - private final SqlMediaTypeParser sqlMediaTypeParser = new SqlMediaTypeParser(); MediaType responseMediaType; + private final SqlMediaTypeParser sqlMediaTypeParser; + + public RestSqlQueryAction(MediaTypeParser mediaTypeParser) { + this.sqlMediaTypeParser = new SqlMediaTypeParser(mediaTypeParser); + } @Override public List routes() { @@ -84,6 +89,8 @@ public RestResponse buildResponse(SqlQueryResponse response) throws Exception { }); } + //we should 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 index 189dc137b654c..5ccbc65866159 100644 --- 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 @@ -13,21 +13,17 @@ 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(); + private MediaTypeParser mediaTypeParser; + + public SqlMediaTypeParser(MediaTypeParser mediaTypeParser) { + //sql media types are already registered via plugin api + this.mediaTypeParser = mediaTypeParser; + } + /* * Since we support {@link TextFormat} and @@ -48,19 +44,19 @@ public MediaType getMediaType(RestRequest request, SqlQueryRequest sqlRequest) { // 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))); + return validateColumnarRequest(sqlRequest.columnar(), mediaTypeParser.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)); + return validateColumnarRequest(sqlRequest.columnar(), mediaTypeParser.fromMediaType(accept)); } } String contentType = request.header("Content-Type"); assert contentType != null : "The Content-Type header is required"; - return validateColumnarRequest(sqlRequest.columnar(), parser.fromMediaType(contentType)); + return validateColumnarRequest(sqlRequest.columnar(), mediaTypeParser.fromMediaType(contentType)); } private static MediaType validateColumnarRequest(boolean requestIsColumnar, MediaType fromMediaType) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java index 6f490f892b712..57f85ed349293 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java @@ -16,12 +16,18 @@ import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsFilter; +import org.elasticsearch.common.xcontent.MediaType; +import org.elasticsearch.common.xcontent.MediaTypeDefinition; +import org.elasticsearch.common.xcontent.MediaTypeParser; +import org.elasticsearch.common.xcontent.MediaTypeRegistry; import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.env.Environment; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.license.LicenseUtils; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.plugins.ActionPlugin; +import org.elasticsearch.plugins.MediaTypeRegistryPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.rest.RestController; @@ -42,12 +48,14 @@ import org.elasticsearch.xpack.sql.execution.PlanExecutor; import org.elasticsearch.xpack.sql.type.SqlDataTypeRegistry; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.function.Supplier; -public class SqlPlugin extends Plugin implements ActionPlugin { +public class SqlPlugin extends Plugin implements ActionPlugin, MediaTypeRegistryPlugin { private final SqlLicenseChecker sqlLicenseChecker = new SqlLicenseChecker( (mode) -> { @@ -74,6 +82,7 @@ public class SqlPlugin extends Plugin implements ActionPlugin { } } ); + private MediaTypeParser mediaTypeParser; public SqlPlugin(Settings settings) { } @@ -106,7 +115,7 @@ public List getRestHandlers(Settings settings, RestController restC SettingsFilter settingsFilter, IndexNameExpressionResolver indexNameExpressionResolver, Supplier nodesInCluster) { - return Arrays.asList(new RestSqlQueryAction(), + return Arrays.asList(new RestSqlQueryAction(mediaTypeParser), new RestSqlTranslateAction(), new RestSqlClearCursorAction(), new RestSqlStatsAction()); @@ -124,4 +133,46 @@ public List getRestHandlers(Settings settings, RestController restC usageAction, infoAction); } + + @Override + public Collection getAdditionalMediaTypes() { + List mediaTypeDefinitions = new ArrayList<>(); + mediaTypeDefinitions.add(MediaTypeDefinition.of(TextFormat.PLAIN_TEXT.typeWithSubtype(), + TextFormat.PLAIN_TEXT, + TextFormat.PLAIN_TEXT.format(), + Map.of("header", "present|absent", "charset", "utf-8"))); + mediaTypeDefinitions.add(MediaTypeDefinition.of(TextFormat.CSV.typeWithSubtype(), + TextFormat.CSV, + TextFormat.CSV.format(), + Map.of("header", "present|absent", "charset", "utf-8", + "delimiter", ".+"))); + mediaTypeDefinitions.add(MediaTypeDefinition.of(TextFormat.TSV.typeWithSubtype(), + TextFormat.TSV, + TextFormat.TSV.format(), + Map.of("header", "present|absent", "charset", "utf-8"))); + + mediaTypeDefinitions.add(MediaTypeDefinition.of("text/vnd.elasticsearch+plain", + TextFormat.PLAIN_TEXT, + null, + Map.of("header", "present|absent", "charset", "utf-8", + XContentType.COMPATIBLE_WITH_PARAMETER_NAME, XContentType.VERSION_PATTERN))); + mediaTypeDefinitions.add(MediaTypeDefinition.of("text/vnd.elasticsearch+csv", + TextFormat.CSV, + null, + Map.of("header", "present|absent", "charset", "utf-8", + "delimiter", ".+", XContentType.COMPATIBLE_WITH_PARAMETER_NAME, XContentType.VERSION_PATTERN))); + mediaTypeDefinitions.add(MediaTypeDefinition.of("text/vnd.elasticsearch+tsv", + TextFormat.TSV, + null, + Map.of("header", "present|absent", "charset", "utf-8", + XContentType.COMPATIBLE_WITH_PARAMETER_NAME, XContentType.VERSION_PATTERN))); + + return mediaTypeDefinitions; + } + + + @Override + public void setGlobalMediaTypeRegistry(MediaTypeRegistry globalMediaTypeRegistry) { + this.mediaTypeParser = new MediaTypeParser<>(globalMediaTypeRegistry); + } } 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..71fd4babc5701 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 @@ -24,8 +24,10 @@ import java.time.ZonedDateTime; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Objects; import java.util.function.Function; +import java.util.regex.Pattern; import static org.elasticsearch.xpack.sql.action.BasicFormatter.FormatOption.TEXT; import static org.elasticsearch.xpack.sql.proto.Protocol.URL_PARAM_DELIMITER; @@ -107,7 +109,11 @@ public String subtype() { return "plain"; } - + @Override + public Map validatedParameters() { + return Map.of("header", Pattern.compile("present|absent", Pattern.CASE_INSENSITIVE), + "charset", Pattern.compile("utf-8", Pattern.CASE_INSENSITIVE)); + } }, /** @@ -227,6 +233,13 @@ boolean hasHeader(RestRequest request) { public String subtype() { return "csv"; } + + @Override + public Map validatedParameters() { + return Map.of("header", Pattern.compile("present|absent", Pattern.CASE_INSENSITIVE), + "charset", Pattern.compile("utf-8", Pattern.CASE_INSENSITIVE), + "delimiter", Pattern.compile(".+", Pattern.CASE_INSENSITIVE)); + } }, TSV() { @@ -281,6 +294,12 @@ String maybeEscape(String value, Character __) { public String subtype() { return "tab-separated-values"; } + + @Override + public Map validatedParameters() { + return Map.of("header", Pattern.compile("present|absent", Pattern.CASE_INSENSITIVE), + "charset", Pattern.compile("utf-8", Pattern.CASE_INSENSITIVE)); + } }; private static final String FORMAT_TEXT = "txt"; 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 index 0459b777b15f9..194f153610dad 100644 --- 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 @@ -6,8 +6,11 @@ package org.elasticsearch.xpack.sql.plugin; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.MediaType; +import org.elasticsearch.common.xcontent.MediaTypeParser; +import org.elasticsearch.common.xcontent.MediaTypeRegistry; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.ESTestCase; @@ -27,7 +30,10 @@ import static org.hamcrest.CoreMatchers.nullValue; public class SqlMediaTypeParserTests extends ESTestCase { - SqlMediaTypeParser parser = new SqlMediaTypeParser(); + SqlMediaTypeParser parser = new SqlMediaTypeParser(new MediaTypeParser( + new MediaTypeRegistry() + .register(new SqlPlugin(Settings.EMPTY).getAdditionalMediaTypes()) + /*.register(XContentType)*/)); public void testPlainTextDetection() { MediaType text = parser.getMediaType(reqWithAccept("text/plain"), createTestInstance(false, Mode.PLAIN, false)); From 7bdf64fbbbeed88faf3e9c4ff68a9889d7146128 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 12 Oct 2020 17:18:12 +0200 Subject: [PATCH 2/5] cleanup --- .../common/xcontent/MediaTypeRegistry.java | 3 +- .../common/xcontent/XContentType.java | 45 ++++++++++--------- .../java/org/elasticsearch/node/Node.java | 8 ++-- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeRegistry.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeRegistry.java index 3750a4a0e11b5..c634d638a3090 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeRegistry.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeRegistry.java @@ -77,9 +77,10 @@ public MediaTypeRegistry register(Collection mediaTypes) { return this; } - public void add(MediaTypeRegistry xContentTypeRegistry) { + public MediaTypeRegistry register(MediaTypeRegistry xContentTypeRegistry) { formatToMediaType.putAll(xContentTypeRegistry.formatToMediaType); typeWithSubtypeToMediaType.putAll(xContentTypeRegistry.typeWithSubtypeToMediaType); parametersMap.putAll(xContentTypeRegistry.parametersMap); + return this; } } 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 2c8a7695c5d38..8b514178fdd36 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 @@ -116,28 +116,29 @@ public XContent xContent() { public static final String COMPATIBLE_WITH_PARAMETER_NAME = "compatible-with"; public static final String VERSION_PATTERN = "\\d+"; - public static MediaTypeParser mediaTypeParser; - - public static void addMediaTypesToRegistry(MediaTypeRegistry global) { - MediaTypeRegistry xContentTypeRegistry = new MediaTypeRegistry() - .register("application/smile", SMILE, Collections.emptyMap()) - .register("application/cbor", CBOR, Collections.emptyMap()) - .register("application/json", JSON, Map.of("charset", "UTF-8")) - .register("application/yaml", YAML, Map.of("charset", "UTF-8")) - .register("application/*", JSON, Map.of("charset", "UTF-8")) - .register("application/x-ndjson", JSON, Map.of("charset", "UTF-8")) - .register("application/vnd.elasticsearch+json", JSON, - Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) - .register("application/vnd.elasticsearch+smile", SMILE, - Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) - .register("application/vnd.elasticsearch+yaml", YAML, - Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) - .register("application/vnd.elasticsearch+cbor", CBOR, - Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) - .register("application/vnd.elasticsearch+x-ndjson", JSON, - Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")); - global.add(xContentTypeRegistry); - mediaTypeParser = new MediaTypeParser<>(xContentTypeRegistry); + + private static final MediaTypeRegistry mediaTypeRegistry = new MediaTypeRegistry() + .register("application/smile", SMILE, Collections.emptyMap()) + .register("application/cbor", CBOR, Collections.emptyMap()) + .register("application/json", JSON, Map.of("charset", "UTF-8")) + .register("application/yaml", YAML, Map.of("charset", "UTF-8")) + .register("application/*", JSON, Map.of("charset", "UTF-8")) + .register("application/x-ndjson", JSON, Map.of("charset", "UTF-8")) + .register("application/vnd.elasticsearch+json", JSON, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .register("application/vnd.elasticsearch+smile", SMILE, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .register("application/vnd.elasticsearch+yaml", YAML, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .register("application/vnd.elasticsearch+cbor", CBOR, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")) + .register("application/vnd.elasticsearch+x-ndjson", JSON, + Map.of(COMPATIBLE_WITH_PARAMETER_NAME, VERSION_PATTERN, "charset", "UTF-8")); + + private static MediaTypeParser mediaTypeParser = new MediaTypeParser<>(mediaTypeRegistry); + + public static MediaTypeRegistry getMediaTypeRegistry() { + return mediaTypeRegistry; } public static MediaTypeParser getMediaTypeParser() { diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index f3bbcd27dc1f6..f5324f78d2d89 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -334,15 +334,15 @@ protected Node(final Environment initialEnvironment, .collect(Collectors.toSet()); DiscoveryNode.setAdditionalRoles(additionalRoles); - Set mediaTypes = pluginsService.filterPlugins(ActionPlugin.class) + Set mediaTypesFromPlugins = pluginsService.filterPlugins(ActionPlugin.class) .stream() .map(ActionPlugin::getAdditionalMediaTypes) .flatMap(Collection::stream) .collect(Collectors.toSet()); - MediaTypeRegistry globalMediaTypeRegistry = new MediaTypeRegistry(); - globalMediaTypeRegistry.register(mediaTypes); - XContentType.addMediaTypesToRegistry(globalMediaTypeRegistry); + MediaTypeRegistry globalMediaTypeRegistry = new MediaTypeRegistry() + .register(mediaTypesFromPlugins) + .register(XContentType.getMediaTypeRegistry()); // passes down to SQL and CompatibleVersion plugins pluginsService.filterPlugins(MediaTypeRegistryPlugin.class) From ff93fd256743f5c06cdfb4c0563617ce9b806ad0 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Tue, 13 Oct 2020 10:21:26 +0200 Subject: [PATCH 3/5] remove mediatypedefinition --- .../common/xcontent/MediaTypeDefinition.java | 68 ------------------- .../common/xcontent/MediaTypeRegistry.java | 33 ++++++--- .../common/xcontent/XContentType.java | 4 -- .../java/org/elasticsearch/node/Node.java | 6 +- .../elasticsearch/plugins/ActionPlugin.java | 6 +- .../xpack/sql/plugin/RestSqlQueryAction.java | 5 +- .../xpack/sql/plugin/SqlMediaTypeParser.java | 5 +- .../xpack/sql/plugin/SqlPlugin.java | 39 +++++------ .../sql/plugin/SqlMediaTypeParserTests.java | 5 +- 9 files changed, 54 insertions(+), 117 deletions(-) delete mode 100644 libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeDefinition.java diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeDefinition.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeDefinition.java deleted file mode 100644 index 6fddf5de1224f..0000000000000 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeDefinition.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.common.xcontent; - -import java.util.HashMap; -import java.util.Locale; -import java.util.Map; -import java.util.regex.Pattern; - -public class MediaTypeDefinition { - private final String typeWithSubtype; - private final MediaType mediaType; - private final String format; - private final Map parameters; - - public MediaTypeDefinition(String typeWithSubtype, MediaType mediaType, String format, Map parameters) { - - this.typeWithSubtype = typeWithSubtype; - this.mediaType = mediaType; - this.format = format; - Map parametersForMediaType = new HashMap<>(parameters.size()); - for (Map.Entry params : parameters.entrySet()) { - String parameterName = params.getKey().toLowerCase(Locale.ROOT); - String parameterRegex = params.getValue(); - Pattern pattern = Pattern.compile(parameterRegex, Pattern.CASE_INSENSITIVE); - parametersForMediaType.put(parameterName, pattern); - } - this.parameters = parametersForMediaType; - } - - - public static MediaTypeDefinition of(String typeWithSubtype, MediaType mediaType, String format, Map parameters) { - return new MediaTypeDefinition(typeWithSubtype, mediaType, format, parameters); - } - - public String getTypeWithSubtype() { - return typeWithSubtype; - } - - public MediaType getMediaType() { - return mediaType; - } - - public String getFormat() { - return format; - } - - public Map getParameters() { - return parameters; - } -} diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeRegistry.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeRegistry.java index c634d638a3090..913d4d677b274 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeRegistry.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/MediaTypeRegistry.java @@ -39,6 +39,21 @@ public MediaTypeRegistry register(Map formatToM return this; } + public MediaTypeRegistry register(String typeWithSubtype, T mediaType, String format, Map parametersMap) { + if (format != null) { + this.formatToMediaType.put(format, mediaType); + } + this.typeWithSubtypeToMediaType.put(typeWithSubtype,mediaType); + Map parametersForMediaType = new HashMap<>(parametersMap.size()); + for (Map.Entry params : parametersMap.entrySet()) { + String parameterName = params.getKey().toLowerCase(Locale.ROOT); + String parameterRegex = params.getValue(); + Pattern pattern = Pattern.compile(parameterRegex, Pattern.CASE_INSENSITIVE); + parametersForMediaType.put(parameterName, pattern); + } + this.parametersMap.put(typeWithSubtype,parametersForMediaType); + return this; + } public MediaType formatToMediaType(String format) { return formatToMediaType.get(format); } @@ -66,21 +81,17 @@ public MediaTypeRegistry register(String alternativeMediaType, MediaType mediaTy return this; } - public MediaTypeRegistry register(Collection mediaTypes) { - for (MediaTypeDefinition mediaTypeDefinition : mediaTypes) { - this.typeWithSubtypeToMediaType.put(mediaTypeDefinition.getTypeWithSubtype(), mediaTypeDefinition.getMediaType()); - this.parametersMap.put(mediaTypeDefinition.getTypeWithSubtype(), mediaTypeDefinition.getParameters()); - if(mediaTypeDefinition.getFormat() != null){ - this.formatToMediaType.put(mediaTypeDefinition.getFormat(), mediaTypeDefinition.getMediaType()); - } - } - return this; - } - public MediaTypeRegistry register(MediaTypeRegistry xContentTypeRegistry) { formatToMediaType.putAll(xContentTypeRegistry.formatToMediaType); typeWithSubtypeToMediaType.putAll(xContentTypeRegistry.typeWithSubtypeToMediaType); parametersMap.putAll(xContentTypeRegistry.parametersMap); return this; } + public MediaTypeRegistry register(Collection mediaTypeRegistries ) { + for (MediaTypeRegistry mediaTypeRegistry : mediaTypeRegistries) { + register(mediaTypeRegistry); + } + return this; + } + } 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 8b514178fdd36..be8d9189f6cd4 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 @@ -141,10 +141,6 @@ public static MediaTypeRegistry getMediaTypeRegistry() { return mediaTypeRegistry; } - public static MediaTypeParser getMediaTypeParser() { - return mediaTypeParser; - } - /** * Accepts a format string, which is most of the time is equivalent to {@link XContentType#subtype()} * and attempts to match the value to an {@link XContentType}. diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index f5324f78d2d89..cebeba75599c6 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -87,7 +87,6 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.common.util.PageCacheRecycler; -import org.elasticsearch.common.xcontent.MediaTypeDefinition; import org.elasticsearch.common.xcontent.MediaTypeRegistry; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentType; @@ -334,11 +333,10 @@ protected Node(final Environment initialEnvironment, .collect(Collectors.toSet()); DiscoveryNode.setAdditionalRoles(additionalRoles); - Set mediaTypesFromPlugins = pluginsService.filterPlugins(ActionPlugin.class) + Collection mediaTypesFromPlugins = pluginsService.filterPlugins(ActionPlugin.class) .stream() .map(ActionPlugin::getAdditionalMediaTypes) - .flatMap(Collection::stream) - .collect(Collectors.toSet()); + .collect(toList()); MediaTypeRegistry globalMediaTypeRegistry = new MediaTypeRegistry() .register(mediaTypesFromPlugins) diff --git a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java index c73e35375aade..266a0e7550be7 100644 --- a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java @@ -34,7 +34,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsFilter; import org.elasticsearch.common.util.concurrent.ThreadContext; -import org.elasticsearch.common.xcontent.MediaTypeDefinition; +import org.elasticsearch.common.xcontent.MediaTypeRegistry; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestHeaderDefinition; @@ -182,7 +182,7 @@ default Collection> in return Collections.emptyList(); } - default Collection getAdditionalMediaTypes(){ - return Collections.emptyList(); + default MediaTypeRegistry getAdditionalMediaTypes(){ + return new MediaTypeRegistry(); } } 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 d89a97bbb730a..64495ed816eb0 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 @@ -9,6 +9,7 @@ import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.xcontent.MediaType; import org.elasticsearch.common.xcontent.MediaTypeParser; +import org.elasticsearch.common.xcontent.MediaTypeRegistry; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; @@ -38,8 +39,8 @@ public class RestSqlQueryAction extends BaseRestHandler { MediaType responseMediaType; private final SqlMediaTypeParser sqlMediaTypeParser; - public RestSqlQueryAction(MediaTypeParser mediaTypeParser) { - this.sqlMediaTypeParser = new SqlMediaTypeParser(mediaTypeParser); + public RestSqlQueryAction(MediaTypeRegistry globalMediaTypeRegistry) { + this.sqlMediaTypeParser = new SqlMediaTypeParser(globalMediaTypeRegistry); } @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 index 5ccbc65866159..30e28fb4a35bb 100644 --- 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 @@ -8,6 +8,7 @@ import org.elasticsearch.common.xcontent.MediaType; import org.elasticsearch.common.xcontent.MediaTypeParser; +import org.elasticsearch.common.xcontent.MediaTypeRegistry; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.xpack.sql.action.SqlQueryRequest; @@ -19,9 +20,9 @@ public class SqlMediaTypeParser { private MediaTypeParser mediaTypeParser; - public SqlMediaTypeParser(MediaTypeParser mediaTypeParser) { + public SqlMediaTypeParser(MediaTypeRegistry globalMediaTypeRegistry) { //sql media types are already registered via plugin api - this.mediaTypeParser = mediaTypeParser; + this.mediaTypeParser = new MediaTypeParser<>(globalMediaTypeRegistry); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java index 57f85ed349293..f06d368843f23 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java @@ -16,9 +16,6 @@ import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsFilter; -import org.elasticsearch.common.xcontent.MediaType; -import org.elasticsearch.common.xcontent.MediaTypeDefinition; -import org.elasticsearch.common.xcontent.MediaTypeParser; import org.elasticsearch.common.xcontent.MediaTypeRegistry; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentType; @@ -82,7 +79,7 @@ public class SqlPlugin extends Plugin implements ActionPlugin, MediaTypeRegistry } } ); - private MediaTypeParser mediaTypeParser; + private MediaTypeRegistry globalMediaTypeRegistry; public SqlPlugin(Settings settings) { } @@ -115,7 +112,7 @@ public List getRestHandlers(Settings settings, RestController restC SettingsFilter settingsFilter, IndexNameExpressionResolver indexNameExpressionResolver, Supplier nodesInCluster) { - return Arrays.asList(new RestSqlQueryAction(mediaTypeParser), + return Arrays.asList(new RestSqlQueryAction(globalMediaTypeRegistry), new RestSqlTranslateAction(), new RestSqlClearCursorAction(), new RestSqlStatsAction()); @@ -135,44 +132,44 @@ public List getRestHandlers(Settings settings, RestController restC } @Override - public Collection getAdditionalMediaTypes() { - List mediaTypeDefinitions = new ArrayList<>(); - mediaTypeDefinitions.add(MediaTypeDefinition.of(TextFormat.PLAIN_TEXT.typeWithSubtype(), + public MediaTypeRegistry getAdditionalMediaTypes() { + MediaTypeRegistry mediaTypeRegistry = new MediaTypeRegistry(); + mediaTypeRegistry.register(TextFormat.PLAIN_TEXT.typeWithSubtype(), TextFormat.PLAIN_TEXT, TextFormat.PLAIN_TEXT.format(), - Map.of("header", "present|absent", "charset", "utf-8"))); - mediaTypeDefinitions.add(MediaTypeDefinition.of(TextFormat.CSV.typeWithSubtype(), + Map.of("header", "present|absent", "charset", "utf-8")) + .register(TextFormat.CSV.typeWithSubtype(), TextFormat.CSV, TextFormat.CSV.format(), Map.of("header", "present|absent", "charset", "utf-8", - "delimiter", ".+"))); - mediaTypeDefinitions.add(MediaTypeDefinition.of(TextFormat.TSV.typeWithSubtype(), + "delimiter", ".+")) + .register(TextFormat.TSV.typeWithSubtype(), TextFormat.TSV, TextFormat.TSV.format(), - Map.of("header", "present|absent", "charset", "utf-8"))); + Map.of("header", "present|absent", "charset", "utf-8")) - mediaTypeDefinitions.add(MediaTypeDefinition.of("text/vnd.elasticsearch+plain", + .register("text/vnd.elasticsearch+plain", TextFormat.PLAIN_TEXT, null, Map.of("header", "present|absent", "charset", "utf-8", - XContentType.COMPATIBLE_WITH_PARAMETER_NAME, XContentType.VERSION_PATTERN))); - mediaTypeDefinitions.add(MediaTypeDefinition.of("text/vnd.elasticsearch+csv", + XContentType.COMPATIBLE_WITH_PARAMETER_NAME, XContentType.VERSION_PATTERN)) + .register("text/vnd.elasticsearch+csv", TextFormat.CSV, null, Map.of("header", "present|absent", "charset", "utf-8", - "delimiter", ".+", XContentType.COMPATIBLE_WITH_PARAMETER_NAME, XContentType.VERSION_PATTERN))); - mediaTypeDefinitions.add(MediaTypeDefinition.of("text/vnd.elasticsearch+tsv", + "delimiter", ".+", XContentType.COMPATIBLE_WITH_PARAMETER_NAME, XContentType.VERSION_PATTERN)) + .register("text/vnd.elasticsearch+tsv", TextFormat.TSV, null, Map.of("header", "present|absent", "charset", "utf-8", - XContentType.COMPATIBLE_WITH_PARAMETER_NAME, XContentType.VERSION_PATTERN))); + XContentType.COMPATIBLE_WITH_PARAMETER_NAME, XContentType.VERSION_PATTERN)); - return mediaTypeDefinitions; + return mediaTypeRegistry; } @Override public void setGlobalMediaTypeRegistry(MediaTypeRegistry globalMediaTypeRegistry) { - this.mediaTypeParser = new MediaTypeParser<>(globalMediaTypeRegistry); + this.globalMediaTypeRegistry = globalMediaTypeRegistry; } } 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 index 194f153610dad..fd3e49801655f 100644 --- 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 @@ -12,6 +12,7 @@ import org.elasticsearch.common.xcontent.MediaTypeParser; import org.elasticsearch.common.xcontent.MediaTypeRegistry; import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; @@ -30,10 +31,10 @@ import static org.hamcrest.CoreMatchers.nullValue; public class SqlMediaTypeParserTests extends ESTestCase { - SqlMediaTypeParser parser = new SqlMediaTypeParser(new MediaTypeParser( + SqlMediaTypeParser parser = new SqlMediaTypeParser( new MediaTypeRegistry() .register(new SqlPlugin(Settings.EMPTY).getAdditionalMediaTypes()) - /*.register(XContentType)*/)); + .register(XContentType.getMediaTypeRegistry())); public void testPlainTextDetection() { MediaType text = parser.getMediaType(reqWithAccept("text/plain"), createTestInstance(false, Mode.PLAIN, false)); From f5c60ff67b567a9be9655e9fa15f05685d5989ab Mon Sep 17 00:00:00 2001 From: pgomulka Date: Thu, 15 Oct 2020 11:42:56 +0200 Subject: [PATCH 4/5] passing a mediatyperegistry to plugin, but keeping the signature the same --- .../java/org/elasticsearch/node/Node.java | 36 ++++++++++--------- .../plugins/RestCompatibilityPlugin.java | 2 +- .../org/elasticsearch/node/NodeTests.java | 12 ++++--- .../rest/RestControllerTests.java | 2 +- .../compat/CompatibleVersionPlugin.java | 8 +++-- .../compat/CompatibleVersionPluginTests.java | 4 ++- 6 files changed, 38 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index cebeba75599c6..dd605ff9ef366 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -139,7 +139,6 @@ import org.elasticsearch.plugins.IndexStorePlugin; import org.elasticsearch.plugins.IngestPlugin; import org.elasticsearch.plugins.MapperPlugin; -import org.elasticsearch.plugins.MediaTypeRegistryPlugin; import org.elasticsearch.plugins.MetadataUpgrader; import org.elasticsearch.plugins.NetworkPlugin; import org.elasticsearch.plugins.PersistentTaskPlugin; @@ -333,18 +332,6 @@ protected Node(final Environment initialEnvironment, .collect(Collectors.toSet()); DiscoveryNode.setAdditionalRoles(additionalRoles); - Collection mediaTypesFromPlugins = pluginsService.filterPlugins(ActionPlugin.class) - .stream() - .map(ActionPlugin::getAdditionalMediaTypes) - .collect(toList()); - - MediaTypeRegistry globalMediaTypeRegistry = new MediaTypeRegistry() - .register(mediaTypesFromPlugins) - .register(XContentType.getMediaTypeRegistry()); - - // passes down to SQL and CompatibleVersion plugins - pluginsService.filterPlugins(MediaTypeRegistryPlugin.class) - .forEach(plugin -> plugin.setGlobalMediaTypeRegistry(globalMediaTypeRegistry)); /* * Create the environment based on the finalized view of the settings. This is to ensure that components get the same setting @@ -545,10 +532,25 @@ protected Node(final Environment initialEnvironment, repositoriesServiceReference::get).stream()) .collect(Collectors.toList()); + Collection mediaTypesFromPlugins = pluginsService.filterPlugins(ActionPlugin.class) + .stream() + .map(ActionPlugin::getAdditionalMediaTypes) + .collect(toList()); + + MediaTypeRegistry globalMediaTypeRegistry = new MediaTypeRegistry() + .register(mediaTypesFromPlugins) + .register(XContentType.getMediaTypeRegistry()); + + // passes down to SQL and CompatibleVersion plugins +// pluginsService.filterPlugins(MediaTypeRegistryPlugin.class) +// .forEach(plugin -> plugin.setGlobalMediaTypeRegistry(globalMediaTypeRegistry)); + + CompatibleVersion restCompatibleFunction = getRestCompatibleFunction(globalMediaTypeRegistry); + ActionModule actionModule = new ActionModule(settings, clusterModule.getIndexNameExpressionResolver(), settingsModule.getIndexScopedSettings(), settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(), threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService, - systemIndices, getRestCompatibleFunction()); + systemIndices, restCompatibleFunction); modules.add(actionModule); final RestController restController = actionModule.getRestController(); @@ -726,14 +728,16 @@ protected Node(final Environment initialEnvironment, /** * @return A function that can be used to determine the requested REST compatible version * package scope for testing + * @param globalMediaTypeRegistry */ - CompatibleVersion getRestCompatibleFunction() { + CompatibleVersion getRestCompatibleFunction(MediaTypeRegistry globalMediaTypeRegistry) { List restCompatibilityPlugins = pluginsService.filterPlugins(RestCompatibilityPlugin.class); final CompatibleVersion compatibleVersion; if (restCompatibilityPlugins.size() > 1) { throw new IllegalStateException("Only one RestCompatibilityPlugin is allowed"); } else if (restCompatibilityPlugins.size() == 1) { - compatibleVersion = restCompatibilityPlugins.get(0)::getCompatibleVersion; + compatibleVersion = (acceptHeader, contentTypeHeader, hasContent) -> + restCompatibilityPlugins.get(0).getCompatibleVersion(acceptHeader, contentTypeHeader, hasContent, globalMediaTypeRegistry); } else { compatibleVersion = CompatibleVersion.CURRENT_VERSION; } diff --git a/server/src/main/java/org/elasticsearch/plugins/RestCompatibilityPlugin.java b/server/src/main/java/org/elasticsearch/plugins/RestCompatibilityPlugin.java index 0346602af8277..cea394036d6e2 100644 --- a/server/src/main/java/org/elasticsearch/plugins/RestCompatibilityPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/RestCompatibilityPlugin.java @@ -36,6 +36,6 @@ public interface RestCompatibilityPlugin { * @param hasContent - a flag indicating if a request has content * @return a requested Compatible API Version */ - Version getCompatibleVersion(@Nullable String acceptHeader, @Nullable String contentTypeHeader, boolean hasContent); + Version getCompatibleVersion(@Nullable String acceptHeader, @Nullable String contentTypeHeader, boolean hasContent, MediaTypeRegistry mediaTypeRegistry); } diff --git a/server/src/test/java/org/elasticsearch/node/NodeTests.java b/server/src/test/java/org/elasticsearch/node/NodeTests.java index 3741b172653a1..e6e60c6d5bdd4 100644 --- a/server/src/test/java/org/elasticsearch/node/NodeTests.java +++ b/server/src/test/java/org/elasticsearch/node/NodeTests.java @@ -28,6 +28,8 @@ import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; +import org.elasticsearch.common.xcontent.MediaTypeRegistry; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.env.Environment; import org.elasticsearch.index.IndexService; import org.elasticsearch.index.engine.Engine.Searcher; @@ -345,14 +347,16 @@ public void setCircuitBreaker(CircuitBreaker circuitBreaker) { public static class TestRestCompatibility1 extends Plugin implements RestCompatibilityPlugin { @Override - public Version getCompatibleVersion(String acceptHeader, String contentTypeHeader, boolean hasContent) { + public Version getCompatibleVersion(String acceptHeader, String contentTypeHeader, boolean hasContent, + MediaTypeRegistry mediaTypeRegistry) { return Version.CURRENT.previousMajor(); } } public static class TestRestCompatibility2 extends Plugin implements RestCompatibilityPlugin { @Override - public Version getCompatibleVersion(String acceptHeader, String contentTypeHeader, boolean hasContent) { + public Version getCompatibleVersion(String acceptHeader, String contentTypeHeader, boolean hasContent, + MediaTypeRegistry mediaTypeRegistry) { return null; } } @@ -376,7 +380,7 @@ public void testCorrectUsageOfRestCompatibilityPlugin() throws IOException { plugins.add(TestRestCompatibility1.class); try (Node node = new MockNode(settings.build(), plugins)) { - CompatibleVersion restCompatibleFunction = node.getRestCompatibleFunction(); + CompatibleVersion restCompatibleFunction = node.getRestCompatibleFunction(XContentType.getMediaTypeRegistry()); assertThat(restCompatibleFunction.get("", "", false), equalTo(Version.CURRENT.previousMajor())); } } @@ -389,7 +393,7 @@ public void testDefaultingRestCompatibilityPlugin() throws IOException { List> plugins = basePlugins(); try (Node node = new MockNode(settings.build(), plugins)) { - CompatibleVersion restCompatibleFunction = node.getRestCompatibleFunction(); + CompatibleVersion restCompatibleFunction = node.getRestCompatibleFunction(XContentType.getMediaTypeRegistry()); assertThat(restCompatibleFunction.get("", "", false), equalTo(Version.CURRENT)); } } diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index 2dd6d00b43e88..4346600692091 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -98,7 +98,7 @@ public void setup() { HttpServerTransport httpServerTransport = new TestHttpServerTransport(); client = new NoOpNodeClient(this.getTestName()); restController = new RestController(Collections.emptySet(), null, client, circuitBreakerService, usageService, - , CompatibleVersion.CURRENT_VERSION); + CompatibleVersion.CURRENT_VERSION); restController.registerHandler(RestRequest.Method.GET, "/", (request, channel, client) -> channel.sendResponse( new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY))); diff --git a/x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java b/x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java index 108899d6d1575..4f0c2426018cb 100644 --- a/x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java +++ b/x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java @@ -18,10 +18,12 @@ public class CompatibleVersionPlugin extends Plugin implements RestCompatibilityPlugin, MediaTypeRegistryPlugin { - private MediaTypeParser mediaTypeParser; +// private MediaTypeParser mediaTypeParser; @Override - public Version getCompatibleVersion(@Nullable String acceptHeader, @Nullable String contentTypeHeader, boolean hasContent) { + public Version getCompatibleVersion(@Nullable String acceptHeader, @Nullable String contentTypeHeader, boolean hasContent, + MediaTypeRegistry mediaTypeRegistry) { + MediaTypeParser mediaTypeParser = new MediaTypeParser<>(mediaTypeRegistry); Byte aVersion = mediaTypeParser.parseVersion(acceptHeader); byte acceptVersion = aVersion == null ? Version.CURRENT.major : Integer.valueOf(aVersion).byteValue(); Byte cVersion = mediaTypeParser.parseVersion(contentTypeHeader); @@ -80,6 +82,6 @@ public Version getCompatibleVersion(@Nullable String acceptHeader, @Nullable Str @Override public void setGlobalMediaTypeRegistry(MediaTypeRegistry globalMediaTypeRegistry) { - this.mediaTypeParser = new MediaTypeParser<>(globalMediaTypeRegistry); +// this.mediaTypeParser = new MediaTypeParser<>(globalMediaTypeRegistry); } } diff --git a/x-pack/plugin/rest-compatibility/src/test/java/org/elasticsearch/compat/CompatibleVersionPluginTests.java b/x-pack/plugin/rest-compatibility/src/test/java/org/elasticsearch/compat/CompatibleVersionPluginTests.java index 32f792ffc2a92..334a13dd62647 100644 --- a/x-pack/plugin/rest-compatibility/src/test/java/org/elasticsearch/compat/CompatibleVersionPluginTests.java +++ b/x-pack/plugin/rest-compatibility/src/test/java/org/elasticsearch/compat/CompatibleVersionPluginTests.java @@ -8,6 +8,7 @@ import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.Version; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.hamcrest.ElasticsearchMatchers; import org.hamcrest.Matcher; @@ -206,7 +207,8 @@ private String mediaType(String version) { } private Version requestWith(String accept, String contentType, String body) { - return compatibleVersionPlugin.getCompatibleVersion(accept, contentType, body.isEmpty() == false); + return compatibleVersionPlugin.getCompatibleVersion(accept, contentType, body.isEmpty() == false, + XContentType.getMediaTypeRegistry()); } } From 5ad227ad20ded0a92d6f1b5fa10baa12042d0bbc Mon Sep 17 00:00:00 2001 From: pgomulka Date: Thu, 15 Oct 2020 17:34:06 +0200 Subject: [PATCH 5/5] do not use global media type registry in sql --- .../java/org/elasticsearch/node/Node.java | 1 - .../plugins/MediaTypeRegistryPlugin.java | 27 ------------------- .../compat/CompatibleVersionPlugin.java | 7 +---- .../xpack/sql/plugin/RestSqlQueryAction.java | 6 ++--- .../xpack/sql/plugin/SqlMediaTypeParser.java | 10 ++++--- .../xpack/sql/plugin/SqlPlugin.java | 12 ++------- 6 files changed, 12 insertions(+), 51 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/plugins/MediaTypeRegistryPlugin.java diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index dd605ff9ef366..4cf13f117d62c 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -728,7 +728,6 @@ protected Node(final Environment initialEnvironment, /** * @return A function that can be used to determine the requested REST compatible version * package scope for testing - * @param globalMediaTypeRegistry */ CompatibleVersion getRestCompatibleFunction(MediaTypeRegistry globalMediaTypeRegistry) { List restCompatibilityPlugins = pluginsService.filterPlugins(RestCompatibilityPlugin.class); diff --git a/server/src/main/java/org/elasticsearch/plugins/MediaTypeRegistryPlugin.java b/server/src/main/java/org/elasticsearch/plugins/MediaTypeRegistryPlugin.java deleted file mode 100644 index 62b907375ea23..0000000000000 --- a/server/src/main/java/org/elasticsearch/plugins/MediaTypeRegistryPlugin.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.plugins; - -import org.elasticsearch.common.xcontent.MediaTypeRegistry; - -public interface MediaTypeRegistryPlugin { - void setGlobalMediaTypeRegistry(MediaTypeRegistry globalMediaTypeRegistry); - -} diff --git a/x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java b/x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java index 4f0c2426018cb..c8a34db445f1e 100644 --- a/x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java +++ b/x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java @@ -11,12 +11,11 @@ import org.elasticsearch.common.xcontent.MediaType; import org.elasticsearch.common.xcontent.MediaTypeParser; import org.elasticsearch.common.xcontent.MediaTypeRegistry; -import org.elasticsearch.plugins.MediaTypeRegistryPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.RestCompatibilityPlugin; import org.elasticsearch.rest.RestStatus; -public class CompatibleVersionPlugin extends Plugin implements RestCompatibilityPlugin, MediaTypeRegistryPlugin { +public class CompatibleVersionPlugin extends Plugin implements RestCompatibilityPlugin { // private MediaTypeParser mediaTypeParser; @@ -80,8 +79,4 @@ public Version getCompatibleVersion(@Nullable String acceptHeader, @Nullable Str return Version.CURRENT; } - @Override - public void setGlobalMediaTypeRegistry(MediaTypeRegistry globalMediaTypeRegistry) { -// this.mediaTypeParser = new MediaTypeParser<>(globalMediaTypeRegistry); - } } 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 64495ed816eb0..431651e7b1412 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 @@ -37,10 +37,10 @@ public class RestSqlQueryAction extends BaseRestHandler { MediaType responseMediaType; - private final SqlMediaTypeParser sqlMediaTypeParser; + private final SqlMediaTypeParser sqlMediaTypeParser ; - public RestSqlQueryAction(MediaTypeRegistry globalMediaTypeRegistry) { - this.sqlMediaTypeParser = new SqlMediaTypeParser(globalMediaTypeRegistry); + public RestSqlQueryAction(MediaTypeRegistry additionalMediaTypes) { + sqlMediaTypeParser = new SqlMediaTypeParser(additionalMediaTypes); } @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 index 30e28fb4a35bb..7f4fc14d8f353 100644 --- 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 @@ -18,11 +18,13 @@ import static org.elasticsearch.xpack.sql.proto.Protocol.URL_PARAM_FORMAT; public class SqlMediaTypeParser { - private MediaTypeParser mediaTypeParser; + private MediaTypeParser mediaTypeParser ; - public SqlMediaTypeParser(MediaTypeRegistry globalMediaTypeRegistry) { - //sql media types are already registered via plugin api - this.mediaTypeParser = new MediaTypeParser<>(globalMediaTypeRegistry); + public SqlMediaTypeParser(MediaTypeRegistry additionalMediaTypes) { + MediaTypeRegistry register = new MediaTypeRegistry() + .register(XContentType.getMediaTypeRegistry()) + .register(additionalMediaTypes); + mediaTypeParser = new MediaTypeParser<>(register); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java index f06d368843f23..24b6973fd298e 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java @@ -24,7 +24,6 @@ import org.elasticsearch.license.LicenseUtils; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.plugins.ActionPlugin; -import org.elasticsearch.plugins.MediaTypeRegistryPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.rest.RestController; @@ -45,14 +44,13 @@ import org.elasticsearch.xpack.sql.execution.PlanExecutor; import org.elasticsearch.xpack.sql.type.SqlDataTypeRegistry; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Map; import java.util.function.Supplier; -public class SqlPlugin extends Plugin implements ActionPlugin, MediaTypeRegistryPlugin { +public class SqlPlugin extends Plugin implements ActionPlugin { private final SqlLicenseChecker sqlLicenseChecker = new SqlLicenseChecker( (mode) -> { @@ -79,7 +77,6 @@ public class SqlPlugin extends Plugin implements ActionPlugin, MediaTypeRegistry } } ); - private MediaTypeRegistry globalMediaTypeRegistry; public SqlPlugin(Settings settings) { } @@ -112,7 +109,7 @@ public List getRestHandlers(Settings settings, RestController restC SettingsFilter settingsFilter, IndexNameExpressionResolver indexNameExpressionResolver, Supplier nodesInCluster) { - return Arrays.asList(new RestSqlQueryAction(globalMediaTypeRegistry), + return Arrays.asList(new RestSqlQueryAction(getAdditionalMediaTypes()), new RestSqlTranslateAction(), new RestSqlClearCursorAction(), new RestSqlStatsAction()); @@ -167,9 +164,4 @@ public MediaTypeRegistry getAdditionalMediaTypes() { return mediaTypeRegistry; } - - @Override - public void setGlobalMediaTypeRegistry(MediaTypeRegistry globalMediaTypeRegistry) { - this.globalMediaTypeRegistry = globalMediaTypeRegistry; - } }