Support HTTP QUERY method (RFC 10008) on _search (#22409)

[marceloboeira]

Description

Add QUERY as a first-class HTTP method on _search, _msearch routes, and count operations. The method is safe, idempotent, and cacheable per RFC 10008, filling the gap between GET (URI-only) and POST (mutating).

Key changes:

• RestRequest.Method enum now includes QUERY
• RestSearchAction routes register QUERY on _search and /{index}/_search
• RestMsearchAction registers QUERY similarly
• rest-api-spec schema documents QUERY as accepted method
• Unit tests added to RestSearchActionTests and RestControllerTests

Backward compatible: GET and POST _search behavior unchanged. QUERY is purely opt-in for new clients that support RFC 10008.

Related Issues

Integration tests deferred: elasticsearch-java and opensearch-java client libraries must add QUERY to their HttpMethod enums first. Coordination tracked in #22409 and elastic/elasticsearch#153255.

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

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.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 708b29c)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: Add QUERY method support to low-level REST client Relevant files: client/rest/src/main/java/org/opensearch/client/RestClient.java client/rest/src/test/java/org/opensearch/client/RestClientSingleHostTests.java client/test/src/main/java/org/opensearch/client/RestClientTestUtil.java Sub-PR theme: Map QUERY HTTP method in Netty transport layers Relevant files: modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4HttpRequest.java modules/transport-netty4/src/test/java/org/opensearch/http/netty4/Netty4HttpRequestTests.java plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/HttpConversionUtil.java plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/HttpConversionUtilTests.java Sub-PR theme: Register QUERY routes on _search and update REST API spec Relevant files: server/src/main/java/org/opensearch/rest/RestRequest.java server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java server/src/test/java/org/opensearch/rest/action/search/RestSearchActionTests.java rest-api-spec/src/main/resources/rest-api-spec/api/search.json rest-api-spec/src/main/resources/schema.json

⚡ Recommended focus areas for review Enum ordering breaks BWC The Method enum was reordered: CONNECT was moved from position 8 (after TRACE) to a new position, and QUERY was appended. Any serialization/persistence that relies on Method.ordinal() or transmits enum ordinals over the wire (e.g., stream serialization between nodes) will misinterpret values in mixed-version clusters. Even if no current code path uses ordinals, changing the declared order of an existing enum is fragile. Consider appending QUERY at the end without touching CONNECT's position — but here CONNECT was already the last entry, so simply appending QUERY after CONNECT (rather than reordering) would preserve ordinals. public enum Method { GET, POST, PUT, DELETE, OPTIONS, HEAD, PATCH, TRACE, CONNECT, QUERY } Possible Issue The diff shows new Route(POST, "/{index}/_search") being removed from the old hunk and re-added in the new hunk, but the trailing comma after new Route(GET, "/{index}/_search") in the new hunk suggests the list literal may have a syntax issue if the POST route is not actually present. Verify the final asList(...) call compiles and that both GET and POST /{index}/_search routes remain registered, since the old hunk indicates POST was previously removed. public List<Route> routes() { return unmodifiableList( asList( new Route(GET, "/_search"), new Route(POST, "/_search"), new Route(QUERY, "/_search"), new Route(GET, "/{index}/_search"), new Route(POST, "/{index}/_search"), new Route(QUERY, "/{index}/_search") ) ); } Version gate may be incorrect supportedMethodsForVersion uses Version.V_3_8_0 as the cutoff for QUERY support. If this PR is targeted at a different release (e.g., merged into a 3.x branch that ends up shipping QUERY in a different version), the gate will be wrong and BWC randomization will either allow QUERY against nodes that reject it, or forbid it against nodes that accept it. Confirm V_3_8_0 is the actual first version containing this change. static List<String> supportedMethodsForVersion(String[] methods, Version esVersion) { List<String> supportedMethods = Arrays.asList(methods); if (esVersion.before(Version.V_3_8_0)) { supportedMethods = supportedMethods.stream().filter(method -> "QUERY".equals(method) == false).collect(Collectors.toList()); } return supportedMethods; }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 708b29c

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Cache QUERY HttpMethod instance HttpMethod.valueOf("QUERY") allocates a new HttpMethod instance on every request since QUERY is not cached by Netty. Hoist this to a private static final HttpMethod QUERY = HttpMethod.valueOf("QUERY"); constant to avoid per-request allocations in a hot path. The same applies to HttpConversionUtil.convertMethod. modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4HttpRequest.java [155-158] // QUERY (RFC 10008) is not a cached Netty constant, so compare by name rather than identity. -if (HttpMethod.valueOf("QUERY").equals(httpMethod)) { +if (QUERY_METHOD.equals(httpMethod)) { return RestRequest.Method.QUERY; } Suggestion importance[1-10]: 6 __ Why: Valid performance suggestion: caching the HttpMethod.valueOf("QUERY") instance avoids allocation on every request in a hot path, which is a reasonable optimization for HTTP request handling.	Low
	Return a mutable list consistently Arrays.asList returns a fixed-size list backed by the array; callers downstream may attempt to mutate supportedMethods (e.g., remove/add), which would throw UnsupportedOperationException. Wrap it in a mutable ArrayList to preserve the previous behavior of Arrays.asList(path.getMethods()) callers expected, or at minimum return a consistently-typed list in both branches. test/framework/src/main/java/org/opensearch/test/rest/yaml/ClientYamlTestClient.java [118-124] static List<String> supportedMethodsForVersion(String[] methods, Version esVersion) { - List<String> supportedMethods = Arrays.asList(methods); + List<String> supportedMethods = new ArrayList<>(Arrays.asList(methods)); if (esVersion.before(Version.V_3_8_0)) { supportedMethods = supportedMethods.stream().filter(method -> "QUERY".equals(method) == false).collect(Collectors.toList()); } return supportedMethods; } Suggestion importance[1-10]: 4 __ Why: The concern about mutability is minor since the downstream code in callApi doesn't appear to mutate the returned list, but returning a consistently-typed mutable list is a reasonable defensive improvement.	Low

---

Previous suggestions

Suggestions up to commit 7432891

Category	Suggestion	Impact
General	Avoid per-request HttpMethod allocation HttpMethod.valueOf("QUERY") allocates a new HttpMethod object on every incoming request just to perform a name comparison, which is wasteful on a hot path. Cache the constant in a static final field (or compare httpMethod.name() to the "QUERY" string) to avoid per-request allocation. modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4HttpRequest.java [155-158] // QUERY (RFC 10008) is not a cached Netty constant, so compare by name rather than identity. -if (HttpMethod.valueOf("QUERY").equals(httpMethod)) { +if ("QUERY".equals(httpMethod.name())) { return RestRequest.Method.QUERY; } Suggestion importance[1-10]: 6 __ Why: Valid performance improvement on a hot request-handling path; HttpMethod.valueOf allocates a new instance each call, and comparing httpMethod.name() to the string literal avoids that allocation.	Low
	Return a mutable list consistently Arrays.asList returns a fixed-size list backed by the input array. If the caller later mutates the returned list (e.g., RandomizedTest-style shuffling or removal in test scaffolding), it will throw UnsupportedOperationException when the QUERY branch is not taken. Return a mutable copy for consistency with the filtered branch. test/framework/src/main/java/org/opensearch/test/rest/yaml/ClientYamlTestClient.java [118-124] static List<String> supportedMethodsForVersion(String[] methods, Version esVersion) { - List<String> supportedMethods = Arrays.asList(methods); + List<String> supportedMethods = new ArrayList<>(Arrays.asList(methods)); if (esVersion.before(Version.V_3_8_0)) { supportedMethods = supportedMethods.stream().filter(method -> "QUERY".equals(method) == false).collect(Collectors.toList()); } return supportedMethods; } Suggestion importance[1-10]: 3 __ Why: The concern is speculative; the existing callers do not appear to mutate the returned list, and the original code before the PR also used Arrays.asList. Minor consistency improvement.	Low

Suggestions up to commit 5a43f8a

Category	Suggestion	Impact
General	Return a mutable list to avoid UnsupportedOperationException Arrays.asList returns a fixed-size list backed by the input array. If a caller mutates the returned list (e.g. RandomizingClient shuffling), the current-version branch will fail with UnsupportedOperationException. Wrap the result in a new mutable ArrayList to make the returned list safely mutable and consistent across branches. test/framework/src/main/java/org/opensearch/test/rest/yaml/ClientYamlTestClient.java [118-124] static List<String> supportedMethodsForVersion(String[] methods, Version esVersion) { - List<String> supportedMethods = Arrays.asList(methods); + List<String> supportedMethods = new ArrayList<>(Arrays.asList(methods)); if (esVersion.before(Version.V_3_8_0)) { supportedMethods = supportedMethods.stream().filter(method -> "QUERY".equals(method) == false).collect(Collectors.toList()); } return supportedMethods; } Suggestion importance[1-10]: 4 __ Why: The concern is theoretical - callers may or may not mutate the returned list, and the stream collect(Collectors.toList()) branch already returns a mutable list, so the inconsistency is real but low impact.	Low
	Ensure case-insensitive method matching The switch above uses constants like HttpPost.METHOD_NAME for case labels; using the string literal "QUERY" here is inconsistent and case-sensitive, so a caller passing "query" would fall through to the default. Since the surrounding switch appears to match on the raw method string, ensure the method is normalized (e.g. upper-cased) before the switch, or accept common variants, to avoid surprising UnsupportedOperationException for lower-case input. client/rest/src/main/java/org/opensearch/client/RestClient.java [807-808] +case "QUERY": + return addRequestBody(new HttpUriRequestBase("QUERY", uri), entity); - Suggestion importance[1-10]: 2 __ Why: The existing_code and improved_code are identical, and other cases in the switch also use uppercase method names (HTTP methods are conventionally uppercase), so this suggestion has minimal value.	Low

Suggestions up to commit 60a4546

Category	Suggestion	Impact
General	Return a mutable list from helper method Arrays.asList returns a fixed-size list backed by the array, and if callers later attempt to mutate supportedMethods (e.g., remove) it will throw UnsupportedOperationException. Wrap in a new ArrayList to ensure the returned list is mutable and consistent regardless of the version branch taken. test/framework/src/main/java/org/opensearch/test/rest/yaml/ClientYamlTestClient.java [118-124] static List<String> supportedMethodsForVersion(String[] methods, Version esVersion) { - List<String> supportedMethods = Arrays.asList(methods); + List<String> supportedMethods = new ArrayList<>(Arrays.asList(methods)); if (esVersion.before(Version.V_3_8_0)) { supportedMethods = supportedMethods.stream().filter(method -> "QUERY".equals(method) == false).collect(Collectors.toList()); } return supportedMethods; } Suggestion importance[1-10]: 2 __ Why: The current implementation works for callers that only read the list; there's no evidence in the PR that callers mutate it. The suggestion is a minor defensive improvement with low impact.	Low
	Avoid duplicating the QUERY string literal The case label uses a hardcoded literal while the other cases use HttpXxx.METHOD_NAME constants; also the constructor argument duplicates the same literal. Extract a constant (or reuse the case label) to avoid drift between the two strings and keep style consistent with surrounding cases. client/rest/src/main/java/org/opensearch/client/RestClient.java [807-808] case "QUERY": - return addRequestBody(new HttpUriRequestBase("QUERY", uri), entity); + final String queryMethod = "QUERY"; + return addRequestBody(new HttpUriRequestBase(queryMethod, uri), entity); Suggestion importance[1-10]: 2 __ Why: Minor stylistic improvement. Introducing a local final String inside a case still duplicates the literal in the case label, so it only marginally reduces drift risk.	Low

Suggestions up to commit 9a5211b

Category	Suggestion	Impact
General	Return a mutable list to callers Arrays.asList returns a fixed-size list backed by the input array. If the caller later tries to mutate the returned list (e.g., in the non-BWC path), it will throw UnsupportedOperationException. Return a new mutable ArrayList to be safe and consistent with the filtered branch. test/framework/src/main/java/org/opensearch/test/rest/yaml/ClientYamlTestClient.java [118-124] static List<String> supportedMethodsForVersion(String[] methods, Version esVersion) { - List<String> supportedMethods = Arrays.asList(methods); + List<String> supportedMethods = new ArrayList<>(Arrays.asList(methods)); if (esVersion.before(Version.V_3_8_0)) { supportedMethods = supportedMethods.stream().filter(method -> "QUERY".equals(method) == false).collect(Collectors.toList()); } return supportedMethods; } Suggestion importance[1-10]: 3 __ Why: The returned list is not mutated by the current caller, and Collectors.toList() in the filtered branch also does not guarantee mutability. The concern is speculative and low-impact.	Low
	Extract QUERY method name into a constant The other cases in this switch use constants (e.g., HttpPost.METHOD_NAME) for method names. Extracting the "QUERY" literal into a private static final String constant would prevent typos and keep the string in sync between the case label and the constructor argument. client/rest/src/main/java/org/opensearch/client/RestClient.java [807-808] -case "QUERY": - return addRequestBody(new HttpUriRequestBase("QUERY", uri), entity); +case QUERY_METHOD_NAME: + return addRequestBody(new HttpUriRequestBase(QUERY_METHOD_NAME, uri), entity); Suggestion importance[1-10]: 3 __ Why: Minor code style/maintainability improvement to avoid duplicated string literals, but has no functional impact.	Low

Suggestions up to commit 1fd1d8b

Category	Suggestion	Impact
General	Avoid per-request allocation on hot path HttpMethod.valueOf("QUERY") allocates a new HttpMethod instance on every request that reaches this branch, which is on the hot path for HTTP request handling. Cache the instance in a static final field (or compare via httpMethod.name().equals("QUERY")) to avoid the repeated allocation and equality overhead. modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4HttpRequest.java [155-158] // QUERY (RFC 10008) is not a cached Netty constant, so compare by name rather than identity. -if (HttpMethod.valueOf("QUERY").equals(httpMethod)) { +if ("QUERY".equals(httpMethod.name())) { return RestRequest.Method.QUERY; } Suggestion importance[1-10]: 6 __ Why: Valid performance improvement — HttpMethod.valueOf("QUERY") allocates a new object per request on the hot path. Comparing via httpMethod.name() avoids allocation, though the impact is moderate.	Low

[github-actions]

❌ Gradle check result for a3e34db: 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?

[github-actions]

Persistent review updated to latest commit e32ad6b

[github-actions]

❌ Gradle check result for e32ad6b: 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?

[github-actions]

Persistent review updated to latest commit fa8abe4

[github-actions]

❌ Gradle check result for fa8abe4: 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?

[github-actions]

Persistent review updated to latest commit dfb9cd6

[github-actions]

❌ Gradle check result for dfb9cd6: 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?

[msfroh]

@marceloboeira -- It looks like we need to add QUERY as a valid method to some enum:

> Task :rest-api-spec:validateRestSpec FAILED
[validate JSON][ERROR][search.json][$.search.url.paths[0].methods[2]: does not have a value in the enumeration [DELETE, GET, HEAD, POST, PUT]]
[validate JSON][ERROR][search.json][$.search.url.paths[1].methods[2]: does not have a value in the enumeration [DELETE, GET, HEAD, POST, PUT]]

Specifically, I think you need to add QUERY to list of valid methods here:

OpenSearch/rest-api-spec/src/main/resources/schema.json

Line 256 in 920677a

"enum": ["DELETE", "GET", "HEAD", "POST", "PUT"],

[github-actions]

Persistent review updated to latest commit 2492d68

[github-actions]

Persistent review updated to latest commit ae71a80

[github-actions]

❌ Gradle check result for ae71a80: 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?

[github-actions]

Persistent review updated to latest commit 95c9051

[github-actions]

❌ Gradle check result for 95c9051: 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?

[github-actions]

Persistent review updated to latest commit fe2927e

[github-actions]

❌ Gradle check result for fe2927e: null

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?

[github-actions]

Persistent review updated to latest commit 1fd1d8b

[github-actions]

❌ Gradle check result for 1fd1d8b: 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?

[github-actions]

Persistent review updated to latest commit 9a5211b

[github-actions]

Persistent review updated to latest commit 60a4546

[github-actions]

❌ Gradle check result for 60a4546: 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?

[marceloboeira]

The failure is a flaky test unrelated to this change.

IngestFromKafkaIT > testKafkaIngestion_RewindByTimeStamp FAILED
    org.awaitility.core.ConditionTimeoutException: Condition ... was not fulfilled within 1 minutes.
        at org.opensearch.plugin.kafka.KafkaUtils.createTopic(KafkaUtils.java:56)
        at org.opensearch.plugin.kafka.KafkaIngestionBaseIT.setupKafka(KafkaIngestionBaseIT.java:80)
        at org.opensearch.plugin.kafka.KafkaIngestionBaseIT.setup(KafkaIngestionBaseIT.java:64)

it fails in @Before setup, timing out while creating a Kafka topic against the test fixture (broker readiness), in plugins:ingestion-kafka unrelated to my change, which only touches the REST/search layer (RestRequest, RestSearchAction, the netty transports, client/rest, and the yaml test framework).

This test is already tracked as flaky in #17215 ([AUTOCUT] Gradle Check Flaky Test Report for IngestFromKafkaIT).

[github-actions]

Persistent review updated to latest commit 5a43f8a

[github-actions]

❌ Gradle check result for 5a43f8a: 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?

[github-actions]

Persistent review updated to latest commit 7432891

[github-actions]

✅ Gradle check result for 7432891: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.48%. Comparing base (993f75a) to head (7432891).

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #22410      +/-   ##
============================================
- Coverage     73.50%   73.48%   -0.03%     
+ Complexity    76507    76488      -19     
============================================
  Files          6104     6104              
  Lines        346618   346624       +6     
  Branches      49888    49890       +2     
============================================
- Hits         254793   254727      -66     
- Misses        71541    71660     +119     
+ Partials      20284    20237      -47     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Persistent review updated to latest commit 708b29c

[github-actions]

❌ Gradle check result for 708b29c: null

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?