Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Refactor] MediaTypeParser to MediaTypeParserRegistry #8636

Merged
merged 3 commits into from
Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
defaultPipeline,
defaultRequireAlias,
true,
request.getXContentType()
request.getMediaType()

Check warning on line 102 in client/client-benchmark-noop-api-plugin/src/main/java/org/opensearch/plugin/noop/action/bulk/RestNoopBulkAction.java

View check run for this annotation

Codecov / codecov/patch

client/client-benchmark-noop-api-plugin/src/main/java/org/opensearch/plugin/noop/action/bulk/RestNoopBulkAction.java#L102

Added line #L102 was not covered by tests
);

// short circuit the call to the transport layer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@
// 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 @@
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 @@
NamedXContentRegistry.EMPTY,
DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
indexSource,
indexXContentType
mediaType
)
) {
try (XContentBuilder builder = XContentBuilder.builder(bulkContentType.xContent())) {
Expand All @@ -266,7 +266,7 @@
}
}
} 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 @@
// 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;

Check warning on line 415 in client/rest-high-level/src/main/java/org/opensearch/client/RequestConverters.java

View check run for this annotation

Codecov / codecov/patch

client/rest-high-level/src/main/java/org/opensearch/client/RequestConverters.java#L415

Added line #L415 was not covered by tests
}
request.addParameters(parameters.asMap());
request.setEntity(createEntity(updateRequest, xContentType));
request.setEntity(createEntity(updateRequest, mediaType));
return request;
}

Expand Down Expand Up @@ -816,14 +816,13 @@
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 @@
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 @@
*
* @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
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
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
Loading