SONAR-5010 improve WebService extension point

2 files changed, 138 insertions, 33 deletions

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java
index 1a18aa540dc..d23bdeb4c6e 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java
@@ -33,7 +33,62 @@ import java.util.List;
 import java.util.Map;
 
 /**
- * Defines a web service implemented in Java (no Ruby on Rails at all).
+ * Defines a web service. Note that contrary to the deprecated {@link org.sonar.api.web.Webservice}
+ * the ws is fully implemented in Java and does not require any Ruby on Rails code.
+ *
+ * <p/>
+ * The classes implementing this extension point must be declared in {@link org.sonar.api.SonarPlugin#getExtensions()}.
+ *
+ * <h2>How to use</h2>
+ * <pre>
+ * public class HelloWs implements WebService {
+ *   @Override
+ *   public void define(Context context) {
+ *     NewController controller = context.createController("api/hello");
+ *     controller.setDescription("Web service example");
+ *
+ *     // create the URL /api/hello/show
+ *     controller.createAction("show")
+ *       .setDescription("Entry point")
+ *       .setHandler(new RequestHandler() {
+ *         @Override
+ *         public void handle(Request request, Response response) {
+ *           // read request parameters and generates response output
+ *           response.newJsonWriter()
+ *             .prop("hello", request.mandatoryParam("key"))
+ *             .close();
+ *         }
+ *      })
+ *      .createParam("key", "Example key");
+ *
+ *    // important to apply changes
+ *    controller.done();
+ *   }
+ * }
+ * </pre>
+ * <h2>How to unit test</h2>
+ * <pre>
+ * public class HelloWsTest {
+ *   WebService ws = new HelloWs();
+ *
+ *   @Test
+ *   public void should_define_ws() throws Exception {
+ *     // WsTester is available in the Maven artifact org.codehaus.sonar:sonar-plugin-api
+ *     // with type "test-jar"
+ *     WsTester tester = new WsTester(ws);
+ *     WebService.Controller controller = tester.controller("api/hello");
+ *     assertThat(controller).isNotNull();
+ *     assertThat(controller.path()).isEqualTo("api/hello");
+ *     assertThat(controller.description()).isNotEmpty();
+ *     assertThat(controller.actions()).hasSize(1);
+ *
+ *     WebService.Action show = controller.action("show");
+ *     assertThat(show).isNotNull();
+ *     assertThat(show.key()).isEqualTo("show");
+ *     assertThat(index.handler()).isNotNull();
+ *   }
+ * }
+ * </pre>
  *
  * @since 4.2
  */
@@ -42,7 +97,14 @@ public interface WebService extends ServerExtension {
   class Context {
     private final Map<String, Controller> controllers = Maps.newHashMap();
 
-    public NewController newController(String path) {
+    /**
+     * Create a new controller.
+     * <p/>
+     * Structure of request URL is <code>http://&lt;server&gt;/&lt>controller path&gt;/&lt;action path&gt;?&lt;parameters&gt;</code>.
+     *
+     * @param path the controller path must not start or end with "/". It is recommended to start with "api/"
+     */
+    public NewController createController(String path) {
       return new NewController(this, path);
     }
 
@@ -73,27 +135,40 @@ public interface WebService extends ServerExtension {
 
     private NewController(Context context, String path) {
       if (StringUtils.isBlank(path)) {
-        throw new IllegalArgumentException("Web service path can't be empty");
+        throw new IllegalArgumentException("WS controller path must not be empty");
+      }
+      if (StringUtils.startsWith(path, "/") || StringUtils.endsWith(path, "/")) {
+        throw new IllegalArgumentException("WS controller path must not start or end with slash: " + path);
       }
       this.context = context;
       this.path = path;
     }
 
+    /**
+     * Important - this method must be called in order to apply changes and make the
+     * controller available in {@link org.sonar.api.server.ws.WebService.Context#controllers()}
+     */
     public void done() {
       context.register(this);
     }
 
+    /**
+     * Optional plain-text description
+     */
     public NewController setDescription(@Nullable String s) {
       this.description = s;
       return this;
     }
 
+    /**
+     * Optional version when the controller was created
+     */
     public NewController setSince(@Nullable String s) {
       this.since = s;
       return this;
     }
 
-    public NewAction newAction(String actionKey) {
+    public NewAction createAction(String actionKey) {
       if (actions.containsKey(actionKey)) {
         throw new IllegalStateException(
           String.format("The action '%s' is defined multiple times in the web service '%s'", actionKey, path)
@@ -201,7 +276,7 @@ public interface WebService extends ServerExtension {
       return this;
     }
 
-    public NewParam newParam(String paramKey) {
+    public NewParam createParam(String paramKey) {
       if (newParams.containsKey(paramKey)) {
         throw new IllegalStateException(
           String.format("The parameter '%s' is defined multiple times in the action '%s'", paramKey, key)
@@ -212,8 +287,8 @@ public interface WebService extends ServerExtension {
       return newParam;
     }
 
-    public NewAction newParam(String paramKey, @Nullable String description) {
-      newParam(paramKey).setDescription(description);
+    public NewAction createParam(String paramKey, @Nullable String description) {
+      createParam(paramKey).setDescription(description);
       return this;
     }
   }
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java
index cb5d617fd3d..5848ee53066 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java
@@ -32,11 +32,11 @@ public class WebServiceTest {
 
     @Override
     public void define(Context context) {
-      NewController newController = context.newController("api/metric")
+      NewController newController = context.createController("api/metric")
         .setDescription("Metrics")
         .setSince("3.2");
 
-      newController.newAction("show")
+      newController.createAction("show")
         .setDescription("Show metric")
         .setHandler(new RequestHandler() {
           @Override
@@ -45,7 +45,7 @@ public class WebServiceTest {
           }
         });
 
-      newController.newAction("create")
+      newController.createAction("create")
         .setDescription("Create metric")
         .setSince("4.1")
         .setPost(true)
@@ -119,8 +119,8 @@ public class WebServiceTest {
       new WebService() {
         @Override
         public void define(Context context) {
-          NewController newController = context.newController("api/metric");
-          newController.newAction("delete");
+          NewController newController = context.createController("api/metric");
+          newController.createAction("delete");
           newController.done();
         }
       }.define(context);
@@ -136,8 +136,8 @@ public class WebServiceTest {
       new WebService() {
         @Override
         public void define(Context context) {
-          NewController controller = context.newController("rule");
-          controller.newAction("show");
+          NewController controller = context.createController("rule");
+          controller.createAction("show");
           controller.done();
         }
       }.define(context);
@@ -153,10 +153,10 @@ public class WebServiceTest {
       new WebService() {
         @Override
         public void define(Context context) {
-          NewController newController = context.newController("rule");
-          newController.newAction("create");
-          newController.newAction("delete");
-          newController.newAction("delete");
+          NewController newController = context.createController("rule");
+          newController.createAction("create");
+          newController.createAction("delete");
+          newController.createAction("delete");
           newController.done();
         }
       }.define(context);
@@ -172,7 +172,7 @@ public class WebServiceTest {
       new WebService() {
         @Override
         public void define(Context context) {
-          context.newController("rule").done();
+          context.createController("rule").done();
         }
       }.define(context);
       fail();
@@ -182,17 +182,47 @@ public class WebServiceTest {
   }
 
   @Test
-  public void fail_if_no_ws_path() {
+  public void fail_if_no_controller_path() {
     try {
       new WebService() {
         @Override
         public void define(Context context) {
-          context.newController(null).done();
+          context.createController(null).done();
         }
       }.define(context);
       fail();
     } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("Web service path can't be empty");
+      assertThat(e).hasMessage("WS controller path must not be empty");
+    }
+  }
+
+  @Test
+  public void controller_path_must_not_start_with_slash() {
+    try {
+      new WebService() {
+        @Override
+        public void define(Context context) {
+          context.createController("/hello").done();
+        }
+      }.define(context);
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("WS controller path must not start or end with slash: /hello");
+    }
+  }
+
+  @Test
+  public void controller_path_must_not_end_with_slash() {
+    try {
+      new WebService() {
+        @Override
+        public void define(Context context) {
+          context.createController("hello/").done();
+        }
+      }.define(context);
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("WS controller path must not start or end with slash: hello/");
     }
   }
 
@@ -215,10 +245,10 @@ public class WebServiceTest {
     new WebService() {
       @Override
       public void define(Context context) {
-        NewController newController = context.newController("api/rule");
-        NewAction create = newController.newAction("create").setHandler(mock(RequestHandler.class));
-        create.newParam("key").setDescription("Key of the new rule");
-        create.newParam("severity");
+        NewController newController = context.createController("api/rule");
+        NewAction create = newController.createAction("create").setHandler(mock(RequestHandler.class));
+        create.createParam("key").setDescription("Key of the new rule");
+        create.createParam("severity");
         newController.done();
       }
     }.define(context);
@@ -240,10 +270,10 @@ public class WebServiceTest {
       new WebService() {
         @Override
         public void define(Context context) {
-          NewController controller = context.newController("api/rule");
-          NewAction action = controller.newAction("create").setHandler(mock(RequestHandler.class));
-          action.newParam("key");
-          action.newParam("key");
+          NewController controller = context.createController("api/rule");
+          NewAction action = controller.createAction("create").setHandler(mock(RequestHandler.class));
+          action.createParam("key");
+          action.createParam("key");
           controller.done();
         }
       }.define(context);
@@ -258,9 +288,9 @@ public class WebServiceTest {
     new WebService() {
       @Override
       public void define(Context context) {
-        NewController newController = context.newController("api/rule");
-        newController.newAction("create").setInternal(true).setHandler(mock(RequestHandler.class));
-        newController.newAction("update").setInternal(true).setHandler(mock(RequestHandler.class));
+        NewController newController = context.createController("api/rule");
+        newController.createAction("create").setInternal(true).setHandler(mock(RequestHandler.class));
+        newController.createAction("update").setInternal(true).setHandler(mock(RequestHandler.class));
         newController.done();
       }
     }.define(context);