REST API parser should fail on duplicate params/paths/methods/parts (#20940)

This commit changes the current REST API parser to make it fail and throw an exception when a REST specification file contains a duplicated parameters, or path, or method, or path part.

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

@@ -42,7 +42,11 @@ public class ClientYamlSuiteRestApiParser {
                 if ("methods".equals(parser.currentName())) {
                     parser.nextToken();
                     while (parser.nextToken() == XContentParser.Token.VALUE_STRING) {
-                        restApi.addMethod(parser.text());
+                        String method = parser.text();
+                        if (restApi.getMethods().contains(method)) {
+                            throw new IllegalArgumentException("Found duplicate method [" + method + "]");
+                        }
+                        restApi.addMethod(method);
                     }
                 }
 
@@ -56,16 +60,24 @@ public class ClientYamlSuiteRestApiParser {
 
                         if (parser.currentToken() == XContentParser.Token.START_ARRAY && "paths".equals(currentFieldName)) {
                             while (parser.nextToken() == XContentParser.Token.VALUE_STRING) {
-                                restApi.addPath(parser.text());
+                                String path = parser.text();
+                                if (restApi.getPaths().contains(path)) {
+                                    throw new IllegalArgumentException("Found duplicate path [" + path + "]");
+                                }
+                                restApi.addPath(path);
                             }
                         }
 
                         if (parser.currentToken() == XContentParser.Token.START_OBJECT && "parts".equals(currentFieldName)) {
                             while (parser.nextToken() == XContentParser.Token.FIELD_NAME) {
-                                restApi.addPathPart(parser.currentName());
+                                String part = parser.currentName();
+                                if (restApi.getPathParts().contains(part)) {
+                                    throw new IllegalArgumentException("Found duplicate part [" + part + "]");
+                                }
+                                restApi.addPathPart(part);
                                 parser.nextToken();
                                 if (parser.currentToken() != XContentParser.Token.START_OBJECT) {
-                                    throw new IOException("Expected parts field in rest api definition to contain an object");
+                                    throw new IllegalArgumentException("Expected parts field in rest api definition to contain an object");
                                 }
                                 parser.skipChildren();
                             }
@@ -73,10 +85,14 @@ public class ClientYamlSuiteRestApiParser {
 
                         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)) {
+                                    throw new IllegalArgumentException("Found duplicate param [" + param + "]");
+                                }
                                 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");
+                                    throw new IllegalArgumentException("Expected params field in rest api definition to contain an object");
                                 }
                                 parser.skipChildren();
                             }

test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserFailingTests.java

@@ -32,6 +32,109 @@ import static org.hamcrest.Matchers.containsString;
  * stream
  */
 public class ClientYamlSuiteRestApiParserFailingTests extends ESTestCase {
+
+    public void testDuplicateMethods() throws Exception {
+       parseAndExpectFailure("{\n" +
+               "  \"ping\": {" +
+               "    \"documentation\": \"http://www.elasticsearch.org/guide/\"," +
+               "    \"methods\": [\"PUT\", \"PUT\"]," +
+               "    \"url\": {" +
+               "      \"path\": \"/\"," +
+               "      \"paths\": [\"/\"]," +
+               "      \"parts\": {" +
+               "      }," +
+               "      \"params\": {" +
+               "        \"type\" : \"boolean\",\n" +
+               "        \"description\" : \"Whether specified concrete indices should be ignored when unavailable (missing or closed)\"" +
+               "      }" +
+               "    }," +
+               "    \"body\": null" +
+               "  }" +
+               "}", "Found duplicate method [PUT]");
+    }
+
+    public void testDuplicatePaths() throws Exception {
+        parseAndExpectFailure("{\n" +
+                "  \"ping\": {" +
+                "    \"documentation\": \"http://www.elasticsearch.org/guide/\"," +
+                "    \"methods\": [\"PUT\"]," +
+                "    \"url\": {" +
+                "      \"path\": \"/pingone\"," +
+                "      \"paths\": [\"/pingone\", \"/pingtwo\", \"/pingtwo\"]," +
+                "      \"parts\": {" +
+                "      }," +
+                "      \"params\": {" +
+                "        \"type\" : \"boolean\",\n" +
+                "        \"description\" : \"Whether specified concrete indices should be ignored when unavailable (missing or closed)\"" +
+                "      }" +
+                "    }," +
+                "    \"body\": null" +
+                "  }" +
+                "}", "Found duplicate path [/pingtwo]");
+    }
+
+    public void testDuplicateParts() throws Exception {
+        parseAndExpectFailure("{\n" +
+                "  \"ping\": {" +
+                "    \"documentation\": \"http://www.elasticsearch.org/guide/\"," +
+                "    \"methods\": [\"PUT\"]," +
+                "    \"url\": {" +
+                "      \"path\": \"/\"," +
+                "      \"paths\": [\"/\"]," +
+                "      \"parts\": {" +
+                "        \"index\": {" +
+                "          \"type\" : \"string\",\n" +
+                "          \"description\" : \"index part\"\n" +
+                "        }," +
+                "        \"type\": {" +
+                "          \"type\" : \"string\",\n" +
+                "          \"description\" : \"type part\"\n" +
+                "        }," +
+                "        \"index\": {" +
+                "          \"type\" : \"string\",\n" +
+                "          \"description\" : \"index parameter part\"\n" +
+                "        }" +
+                "      }," +
+                "      \"params\": {" +
+                "        \"type\" : \"boolean\",\n" +
+                "        \"description\" : \"Whether specified concrete indices should be ignored when unavailable (missing or closed)\"" +
+                "      }" +
+                "    }," +
+                "    \"body\": null" +
+                "  }" +
+                "}", "Found duplicate part [index]");
+    }
+
+    public void testDuplicateParams() throws Exception {
+        parseAndExpectFailure("{\n" +
+                "  \"ping\": {" +
+                "    \"documentation\": \"http://www.elasticsearch.org/guide/\"," +
+                "    \"methods\": [\"PUT\"]," +
+                "    \"url\": {" +
+                "      \"path\": \"/\"," +
+                "      \"paths\": [\"/\"]," +
+                "      \"parts\": {" +
+                "      }," +
+                "      \"params\": {" +
+                "        \"timeout\": {" +
+                "          \"type\" : \"string\",\n" +
+                "          \"description\" : \"timeout parameter\"\n" +
+                "        }," +
+                "        \"refresh\": {" +
+                "          \"type\" : \"string\",\n" +
+                "          \"description\" : \"refresh parameter\"\n" +
+                "        }," +
+                "        \"timeout\": {" +
+                "          \"type\" : \"string\",\n" +
+                "          \"description\" : \"timeout parameter again\"\n" +
+                "        }" +
+                "      }" +
+                "    }," +
+                "    \"body\": null" +
+                "  }" +
+                "}", "Found duplicate param [timeout]");
+    }
+
     public void testBrokenSpecShouldThrowUsefulExceptionWhenParsingFailsOnParams() throws Exception {
         parseAndExpectFailure(BROKEN_SPEC_PARAMS, "Expected params field in rest api definition to contain an object");
     }
@@ -42,12 +145,10 @@ public class ClientYamlSuiteRestApiParserFailingTests extends ESTestCase {
 
     private void parseAndExpectFailure(String brokenJson, String expectedErrorMessage) throws Exception {
         XContentParser parser = JsonXContent.jsonXContent.createParser(brokenJson);
-        try {
-            new ClientYamlSuiteRestApiParser().parse("location", parser);
-            fail("Expected to fail parsing but did not happen");
-        } catch (IOException e) {
-            assertThat(e.getMessage(), containsString(expectedErrorMessage));
-        }
+        ClientYamlSuiteRestApiParser restApiParser = new ClientYamlSuiteRestApiParser();
+
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> restApiParser.parse("location", parser));
+        assertThat(e.getMessage(), containsString(expectedErrorMessage));
     }
 
     // see params section is broken, an inside param is missing