Revert "SONAR-6916 Enforce some mandatory web service metadata"

This reverts commit 3608f17625b2c54882ebef237b0bc9525eda7e19.

2 files changed, 191 insertions, 281 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 2d2c8ca5035..045d0db25b5 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
@@ -45,9 +45,6 @@ import org.sonar.api.ExtensionPoint;
 import org.sonar.api.server.ServerSide;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
-import static com.google.common.base.Strings.isNullOrEmpty;
-import static java.lang.String.format;
 
 /**
  * Defines a web service. Note that contrary to the deprecated {@link org.sonar.api.web.Webservice}
@@ -134,7 +131,7 @@ public interface WebService extends Definable<WebService.Context> {
     private void register(NewController newController) {
       if (controllers.containsKey(newController.path)) {
         throw new IllegalStateException(
-          format("The web service '%s' is defined multiple times", newController.path));
+          String.format("The web service '%s' is defined multiple times", newController.path));
       }
       controllers.put(newController.path, new Controller(newController));
     }
@@ -194,7 +191,7 @@ public interface WebService extends Definable<WebService.Context> {
     public NewAction createAction(String actionKey) {
       if (actions.containsKey(actionKey)) {
         throw new IllegalStateException(
-          format("The action '%s' is defined multiple times in the web service '%s'", actionKey, path));
+          String.format("The action '%s' is defined multiple times in the web service '%s'", actionKey, path));
       }
       NewAction action = new NewAction(actionKey);
       actions.put(actionKey, action);
@@ -210,7 +207,10 @@ public interface WebService extends Definable<WebService.Context> {
     private final Map<String, Action> actions;
 
     private Controller(NewController newController) {
-      checkState(!newController.actions.isEmpty(), format("At least one action must be declared in the web service '%s'", newController.path));
+      if (newController.actions.isEmpty()) {
+        throw new IllegalStateException(
+          String.format("At least one action must be declared in the web service '%s'", newController.path));
+      }
       this.path = newController.path;
       this.description = newController.description;
       this.since = newController.since;
@@ -327,8 +327,10 @@ public interface WebService extends Definable<WebService.Context> {
     }
 
     public NewParam createParam(String paramKey) {
-      checkState(!newParams.containsKey(paramKey),
-        format("The parameter '%s' is defined multiple times in the action '%s'", paramKey, key));
+      if (newParams.containsKey(paramKey)) {
+        throw new IllegalStateException(
+          String.format("The parameter '%s' is defined multiple times in the action '%s'", paramKey, key));
+      }
       NewParam newParam = new NewParam(paramKey);
       newParams.put(paramKey, newParam);
       return newParam;
@@ -379,7 +381,7 @@ public interface WebService extends Definable<WebService.Context> {
      * The fields must be in the <strong>plural</strong> form (ex: "names", "keys")
      */
     public NewAction addSearchQuery(String exampleValue, String... pluralFields) {
-      String actionDescription = format("Limit search to %s that contain the supplied string.", Joiner.on(" or ").join(pluralFields));
+      String actionDescription = String.format("Limit search to %s that contain the supplied string.", Joiner.on(" or ").join(pluralFields));
       createParam(Param.TEXT_QUERY)
         .setDescription(actionDescription)
         .setExampleValue(exampleValue);
@@ -433,19 +435,18 @@ public interface WebService extends Definable<WebService.Context> {
     private Action(Controller controller, NewAction newAction) {
       this.key = newAction.key;
       this.deprecatedKey = newAction.deprecatedKey;
-      this.path = format("%s/%s", controller.path(), key);
+      this.path = String.format("%s/%s", controller.path(), key);
       this.description = newAction.description;
-      this.since = newAction.since;
+      this.since = StringUtils.defaultIfBlank(newAction.since, controller.since);
       this.deprecatedSince = newAction.deprecatedSince;
       this.post = newAction.post;
       this.isInternal = newAction.isInternal;
       this.responseExample = newAction.responseExample;
-      this.handler = newAction.handler;
 
-      checkState(this.handler != null, "RequestHandler is not set on action " + path);
-      checkState(!isNullOrEmpty(this.description), "Description is not set on action " + path);
-      checkState(!isNullOrEmpty(this.since), "Since is not set on action " + path);
-      checkState(this.post || this.responseExample != null, "The response example is not set on action " + path);
+      if (newAction.handler == null) {
+        throw new IllegalArgumentException("RequestHandler is not set on action " + path);
+      }
+      this.handler = newAction.handler;
 
       ImmutableMap.Builder<String, Param> paramsBuilder = ImmutableMap.builder();
       for (NewParam newParam : newAction.newParams.values()) {
@@ -712,7 +713,7 @@ public interface WebService extends Definable<WebService.Context> {
       this.required = newParam.required;
       this.possibleValues = newParam.possibleValues;
       if (required && defaultValue != null) {
-        throw new IllegalArgumentException(format("Default value must not be set on parameter '%s?%s' as it's marked as required", action, key));
+        throw new IllegalArgumentException(String.format("Default value must not be set on parameter '%s?%s' as it's marked as required", action, key));
       }
     }
 
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 b762d1d373b..bdca29a4702 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
@@ -24,12 +24,8 @@ import java.net.URL;
 import java.util.Arrays;
 import java.util.Collections;
 import org.apache.commons.lang.StringUtils;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.sonar.api.rule.RuleStatus;
-import org.sonar.api.server.ws.WebService.NewAction;
-import org.sonar.api.server.ws.WebService.NewController;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
@@ -37,10 +33,51 @@ import static org.mockito.Mockito.mock;
 
 public class WebServiceTest {
 
-  @Rule
-  public ExpectedException expectedException = ExpectedException.none();
+  static class MetricWs implements WebService {
+    boolean showCalled = false;
+    boolean createCalled = false;
+
+    @Override
+    public void define(Context context) {
+      NewController newController = context.createController("api/metric")
+        .setDescription("Metrics")
+        .setSince("3.2");
+
+      newController.createAction("show")
+        .setDescription("Show metric")
+        .setHandler(new RequestHandler() {
+          @Override
+          public void handle(Request request, Response response) {
+            show(request, response);
+          }
+        });
+
+      newController.createAction("create")
+        .setDescription("Create metric")
+        .setSince("4.1")
+        .setDeprecatedSince("5.3")
+        .setPost(true)
+        .setInternal(true)
+        .setResponseExample(getClass().getResource("/org/sonar/api/server/ws/WebServiceTest/response-example.txt"))
+        .setHandler(new RequestHandler() {
+          @Override
+          public void handle(Request request, Response response) {
+            create(request, response);
+          }
+        });
+
+      newController.done();
+    }
+
+    void show(Request request, Response response) {
+      showCalled = true;
+    }
+
+    void create(Request request, Response response) {
+      createCalled = true;
+    }
+  }
 
-  
   WebService.Context context = new WebService.Context();
 
   @Test
@@ -67,12 +104,12 @@ public class WebServiceTest {
     assertThat(showAction.key()).isEqualTo("show");
     assertThat(showAction.description()).isEqualTo("Show metric");
     assertThat(showAction.handler()).isNotNull();
-    assertThat(showAction.responseExample()).isNotNull();
-    assertThat(showAction.responseExampleFormat()).isNotEmpty();
-    assertThat(showAction.responseExampleAsString()).isNotEmpty();
+    assertThat(showAction.responseExample()).isNull();
+    assertThat(showAction.responseExampleFormat()).isNull();
+    assertThat(showAction.responseExampleAsString()).isNull();
     assertThat(showAction.deprecatedSince()).isNull();
     // same as controller
-    assertThat(showAction.since()).isEqualTo("4.2");
+    assertThat(showAction.since()).isEqualTo("3.2");
     assertThat(showAction.isPost()).isFalse();
     assertThat(showAction.isInternal()).isFalse();
     assertThat(showAction.path()).isEqualTo("api/metric/show");
@@ -89,104 +126,117 @@ public class WebServiceTest {
 
   @Test
   public void fail_if_duplicated_ws_keys() {
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("The web service 'api/metric' is defined multiple times");
-
     MetricWs metricWs = new MetricWs();
     metricWs.define(context);
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        NewController newController = context.createController("api/metric");
-        newDefaultAction(newController, "delete");
-        newController.done();
-      }
-    }.define(context);
+    try {
+      new WebService() {
+        @Override
+        public void define(Context context) {
+          NewController newController = context.createController("api/metric");
+          newController.createAction("delete");
+          newController.done();
+        }
+      }.define(context);
+      fail();
+    } catch (IllegalStateException e) {
+      assertThat(e).hasMessage("The web service 'api/metric' is defined multiple times");
+    }
   }
 
   @Test
   public void fail_if_no_action_handler() {
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("RequestHandler is not set on action rule/show");
-
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        NewController controller = context.createController("rule");
-        newDefaultAction(controller, "show")
-          .setHandler(null);
-        controller.done();
-      }
-    }.define(context);
+    try {
+      new WebService() {
+        @Override
+        public void define(Context context) {
+          NewController controller = context.createController("rule");
+          controller.createAction("show");
+          controller.done();
+        }
+      }.define(context);
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("RequestHandler is not set on action rule/show");
+    }
   }
 
   @Test
   public void fail_if_duplicated_action_keys() {
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("The action 'delete' is defined multiple times in the web service 'rule'");
-
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        NewController newController = context.createController("rule");
-        newDefaultAction(newController, "create");
-        newDefaultAction(newController, "delete");
-        newDefaultAction(newController, "delete");
-        newController.done();
-      }
-    }.define(context);
+    try {
+      new WebService() {
+        @Override
+        public void define(Context context) {
+          NewController newController = context.createController("rule");
+          newController.createAction("create");
+          newController.createAction("delete");
+          newController.createAction("delete");
+          newController.done();
+        }
+      }.define(context);
+      fail();
+    } catch (IllegalStateException e) {
+      assertThat(e).hasMessage("The action 'delete' is defined multiple times in the web service 'rule'");
+    }
   }
 
   @Test
   public void fail_if_no_actions() {
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("At least one action must be declared in the web service 'rule'");
-
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        context.createController("rule").done();
-      }
-    }.define(context);
+    try {
+      new WebService() {
+        @Override
+        public void define(Context context) {
+          context.createController("rule").done();
+        }
+      }.define(context);
+      fail();
+    } catch (IllegalStateException e) {
+      assertThat(e).hasMessage("At least one action must be declared in the web service 'rule'");
+    }
   }
 
   @Test
   public void fail_if_no_controller_path() {
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("WS controller path must not be empty");
-
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        context.createController(null).done();
-      }
-    }.define(context);
+    try {
+      new WebService() {
+        @Override
+        public void define(Context context) {
+          context.createController(null).done();
+        }
+      }.define(context);
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("WS controller path must not be empty");
+    }
   }
 
   @Test
   public void controller_path_must_not_start_with_slash() {
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("WS controller path must not start or end with slash: /hello");
-
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        context.createController("/hello").done();
-      }
-    }.define(context);
+    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() {
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("WS controller path must not start or end with slash: hello/");
-
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        context.createController("hello/").done();
-      }
-    }.define(context);
+    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
@@ -209,7 +259,7 @@ public class WebServiceTest {
       @Override
       public void define(Context context) {
         NewController newController = context.createController("api/rule");
-        NewAction newAction = newDefaultAction(newController, "create");
+        NewAction newAction = newController.createAction("create").setHandler(mock(RequestHandler.class));
         newAction
           .createParam("key")
           .setDescription("Key of the new rule");
@@ -261,7 +311,7 @@ public class WebServiceTest {
       @Override
       public void define(Context context) {
         NewController newController = context.createController("api/rule");
-        NewAction create = newDefaultAction(newController, "create");
+        NewAction create = newController.createAction("create").setHandler(mock(RequestHandler.class));
         create.createParam("status")
           .setDefaultValue(RuleStatus.BETA)
           .setPossibleValues(RuleStatus.BETA, RuleStatus.READY)
@@ -289,7 +339,7 @@ public class WebServiceTest {
       @Override
       public void define(Context context) {
         NewController newController = context.createController("api/rule");
-        NewAction create = newDefaultAction(newController, "create");
+        NewAction create = newController.createAction("create").setHandler(mock(RequestHandler.class));
         create.createParam("status")
           .setDefaultValue(null)
           .setPossibleValues(Collections.emptyList())
@@ -313,7 +363,7 @@ public class WebServiceTest {
       @Override
       public void define(Context context) {
         NewController newController = context.createController("api/rule");
-        NewAction create = newDefaultAction(newController, "create");
+        NewAction create = newController.createAction("create").setHandler(mock(RequestHandler.class));
         create.createParam("status")
           .setPossibleValues(Collections.emptyList());
         newController.done();
@@ -327,34 +377,39 @@ public class WebServiceTest {
 
   @Test
   public void fail_if_required_param_has_default_value() {
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("Default value must not be set on parameter 'api/rule/create?key' as it's marked as required");
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        NewController controller = context.createController("api/rule");
-        NewAction action = newDefaultAction(controller, "create");
-        action.createParam("key").setRequired(true).setDefaultValue("abc");
-        controller.done();
-      }
-    }.define(context);
+    try {
+      new WebService() {
+        @Override
+        public void define(Context context) {
+          NewController controller = context.createController("api/rule");
+          NewAction action = controller.createAction("create").setHandler(mock(RequestHandler.class));
+          action.createParam("key").setRequired(true).setDefaultValue("abc");
+          controller.done();
+        }
+      }.define(context);
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("Default value must not be set on parameter 'api/rule/create?key' as it's marked as required");
+    }
   }
 
   @Test
   public void fail_if_duplicated_action_parameters() {
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("The parameter 'key' is defined multiple times in the action 'create'");
-
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        NewController controller = context.createController("api/rule");
-        NewAction action = newDefaultAction(controller, "create");
-        action.createParam("key");
-        action.createParam("key");
-        controller.done();
-      }
-    }.define(context);
+    try {
+      new WebService() {
+        @Override
+        public void define(Context context) {
+          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);
+      fail();
+    } catch (IllegalStateException e) {
+      assertThat(e).hasMessage("The parameter 'key' is defined multiple times in the action 'create'");
+    }
   }
 
   @Test
@@ -363,8 +418,8 @@ public class WebServiceTest {
       @Override
       public void define(Context context) {
         NewController newController = context.createController("api/rule");
-        newDefaultAction(newController, "create").setInternal(true);
-        newDefaultAction(newController, "update").setInternal(true);
+        newController.createAction("create").setInternal(true).setHandler(mock(RequestHandler.class));
+        newController.createAction("update").setInternal(true).setHandler(mock(RequestHandler.class));
         newController.done();
       }
     }.define(context);
@@ -390,8 +445,12 @@ public class WebServiceTest {
       public void define(Context context) {
         try {
           NewController controller = context.createController("foo");
-          newDefaultAction(controller, "bar").setResponseExample(new URL("file:/does/not/exist"));
+          controller
+            .createAction("bar")
+            .setHandler(mock(RequestHandler.class))
+            .setResponseExample(new URL("file:/does/not/exist"));
           controller.done();
+
         } catch (MalformedURLException e) {
           e.printStackTrace();
         }
@@ -407,154 +466,4 @@ public class WebServiceTest {
       assertThat(e).hasMessage("Fail to load file:/does/not/exist");
     }
   }
-
-  @Test
-  public void post_action_without_response_example() {
-    expectedException = ExpectedException.none();
-
-    WebService ws = new WebService() {
-      @Override
-      public void define(Context context) {
-        NewController newController = context.createController("api/rule");
-        newDefaultAction(newController, "list").setPost(true).setResponseExample(null);
-        newController.done();
-      }
-    };
-    ws.define(context);
-  }
-
-  @Test
-  public void fail_if_get_and_no_response_example() {
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("The response example is not set on action api/rule/list");
-
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        NewController newController = context.createController("api/rule");
-        newDefaultAction(newController, "list").setResponseExample(null);
-        newController.done();
-      }
-    }.define(context);
-  }
-
-  @Test
-  public void fail_if_since_on_an_action_is_empty() {
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("Since is not set on action api/rule/list");
-
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        NewController newController = context.createController("api/rule");
-        newDefaultAction(newController, "list")
-          .setSince("");
-        newController.done();
-      }
-    }.define(context);
-  }
-
-  @Test
-  public void fail_if_since_on_an_action_is_null() {
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("Since is not set on action api/rule/list");
-
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        NewController newController = context.createController("api/rule");
-        newDefaultAction(newController, "list")
-          .setSince(null);
-        newController.done();
-      }
-    }.define(context);
-  }
-
-  @Test
-  public void fail_if_action_description_is_empty() {
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("Description is not set on action api/rule/list");
-
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        NewController newController = context.createController("api/rule");
-        newDefaultAction(newController, "list")
-          .setDescription("");
-        newController.done();
-      }
-    }.define(context);
-  }
-
-
-  @Test
-  public void fail_if_action_description_is_null() {
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("Description is not set on action api/rule/list");
-
-    new WebService() {
-      @Override
-      public void define(Context context) {
-        NewController newController = context.createController("api/rule");
-        newDefaultAction(newController, "list")
-          .setDescription(null);
-        newController.done();
-      }
-    }.define(context);
-  }
-
-  static class MetricWs implements WebService {
-    boolean showCalled = false;
-    boolean createCalled = false;
-
-    @Override
-    public void define(Context context) {
-      NewController newController = context.createController("api/metric")
-        .setDescription("Metrics")
-        .setSince("3.2");
-
-      newController.createAction("show")
-        .setDescription("Show metric")
-        .setSince("4.2")
-        .setResponseExample(getClass().getResource("WebServiceTest/response-example.txt"))
-        .setHandler(new RequestHandler() {
-          @Override
-          public void handle(Request request, Response response) {
-            show(request, response);
-          }
-        });
-
-      newController.createAction("create")
-        .setDescription("Create metric")
-        .setSince("4.1")
-        .setDeprecatedSince("5.3")
-        .setPost(true)
-        .setInternal(true)
-        .setResponseExample(getClass().getResource("WebServiceTest/response-example.txt"))
-        .setHandler(new RequestHandler() {
-          @Override
-          public void handle(Request request, Response response) {
-            create(request, response);
-          }
-        });
-
-      newController.done();
-    }
-
-    void show(Request request, Response response) {
-      showCalled = true;
-    }
-
-    void create(Request request, Response response) {
-      createCalled = true;
-    }
-  }
-
-  private NewAction newDefaultAction(NewController controller, String actionKey) {
-    return controller.createAction(actionKey)
-      .setDescription("default description")
-      .setSince("5.3")
-      .setResponseExample(getClass().getResource("WebServiceTest/response-example.txt"))
-      .setHandler(mock(RequestHandler.class));
-  }
 }