From e36ff3167053333598d532f583758ca72c031acc Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Fri, 6 Feb 2026 13:58:52 +0100 Subject: [PATCH] Support selecting an explicit method for apis in YAML specs. (#141698) If a specification defines multiple methods for an api, the client randomly picks one. This can lead to BWC test issues if some method is not available in earlier versions and requires to explicitly select the method to use. --- .../rest/yaml/CcsCommonYamlTestSuiteIT.java | 4 +++- .../rest-api-spec/test/cat.count/10_basic.yml | 10 ++++++-- .../rest/yaml/ClientYamlDocsTestClient.java | 9 +++---- .../test/rest/yaml/ClientYamlTestClient.java | 11 ++++++++- .../yaml/ClientYamlTestExecutionContext.java | 24 +++++++++++++++---- .../ImpersonateOfficialClientTestClient.java | 3 ++- .../rest/yaml/section/ApiCallSection.java | 16 +++++++++++++ .../test/rest/yaml/section/DoSection.java | 5 ++++ .../ClientYamlTestExecutionContextTests.java | 6 +++-- .../rest/yaml/section/DoSectionTests.java | 18 ++++++++++++++ 10 files changed, 91 insertions(+), 15 deletions(-) diff --git a/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java b/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java index 89faa04ff8d70..1e970f86689d4 100644 --- a/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java +++ b/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java @@ -379,8 +379,10 @@ public void setTestCandidate(ClientYamlTestCandidate testCandidate) { // we overwrite this method so the search client can modify the index names by prefixing them with the // remote cluster name before sending the requests + @Override public ClientYamlTestResponse callApi( String apiName, + String method, Map params, HttpEntity entity, Map headers, @@ -406,7 +408,7 @@ public ClientYamlTestResponse callApi( } params.put(parameterName, String.join(",", expandedIndices)); } - return super.callApi(apiName, params, entity, headers, nodeSelector, pathPredicate); + return super.callApi(apiName, method, params, entity, headers, nodeSelector, pathPredicate); } private boolean shouldReplaceIndexWithRemote(String apiName) { diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cat.count/10_basic.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cat.count/10_basic.yml index 9cbec3e33e589..58d02bcf53fab 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cat.count/10_basic.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cat.count/10_basic.yml @@ -2,6 +2,7 @@ "Test cat count help": - do: cat.count: + method: GET help: true - match: @@ -14,7 +15,8 @@ "Test cat count output": - do: - cat.count: {} + cat.count: + method: GET - match: $body: | @@ -29,7 +31,8 @@ refresh: true - do: - cat.count: {} + cat.count: + method: GET - match: $body: | @@ -45,6 +48,7 @@ - do: cat.count: + method: GET h: count - match: @@ -55,6 +59,7 @@ - do: cat.count: + method: GET index: index1 - match: @@ -64,6 +69,7 @@ - do: cat.count: + method: GET index: index2 v: true diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java index 36274b82a32e0..d0a6e5407b60b 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java @@ -47,6 +47,7 @@ public ClientYamlDocsTestClient( @Override public ClientYamlTestResponse callApi( String apiName, + String method, Map params, HttpEntity entity, Map headers, @@ -57,9 +58,9 @@ public ClientYamlTestResponse callApi( if ("raw".equals(apiName)) { // Raw requests don't use the rest spec at all and are configured entirely by their parameters Map queryStringParams = new HashMap<>(params); - String method = Objects.requireNonNull(queryStringParams.remove("method"), "Method must be set to use raw request"); - String path = "/" + Objects.requireNonNull(queryStringParams.remove("path"), "Path must be set to use raw request"); - Request request = new Request(method, path); + String rawMethod = Objects.requireNonNull(queryStringParams.remove("method"), "Method must be set to use raw request"); + String rawPath = "/" + Objects.requireNonNull(queryStringParams.remove("path"), "Path must be set to use raw request"); + Request request = new Request(rawMethod, rawPath); // All other parameters are url parameters for (Map.Entry param : queryStringParams.entrySet()) { request.addParameter(param.getKey(), param.getValue()); @@ -73,6 +74,6 @@ public ClientYamlTestResponse callApi( throw new ClientYamlTestResponseException(e); } } - return super.callApi(apiName, params, entity, headers, nodeSelector, pathPredicate); + return super.callApi(apiName, method, params, entity, headers, nodeSelector, pathPredicate); } } diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java index 47f0df776b4cd..d22c7b736fb62 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java @@ -82,6 +82,7 @@ public class ClientYamlTestClient implements Closeable { */ public ClientYamlTestResponse callApi( String apiName, + String method, Map params, HttpEntity entity, Map headers, @@ -161,7 +162,15 @@ public ClientYamlTestResponse callApi( List supportedMethods = Arrays.asList(path.methods()); String requestMethod; - if (entity != null) { + if (method != null) { + // Method override specified - validate it's supported + if (supportedMethods.contains(method) == false) { + throw new IllegalArgumentException( + "method [" + method + "] is not supported by path [" + path.path() + "]. Supported methods: " + supportedMethods + ); + } + requestMethod = method; + } else if (entity != null) { if (false == restApi.isBodySupported()) { throw new IllegalArgumentException("body is not supported by [" + restApi.getName() + "] api"); } diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java index 031f8f908847c..fda0268f793ba 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java @@ -115,7 +115,7 @@ public ClientYamlTestResponse callApi( List> bodies, Map headers ) throws IOException { - return callApi(apiName, params, bodies, headers, NodeSelector.ANY); + return callApi(apiName, null, params, bodies, headers, NodeSelector.ANY); } /** @@ -128,6 +128,21 @@ public ClientYamlTestResponse callApi( List> bodies, Map headers, NodeSelector nodeSelector + ) throws IOException { + return callApi(apiName, null, params, bodies, headers, nodeSelector); + } + + /** + * Calls an elasticsearch api with the parameters and request body provided as arguments. + * Saves the obtained response in the execution context. + */ + public ClientYamlTestResponse callApi( + String apiName, + String method, + Map params, + List> bodies, + Map headers, + NodeSelector nodeSelector ) throws IOException { // makes a copy of the parameters before modifying them for this specific request Map requestParams = new HashMap<>(params); @@ -156,7 +171,7 @@ public ClientYamlTestResponse callApi( HttpEntity entity = createEntity(bodies, requestHeaders); try { - response = callApiInternal(apiName, requestParams, entity, requestHeaders, nodeSelector); + response = callApiInternal(apiName, method, requestParams, entity, requestHeaders, nodeSelector); return response; } catch (ClientYamlTestResponseException e) { response = e.getRestTestResponse(); @@ -231,12 +246,13 @@ private BytesRef bodyAsBytesRef(Map bodyAsMap, XContentType xCon // pkg-private for testing ClientYamlTestResponse callApiInternal( String apiName, + String method, Map params, HttpEntity entity, Map headers, NodeSelector nodeSelector ) throws IOException { - return clientYamlTestClient(apiName).callApi(apiName, params, entity, headers, nodeSelector, pathPredicate); + return clientYamlTestClient(apiName).callApi(apiName, method, params, entity, headers, nodeSelector, pathPredicate); } protected ClientYamlTestClient clientYamlTestClient(String apiName) { @@ -321,7 +337,7 @@ public Optional clusterHasCapabilities( private Optional checkCapability(NodeSelector nodeSelector, Map params) { try { - ClientYamlTestResponse resp = callApi("capabilities", params, emptyList(), emptyMap(), nodeSelector); + ClientYamlTestResponse resp = callApi("capabilities", null, params, emptyList(), emptyMap(), nodeSelector); // anything other than 200 should result in an exception, handled below assert resp.getStatusCode() == 200 : "Unknown response code " + resp.getStatusCode(); return Optional.ofNullable(resp.evaluate("supported")); diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ImpersonateOfficialClientTestClient.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ImpersonateOfficialClientTestClient.java index 3049dcfaa988d..7bbae69b902e8 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ImpersonateOfficialClientTestClient.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ImpersonateOfficialClientTestClient.java @@ -43,6 +43,7 @@ public ImpersonateOfficialClientTestClient( @Override public ClientYamlTestResponse callApi( String apiName, + String method, Map params, HttpEntity entity, Map headers, @@ -50,6 +51,6 @@ public ClientYamlTestResponse callApi( BiPredicate pathPredicate ) throws IOException { headers.put("x-elastic-client-meta", meta); - return super.callApi(apiName, params, entity, headers, nodeSelector, pathPredicate); + return super.callApi(apiName, method, params, entity, headers, nodeSelector, pathPredicate); } } diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/ApiCallSection.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/ApiCallSection.java index 7570097f1275c..ebaacfeebf58a 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/ApiCallSection.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/ApiCallSection.java @@ -24,6 +24,7 @@ public class ApiCallSection { private final String api; + private String method; private final Map params = new HashMap<>(); private final Map headers = new HashMap<>(); private final List> bodies = new ArrayList<>(); @@ -47,9 +48,24 @@ public ApiCallSection copyWithNewApi(String api) { copy.addBody(b); } copy.nodeSelector = nodeSelector; + copy.method = method; return copy; } + /** + * Gets the HTTP method override for this request, or null if not specified to randomly pick one of the methods specified in the spec. + */ + public String getMethod() { + return method; + } + + /** + * Sets the HTTP method override for this request. If null, a method will be randomly picked from the ones specified in the spec. + */ + public void setMethod(String method) { + this.method = method; + } + public Map getParams() { // make sure we never modify the parameters once returned return unmodifiableMap(params); diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index 79ceec5fdf04d..8622684f10e41 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -89,6 +89,7 @@ public static DoSection parse(XContentParser parser) throws IOException { ApiCallSection apiCallSection = null; NodeSelector nodeSelector = NodeSelector.ANY; Map headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + String method = null; List expectedWarnings = new ArrayList<>(); List expectedWarningsRegex = new ArrayList<>(); List allowedWarnings = new ArrayList<>(); @@ -197,6 +198,8 @@ public static DoSection parse(XContentParser parser) throws IOException { apiCallSection.addBody(bodyParser.mapOrdered()); } } + } else if ("method".equals(paramName)) { + method = parser.text(); } else { apiCallSection.addParam(paramName, parser.text()); } @@ -225,6 +228,7 @@ public static DoSection parse(XContentParser parser) throws IOException { } apiCallSection.addHeaders(headers); apiCallSection.setNodeSelector(nodeSelector); + apiCallSection.setMethod(method); doSection.setApiCallSection(apiCallSection); doSection.setExpectedWarningHeaders(unmodifiableList(expectedWarnings)); doSection.setExpectedWarningHeadersRegex(unmodifiableList(expectedWarningsRegex)); @@ -349,6 +353,7 @@ public void execute(ClientYamlTestExecutionContext executionContext) throws IOEx try { ClientYamlTestResponse response = executionContext.callApi( apiCallSection.getApi(), + apiCallSection.getMethod(), apiCallSection.getParams(), apiCallSection.getBodies(), apiCallSection.getHeaders(), diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContextTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContextTests.java index 2ef73b6ab0734..f6e1f0e9f6a0b 100644 --- a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContextTests.java +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContextTests.java @@ -39,6 +39,7 @@ public void testHeadersSupportStashedValueReplacement() throws IOException { @Override ClientYamlTestResponse callApiInternal( String apiName, + String method, Map params, HttpEntity entity, Map headers, @@ -56,7 +57,7 @@ ClientYamlTestResponse callApiInternal( context.stash().stashValue("c", "bar1"); assertNull(headersRef.get()); - context.callApi("test", Collections.emptyMap(), Collections.emptyList(), headers); + context.callApi("test", null, Collections.emptyMap(), Collections.emptyList(), headers, NodeSelector.ANY); assertNotNull(headersRef.get()); assertNotEquals(headers, headersRef.get()); @@ -77,6 +78,7 @@ public void testStashHeadersOnException() throws IOException { @Override ClientYamlTestResponse callApiInternal( String apiName, + String method, Map params, HttpEntity entity, Map headers, @@ -89,7 +91,7 @@ ClientYamlTestResponse callApiInternal( headers.put("Accept", "application/json"); headers.put("Authorization", "Basic password=="); try { - context.callApi("test", Collections.emptyMap(), Collections.emptyList(), headers); + context.callApi("test", null, Collections.emptyMap(), Collections.emptyList(), headers, NodeSelector.ANY); } catch (Exception e) { // do nothing...behavior we are testing is the finally block of the production code } diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java index 465ff7c73e74b..757fe2111db6d 100644 --- a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java @@ -34,6 +34,7 @@ import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -202,6 +203,23 @@ public void testWarningHeadersRegex() { section.checkWarningHeaders(singletonList(testHeaderWithQuotesAndBackslashes)); } + public void testParseDoSectionWithExplicitMethod() throws Exception { + // If the specification defines multiple methods for an api, it randomly picks one. This can lead to bwc test issues, + // if some methods are not available in earlier versions and requires to explicitly set the method to use. + parser = createParser(YamlXContent.yamlXContent, """ + api: + method: GET + body: {"foo": "bar"}"""); + + DoSection doSection = DoSection.parse(parser); + ApiCallSection apiCallSection = doSection.getApiCallSection(); + + assertThat(apiCallSection, notNullValue()); + assertThat(apiCallSection.getApi(), equalTo("api")); + assertThat(apiCallSection.getMethod(), equalTo("GET")); + assertThat(apiCallSection.getBodies(), contains(Map.of("foo", "bar"))); + } + public void testParseDoSectionNoBody() throws Exception { parser = createParser(YamlXContent.yamlXContent, """ get: