Split responsibility for format parsing

client/rest-high-level/src/main/java/org/elasticsearch/client/LicenseClient.java

@@ -217,7 +217,7 @@ static String convertResponseToJson(Response response) throws IOException {
         if (entity.getContentType() == null) {
             throw new IllegalStateException("Elasticsearch didn't return the [Content-Type] header, unable to parse response body");
         }
-        XContentType xContentType = XContentType.fromMediaTypeOrFormat(entity.getContentType().getValue());
+        XContentType xContentType = XContentType.fromMediaType(entity.getContentType().getValue());
         if (xContentType == null) {
             throw new IllegalStateException("Unsupported Content-Type: " + entity.getContentType().getValue());
         }

client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java

@@ -1899,7 +1899,7 @@ protected final <Resp> Resp parseEntity(final HttpEntity entity,
         if (entity.getContentType() == null) {
             throw new IllegalStateException("Elasticsearch didn't return the [Content-Type] header, unable to parse response body");
         }
-        XContentType xContentType = XContentType.fromMediaTypeOrFormat(entity.getContentType().getValue());
+        XContentType xContentType = XContentType.fromMediaType(entity.getContentType().getValue());
         if (xContentType == null) {
             throw new IllegalStateException("Unsupported Content-Type: " + entity.getContentType().getValue());
         }

client/rest-high-level/src/main/java/org/elasticsearch/client/ml/PostDataRequest.java

@@ -47,7 +47,7 @@ public class PostDataRequest implements Validatable, ToXContentObject {
 
     public static final ConstructingObjectParser<PostDataRequest, Void> PARSER =
         new ConstructingObjectParser<>("post_data_request",
-            (a) -> new PostDataRequest((String)a[0], XContentType.fromMediaTypeOrFormat((String)a[1]), new byte[0]));
+            (a) -> new PostDataRequest((String)a[0], XContentType.fromMediaType((String)a[1]), new byte[0]));
 
     static {
         PARSER.declareString(ConstructingObjectParser.constructorArg(), Job.ID);

client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java

@@ -767,7 +767,7 @@ public void testUpdate() throws IOException {
 
         UpdateRequest parsedUpdateRequest = new UpdateRequest();
 
-        XContentType entityContentType = XContentType.fromMediaTypeOrFormat(entity.getContentType().getValue());
+        XContentType entityContentType = XContentType.fromMediaType(entity.getContentType().getValue());
         try (XContentParser parser = createParser(entityContentType.xContent(), entity.getContent())) {
             parsedUpdateRequest.fromXContent(parser);
         }

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentType.java

@@ -25,7 +25,8 @@
 import org.elasticsearch.common.xcontent.yaml.YamlXContent;
 
 import java.util.Locale;
-import java.util.Objects;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * The content type of {@link org.elasticsearch.common.xcontent.XContent}.
@@ -113,26 +114,35 @@ public XContent xContent() {
             return CborXContent.cborXContent;
         }
     };
+    /**
+     * A regexp to allow parsing media types. It covers two use cases.
+     * 1. Media type with a version - requires a custom vnd.elasticserach subtype and a compatible-with parameter
+     * i.e. application/vnd.elasticsearch+json;compatible-with
+     * 2. Media type without a version - for users not using compatible API i.e. application/json
+     */
+    private static final Pattern MEDIA_TYPE_PATTERN = Pattern.compile(
+        //type
+        "^(application|text)/" +
+            "([^;\\s]+)" + //subtype: json,yaml,etc some of these are defined in x-pack so can't be enumerated
+            "(?:\\s*;.*)?$",
+        Pattern.CASE_INSENSITIVE);
 
     /**
      * Accepts either a format string, which is equivalent to {@link XContentType#shortName()} or a media type that optionally has
      * parameters and attempts to match the value to an {@link XContentType}. The comparisons are done in lower case format and this method
      * also supports a wildcard accept for {@code application/*}. This method can be used to parse the {@code Accept} HTTP header or a
      * format query string parameter. This method will return {@code null} if no match is found
      */
-    public static XContentType fromMediaTypeOrFormat(String mediaType) {
+    public static XContentType fromFormat(String mediaType) {
+
         if (mediaType == null) {
             return null;
         }
         for (XContentType type : values()) {
-            if (isSameMediaTypeOrFormatAs(mediaType, type)) {
+            if (type.shortName().equalsIgnoreCase(mediaType)) {
                 return type;
             }
         }
-        final String lowercaseMediaType = mediaType.toLowerCase(Locale.ROOT);
-        if (lowercaseMediaType.startsWith("application/*")) {
-            return JSON;
-        }
 
         return null;
     }
@@ -142,16 +152,34 @@ public static XContentType fromMediaTypeOrFormat(String mediaType) {
      * The provided media type should not include any parameters. This method is suitable for parsing part of the {@code Content-Type}
      * HTTP header. This method will return {@code null} if no match is found
      */
-    public static XContentType fromMediaType(String mediaType) {
-        final String lowercaseMediaType = Objects.requireNonNull(mediaType, "mediaType cannot be null").toLowerCase(Locale.ROOT);
+    public static XContentType fromMediaType(String mediaTypeHeaderValue) {
+        if (mediaTypeHeaderValue == null) {
+            return null;
+        }
+        // we also support newline delimited JSON: http://specs.okfnlabs.org/ndjson/
+        if (mediaTypeHeaderValue.toLowerCase(Locale.ROOT).equals("application/x-ndjson")) {
+            return XContentType.JSON;
+        }
+        if (mediaTypeHeaderValue.toLowerCase(Locale.ROOT).startsWith("application/*")) {
+            return JSON;
+        }
+
+        String mediaType = parseMediaType(mediaTypeHeaderValue);
         for (XContentType type : values()) {
-            if (type.mediaTypeWithoutParameters().equals(lowercaseMediaType)) {
+            if (type.mediaTypeWithoutParameters().equals(mediaType)) {
                 return type;
             }
         }
-        // we also support newline delimited JSON: http://specs.okfnlabs.org/ndjson/
-        if (lowercaseMediaType.toLowerCase(Locale.ROOT).equals("application/x-ndjson")) {
-            return XContentType.JSON;
+
+        return null;
+    }
+
+    public static String parseMediaType(String mediaType) {
+        if (mediaType != null) {
+            Matcher matcher = MEDIA_TYPE_PATTERN.matcher(mediaType);
+            if (matcher.find()) {
+                return (matcher.group(1) + "/" + matcher.group(2)).toLowerCase(Locale.ROOT);
+            }
         }
 
         return null;

server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java

@@ -45,6 +45,7 @@ public abstract class AbstractRestChannel implements RestChannel {
     private final String filterPath;
     private final boolean pretty;
     private final boolean human;
+    private final String acceptHeader;
 
     private BytesStreamOutput bytesOut;
 
@@ -58,7 +59,8 @@ public abstract class AbstractRestChannel implements RestChannel {
     protected AbstractRestChannel(RestRequest request, boolean detailedErrorsEnabled) {
         this.request = request;
         this.detailedErrorsEnabled = detailedErrorsEnabled;
-        this.format = request.param("format", request.header("Accept"));
+        this.format = request.param("format");
+        this.acceptHeader = request.header("Accept");
         this.filterPath = request.param("filter_path", null);
         this.pretty = request.paramAsBoolean("pretty", false);
         this.human = request.paramAsBoolean("human", false);
@@ -96,7 +98,11 @@ public XContentBuilder newBuilder(@Nullable XContentType requestContentType, boo
     public XContentBuilder newBuilder(@Nullable XContentType requestContentType, @Nullable XContentType responseContentType,
             boolean useFiltering) throws IOException {
         if (responseContentType == null) {
-            responseContentType = XContentType.fromMediaTypeOrFormat(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);
+            }
         }
         // try to determine the response content type from the media type or the format query string parameter, with the format parameter
         // taking precedence over the Accept header

server/src/main/java/org/elasticsearch/rest/action/cat/RestTable.java

@@ -51,13 +51,20 @@ public class RestTable {
 
     public static RestResponse buildResponse(Table table, RestChannel channel) throws Exception {
         RestRequest request = channel.request();
-        XContentType xContentType = XContentType.fromMediaTypeOrFormat(request.param("format", request.header("Accept")));
+        XContentType xContentType = getxContentType(request);
         if (xContentType != null) {
             return buildXContentBuilder(table, channel);
         }
         return buildTextPlainResponse(table, channel);
     }
 
+    private static XContentType getxContentType(RestRequest request) {
+        if(request.hasParam("format")) {
+            return XContentType.fromFormat(request.param("format"));
+        }
+        return XContentType.fromMediaType(request.header("Accept"));
+    }
+
     public static RestResponse buildXContentBuilder(Table table, RestChannel channel) throws Exception {
         RestRequest request = channel.request();
         XContentBuilder builder = channel.newBuilder();

server/src/test/java/org/elasticsearch/common/xcontent/XContentTypeTests.java

@@ -29,59 +29,59 @@ public class XContentTypeTests extends ESTestCase {
     public void testFromJson() throws Exception {
         String mediaType = "application/json";
         XContentType expectedXContentType = XContentType.JSON;
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType), equalTo(expectedXContentType));
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType + ";"), equalTo(expectedXContentType));
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType + "; charset=UTF-8"), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType + ";"), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType + "; charset=UTF-8"), equalTo(expectedXContentType));
     }
 
     public void testFromJsonUppercase() throws Exception {
         String mediaType = "application/json".toUpperCase(Locale.ROOT);
         XContentType expectedXContentType = XContentType.JSON;
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType), equalTo(expectedXContentType));
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType + ";"), equalTo(expectedXContentType));
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType + "; charset=UTF-8"), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType + ";"), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType + "; charset=UTF-8"), equalTo(expectedXContentType));
     }
 
     public void testFromYaml() throws Exception {
         String mediaType = "application/yaml";
         XContentType expectedXContentType = XContentType.YAML;
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType), equalTo(expectedXContentType));
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType + ";"), equalTo(expectedXContentType));
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType + "; charset=UTF-8"), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType + ";"), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType + "; charset=UTF-8"), equalTo(expectedXContentType));
     }
 
     public void testFromSmile() throws Exception {
         String mediaType = "application/smile";
         XContentType expectedXContentType = XContentType.SMILE;
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType), equalTo(expectedXContentType));
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType + ";"), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType + ";"), equalTo(expectedXContentType));
     }
 
     public void testFromCbor() throws Exception {
         String mediaType = "application/cbor";
         XContentType expectedXContentType = XContentType.CBOR;
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType), equalTo(expectedXContentType));
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType + ";"), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType + ";"), equalTo(expectedXContentType));
     }
 
     public void testFromWildcard() throws Exception {
         String mediaType = "application/*";
         XContentType expectedXContentType = XContentType.JSON;
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType), equalTo(expectedXContentType));
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType + ";"), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType + ";"), equalTo(expectedXContentType));
     }
 
     public void testFromWildcardUppercase() throws Exception {
         String mediaType = "APPLICATION/*";
         XContentType expectedXContentType = XContentType.JSON;
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType), equalTo(expectedXContentType));
-        assertThat(XContentType.fromMediaTypeOrFormat(mediaType + ";"), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType), equalTo(expectedXContentType));
+        assertThat(XContentType.fromMediaType(mediaType + ";"), equalTo(expectedXContentType));
     }
 
     public void testFromRubbish() throws Exception {
-        assertThat(XContentType.fromMediaTypeOrFormat(null), nullValue());
-        assertThat(XContentType.fromMediaTypeOrFormat(""), nullValue());
-        assertThat(XContentType.fromMediaTypeOrFormat("text/plain"), nullValue());
-        assertThat(XContentType.fromMediaTypeOrFormat("gobbly;goop"), nullValue());
+        assertThat(XContentType.fromMediaType(null), nullValue());
+        assertThat(XContentType.fromMediaType(""), nullValue());
+        assertThat(XContentType.fromMediaType("text/plain"), nullValue());
+        assertThat(XContentType.fromMediaType("gobbly;goop"), nullValue());
     }
 }

server/src/test/java/org/elasticsearch/rest/BytesRestResponseTests.java

@@ -299,7 +299,7 @@ public void testErrorToAndFromXContent() throws IOException {
 
         final XContentType xContentType = randomFrom(XContentType.values());
 
-        Map<String, String> params = Collections.singletonMap("format", xContentType.mediaType());
+        Map<String, String> params = Collections.singletonMap("format", xContentType.shortName());
         RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
         RestChannel channel = detailed ? new DetailedExceptionRestChannel(request) : new SimpleExceptionRestChannel(request);
 

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

@@ -121,7 +121,7 @@ public abstract class ESRestTestCase extends ESTestCase {
      * Convert the entity from a {@link Response} into a map of maps.
      */
     public static Map<String, Object> entityAsMap(Response response) throws IOException {
-        XContentType xContentType = XContentType.fromMediaTypeOrFormat(response.getEntity().getContentType().getValue());
+        XContentType xContentType = XContentType.fromMediaType(response.getEntity().getContentType().getValue());
         // EMPTY and THROW are fine here because `.map` doesn't use named x content or deprecation
         try (XContentParser parser = xContentType.xContent().createParser(
                 NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
@@ -134,7 +134,7 @@ public static Map<String, Object> entityAsMap(Response response) throws IOExcept
      * Convert the entity from a {@link Response} into a list of maps.
      */
     public static List<Object> entityAsList(Response response) throws IOException {
-        XContentType xContentType = XContentType.fromMediaTypeOrFormat(response.getEntity().getContentType().getValue());
+        XContentType xContentType = XContentType.fromMediaType(response.getEntity().getContentType().getValue());
         // EMPTY and THROW are fine here because `.map` doesn't use named x content or deprecation
         try (XContentParser parser = xContentType.xContent().createParser(
             NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
@@ -1190,7 +1190,7 @@ protected static Map<String, Object> getAsMap(final String endpoint) throws IOEx
     }
 
     protected static Map<String, Object> responseAsMap(Response response) throws IOException {
-        XContentType entityContentType = XContentType.fromMediaTypeOrFormat(response.getEntity().getContentType().getValue());
+        XContentType entityContentType = XContentType.fromMediaType(response.getEntity().getContentType().getValue());
         Map<String, Object> responseEntity = XContentHelper.convertToMap(entityContentType.xContent(),
                 response.getEntity().getContent(), false);
         assertNotNull(responseEntity);
@@ -1464,7 +1464,7 @@ protected static void waitForActiveLicense(final RestClient restClient) throws E
             assertOK(response);
 
             try (InputStream is = response.getEntity().getContent()) {
-                XContentType xContentType = XContentType.fromMediaTypeOrFormat(response.getEntity().getContentType().getValue());
+                XContentType xContentType = XContentType.fromMediaType(response.getEntity().getContentType().getValue());
                 final Map<String, ?> map = XContentHelper.convertToMap(xContentType.xContent(), is, true);
                 assertThat(map, notNullValue());
                 assertThat("License must exist", map.containsKey("license"), equalTo(true));

test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestResponse.java

@@ -53,7 +53,7 @@ public ClientYamlTestResponse(Response response) throws IOException {
         this.response = response;
         if (response.getEntity() != null) {
             String contentType = response.getHeader("Content-Type");
-            this.bodyContentType = XContentType.fromMediaTypeOrFormat(contentType);
+            this.bodyContentType = XContentType.fromMediaType(contentType);
             try {
                 byte[] bytes = EntityUtils.toByteArray(response.getEntity());
                 //skip parsing if we got text back (e.g. if we called _cat apis)

test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ObjectPath.java

@@ -44,7 +44,7 @@ public class ObjectPath {
     public static ObjectPath createFromResponse(Response response) throws IOException {
         byte[] bytes = EntityUtils.toByteArray(response.getEntity());
         String contentType = response.getHeader("Content-Type");
-        XContentType xContentType = XContentType.fromMediaTypeOrFormat(contentType);
+        XContentType xContentType = XContentType.fromMediaType(contentType);
         return ObjectPath.createFromXContent(xContentType.xContent(), new BytesArray(bytes));
     }
 

x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/TimeSeriesRestDriver.java

@@ -234,7 +234,7 @@ public static void createIndexWithSettings(RestClient client, String index, Stri
     @SuppressWarnings("unchecked")
     public static Integer getNumberOfSegments(RestClient client, String index) throws IOException {
         Response response = client.performRequest(new Request("GET", index + "/_segments"));
-        XContentType entityContentType = XContentType.fromMediaTypeOrFormat(response.getEntity().getContentType().getValue());
+        XContentType entityContentType = XContentType.fromMediaType(response.getEntity().getContentType().getValue());
         Map<String, Object> responseEntity = XContentHelper.convertToMap(entityContentType.xContent(),
             response.getEntity().getContent(), false);
         responseEntity = (Map<String, Object>) responseEntity.get("indices");