From 3ee08b34a578643b7e5f59768562a65e10749231 Mon Sep 17 00:00:00 2001 From: Karen X Date: Thu, 4 Dec 2025 00:23:04 +0000 Subject: [PATCH 1/2] [GRPC] Throw exceptions for currently unsupported request-side fields Signed-off-by: Karen X --- CHANGELOG.md | 1 + .../request/document/bulk/BulkRequestProtoUtils.java | 10 ++++++++++ .../request/search/SearchRequestProtoUtils.java | 10 ++++++++++ .../search/SearchSourceBuilderProtoUtils.java | 12 +++++++----- .../search/query/TermsQueryBuilderProtoUtils.java | 3 +++ 5 files changed, 31 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4433e1bbfe9fb..bed091fa5ea59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Bump opensearch-protobufs dependency to 0.24.0 and update transport-grpc module compatibility ([#20059](https://github.com/opensearch-project/OpenSearch/pull/20059)) - Refactor the ShardStats, WarmerStats and IndexingPressureStats class to use the Builder pattern instead of constructors ([#19966](https://github.com/opensearch-project/OpenSearch/pull/19966)) +- Throw exceptions for currently unsupported GRPC request-side fields ([#20162](https://github.com/opensearch-project/OpenSearch/pull/20162)) ### Fixed - Fix Allocation and Rebalance Constraints of WeightFunction are incorrectly reset ([#19012](https://github.com/opensearch-project/OpenSearch/pull/19012)) diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtils.java index 6d4ee623b066b..85e9ab47d873c 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtils.java @@ -85,6 +85,16 @@ public static org.opensearch.action.bulk.BulkRequest prepareRequest(BulkRequest ) ); + // Type is a deprecated field according to the spec, thus no plans to support it + if (request.hasType()) { + throw new UnsupportedOperationException("type param is not supported"); + } + + // TODO support global_params + if (request.hasGlobalParams()) { + throw new UnsupportedOperationException("global_params param is not supported yet"); + } + return bulkRequest; } } 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 34e4091475248..0473c46c3156e 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 @@ -183,6 +183,16 @@ protected static void parseSearchRequest( ? parseTimeValue(request.getCancelAfterTimeInterval(), null, "cancel_after_time_interval") : null ); + + // TODO support typed_keys + if (request.hasTypedKeys()) { + throw new UnsupportedOperationException("typed_keys param is not supported yet"); + } + + // TODO support global_params + if (request.hasGlobalParams()) { + throw new UnsupportedOperationException("global_params param is not supported yet"); + } } /** diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java index 7ab5a604f87f8..ce2e8bf5a4afe 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java @@ -21,7 +21,6 @@ import org.opensearch.transport.grpc.proto.request.common.ScriptProtoUtils; import org.opensearch.transport.grpc.proto.request.search.query.AbstractQueryBuilderProtoUtils; import org.opensearch.transport.grpc.proto.request.search.sort.SortBuilderProtoUtils; -import org.opensearch.transport.grpc.proto.request.search.suggest.SuggestBuilderProtoUtils; import org.opensearch.transport.grpc.spi.QueryBuilderProtoConverterRegistry; import java.io.IOException; @@ -154,15 +153,18 @@ private static void parseNonQueryFields( } // TODO support aggregations - /* - if(protoRequest.hasAggs()){} - */ + if (protoRequest.getAggregationsCount() > 0) { + throw new UnsupportedOperationException("aggregations param is not supported yet"); + } if (protoRequest.hasHighlight()) { searchSourceBuilder.highlighter(HighlightBuilderProtoUtils.fromProto(protoRequest.getHighlight(), registry)); } + + // TODO support suggest if (protoRequest.hasSuggest()) { - searchSourceBuilder.suggest(SuggestBuilderProtoUtils.fromProto(protoRequest.getSuggest())); + throw new UnsupportedOperationException("suggest param is not supported yet"); + // searchSourceBuilder.suggest(SuggestBuilderProtoUtils.fromProto(protoRequest.getSuggest())); } if (protoRequest.getRescoreCount() > 0) { for (Rescore rescore : protoRequest.getRescoreList()) { diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java index 30a8dfcb3c181..c751a5586c68a 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java @@ -249,6 +249,9 @@ private static TermsLookup parseTermsLookup(org.opensearch.protobufs.TermsLookup if (lookup.hasRouting()) { tl.routing(lookup.getRouting()); } + if (lookup.hasStore()) { + tl.store(lookup.getStore()); + } return tl; } } From b4856525274ccf56a554e3008dc62b9a35cc7319 Mon Sep 17 00:00:00 2001 From: Karen X Date: Thu, 4 Dec 2025 21:33:36 +0000 Subject: [PATCH 2/2] tests Signed-off-by: Karen X --- .../bulk/BulkRequestProtoUtilsTests.java | 26 ++++++++++++++ .../search/SearchRequestProtoUtilsTests.java | 36 +++++++++++++++++++ .../SearchSourceBuilderProtoUtilsTests.java | 27 ++++++++++++-- 3 files changed, 86 insertions(+), 3 deletions(-) diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtilsTests.java index df22a8edd83d9..699ab999a40c9 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtilsTests.java @@ -126,4 +126,30 @@ public void testPrepareRequestWithRefreshFalse() { assertNotNull("BulkRequest should not be null", bulkRequest); assertEquals("Refresh policy should be NONE", WriteRequest.RefreshPolicy.NONE, bulkRequest.getRefreshPolicy()); } + + public void testPrepareRequestWithTypeThrowsUnsupportedOperationException() { + // Create a protobuf BulkRequest with deprecated type field + BulkRequest request = BulkRequest.newBuilder().setType("_doc").build(); + + // Call prepareRequest, should throw UnsupportedOperationException + UnsupportedOperationException exception = expectThrows( + UnsupportedOperationException.class, + () -> BulkRequestProtoUtils.prepareRequest(request) + ); + + assertEquals("type param is not supported", exception.getMessage()); + } + + public void testPrepareRequestWithGlobalParamsThrowsUnsupportedOperationException() { + // Create a protobuf BulkRequest with global_params + BulkRequest request = BulkRequest.newBuilder().setGlobalParams(org.opensearch.protobufs.GlobalParams.newBuilder().build()).build(); + + // Call prepareRequest, should throw UnsupportedOperationException + UnsupportedOperationException exception = expectThrows( + UnsupportedOperationException.class, + () -> BulkRequestProtoUtils.prepareRequest(request) + ); + + assertEquals("global_params param is not supported yet", exception.getMessage()); + } } 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 ac318d630c084..2faddbdbcc4ea 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 @@ -388,4 +388,40 @@ public void testParseSearchSourceWithInvalidTerminateAfter() throws IOException ); } + public void testParseSearchRequestWithTypedKeysThrowsUnsupportedOperationException() throws IOException { + // Create a protobuf SearchRequest with typed_keys + org.opensearch.protobufs.SearchRequest protoRequest = org.opensearch.protobufs.SearchRequest.newBuilder() + .setTypedKeys(true) + .build(); + + // Create a SearchRequest to populate + SearchRequest searchRequest = new SearchRequest(); + + // Call the method under test, should throw UnsupportedOperationException + UnsupportedOperationException exception = expectThrows( + UnsupportedOperationException.class, + () -> SearchRequestProtoUtils.parseSearchRequest(searchRequest, protoRequest, namedWriteableRegistry, size -> {}, queryUtils) + ); + + assertEquals("typed_keys param is not supported yet", exception.getMessage()); + } + + public void testParseSearchRequestWithGlobalParamsThrowsUnsupportedOperationException() throws IOException { + // Create a protobuf SearchRequest with global_params + org.opensearch.protobufs.SearchRequest protoRequest = org.opensearch.protobufs.SearchRequest.newBuilder() + .setGlobalParams(org.opensearch.protobufs.GlobalParams.newBuilder().build()) + .build(); + + // Create a SearchRequest to populate + SearchRequest searchRequest = new SearchRequest(); + + // Call the method under test, should throw UnsupportedOperationException + UnsupportedOperationException exception = expectThrows( + UnsupportedOperationException.class, + () -> SearchRequestProtoUtils.parseSearchRequest(searchRequest, protoRequest, namedWriteableRegistry, size -> {}, queryUtils) + ); + + assertEquals("global_params param is not supported yet", exception.getMessage()); + } + } 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 417d8c16d5768..d10a17e89dce7 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 @@ -611,6 +611,24 @@ public void testParseProtoWithExtThrowsUnsupportedOperationException() throws IO assertTrue("Exception message should mention ext param", exception.getMessage().contains("ext param is not supported yet")); } + public void testParseProtoWithAggregationsThrowsUnsupportedOperationException() throws IOException { + // Create a protobuf SearchRequestBody with aggregations + SearchRequestBody protoRequest = SearchRequestBody.newBuilder() + .putAggregations("test_agg", org.opensearch.protobufs.AggregationContainer.newBuilder().build()) + .build(); + + // Create a SearchSourceBuilder to populate + SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); + + // Call the method under test, should throw UnsupportedOperationException + UnsupportedOperationException exception = expectThrows( + UnsupportedOperationException.class, + () -> SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils) + ); + + assertEquals("aggregations param is not supported yet", exception.getMessage()); + } + public void testScriptFieldProtoUtilsFromProto() throws IOException { // Create a protobuf ScriptField ScriptField scriptFieldProto = ScriptField.newBuilder() @@ -732,9 +750,12 @@ public void testParseProtoWithSuggest() throws IOException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); - SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils); - - assertNotNull("SuggestBuilder should not be null", searchSourceBuilder.suggest()); + // suggest is not yet implemented, should throw UnsupportedOperationException + UnsupportedOperationException exception = expectThrows( + UnsupportedOperationException.class, + () -> SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils) + ); + assertEquals("suggest param is not supported yet", exception.getMessage()); } public void testParseProtoWithXSourceIncludes() throws IOException {