[GRPC] Upgrade protobufs to 1.1.0#20396
Conversation
📝 WalkthroughWalkthroughBumps opensearch-protobufs from 1.0.0 to 1.1.0 and updates transport-grpc code, tests, and checksum files to match protobuf API renames for coords, value fields, and matched-queries handling. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: xil <fridalu66@gmail.com>
Signed-off-by: xil <fridalu66@gmail.com>
|
❌ Gradle check result for 4c8005e: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java (1)
103-103: Update error message to use "value" instead of "field_value_array".This error message still references the old API terminology
field_value_array. It should be updated to match the new terminology for consistency with the changes on lines 99 and 147.📝 Proposed fix
- throw new IllegalArgumentException("Either field_value_array or lookup must be set"); + throw new IllegalArgumentException("Either value or lookup must be set");
🤖 Fix all issues with AI agents
In @CHANGELOG.md:
- Line 60: Update the changelog entry that currently reads "Bump
`opensearch-protobufs` from to 1.1.0 and update transport-grpc module
compatibility ([#20396])" to include the missing source version so it reads
"Bump `opensearch-protobufs` from 1.0.0 to 1.1.0 and update transport-grpc
module compatibility ([#20396])", ensuring the version range is complete and
accurate.
In @gradle/libs.versions.toml:
- Line 27: The dependency version for the opensearch protobufs catalog entry
(opensearchprotobufs = "1.1.0") is invalid on Maven Central and will break
Gradle resolution; update the value of the opensearchprotobufs entry in
gradle/libs.versions.toml to "0.24.0" (or the correct published version) or
add/verify the repository that hosts 1.1.0, ensuring the catalog key
opensearchprotobufs matches the published artifact version.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
CHANGELOG.mdgradle/libs.versions.tomlmodules/transport-grpc/licenses/protobufs-1.0.0.jar.sha1modules/transport-grpc/licenses/protobufs-1.1.0.jar.sha1modules/transport-grpc/spi/licenses/protobufs-1.0.0.jar.sha1modules/transport-grpc/spi/licenses/protobufs-1.1.0.jar.sha1modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/common/GeoPointProtoUtils.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/common/GeoPointProtoUtilsTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/GeoDistanceQueryBuilderProtoUtilsTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverterTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtilsTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtilsTests.java
💤 Files with no reviewable changes (2)
- modules/transport-grpc/spi/licenses/protobufs-1.0.0.jar.sha1
- modules/transport-grpc/licenses/protobufs-1.0.0.jar.sha1
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: The gRPC search API in OpenSearch is marked as "experimental" in official documentation, so changes to proto schemas that remove previously unsupported fields (those throwing UnsupportedOperationException) are not considered breaking changes.
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: In the transport-grpc module, suggest and aggregations protos were removed from SearchRequestBody in protobufs 1.0.0 because they haven't been vetted for accuracy in the API specification. The URL parameter suggest support (suggest_field, suggest_mode, suggest_size, suggest_text) is a minimized subset and not intended as a replacement for full Suggester functionality.
📚 Learning: 2026-01-02T19:23:16.689Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:16.689Z
Learning: In OpenSearch's gRPC transport module, treat removals of previously-unsupported fields (those throwing UnsupportedOperationException) from experimental proto schemas as non-breaking changes. When reviewing changes to proto-related Java code in this module, document that such removals do not count as breaking API changes, and ensure tests reflect compatibility expectations accordingly.
Applied to files:
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtilsTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/common/GeoPointProtoUtilsTests.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/common/GeoPointProtoUtils.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverterTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/GeoDistanceQueryBuilderProtoUtilsTests.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtilsTests.java
📚 Learning: 2026-01-02T19:23:29.698Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: In the transport-grpc module, suggest and aggregations protos were removed from SearchRequestBody in protobufs 1.0.0 because they haven't been vetted for accuracy in the API specification. The URL parameter suggest support (suggest_field, suggest_mode, suggest_size, suggest_text) is a minimized subset and not intended as a replacement for full Suggester functionality.
Applied to files:
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtilsTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryTests.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/common/GeoPointProtoUtils.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverterTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/GeoDistanceQueryBuilderProtoUtilsTests.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtilsTests.java
📚 Learning: 2026-01-02T19:23:29.698Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: The gRPC search API in OpenSearch is marked as "experimental" in official documentation, so changes to proto schemas that remove previously unsupported fields (those throwing UnsupportedOperationException) are not considered breaking changes.
Applied to files:
gradle/libs.versions.tomlCHANGELOG.md
🔇 Additional comments (14)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/common/GeoPointProtoUtils.java (1)
69-70: LGTM! Protobuf API accessor updated correctly.The migration from
hasDoubleArray()/getDoubleArray()tohasCoords()/getCoords()aligns with the protobuf 1.1.0 API changes. The validation and coordinate parsing logic remains functionally identical.modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.java (1)
263-281: LGTM! Matched queries now use structured HitMatchedQueries with setMatchedQueries2().The implementation correctly builds either a DoubleMap (when scores are included) or a StringArray (names only) based on the
includeMatchedQueriesScoreflag. This aligns with the protobuf 1.1.0 schema changes and provides better type safety.Note: Line 261 has a TODO comment indicating that
includeMatchedQueriesScoreis hardcoded tofalse. This means matched query scores are currently never included in responses.modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java (2)
91-92: LGTM! Enum case and accessor updated from FIELD_VALUE_ARRAY to VALUE.The migration correctly replaces
FIELD_VALUE_ARRAYwithVALUEand updates the corresponding accessor fromgetFieldValueArray()togetValue(). This aligns with the protobuf 1.1.0 API changes.Also applies to: 139-140
99-99: LGTM! Error messages updated to reflect new API terminology.The error messages correctly changed from "Neither field_value_array nor lookup is set" to "Neither value nor lookup is set" to match the updated protobuf field names.
Also applies to: 147-147
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/GeoDistanceQueryBuilderProtoUtilsTests.java (1)
144-144: LGTM! Test updated to use setCoords() builder method.The test correctly uses
setCoords()instead ofsetDoubleArray(), aligning with the protobuf 1.1.0 API changes in GeoPointProtoUtils.java.modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtilsTests.java (2)
27-27: LGTM! All test usages updated to setValue() builder method.All occurrences of
setFieldValueArray()have been consistently updated tosetValue(), aligning with the protobuf 1.1.0 API changes in TermsQueryBuilderProtoUtils.java. The test coverage remains comprehensive across all scenarios (default behavior, lookup, bitmap types, error cases, etc.).Also applies to: 85-85, 108-108, 186-186, 211-211, 256-256, 300-301, 344-344, 366-366, 388-388, 406-406, 429-429
1-441: Verify that the protobuf 1.1.0 migration is complete across all production code.The test file updates appear correct. Ensure no old protobuf 1.0.0 API calls remain in the implementation files (e.g.,
getDoubleArray(),setDoubleArray(),getFieldValueArray(),setFieldValueArray()) and confirm theopensearch-protobufsdependency in build files has been updated to version 1.1.0 or later.modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoConverterTests.java (1)
43-43: LGTM! Protobuf API updates correctly applied.The test code correctly updates from
setFieldValueArray()tosetValue()to match the protobufs 1.1.0 API. The test logic remains unchanged and properly validates the TermsQuery conversion behavior.Also applies to: 92-92, 119-119, 177-177
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryTests.java (2)
460-460: LGTM! GeoLocation API correctly updated.The test properly updates from
setDoubleArray()tosetCoords()to align with the protobufs 1.1.0 API rename. The method name better reflects the coordinate semantics.
607-607: LGTM! TermsQueryField API correctly updated.The test correctly updates from
setFieldValueArray()tosetValue()to match the protobufs 1.1.0 API. The change aligns with similar updates across other test files.modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/common/GeoPointProtoUtilsTests.java (1)
56-56: LGTM! Coordinate API updates consistently applied.All test methods correctly update from
setDoubleArray()tosetCoords()to align with the protobufs 1.1.0 API. The comprehensive test coverage for various coordinate scenarios (2D, 3D, dimension validation, z-value handling) remains intact.Also applies to: 73-73, 93-93, 166-166, 188-188, 209-209
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtilsTests.java (1)
524-544: LGTM! New matched_queries_2 tests are well-structured.The two new test methods properly validate the matched_queries_2 field behavior introduced in protobufs 1.1.0:
testToProtoWithMatchedQueriesScoreFalsecorrectly verifies the StringArray variant (names without scores)testToProtoWithNoMatchedQueriesappropriately tests the absence caseBoth tests follow the existing patterns in the file and provide good coverage for the new field.
Note: The existing
testToProtoWithMatchedQueries(line 193) still tests the originalmatched_queriesfield. If both fields are being maintained for backwards compatibility during a transition period, this is appropriate. Otherwise, you may want to verify whether the old field/test should be deprecated or updated.modules/transport-grpc/licenses/protobufs-1.1.0.jar.sha1 (1)
1-1: SHA1 hash is consistent with the SPI licenses file.The hash matches the one in
modules/transport-grpc/spi/licenses/protobufs-1.1.0.jar.sha1, which is expected since they reference the same artifact. See the verification script in the SPI file review.modules/transport-grpc/spi/licenses/protobufs-1.1.0.jar.sha1 (1)
1-1: SHA1 hash is correct and verified. The hash4d49daae7d84f850a876540ee7b6a979e769d4c7matches the publishedorg.opensearch:protobufs:1.1.0artifact on Maven Central.
Signed-off-by: xil <fridalu66@gmail.com>
|
❌ Gradle check result for 68b4ee7: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
...c/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.java
Show resolved
Hide resolved
Signed-off-by: xil <fridalu66@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.java (1)
259-261: Method signature needs updating to support dynamic parameter passing for matched queries scores.The
includeMatchedQueriesScoreis hardcoded tofalse, leaving the scores path (lines 265-271) unreachable. To enable this feature, theprocessMatchedQueriesmethod signature must be updated to receive search parameters, similar to howSearchHit.toXContent()in the REST implementation does it (seeSearchHit.java:734). Additionally, a similar TODO exists inSearchRequestProtoUtils.java:171. This architectural change should be tracked as a follow-up task to complete the protobuf 1.1.0 upgrade, ensuring gRPC responses can optionally include matched query scores via theinclude_named_queries_scoreparameter.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: The gRPC search API in OpenSearch is marked as "experimental" in official documentation, so changes to proto schemas that remove previously unsupported fields (those throwing UnsupportedOperationException) are not considered breaking changes.
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: In the transport-grpc module, suggest and aggregations protos were removed from SearchRequestBody in protobufs 1.0.0 because they haven't been vetted for accuracy in the API specification. The URL parameter suggest support (suggest_field, suggest_mode, suggest_size, suggest_text) is a minimized subset and not intended as a replacement for full Suggester functionality.
📚 Learning: 2026-01-02T19:23:29.698Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: In the transport-grpc module, suggest and aggregations protos were removed from SearchRequestBody in protobufs 1.0.0 because they haven't been vetted for accuracy in the API specification. The URL parameter suggest support (suggest_field, suggest_mode, suggest_size, suggest_text) is a minimized subset and not intended as a replacement for full Suggester functionality.
Applied to files:
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.java
📚 Learning: 2026-01-02T19:23:16.689Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:16.689Z
Learning: In OpenSearch's gRPC transport module, treat removals of previously-unsupported fields (those throwing UnsupportedOperationException) from experimental proto schemas as non-breaking changes. When reviewing changes to proto-related Java code in this module, document that such removals do not count as breaking API changes, and ensure tests reflect compatibility expectations accordingly.
Applied to files:
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: gradle-check
- GitHub Check: precommit (25, macos-15-intel)
- GitHub Check: precommit (25, ubuntu-latest)
- GitHub Check: precommit (21, macos-15)
- GitHub Check: precommit (25, windows-latest)
- GitHub Check: precommit (21, ubuntu-24.04-arm)
- GitHub Check: precommit (21, windows-2025, true)
- GitHub Check: precommit (25, ubuntu-24.04-arm)
- GitHub Check: precommit (25, macos-15)
- GitHub Check: precommit (21, macos-15-intel)
- GitHub Check: precommit (21, windows-latest)
- GitHub Check: precommit (21, ubuntu-latest)
- GitHub Check: Analyze (java)
- GitHub Check: assemble (21, ubuntu-24.04-arm)
- GitHub Check: assemble (25, windows-latest)
- GitHub Check: assemble (21, ubuntu-latest)
- GitHub Check: assemble (21, windows-latest)
- GitHub Check: assemble (25, ubuntu-24.04-arm)
- GitHub Check: assemble (25, ubuntu-latest)
- GitHub Check: detect-breaking-change
🔇 Additional comments (1)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.java (1)
263-286: Backward compatibility strategy is well-implemented.The approach correctly maintains compatibility:
- Old clients receive both
matched_queries(deprecated) andmatched_queries_2.names- New clients requesting scores receive only
matched_queries_2.scores- The deprecated field is populated only when scores are not included, which ensures old clients continue working without modification
This aligns well with the protobuf upgrade objectives and the experimental status of the gRPC API.
Based on learnings, the gRPC API is experimental, so this controlled evolution of the API is appropriate.
|
❌ Gradle check result for 992a301: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #20396 +/- ##
============================================
- Coverage 73.23% 73.16% -0.08%
+ Complexity 71735 71651 -84
============================================
Files 5784 5784
Lines 328153 328185 +32
Branches 47270 47273 +3
============================================
- Hits 240333 240122 -211
- Misses 68498 68840 +342
+ Partials 19322 19223 -99 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: xil <fridalu66@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java (1)
107-109: Inconsistent error message terminology.Line 108 still uses the old
field_value_arrayterm while line 104 was updated to usevalue. Update for consistency.Proposed fix
if (values == null && termsLookup == null) { - throw new IllegalArgumentException("Either field_value_array or lookup must be set"); + throw new IllegalArgumentException("Either value or lookup must be set"); }
🧹 Nitpick comments (2)
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtilsTests.java (2)
524-535: Consider verifying actual matched query values and backward compatibility.The test correctly validates the structure of
matched_queries_2, but could be more thorough:
- Verify the actual string values in the array (not just the count) to catch potential ordering or content issues.
- Verify that the deprecated
matched_queriesfield is also populated for backward compatibility, as the implementation populates both fields.♻️ Suggested enhancements
public void testToProtoWithMatchedQueriesWithoutScores() throws IOException { SearchHit searchHit = new SearchHit(1); searchHit.matchedQueries(new String[] { "filter1", "filter2" }); HitsMetadataHitsInner hit = SearchHitProtoUtils.toProto(searchHit); assertNotNull("Hit should not be null", hit); assertTrue("matched_queries_2 should be set", hit.hasMatchedQueries2()); assertTrue("Should use names (StringArray)", hit.getMatchedQueries2().hasNames()); assertFalse("Should not use scores (DoubleMap)", hit.getMatchedQueries2().hasScores()); assertEquals("Should have 2 matched query names", 2, hit.getMatchedQueries2().getNames().getStringArrayCount()); + assertEquals("First matched query name should match", "filter1", hit.getMatchedQueries2().getNames().getStringArray(0)); + assertEquals("Second matched query name should match", "filter2", hit.getMatchedQueries2().getNames().getStringArray(1)); + + // Verify backward compatibility with deprecated field + assertEquals("Deprecated matched_queries should also have 2 entries", 2, hit.getMatchedQueriesCount()); + assertEquals("Deprecated first matched query should match", "filter1", hit.getMatchedQueries(0)); + assertEquals("Deprecated second matched query should match", "filter2", hit.getMatchedQueries(1)); }
537-544: LGTM! Consider optionally verifying deprecated field.The negative test correctly validates that
matched_queries_2is not set when there are no matched queries. For completeness, you could also verify that the deprecatedmatched_queriesfield is empty (count is 0).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtilsTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtilsTests.java
🚧 Files skipped from review as they are similar to previous changes (1)
- modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: The gRPC search API in OpenSearch is marked as "experimental" in official documentation, so changes to proto schemas that remove previously unsupported fields (those throwing UnsupportedOperationException) are not considered breaking changes.
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: In the transport-grpc module, suggest and aggregations protos were removed from SearchRequestBody in protobufs 1.0.0 because they haven't been vetted for accuracy in the API specification. The URL parameter suggest support (suggest_field, suggest_mode, suggest_size, suggest_text) is a minimized subset and not intended as a replacement for full Suggester functionality.
📚 Learning: 2026-01-02T19:23:16.689Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:16.689Z
Learning: In OpenSearch's gRPC transport module, treat removals of previously-unsupported fields (those throwing UnsupportedOperationException) from experimental proto schemas as non-breaking changes. When reviewing changes to proto-related Java code in this module, document that such removals do not count as breaking API changes, and ensure tests reflect compatibility expectations accordingly.
Applied to files:
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtilsTests.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtilsTests.java
📚 Learning: 2026-01-02T19:23:29.698Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: In the transport-grpc module, suggest and aggregations protos were removed from SearchRequestBody in protobufs 1.0.0 because they haven't been vetted for accuracy in the API specification. The URL parameter suggest support (suggest_field, suggest_mode, suggest_size, suggest_text) is a minimized subset and not intended as a replacement for full Suggester functionality.
Applied to files:
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtilsTests.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtilsTests.java
🧬 Code graph analysis (1)
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtilsTests.java (1)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchHitProtoUtils.java (1)
SearchHitProtoUtils(36-405)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: gradle-check
- GitHub Check: precommit (21, windows-2025, true)
- GitHub Check: precommit (25, windows-latest)
- GitHub Check: precommit (21, macos-15-intel)
- GitHub Check: precommit (25, macos-15-intel)
- GitHub Check: precommit (25, ubuntu-24.04-arm)
- GitHub Check: precommit (25, macos-15)
- GitHub Check: precommit (21, windows-latest)
- GitHub Check: precommit (21, macos-15)
- GitHub Check: precommit (21, ubuntu-latest)
- GitHub Check: precommit (21, ubuntu-24.04-arm)
- GitHub Check: Analyze (java)
- GitHub Check: precommit (25, ubuntu-latest)
- GitHub Check: assemble (25, ubuntu-24.04-arm)
- GitHub Check: assemble (21, windows-latest)
- GitHub Check: assemble (25, windows-latest)
- GitHub Check: assemble (21, ubuntu-24.04-arm)
- GitHub Check: assemble (21, ubuntu-latest)
- GitHub Check: assemble (25, ubuntu-latest)
- GitHub Check: detect-breaking-change
🔇 Additional comments (2)
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtilsTests.java (1)
27-27: Test API updates correctly align with protobuf 1.1.0.All test usages of
TermsQueryField.Builderare correctly updated fromsetFieldValueArray(...)tosetValue(...), matching the renamed proto API. The underlying test logic remains sound, and the comprehensive coverage of scenarios (default behavior, lookups, bitmap encoding, edge cases) is well maintained.Also applies to: 85-85, 108-108, 186-186, 211-211, 256-256, 300-301, 344-344, 366-366, 388-388, 406-406, 429-429
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/TermsQueryBuilderProtoUtils.java (1)
96-104: API updates correctly align with protobuf 1.1.0.The switch case and getter are properly updated from
FIELD_VALUE_ARRAY/getFieldValueArray()toVALUE/getValue(), and the error message at line 104 is updated accordingly.
2 participants
Description
Address Compilation for Bumping OpenSearch Protobuf Version to 1.1.0.
opensearch-project/opensearch-protobufs@1.0.0...1.1.0#diff-2b931e620077c456566a9d1378628816a249e38700b6ca55bbea3e8ec080f1d1
Protobuf Changes
Test Plan
1, create mapping
2, Index Sample Documents
3, grpc Request (Default false)
4, gRPC response
5, Manual change hardcoded params to true
6, gRPC request
7, gRPC response
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Summary by CodeRabbit
Chores
Chores (API)
Tests
✏️ Tip: You can customize this high-level summary in your review settings.