Skip to content

Commit b67892e

Browse files
committed
[REST tests] Randomized REST path chosen if more than one is available with current parameters
Previously we would always take the same path if more than one was available
1 parent 579a67b commit b67892e

File tree

2 files changed

+28
-39
lines changed

2 files changed

+28
-39
lines changed

src/test/java/org/elasticsearch/test/rest/client/RestClient.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ private HttpRequestBuilder callApiBuilder(String apiName, Map<String, String> pa
174174

175175
//the http method is randomized (out of the available ones with the chosen api)
176176
return httpRequestBuilder.method(RandomizedTest.randomFrom(restApi.getSupportedMethods(pathParts.keySet())))
177-
.path(restApi.getFinalPath(pathParts));
177+
.path(RandomizedTest.randomFrom(restApi.getFinalPaths(pathParts)));
178178
}
179179

180180
private RestApi restApi(String apiName) {

src/test/java/org/elasticsearch/test/rest/spec/RestApi.java

Lines changed: 27 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.google.common.collect.Maps;
2323
import org.apache.http.client.methods.HttpPost;
2424
import org.apache.http.client.methods.HttpPut;
25-
import org.apache.lucene.util.PriorityQueue;
2625

2726
import java.util.Arrays;
2827
import java.util.List;
@@ -117,60 +116,50 @@ void addPathPart(String pathPart) {
117116
* Finds the best matching rest path given the current parameters and replaces
118117
* placeholders with their corresponding values received as arguments
119118
*/
120-
public String getFinalPath(Map<String, String> pathParams) {
121-
RestPath matchingRestPath = findMatchingRestPath(pathParams.keySet());
122-
String path = matchingRestPath.path;
123-
for (Map.Entry<String, String> paramEntry : matchingRestPath.params.entrySet()) {
124-
// replace path placeholders with actual values
125-
String value = pathParams.get(paramEntry.getValue());
126-
if (value == null) {
127-
// if a value is missing, we got the wrong path or the test was
128-
// specified incorrectly
129-
// TODO: What if more than one path exists? for example: PUT
130-
// index/type/_mapping vs. PUT index/_maping/type? Should we
131-
// randomize?
132-
throw new IllegalArgumentException("parameter [" + paramEntry.getValue() + "] missing");
133-
} else {
119+
public String[] getFinalPaths(Map<String, String> pathParams) {
120+
121+
List<RestPath> matchingRestPaths = findMatchingRestPaths(pathParams.keySet());
122+
if (matchingRestPaths == null || matchingRestPaths.isEmpty()) {
123+
throw new IllegalArgumentException("unable to find matching rest path for api [" + name + "] and params " + pathParams);
124+
}
125+
126+
String[] paths = new String[matchingRestPaths.size()];
127+
for (int i = 0; i < matchingRestPaths.size(); i++) {
128+
RestPath restPath = matchingRestPaths.get(i);
129+
String path = restPath.path;
130+
for (Map.Entry<String, String> paramEntry : restPath.params.entrySet()) {
131+
// replace path placeholders with actual values
132+
String value = pathParams.get(paramEntry.getValue());
133+
if (value == null) {
134+
throw new IllegalArgumentException("parameter [" + paramEntry.getValue() + "] missing");
135+
}
134136
path = path.replace(paramEntry.getKey(), value);
135137
}
138+
paths[i] = path;
136139
}
137-
return path;
140+
return paths;
138141
}
139142

140143
/**
141-
* Finds the best matching rest path out of the available ones with the current api (based on REST spec).
144+
* Finds the matching rest paths out of the available ones with the current api (based on REST spec).
142145
*
143146
* The best path is the one that has exactly the same number of placeholders to replace
144147
* (e.g. /{index}/{type}/{id} when the params are exactly index, type and id).
145-
* Otherwise there might be additional placeholders, thus we use the path with the least additional placeholders.
146-
* (e.g. get with only index and id as parameters, the closest (and only) path contains {type} too, which becomes _all)
147148
*/
148-
private RestPath findMatchingRestPath(Set<String> restParams) {
149+
private List<RestPath> findMatchingRestPaths(Set<String> restParams) {
149150

151+
List<RestPath> matchingRestPaths = Lists.newArrayList();
150152
RestPath[] restPaths = buildRestPaths();
151153

152-
//We need to find the path that has exactly the placeholders corresponding to our params
153-
//If there's no exact match we fallback to the closest one (with as less additional placeholders as possible)
154-
//The fallback is needed for:
155-
//1) get, get_source and exists with only index and id => /{index}/_all/{id} (
156-
//2) search with only type => /_all/{type/_search
157-
PriorityQueue<RestPath> restPathQueue = new PriorityQueue<RestPath>(1) {
158-
@Override
159-
protected boolean lessThan(RestPath a, RestPath b) {
160-
return a.params.size() >= b.params.size();
161-
}
162-
};
163154
for (RestPath restPath : restPaths) {
164-
if (restPath.params.values().containsAll(restParams)) {
165-
restPathQueue.insertWithOverflow(restPath);
155+
if (restPath.params.size() == restParams.size()) {
156+
if (restPath.params.values().containsAll(restParams)) {
157+
matchingRestPaths.add(restPath);
158+
}
166159
}
167160
}
168161

169-
if (restPathQueue.size() > 0) {
170-
return restPathQueue.top();
171-
}
172-
173-
throw new IllegalArgumentException("unable to find best path for api [" + name + "] and params " + restParams);
162+
return matchingRestPaths;
174163
}
175164

176165
private RestPath[] buildRestPaths() {

0 commit comments

Comments
 (0)