[Refactor] MediaTypeParser to MediaTypeParserRegistry (opensearch-pro…

…ject#8636) Refactors the static MediaTypeParser utility to a dynamic registry to support registration of new MediaType definitions. This is a next step to decoupling the x-content library from core classes in the server module to support modularity and cloud or serverless implementations. Signed-off-by: Nicholas Walter Knize <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
Gaurav614 · Apr 25, 2024 · 97d01be · 97d01be
1 parent 9bdcc41
commit 97d01be
Show file tree

Hide file tree

Showing 87 changed files with 712 additions and 753 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -92,6 +92,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Make Span exporter configurable ([#8620](https://github.com/opensearch-project/OpenSearch/issues/8620))
 - Change InternalSignificantTerms to sum shard-level superset counts only in final reduce ([#8735](https://github.com/opensearch-project/OpenSearch/pull/8735))
 - Exclude 'benchmarks' from codecov report ([#8805](https://github.com/opensearch-project/OpenSearch/pull/8805))
+- [Refactor] MediaTypeParser to MediaTypeParserRegistry ([#8636](https://github.com/opensearch-project/OpenSearch/pull/8636))
 
 ### Deprecated
 

diff --git a/...p-api-plugin/src/main/java/org/opensearch/plugin/noop/action/bulk/RestNoopBulkAction.java b/...p-api-plugin/src/main/java/org/opensearch/plugin/noop/action/bulk/RestNoopBulkAction.java
@@ -99,7 +99,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
             defaultPipeline,
             defaultRequireAlias,
             true,
-            request.getXContentType()
+            request.getMediaType()
         );
 
         // short circuit the call to the transport layer

diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/RequestConverters.java b/client/rest-high-level/src/main/java/org/opensearch/client/RequestConverters.java
@@ -157,7 +157,7 @@ static Request bulk(BulkRequest bulkRequest) throws IOException {
         // Bulk API only supports newline delimited JSON or Smile. Before executing
         // the bulk, we need to check that all requests have the same content-type
         // and this content-type is supported by the Bulk API.
-        XContentType bulkContentType = null;
+        MediaType bulkContentType = null;
         for (int i = 0; i < bulkRequest.numberOfActions(); i++) {
             DocWriteRequest<?> action = bulkRequest.requests().get(i);
 
@@ -245,7 +245,7 @@ static Request bulk(BulkRequest bulkRequest) throws IOException {
             if (opType == DocWriteRequest.OpType.INDEX || opType == DocWriteRequest.OpType.CREATE) {
                 IndexRequest indexRequest = (IndexRequest) action;
                 BytesReference indexSource = indexRequest.source();
-                XContentType indexXContentType = indexRequest.getContentType();
+                MediaType mediaType = indexRequest.getContentType();
 
                 try (
                     XContentParser parser = XContentHelper.createParser(
@@ -257,7 +257,7 @@ static Request bulk(BulkRequest bulkRequest) throws IOException {
                         NamedXContentRegistry.EMPTY,
                         DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
                         indexSource,
-                        indexXContentType
+                        mediaType
                     )
                 ) {
                     try (XContentBuilder builder = XContentBuilder.builder(bulkContentType.xContent())) {
@@ -266,7 +266,7 @@ static Request bulk(BulkRequest bulkRequest) throws IOException {
                     }
                 }
             } else if (opType == DocWriteRequest.OpType.UPDATE) {
-                source = XContentHelper.toXContent((UpdateRequest) action, bulkContentType, false).toBytesRef();
+                source = XContentHelper.toXContent((UpdateRequest) action, bulkContentType, ToXContent.EMPTY_PARAMS, false).toBytesRef();
             }
 
             if (source != null) {
@@ -392,30 +392,30 @@ static Request update(UpdateRequest updateRequest) throws IOException {
         // set for the partial document and the upsert document. This client
         // only accepts update requests that have the same content types set
         // for both doc and upsert.
-        XContentType xContentType = null;
+        MediaType mediaType = null;
         if (updateRequest.doc() != null) {
-            xContentType = updateRequest.doc().getContentType();
+            mediaType = updateRequest.doc().getContentType();
         }
         if (updateRequest.upsertRequest() != null) {
-            XContentType upsertContentType = updateRequest.upsertRequest().getContentType();
-            if ((xContentType != null) && (xContentType != upsertContentType)) {
+            MediaType upsertContentType = updateRequest.upsertRequest().getContentType();
+            if ((mediaType != null) && (mediaType != upsertContentType)) {
                 throw new IllegalStateException(
                     "Update request cannot have different content types for doc ["
-                        + xContentType
+                        + mediaType
                         + "]"
                         + " and upsert ["
                         + upsertContentType
                         + "] documents"
                 );
             } else {
-                xContentType = upsertContentType;
+                mediaType = upsertContentType;
             }
         }
-        if (xContentType == null) {
-            xContentType = Requests.INDEX_CONTENT_TYPE;
+        if (mediaType == null) {
+            mediaType = Requests.INDEX_CONTENT_TYPE;
         }
         request.addParameters(parameters.asMap());
-        request.setEntity(createEntity(updateRequest, xContentType));
+        request.setEntity(createEntity(updateRequest, mediaType));
         return request;
     }
 
@@ -816,14 +816,13 @@ static Request deleteScript(DeleteStoredScriptRequest deleteStoredScriptRequest)
         return request;
     }
 
-    static HttpEntity createEntity(ToXContent toXContent, XContentType xContentType) throws IOException {
-        return createEntity(toXContent, xContentType, ToXContent.EMPTY_PARAMS);
+    static HttpEntity createEntity(ToXContent toXContent, MediaType mediaType) throws IOException {
+        return createEntity(toXContent, mediaType, ToXContent.EMPTY_PARAMS);
     }
 
-    static HttpEntity createEntity(ToXContent toXContent, XContentType xContentType, ToXContent.Params toXContentParams)
-        throws IOException {
-        BytesRef source = XContentHelper.toXContent(toXContent, xContentType, toXContentParams, false).toBytesRef();
-        return new ByteArrayEntity(source.bytes, source.offset, source.length, createContentType(xContentType));
+    static HttpEntity createEntity(ToXContent toXContent, MediaType mediaType, ToXContent.Params toXContentParams) throws IOException {
+        BytesRef source = XContentHelper.toXContent(toXContent, mediaType, toXContentParams, false).toBytesRef();
+        return new ByteArrayEntity(source.bytes, source.offset, source.length, createContentType(mediaType));
     }
 
     static String endpoint(String index, String id) {
@@ -868,20 +867,6 @@ static String endpoint(String[] indices, String endpoint, String type) {
         return new EndpointBuilder().addCommaSeparatedPathParts(indices).addPathPartAsIs(endpoint).addPathPart(type).build();
     }
 
-    /**
-     * Returns a {@link ContentType} from a given {@link XContentType}.
-     *
-     * @param xContentType the {@link XContentType}
-     * @return the {@link ContentType}
-     *
-     * @deprecated use {@link #createContentType(MediaType)} instead
-     */
-    @Deprecated
-    @SuppressForbidden(reason = "Only allowed place to convert a XContentType to a ContentType")
-    public static ContentType createContentType(final XContentType xContentType) {
-        return ContentType.create(xContentType.mediaTypeWithoutParameters(), (Charset) null);
-    }
-
     /**
      * Returns a {@link ContentType} from a given {@link XContentType}.
      *
@@ -1265,28 +1250,28 @@ Params withWaitForEvents(Priority waitForEvents) {
      *
      * @return the {@link IndexRequest}'s content type
      */
-    static XContentType enforceSameContentType(IndexRequest indexRequest, @Nullable XContentType xContentType) {
-        XContentType requestContentType = indexRequest.getContentType();
+    static MediaType enforceSameContentType(IndexRequest indexRequest, @Nullable MediaType mediaType) {
+        MediaType requestContentType = indexRequest.getContentType();
         if (requestContentType != XContentType.JSON && requestContentType != XContentType.SMILE) {
             throw new IllegalArgumentException(
                 "Unsupported content-type found for request with content-type ["
                     + requestContentType
                     + "], only JSON and SMILE are supported"
             );
         }
-        if (xContentType == null) {
+        if (mediaType == null) {
             return requestContentType;
         }
-        if (requestContentType != xContentType) {
+        if (requestContentType != mediaType) {
             throw new IllegalArgumentException(
                 "Mismatching content-type found for request with content-type ["
                     + requestContentType
                     + "], previous requests have content-type ["
-                    + xContentType
+                    + mediaType
                     + "]"
             );
         }
-        return xContentType;
+        return mediaType;
     }
 
     /**

diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/RestHighLevelClient.java b/client/rest-high-level/src/main/java/org/opensearch/client/RestHighLevelClient.java
@@ -88,9 +88,9 @@
 import org.opensearch.core.ParseField;
 import org.opensearch.core.xcontent.ContextParser;
 import org.opensearch.core.xcontent.DeprecationHandler;
+import org.opensearch.core.xcontent.MediaType;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.core.xcontent.XContentParser;
-import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.index.rankeval.RankEvalRequest;
 import org.opensearch.index.rankeval.RankEvalResponse;
 import org.opensearch.index.reindex.BulkByScrollResponse;
@@ -2227,11 +2227,11 @@ protected final <Resp> Resp parseEntity(final HttpEntity entity, final CheckedFu
         if (entity.getContentType() == null) {
             throw new IllegalStateException("OpenSearch didn't return the [Content-Type] header, unable to parse response body");
         }
-        XContentType xContentType = XContentType.fromMediaType(entity.getContentType());
-        if (xContentType == null) {
+        MediaType medaiType = MediaType.fromMediaType(entity.getContentType());
+        if (medaiType == null) {
             throw new IllegalStateException("Unsupported Content-Type: " + entity.getContentType());
         }
-        try (XContentParser parser = xContentType.xContent().createParser(registry, DEPRECATION_HANDLER, entity.getContent())) {
+        try (XContentParser parser = medaiType.xContent().createParser(registry, DEPRECATION_HANDLER, entity.getContent())) {
             return entityParser.apply(parser);
         }
     }

diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/indices/CreateIndexRequest.java b/client/rest-high-level/src/main/java/org/opensearch/client/indices/CreateIndexRequest.java
@@ -44,10 +44,10 @@
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.xcontent.XContentFactory;
 import org.opensearch.common.xcontent.XContentHelper;
-import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.core.ParseField;
 import org.opensearch.core.xcontent.DeprecationHandler;
 import org.opensearch.core.xcontent.MediaType;
+import org.opensearch.core.xcontent.MediaTypeParserRegistry;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.core.xcontent.ToXContentObject;
 import org.opensearch.core.xcontent.XContentBuilder;
@@ -77,7 +77,7 @@ public class CreateIndexRequest extends TimedRequest implements Validatable, ToX
     private Settings settings = EMPTY_SETTINGS;
 
     private BytesReference mappings;
-    private XContentType mappingsXContentType;
+    private MediaType mappingsMediaType;
 
     private final Set<Alias> aliases = new HashSet<>();
 
@@ -123,17 +123,6 @@ public CreateIndexRequest settings(Settings settings) {
         return this;
     }
 
-    /**
-     * The settings to create the index with (either json or yaml format)
-     *
-     * @deprecated use {@link #settings(String source, MediaType mediaType)} instead
-     */
-    @Deprecated
-    public CreateIndexRequest settings(String source, XContentType xContentType) {
-        this.settings = Settings.builder().loadFromSource(source, xContentType).build();
-        return this;
-    }
-
     /**
      * The settings to create the index with (either json or yaml format)
      */
@@ -162,23 +151,8 @@ public BytesReference mappings() {
         return mappings;
     }
 
-    public XContentType mappingsXContentType() {
-        return mappingsXContentType;
-    }
-
-    /**
-     * Adds mapping that will be added when the index gets created.
-     *
-     * Note that the definition should *not* be nested under a type name.
-     *
-     * @param source The mapping source
-     * @param xContentType The content type of the source
-     *
-     * @deprecated use {@link #mapping(String source, MediaType mediaType)} instead
-     */
-    @Deprecated
-    public CreateIndexRequest mapping(String source, XContentType xContentType) {
-        return mapping(new BytesArray(source), xContentType);
+    public MediaType mappingsMediaType() {
+        return mappingsMediaType;
     }
 
     /**
@@ -213,32 +187,14 @@ public CreateIndexRequest mapping(XContentBuilder source) {
      */
     public CreateIndexRequest mapping(Map<String, ?> source) {
         try {
-            XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON);
+            XContentBuilder builder = XContentFactory.contentBuilder(MediaTypeParserRegistry.getDefaultMediaType());
             builder.map(source);
             return mapping(BytesReference.bytes(builder), builder.contentType());
         } catch (IOException e) {
             throw new OpenSearchGenerationException("Failed to generate [" + source + "]", e);
         }
     }
 
-    /**
-     * Adds mapping that will be added when the index gets created.
-     *
-     * Note that the definition should *not* be nested under a type name.
-     *
-     * @param source The mapping source
-     * @param xContentType the content type of the mapping source
-     *
-     * @deprecated use {@link #mapping(BytesReference source, MediaType mediaType)} instead
-     */
-    @Deprecated
-    public CreateIndexRequest mapping(BytesReference source, XContentType xContentType) {
-        Objects.requireNonNull(xContentType);
-        mappings = source;
-        mappingsXContentType = xContentType;
-        return this;
-    }
-
     /**
      * Adds mapping that will be added when the index gets created.
      *
@@ -250,7 +206,7 @@ public CreateIndexRequest mapping(BytesReference source, XContentType xContentTy
     public CreateIndexRequest mapping(BytesReference source, MediaType mediaType) {
         Objects.requireNonNull(mediaType);
         mappings = source;
-        mappingsXContentType = XContentType.fromMediaType(mediaType);
+        mappingsMediaType = mediaType;
         return this;
     }
 
@@ -278,33 +234,13 @@ public CreateIndexRequest aliases(XContentBuilder source) {
         return aliases(BytesReference.bytes(source), source.contentType());
     }
 
-    /**
-     * Sets the aliases that will be associated with the index when it gets created
-     *
-     * @deprecated use {@link #aliases(String, MediaType)} instead
-     */
-    @Deprecated
-    public CreateIndexRequest aliases(String source, XContentType contentType) {
-        return aliases(new BytesArray(source), contentType);
-    }
-
     /**
      * Sets the aliases that will be associated with the index when it gets created
      */
     public CreateIndexRequest aliases(String source, MediaType mediaType) {
         return aliases(new BytesArray(source), mediaType);
     }
 
-    /**
-     * Sets the aliases that will be associated with the index when it gets created
-     *
-     * @deprecated use {@link #aliases(BytesReference source, MediaType contentType)} instead
-     */
-    @Deprecated
-    public CreateIndexRequest aliases(BytesReference source, XContentType contentType) {
-        return aliases(source, (MediaType) contentType);
-    }
-
     /**
      * Sets the aliases that will be associated with the index when it gets created
      */
@@ -345,18 +281,6 @@ public CreateIndexRequest aliases(Collection<Alias> aliases) {
         return this;
     }
 
-    /**
-     * Sets the settings and mappings as a single source.
-     *
-     * Note that the mapping definition should *not* be nested under a type name.
-     *
-     * @deprecated use {@link #source(String, MediaType)} instead
-     */
-    @Deprecated
-    public CreateIndexRequest source(String source, XContentType xContentType) {
-        return source(new BytesArray(source), xContentType);
-    }
-
     /**
      * Sets the settings and mappings as a single source.
      *
@@ -375,20 +299,6 @@ public CreateIndexRequest source(XContentBuilder source) {
         return source(BytesReference.bytes(source), source.contentType());
     }
 
-    /**
-     * Sets the settings and mappings as a single source.
-     *
-     * Note that the mapping definition should *not* be nested under a type name.
-     *
-     * @deprecated use {@link #source(BytesReference, MediaType)} instead
-     */
-    @Deprecated
-    public CreateIndexRequest source(BytesReference source, XContentType xContentType) {
-        Objects.requireNonNull(xContentType);
-        source(XContentHelper.convertToMap(source, false, xContentType).v2());
-        return this;
-    }
-
     /**
      * Sets the settings and mappings as a single source.
      *
@@ -458,7 +368,7 @@ public XContentBuilder innerToXContent(XContentBuilder builder, Params params) t
 
         if (mappings != null) {
             try (InputStream stream = mappings.streamInput()) {
-                builder.rawField(MAPPINGS.getPreferredName(), stream, mappingsXContentType);
+                builder.rawField(MAPPINGS.getPreferredName(), stream, mappingsMediaType);
             }
         }