Skip to content

Commit

Permalink
[Refactor] MediaTypeParser to MediaTypeParserRegistry (opensearch-pro…
Browse files Browse the repository at this point in the history 
…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 <nknize@apache.org>
  • Loading branch information
@nknize @baba-devv
nknize authored and baba-devv committed Jul 29, 2023
1 parent fdf6479 commit 1b1d440
Show file tree
Hide file tree
Showing 87 changed files with 712 additions and 753 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,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

Expand Down
2 changes: 1 addition & 1 deletion ...p-api-plugin/src/main/java/org/opensearch/plugin/noop/action/bulk/RestNoopBulkAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
63 changes: 24 additions & 39 deletions client/rest-high-level/src/main/java/org/opensearch/client/RequestConverters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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(
Expand All @@ -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())) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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}.
*
Expand Down Expand Up @@ -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;
}

/**
Expand Down
8 changes: 4 additions & 4 deletions client/rest-high-level/src/main/java/org/opensearch/client/RestHighLevelClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down
104 changes: 7 additions & 97 deletions client/rest-high-level/src/main/java/org/opensearch/client/indices/CreateIndexRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<>();

Expand Down Expand Up @@ -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)
*/
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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.
*
Expand All @@ -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;
}

Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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.
*
Expand All @@ -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.
*
Expand Down Expand Up @@ -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);
}
}

Expand Down
Loading

0 comments on commit 1b1d440

Please sign in to comment.