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

Serialize MediaType as an integer to avoid string parsing #16073

Closed
wants to merge 1 commit into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import org.opensearch.core.common.text.Text;
import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException;
import org.opensearch.core.xcontent.MediaType;
import org.opensearch.core.xcontent.MediaTypeRegistry;
import org.opensearch.semver.SemverRange;

import java.io.ByteArrayInputStream;
Expand Down Expand Up @@ -353,7 +352,7 @@
}

public MediaType readMediaType() throws IOException {
return MediaTypeRegistry.fromMediaType(readString());
return MediaType.readFrom(this);

Check warning on line 355 in libs/core/src/main/java/org/opensearch/core/common/io/stream/StreamInput.java

View check run for this annotation

Codecov / codecov/patch

libs/core/src/main/java/org/opensearch/core/common/io/stream/StreamInput.java#L355

Added line #L355 was not covered by tests
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@

package org.opensearch.core.xcontent;

import org.opensearch.Version;
import org.opensearch.common.annotation.PublicApi;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
import org.opensearch.core.common.io.stream.Writeable;

import java.io.IOException;
Expand Down Expand Up @@ -73,6 +76,13 @@
return type() + "/" + subtype();
}

/**
* Unique identifier typically used for binary serialization. Must be distinct
* from the unique IDs of all other MediaTypes registered with {@link MediaTypeRegistry}.
* See {@link MediaType#readFrom} and {@link MediaType#writeTo}.
*/
int uniqueId();

XContent xContent();

boolean detectedXContent(final byte[] bytes, int offset, int length);
Expand All @@ -89,6 +99,29 @@

XContentBuilder contentBuilder(final OutputStream os) throws IOException;

/**
* Serializes this MediaType to the given StreamOutput.
*/
@Override
default void writeTo(StreamOutput output) throws IOException {
if (output.getVersion().onOrAfter(Version.V_2_10_0) && output.getVersion().before(Version.V_3_0_0)) {
output.writeString(this.mediaType());

Check warning on line 108 in libs/core/src/main/java/org/opensearch/core/xcontent/MediaType.java

View check run for this annotation

Codecov / codecov/patch

libs/core/src/main/java/org/opensearch/core/xcontent/MediaType.java#L108

Added line #L108 was not covered by tests
} else {
output.writeVInt(uniqueId());
}
}

/**
* Reads a MediaType instance from the given StreamInput.
*/
static MediaType readFrom(StreamInput in) throws IOException {
if (in.getVersion().onOrAfter(Version.V_2_10_0) && in.getVersion().before(Version.V_3_0_0)) {
return MediaTypeRegistry.fromMediaType(in.readString());
} else {
return MediaTypeRegistry.fromUniqueId(in.readVInt());
}
}

/**
* Accepts a format string, which is most of the time is equivalent to {@link MediaType#subtype()}
* and attempts to match the value to an {@link MediaType}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
public final class MediaTypeRegistry {
private static Map<String, MediaType> formatToMediaType = Map.of();
private static Map<String, MediaType> typeWithSubtypeToMediaType = Map.of();
private static Map<Integer, MediaType> uniqueIdToMediaType = Map.of();

// Default mediaType singleton
private static MediaType DEFAULT_MEDIA_TYPE;
Expand Down Expand Up @@ -84,12 +85,19 @@
// ensures the map is not overwritten:
Map<String, MediaType> typeMap = new HashMap<>(typeWithSubtypeToMediaType);
Map<String, MediaType> formatMap = new HashMap<>(formatToMediaType);
Map<Integer, MediaType> uniqueIdMap = new HashMap<>(uniqueIdToMediaType);
for (MediaType mediaType : acceptedMediaTypes) {
if (formatMap.containsKey(mediaType.format())) {
throw new IllegalArgumentException("unable to register mediaType: [" + mediaType.format() + "]. Type already exists.");
}
if (uniqueIdMap.containsKey(mediaType.uniqueId())) {
throw new IllegalArgumentException(
"unable to register mediaType with ID: [" + mediaType.uniqueId() + "]. ID already exists."

Check warning on line 95 in libs/core/src/main/java/org/opensearch/core/xcontent/MediaTypeRegistry.java

View check run for this annotation

Codecov / codecov/patch

libs/core/src/main/java/org/opensearch/core/xcontent/MediaTypeRegistry.java#L94-L95

Added lines #L94 - L95 were not covered by tests
);
}
typeMap.put(mediaType.typeWithSubtype(), mediaType);
formatMap.put(mediaType.format(), mediaType);
uniqueIdMap.put(mediaType.uniqueId(), mediaType);
}
for (Map.Entry<String, MediaType> entry : additionalMediaTypes.entrySet()) {
String typeWithSubtype = entry.getKey().toLowerCase(Locale.ROOT);
Expand All @@ -111,6 +119,11 @@

formatToMediaType = Map.copyOf(formatMap);
typeWithSubtypeToMediaType = Map.copyOf(typeMap);
uniqueIdToMediaType = Map.copyOf(uniqueIdMap);
}

public static MediaType fromUniqueId(int id) {
return uniqueIdToMediaType.get(id);
}

public static MediaType fromMediaType(String mediaType) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.core.xcontent;

import org.opensearch.Version;
import org.opensearch.common.io.stream.BytesStreamOutput;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.test.OpenSearchTestCase;

import java.io.IOException;

public class MediaTypeSerializationTests extends OpenSearchTestCase {

public void testRoundtrip() throws IOException {
try (BytesStreamOutput output = new BytesStreamOutput()) {
XContentType.JSON.writeTo(output);
XContentType.SMILE.writeTo(output);
XContentType.YAML.writeTo(output);
XContentType.CBOR.writeTo(output);
try (StreamInput in = output.bytes().streamInput()) {
assertEquals(XContentType.JSON, MediaType.readFrom(in));
assertEquals(XContentType.SMILE, MediaType.readFrom(in));
assertEquals(XContentType.YAML, MediaType.readFrom(in));
assertEquals(XContentType.CBOR, MediaType.readFrom(in));
}
}
}

public void testHardcodedOrdinals() throws IOException {
try (BytesStreamOutput output = new BytesStreamOutput()) {
output.writeVInt(0);
output.writeVInt(1);
output.writeVInt(2);
output.writeVInt(3);
try (StreamInput in = output.bytes().streamInput()) {
assertEquals(XContentType.JSON, MediaType.readFrom(in));
assertEquals(XContentType.SMILE, MediaType.readFrom(in));
assertEquals(XContentType.YAML, MediaType.readFrom(in));
assertEquals(XContentType.CBOR, MediaType.readFrom(in));
}
}
}

public void testBackwardsCompatibilityWithSerializedEnums() throws IOException {
// Prior to version 2.10, OpenSearch would serialize XContentType as enums, which
// writes the ordinal as a VInt. This test ensure the new MediaType.readFrom method is
// functionally compatible with this previous approach.
try (BytesStreamOutput output = new BytesStreamOutput()) {
output.writeEnum(XContentType.JSON);
output.writeEnum(XContentType.SMILE);
output.writeEnum(XContentType.YAML);
output.writeEnum(XContentType.CBOR);
try (StreamInput in = output.bytes().streamInput()) {
assertEquals(XContentType.JSON, MediaType.readFrom(in));
assertEquals(XContentType.SMILE, MediaType.readFrom(in));
assertEquals(XContentType.YAML, MediaType.readFrom(in));
assertEquals(XContentType.CBOR, MediaType.readFrom(in));
}
}
}

public void testStringVersion() throws IOException {
try (BytesStreamOutput output = new BytesStreamOutput()) {
output.writeString(XContentType.JSON.mediaType());
output.writeString(XContentType.SMILE.mediaType());
output.writeString(XContentType.YAML.mediaType());
output.writeString(XContentType.CBOR.mediaType());
try (StreamInput in = output.bytes().streamInput()) {
in.setVersion(Version.V_2_11_0);
assertEquals(XContentType.JSON, MediaType.readFrom(in));
assertEquals(XContentType.SMILE, MediaType.readFrom(in));
assertEquals(XContentType.YAML, MediaType.readFrom(in));
assertEquals(XContentType.CBOR, MediaType.readFrom(in));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.opensearch.common.xcontent.json.JsonXContent;
import org.opensearch.common.xcontent.smile.SmileXContent;
import org.opensearch.common.xcontent.yaml.YamlXContent;
import org.opensearch.core.common.io.stream.StreamOutput;
import org.opensearch.core.xcontent.MediaType;
import org.opensearch.core.xcontent.XContent;
import org.opensearch.core.xcontent.XContentBuilder;
Expand Down Expand Up @@ -238,7 +237,7 @@ public XContentBuilder contentBuilder(final OutputStream os) throws IOException
}
};

private int index;
private final int index;

XContentType(int index) {
this.index = index;
Expand All @@ -259,7 +258,7 @@ public String format() {
}

@Override
public void writeTo(StreamOutput output) throws IOException {
output.writeString(this.mediaType());
public int uniqueId() {
return index;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
import org.opensearch.common.xcontent.LoggingDeprecationHandler;
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.action.ActionListener;
import org.opensearch.core.common.bytes.BytesReference;
Expand Down Expand Up @@ -254,11 +253,7 @@ protected PercolateQueryBuilder(String field, Supplier<BytesReference> documentS
}
documents = in.readList(StreamInput::readBytesReference);
if (documents.isEmpty() == false) {
if (in.getVersion().onOrAfter(Version.V_2_10_0)) {
documentXContentType = in.readMediaType();
} else {
documentXContentType = in.readEnum(XContentType.class);
}
documentXContentType = MediaType.readFrom(in);
} else {
documentXContentType = null;
}
Expand Down Expand Up @@ -304,11 +299,7 @@ protected void doWriteTo(StreamOutput out) throws IOException {
out.writeBytesReference(document);
}
if (documents.isEmpty() == false) {
if (out.getVersion().onOrAfter(Version.V_2_10_0)) {
documentXContentType.writeTo(out);
} else {
out.writeEnum((XContentType) documentXContentType);
}
documentXContentType.writeTo(out);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,10 @@

package org.opensearch.action.admin.cluster.storedscripts;

import org.opensearch.Version;
import org.opensearch.action.ActionRequestValidationException;
import org.opensearch.action.support.master.AcknowledgedRequest;
import org.opensearch.common.annotation.PublicApi;
import org.opensearch.common.xcontent.XContentHelper;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.core.common.Strings;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.core.common.io.stream.StreamInput;
Expand Down Expand Up @@ -70,11 +68,7 @@ public PutStoredScriptRequest(StreamInput in) throws IOException {
super(in);
id = in.readOptionalString();
content = in.readBytesReference();
if (in.getVersion().onOrAfter(Version.V_2_10_0)) {
mediaType = in.readMediaType();
} else {
mediaType = in.readEnum(XContentType.class);
}
mediaType = MediaType.readFrom(in);
context = in.readOptionalString();
source = new StoredScriptSource(in);
}
Expand Down Expand Up @@ -154,11 +148,7 @@ public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeOptionalString(id);
out.writeBytesReference(content);
if (out.getVersion().onOrAfter(Version.V_2_10_0)) {
mediaType.writeTo(out);
} else {
out.writeEnum((XContentType) mediaType);
}
mediaType.writeTo(out);
out.writeOptionalString(context);
source.writeTo(out);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
import org.opensearch.common.annotation.PublicApi;
import org.opensearch.common.lucene.uid.Versions;
import org.opensearch.common.xcontent.XContentHelper;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.core.common.Strings;
import org.opensearch.core.common.bytes.BytesArray;
import org.opensearch.core.common.bytes.BytesReference;
Expand Down Expand Up @@ -161,11 +160,7 @@ public IndexRequest(@Nullable ShardId shardId, StreamInput in) throws IOExceptio
isRetry = in.readBoolean();
autoGeneratedTimestamp = in.readLong();
if (in.readBoolean()) {
if (in.getVersion().onOrAfter(Version.V_2_10_0)) {
contentType = in.readMediaType();
} else {
contentType = in.readEnum(XContentType.class);
}
contentType = MediaType.readFrom(in);
} else {
contentType = null;
}
Expand Down Expand Up @@ -672,11 +667,7 @@ private void writeBody(StreamOutput out) throws IOException {
out.writeLong(autoGeneratedTimestamp);
if (contentType != null) {
out.writeBoolean(true);
if (out.getVersion().onOrAfter(Version.V_2_10_0)) {
contentType.writeTo(out);
} else {
out.writeEnum((XContentType) contentType);
}
contentType.writeTo(out);
} else {
out.writeBoolean(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,9 @@

package org.opensearch.action.ingest;

import org.opensearch.Version;
import org.opensearch.action.ActionRequestValidationException;
import org.opensearch.action.support.master.AcknowledgedRequest;
import org.opensearch.common.annotation.PublicApi;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
Expand Down Expand Up @@ -72,11 +70,7 @@ public PutPipelineRequest(StreamInput in) throws IOException {
super(in);
id = in.readString();
source = in.readBytesReference();
if (in.getVersion().onOrAfter(Version.V_2_10_0)) {
mediaType = in.readMediaType();
} else {
mediaType = in.readEnum(XContentType.class);
}
mediaType = MediaType.readFrom(in);
}

PutPipelineRequest() {}
Expand All @@ -103,11 +97,7 @@ public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeString(id);
out.writeBytesReference(source);
if (out.getVersion().onOrAfter(Version.V_2_10_0)) {
mediaType.writeTo(out);
} else {
out.writeEnum((XContentType) mediaType);
}
mediaType.writeTo(out);
}

@Override
Expand Down
Loading
Loading