From c669c3a34dfce5839f8d4f614165cd3ef0240727 Mon Sep 17 00:00:00 2001
From: Teryk Bellahsene <teryk.bellahsene@sonarsource.com>
Date: Tue, 18 Aug 2015 17:57:29 +0200
Subject: [PATCH] Add consistency in error messages of permissions WS

---
 .../java/org/sonar/server/component/ComponentFinder.java | 7 ++++++-
 .../org/sonar/server/permission/ws/AddGroupAction.java   | 4 ++--
 .../org/sonar/server/permission/ws/AddUserAction.java    | 4 ++--
 .../sonar/server/permission/ws/PermissionRequest.java    | 9 +++++----
 .../sonar/server/permission/ws/PermissionWsCommons.java  | 8 +-------
 .../sonar/server/permission/ws/RemoveGroupAction.java    | 6 +++---
 .../org/sonar/server/permission/ws/RemoveUserAction.java | 4 ++--
 .../permission/ws/SearchGlobalPermissionsAction.java     | 2 +-
 .../java/org/sonar/server/permission/ws/UsersAction.java | 2 +-
 .../org/sonar/server/plugins/ws/InstallActionTest.java   | 1 -
 .../org/sonar/server/plugins/ws/UninstallActionTest.java | 1 -
 .../org/sonar/server/plugins/ws/UpdateActionTest.java    | 1 -
 .../java/org/sonar/server/ws/WebServiceEngineTest.java   | 2 +-
 .../src/main/java/org/sonar/api/server/ws/Request.java   | 4 ++--
 .../test/java/org/sonar/api/server/ws/RequestTest.java   | 4 ++--
 15 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/server/sonar-server/src/main/java/org/sonar/server/component/ComponentFinder.java b/server/sonar-server/src/main/java/org/sonar/server/component/ComponentFinder.java
index aadbf41dfa7..19c594d2289 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/component/ComponentFinder.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/component/ComponentFinder.java
@@ -57,7 +57,12 @@ public class ComponentFinder {
   }
 
   public ComponentDto getProjectByUuidOrKey(DbSession dbSession, @Nullable String projectUuid, @Nullable String projectKey) {
-    ComponentDto project = getByUuidOrKey(dbSession, projectUuid, projectKey);
+    ComponentDto project;
+    if (projectUuid != null) {
+      project = getIfPresentOrFail(dbClient.componentDao().selectByUuid(dbSession, projectUuid), String.format("Project id '%s' not found", projectUuid));
+    } else {
+      project = getIfPresentOrFail(dbClient.componentDao().selectByKey(dbSession, projectKey), String.format("Project key '%s' not found", projectKey));
+    }
     checkIsProjectOrModule(project);
 
     return project;
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddGroupAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddGroupAction.java
index 6e7ec7c6afc..0d52a7e2e44 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddGroupAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddGroupAction.java
@@ -53,9 +53,9 @@ public class AddGroupAction implements PermissionsWsAction {
   public void define(WebService.NewController context) {
     WebService.NewAction action = context.createAction(ACTION)
       .setDescription("Add permission to a group.<br /> " +
-        "If the project id is provided, a project permission is created.<br />" +
+        "This service defaults to global permissions, but can be limited to project permissions by providing project id or project key.<br /> " +
         "The group name or group id must be provided. <br />" +
-        "Requires 'Administer System' permission.")
+        "It requires administration permissions to access.")
       .setSince("5.2")
       .setPost(true)
       .setHandler(this);
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddUserAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddUserAction.java
index 525bb3a98c5..56516f8cae9 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddUserAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddUserAction.java
@@ -52,8 +52,8 @@ public class AddUserAction implements PermissionsWsAction {
   public void define(WebService.NewController context) {
     WebService.NewAction action = context.createAction(ACTION)
       .setDescription("Add permission to a user.<br /> " +
-        "If the project id or project key is provided, a project permission is created.<br />" +
-        "Requires 'Administer System' permission.")
+        "This service defaults to global permissions, but can be limited to project permissions by providing project id or project key.<br />" +
+        "It requires administration permissions to access.")
       .setSince("5.2")
       .setPost(true)
       .setHandler(this);
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionRequest.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionRequest.java
index 43431e0be3d..96f3661ea20 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionRequest.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionRequest.java
@@ -175,12 +175,13 @@ public class PermissionRequest {
     private void checkPermissionParameter() {
       if (_hasProject) {
         if (!ComponentPermissions.ALL.contains(_permission)) {
-          throw new BadRequestException(String.format("Value of parameter '%s' (%s) for project permissions must be one of %s.", PARAM_PERMISSION, _permission,
-            PROJECT_PERMISSIONS_ONE_LINE));
+          throw new BadRequestException(String.format("The '%s' parameter for project permissions must be one of %s. '%s' was passed.", PARAM_PERMISSION,
+            PROJECT_PERMISSIONS_ONE_LINE, _permission));
         }
       } else if (!GlobalPermissions.ALL.contains(_permission)) {
-        throw new BadRequestException(String.format("Value of parameter '%s' (%s) for global permissions must be one of: %s.", PARAM_PERMISSION, _permission,
-          GLOBAL_PERMISSIONS_ONE_LINE));
+        throw new BadRequestException(String.format("The '%s' parameter for global permissions must be one of %s. '%s' was passed.", PARAM_PERMISSION, GLOBAL_PERMISSIONS_ONE_LINE,
+          _permission
+          ));
       }
     }
   }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionWsCommons.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionWsCommons.java
index a825a075c71..8a0a5f7ed34 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionWsCommons.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionWsCommons.java
@@ -22,7 +22,6 @@ package org.sonar.server.permission.ws;
 
 import com.google.common.base.Joiner;
 import com.google.common.base.Optional;
-import com.google.common.collect.ImmutableSortedSet;
 import javax.annotation.Nullable;
 import org.sonar.api.server.ws.WebService.NewAction;
 import org.sonar.api.web.UserRole;
@@ -56,10 +55,6 @@ public class PermissionWsCommons {
     "</ul>",
     GLOBAL_PERMISSIONS_ONE_LINE,
     PROJECT_PERMISSIONS_ONE_LINE);
-  private static final ImmutableSortedSet<Comparable<?>> POSSIBLE_PERMISSIONS = ImmutableSortedSet.naturalOrder()
-    .addAll(GlobalPermissions.ALL)
-    .addAll(ComponentPermissions.ALL)
-    .build();
 
   private final DbClient dbClient;
   private final ComponentFinder componentFinder;
@@ -141,8 +136,7 @@ public class PermissionWsCommons {
   static void createPermissionParameter(NewAction action) {
     action.createParam(PARAM_PERMISSION)
       .setDescription(PERMISSION_PARAM_DESCRIPTION)
-      .setRequired(true)
-      .setPossibleValues(POSSIBLE_PERMISSIONS);
+      .setRequired(true);
   }
 
   static void createGroupNameParameter(NewAction action) {
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java
index 0df209ca6ba..a95ce9dbc0b 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java
@@ -52,10 +52,10 @@ public class RemoveGroupAction implements PermissionsWsAction {
   @Override
   public void define(WebService.NewController context) {
     WebService.NewAction action = context.createAction(ACTION)
-      .setDescription("Remove permission from a group.<br /> " +
+      .setDescription("Remove a permission from a group.<br /> " +
+        "This service defaults to global permissions, but can be limited to project permissions by providing project id or project key.<br /> " +
         "The group id or group name must be provided, not both.<br />" +
-        "If the project id or project key is provided, a project permission is removed.<br />" +
-        "Requires 'Administer System' permission.")
+        "It requires administration permissions to access.")
       .setSince("5.2")
       .setPost(true)
       .setHandler(this);
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java
index ae94c3b133b..cf5a3509744 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java
@@ -52,8 +52,8 @@ public class RemoveUserAction implements PermissionsWsAction {
   public void define(WebService.NewController context) {
     WebService.NewAction action = context.createAction(ACTION)
       .setDescription("Remove permission from a user.<br /> " +
-        "If the project id or project key is provided, a project permission is removed.<br />" +
-        "Requires 'Administer System' permission.")
+        "This service defaults to global permissions, but can be limited to project permissions by providing project id or project key.<br /> " +
+        "It requires administration permissions to access.")
       .setSince("5.2")
       .setPost(true)
       .setHandler(this);
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/SearchGlobalPermissionsAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/SearchGlobalPermissionsAction.java
index 04f7ebffcdf..6d5591a5c5b 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/SearchGlobalPermissionsAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/SearchGlobalPermissionsAction.java
@@ -52,7 +52,7 @@ public class SearchGlobalPermissionsAction implements PermissionsWsAction {
   public void define(WebService.NewController context) {
     context.createAction("search_global_permissions")
       .setDescription("List global permissions. <br />" +
-        "Requires 'Administer System' permission.")
+        "It requires administration permissions to access.")
       .setResponseExample(getClass().getResource("search_global_permissions-example.json"))
       .setSince("5.2")
       .setHandler(this);
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/UsersAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/UsersAction.java
index 45978a3680f..3160673efea 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/UsersAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/UsersAction.java
@@ -60,7 +60,7 @@ public class UsersAction implements PermissionsWsAction {
       .setDescription(String.format("Lists the users that have been granted the specified permission as individual users rather than through group affiliation. <br />" +
         "This service defaults to global permissions, but can be limited to project permissions by providing project id or project key.<br /> " +
         "If the query parameter '%s' is specified, the '%s' parameter is forced to '%s'.<br />" +
-        "It requires administration permissions to access.<br /> ",
+        "It requires administration permissions to access.<br />",
         Param.TEXT_QUERY, Param.SELECTED, SelectionMode.ALL.value()))
       .addPagingParams(100)
       .addSearchQuery("stas", "names")
diff --git a/server/sonar-server/src/test/java/org/sonar/server/plugins/ws/InstallActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/plugins/ws/InstallActionTest.java
index fd4d2cd478f..284b780b238 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/plugins/ws/InstallActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/plugins/ws/InstallActionTest.java
@@ -111,7 +111,6 @@ public class InstallActionTest {
   @Test
   public void IAE_is_raised_when_key_param_is_not_provided() throws Exception {
     expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("Parameter 'key' is missing");
 
     invalidRequest.execute();
   }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/plugins/ws/UninstallActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/plugins/ws/UninstallActionTest.java
index 4bf85e00f1b..6fe4c07ceb4 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/plugins/ws/UninstallActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/plugins/ws/UninstallActionTest.java
@@ -92,7 +92,6 @@ public class UninstallActionTest {
   @Test
   public void IAE_is_raised_when_key_param_is_not_provided() throws Exception {
     expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("Parameter 'key' is missing");
 
     underTest.handle(invalidRequest, response);
   }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/plugins/ws/UpdateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/plugins/ws/UpdateActionTest.java
index 4f840c72b49..d56aae153f0 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/plugins/ws/UpdateActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/plugins/ws/UpdateActionTest.java
@@ -113,7 +113,6 @@ public class UpdateActionTest {
   @Test
   public void IAE_is_raised_when_key_param_is_not_provided() throws Exception {
     expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("Parameter 'key' is missing");
 
     underTest.handle(invalidRequest, response);
   }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java
index 3a95eefd30b..1dc9fa46239 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java
@@ -187,7 +187,7 @@ public class WebServiceEngineTest {
     ServletResponse response = new ServletResponse();
     engine.execute(request, response, "api/system", "print");
 
-    assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"Parameter 'message' is missing\"}]}");
+    assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"The 'message' parameter is missing\"}]}");
   }
 
   @Test
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java
index 5c6eb1d4b4c..53ada6440c5 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java
@@ -59,7 +59,7 @@ public abstract class Request {
   public String mandatoryParam(String key) {
     String value = param(key);
     if (value == null) {
-      throw new IllegalArgumentException(String.format("Parameter '%s' is missing", key));
+      throw new IllegalArgumentException(String.format("The '%s' parameter is missing", key));
     }
     return value;
   }
@@ -101,7 +101,7 @@ public abstract class Request {
   public List<String> mandatoryParamAsStrings(String key) {
     List<String> values = paramAsStrings(key);
     if (values == null) {
-      throw new IllegalArgumentException(String.format("Parameter '%s' is missing", key));
+      throw new IllegalArgumentException(String.format("The '%s' parameter is missing", key));
     }
     return values;
   }
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java
index 6e86a7c9be3..5cae0e2cdda 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java
@@ -137,7 +137,7 @@ public class RequestTest {
       request.mandatoryParam("required_param");
       fail();
     } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("Parameter 'required_param' is missing");
+      assertThat(e).hasMessage("The 'required_param' parameter is missing");
     }
   }
 
@@ -160,7 +160,7 @@ public class RequestTest {
     try {
       request.mandatoryParamAsStrings("a_required_string");
     } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("Parameter 'a_required_string' is missing");
+      assertThat(e).hasMessage("The 'a_required_string' parameter is missing");
     }
 
     request.setParam("a_required_string", "foo,bar");
-- 
2.39.5