SONAR-6916 Enforce some mandatory web service metadata

1 files changed, 27 insertions, 18 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 045d0db25b5..6141bed8d8f 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
@@ -43,8 +43,13 @@ import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
 import org.sonar.api.ExtensionPoint;
 import org.sonar.api.server.ServerSide;
+import org.sonar.api.utils.log.Logger;
+import org.sonar.api.utils.log.Loggers;
 
 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}
@@ -131,7 +136,7 @@ public interface WebService extends Definable<WebService.Context> {
     private void register(NewController newController) {
       if (controllers.containsKey(newController.path)) {
         throw new IllegalStateException(
-          String.format("The web service '%s' is defined multiple times", newController.path));
+          format("The web service '%s' is defined multiple times", newController.path));
       }
       controllers.put(newController.path, new Controller(newController));
     }
@@ -191,7 +196,7 @@ public interface WebService extends Definable<WebService.Context> {
     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));
+          format("The action '%s' is defined multiple times in the web service '%s'", actionKey, path));
       }
       NewAction action = new NewAction(actionKey);
       actions.put(actionKey, action);
@@ -207,10 +212,7 @@ public interface WebService extends Definable<WebService.Context> {
     private final Map<String, Action> actions;
 
     private Controller(NewController newController) {
-      if (newController.actions.isEmpty()) {
-        throw new IllegalStateException(
-          String.format("At least one action must be declared in the web service '%s'", newController.path));
-      }
+      checkState(!newController.actions.isEmpty(), 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,10 +329,8 @@ public interface WebService extends Definable<WebService.Context> {
     }
 
     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));
-      }
+      checkState(!newParams.containsKey(paramKey),
+        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;
@@ -381,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 = String.format("Limit search to %s that contain the supplied string.", Joiner.on(" or ").join(pluralFields));
+      String actionDescription = format("Limit search to %s that contain the supplied string.", Joiner.on(" or ").join(pluralFields));
       createParam(Param.TEXT_QUERY)
         .setDescription(actionDescription)
         .setExampleValue(exampleValue);
@@ -420,6 +420,8 @@ public interface WebService extends Definable<WebService.Context> {
 
   @Immutable
   class Action {
+    private static final Logger LOGGER = Loggers.get(Action.class);
+
     private final String key;
     private final String deprecatedKey;
     private final String path;
@@ -435,19 +437,20 @@ public interface WebService extends Definable<WebService.Context> {
     private Action(Controller controller, NewAction newAction) {
       this.key = newAction.key;
       this.deprecatedKey = newAction.deprecatedKey;
-      this.path = String.format("%s/%s", controller.path(), key);
+      this.path = format("%s/%s", controller.path(), key);
       this.description = newAction.description;
-      this.since = StringUtils.defaultIfBlank(newAction.since, controller.since);
+      this.since = newAction.since;
       this.deprecatedSince = newAction.deprecatedSince;
       this.post = newAction.post;
       this.isInternal = newAction.isInternal;
       this.responseExample = newAction.responseExample;
-
-      if (newAction.handler == null) {
-        throw new IllegalArgumentException("RequestHandler is not set on action " + path);
-      }
       this.handler = newAction.handler;
 
+      checkState(this.handler != null, "RequestHandler is not set on action " + path);
+      logWarningIf(isNullOrEmpty(this.description), "Description is not set on action " + path);
+      logWarningIf(isNullOrEmpty(this.since), "Since is not set on action " + path);
+      logWarningIf(!this.post && this.responseExample == null, "The response example is not set on action " + path);
+
       ImmutableMap.Builder<String, Param> paramsBuilder = ImmutableMap.builder();
       for (NewParam newParam : newAction.newParams.values()) {
         paramsBuilder.put(newParam.key, new Param(this, newParam));
@@ -455,6 +458,12 @@ public interface WebService extends Definable<WebService.Context> {
       this.params = paramsBuilder.build();
     }
 
+    private static void logWarningIf(boolean condition, String message) {
+      if (condition) {
+        LOGGER.warn(message);
+      }
+    }
+
     public String key() {
       return key;
     }
@@ -713,7 +722,7 @@ public interface WebService extends Definable<WebService.Context> {
       this.required = newParam.required;
       this.possibleValues = newParam.possibleValues;
       if (required && defaultValue != null) {
-        throw new IllegalArgumentException(String.format("Default value must not be set on parameter '%s?%s' as it's marked as required", action, key));
+        throw new IllegalArgumentException(format("Default value must not be set on parameter '%s?%s' as it's marked as required", action, key));
       }
     }