From 2f070c2e0973d5fc38d002098beaff6962be2483 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 Resolves #15979 Signed-off-by: Andrew Ross --- .../core/common/io/stream/StreamInput.java | 2 +- .../opensearch/core/xcontent/MediaType.java | 26 ++++++ .../core/xcontent/MediaTypeRegistry.java | 11 +++ .../xcontent/MediaTypeSerializationTests.java | 84 +++++++++++++++++++ .../common/xcontent/XContentType.java | 12 ++- .../percolator/PercolateQueryBuilder.java | 6 +- .../storedscripts/PutStoredScriptRequest.java | 6 +- .../opensearch/action/index/IndexRequest.java | 6 +- .../action/ingest/PutPipelineRequest.java | 6 +- .../ingest/SimulatePipelineRequest.java | 6 +- .../search/PutSearchPipelineRequest.java | 6 +- .../termvectors/TermVectorsRequest.java | 6 +- .../extensions/rest/ExtensionRestRequest.java | 6 +- .../index/query/MoreLikeThisQueryBuilder.java | 6 +- .../ingest/PipelineConfiguration.java | 2 +- .../pipeline/PipelineConfiguration.java | 2 +- 16 files changed, 144 insertions(+), 49 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..d17375823adac 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 @@ -353,7 +353,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..0bd58586da460 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,22 @@ default String mediaType() { XContentBuilder contentBuilder(final OutputStream os) throws IOException; + default void writeTo(StreamOutput output) throws IOException { + if (output.getVersion().onOrAfter(Version.V_3_0_0)) { + output.writeVInt(uniqueId()); + } else { + output.writeString(this.mediaType()); + } + } + + 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..86e64ae6cb673 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,17 @@ 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 +117,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; } 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..f9360e6765a31 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 @@ -70,11 +70,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); } 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..39d789ac42ac9 100644 --- a/server/src/main/java/org/opensearch/action/index/IndexRequest.java +++ b/server/src/main/java/org/opensearch/action/index/IndexRequest.java @@ -161,11 +161,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; } 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..2dac178039f07 100644 --- a/server/src/main/java/org/opensearch/action/ingest/PutPipelineRequest.java +++ b/server/src/main/java/org/opensearch/action/ingest/PutPipelineRequest.java @@ -72,11 +72,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() {} 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..6158fc7d85679 100644 --- a/server/src/main/java/org/opensearch/action/ingest/SimulatePipelineRequest.java +++ b/server/src/main/java/org/opensearch/action/ingest/SimulatePipelineRequest.java @@ -87,11 +87,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 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..7eba7f180956e 100644 --- a/server/src/main/java/org/opensearch/action/search/PutSearchPipelineRequest.java +++ b/server/src/main/java/org/opensearch/action/search/PutSearchPipelineRequest.java @@ -49,11 +49,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 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..442e538b4ee9b 100644 --- a/server/src/main/java/org/opensearch/action/termvectors/TermVectorsRequest.java +++ b/server/src/main/java/org/opensearch/action/termvectors/TermVectorsRequest.java @@ -189,11 +189,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(); 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..76d3477112b4b 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java +++ b/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java @@ -106,11 +106,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(); 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..e38c785a88ab7 100644 --- a/server/src/main/java/org/opensearch/index/query/MoreLikeThisQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MoreLikeThisQueryBuilder.java @@ -236,11 +236,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(); } diff --git a/server/src/main/java/org/opensearch/ingest/PipelineConfiguration.java b/server/src/main/java/org/opensearch/ingest/PipelineConfiguration.java index 477be3e74f1c7..238292422aa2e 100644 --- a/server/src/main/java/org/opensearch/ingest/PipelineConfiguration.java +++ b/server/src/main/java/org/opensearch/ingest/PipelineConfiguration.java @@ -144,7 +144,7 @@ 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) + MediaType.readFrom(in) ); } 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..72c2b1e921a15 100644 --- a/server/src/main/java/org/opensearch/search/pipeline/PipelineConfiguration.java +++ b/server/src/main/java/org/opensearch/search/pipeline/PipelineConfiguration.java @@ -125,7 +125,7 @@ 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) + MediaType.readFrom(in) ); }