From ff7241849149e63a1015f9cc0ba4b72313d75119 Mon Sep 17 00:00:00 2001 From: Andrew Ross Date: Wed, 18 Sep 2024 06:17:56 -0700 Subject: [PATCH] Serialize MediaType as an integer to avoid string parsing Signed-off-by: Andrew Ross --- .../core/common/io/stream/StreamInput.java | 3 +- .../opensearch/core/xcontent/MediaType.java | 33 ++++++++ .../core/xcontent/MediaTypeRegistry.java | 13 +++ .../xcontent/MediaTypeSerializationTests.java | 84 +++++++++++++++++++ .../common/xcontent/XContentType.java | 7 +- .../percolator/PercolateQueryBuilder.java | 13 +-- .../storedscripts/PutStoredScriptRequest.java | 14 +--- .../opensearch/action/index/IndexRequest.java | 13 +-- .../action/ingest/PutPipelineRequest.java | 14 +--- .../ingest/SimulatePipelineRequest.java | 14 +--- .../search/PutSearchPipelineRequest.java | 13 +-- .../termvectors/TermVectorsRequest.java | 13 +-- .../extensions/rest/ExtensionRestRequest.java | 14 +--- .../index/query/MoreLikeThisQueryBuilder.java | 13 +-- .../ingest/PipelineConfiguration.java | 13 +-- .../pipeline/PipelineConfiguration.java | 13 +-- 16 files changed, 156 insertions(+), 131 deletions(-) create mode 100644 libs/core/src/test/java/org/opensearch/core/xcontent/MediaTypeSerializationTests.java diff --git a/libs/core/src/main/java/org/opensearch/core/common/io/stream/StreamInput.java b/libs/core/src/main/java/org/opensearch/core/common/io/stream/StreamInput.java index f4c52cb8a6506..83203d4c52ef2 100644 --- a/libs/core/src/main/java/org/opensearch/core/common/io/stream/StreamInput.java +++ b/libs/core/src/main/java/org/opensearch/core/common/io/stream/StreamInput.java @@ -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; @@ -353,7 +352,7 @@ public BigInteger readBigInteger() throws IOException { } public MediaType readMediaType() throws IOException { - return MediaTypeRegistry.fromMediaType(readString()); + return MediaType.readFrom(this); } @Nullable diff --git a/libs/core/src/main/java/org/opensearch/core/xcontent/MediaType.java b/libs/core/src/main/java/org/opensearch/core/xcontent/MediaType.java index c58b3e80d98b5..1e8dd6a0f2adc 100644 --- a/libs/core/src/main/java/org/opensearch/core/xcontent/MediaType.java +++ b/libs/core/src/main/java/org/opensearch/core/xcontent/MediaType.java @@ -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; @@ -73,6 +76,13 @@ default String typeWithSubtype() { 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); @@ -89,6 +99,29 @@ default String mediaType() { 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()); + } 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}. diff --git a/libs/core/src/main/java/org/opensearch/core/xcontent/MediaTypeRegistry.java b/libs/core/src/main/java/org/opensearch/core/xcontent/MediaTypeRegistry.java index bbb55204712d1..45c4b767837c1 100644 --- a/libs/core/src/main/java/org/opensearch/core/xcontent/MediaTypeRegistry.java +++ b/libs/core/src/main/java/org/opensearch/core/xcontent/MediaTypeRegistry.java @@ -57,6 +57,7 @@ public final class MediaTypeRegistry { private static Map formatToMediaType = Map.of(); private static Map typeWithSubtypeToMediaType = Map.of(); + private static Map uniqueIdToMediaType = Map.of(); // Default mediaType singleton private static MediaType DEFAULT_MEDIA_TYPE; @@ -84,12 +85,19 @@ private static void register(MediaType[] acceptedMediaTypes, Map typeMap = new HashMap<>(typeWithSubtypeToMediaType); Map formatMap = new HashMap<>(formatToMediaType); + Map 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." + ); + } typeMap.put(mediaType.typeWithSubtype(), mediaType); formatMap.put(mediaType.format(), mediaType); + uniqueIdMap.put(mediaType.uniqueId(), mediaType); } for (Map.Entry entry : additionalMediaTypes.entrySet()) { String typeWithSubtype = entry.getKey().toLowerCase(Locale.ROOT); @@ -111,6 +119,11 @@ private static void register(MediaType[] acceptedMediaTypes, Map 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; } @@ -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); } } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/storedscripts/PutStoredScriptRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/storedscripts/PutStoredScriptRequest.java index 8731b18fff338..46311d50315e9 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/storedscripts/PutStoredScriptRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/storedscripts/PutStoredScriptRequest.java @@ -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; @@ -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); } @@ -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); } diff --git a/server/src/main/java/org/opensearch/action/index/IndexRequest.java b/server/src/main/java/org/opensearch/action/index/IndexRequest.java index a5958f8b9f499..6fbce7411e83b 100644 --- a/server/src/main/java/org/opensearch/action/index/IndexRequest.java +++ b/server/src/main/java/org/opensearch/action/index/IndexRequest.java @@ -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; @@ -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; } @@ -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); } diff --git a/server/src/main/java/org/opensearch/action/ingest/PutPipelineRequest.java b/server/src/main/java/org/opensearch/action/ingest/PutPipelineRequest.java index 06e89b5f2908b..a62c4b528df70 100644 --- a/server/src/main/java/org/opensearch/action/ingest/PutPipelineRequest.java +++ b/server/src/main/java/org/opensearch/action/ingest/PutPipelineRequest.java @@ -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; @@ -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() {} @@ -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 diff --git a/server/src/main/java/org/opensearch/action/ingest/SimulatePipelineRequest.java b/server/src/main/java/org/opensearch/action/ingest/SimulatePipelineRequest.java index b51f25d2e62b1..1c0a0af533ade 100644 --- a/server/src/main/java/org/opensearch/action/ingest/SimulatePipelineRequest.java +++ b/server/src/main/java/org/opensearch/action/ingest/SimulatePipelineRequest.java @@ -32,12 +32,10 @@ package org.opensearch.action.ingest; -import org.opensearch.Version; import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.logging.DeprecationLogger; -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; @@ -87,11 +85,7 @@ public SimulatePipelineRequest(BytesReference source, MediaType mediaType) { id = in.readOptionalString(); verbose = in.readBoolean(); 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); } @Override @@ -129,11 +123,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(id); out.writeBoolean(verbose); out.writeBytesReference(source); - if (out.getVersion().onOrAfter(Version.V_2_10_0)) { - mediaType.writeTo(out); - } else { - out.writeEnum((XContentType) mediaType); - } + mediaType.writeTo(out); } @Override diff --git a/server/src/main/java/org/opensearch/action/search/PutSearchPipelineRequest.java b/server/src/main/java/org/opensearch/action/search/PutSearchPipelineRequest.java index 15b4ea648af29..c5ece3e3950c1 100644 --- a/server/src/main/java/org/opensearch/action/search/PutSearchPipelineRequest.java +++ b/server/src/main/java/org/opensearch/action/search/PutSearchPipelineRequest.java @@ -8,7 +8,6 @@ package org.opensearch.action.search; -import org.opensearch.Version; import org.opensearch.action.ActionRequestValidationException; import org.opensearch.action.support.master.AcknowledgedRequest; import org.opensearch.common.annotation.PublicApi; @@ -49,11 +48,7 @@ public PutSearchPipelineRequest(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); } @Override @@ -78,11 +73,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 diff --git a/server/src/main/java/org/opensearch/action/termvectors/TermVectorsRequest.java b/server/src/main/java/org/opensearch/action/termvectors/TermVectorsRequest.java index a761cabb9599a..6ddcbab861c71 100644 --- a/server/src/main/java/org/opensearch/action/termvectors/TermVectorsRequest.java +++ b/server/src/main/java/org/opensearch/action/termvectors/TermVectorsRequest.java @@ -43,7 +43,6 @@ import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.lucene.uid.Versions; import org.opensearch.common.util.set.Sets; -import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.ParseField; import org.opensearch.core.common.bytes.BytesArray; import org.opensearch.core.common.bytes.BytesReference; @@ -189,11 +188,7 @@ public TermVectorsRequest() {} if (in.readBoolean()) { doc = in.readBytesReference(); - if (in.getVersion().onOrAfter(Version.V_2_10_0)) { - mediaType = in.readMediaType(); - } else { - mediaType = in.readEnum(XContentType.class); - } + mediaType = MediaType.readFrom(in); } routing = in.readOptionalString(); preference = in.readOptionalString(); @@ -541,11 +536,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeBoolean(doc != null); if (doc != null) { out.writeBytesReference(doc); - if (out.getVersion().onOrAfter(Version.V_2_10_0)) { - mediaType.writeTo(out); - } else { - out.writeEnum((XContentType) mediaType); - } + mediaType.writeTo(out); } out.writeOptionalString(routing); out.writeOptionalString(preference); diff --git a/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java b/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java index 89df1e4fbde35..c16302d9999eb 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java +++ b/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java @@ -9,9 +9,7 @@ package org.opensearch.extensions.rest; import org.opensearch.OpenSearchParseException; -import org.opensearch.Version; import org.opensearch.common.xcontent.LoggingDeprecationHandler; -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; @@ -106,11 +104,7 @@ public ExtensionRestRequest(StreamInput in) throws IOException { params = in.readMap(StreamInput::readString, StreamInput::readString); headers = in.readMap(StreamInput::readString, StreamInput::readStringList); if (in.readBoolean()) { - if (in.getVersion().onOrAfter(Version.V_2_10_0)) { - mediaType = in.readMediaType(); - } else { - mediaType = in.readEnum(XContentType.class); - } + mediaType = MediaType.readFrom(in); } content = in.readBytesReference(); principalIdentifierToken = in.readString(); @@ -127,11 +121,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeMap(headers, StreamOutput::writeString, StreamOutput::writeStringCollection); out.writeBoolean(mediaType != null); if (mediaType != null) { - if (out.getVersion().onOrAfter(Version.V_2_10_0)) { - mediaType.writeTo(out); - } else { - out.writeEnum((XContentType) mediaType); - } + mediaType.writeTo(out); } out.writeBytesReference(content); out.writeString(principalIdentifierToken); diff --git a/server/src/main/java/org/opensearch/index/query/MoreLikeThisQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/MoreLikeThisQueryBuilder.java index e6472afef2215..e6edc7f1e4cb1 100644 --- a/server/src/main/java/org/opensearch/index/query/MoreLikeThisQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MoreLikeThisQueryBuilder.java @@ -52,7 +52,6 @@ import org.opensearch.common.lucene.search.XMoreLikeThis; import org.opensearch.common.lucene.uid.Versions; import org.opensearch.common.xcontent.XContentFactory; -import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.ParseField; import org.opensearch.core.common.ParsingException; import org.opensearch.core.common.Strings; @@ -236,11 +235,7 @@ public Item(@Nullable String index, XContentBuilder doc) { } if (in.readBoolean()) { doc = (BytesReference) in.readGenericValue(); - if (in.getVersion().onOrAfter(Version.V_2_10_0)) { - mediaType = in.readMediaType(); - } else { - mediaType = in.readEnum(XContentType.class); - } + mediaType = MediaType.readFrom(in); } else { id = in.readString(); } @@ -261,11 +256,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeBoolean(doc != null); if (doc != null) { out.writeGenericValue(doc); - if (out.getVersion().onOrAfter(Version.V_2_10_0)) { - mediaType.writeTo(out); - } else { - out.writeEnum((XContentType) mediaType); - } + mediaType.writeTo(out); } else { out.writeString(id); } diff --git a/server/src/main/java/org/opensearch/ingest/PipelineConfiguration.java b/server/src/main/java/org/opensearch/ingest/PipelineConfiguration.java index 477be3e74f1c7..755e9a5395e1f 100644 --- a/server/src/main/java/org/opensearch/ingest/PipelineConfiguration.java +++ b/server/src/main/java/org/opensearch/ingest/PipelineConfiguration.java @@ -32,7 +32,6 @@ package org.opensearch.ingest; -import org.opensearch.Version; import org.opensearch.cluster.AbstractDiffable; import org.opensearch.cluster.Diff; import org.opensearch.common.annotation.PublicApi; @@ -141,11 +140,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } public static PipelineConfiguration readFrom(StreamInput in) throws IOException { - return new PipelineConfiguration( - in.readString(), - in.readBytesReference(), - in.getVersion().onOrAfter(Version.V_2_10_0) ? in.readMediaType() : in.readEnum(XContentType.class) - ); + return new PipelineConfiguration(in.readString(), in.readBytesReference(), MediaType.readFrom(in)); } public static Diff readDiffFrom(StreamInput in) throws IOException { @@ -161,11 +156,7 @@ public String toString() { public void writeTo(StreamOutput out) throws IOException { out.writeString(id); out.writeBytesReference(config); - if (out.getVersion().onOrAfter(Version.V_2_10_0)) { - mediaType.writeTo(out); - } else { - out.writeEnum((XContentType) mediaType); - } + mediaType.writeTo(out); } @Override diff --git a/server/src/main/java/org/opensearch/search/pipeline/PipelineConfiguration.java b/server/src/main/java/org/opensearch/search/pipeline/PipelineConfiguration.java index 2ed770be60458..881d999287f06 100644 --- a/server/src/main/java/org/opensearch/search/pipeline/PipelineConfiguration.java +++ b/server/src/main/java/org/opensearch/search/pipeline/PipelineConfiguration.java @@ -8,7 +8,6 @@ package org.opensearch.search.pipeline; -import org.opensearch.Version; import org.opensearch.cluster.AbstractDiffable; import org.opensearch.cluster.Diff; import org.opensearch.common.annotation.PublicApi; @@ -122,11 +121,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } public static PipelineConfiguration readFrom(StreamInput in) throws IOException { - return new PipelineConfiguration( - in.readString(), - in.readBytesReference(), - in.getVersion().onOrAfter(Version.V_2_10_0) ? in.readMediaType() : in.readEnum(XContentType.class) - ); + return new PipelineConfiguration(in.readString(), in.readBytesReference(), MediaType.readFrom(in)); } public static Diff readDiffFrom(StreamInput in) throws IOException { @@ -142,11 +137,7 @@ public String toString() { public void writeTo(StreamOutput out) throws IOException { out.writeString(id); out.writeBytesReference(config); - if (out.getVersion().onOrAfter(Version.V_2_10_0)) { - mediaType.writeTo(out); - } else { - out.writeEnum((XContentType) mediaType); - } + mediaType.writeTo(out); } @Override