Addressing comments

Signed-off-by: Vacha Shah <[email protected]>
opensearch-project · Apr 17, 2024 · 288f99c · 288f99c
1 parent 9fc6866
commit 288f99c
Show file tree

Hide file tree

Showing 18 changed files with 90 additions and 75 deletions.
diff --git a/server/build.gradle b/server/build.gradle
@@ -350,32 +350,6 @@ tasks.named("dependencyLicenses").configure {
     }
 }
 
-tasks.named("missingJavadoc").configure {
-  /*
-   * annotate_code in L210 does not add the Generated annotation to nested code generated using protobuf.
-   * TODO: Add support to missingJavadoc task to ignore all such nested classes.
-   *  https://github.com/opensearch-project/OpenSearch/issues/11913
-   */
-  dependsOn("generateProto")
-  javadocMissingIgnore = [
-    "org.opensearch.server.proto.QuerySearchResultProto.QuerySearchResult.RescoreDocIds.setIntegerOrBuilder",
-    "org.opensearch.server.proto.QuerySearchResultProto.QuerySearchResult.RescoreDocIdsOrBuilder",
-    "org.opensearch.server.proto.QuerySearchResultProto.QuerySearchResult.TopDocs.ScoreDocOrBuilder",
-    "org.opensearch.server.proto.QuerySearchResultProto.QuerySearchResult.TopDocsOrBuilder",
-    "org.opensearch.server.proto.QuerySearchResultProto.QuerySearchResult.TopDocsAndMaxScoreOrBuilder",
-    "org.opensearch.server.proto.FetchSearchResultProto.SearchHit.SearchSortValuesOrBuilder",
-    "org.opensearch.server.proto.FetchSearchResultProto.SearchHit.HighlightFieldOrBuilder",
-    "org.opensearch.server.proto.FetchSearchResultProto.SearchHit.DocumentFieldOrBuilder",
-    "org.opensearch.server.proto.FetchSearchResultProto.SearchHit.NestedIdentityOrBuilder",
-    "org.opensearch.server.proto.NodeToNodeMessageProto.NodeToNodeMessage.MessageCase",
-    "org.opensearch.server.proto.NodeToNodeMessageProto.NodeToNodeMessage.ResponseHandlersListOrBuilder",
-    "org.opensearch.server.proto.NodeToNodeMessageProto.NodeToNodeMessage.HeaderOrBuilder",
-    "org.opensearch.server.proto.FetchSearchResultProto.SearchHit.Explanation.ExplanationValueCase",
-    "org.opensearch.server.proto.FetchSearchResultProto.SearchHit.ExplanationOrBuilder",
-    "org.opensearch.server.proto.ShardSearchRequestProto.OriginalIndices.IndicesOptionsOrBuilder",
-  ]
-}
-
 tasks.named("filepermissions").configure {
   mustRunAfter("generateProto")
 }

diff --git a/...t/serializer/DocumentFieldSerializer.java → ...serializer/DocumentFieldDeserializer.java b/...t/serializer/DocumentFieldSerializer.java → ...serializer/DocumentFieldDeserializer.java
@@ -13,9 +13,9 @@
 import java.io.IOException;
 
 /**
- * Serializer for {@link DocumentField} which can be implemented for different types of serialization.
+ * Deserializer for {@link DocumentField} which can be implemented for different types of serde mechanisms.
  */
-public interface DocumentFieldSerializer<T> {
+public interface DocumentFieldDeserializer<T> {
 
     DocumentField createDocumentField(T inputStream) throws IOException;
 

diff --git a/...izer/DocumentFieldProtobufSerializer.java → ...uf/DocumentFieldProtobufDeserializer.java b/...izer/DocumentFieldProtobufSerializer.java → ...uf/DocumentFieldProtobufDeserializer.java
@@ -6,11 +6,12 @@
  * compatible open source license.
  */
 
-package org.opensearch.common.document.serializer;
+package org.opensearch.common.document.serializer.protobuf;
 
 import com.google.protobuf.ByteString;
 import org.opensearch.OpenSearchException;
 import org.opensearch.common.document.DocumentField;
+import org.opensearch.common.document.serializer.DocumentFieldDeserializer;
 import org.opensearch.core.common.text.Text;
 import org.opensearch.server.proto.FetchSearchResultProto;
 import org.opensearch.server.proto.FetchSearchResultProto.DocumentFieldValue;
@@ -29,9 +30,9 @@
 import java.util.Map;
 
 /**
- * Serializer for {@link DocumentField} to/from protobuf.
+ * Deserializer for {@link DocumentField} to/from protobuf.
  */
-public class DocumentFieldProtobufSerializer implements DocumentFieldSerializer<InputStream> {
+public class DocumentFieldProtobufDeserializer implements DocumentFieldDeserializer<InputStream> {
 
     private FetchSearchResultProto.SearchHit.DocumentField documentField;
 
@@ -135,7 +136,7 @@ public static FetchSearchResultProto.SearchHit.DocumentField convertDocumentFiel
         builder.setName(documentField.getName());
         for (Object value : documentField.getValues()) {
             FetchSearchResultProto.DocumentFieldValue.Builder valueBuilder = FetchSearchResultProto.DocumentFieldValue.newBuilder();
-            builder.addValues(DocumentFieldProtobufSerializer.convertDocumentFieldValueToProto(value, valueBuilder));
+            builder.addValues(convertDocumentFieldValueToProto(value, valueBuilder));
         }
         return builder.build();
     }

diff --git a/server/src/main/java/org/opensearch/common/document/serializer/protobuf/package-info.java b/server/src/main/java/org/opensearch/common/document/serializer/protobuf/package-info.java
@@ -0,0 +1,10 @@
+/*
+ * 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.
+ */
+
+/** Protobuf Serializer package for documents. */
+package org.opensearch.common.document.serializer.protobuf;
diff --git a/server/src/main/java/org/opensearch/search/SearchSortValues.java b/server/src/main/java/org/opensearch/search/SearchSortValues.java
@@ -68,7 +68,12 @@ public class SearchSortValues implements ToXContentFragment, Writeable {
     }
 
     public SearchSortValues(Object[] sortValues, Object[] rawSortValues) {
-        this.formattedSortValues = Objects.requireNonNull(sortValues, "sort values must not be empty");
+        Objects.requireNonNull(rawSortValues);
+        Objects.requireNonNull(sortValues);
+        if (rawSortValues.length != sortValues.length) {
+            throw new IllegalArgumentException("formattedSortValues and sortValues must hold the same number of items");
+        }
+        this.formattedSortValues = sortValues;
         this.rawSortValues = rawSortValues;
     }
 

diff --git a/server/src/main/java/org/opensearch/search/fetch/FetchSearchResult.java b/server/src/main/java/org/opensearch/search/fetch/FetchSearchResult.java
@@ -42,7 +42,7 @@
 import org.opensearch.search.SearchShardTarget;
 import org.opensearch.search.internal.ShardSearchContextId;
 import org.opensearch.search.query.QuerySearchResult;
-import org.opensearch.search.serializer.SearchHitsProtobufSerializer;
+import org.opensearch.search.serializer.protobuf.SearchHitsProtobufDeserializer;
 import org.opensearch.server.proto.FetchSearchResultProto;
 import org.opensearch.server.proto.ShardSearchRequestProto;
 
@@ -78,7 +78,7 @@ public FetchSearchResult(InputStream in) throws IOException {
             this.fetchSearchResultProto.getContextId().getSessionId(),
             this.fetchSearchResultProto.getContextId().getId()
         );
-        SearchHitsProtobufSerializer protobufSerializer = new SearchHitsProtobufSerializer();
+        SearchHitsProtobufDeserializer protobufSerializer = new SearchHitsProtobufDeserializer();
         hits = protobufSerializer.createSearchHits(new ByteArrayInputStream(this.fetchSearchResultProto.getHits().toByteArray()));
     }
 
@@ -107,7 +107,7 @@ public void hits(SearchHits hits) {
         this.hits = hits;
         if (FeatureFlags.isEnabled(FeatureFlags.PROTOBUF_SETTING) && this.fetchSearchResultProto != null) {
             this.fetchSearchResultProto = this.fetchSearchResultProto.toBuilder()
-                .setHits(SearchHitsProtobufSerializer.convertHitsToProto(hits))
+                .setHits(SearchHitsProtobufDeserializer.convertHitsToProto(hits))
                 .build();
         }
     }
@@ -123,7 +123,7 @@ public SearchHits hits() {
         if (FeatureFlags.isEnabled(FeatureFlags.PROTOBUF_SETTING) && this.fetchSearchResultProto != null) {
             SearchHits hits;
             try {
-                SearchHitsProtobufSerializer protobufSerializer = new SearchHitsProtobufSerializer();
+                SearchHitsProtobufDeserializer protobufSerializer = new SearchHitsProtobufDeserializer();
                 hits = protobufSerializer.createSearchHits(new ByteArrayInputStream(this.fetchSearchResultProto.getHits().toByteArray()));
                 return hits;
             } catch (IOException e) {

diff --git a/.../serializer/HighlightFieldSerializer.java → ...erializer/HighlightFieldDeserializer.java b/.../serializer/HighlightFieldSerializer.java → ...erializer/HighlightFieldDeserializer.java
@@ -13,9 +13,9 @@
 import java.io.IOException;
 
 /**
- * Serializer for {@link HighlightField} which can be implemented for different types of serialization.
+ * Deserializer for {@link HighlightField} which can be implemented for different types of serde mechanisms.
  */
-public interface HighlightFieldSerializer<T> {
+public interface HighlightFieldDeserializer<T> {
 
     HighlightField createHighLightField(T inputStream) throws IOException;
 }
diff --git a/...zer/HighlightFieldProtobufSerializer.java → ...f/HighlightFieldProtobufDeserializer.java b/...zer/HighlightFieldProtobufSerializer.java → ...f/HighlightFieldProtobufDeserializer.java
@@ -6,10 +6,11 @@
  * compatible open source license.
  */
 
-package org.opensearch.search.fetch.subphase.highlight.serializer;
+package org.opensearch.search.fetch.subphase.highlight.serializer.protobuf;
 
 import org.opensearch.core.common.text.Text;
 import org.opensearch.search.fetch.subphase.highlight.HighlightField;
+import org.opensearch.search.fetch.subphase.highlight.serializer.HighlightFieldDeserializer;
 import org.opensearch.server.proto.FetchSearchResultProto;
 
 import java.io.IOException;
@@ -18,9 +19,9 @@
 import java.util.List;
 
 /**
- * Serializer for {@link HighlightField} to/from protobuf.
+ * Deserializer for {@link HighlightField} to/from protobuf.
  */
-public class HighlightFieldProtobufSerializer implements HighlightFieldSerializer<InputStream> {
+public class HighlightFieldProtobufDeserializer implements HighlightFieldDeserializer<InputStream> {
 
     @Override
     public HighlightField createHighLightField(InputStream inputStream) throws IOException {

diff --git a/...java/org/opensearch/search/fetch/subphase/highlight/serializer/protobuf/package-info.java b/...java/org/opensearch/search/fetch/subphase/highlight/serializer/protobuf/package-info.java
@@ -0,0 +1,10 @@
+/*
+ * 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.
+ */
+
+/** Protobuf Serializer package for highlights. */
+package org.opensearch.search.fetch.subphase.highlight.serializer.protobuf;
diff --git a/.../serializer/NestedIdentitySerializer.java → ...erializer/NestedIdentityDeserializer.java b/.../serializer/NestedIdentitySerializer.java → ...erializer/NestedIdentityDeserializer.java
@@ -13,9 +13,9 @@
 import java.io.IOException;
 
 /**
- * Serializer for {@link NestedIdentity} which can be implemented for different types of serialization.
+ * Deserializer for {@link NestedIdentity} which can be implemented for different types of serde mechanisms.
  */
-public interface NestedIdentitySerializer<T> {
+public interface NestedIdentityDeserializer<T> {
 
-    public NestedIdentity createNestedIdentity(T inputStream) throws IOException, Exception;
+    public NestedIdentity createNestedIdentity(T inputStream) throws IOException;
 }
diff --git a/...earch/serializer/SearchHitSerializer.java → ...rch/serializer/SearchHitDeserializer.java b/...earch/serializer/SearchHitSerializer.java → ...rch/serializer/SearchHitDeserializer.java
@@ -13,9 +13,9 @@
 import java.io.IOException;
 
 /**
- * Serializer for {@link SearchHit} which can be implemented for different types of serialization.
+ * Deserializer for {@link SearchHit} which can be implemented for different types of serde mechanisms.
  */
-public interface SearchHitSerializer<T> {
+public interface SearchHitDeserializer<T> {
 
     SearchHit createSearchHit(T inputStream) throws IOException;
 

diff --git a/...arch/serializer/SearchHitsSerializer.java → ...ch/serializer/SearchHitsDeserializer.java b/...arch/serializer/SearchHitsSerializer.java → ...ch/serializer/SearchHitsDeserializer.java
@@ -13,9 +13,9 @@
 import java.io.IOException;
 
 /**
- * Serializer for {@link SearchHits} which can be implemented for different types of serialization.
+ * Deserializer for {@link SearchHits} which can be implemented for different types of serde mechanisms.
  */
-public interface SearchHitsSerializer<T> {
+public interface SearchHitsDeserializer<T> {
 
     SearchHits createSearchHits(T inputStream) throws IOException;
 

diff --git a/...erializer/SearchSortValuesSerializer.java → ...ializer/SearchSortValuesDeserializer.java b/...erializer/SearchSortValuesSerializer.java → ...ializer/SearchSortValuesDeserializer.java
@@ -13,9 +13,9 @@
 import java.io.IOException;
 
 /**
- * Serializer for {@link SearchSortValues} which can be implemented for different types of serialization.
+ * Deserializer for {@link SearchSortValues} which can be implemented for different types of serde mechanisms.
  */
-public interface SearchSortValuesSerializer<T> {
+public interface SearchSortValuesDeserializer<T> {
 
     SearchSortValues createSearchSortValues(T inputStream) throws IOException;
 

diff --git a/...zer/NestedIdentityProtobufSerializer.java → ...f/NestedIdentityProtobufDeserializer.java b/...zer/NestedIdentityProtobufSerializer.java → ...f/NestedIdentityProtobufDeserializer.java
@@ -6,19 +6,20 @@
  * compatible open source license.
  */
 
-package org.opensearch.search.serializer;
+package org.opensearch.search.serializer.protobuf;
 
 import org.opensearch.search.SearchHit.NestedIdentity;
+import org.opensearch.search.serializer.NestedIdentityDeserializer;
 import org.opensearch.server.proto.FetchSearchResultProto;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 
 /**
- * Serializer for {@link NestedIdentity} to/from protobuf.
+ * Deserializer for {@link NestedIdentity} to/from protobuf.
  */
-public class NestedIdentityProtobufSerializer implements NestedIdentitySerializer<InputStream> {
+public class NestedIdentityProtobufDeserializer implements NestedIdentityDeserializer<InputStream> {
 
     @Override
     public NestedIdentity createNestedIdentity(InputStream inputStream) throws IOException {

diff --git a/...rializer/SearchHitProtobufSerializer.java → ...otobuf/SearchHitProtobufDeserializer.java b/...rializer/SearchHitProtobufSerializer.java → ...otobuf/SearchHitProtobufDeserializer.java
@@ -6,14 +6,14 @@
  * compatible open source license.
  */
 
-package org.opensearch.search.serializer;
+package org.opensearch.search.serializer.protobuf;
 
 import com.google.protobuf.ByteString;
 import org.apache.lucene.search.Explanation;
 import org.opensearch.OpenSearchParseException;
 import org.opensearch.action.OriginalIndices;
 import org.opensearch.common.document.DocumentField;
-import org.opensearch.common.document.serializer.DocumentFieldProtobufSerializer;
+import org.opensearch.common.document.serializer.protobuf.DocumentFieldProtobufDeserializer;
 import org.opensearch.core.common.bytes.BytesReference;
 import org.opensearch.core.index.shard.ShardId;
 import org.opensearch.search.SearchHit;
@@ -22,7 +22,8 @@
 import org.opensearch.search.SearchShardTarget;
 import org.opensearch.search.SearchSortValues;
 import org.opensearch.search.fetch.subphase.highlight.HighlightField;
-import org.opensearch.search.fetch.subphase.highlight.serializer.HighlightFieldProtobufSerializer;
+import org.opensearch.search.fetch.subphase.highlight.serializer.protobuf.HighlightFieldProtobufDeserializer;
+import org.opensearch.search.serializer.SearchHitDeserializer;
 import org.opensearch.server.proto.FetchSearchResultProto;
 
 import java.io.ByteArrayInputStream;
@@ -35,9 +36,9 @@
 import java.util.stream.Collectors;
 
 /**
- * Serializer for {@link SearchHit} to/from protobuf.
+ * Deserializer for {@link SearchHit} to/from protobuf.
  */
-public class SearchHitProtobufSerializer implements SearchHitSerializer<InputStream> {
+public class SearchHitProtobufDeserializer implements SearchHitDeserializer<InputStream> {
 
     private FetchSearchResultProto.SearchHit searchHitProto;
 
@@ -49,7 +50,7 @@ public SearchHit createSearchHit(InputStream inputStream) throws IOException {
         String id = this.searchHitProto.getId();
         NestedIdentity nestedIdentity;
         if (!this.searchHitProto.hasNestedIdentity() && this.searchHitProto.getNestedIdentity().toByteArray().length > 0) {
-            NestedIdentityProtobufSerializer protobufSerializer = new NestedIdentityProtobufSerializer();
+            NestedIdentityProtobufDeserializer protobufSerializer = new NestedIdentityProtobufDeserializer();
             nestedIdentity = protobufSerializer.createNestedIdentity(
                 new ByteArrayInputStream(this.searchHitProto.getNestedIdentity().toByteArray())
             );
@@ -64,7 +65,7 @@ public SearchHit createSearchHit(InputStream inputStream) throws IOException {
             source = null;
         }
         Map<String, DocumentField> documentFields = new HashMap<>();
-        DocumentFieldProtobufSerializer protobufSerializer = new DocumentFieldProtobufSerializer();
+        DocumentFieldProtobufDeserializer protobufSerializer = new DocumentFieldProtobufDeserializer();
         this.searchHitProto.getDocumentFieldsMap().forEach((k, v) -> {
             try {
                 documentFields.put(k, protobufSerializer.createDocumentField(new ByteArrayInputStream(v.toByteArray())));
@@ -81,15 +82,15 @@ public SearchHit createSearchHit(InputStream inputStream) throws IOException {
             }
         });
         Map<String, HighlightField> highlightFields = new HashMap<>();
-        HighlightFieldProtobufSerializer highlightFieldProtobufSerializer = new HighlightFieldProtobufSerializer();
+        HighlightFieldProtobufDeserializer highlightFieldProtobufSerializer = new HighlightFieldProtobufDeserializer();
         this.searchHitProto.getHighlightFieldsMap().forEach((k, v) -> {
             try {
                 highlightFields.put(k, highlightFieldProtobufSerializer.createHighLightField(new ByteArrayInputStream(v.toByteArray())));
             } catch (IOException e) {
                 throw new OpenSearchParseException("failed to parse highlight field", e);
             }
         });
-        SearchSortValuesProtobufSerializer sortValueProtobufSerializer = new SearchSortValuesProtobufSerializer();
+        SearchSortValuesProtobufDeserializer sortValueProtobufSerializer = new SearchSortValuesProtobufDeserializer();
         SearchSortValues sortValues = sortValueProtobufSerializer.createSearchSortValues(
             new ByteArrayInputStream(this.searchHitProto.getSortValues().toByteArray())
         );
@@ -129,7 +130,7 @@ public SearchHit createSearchHit(InputStream inputStream) throws IOException {
             innerHits = new HashMap<>();
             this.searchHitProto.getInnerHitsMap().forEach((k, v) -> {
                 try {
-                    SearchHitsProtobufSerializer protobufHitsFactory = new SearchHitsProtobufSerializer();
+                    SearchHitsProtobufDeserializer protobufHitsFactory = new SearchHitsProtobufDeserializer();
                     innerHits.put(k, protobufHitsFactory.createSearchHits(new ByteArrayInputStream(v.toByteArray())));
                 } catch (IOException e) {
                     throw new OpenSearchParseException("failed to parse inner hits", e);
@@ -170,7 +171,7 @@ public static FetchSearchResultProto.SearchHit convertHitToProto(SearchHit hit)
         for (Map.Entry<String, DocumentField> entry : hit.getFields().entrySet()) {
             searchHitBuilder.putDocumentFields(
                 entry.getKey(),
-                DocumentFieldProtobufSerializer.convertDocumentFieldToProto(entry.getValue())
+                DocumentFieldProtobufDeserializer.convertDocumentFieldToProto(entry.getValue())
             );
         }
         return searchHitBuilder.build();