]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-5010 improve WS API
authorSimon Brandhof <simon.brandhof@gmail.com>
Fri, 24 Jan 2014 09:01:44 +0000 (10:01 +0100)
committerSimon Brandhof <simon.brandhof@gmail.com>
Fri, 24 Jan 2014 09:26:00 +0000 (10:26 +0100)
* remove "throws Exception" from signature of RequestHandler
* add Request#action()
* prefer action "list" over "index"

19 files changed:
sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java
sonar-plugin-api/src/main/java/org/sonar/api/server/ws/RequestHandler.java
sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java
sonar-server/src/main/java/org/sonar/server/issue/filter/IssueFilterWs.java
sonar-server/src/main/java/org/sonar/server/rule/ws/RuleTagsWs.java
sonar-server/src/main/java/org/sonar/server/rule/ws/RulesWs.java
sonar-server/src/main/java/org/sonar/server/rule/ws/package-info.java [new file with mode: 0644]
sonar-server/src/main/java/org/sonar/server/ws/InternalRequest.java [new file with mode: 0644]
sonar-server/src/main/java/org/sonar/server/ws/ListingWs.java
sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java
sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java
sonar-server/src/test/java/org/sonar/server/rule/ws/RulesWsTest.java
sonar-server/src/test/java/org/sonar/server/ws/ListingWsTest.java
sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java
sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWsTest/list.json [new file with mode: 0644]
sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWsTest/search.json [deleted file]
sonar-server/src/test/resources/org/sonar/server/ws/ListingWsTest/index.json [deleted file]
sonar-server/src/test/resources/org/sonar/server/ws/ListingWsTest/list.json [new file with mode: 0644]
sonar-testing-harness/src/main/java/org/sonar/api/server/ws/WsTester.java

index 3892b275ebc5a151f6239d1497df3c3a04dbe328..d033fe7bb073be24c90d2c72b0d07b8d506646f5 100644 (file)
@@ -28,6 +28,8 @@ import javax.annotation.CheckForNull;
  */
 public abstract class Request {
 
+  public abstract WebService.Action action();
+
   /**
    * Returns the name of the HTTP method with which this request was made. Possible
    * values are GET and POST. Others are not supported.
index 48f76926b8118a7c8cdac10a22328882deb48c4b..25de77a862ee2c91893537bdf7a7a272b54f0e07 100644 (file)
@@ -26,6 +26,6 @@ import org.sonar.api.ServerExtension;
  */
 public interface RequestHandler extends ServerExtension {
 
-  void handle(Request request, Response response) throws Exception;
+  void handle(Request request, Response response);
 
 }
index f7f4f8f738d0d82897adb4eb31cb2e209ca6071e..944412644f532bbe75f5f5bd85925b522e8ec5b7 100644 (file)
@@ -27,12 +27,23 @@ import java.util.Map;
 
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
+import static org.mockito.Mockito.mock;
 
 public class RequestTest {
 
   static class SimpleRequest extends Request {
 
     private final Map<String, String> params = new HashMap<String, String>();
+    private final WebService.Action action;
+
+    private SimpleRequest(WebService.Action action) {
+      this.action = action;
+    }
+
+    @Override
+    public WebService.Action action() {
+      return action;
+    }
 
     @Override
     public String method() {
@@ -54,7 +65,7 @@ public class RequestTest {
   }
 
 
-  SimpleRequest request = new SimpleRequest();
+  SimpleRequest request = new SimpleRequest(mock(WebService.Action.class));
 
   @Test
   public void required_param_is_missing() throws Exception {
index 4326cc4319bde220fd74cabd73cc81393bcf86fe..c6770b43e94f5048b9d4d310ede9beb1331f49c1 100644 (file)
@@ -49,7 +49,7 @@ public class IssueFilterWs implements WebService {
       .setPrivate(true)
       .setHandler(new RequestHandler() {
         @Override
-        public void handle(Request request, Response response) throws Exception {
+        public void handle(Request request, Response response) {
           page(request, response);
         }
       });
index d81d7980c3160453f047524ca3457947746f1098..de1dc6047d7908122dbb58e371ee3e7c3ecc8e27 100644 (file)
@@ -45,7 +45,7 @@ public class RuleTagsWs implements WebService {
       .setSince("4.2")
       .setHandler(new RequestHandler() {
         @Override
-        public void handle(Request request, Response response) throws Exception {
+        public void handle(Request request, Response response) {
           list(request, response);
         }
       });
@@ -56,7 +56,7 @@ public class RuleTagsWs implements WebService {
       .setSince("4.2")
       .setHandler(new RequestHandler() {
         @Override
-        public void handle(Request request, Response response) throws Exception {
+        public void handle(Request request, Response response) {
           create(request, response);
         }
       })
index 96f793906d7f6ae72ee5b4ffc745f73aa7fe2422..31066a1c6fa7a55d03c53b1d4d038ed197c14388 100644 (file)
@@ -31,22 +31,21 @@ public class RulesWs implements WebService {
     NewController controller = context.newController("api/rules")
       .setDescription("Coding rules");
 
-    controller.newAction("search")
-      .setDescription("Search for coding rules")
+    controller.newAction("list")
+      .setDescription("List coding rules")
       .setSince("4.2")
       .setHandler(new RequestHandler() {
         @Override
-        public void handle(Request request, Response response) throws Exception {
-          search(request, response);
+        public void handle(Request request, Response response) {
+          list(request, response);
         }
       });
 
     controller.done();
   }
 
-  void search(Request request, Response response) {
+  void list(Request request, Response response) {
     response.newJsonWriter().beginObject()
-      // TODO
       .prop("TODO", true)
       .endObject()
       .close();
diff --git a/sonar-server/src/main/java/org/sonar/server/rule/ws/package-info.java b/sonar-server/src/main/java/org/sonar/server/rule/ws/package-info.java
new file mode 100644 (file)
index 0000000..109c00e
--- /dev/null
@@ -0,0 +1,23 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2013 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * SonarQube is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * SonarQube is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+@ParametersAreNonnullByDefault
+package org.sonar.server.rule.ws;
+
+import javax.annotation.ParametersAreNonnullByDefault;
diff --git a/sonar-server/src/main/java/org/sonar/server/ws/InternalRequest.java b/sonar-server/src/main/java/org/sonar/server/ws/InternalRequest.java
new file mode 100644 (file)
index 0000000..3ce2119
--- /dev/null
@@ -0,0 +1,36 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2013 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * SonarQube is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * SonarQube is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.server.ws;
+
+import org.sonar.api.server.ws.Request;
+import org.sonar.api.server.ws.WebService;
+
+public abstract class InternalRequest extends Request {
+  private WebService.Action action;
+
+  @Override
+  public WebService.Action action() {
+    return action;
+  }
+
+  void setAction(WebService.Action action) {
+    this.action = action;
+  }
+}
index 3176315cd666246bf31542477300e0135b939c25..ba1b0d47ee6a3128679bddf016990db3ab65bd1a 100644 (file)
@@ -42,7 +42,7 @@ public class ListingWs implements WebService {
     NewController controller = context.newController("api/webservices")
       .setDescription("List web services")
       .setSince("4.2");
-    controller.newAction("index")
+    controller.newAction("list")
       .setHandler(new RequestHandler() {
         @Override
         public void handle(Request request, Response response) {
index 4e425c745829f9aacf6559520f00ad152c13fa4c..cd3d5d8b7a5f97bca329b383142a74aaa54f8545 100644 (file)
  */
 package org.sonar.server.ws;
 
-import org.sonar.api.server.ws.Request;
-
 import javax.servlet.http.HttpServletRequest;
 
-public class ServletRequest extends Request {
+public class ServletRequest extends InternalRequest {
 
   private final HttpServletRequest source;
 
index 83ba59bdcf105ea021ae84a7243c7e817f03a6c4..76c14ae59438f4459479820b4ec035ceb729b277 100644 (file)
@@ -65,10 +65,11 @@ public class WebServiceEngine implements ServerComponent, Startable {
     return context.controllers();
   }
 
-  public void execute(Request request, Response response,
+  public void execute(InternalRequest request, Response response,
                       String controllerPath, String actionKey) {
     try {
       WebService.Action action = getAction(controllerPath, actionKey);
+      request.setAction(action);
       verifyRequest(action, request);
       action.handler().handle(request, response);
 
index e9aac3290434fce735f755b7fe27bd3df267c923..5902da2f7791a7320b0091b4ca1db04d0c83d1eb 100644 (file)
@@ -35,17 +35,18 @@ public class RulesWsTest {
     assertThat(controller).isNotNull();
     assertThat(controller.path()).isEqualTo("api/rules");
     assertThat(controller.description()).isNotEmpty();
+    assertThat(controller.actions()).hasSize(1);
 
-    WebService.Action search = controller.action("search");
+    WebService.Action search = controller.action("list");
     assertThat(search).isNotNull();
-    assertThat(search.key()).isEqualTo("search");
     assertThat(search.handler()).isNotNull();
     assertThat(search.since()).isEqualTo("4.2");
     assertThat(search.isPost()).isFalse();
+    assertThat(search.isPrivate()).isFalse();
   }
 
   @Test
   public void search_for_rules() throws Exception {
-    tester.newRequest("search").execute().assertJson(getClass(), "search.json");
+    tester.newRequest("list").execute().assertJson(getClass(), "list.json");
   }
 }
index 0e73f8562020f48331b8768b642d8c1ed9fd64c3..071c50bcba4fc1d47d21047186a16861d536551c 100644 (file)
@@ -38,9 +38,9 @@ public class ListingWsTest {
     assertThat(controller.since()).isEqualTo("4.2");
     assertThat(controller.actions()).hasSize(1);
 
-    WebService.Action index = controller.action("index");
+    WebService.Action index = controller.action("list");
     assertThat(index).isNotNull();
-    assertThat(index.key()).isEqualTo("index");
+    assertThat(index.key()).isEqualTo("list");
     assertThat(index.handler()).isNotNull();
     assertThat(index.since()).isEqualTo("4.2");
     assertThat(index.isPost()).isFalse();
@@ -50,7 +50,7 @@ public class ListingWsTest {
   @Test
   public void index() throws Exception {
     WsTester tester = new WsTester(ws, new MetricWebService());
-    tester.newRequest("api/webservices", "index").execute().assertJson(getClass(), "index.json");
+    tester.newRequest("api/webservices", "list").execute().assertJson(getClass(), "list.json");
   }
 
   static class MetricWebService implements WebService {
index a5aff474302cabc3e1a26442fe4e39e7672925bc..1511490a470a6dd8b141da6695530cde875fb1aa 100644 (file)
@@ -33,17 +33,17 @@ import org.sonar.api.utils.text.XmlWriter;
 
 import javax.annotation.CheckForNull;
 import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.util.HashMap;
 import java.util.Map;
 
 import static org.fest.assertions.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
 
 public class WebServiceEngineTest {
 
-  private static class SimpleRequest extends Request {
+  private static class SimpleRequest extends InternalRequest {
     private String method = "GET";
     private Map<String, String> params = new HashMap<String, String>();
 
@@ -152,7 +152,7 @@ public class WebServiceEngineTest {
 
   @Test
   public void execute_request() throws Exception {
-    Request request = new SimpleRequest();
+    InternalRequest request = new SimpleRequest();
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "health");
 
@@ -162,7 +162,7 @@ public class WebServiceEngineTest {
 
   @Test
   public void bad_controller() throws Exception {
-    Request request = new SimpleRequest();
+    InternalRequest request = new SimpleRequest();
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/xxx", "health");
 
@@ -172,7 +172,7 @@ public class WebServiceEngineTest {
 
   @Test
   public void bad_action() throws Exception {
-    Request request = new SimpleRequest();
+    InternalRequest request = new SimpleRequest();
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "xxx");
 
@@ -182,7 +182,7 @@ public class WebServiceEngineTest {
 
   @Test
   public void method_get_not_allowed() throws Exception {
-    Request request = new SimpleRequest();
+    InternalRequest request = new SimpleRequest();
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "ping");
 
@@ -192,7 +192,7 @@ public class WebServiceEngineTest {
 
   @Test
   public void method_post_required() throws Exception {
-    Request request = new SimpleRequest().setMethod("POST");
+    InternalRequest request = new SimpleRequest().setMethod("POST");
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "ping");
 
@@ -202,7 +202,7 @@ public class WebServiceEngineTest {
 
   @Test
   public void required_parameter_is_not_set() throws Exception {
-    Request request = new SimpleRequest();
+    InternalRequest request = new SimpleRequest();
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "print");
 
@@ -212,7 +212,7 @@ public class WebServiceEngineTest {
 
   @Test
   public void optional_parameter_is_not_set() throws Exception {
-    Request request = new SimpleRequest().setParam("message", "Hello World");
+    InternalRequest request = new SimpleRequest().setParam("message", "Hello World");
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "print");
 
@@ -222,7 +222,7 @@ public class WebServiceEngineTest {
 
   @Test
   public void optional_parameter_is_set() throws Exception {
-    Request request = new SimpleRequest()
+    InternalRequest request = new SimpleRequest()
       .setParam("message", "Hello World")
       .setParam("author", "Marcel");
     SimpleResponse response = new SimpleResponse();
@@ -234,7 +234,7 @@ public class WebServiceEngineTest {
 
   @Test
   public void internal_error() throws Exception {
-    Request request = new SimpleRequest();
+    InternalRequest request = new SimpleRequest();
     SimpleResponse response = new SimpleResponse();
     engine.execute(request, response, "api/system", "fail");
 
@@ -249,22 +249,30 @@ public class WebServiceEngineTest {
       newController.newAction("health")
         .setHandler(new RequestHandler() {
           @Override
-          public void handle(Request request, Response response) throws Exception {
-            response.stream().output().write("good".getBytes());
+          public void handle(Request request, Response response) {
+            try {
+              response.stream().output().write("good".getBytes());
+            } catch (IOException e) {
+              throw new IllegalStateException(e);
+            }
           }
         });
       newController.newAction("ping")
         .setPost(true)
         .setHandler(new RequestHandler() {
           @Override
-          public void handle(Request request, Response response) throws Exception {
-            response.stream().output().write("pong".getBytes());
+          public void handle(Request request, Response response) {
+            try {
+              response.stream().output().write("pong".getBytes());
+            } catch (IOException e) {
+              throw new IllegalStateException(e);
+            }
           }
         });
       newController.newAction("fail")
         .setHandler(new RequestHandler() {
           @Override
-          public void handle(Request request, Response response) throws Exception {
+          public void handle(Request request, Response response) {
             throw new IllegalStateException("Unexpected");
           }
         });
@@ -275,9 +283,13 @@ public class WebServiceEngineTest {
         .newParam("author", "optional author")
         .setHandler(new RequestHandler() {
           @Override
-          public void handle(Request request, Response response) throws Exception {
-            IOUtils.write(
+          public void handle(Request request, Response response) {
+            try {
+              IOUtils.write(
               request.requiredParam("message") + " by " + request.param("author", "-"), response.stream().output());
+            } catch (IOException e) {
+              throw new IllegalStateException(e);
+            }
           }
         });
       newController.done();
diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWsTest/list.json b/sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWsTest/list.json
new file mode 100644 (file)
index 0000000..a3a00e8
--- /dev/null
@@ -0,0 +1,3 @@
+{
+  "TODO": true
+}
diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWsTest/search.json b/sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWsTest/search.json
deleted file mode 100644 (file)
index a3a00e8..0000000
+++ /dev/null
@@ -1,3 +0,0 @@
-{
-  "TODO": true
-}
diff --git a/sonar-server/src/test/resources/org/sonar/server/ws/ListingWsTest/index.json b/sonar-server/src/test/resources/org/sonar/server/ws/ListingWsTest/index.json
deleted file mode 100644 (file)
index 6ee68a2..0000000
+++ /dev/null
@@ -1,42 +0,0 @@
-{
-    "webServices": [
-        {
-            "path": "api/metric",
-            "since": "3.2",
-            "description": "Metrics",
-            "actions": [
-                {
-                    "key": "create",
-                    "description": "Create metric",
-                    "since": "4.1",
-                    "post": true,
-                    "params": [
-                        {
-                            "key": "key",
-                            "description": "Key of new metric"
-                        },
-                        {
-                            "key": "name"
-                        }
-                    ]
-                },
-                {
-                    "key": "show",
-                    "since": "3.2",
-                    "post": false
-                }
-            ]
-        },
-        {
-            "path": "api/webservices",
-            "since": "4.2",
-            "description": "List web services",
-            "actions": [
-                {
-                    "key": "index",
-                    "since": "4.2",
-                    "post": false
-                }
-            ]
-        }
-    ]}
diff --git a/sonar-server/src/test/resources/org/sonar/server/ws/ListingWsTest/list.json b/sonar-server/src/test/resources/org/sonar/server/ws/ListingWsTest/list.json
new file mode 100644 (file)
index 0000000..8d8ec4a
--- /dev/null
@@ -0,0 +1,42 @@
+{
+    "webServices": [
+        {
+            "path": "api/metric",
+            "since": "3.2",
+            "description": "Metrics",
+            "actions": [
+                {
+                    "key": "create",
+                    "description": "Create metric",
+                    "since": "4.1",
+                    "post": true,
+                    "params": [
+                        {
+                            "key": "key",
+                            "description": "Key of new metric"
+                        },
+                        {
+                            "key": "name"
+                        }
+                    ]
+                },
+                {
+                    "key": "show",
+                    "since": "3.2",
+                    "post": false
+                }
+            ]
+        },
+        {
+            "path": "api/webservices",
+            "since": "4.2",
+            "description": "List web services",
+            "actions": [
+                {
+                    "key": "list",
+                    "since": "4.2",
+                    "post": false
+                }
+            ]
+        }
+    ]}
index aad0844ea5c888c4fa46cff7b7af583d57824da2..6ae58c338cb39972508515d9b964cd78c95983f0 100644 (file)
@@ -43,13 +43,18 @@ public class WsTester {
   public static class TestRequest extends Request {
 
     private final WebService.Controller controller;
-    private final String actionKey;
+    private final WebService.Action action;
     private String method = "GET";
     private Map<String, String> params = new HashMap<String, String>();
 
-    private TestRequest(WebService.Controller controller, String actionKey) {
+    private TestRequest(WebService.Controller controller, WebService.Action action) {
       this.controller = controller;
-      this.actionKey = actionKey;
+      this.action = action;
+    }
+
+    @Override
+    public WebService.Action action() {
+      return action;
     }
 
     @Override
@@ -81,10 +86,6 @@ public class WsTester {
     }
 
     public Result execute() throws Exception {
-      WebService.Action action = controller.action(actionKey);
-      if (action == null) {
-        throw new IllegalArgumentException("Action not found: " + actionKey);
-      }
       TestResponse response = new TestResponse();
       action.handler().handle(this, response);
       return new Result(response);
@@ -207,10 +208,17 @@ public class WsTester {
     if (context.controllers().size() != 1) {
       throw new IllegalStateException("The method newRequest(String) requires to define one, and only one, controller");
     }
-    return new TestRequest(context.controllers().get(0), actionKey);
+    WebService.Controller controller = context.controllers().get(0);
+    WebService.Action action = controller.action(actionKey);
+    if (action == null) {
+      throw new IllegalArgumentException("Action not found: " + actionKey);
+    }
+    return new TestRequest(controller, action);
   }
 
   public TestRequest newRequest(String controllerPath, String actionKey) {
-    return new TestRequest(context.controller(controllerPath), actionKey);
+    WebService.Controller controller = context.controller(controllerPath);
+    WebService.Action action = controller.action(actionKey);
+    return new TestRequest(controller, action);
   }
 }