prevent registration of duplicated rest spec (#18504)

Rather than having one win against the other, reject duplicated apis. Also enforce the convention that see the api name have the same name as the name of the rest spec file that defines it.

test/framework/src/main/java/org/elasticsearch/test/rest/parser/RestTestSuiteParser.java

@@ -18,6 +18,11 @@
  */
 package org.elasticsearch.test.rest.parser;
 
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.yaml.YamlXContent;
+import org.elasticsearch.test.rest.section.RestTestSuite;
+import org.elasticsearch.test.rest.section.TestSection;
+
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
@@ -25,11 +30,6 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
 
-import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.common.xcontent.yaml.YamlXContent;
-import org.elasticsearch.test.rest.section.RestTestSuite;
-import org.elasticsearch.test.rest.section.TestSection;
-
 /**
  * Parser for a complete test suite (yaml file)
  */
@@ -57,14 +57,11 @@ public class RestTestSuiteParser implements RestTestFragmentParser<RestTestSuite
             }
         }
 
-        XContentParser parser = YamlXContent.yamlXContent.createParser(Files.newInputStream(file));
-        try {
+        try (XContentParser parser = YamlXContent.yamlXContent.createParser(Files.newInputStream(file))) {
             RestTestSuiteParseContext testParseContext = new RestTestSuiteParseContext(api, filename, parser);
             return parse(testParseContext);
         } catch(Exception e) {
             throw new RestTestParseException("Error parsing " + api + "/" + filename, e);
-        } finally {
-            parser.close();
         }
     }
 

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

@@ -32,6 +32,7 @@ import java.util.Set;
  */
 public class RestApi {
 
+    private final String location;
     private final String name;
     private List<String> methods = new ArrayList<>();
     private List<String> paths = new ArrayList<>();
@@ -43,7 +44,8 @@ public class RestApi {
         NOT_SUPPORTED, OPTIONAL, REQUIRED
     }
 
-    RestApi(String name) {
+    RestApi(String location, String name) {
+        this.location = location;
         this.name = name;
     }
 
@@ -51,6 +53,10 @@ public class RestApi {
         return name;
     }
 
+    public String getLocation() {
+        return location;
+    }
+
     public List<String> getMethods() {
         return methods;
     }

test/framework/src/main/java/org/elasticsearch/test/rest/spec/RestApiParser.java

@@ -27,113 +27,107 @@ import java.io.IOException;
  */
 public class RestApiParser {
 
-    public RestApi parse(XContentParser parser) throws IOException {
+    public RestApi parse(String location, XContentParser parser) throws IOException {
 
-        try {
-            while ( parser.nextToken() != XContentParser.Token.FIELD_NAME ) {
-                //move to first field name
-            }
+        while ( parser.nextToken() != XContentParser.Token.FIELD_NAME ) {
+            //move to first field name
+        }
 
-            RestApi restApi = new RestApi(parser.currentName());
+        RestApi restApi = new RestApi(location, parser.currentName());
 
-            int level = -1;
-            while (parser.nextToken() != XContentParser.Token.END_OBJECT || level >= 0) {
+        int level = -1;
+        while (parser.nextToken() != XContentParser.Token.END_OBJECT || level >= 0) {
 
-                if (parser.currentToken() == XContentParser.Token.FIELD_NAME) {
-                    if ("methods".equals(parser.currentName())) {
-                        parser.nextToken();
-                        while (parser.nextToken() == XContentParser.Token.VALUE_STRING) {
-                            restApi.addMethod(parser.text());
-                        }
+            if (parser.currentToken() == XContentParser.Token.FIELD_NAME) {
+                if ("methods".equals(parser.currentName())) {
+                    parser.nextToken();
+                    while (parser.nextToken() == XContentParser.Token.VALUE_STRING) {
+                        restApi.addMethod(parser.text());
                     }
+                }
 
-                    if ("url".equals(parser.currentName())) {
-                        String currentFieldName = "url";
-                        int innerLevel = -1;
-                        while(parser.nextToken() != XContentParser.Token.END_OBJECT || innerLevel >= 0) {
-                            if (parser.currentToken() == XContentParser.Token.FIELD_NAME) {
-                                currentFieldName = parser.currentName();
-                            }
+                if ("url".equals(parser.currentName())) {
+                    String currentFieldName = "url";
+                    int innerLevel = -1;
+                    while(parser.nextToken() != XContentParser.Token.END_OBJECT || innerLevel >= 0) {
+                        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) {
-                                    restApi.addPath(parser.text());
-                                }
+                        if (parser.currentToken() == XContentParser.Token.START_ARRAY && "paths".equals(currentFieldName)) {
+                            while (parser.nextToken() == XContentParser.Token.VALUE_STRING) {
+                                restApi.addPath(parser.text());
                             }
+                        }
 
-                            if (parser.currentToken() == XContentParser.Token.START_OBJECT && "parts".equals(currentFieldName)) {
-                                while (parser.nextToken() == XContentParser.Token.FIELD_NAME) {
-                                    restApi.addPathPart(parser.currentName());
-                                    parser.nextToken();
-                                    if (parser.currentToken() != XContentParser.Token.START_OBJECT) {
-                                        throw new IOException("Expected parts field in rest api definition to contain an object");
-                                    }
-                                    parser.skipChildren();
+                        if (parser.currentToken() == XContentParser.Token.START_OBJECT && "parts".equals(currentFieldName)) {
+                            while (parser.nextToken() == XContentParser.Token.FIELD_NAME) {
+                                restApi.addPathPart(parser.currentName());
+                                parser.nextToken();
+                                if (parser.currentToken() != XContentParser.Token.START_OBJECT) {
+                                    throw new IOException("Expected parts field in rest api definition to contain an object");
                                 }
+                                parser.skipChildren();
                             }
+                        }
 
-                            if (parser.currentToken() == XContentParser.Token.START_OBJECT && "params".equals(currentFieldName)) {
-                                while (parser.nextToken() == XContentParser.Token.FIELD_NAME) {
-                                    restApi.addParam(parser.currentName());
-                                    parser.nextToken();
-                                    if (parser.currentToken() != XContentParser.Token.START_OBJECT) {
-                                        throw new IOException("Expected params field in rest api definition to contain an object");
-                                    }
-                                    parser.skipChildren();
+                        if (parser.currentToken() == XContentParser.Token.START_OBJECT && "params".equals(currentFieldName)) {
+                            while (parser.nextToken() == XContentParser.Token.FIELD_NAME) {
+                                restApi.addParam(parser.currentName());
+                                parser.nextToken();
+                                if (parser.currentToken() != XContentParser.Token.START_OBJECT) {
+                                    throw new IOException("Expected params field in rest api definition to contain an object");
                                 }
+                                parser.skipChildren();
                             }
+                        }
 
-                            if (parser.currentToken() == XContentParser.Token.START_OBJECT) {
-                                innerLevel++;
-                            }
-                            if (parser.currentToken() == XContentParser.Token.END_OBJECT) {
-                                innerLevel--;
-                            }
+                        if (parser.currentToken() == XContentParser.Token.START_OBJECT) {
+                            innerLevel++;
+                        }
+                        if (parser.currentToken() == XContentParser.Token.END_OBJECT) {
+                            innerLevel--;
                         }
                     }
+                }
 
-                    if ("body".equals(parser.currentName())) {
-                        parser.nextToken();
-                        if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
-                            boolean requiredFound = false;
-                            while(parser.nextToken() != XContentParser.Token.END_OBJECT) {
-                                if (parser.currentToken() == XContentParser.Token.FIELD_NAME) {
-                                    if ("required".equals(parser.currentName())) {
-                                        requiredFound = true;
-                                        parser.nextToken();
-                                        if (parser.booleanValue()) {
-                                            restApi.setBodyRequired();
-                                        } else {
-                                            restApi.setBodyOptional();
-                                        }
+                if ("body".equals(parser.currentName())) {
+                    parser.nextToken();
+                    if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
+                        boolean requiredFound = false;
+                        while(parser.nextToken() != XContentParser.Token.END_OBJECT) {
+                            if (parser.currentToken() == XContentParser.Token.FIELD_NAME) {
+                                if ("required".equals(parser.currentName())) {
+                                    requiredFound = true;
+                                    parser.nextToken();
+                                    if (parser.booleanValue()) {
+                                        restApi.setBodyRequired();
+                                    } else {
+                                        restApi.setBodyOptional();
                                     }
                                 }
                             }
-                            if (!requiredFound) {
-                                restApi.setBodyOptional();
-                            }
+                        }
+                        if (!requiredFound) {
+                            restApi.setBodyOptional();
                         }
                     }
                 }
+            }
 
-                if (parser.currentToken() == XContentParser.Token.START_OBJECT) {
-                    level++;
-                }
-                if (parser.currentToken() == XContentParser.Token.END_OBJECT) {
-                    level--;
-                }
-
+            if (parser.currentToken() == XContentParser.Token.START_OBJECT) {
+                level++;
+            }
+            if (parser.currentToken() == XContentParser.Token.END_OBJECT) {
+                level--;
             }
 
-            parser.nextToken();
-            assert parser.currentToken() == XContentParser.Token.END_OBJECT : "Expected [END_OBJECT] but was ["  + parser.currentToken() +"]";
-            parser.nextToken();
+        }
 
-            return restApi;
+        parser.nextToken();
+        assert parser.currentToken() == XContentParser.Token.END_OBJECT : "Expected [END_OBJECT] but was ["  + parser.currentToken() +"]";
+        parser.nextToken();
 
-        } finally {
-            parser.close();
-        }
+        return restApi;
     }
-
 }

test/framework/src/main/java/org/elasticsearch/test/rest/spec/RestSpec.java

@@ -41,7 +41,11 @@ public class RestSpec {
     }
 
     void addApi(RestApi restApi) {
-        restApiMap.put(restApi.getName(), restApi);
+        RestApi previous = restApiMap.putIfAbsent(restApi.getName(), restApi);
+        if (previous != null) {
+            throw new IllegalArgumentException("cannot register api [" + restApi.getName() + "] found in [" + restApi.getLocation() + "]. "
+                    + "api with same name was already found in [" + previous.getLocation() + "]");
+        }
     }
 
     public RestApi getApi(String api) {
@@ -57,12 +61,20 @@ public class RestSpec {
      */
     public static RestSpec parseFrom(FileSystem fileSystem, String optionalPathPrefix, String... paths) throws IOException {
         RestSpec restSpec = new RestSpec();
+        RestApiParser restApiParser = new RestApiParser();
         for (String path : paths) {
             for (Path jsonFile : FileUtils.findJsonSpec(fileSystem, optionalPathPrefix, path)) {
                 try (InputStream stream = Files.newInputStream(jsonFile)) {
-                    XContentParser parser = JsonXContent.jsonXContent.createParser(stream);
-                    RestApi restApi = new RestApiParser().parse(parser);
-                    restSpec.addApi(restApi);
+                    try (XContentParser parser = JsonXContent.jsonXContent.createParser(stream)) {
+                        RestApi restApi = restApiParser.parse(jsonFile.toString(), parser);
+                        String filename = jsonFile.getFileName().toString();
+                        String expectedApiName = filename.substring(0, filename.lastIndexOf('.'));
+                        if (restApi.getName().equals(expectedApiName) == false) {
+                            throw new IllegalArgumentException("found api [" + restApi.getName() + "] in [" + jsonFile.toString() + "]. " +
+                                    "Each api is expected to have the same name as the file that defines it.");
+                        }
+                        restSpec.addApi(restApi);
+                    }
                 } catch (Throwable ex) {
                     throw new IOException("Can't parse rest spec file: [" + jsonFile + "]", ex);
                 }

test/framework/src/test/java/org/elasticsearch/test/rest/test/RestApiParserFailingTests.java

@@ -28,7 +28,7 @@ import java.io.IOException;
 import static org.hamcrest.Matchers.containsString;
 
 /**
- *
+ * These tests are not part of {@link RestApiParserTests} because the tested failures don't allow to consume the whole yaml stream
  */
 public class RestApiParserFailingTests extends ESTestCase {
     public void testBrokenSpecShouldThrowUsefulExceptionWhenParsingFailsOnParams() throws Exception {
@@ -42,7 +42,7 @@ public class RestApiParserFailingTests extends ESTestCase {
     private void parseAndExpectFailure(String brokenJson, String expectedErrorMessage) throws Exception {
         XContentParser parser = JsonXContent.jsonXContent.createParser(brokenJson);
         try {
-            new RestApiParser().parse(parser);
+            new RestApiParser().parse("location", parser);
             fail("Expected to fail parsing but did not happen");
         } catch (IOException e) {
             assertThat(e.getMessage(), containsString(expectedErrorMessage));

test/framework/src/test/java/org/elasticsearch/test/rest/test/RestApiParserTests.java

@@ -29,7 +29,7 @@ import static org.hamcrest.Matchers.notNullValue;
 public class RestApiParserTests extends AbstractParserTestCase {
     public void testParseRestSpecIndexApi() throws Exception {
         parser = JsonXContent.jsonXContent.createParser(REST_SPEC_INDEX_API);
-        RestApi restApi = new RestApiParser().parse(parser);
+        RestApi restApi = new RestApiParser().parse("location", parser);
 
         assertThat(restApi, notNullValue());
         assertThat(restApi.getName(), equalTo("index"));
@@ -51,7 +51,7 @@ public class RestApiParserTests extends AbstractParserTestCase {
 
     public void testParseRestSpecGetTemplateApi() throws Exception {
         parser = JsonXContent.jsonXContent.createParser(REST_SPEC_GET_TEMPLATE_API);
-        RestApi restApi = new RestApiParser().parse(parser);
+        RestApi restApi = new RestApiParser().parse("location", parser);
         assertThat(restApi, notNullValue());
         assertThat(restApi.getName(), equalTo("indices.get_template"));
         assertThat(restApi.getMethods().size(), equalTo(1));
@@ -68,7 +68,7 @@ public class RestApiParserTests extends AbstractParserTestCase {
 
     public void testParseRestSpecCountApi() throws Exception {
         parser = JsonXContent.jsonXContent.createParser(REST_SPEC_COUNT_API);
-        RestApi restApi = new RestApiParser().parse(parser);
+        RestApi restApi = new RestApiParser().parse("location", parser);
         assertThat(restApi, notNullValue());
         assertThat(restApi.getName(), equalTo("count"));
         assertThat(restApi.getMethods().size(), equalTo(2));