honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-02-17 10:25:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

Make yaml test runner stricter by enforcing required for paths and parameters (#27035)

Browse Source 
Till now the yaml test runner was verifying that the provided path parts and parameters are supported.
With this PR, yaml test runner also checks that all required path parts and parameters are provided.
This commit is contained in:
olcbean 2017-10-25 21:36:42 +02:00 committed by Nik Everett
parent 6533b165d6
commit 981b7f4d39
4 changed files with 83 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -19,6 +19,7 @@
package org.elasticsearch.test.rest.yaml;
import com.carrotsearch.randomizedtesting.RandomizedTest;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpHost;
@ -42,6 +43,9 @@ import java.net.URISyntaxException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.stream.Collectors;
/**
* Used by {@link ESClientYamlSuiteTestCase} to execute REST requests according to the tests written in yaml suite files. Wraps a
@ -80,24 +84,40 @@ public class ClientYamlTestClient {
//divide params between ones that go within query string and ones that go within path
Map<String, String> pathParts = new HashMap<>();
Map<String, String> queryStringParams = new HashMap<>();
Set<String> apiRequiredPathParts = restApi.getPathParts().entrySet().stream().filter(e -> e.getValue() == true).map(Entry::getKey)
.collect(Collectors.toSet());
Set<String> apiRequiredParameters = restApi.getParams().entrySet().stream().filter(e -> e.getValue() == true).map(Entry::getKey)
.collect(Collectors.toSet());
for (Map.Entry<String, String> entry : params.entrySet()) {
if (restApi.getPathParts().contains(entry.getKey())) {
if (restApi.getPathParts().containsKey(entry.getKey())) {
pathParts.put(entry.getKey(), entry.getValue());
apiRequiredPathParts.remove(entry.getKey());
} else if (restApi.getParams().containsKey(entry.getKey())
|| restSpec.isGlobalParameter(entry.getKey())
|| restSpec.isClientParameter(entry.getKey())) {
queryStringParams.put(entry.getKey(), entry.getValue());
apiRequiredParameters.remove(entry.getKey());
} else {
if (restApi.getParams().contains(entry.getKey()) || restSpec.isGlobalParameter(entry.getKey())
|| restSpec.isClientParameter(entry.getKey())) {
queryStringParams.put(entry.getKey(), entry.getValue());
} else {
throw new IllegalArgumentException("param [" + entry.getKey() + "] not supported in ["
+ restApi.getName() + "] " + "api");
}
throw new IllegalArgumentException(
"path/param [" + entry.getKey() + "] not supported by [" + restApi.getName() + "] " + "api");
}
}
if (false == apiRequiredPathParts.isEmpty()) {
throw new IllegalArgumentException(
"missing required path part: " + apiRequiredPathParts + " by [" + restApi.getName() + "] api");
}
if (false == apiRequiredParameters.isEmpty()) {
throw new IllegalArgumentException(
"missing required parameter: " + apiRequiredParameters + " by [" + restApi.getName() + "] api");
}
List<String> supportedMethods = restApi.getSupportedMethods(pathParts.keySet());
String requestMethod;
if (entity != null) {
if (!restApi.isBodySupported()) {
if (false == restApi.isBodySupported()) {
throw new IllegalArgumentException("body is not supported by [" + restApi.getName() + "] api");
}
String contentType = entity.getContentType().getValue();

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

@ -22,6 +22,7 @@ import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpPut;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -35,8 +36,8 @@ public class ClientYamlSuiteRestApi {
private final String name;
private List<String> methods = new ArrayList<>();
private List<String> paths = new ArrayList<>();
private List<String> pathParts = new ArrayList<>();
private List<String> params = new ArrayList<>();
private Map<String, Boolean> pathParts = new HashMap<>();
private Map<String, Boolean> params = new HashMap<>();
private Body body = Body.NOT_SUPPORTED;
public enum Body {
@ -98,20 +99,28 @@ public class ClientYamlSuiteRestApi {
this.paths.add(path);
}
public List<String> getPathParts() {
/**
* Gets all path parts supported by the api. For every path part defines if it
* is required or optional.
*/
public Map<String, Boolean> getPathParts() {
return pathParts;
}
void addPathPart(String pathPart) {
this.pathParts.add(pathPart);
void addPathPart(String pathPart, boolean required) {
this.pathParts.put(pathPart, required);
}
public List<String> getParams() {
/**
* Gets all parameters supported by the api. For every parameter defines if it
* is required or optional.
*/
public Map<String, Boolean> getParams() {
return params;
}
void addParam(String param) {
this.params.add(param);
void addParam(String param, boolean required) {
this.params.put(param, required);
}
void setBodyOptional() {

32
test/framework/src/main/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParser.java
View File

@ -18,6 +18,8 @@
*/
package org.elasticsearch.test.rest.yaml.restspec;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.XContentParser;
import java.io.IOException;
@ -27,6 +29,11 @@ import java.io.IOException;
*/
public class ClientYamlSuiteRestApiParser {
private static final ObjectParser<Parameter, Void> PARAMETER_PARSER = new ObjectParser<>("parameter", true, Parameter::new);
static {
PARAMETER_PARSER.declareBoolean(Parameter::setRequired, new ParseField("required"));
}
public ClientYamlSuiteRestApi parse(String location, XContentParser parser) throws IOException {
while ( parser.nextToken() != XContentParser.Token.FIELD_NAME ) {
@ -57,7 +64,6 @@ public class ClientYamlSuiteRestApiParser {
if (parser.currentToken() == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
}
if (parser.currentToken() == XContentParser.Token.START_ARRAY && "paths".equals(currentFieldName)) {
while (parser.nextToken() == XContentParser.Token.VALUE_STRING) {
String path = parser.text();
@ -71,30 +77,30 @@ public class ClientYamlSuiteRestApiParser {
if (parser.currentToken() == XContentParser.Token.START_OBJECT && "parts".equals(currentFieldName)) {
while (parser.nextToken() == XContentParser.Token.FIELD_NAME) {
String part = parser.currentName();
if (restApi.getPathParts().contains(part)) {
if (restApi.getPathParts().containsKey(part)) {
throw new IllegalArgumentException("Found duplicate part [" + part + "]");
}
restApi.addPathPart(part);
parser.nextToken();
if (parser.currentToken() != XContentParser.Token.START_OBJECT) {
throw new IllegalArgumentException("Expected parts field in rest api definition to contain an object");
}
parser.skipChildren();
restApi.addPathPart(part, PARAMETER_PARSER.parse(parser, null).isRequired());
}
}
if (parser.currentToken() == XContentParser.Token.START_OBJECT && "params".equals(currentFieldName)) {
while (parser.nextToken() == XContentParser.Token.FIELD_NAME) {
String param = parser.currentName();
if (restApi.getParams().contains(param)) {
if (restApi.getParams().containsKey(param)) {
throw new IllegalArgumentException("Found duplicate param [" + param + "]");
}
restApi.addParam(parser.currentName());
parser.nextToken();
if (parser.currentToken() != XContentParser.Token.START_OBJECT) {
throw new IllegalArgumentException("Expected params field in rest api definition to contain an object");
}
parser.skipChildren();
restApi.addParam(param, PARAMETER_PARSER.parse(parser, null).isRequired());
}
}
@ -124,7 +130,7 @@ public class ClientYamlSuiteRestApiParser {
}
}
}
if (!requiredFound) {
if (false == requiredFound) {
restApi.setBodyOptional();
}
}
@ -146,4 +152,14 @@ public class ClientYamlSuiteRestApiParser {
return restApi;
}
private static class Parameter {
private boolean required;
public boolean isRequired() {
return required;
}
public void setRequired(boolean required) {
this.required = required;
}
}
}

21
test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserTests.java
View File

@ -22,7 +22,9 @@ import org.elasticsearch.common.xcontent.yaml.YamlXContent;
import org.elasticsearch.test.rest.yaml.section.AbstractClientYamlTestFragmentParserTestCase;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.notNullValue;
public class ClientYamlSuiteRestApiParserTests extends AbstractClientYamlTestFragmentParserTestCase {
@ -39,11 +41,13 @@ public class ClientYamlSuiteRestApiParserTests extends AbstractClientYamlTestFra
assertThat(restApi.getPaths().get(0), equalTo("/{index}/{type}"));
assertThat(restApi.getPaths().get(1), equalTo("/{index}/{type}/{id}"));
assertThat(restApi.getPathParts().size(), equalTo(3));
assertThat(restApi.getPathParts().get(0), equalTo("id"));
assertThat(restApi.getPathParts().get(1), equalTo("index"));
assertThat(restApi.getPathParts().get(2), equalTo("type"));
assertThat(restApi.getPathParts().keySet(), containsInAnyOrder("id", "index", "type"));
assertThat(restApi.getPathParts(), hasEntry("index", true));
assertThat(restApi.getPathParts(), hasEntry("type", true));
assertThat(restApi.getPathParts(), hasEntry("id", false));
assertThat(restApi.getParams().size(), equalTo(4));
assertThat(restApi.getParams(), contains("wait_for_active_shards", "op_type", "parent", "refresh"));
assertThat(restApi.getParams().keySet(), containsInAnyOrder("wait_for_active_shards", "op_type", "parent", "refresh"));
restApi.getParams().entrySet().stream().forEach(e -> assertThat(e.getValue(), equalTo(false)));
assertThat(restApi.isBodySupported(), equalTo(true));
assertThat(restApi.isBodyRequired(), equalTo(true));
}
@ -59,7 +63,7 @@ public class ClientYamlSuiteRestApiParserTests extends AbstractClientYamlTestFra
assertThat(restApi.getPaths().get(0), equalTo("/_template"));
assertThat(restApi.getPaths().get(1), equalTo("/_template/{name}"));
assertThat(restApi.getPathParts().size(), equalTo(1));
assertThat(restApi.getPathParts().get(0), equalTo("name"));
assertThat(restApi.getPathParts(), hasEntry("name", false));
assertThat(restApi.getParams().size(), equalTo(0));
assertThat(restApi.isBodySupported(), equalTo(false));
assertThat(restApi.isBodyRequired(), equalTo(false));
@ -78,10 +82,11 @@ public class ClientYamlSuiteRestApiParserTests extends AbstractClientYamlTestFra
assertThat(restApi.getPaths().get(1), equalTo("/{index}/_count"));
assertThat(restApi.getPaths().get(2), equalTo("/{index}/{type}/_count"));
assertThat(restApi.getPathParts().size(), equalTo(2));
assertThat(restApi.getPathParts().get(0), equalTo("index"));
assertThat(restApi.getPathParts().get(1), equalTo("type"));
assertThat(restApi.getPathParts().keySet(), containsInAnyOrder("index", "type"));
restApi.getPathParts().entrySet().stream().forEach(e -> assertThat(e.getValue(), equalTo(false)));
assertThat(restApi.getParams().size(), equalTo(1));
assertThat(restApi.getParams().get(0), equalTo("ignore_unavailable"));
assertThat(restApi.getParams().keySet(), contains("ignore_unavailable"));
assertThat(restApi.getParams(), hasEntry("ignore_unavailable", false));
assertThat(restApi.isBodySupported(), equalTo(true));
assertThat(restApi.isBodyRequired(), equalTo(false));
}