From f9904bfe59903fbb0e2cd7dc9c3604a902e622ef Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 14 Nov 2017 14:53:00 +0100 Subject: [PATCH] REST spec: Validate that api name matches file name that contains it (#27366) This commit validates that each spec json file contains an API that has the same name as the file --- .../ClientYamlSuiteRestApiParser.java | 7 ++++++- ...entYamlSuiteRestApiParserFailingTests.java | 21 +++++++++++-------- .../ClientYamlSuiteRestApiParserTests.java | 10 ++++----- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParser.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParser.java index 66dc0a5705400..b0c2a713d615a 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParser.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParser.java @@ -40,7 +40,12 @@ public ClientYamlSuiteRestApi parse(String location, XContentParser parser) thro //move to first field name } - ClientYamlSuiteRestApi restApi = new ClientYamlSuiteRestApi(location, parser.currentName()); + String apiName = parser.currentName(); + if (location.endsWith(apiName + ".json") == false) { + throw new IllegalArgumentException("API [" + apiName + "] should have the same name as its file [" + location + "]"); + } + + ClientYamlSuiteRestApi restApi = new ClientYamlSuiteRestApi(location, apiName); int level = -1; while (parser.nextToken() != XContentParser.Token.END_OBJECT || level >= 0) { diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserFailingTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserFailingTests.java index a1d713db127b1..208232dcfd7cd 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserFailingTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserFailingTests.java @@ -20,7 +20,6 @@ import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.common.xcontent.yaml.YamlXContent; import org.elasticsearch.test.ESTestCase; @@ -49,7 +48,7 @@ public void testDuplicateMethods() throws Exception { " }," + " \"body\": null" + " }" + - "}", "Found duplicate method [PUT]"); + "}", "ping.json", "Found duplicate method [PUT]"); } public void testDuplicatePaths() throws Exception { @@ -69,7 +68,7 @@ public void testDuplicatePaths() throws Exception { " }," + " \"body\": null" + " }" + - "}", "Found duplicate path [/pingtwo]"); + "}", "ping.json", "Found duplicate path [/pingtwo]"); } public void testDuplicateParts() throws Exception { @@ -103,7 +102,7 @@ public void testDuplicateParts() throws Exception { " }," + " \"body\": null" + " }" + - "}", "Found duplicate part [index]"); + "}", "ping.json", "Found duplicate part [index]"); } public void testDuplicateParams() throws Exception { @@ -135,22 +134,26 @@ public void testDuplicateParams() throws Exception { " }," + " \"body\": null" + " }" + - "}", "Found duplicate param [timeout]"); + "}", "ping.json", "Found duplicate param [timeout]"); } public void testBrokenSpecShouldThrowUsefulExceptionWhenParsingFailsOnParams() throws Exception { - parseAndExpectFailure(BROKEN_SPEC_PARAMS, "Expected params field in rest api definition to contain an object"); + parseAndExpectFailure(BROKEN_SPEC_PARAMS, "ping.json", "Expected params field in rest api definition to contain an object"); } public void testBrokenSpecShouldThrowUsefulExceptionWhenParsingFailsOnParts() throws Exception { - parseAndExpectFailure(BROKEN_SPEC_PARTS, "Expected parts field in rest api definition to contain an object"); + parseAndExpectFailure(BROKEN_SPEC_PARTS, "ping.json", "Expected parts field in rest api definition to contain an object"); } - private void parseAndExpectFailure(String brokenJson, String expectedErrorMessage) throws Exception { + public void testSpecNameMatchesFilename() throws Exception { + parseAndExpectFailure("{\"ping\":{}}", "not_matching.json", "API [ping] should have the same name as its file [not_matching.json]"); + } + + private void parseAndExpectFailure(String brokenJson, String location, String expectedErrorMessage) throws Exception { XContentParser parser = createParser(YamlXContent.yamlXContent, brokenJson); ClientYamlSuiteRestApiParser restApiParser = new ClientYamlSuiteRestApiParser(); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> restApiParser.parse("location", parser)); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> restApiParser.parse(location, parser)); assertThat(e.getMessage(), containsString(expectedErrorMessage)); } diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserTests.java index ddf89f9a6fcca..cbc1f63261709 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserTests.java @@ -30,7 +30,7 @@ public class ClientYamlSuiteRestApiParserTests extends AbstractClientYamlTestFragmentParserTestCase { public void testParseRestSpecIndexApi() throws Exception { parser = createParser(YamlXContent.yamlXContent, REST_SPEC_INDEX_API); - ClientYamlSuiteRestApi restApi = new ClientYamlSuiteRestApiParser().parse("location", parser); + ClientYamlSuiteRestApi restApi = new ClientYamlSuiteRestApiParser().parse("index.json", parser); assertThat(restApi, notNullValue()); assertThat(restApi.getName(), equalTo("index")); @@ -47,14 +47,14 @@ public void testParseRestSpecIndexApi() throws Exception { assertThat(restApi.getPathParts(), hasEntry("id", false)); assertThat(restApi.getParams().size(), equalTo(4)); assertThat(restApi.getParams().keySet(), containsInAnyOrder("wait_for_active_shards", "op_type", "parent", "refresh")); - restApi.getParams().entrySet().stream().forEach(e -> assertThat(e.getValue(), equalTo(false))); + restApi.getParams().entrySet().forEach(e -> assertThat(e.getValue(), equalTo(false))); assertThat(restApi.isBodySupported(), equalTo(true)); assertThat(restApi.isBodyRequired(), equalTo(true)); } public void testParseRestSpecGetTemplateApi() throws Exception { parser = createParser(YamlXContent.yamlXContent, REST_SPEC_GET_TEMPLATE_API); - ClientYamlSuiteRestApi restApi = new ClientYamlSuiteRestApiParser().parse("location", parser); + ClientYamlSuiteRestApi restApi = new ClientYamlSuiteRestApiParser().parse("indices.get_template.json", parser); assertThat(restApi, notNullValue()); assertThat(restApi.getName(), equalTo("indices.get_template")); assertThat(restApi.getMethods().size(), equalTo(1)); @@ -71,7 +71,7 @@ public void testParseRestSpecGetTemplateApi() throws Exception { public void testParseRestSpecCountApi() throws Exception { parser = createParser(YamlXContent.yamlXContent, REST_SPEC_COUNT_API); - ClientYamlSuiteRestApi restApi = new ClientYamlSuiteRestApiParser().parse("location", parser); + ClientYamlSuiteRestApi restApi = new ClientYamlSuiteRestApiParser().parse("count.json", parser); assertThat(restApi, notNullValue()); assertThat(restApi.getName(), equalTo("count")); assertThat(restApi.getMethods().size(), equalTo(2)); @@ -83,7 +83,7 @@ public void testParseRestSpecCountApi() throws Exception { assertThat(restApi.getPaths().get(2), equalTo("/{index}/{type}/_count")); assertThat(restApi.getPathParts().size(), equalTo(2)); assertThat(restApi.getPathParts().keySet(), containsInAnyOrder("index", "type")); - restApi.getPathParts().entrySet().stream().forEach(e -> assertThat(e.getValue(), equalTo(false))); + restApi.getPathParts().entrySet().forEach(e -> assertThat(e.getValue(), equalTo(false))); assertThat(restApi.getParams().size(), equalTo(1)); assertThat(restApi.getParams().keySet(), contains("ignore_unavailable")); assertThat(restApi.getParams(), hasEntry("ignore_unavailable", false));