Skip to content

Commit 0449ce8

Browse files
karenyrxkarenyrx
authored
[GRPC] Throw exceptions for currently unsupported request-side fields (#20162)
* [GRPC] Throw exceptions for currently unsupported request-side fields Signed-off-by: Karen X <[email protected]> * tests Signed-off-by: Karen X <[email protected]> --------- Signed-off-by: Karen X <[email protected]>
1 parent 00cf7c1 commit 0449ce8

File tree

8 files changed

+117
-8
lines changed

8 files changed

+117
-8
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
7373
- Bump opensearch-protobufs dependency to 0.24.0 and update transport-grpc module compatibility ([#20059](https://github.com/opensearch-project/OpenSearch/pull/20059))
7474

7575
- Refactor the ShardStats, WarmerStats and IndexingPressureStats class to use the Builder pattern instead of constructors ([#19966](https://github.com/opensearch-project/OpenSearch/pull/19966))
76+
- Throw exceptions for currently unsupported GRPC request-side fields ([#20162](https://github.com/opensearch-project/OpenSearch/pull/20162))
7677

7778
### Fixed
7879
- Fix Allocation and Rebalance Constraints of WeightFunction are incorrectly reset ([#19012](https://github.com/opensearch-project/OpenSearch/pull/19012))

‎modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtils.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,16 @@ public static org.opensearch.action.bulk.BulkRequest prepareRequest(BulkRequest
8585
)
8686
);
8787

88+
// Type is a deprecated field according to the spec, thus no plans to support it
89+
if (request.hasType()) {
90+
throw new UnsupportedOperationException("type param is not supported");
91+
}
92+
93+
// TODO support global_params
94+
if (request.hasGlobalParams()) {
95+
throw new UnsupportedOperationException("global_params param is not supported yet");
96+
}
97+
8898
return bulkRequest;
8999
}
90100
}

‎modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtils.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,16 @@ protected static void parseSearchRequest(
183183
? parseTimeValue(request.getCancelAfterTimeInterval(), null, "cancel_after_time_interval")
184184
: null
185185
);
186+
187+
// TODO support typed_keys
188+
if (request.hasTypedKeys()) {
189+
throw new UnsupportedOperationException("typed_keys param is not supported yet");
190+
}
191+
192+
// TODO support global_params
193+
if (request.hasGlobalParams()) {
194+
throw new UnsupportedOperationException("global_params param is not supported yet");
195+
}
186196
}
187197

188198
/**

‎modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.opensearch.transport.grpc.proto.request.common.ScriptProtoUtils;
2222
import org.opensearch.transport.grpc.proto.request.search.query.AbstractQueryBuilderProtoUtils;
2323
import org.opensearch.transport.grpc.proto.request.search.sort.SortBuilderProtoUtils;
24-
import org.opensearch.transport.grpc.proto.request.search.suggest.SuggestBuilderProtoUtils;
2524
import org.opensearch.transport.grpc.spi.QueryBuilderProtoConverterRegistry;
2625

2726
import java.io.IOException;
@@ -154,15 +153,18 @@ private static void parseNonQueryFields(
154153
}
155154

156155
// TODO support aggregations
157-
/*
158-
if(protoRequest.hasAggs()){}
159-
*/
156+
if (protoRequest.getAggregationsCount() > 0) {
157+
throw new UnsupportedOperationException("aggregations param is not supported yet");
158+
}
160159

161160
if (protoRequest.hasHighlight()) {
162161
searchSourceBuilder.highlighter(HighlightBuilderProtoUtils.fromProto(protoRequest.getHighlight(), registry));
163162
}
163+
164+
// TODO support suggest
164165
if (protoRequest.hasSuggest()) {
165-
searchSourceBuilder.suggest(SuggestBuilderProtoUtils.fromProto(protoRequest.getSuggest()));
166+
throw new UnsupportedOperationException("suggest param is not supported yet");
167+
// searchSourceBuilder.suggest(SuggestBuilderProtoUtils.fromProto(protoRequest.getSuggest()));
166168
}
167169
if (protoRequest.getRescoreCount() > 0) {
168170
for (Rescore rescore : protoRequest.getRescoreList()) {

‎modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,9 @@ private static TermsLookup parseTermsLookup(org.opensearch.protobufs.TermsLookup
249249
if (lookup.hasRouting()) {
250250
tl.routing(lookup.getRouting());
251251
}
252+
if (lookup.hasStore()) {
253+
tl.store(lookup.getStore());
254+
}
252255
return tl;
253256
}
254257
}

‎modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtilsTests.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,4 +126,30 @@ public void testPrepareRequestWithRefreshFalse() {
126126
assertNotNull("BulkRequest should not be null", bulkRequest);
127127
assertEquals("Refresh policy should be NONE", WriteRequest.RefreshPolicy.NONE, bulkRequest.getRefreshPolicy());
128128
}
129+
130+
public void testPrepareRequestWithTypeThrowsUnsupportedOperationException() {
131+
// Create a protobuf BulkRequest with deprecated type field
132+
BulkRequest request = BulkRequest.newBuilder().setType("_doc").build();
133+
134+
// Call prepareRequest, should throw UnsupportedOperationException
135+
UnsupportedOperationException exception = expectThrows(
136+
UnsupportedOperationException.class,
137+
() -> BulkRequestProtoUtils.prepareRequest(request)
138+
);
139+
140+
assertEquals("type param is not supported", exception.getMessage());
141+
}
142+
143+
public void testPrepareRequestWithGlobalParamsThrowsUnsupportedOperationException() {
144+
// Create a protobuf BulkRequest with global_params
145+
BulkRequest request = BulkRequest.newBuilder().setGlobalParams(org.opensearch.protobufs.GlobalParams.newBuilder().build()).build();
146+
147+
// Call prepareRequest, should throw UnsupportedOperationException
148+
UnsupportedOperationException exception = expectThrows(
149+
UnsupportedOperationException.class,
150+
() -> BulkRequestProtoUtils.prepareRequest(request)
151+
);
152+
153+
assertEquals("global_params param is not supported yet", exception.getMessage());
154+
}
129155
}

‎modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,4 +388,40 @@ public void testParseSearchSourceWithInvalidTerminateAfter() throws IOException
388388
);
389389
}
390390

391+
public void testParseSearchRequestWithTypedKeysThrowsUnsupportedOperationException() throws IOException {
392+
// Create a protobuf SearchRequest with typed_keys
393+
org.opensearch.protobufs.SearchRequest protoRequest = org.opensearch.protobufs.SearchRequest.newBuilder()
394+
.setTypedKeys(true)
395+
.build();
396+
397+
// Create a SearchRequest to populate
398+
SearchRequest searchRequest = new SearchRequest();
399+
400+
// Call the method under test, should throw UnsupportedOperationException
401+
UnsupportedOperationException exception = expectThrows(
402+
UnsupportedOperationException.class,
403+
() -> SearchRequestProtoUtils.parseSearchRequest(searchRequest, protoRequest, namedWriteableRegistry, size -> {}, queryUtils)
404+
);
405+
406+
assertEquals("typed_keys param is not supported yet", exception.getMessage());
407+
}
408+
409+
public void testParseSearchRequestWithGlobalParamsThrowsUnsupportedOperationException() throws IOException {
410+
// Create a protobuf SearchRequest with global_params
411+
org.opensearch.protobufs.SearchRequest protoRequest = org.opensearch.protobufs.SearchRequest.newBuilder()
412+
.setGlobalParams(org.opensearch.protobufs.GlobalParams.newBuilder().build())
413+
.build();
414+
415+
// Create a SearchRequest to populate
416+
SearchRequest searchRequest = new SearchRequest();
417+
418+
// Call the method under test, should throw UnsupportedOperationException
419+
UnsupportedOperationException exception = expectThrows(
420+
UnsupportedOperationException.class,
421+
() -> SearchRequestProtoUtils.parseSearchRequest(searchRequest, protoRequest, namedWriteableRegistry, size -> {}, queryUtils)
422+
);
423+
424+
assertEquals("global_params param is not supported yet", exception.getMessage());
425+
}
426+
391427
}

‎modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtilsTests.java‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,24 @@ public void testParseProtoWithExtThrowsUnsupportedOperationException() throws IO
611611
assertTrue("Exception message should mention ext param", exception.getMessage().contains("ext param is not supported yet"));
612612
}
613613

614+
public void testParseProtoWithAggregationsThrowsUnsupportedOperationException() throws IOException {
615+
// Create a protobuf SearchRequestBody with aggregations
616+
SearchRequestBody protoRequest = SearchRequestBody.newBuilder()
617+
.putAggregations("test_agg", org.opensearch.protobufs.AggregationContainer.newBuilder().build())
618+
.build();
619+
620+
// Create a SearchSourceBuilder to populate
621+
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
622+
623+
// Call the method under test, should throw UnsupportedOperationException
624+
UnsupportedOperationException exception = expectThrows(
625+
UnsupportedOperationException.class,
626+
() -> SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils)
627+
);
628+
629+
assertEquals("aggregations param is not supported yet", exception.getMessage());
630+
}
631+
614632
public void testScriptFieldProtoUtilsFromProto() throws IOException {
615633
// Create a protobuf ScriptField
616634
ScriptField scriptFieldProto = ScriptField.newBuilder()
@@ -732,9 +750,12 @@ public void testParseProtoWithSuggest() throws IOException {
732750

733751
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
734752

735-
SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils);
736-
737-
assertNotNull("SuggestBuilder should not be null", searchSourceBuilder.suggest());
753+
// suggest is not yet implemented, should throw UnsupportedOperationException
754+
UnsupportedOperationException exception = expectThrows(
755+
UnsupportedOperationException.class,
756+
() -> SearchSourceBuilderProtoUtils.parseProto(searchSourceBuilder, protoRequest, queryUtils)
757+
);
758+
assertEquals("suggest param is not supported yet", exception.getMessage());
738759
}
739760

740761
public void testParseProtoWithXSourceIncludes() throws IOException {

0 commit comments

Comments
 (0)