Skip to content

Commit e93308c

Browse files
javannakarmi
authored andcommitted
Adapt YAML runner to new REST API specification format
The logic for choosing the path to use when running tests has been simplified, as a consequence of the path parts being listed under each path in the spec. The special case for create and index has been removed. (cherry picked from commit 257fc77)
1 parent 3d9dc30 commit e93308c

File tree

8 files changed

+708
-418
lines changed

8 files changed

+708
-418
lines changed

test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java

Lines changed: 32 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
package org.elasticsearch.test.rest.yaml;
2020

2121
import com.carrotsearch.randomizedtesting.RandomizedTest;
22-
2322
import org.apache.http.HttpEntity;
2423
import org.apache.http.HttpHost;
2524
import org.apache.http.client.methods.HttpGet;
@@ -38,14 +37,14 @@
3837
import org.elasticsearch.client.WarningsHandler;
3938
import org.elasticsearch.common.CheckedSupplier;
4039
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi;
41-
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestPath;
4240
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestSpec;
4341

4442
import java.io.Closeable;
4543
import java.io.IOException;
4644
import java.io.UncheckedIOException;
4745
import java.net.URI;
4846
import java.net.URISyntaxException;
47+
import java.util.Arrays;
4948
import java.util.HashMap;
5049
import java.util.List;
5150
import java.util.Map;
@@ -100,19 +99,20 @@ public ClientYamlTestResponse callApi(String apiName, Map<String, String> params
10099

101100
ClientYamlSuiteRestApi restApi = restApi(apiName);
102101

102+
Set<String> apiRequiredParameters = restApi.getParams().entrySet().stream().filter(Entry::getValue).map(Entry::getKey)
103+
.collect(Collectors.toSet());
104+
105+
List<ClientYamlSuiteRestApi.Path> bestPaths = restApi.getBestMatchingPaths(params.keySet());
106+
//the rest path to use is randomized out of the matching ones (if more than one)
107+
ClientYamlSuiteRestApi.Path path = RandomizedTest.randomFrom(bestPaths);
108+
103109
//divide params between ones that go within query string and ones that go within path
104110
Map<String, String> pathParts = new HashMap<>();
105111
Map<String, String> queryStringParams = new HashMap<>();
106112

107-
Set<String> apiRequiredPathParts = restApi.getPathParts().entrySet().stream().filter(Entry::getValue).map(Entry::getKey)
108-
.collect(Collectors.toSet());
109-
Set<String> apiRequiredParameters = restApi.getParams().entrySet().stream().filter(Entry::getValue).map(Entry::getKey)
110-
.collect(Collectors.toSet());
111-
112113
for (Map.Entry<String, String> entry : params.entrySet()) {
113-
if (restApi.getPathParts().containsKey(entry.getKey())) {
114+
if (path.getParts().contains(entry.getKey())) {
114115
pathParts.put(entry.getKey(), entry.getValue());
115-
apiRequiredPathParts.remove(entry.getKey());
116116
} else if (restApi.getParams().containsKey(entry.getKey())
117117
|| restSpec.isGlobalParameter(entry.getKey())
118118
|| restSpec.isClientParameter(entry.getKey())) {
@@ -124,16 +124,33 @@ public ClientYamlTestResponse callApi(String apiName, Map<String, String> params
124124
}
125125
}
126126

127-
if (false == apiRequiredPathParts.isEmpty()) {
128-
throw new IllegalArgumentException(
129-
"missing required path part: " + apiRequiredPathParts + " by [" + restApi.getName() + "] api");
130-
}
131127
if (false == apiRequiredParameters.isEmpty()) {
132128
throw new IllegalArgumentException(
133129
"missing required parameter: " + apiRequiredParameters + " by [" + restApi.getName() + "] api");
134130
}
135131

136-
List<String> supportedMethods = restApi.getSupportedMethods(pathParts.keySet());
132+
Set<String> partNames = pathParts.keySet();
133+
if (path.getParts().size() != partNames.size() || path.getParts().containsAll(partNames) == false) {
134+
throw new IllegalStateException("provided path parts don't match the best matching path: "
135+
+ path.getParts() + " - " + partNames);
136+
}
137+
138+
String finalPath = path.getPath();
139+
for (Entry<String, String> pathPart : pathParts.entrySet()) {
140+
try {
141+
//Encode rules for path and query string parameters are different. We use URI to encode the path. We need to encode each
142+
// path part separately, as each one might contain slashes that need to be escaped, which needs to be done manually.
143+
// We prepend "/" to the path part to handle parts that start with - or other invalid characters.
144+
URI uri = new URI(null, null, null, -1, "/" + pathPart.getValue(), null, null);
145+
//manually escape any slash that each part may contain
146+
String encodedPathPart = uri.getRawPath().substring(1).replaceAll("/", "%2F");
147+
finalPath = finalPath.replace("{" + pathPart.getKey() + "}", encodedPathPart);
148+
} catch (URISyntaxException e) {
149+
throw new RuntimeException("unable to build uri", e);
150+
}
151+
}
152+
153+
List<String> supportedMethods = Arrays.asList(path.getMethods());
137154
String requestMethod;
138155
if (entity != null) {
139156
if (false == restApi.isBodySupported()) {
@@ -157,32 +174,8 @@ public ClientYamlTestResponse callApi(String apiName, Map<String, String> params
157174
requestMethod = RandomizedTest.randomFrom(supportedMethods);
158175
}
159176

160-
//the rest path to use is randomized out of the matching ones (if more than one)
161-
ClientYamlSuiteRestPath restPath = RandomizedTest.randomFrom(restApi.getFinalPaths(pathParts));
162-
//Encode rules for path and query string parameters are different. We use URI to encode the path.
163-
//We need to encode each path part separately, as each one might contain slashes that need to be escaped, which needs to
164-
//be done manually.
165-
String requestPath;
166-
if (restPath.getPathParts().length == 0) {
167-
requestPath = "/";
168-
} else {
169-
StringBuilder finalPath = new StringBuilder();
170-
for (String pathPart : restPath.getPathParts()) {
171-
try {
172-
finalPath.append('/');
173-
// We prepend "/" to the path part to handle parts that start with - or other invalid characters
174-
URI uri = new URI(null, null, null, -1, "/" + pathPart, null, null);
175-
//manually escape any slash that each part may contain
176-
finalPath.append(uri.getRawPath().substring(1).replaceAll("/", "%2F"));
177-
} catch (URISyntaxException e) {
178-
throw new RuntimeException("unable to build uri", e);
179-
}
180-
}
181-
requestPath = finalPath.toString();
182-
}
183-
184177
logger.debug("calling api [{}]", apiName);
185-
Request request = new Request(requestMethod, requestPath);
178+
Request request = new Request(requestMethod, finalPath);
186179
for (Map.Entry<String, String> param : queryStringParams.entrySet()) {
187180
request.addParameter(param.getKey(), param.getValue());
188181
}

test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import java.nio.file.Files;
4949
import java.nio.file.Path;
5050
import java.util.ArrayList;
51+
import java.util.Arrays;
5152
import java.util.Comparator;
5253
import java.util.HashMap;
5354
import java.util.HashSet;
@@ -280,9 +281,15 @@ private static void validateSpec(ClientYamlSuiteRestSpec restSpec) {
280281
if (validateSpec) {
281282
StringBuilder errorMessage = new StringBuilder();
282283
for (ClientYamlSuiteRestApi restApi : restSpec.getApis()) {
283-
if (restApi.getMethods().contains("GET") && restApi.isBodySupported()) {
284-
if (!restApi.getMethods().contains("POST")) {
285-
errorMessage.append("\n- ").append(restApi.getName()).append(" supports GET with a body but doesn't support POST");
284+
if (restApi.isBodySupported()) {
285+
for (ClientYamlSuiteRestApi.Path path : restApi.getPaths()) {
286+
List<String> methodsList = Arrays.asList(path.getMethods());
287+
if (methodsList.contains("GET") && restApi.isBodySupported()) {
288+
if (!methodsList.contains("POST")) {
289+
errorMessage.append("\n- ").append(restApi.getName())
290+
.append(" supports GET with a body but doesn't support POST");
291+
}
292+
}
286293
}
287294
}
288295
}

test/framework/src/main/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApi.java

Lines changed: 83 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,18 @@
1818
*/
1919
package org.elasticsearch.test.rest.yaml.restspec;
2020

21-
import org.apache.http.client.methods.HttpPost;
22-
import org.apache.http.client.methods.HttpPut;
21+
import org.elasticsearch.common.collect.Tuple;
2322

2423
import java.util.ArrayList;
24+
import java.util.Collection;
25+
import java.util.Comparator;
2526
import java.util.HashMap;
27+
import java.util.LinkedHashSet;
2628
import java.util.List;
2729
import java.util.Locale;
2830
import java.util.Map;
31+
import java.util.Objects;
32+
import java.util.PriorityQueue;
2933
import java.util.Set;
3034

3135
/**
@@ -35,9 +39,7 @@ public class ClientYamlSuiteRestApi {
3539

3640
private final String location;
3741
private final String name;
38-
private List<String> methods = new ArrayList<>();
39-
private List<String> paths = new ArrayList<>();
40-
private Map<String, Boolean> pathParts = new HashMap<>();
42+
private Set<Path> paths = new LinkedHashSet<>();
4143
private Map<String, Boolean> params = new HashMap<>();
4244
private Body body = Body.NOT_SUPPORTED;
4345
private Stability stability = Stability.UNKNOWN;
@@ -64,60 +66,17 @@ public String getLocation() {
6466
return location;
6567
}
6668

67-
public List<String> getMethods() {
68-
return methods;
69-
}
70-
71-
/**
72-
* Returns the supported http methods given the rest parameters provided
73-
*/
74-
public List<String> getSupportedMethods(Set<String> restParams) {
75-
//we try to avoid hardcoded mappings but the index api is the exception
76-
if ("index".equals(name) || "create".equals(name)) {
77-
List<String> indexMethods = new ArrayList<>();
78-
for (String method : methods) {
79-
if (restParams.contains("id")) {
80-
//PUT when the id is provided
81-
if (HttpPut.METHOD_NAME.equals(method)) {
82-
indexMethods.add(method);
83-
}
84-
} else {
85-
//POST without id
86-
if (HttpPost.METHOD_NAME.equals(method)) {
87-
indexMethods.add(method);
88-
}
89-
}
90-
}
91-
return indexMethods;
69+
void addPath(String path, String[] methods, Set<String> parts) {
70+
boolean add = this.paths.add(new Path(path, methods, parts));
71+
if (add == false) {
72+
throw new IllegalArgumentException("Found duplicate path [" + path + "]");
9273
}
93-
94-
return methods;
95-
}
96-
97-
void addMethod(String method) {
98-
this.methods.add(method);
9974
}
10075

101-
public List<String> getPaths() {
76+
public Collection<Path> getPaths() {
10277
return paths;
10378
}
10479

105-
void addPath(String path) {
106-
this.paths.add(path);
107-
}
108-
109-
/**
110-
* Gets all path parts supported by the api. For every path part defines if it
111-
* is required or optional.
112-
*/
113-
public Map<String, Boolean> getPathParts() {
114-
return pathParts;
115-
}
116-
117-
void addPathPart(String pathPart, boolean required) {
118-
this.pathParts.put(pathPart, required);
119-
}
120-
12180
/**
12281
* Gets all parameters supported by the api. For every parameter defines if it
12382
* is required or optional.
@@ -153,46 +112,86 @@ public void setStability(String stability) {
153112
public Stability getStability() { return this.stability; }
154113

155114
/**
156-
* Finds the best matching rest path given the current parameters and replaces
157-
* placeholders with their corresponding values received as arguments
115+
* Returns the best matching paths based on the provided parameters, which may include either path parts or query_string parameters.
116+
* The best path is the one that has exactly the same number of placeholders to replace
117+
* (e.g. /{index}/{type}/{id} when the path params are exactly index, type and id).
118+
* It returns a list instead of a single path as there are cases where there is more than one best matching path:
119+
* - /{index}/_alias/{name}, /{index}/_aliases/{name}
120+
* - /{index}/{type}/_mapping, /{index}/{type}/_mappings, /{index}/_mappings/{type}, /{index}/_mapping/{type}
158121
*/
159-
public ClientYamlSuiteRestPath[] getFinalPaths(Map<String, String> pathParams) {
160-
List<ClientYamlSuiteRestPath> matchingRestPaths = findMatchingRestPaths(pathParams.keySet());
161-
if (matchingRestPaths == null || matchingRestPaths.isEmpty()) {
162-
throw new IllegalArgumentException("unable to find matching rest path for api [" + name + "] and path params " + pathParams);
122+
public List<ClientYamlSuiteRestApi.Path> getBestMatchingPaths(Set<String> params) {
123+
PriorityQueue<Tuple<Integer, Path>> queue = new PriorityQueue<>(Comparator.comparing(Tuple::v1, (a, b) -> Integer.compare(b, a)));
124+
for (ClientYamlSuiteRestApi.Path path : paths) {
125+
int matches = 0;
126+
for (String actualParameter : params) {
127+
if (path.getParts().contains(actualParameter)) {
128+
matches++;
129+
}
130+
}
131+
if (matches == path.parts.size()) {
132+
queue.add(Tuple.tuple(matches, path));
133+
}
163134
}
164-
165-
ClientYamlSuiteRestPath[] restPaths = new ClientYamlSuiteRestPath[matchingRestPaths.size()];
166-
for (int i = 0; i < matchingRestPaths.size(); i++) {
167-
ClientYamlSuiteRestPath restPath = matchingRestPaths.get(i);
168-
restPaths[i] = restPath.replacePlaceholders(pathParams);
135+
if (queue.isEmpty()) {
136+
throw new IllegalStateException("Unable to find a matching path for api [" + name + "]" + params);
169137
}
170-
return restPaths;
138+
List<Path> paths = new ArrayList<>();
139+
Tuple<Integer, Path> poll = queue.poll();
140+
int maxMatches = poll.v1();
141+
do {
142+
paths.add(poll.v2());
143+
poll = queue.poll();
144+
} while (poll != null && poll.v1() == maxMatches);
145+
146+
return paths;
171147
}
172148

173-
/**
174-
* Finds the matching rest paths out of the available ones with the current api (based on REST spec).
175-
*
176-
* The best path is the one that has exactly the same number of placeholders to replace
177-
* (e.g. /{index}/{type}/{id} when the path params are exactly index, type and id).
178-
*/
179-
private List<ClientYamlSuiteRestPath> findMatchingRestPaths(Set<String> restParams) {
149+
public static class Path {
150+
private final String path;
151+
private final String[] methods;
152+
private final Set<String> parts;
180153

181-
List<ClientYamlSuiteRestPath> matchingRestPaths = new ArrayList<>();
182-
ClientYamlSuiteRestPath[] restPaths = buildRestPaths();
183-
for (ClientYamlSuiteRestPath restPath : restPaths) {
184-
if (restPath.matches(restParams)) {
185-
matchingRestPaths.add(restPath);
154+
private Path(String path, String[] methods, Set<String> parts) {
155+
this.path = Objects.requireNonNull(path, "path must not be null");
156+
this.methods = Objects.requireNonNull(methods, "methods must not be null");
157+
if (methods.length == 0) {
158+
throw new IllegalArgumentException("methods is empty, at least one method is required");
159+
}
160+
this.parts = Objects.requireNonNull(parts, "parts must not be null");
161+
for (String part : parts) {
162+
if (path.contains("{" + part + "}") == false) {
163+
throw new IllegalArgumentException("part [" + part + "] not contained in path [" + path + "]");
164+
}
186165
}
187166
}
188-
return matchingRestPaths;
189-
}
190167

191-
private ClientYamlSuiteRestPath[] buildRestPaths() {
192-
ClientYamlSuiteRestPath[] restPaths = new ClientYamlSuiteRestPath[paths.size()];
193-
for (int i = 0; i < restPaths.length; i++) {
194-
restPaths[i] = new ClientYamlSuiteRestPath(paths.get(i));
168+
public String getPath() {
169+
return path;
170+
}
171+
172+
public String[] getMethods() {
173+
return methods;
174+
}
175+
176+
public Set<String> getParts() {
177+
return parts;
178+
}
179+
180+
@Override
181+
public boolean equals(Object o) {
182+
if (this == o) {
183+
return true;
184+
}
185+
if (o == null || getClass() != o.getClass()) {
186+
return false;
187+
}
188+
Path path = (Path) o;
189+
return this.path.equals(path.path);
190+
}
191+
192+
@Override
193+
public int hashCode() {
194+
return Objects.hash(path);
195195
}
196-
return restPaths;
197196
}
198197
}

0 commit comments

Comments
 (0)