From 28b984c94111b8aa9bac90dfb64ecf23af6dca06 Mon Sep 17 00:00:00 2001 From: Karen X Date: Mon, 29 Sep 2025 03:21:29 +0000 Subject: [PATCH 1/2] upgrade to protobufs 0.18.0 Signed-off-by: Karen X --- CHANGELOG.md | 1 + gradle/libs.versions.toml | 2 +- .../licenses/protobufs-0.13.0.jar.sha1 | 1 - .../licenses/protobufs-0.18.0.jar.sha1 | 1 + .../spi/licenses/protobufs-0.13.0.jar.sha1 | 1 - .../spi/licenses/protobufs-0.18.0.jar.sha1 | 1 + .../spi/QueryBuilderProtoConverterTests.java | 4 +- .../common/FetchSourceContextProtoUtils.java | 8 +- .../bulk/ActiveShardCountProtoUtils.java | 4 +- .../search/CollapseBuilderProtoUtils.java | 5 +- .../search/InnerHitsBuilderProtoUtils.java | 164 ++++++++++-------- .../search/SearchRequestProtoUtils.java | 23 --- .../search/sort/SortBuilderProtoUtils.java | 28 +-- .../OpenSearchExceptionProtoUtils.java | 2 +- .../FetchSourceContextProtoUtilsTests.java | 4 +- .../CollapseBuilderProtoUtilsTests.java | 8 +- .../InnerHitsBuilderProtoUtilsTests.java | 40 +++-- .../search/SearchRequestProtoUtilsTests.java | 59 +------ .../SearchSourceBuilderProtoUtilsTests.java | 18 -- .../search/SortBuilderProtoUtilsTests.java | 28 --- ...BuilderProtoConverterSpiRegistryTests.java | 4 +- .../sort/SortBuilderProtoUtilsTests.java | 131 -------------- .../OpenSearchExceptionProtoUtilsTests.java | 6 +- 23 files changed, 147 insertions(+), 396 deletions(-) delete mode 100644 modules/transport-grpc/licenses/protobufs-0.13.0.jar.sha1 create mode 100644 modules/transport-grpc/licenses/protobufs-0.18.0.jar.sha1 delete mode 100644 modules/transport-grpc/spi/licenses/protobufs-0.13.0.jar.sha1 create mode 100644 modules/transport-grpc/spi/licenses/protobufs-0.18.0.jar.sha1 delete mode 100644 modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SortBuilderProtoUtilsTests.java delete mode 100644 modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/sort/SortBuilderProtoUtilsTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index d8bfc0440bab2..16978a9e2922a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add failureaccess as runtime dependency to transport-grpc module ([#19339](https://github.com/opensearch-project/OpenSearch/pull/19339)) - Migrate usages of deprecated `Operations#union` from Lucene ([#19397](https://github.com/opensearch-project/OpenSearch/pull/19397)) - Delegate primitive write methods with ByteSizeCachingDirectory wrapped IndexOutput ([#19432](https://github.com/opensearch-project/OpenSearch/pull/19432)) +- Bump opensearch-protobufs dependency to 0.18.0 and update transport-grpc module compatibility ([#19447](https://github.com/opensearch-project/OpenSearch/issues/19447)) ### Fixed - Fix unnecessary refreshes on update preparation failures ([#15261](https://github.com/opensearch-project/OpenSearch/issues/15261)) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index b545f1abf6c16..c0cc98edc50f0 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -22,7 +22,7 @@ kotlin = "1.7.10" antlr4 = "4.13.1" guava = "33.2.1-jre" gson = "2.13.2" -opensearchprotobufs = "0.13.0" +opensearchprotobufs = "0.18.0" protobuf = "3.25.8" jakarta_annotation = "1.3.5" google_http_client = "1.44.1" diff --git a/modules/transport-grpc/licenses/protobufs-0.13.0.jar.sha1 b/modules/transport-grpc/licenses/protobufs-0.13.0.jar.sha1 deleted file mode 100644 index 88c44863bef72..0000000000000 --- a/modules/transport-grpc/licenses/protobufs-0.13.0.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -14c2a9be2d5e309d7511f734fd43d7d2077db4da \ No newline at end of file diff --git a/modules/transport-grpc/licenses/protobufs-0.18.0.jar.sha1 b/modules/transport-grpc/licenses/protobufs-0.18.0.jar.sha1 new file mode 100644 index 0000000000000..8b343afc9f899 --- /dev/null +++ b/modules/transport-grpc/licenses/protobufs-0.18.0.jar.sha1 @@ -0,0 +1 @@ +e8dc93c60df892184d7c2010e1bd4f15a777c292 \ No newline at end of file diff --git a/modules/transport-grpc/spi/licenses/protobufs-0.13.0.jar.sha1 b/modules/transport-grpc/spi/licenses/protobufs-0.13.0.jar.sha1 deleted file mode 100644 index 88c44863bef72..0000000000000 --- a/modules/transport-grpc/spi/licenses/protobufs-0.13.0.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -14c2a9be2d5e309d7511f734fd43d7d2077db4da \ No newline at end of file diff --git a/modules/transport-grpc/spi/licenses/protobufs-0.18.0.jar.sha1 b/modules/transport-grpc/spi/licenses/protobufs-0.18.0.jar.sha1 new file mode 100644 index 0000000000000..8b343afc9f899 --- /dev/null +++ b/modules/transport-grpc/spi/licenses/protobufs-0.18.0.jar.sha1 @@ -0,0 +1 @@ +e8dc93c60df892184d7c2010e1bd4f15a777c292 \ No newline at end of file diff --git a/modules/transport-grpc/spi/src/test/java/org/opensearch/transport/grpc/spi/QueryBuilderProtoConverterTests.java b/modules/transport-grpc/spi/src/test/java/org/opensearch/transport/grpc/spi/QueryBuilderProtoConverterTests.java index d1f676a25c189..b89654838598d 100644 --- a/modules/transport-grpc/spi/src/test/java/org/opensearch/transport/grpc/spi/QueryBuilderProtoConverterTests.java +++ b/modules/transport-grpc/spi/src/test/java/org/opensearch/transport/grpc/spi/QueryBuilderProtoConverterTests.java @@ -99,9 +99,7 @@ private QueryContainer createMockTermQueryContainer() { * Helper method to create a mock range query container */ private QueryContainer createMockRangeQueryContainer() { - return QueryContainer.newBuilder() - .setRange(org.opensearch.protobufs.RangeQuery.newBuilder().setField("range_field").build()) - .build(); + return QueryContainer.newBuilder().setRange(org.opensearch.protobufs.RangeQuery.newBuilder().build()).build(); } /** diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/common/FetchSourceContextProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/common/FetchSourceContextProtoUtils.java index f863f20b5578d..07c16fcfd21d8 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/common/FetchSourceContextProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/common/FetchSourceContextProtoUtils.java @@ -45,8 +45,8 @@ public static FetchSourceContext parseFromProtoRequest(org.opensearch.protobufs. // Set up source context if source parameters are provided if (request.hasXSource()) { switch (request.getXSource().getSourceConfigParamCase()) { - case BOOL_VALUE: - fetchSource = request.getXSource().getBoolValue(); + case BOOL: + fetchSource = request.getXSource().getBool(); break; case STRING_ARRAY: sourceIncludes = request.getXSource().getStringArray().getStringArrayList().toArray(new String[0]); @@ -84,8 +84,8 @@ public static FetchSourceContext parseFromProtoRequest(org.opensearch.protobufs. if (request.hasXSource()) { SourceConfigParam source = request.getXSource(); - if (source.hasBoolValue()) { - fetchSource = source.getBoolValue(); + if (source.hasBool()) { + fetchSource = source.getBool(); } else { sourceIncludes = source.getStringArray().getStringArrayList().toArray(new String[0]); } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/ActiveShardCountProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/ActiveShardCountProtoUtils.java index cfb591a860861..fdbfee72e5875 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/ActiveShardCountProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/ActiveShardCountProtoUtils.java @@ -47,8 +47,8 @@ public static ActiveShardCount parseProto(WaitForActiveShards waitForActiveShard default: return ActiveShardCount.DEFAULT; } - case INT32_VALUE: - return ActiveShardCount.from(waitForActiveShards.getInt32Value()); + case INT32: + return ActiveShardCount.from(waitForActiveShards.getInt32()); case WAITFORACTIVESHARDS_NOT_SET: default: return ActiveShardCount.DEFAULT; diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/CollapseBuilderProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/CollapseBuilderProtoUtils.java index c538ebb21fd49..344f33d6a0373 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/CollapseBuilderProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/CollapseBuilderProtoUtils.java @@ -8,10 +8,12 @@ package org.opensearch.transport.grpc.proto.request.search; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.index.query.InnerHitBuilder; import org.opensearch.protobufs.FieldCollapse; import org.opensearch.search.collapse.CollapseBuilder; import java.io.IOException; +import java.util.List; /** * Utility class for converting CollapseBuilder Protocol Buffers to OpenSearch objects. @@ -43,7 +45,8 @@ protected static CollapseBuilder fromProto(FieldCollapse collapseProto) throws I collapseBuilder.setMaxConcurrentGroupRequests(collapseProto.getMaxConcurrentGroupSearches()); } if (collapseProto.getInnerHitsCount() > 0) { - collapseBuilder.setInnerHits(InnerHitsBuilderProtoUtils.fromProto(collapseProto.getInnerHitsList())); + List innerHitBuilders = InnerHitsBuilderProtoUtils.fromProto(collapseProto.getInnerHitsList()); + collapseBuilder.setInnerHits(innerHitBuilders); } return collapseBuilder; diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtils.java index f191ec372f0d6..d6a0ad09fb5c7 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtils.java @@ -7,7 +7,6 @@ */ package org.opensearch.transport.grpc.proto.request.search; -import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.query.InnerHitBuilder; import org.opensearch.protobufs.InnerHits; import org.opensearch.protobufs.ScriptField; @@ -34,84 +33,109 @@ private InnerHitsBuilderProtoUtils() { } /** - * Similar to {@link InnerHitBuilder#fromXContent(XContentParser)} + * Converts a single protobuf InnerHits to an OpenSearch InnerHitBuilder. + * Each InnerHits protobuf message represents ONE inner hit definition. * - * @param innerHits + * @param innerHits the protobuf InnerHits to convert + * @return the converted OpenSearch InnerHitBuilder * @throws IOException if there's an error during parsing */ - protected static InnerHitBuilder fromProto(List innerHits) throws IOException { + public static InnerHitBuilder fromProto(InnerHits innerHits) throws IOException { + if (innerHits == null) { + throw new IllegalArgumentException("InnerHits cannot be null"); + } + InnerHitBuilder innerHitBuilder = new InnerHitBuilder(); - for (InnerHits innerHit : innerHits) { - if (innerHit.hasName()) { - innerHitBuilder.setName(innerHit.getName()); - } - if (innerHit.hasIgnoreUnmapped()) { - innerHitBuilder.setIgnoreUnmapped(innerHit.getIgnoreUnmapped()); - } - if (innerHit.hasFrom()) { - innerHitBuilder.setFrom(innerHit.getFrom()); - } - if (innerHit.hasSize()) { - innerHitBuilder.setSize(innerHit.getSize()); - } - if (innerHit.hasExplain()) { - innerHitBuilder.setExplain(innerHit.getExplain()); - } - if (innerHit.hasVersion()) { - innerHitBuilder.setVersion(innerHit.getVersion()); - } - if (innerHit.hasSeqNoPrimaryTerm()) { - innerHitBuilder.setSeqNoAndPrimaryTerm(innerHit.getSeqNoPrimaryTerm()); - } - if (innerHit.hasTrackScores()) { - innerHitBuilder.setTrackScores(innerHit.getTrackScores()); - } - if (innerHit.getStoredFieldsCount() > 0) { - innerHitBuilder.setStoredFieldNames(innerHit.getStoredFieldsList()); - } - if (innerHit.getDocvalueFieldsCount() > 0) { - List fieldAndFormatList = new ArrayList<>(); - for (org.opensearch.protobufs.FieldAndFormat fieldAndFormat : innerHit.getDocvalueFieldsList()) { - fieldAndFormatList.add(FieldAndFormatProtoUtils.fromProto(fieldAndFormat)); - } - innerHitBuilder.setDocValueFields(fieldAndFormatList); - } - if (innerHit.getFieldsCount() > 0) { - List fieldAndFormatList = new ArrayList<>(); - // TODO: this is not correct, we need to use FieldAndFormatProtoUtils.fromProto() and fix the protobufs in 0.11.0 - for (String fieldName : innerHit.getFieldsList()) { - fieldAndFormatList.add(new FieldAndFormat(fieldName, null)); - } - innerHitBuilder.setFetchFields(fieldAndFormatList); - } - if (innerHit.getScriptFieldsCount() > 0) { - Set scriptFields = new HashSet<>(); - for (Map.Entry entry : innerHit.getScriptFieldsMap().entrySet()) { - String name = entry.getKey(); - ScriptField scriptFieldProto = entry.getValue(); - SearchSourceBuilder.ScriptField scriptField = SearchSourceBuilderProtoUtils.ScriptFieldProtoUtils.fromProto( - name, - scriptFieldProto - ); - scriptFields.add(scriptField); - } - innerHitBuilder.setScriptFields(scriptFields); - } - if (innerHit.getSortCount() > 0) { - innerHitBuilder.setSorts(SortBuilderProtoUtils.fromProto(innerHit.getSortList())); - } - if (innerHit.hasXSource()) { - innerHitBuilder.setFetchSourceContext(FetchSourceContextProtoUtils.fromProto(innerHit.getXSource())); - } - if (innerHit.hasHighlight()) { - innerHitBuilder.setHighlightBuilder(HighlightBuilderProtoUtils.fromProto(innerHit.getHighlight())); + if (innerHits.hasName()) { + innerHitBuilder.setName(innerHits.getName()); + } + if (innerHits.hasIgnoreUnmapped()) { + innerHitBuilder.setIgnoreUnmapped(innerHits.getIgnoreUnmapped()); + } + if (innerHits.hasFrom()) { + innerHitBuilder.setFrom(innerHits.getFrom()); + } + if (innerHits.hasSize()) { + innerHitBuilder.setSize(innerHits.getSize()); + } + if (innerHits.hasExplain()) { + innerHitBuilder.setExplain(innerHits.getExplain()); + } + if (innerHits.hasVersion()) { + innerHitBuilder.setVersion(innerHits.getVersion()); + } + if (innerHits.hasSeqNoPrimaryTerm()) { + innerHitBuilder.setSeqNoAndPrimaryTerm(innerHits.getSeqNoPrimaryTerm()); + } + if (innerHits.hasTrackScores()) { + innerHitBuilder.setTrackScores(innerHits.getTrackScores()); + } + if (innerHits.getStoredFieldsCount() > 0) { + innerHitBuilder.setStoredFieldNames(innerHits.getStoredFieldsList()); + } + if (innerHits.getDocvalueFieldsCount() > 0) { + List fieldAndFormatList = new ArrayList<>(); + for (org.opensearch.protobufs.FieldAndFormat fieldAndFormat : innerHits.getDocvalueFieldsList()) { + fieldAndFormatList.add(FieldAndFormatProtoUtils.fromProto(fieldAndFormat)); } - if (innerHit.hasCollapse()) { - innerHitBuilder.setInnerCollapse(CollapseBuilderProtoUtils.fromProto(innerHit.getCollapse())); + innerHitBuilder.setDocValueFields(fieldAndFormatList); + } + if (innerHits.getFieldsCount() > 0) { + List fieldAndFormatList = new ArrayList<>(); + // TODO: this is not correct, we need to use FieldAndFormatProtoUtils.fromProto() and fix the protobufs in 0.11.0 + for (String fieldName : innerHits.getFieldsList()) { + fieldAndFormatList.add(new FieldAndFormat(fieldName, null)); } + innerHitBuilder.setFetchFields(fieldAndFormatList); + } + if (innerHits.getScriptFieldsCount() > 0) { + Set scriptFields = new HashSet<>(); + for (Map.Entry entry : innerHits.getScriptFieldsMap().entrySet()) { + String name = entry.getKey(); + ScriptField scriptFieldProto = entry.getValue(); + SearchSourceBuilder.ScriptField scriptField = SearchSourceBuilderProtoUtils.ScriptFieldProtoUtils.fromProto( + name, + scriptFieldProto + ); + scriptFields.add(scriptField); + } + innerHitBuilder.setScriptFields(scriptFields); + } + if (innerHits.getSortCount() > 0) { + innerHitBuilder.setSorts(SortBuilderProtoUtils.fromProto(innerHits.getSortList())); + } + if (innerHits.hasXSource()) { + innerHitBuilder.setFetchSourceContext(FetchSourceContextProtoUtils.fromProto(innerHits.getXSource())); + } + if (innerHits.hasHighlight()) { + innerHitBuilder.setHighlightBuilder(HighlightBuilderProtoUtils.fromProto(innerHits.getHighlight())); + } + if (innerHits.hasCollapse()) { + innerHitBuilder.setInnerCollapse(CollapseBuilderProtoUtils.fromProto(innerHits.getCollapse())); } + return innerHitBuilder; } + /** + * Converts a list of protobuf InnerHits to a list of OpenSearch InnerHitBuilder objects. + * Each InnerHits protobuf message represents ONE inner hit definition. + * + * @param innerHitsList the list of protobuf InnerHits to convert + * @return the list of converted OpenSearch InnerHitBuilder objects + * @throws IOException if there's an error during parsing + */ + public static List fromProto(List innerHitsList) throws IOException { + if (innerHitsList == null) { + throw new IllegalArgumentException("InnerHits list cannot be null"); + } + + List innerHitBuilders = new ArrayList<>(); + for (InnerHits innerHits : innerHitsList) { + innerHitBuilders.add(fromProto(innerHits)); + } + return innerHitBuilders; + } + } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtils.java index 8b40849ac6fc7..161dffc18a69e 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtils.java @@ -25,7 +25,6 @@ import org.opensearch.search.fetch.StoredFieldsContext; import org.opensearch.search.fetch.subphase.FetchSourceContext; import org.opensearch.search.internal.SearchContext; -import org.opensearch.search.sort.SortOrder; import org.opensearch.search.suggest.SuggestBuilder; import org.opensearch.transport.client.Client; import org.opensearch.transport.client.node.NodeClient; @@ -271,28 +270,6 @@ protected static void parseSearchSource( } } - if (request.getSortCount() > 0) { - for (SearchRequest.SortOrder sort : request.getSortList()) { - String sortField = sort.getField(); - - if (sort.hasDirection()) { - SearchRequest.SortOrder.Direction direction = sort.getDirection(); - switch (direction) { - case DIRECTION_ASC: - searchSourceBuilder.sort(sortField, SortOrder.ASC); - break; - case DIRECTION_DESC: - searchSourceBuilder.sort(sortField, SortOrder.DESC); - break; - default: - throw new IllegalArgumentException("Unsupported sort direction " + direction.toString()); - } - } else { - searchSourceBuilder.sort(sortField); - } - } - } - if (request.getStatsCount() > 0) { searchSourceBuilder.stats(request.getStatsList()); } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/sort/SortBuilderProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/sort/SortBuilderProtoUtils.java index 67d8e6da4d27e..afc8a6b8b3834 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/sort/SortBuilderProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/sort/SortBuilderProtoUtils.java @@ -8,7 +8,7 @@ package org.opensearch.transport.grpc.proto.request.search.sort; import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.protobufs.SortOptions; +import org.opensearch.protobufs.SortCombinations; import org.opensearch.search.sort.FieldSortBuilder; import org.opensearch.search.sort.ScoreSortBuilder; import org.opensearch.search.sort.SortBuilder; @@ -39,31 +39,9 @@ private SortBuilderProtoUtils() { * @throws IllegalArgumentException if invalid sort combinations are provided * @throws UnsupportedOperationException if sort options are not yet supported */ - public static List> fromProto(List sortProto) { + public static List> fromProto(List sortProto) { List> sortFields = new ArrayList<>(2); - - for (SortOptions sortOptions : sortProto) { - switch (sortOptions.getSortOptionsCase()) { - case STRING: - String name = sortOptions.getString(); - sortFields.add(fieldOrScoreSort(name)); - break; - case SORT_OPTIONS_SCORE: - sortFields.add(new ScoreSortBuilder()); - break; - case SORT_OPTIONS_DOC: - sortFields.add(new FieldSortBuilder("_doc")); - break; - case FIELD_SORT: - // Handle field sort - this is more complex and would need FieldSort parsing - throw new UnsupportedOperationException("Field sort not implemented yet"); - case SORT_OPTIONS_ONE_OF: - throw new UnsupportedOperationException("Sort options oneof not supported yet"); - default: - throw new IllegalArgumentException("Invalid sort options provided: " + sortOptions.getSortOptionsCase()); - } - } - return sortFields; + throw new UnsupportedOperationException("sort not supported yet"); } /** diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/opensearchexception/OpenSearchExceptionProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/opensearchexception/OpenSearchExceptionProtoUtils.java index 3e80c10c21b96..d5606c8f91831 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/opensearchexception/OpenSearchExceptionProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/opensearchexception/OpenSearchExceptionProtoUtils.java @@ -184,7 +184,7 @@ public static Map.Entry headerToProto(String key, L if (values.size() == 1) { return new AbstractMap.SimpleEntry( key, - StringOrStringArray.newBuilder().setStringValue(values.get(0)).build() + StringOrStringArray.newBuilder().setString(values.get(0)).build() ); } else { StringArray.Builder stringArrayBuilder = StringArray.newBuilder(); diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/common/FetchSourceContextProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/common/FetchSourceContextProtoUtilsTests.java index fb7b31bed0cff..0b8d170ea51e2 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/common/FetchSourceContextProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/common/FetchSourceContextProtoUtilsTests.java @@ -22,7 +22,7 @@ public class FetchSourceContextProtoUtilsTests extends OpenSearchTestCase { public void testParseFromProtoRequestWithBoolValue() { // Create a BulkRequest with source as boolean - BulkRequest request = BulkRequest.newBuilder().setXSource(SourceConfigParam.newBuilder().setBoolValue(true).build()).build(); + BulkRequest request = BulkRequest.newBuilder().setXSource(SourceConfigParam.newBuilder().setBool(true).build()).build(); // Parse the request FetchSourceContext context = FetchSourceContextProtoUtils.parseFromProtoRequest(request); @@ -181,7 +181,7 @@ public void testFromProtoWithFilterExcludes() { public void testParseFromProtoRequestWithSearchRequestBoolValue() { // Create a SearchRequest with source as boolean - SearchRequest request = SearchRequest.newBuilder().setXSource(SourceConfigParam.newBuilder().setBoolValue(true).build()).build(); + SearchRequest request = SearchRequest.newBuilder().setXSource(SourceConfigParam.newBuilder().setBool(true).build()).build(); // Parse the request FetchSourceContext context = FetchSourceContextProtoUtils.parseFromProtoRequest(request); diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/CollapseBuilderProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/CollapseBuilderProtoUtilsTests.java index 610ccef7500bc..fc3592c93a066 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/CollapseBuilderProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/CollapseBuilderProtoUtilsTests.java @@ -78,9 +78,11 @@ public void testFromProtoWithMultipleInnerHits() throws IOException { assertNotNull("CollapseBuilder should not be null", collapseBuilder); assertEquals("Field name should match", "user_id", collapseBuilder.getField()); assertNotNull("InnerHits should not be null", collapseBuilder.getInnerHits()); - // The last inner hit in the list should be used - assertEquals("InnerHits name should match the last inner hit", "second_inner_hit", collapseBuilder.getInnerHits().get(0).getName()); - assertEquals("InnerHits size should match the last inner hit", 10, collapseBuilder.getInnerHits().get(0).getSize()); + assertEquals("InnerHits size should match", 2, collapseBuilder.getInnerHits().size()); + assertEquals("InnerHits name should match", "first_inner_hit", collapseBuilder.getInnerHits().get(0).getName()); + assertEquals("InnerHits size should match", 5, collapseBuilder.getInnerHits().get(0).getSize()); + assertEquals("InnerHits name should match", "second_inner_hit", collapseBuilder.getInnerHits().get(1).getName()); + assertEquals("InnerHits size should match", 10, collapseBuilder.getInnerHits().get(1).getSize()); } public void testFromProtoWithAllFields() throws IOException { diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtilsTests.java index c36746d2d92b6..f785f2e0d5074 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtilsTests.java @@ -21,7 +21,6 @@ import java.io.IOException; import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.Set; @@ -41,7 +40,7 @@ public void testFromProtoWithBasicFields() throws IOException { .build(); // Call the method under test - InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(Collections.singletonList(innerHits)); + InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(innerHits); // Verify the result assertNotNull("InnerHitBuilder should not be null", innerHitBuilder); @@ -65,7 +64,7 @@ public void testFromProtoWithStoredFields() throws IOException { .build(); // Call the method under test - InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(Collections.singletonList(innerHits)); + InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(innerHits); // Verify the result assertNotNull("InnerHitBuilder should not be null", innerHitBuilder); @@ -85,7 +84,7 @@ public void testFromProtoWithDocValueFields() throws IOException { .build(); // Call the method under test - InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(Collections.singletonList(innerHits)); + InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(innerHits); // Verify the result assertNotNull("InnerHitBuilder should not be null", innerHitBuilder); @@ -113,7 +112,7 @@ public void testFromProtoWithFetchFields() throws IOException { InnerHits innerHits = InnerHits.newBuilder().setName("test_inner_hits").addFields("field1").addFields("field2").build(); // Call the method under test - InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(Collections.singletonList(innerHits)); + InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(innerHits); // Verify the result assertNotNull("InnerHitBuilder should not be null", innerHitBuilder); @@ -169,7 +168,7 @@ public void testFromProtoWithScriptFields() throws IOException { InnerHits innerHits = innerHitsBuilder.build(); // Call the method under test - InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(Collections.singletonList(innerHits)); + InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(innerHits); // Verify the result assertNotNull("InnerHitBuilder should not be null", innerHitBuilder); @@ -202,7 +201,7 @@ public void testFromProtoWithSource() throws IOException { InnerHits innerHits = InnerHits.newBuilder().setName("test_inner_hits").setXSource(sourceContext).build(); // Call the method under test - InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(Collections.singletonList(innerHits)); + InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(innerHits); // Verify the result assertNotNull("InnerHitBuilder should not be null", innerHitBuilder); @@ -222,24 +221,29 @@ public void testFromProtoWithMultipleInnerHits() throws IOException { List innerHitsList = Arrays.asList(innerHits1, innerHits2); // Call the method under test - InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(innerHitsList); + List innerHitBuilders = InnerHitsBuilderProtoUtils.fromProto(innerHitsList); // Verify the result - assertNotNull("InnerHitBuilder should not be null", innerHitBuilder); - // The last inner hits in the list should override previous ones - assertEquals("Name should match the last inner hits", "inner_hits2", innerHitBuilder.getName()); - assertEquals("Size should match the last inner hits", 20, innerHitBuilder.getSize()); + assertNotNull("InnerHitBuilder list should not be null", innerHitBuilders); + assertEquals("Should have 2 InnerHitBuilders", 2, innerHitBuilders.size()); + + // Check first InnerHitBuilder + InnerHitBuilder innerHitBuilder1 = innerHitBuilders.get(0); + assertEquals("First name should match", "inner_hits1", innerHitBuilder1.getName()); + assertEquals("First size should match", 10, innerHitBuilder1.getSize()); + + // Check second InnerHitBuilder + InnerHitBuilder innerHitBuilder2 = innerHitBuilders.get(1); + assertEquals("Second name should match", "inner_hits2", innerHitBuilder2.getName()); + assertEquals("Second size should match", 20, innerHitBuilder2.getSize()); } public void testFromProtoWithEmptyList() throws IOException { // Call the method under test with an empty list - InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(Collections.emptyList()); + List innerHitBuilders = InnerHitsBuilderProtoUtils.fromProto(Arrays.asList()); // Verify the result - assertNotNull("InnerHitBuilder should not be null", innerHitBuilder); - // Should have default values - assertNull("Name should be null", innerHitBuilder.getName()); - assertEquals("From should be default", 0, innerHitBuilder.getFrom()); - assertEquals("Size should be default", 3, innerHitBuilder.getSize()); + assertNotNull("InnerHitBuilder list should not be null", innerHitBuilders); + assertEquals("Should have 0 InnerHitBuilders", 0, innerHitBuilders.size()); } } diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java index 0253d57eeee94..1a0182ab2b991 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java @@ -133,38 +133,6 @@ public void testParseSearchRequestWithRequestBody() throws IOException { assertTrue("Profile should be true", searchRequest.source().profile()); } - public void testParseSearchSourceWithQueryAndSort() throws IOException { - // Create a protobuf SearchRequest with query and sort - org.opensearch.protobufs.SearchRequest protoRequest = org.opensearch.protobufs.SearchRequest.newBuilder() - .setQ("field:value") - .addSort( - org.opensearch.protobufs.SearchRequest.SortOrder.newBuilder() - .setField("field1") - .setDirection(org.opensearch.protobufs.SearchRequest.SortOrder.Direction.DIRECTION_ASC) - .build() - ) - .addSort( - org.opensearch.protobufs.SearchRequest.SortOrder.newBuilder() - .setField("field2") - .setDirection(org.opensearch.protobufs.SearchRequest.SortOrder.Direction.DIRECTION_DESC) - .build() - ) - .addSort(org.opensearch.protobufs.SearchRequest.SortOrder.newBuilder().setField("field3").build()) - .build(); - - // Create a SearchSourceBuilder to populate - SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); - - // Call the method under test - SearchRequestProtoUtils.parseSearchSource(searchSourceBuilder, protoRequest, size -> {}); - - // Verify the result - assertNotNull("SearchSourceBuilder should not be null", searchSourceBuilder); - assertNotNull("Query should not be null", searchSourceBuilder.query()); - assertNotNull("Sorts should not be null", searchSourceBuilder.sorts()); - assertEquals("Should have 3 sorts", 3, searchSourceBuilder.sorts().size()); - } - public void testParseSearchSourceWithStoredFields() throws IOException { // Create a protobuf SearchRequest with stored fields org.opensearch.protobufs.SearchRequest protoRequest = org.opensearch.protobufs.SearchRequest.newBuilder() @@ -211,7 +179,7 @@ public void testParseSearchSourceWithDocValueFields() throws IOException { public void testParseSearchSourceWithSource() throws IOException { // Create a protobuf SearchRequest with source context org.opensearch.protobufs.SearchRequest protoRequest = org.opensearch.protobufs.SearchRequest.newBuilder() - .setXSource(org.opensearch.protobufs.SourceConfigParam.newBuilder().setBoolValue(true).build()) + .setXSource(org.opensearch.protobufs.SourceConfigParam.newBuilder().setBool(true).build()) .addXSourceIncludes("include1") .addXSourceIncludes("include2") .addXSourceExcludes("exclude1") @@ -398,29 +366,4 @@ public void testParseSearchSourceWithInvalidTerminateAfter() throws IOException ); } - public void testParseSearchSourceWithInvalidSortDirection() throws IOException { - // Create a protobuf SearchRequest with invalid sort direction - org.opensearch.protobufs.SearchRequest protoRequest = org.opensearch.protobufs.SearchRequest.newBuilder() - .addSort( - org.opensearch.protobufs.SearchRequest.SortOrder.newBuilder() - .setField("field1") - .setDirection(org.opensearch.protobufs.SearchRequest.SortOrder.Direction.DIRECTION_UNSPECIFIED) - .build() - ) - .build(); - - // Create a SearchSourceBuilder to populate - SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); - - // Call the method under test, should throw IllegalArgumentException - IllegalArgumentException exception = expectThrows( - IllegalArgumentException.class, - () -> SearchRequestProtoUtils.parseSearchSource(searchSourceBuilder, protoRequest, size -> {}) - ); - - assertTrue( - "Exception message should mention unsupported sort direction", - exception.getMessage().contains("Unsupported sort direction") - ); - } } diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtilsTests.java index 26a83d0abb6d5..760444b1f8904 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtilsTests.java @@ -23,7 +23,6 @@ import org.opensearch.protobufs.ScriptField; import org.opensearch.protobufs.SearchRequestBody; import org.opensearch.protobufs.SlicedScroll; -import org.opensearch.protobufs.SortOptions; import org.opensearch.protobufs.TrackHits; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.test.OpenSearchTestCase; @@ -373,23 +372,6 @@ public void testParseProtoWithIndicesBoost() throws IOException { assertEquals("Should have 2 indexBoosts", 2, searchSourceBuilder.indexBoosts().size()); } - public void testParseProtoWithSortString() throws IOException { - // Create a protobuf SearchRequestBody with sort string - SearchRequestBody protoRequest = SearchRequestBody.newBuilder() - .addSort(SortOptions.newBuilder().setString("field1").build()) - .build(); - - // Create a SearchSourceBuilder to populate - SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); - - // Call the method under test - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); - - // Verify the result - assertNotNull("Sorts should not be null", searchSourceBuilder.sorts()); - assertEquals("Should have 1 sort", 1, searchSourceBuilder.sorts().size()); - } - public void testParseProtoWithPostFilter() throws IOException { // Create a protobuf SearchRequestBody with postFilter SearchRequestBody protoRequest = SearchRequestBody.newBuilder() diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SortBuilderProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SortBuilderProtoUtilsTests.java deleted file mode 100644 index 6c39c2d76f5cd..0000000000000 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SortBuilderProtoUtilsTests.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * 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.transport.grpc.proto.request.search; - -import org.opensearch.search.sort.SortBuilder; -import org.opensearch.test.OpenSearchTestCase; -import org.opensearch.transport.grpc.proto.request.search.sort.SortBuilderProtoUtils; - -import java.util.Collections; -import java.util.List; - -public class SortBuilderProtoUtilsTests extends OpenSearchTestCase { - - public void testFromProtoWithEmptyList() { - // Call the method under test with an empty list - List> sortBuilders = SortBuilderProtoUtils.fromProto(Collections.emptyList()); - - // Verify the result - assertNotNull("SortBuilders list should not be null", sortBuilders); - assertTrue("SortBuilders list should be empty", sortBuilders.isEmpty()); - } -} diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterSpiRegistryTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterSpiRegistryTests.java index d5048fdc4635d..236cd3d9a7f14 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterSpiRegistryTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterSpiRegistryTests.java @@ -161,9 +161,7 @@ private QueryContainer createMockTermQueryContainer() { * Helper method to create a mock range query container */ private QueryContainer createMockRangeQueryContainer() { - return QueryContainer.newBuilder() - .setRange(org.opensearch.protobufs.RangeQuery.newBuilder().setField("range_field").build()) - .build(); + return QueryContainer.newBuilder().setRange(org.opensearch.protobufs.RangeQuery.newBuilder().build()).build(); } /** diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/sort/SortBuilderProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/sort/SortBuilderProtoUtilsTests.java deleted file mode 100644 index 210213c75c981..0000000000000 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/sort/SortBuilderProtoUtilsTests.java +++ /dev/null @@ -1,131 +0,0 @@ -/* - * 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.transport.grpc.proto.request.search.sort; - -import org.opensearch.protobufs.SortOptions; -import org.opensearch.search.sort.FieldSortBuilder; -import org.opensearch.search.sort.ScoreSortBuilder; -import org.opensearch.search.sort.SortBuilder; -import org.opensearch.test.OpenSearchTestCase; - -import java.util.Arrays; -import java.util.Collections; -import java.util.List; - -public class SortBuilderProtoUtilsTests extends OpenSearchTestCase { - - public void testFromProtoWithStringSort() { - SortOptions sortOptions = SortOptions.newBuilder().setString("field_name").build(); - List sortProto = Collections.singletonList(sortOptions); - - List> result = SortBuilderProtoUtils.fromProto(sortProto); - - assertEquals("Should return one sort builder", 1, result.size()); - assertTrue("Should be FieldSortBuilder", result.get(0) instanceof FieldSortBuilder); - FieldSortBuilder fieldSort = (FieldSortBuilder) result.get(0); - assertEquals("Field name should match", "field_name", fieldSort.getFieldName()); - } - - public void testFromProtoWithScoreSort() { - SortOptions sortOptions = SortOptions.newBuilder().setSortOptionsScore(true).build(); - List sortProto = Collections.singletonList(sortOptions); - - List> result = SortBuilderProtoUtils.fromProto(sortProto); - - assertEquals("Should return one sort builder", 1, result.size()); - assertTrue("Should be ScoreSortBuilder", result.get(0) instanceof ScoreSortBuilder); - } - - public void testFromProtoWithDocSort() { - SortOptions sortOptions = SortOptions.newBuilder().setSortOptionsDoc(true).build(); - List sortProto = Collections.singletonList(sortOptions); - - List> result = SortBuilderProtoUtils.fromProto(sortProto); - - assertEquals("Should return one sort builder", 1, result.size()); - assertTrue("Should be FieldSortBuilder", result.get(0) instanceof FieldSortBuilder); - FieldSortBuilder fieldSort = (FieldSortBuilder) result.get(0); - assertEquals("Field name should be _doc", "_doc", fieldSort.getFieldName()); - } - - public void testFromProtoWithMultipleSorts() { - SortOptions scoreSort = SortOptions.newBuilder().setSortOptionsScore(true).build(); - SortOptions fieldSort = SortOptions.newBuilder().setString("another_field").build(); - List sortProto = Arrays.asList(scoreSort, fieldSort); - - List> result = SortBuilderProtoUtils.fromProto(sortProto); - - assertEquals("Should return two sort builders", 2, result.size()); - assertTrue("First should be ScoreSortBuilder", result.get(0) instanceof ScoreSortBuilder); - assertTrue("Second should be FieldSortBuilder", result.get(1) instanceof FieldSortBuilder); - } - - public void testFromProtoWithEmptyList() { - List> result = SortBuilderProtoUtils.fromProto(Collections.emptyList()); - assertEquals("Should return empty list", 0, result.size()); - } - - public void testFromProtoWithFieldSortThrowsException() { - SortOptions sortOptions = SortOptions.newBuilder().setFieldSort(org.opensearch.protobufs.FieldSort.newBuilder().build()).build(); - List sortProto = Collections.singletonList(sortOptions); - - UnsupportedOperationException exception = expectThrows( - UnsupportedOperationException.class, - () -> SortBuilderProtoUtils.fromProto(sortProto) - ); - assertEquals("Exception message should match", "Field sort not implemented yet", exception.getMessage()); - } - - public void testFromProtoWithOneOfThrowsException() { - SortOptions sortOptions = SortOptions.newBuilder() - .setSortOptionsOneOf(org.opensearch.protobufs.SortOptionsOneOf.newBuilder().build()) - .build(); - List sortProto = Collections.singletonList(sortOptions); - - UnsupportedOperationException exception = expectThrows( - UnsupportedOperationException.class, - () -> SortBuilderProtoUtils.fromProto(sortProto) - ); - assertEquals("Exception message should match", "Sort options oneof not supported yet", exception.getMessage()); - } - - public void testFromProtoWithInvalidCaseThrowsException() { - SortOptions sortOptions = SortOptions.newBuilder().build(); - List sortProto = Collections.singletonList(sortOptions); - - IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> SortBuilderProtoUtils.fromProto(sortProto)); - assertTrue( - "Exception message should mention invalid sort options", - exception.getMessage().contains("Invalid sort options provided") - ); - } - - public void testFieldOrScoreSortWithScore() { - SortBuilder result = SortBuilderProtoUtils.fieldOrScoreSort("score"); - assertTrue("Should return ScoreSortBuilder", result instanceof ScoreSortBuilder); - } - - public void testFieldOrScoreSortWithFieldName() { - SortBuilder result = SortBuilderProtoUtils.fieldOrScoreSort("field_name"); - assertTrue("Should return FieldSortBuilder", result instanceof FieldSortBuilder); - FieldSortBuilder fieldSort = (FieldSortBuilder) result; - assertEquals("Field name should match", "field_name", fieldSort.getFieldName()); - } - - public void testFieldOrScoreSortWithEmptyString() { - SortBuilder result = SortBuilderProtoUtils.fieldOrScoreSort(""); - assertTrue("Should return FieldSortBuilder", result instanceof FieldSortBuilder); - FieldSortBuilder fieldSort = (FieldSortBuilder) result; - assertEquals("Field name should be empty string", "", fieldSort.getFieldName()); - } - - public void testFieldOrScoreSortWithNull() { - NullPointerException exception = expectThrows(NullPointerException.class, () -> SortBuilderProtoUtils.fieldOrScoreSort(null)); - assertTrue("Exception should be related to null fieldName", exception.getMessage().contains("Cannot invoke")); - } -} diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/OpenSearchExceptionProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/OpenSearchExceptionProtoUtilsTests.java index 8cfa765aaf0ac..421c0677d1bfc 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/OpenSearchExceptionProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/OpenSearchExceptionProtoUtilsTests.java @@ -173,8 +173,8 @@ public void testHeaderToProtoWithSingleValue() throws IOException { // Verify the conversion assertNotNull("Entry should not be null", entry); assertEquals("Key should match", key, entry.getKey()); - assertTrue("Should be a string value", entry.getValue().hasStringValue()); - assertEquals("Value should match", "test-value", entry.getValue().getStringValue()); + assertTrue("Should be a string value", entry.getValue().hasString()); + assertEquals("Value should match", "test-value", entry.getValue().getString()); assertFalse("Should not have a string array", entry.getValue().hasStringArray()); } @@ -189,7 +189,7 @@ public void testHeaderToProtoWithMultipleValues() throws IOException { // Verify the conversion assertNotNull("Entry should not be null", entry); assertEquals("Key should match", key, entry.getKey()); - assertFalse("Should not be a string value", entry.getValue().hasStringValue()); + assertFalse("Should not be a string value", entry.getValue().hasString()); assertTrue("Should have a string array", entry.getValue().hasStringArray()); assertEquals("Array should have correct size", 3, entry.getValue().getStringArray().getStringArrayCount()); assertEquals("First value should match", "value1", entry.getValue().getStringArray().getStringArray(0)); From fef4f49182eefcbfa1cfe7dbbdb9b588bbb8a95b Mon Sep 17 00:00:00 2001 From: Karen X Date: Mon, 29 Sep 2025 05:18:05 +0000 Subject: [PATCH 2/2] add tests Signed-off-by: Karen X --- .../bulk/ActiveShardCountProtoUtilsTests.java | 16 ++-- .../InnerHitsBuilderProtoUtilsTests.java | 67 +++++++++++++++ .../sort/SortBuilderProtoUtilsTests.java | 81 +++++++++++++++++++ 3 files changed, 159 insertions(+), 5 deletions(-) create mode 100644 modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/sort/SortBuilderProtoUtilsTests.java diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/ActiveShardCountProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/ActiveShardCountProtoUtilsTests.java index 57f2dbdd25768..544952ca73fbe 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/ActiveShardCountProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/ActiveShardCountProtoUtilsTests.java @@ -59,19 +59,25 @@ public void testGetActiveShardCountWithWaitForActiveShardsUnspecified() { assertEquals("Should have DEFAULT active shard count", ActiveShardCount.DEFAULT, result); } - // TODO: WaitForActiveShards structure changed in protobufs 0.8.0 - /* public void testGetActiveShardCountWithWaitForActiveShardsInt32() { - // Create a protobuf BulkRequest with wait_for_active_shards = 2 - WaitForActiveShards waitForActiveShards = WaitForActiveShards.newBuilder().setCount(2).build(); + WaitForActiveShards waitForActiveShards = WaitForActiveShards.newBuilder().setInt32(2).build(); ActiveShardCount result = ActiveShardCountProtoUtils.parseProto(waitForActiveShards); // Verify the result assertEquals("Should have active shard count of 2", ActiveShardCount.from(2), result); } - */ + + public void testGetActiveShardCountWithWaitForActiveShardsInt32Zero() { + // Create a protobuf BulkRequest with wait_for_active_shards = 0 + WaitForActiveShards waitForActiveShards = WaitForActiveShards.newBuilder().setInt32(0).build(); + + ActiveShardCount result = ActiveShardCountProtoUtils.parseProto(waitForActiveShards); + + // Verify the result + assertEquals("Should have active shard count of 0", ActiveShardCount.from(0), result); + } public void testGetActiveShardCountWithWaitForActiveShardsNoCase() { // Create a protobuf BulkRequest with wait_for_active_shards but no case set diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtilsTests.java index f785f2e0d5074..11d3f058b7f35 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/InnerHitsBuilderProtoUtilsTests.java @@ -246,4 +246,71 @@ public void testFromProtoWithEmptyList() throws IOException { assertNotNull("InnerHitBuilder list should not be null", innerHitBuilders); assertEquals("Should have 0 InnerHitBuilders", 0, innerHitBuilders.size()); } + + public void testFromProtoWithNullInnerHits() { + // Test null input validation for single InnerHits + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> InnerHitsBuilderProtoUtils.fromProto((InnerHits) null) + ); + + assertEquals("InnerHits cannot be null", exception.getMessage()); + } + + public void testFromProtoWithNullList() { + // Test null input validation for InnerHits list + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> InnerHitsBuilderProtoUtils.fromProto((List) null) + ); + + assertEquals("InnerHits list cannot be null", exception.getMessage()); + } + + public void testFromProtoWithSort() throws IOException { + // Create a protobuf InnerHits with sort (this will throw UnsupportedOperationException due to SortBuilderProtoUtils) + InnerHits innerHits = InnerHits.newBuilder() + .setName("test_inner_hits") + .addSort(org.opensearch.protobufs.SortCombinations.newBuilder().build()) + .build(); + + // This should throw UnsupportedOperationException from SortBuilderProtoUtils.fromProto + UnsupportedOperationException exception = expectThrows( + UnsupportedOperationException.class, + () -> InnerHitsBuilderProtoUtils.fromProto(innerHits) + ); + + assertEquals("sort not supported yet", exception.getMessage()); + } + + public void testFromProtoWithHighlight() throws IOException { + // Create a protobuf InnerHits with highlight + org.opensearch.protobufs.Highlight highlightProto = org.opensearch.protobufs.Highlight.newBuilder().build(); + + InnerHits innerHits = InnerHits.newBuilder().setName("test_inner_hits").setHighlight(highlightProto).build(); + + // This should throw UnsupportedOperationException from HighlightBuilderProtoUtils.fromProto + UnsupportedOperationException exception = expectThrows( + UnsupportedOperationException.class, + () -> InnerHitsBuilderProtoUtils.fromProto(innerHits) + ); + + assertEquals("highlight not supported yet", exception.getMessage()); + } + + public void testFromProtoWithCollapse() throws IOException { + // Create a protobuf InnerHits with collapse + org.opensearch.protobufs.FieldCollapse collapseProto = org.opensearch.protobufs.FieldCollapse.newBuilder() + .setField("category") + .build(); + + InnerHits innerHits = InnerHits.newBuilder().setName("test_inner_hits").setCollapse(collapseProto).build(); + + // This should work and create the InnerHitBuilder with collapse + InnerHitBuilder innerHitBuilder = InnerHitsBuilderProtoUtils.fromProto(innerHits); + + // Verify the result + assertNotNull("InnerHitBuilder should not be null", innerHitBuilder); + assertNotNull("InnerCollapseBuilder should not be null", innerHitBuilder.getInnerCollapseBuilder()); + } } diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/sort/SortBuilderProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/sort/SortBuilderProtoUtilsTests.java new file mode 100644 index 0000000000000..2099e98da6255 --- /dev/null +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/sort/SortBuilderProtoUtilsTests.java @@ -0,0 +1,81 @@ +/* + * 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.transport.grpc.proto.request.search.sort; + +import org.opensearch.protobufs.SortCombinations; +import org.opensearch.search.sort.FieldSortBuilder; +import org.opensearch.search.sort.ScoreSortBuilder; +import org.opensearch.search.sort.SortBuilder; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.ArrayList; +import java.util.List; + +public class SortBuilderProtoUtilsTests extends OpenSearchTestCase { + + public void testFromProtoThrowsUnsupportedOperation() { + // Create an empty list of SortCombinations + List sortProto = new ArrayList<>(); + + // This should throw UnsupportedOperationException + UnsupportedOperationException exception = expectThrows( + UnsupportedOperationException.class, + () -> SortBuilderProtoUtils.fromProto(sortProto) + ); + + assertEquals("sort not supported yet", exception.getMessage()); + } + + public void testFromProtoWithSortCombinationsThrowsUnsupportedOperation() { + // Create a list with a SortCombination + List sortProto = new ArrayList<>(); + sortProto.add(SortCombinations.newBuilder().build()); + + // This should throw UnsupportedOperationException + UnsupportedOperationException exception = expectThrows( + UnsupportedOperationException.class, + () -> SortBuilderProtoUtils.fromProto(sortProto) + ); + + assertEquals("sort not supported yet", exception.getMessage()); + } + + public void testFieldOrScoreSortWithScoreField() { + // Test with "score" field + SortBuilder sortBuilder = SortBuilderProtoUtils.fieldOrScoreSort("score"); + + assertTrue("Should return ScoreSortBuilder for score field", sortBuilder instanceof ScoreSortBuilder); + } + + public void testFieldOrScoreSortWithRegularField() { + // Test with regular field name + SortBuilder sortBuilder = SortBuilderProtoUtils.fieldOrScoreSort("username"); + + assertTrue("Should return FieldSortBuilder for regular field", sortBuilder instanceof FieldSortBuilder); + FieldSortBuilder fieldSortBuilder = (FieldSortBuilder) sortBuilder; + assertEquals("Field name should match", "username", fieldSortBuilder.getFieldName()); + } + + public void testFieldOrScoreSortWithEmptyField() { + // Test with empty field name + SortBuilder sortBuilder = SortBuilderProtoUtils.fieldOrScoreSort(""); + + assertTrue("Should return FieldSortBuilder for empty field", sortBuilder instanceof FieldSortBuilder); + FieldSortBuilder fieldSortBuilder = (FieldSortBuilder) sortBuilder; + assertEquals("Field name should be empty", "", fieldSortBuilder.getFieldName()); + } + + public void testFieldOrScoreSortWithNullField() { + // Test with null field name - should throw NullPointerException + NullPointerException exception = expectThrows(NullPointerException.class, () -> SortBuilderProtoUtils.fieldOrScoreSort(null)); + + // Verify the exception is thrown (the method doesn't handle null gracefully) + assertNotNull("Should throw NullPointerException for null field", exception); + } +}