SONAR-6484 WS permissions/remove_group remove by group id or name

4 files changed, 120 insertions, 42 deletions

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 9d4749bccf5..cf847398204 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
@@ -20,19 +20,16 @@
 
 package org.sonar.server.permission.ws;
 
-import javax.annotation.Nullable;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbClient;
-import org.sonar.db.DbSession;
-import org.sonar.db.user.GroupDto;
-import org.sonar.server.exceptions.BadRequestException;
-import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.permission.PermissionChange;
 import org.sonar.server.permission.PermissionUpdater;
 
+import static org.sonar.server.permission.ws.PermissionWsCommons.searchName;
+
 public class AddGroupAction implements PermissionsWsAction {
 
   public static final String ACTION = "add_group";
@@ -78,41 +75,14 @@ public class AddGroupAction implements PermissionsWsAction {
     String groupNameParam = request.param(PARAM_GROUP_NAME);
     Long groupId = request.paramAsLong(PARAM_GROUP_ID);
 
-    String groupName = searchName(groupNameParam, groupId);
+    String groupName = searchName(dbClient, groupNameParam, groupId);
 
     permissionUpdater.addPermission(
       new PermissionChange()
         .setPermission(permission)
         .setGroup(groupName)
-    );
+      );
 
     response.noContent();
   }
-
-  private String searchName(@Nullable String groupNameParam, @Nullable Long groupId) {
-    checkParameters(groupNameParam, groupId);
-    if (groupNameParam != null) {
-      return groupNameParam;
-    }
-
-    DbSession dbSession = dbClient.openSession(false);
-    try {
-      GroupDto group = dbClient.groupDao().selectById(dbSession, groupId);
-      if (group == null) {
-        throw new NotFoundException(String.format("Group with id '%d' not found", groupId));
-      }
-
-      return group.getName();
-    } finally {
-      dbClient.closeSession(dbSession);
-    }
-  }
-
-  private void checkParameters(@Nullable String groupName, @Nullable Long groupId) {
-    if (groupName != null ^ groupId != null) {
-      return;
-    }
-
-    throw new BadRequestException("Group name or group id must be provided, not both");
-  }
 }
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
new file mode 100644
index 00000000000..5eea1e0293a
--- /dev/null
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionWsCommons.java
@@ -0,0 +1,62 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2014 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.permission.ws;
+
+import javax.annotation.Nullable;
+import org.sonar.db.DbClient;
+import org.sonar.db.DbSession;
+import org.sonar.db.user.GroupDto;
+import org.sonar.server.exceptions.BadRequestException;
+import org.sonar.server.exceptions.NotFoundException;
+
+public class PermissionWsCommons {
+
+  private PermissionWsCommons() {
+    // static stuff only
+  }
+
+  public static String searchName(DbClient dbClient, @Nullable String groupNameParam, @Nullable Long groupId) {
+    checkParameters(groupNameParam, groupId);
+    if (groupNameParam != null) {
+      return groupNameParam;
+    }
+
+    DbSession dbSession = dbClient.openSession(false);
+    try {
+      GroupDto group = dbClient.groupDao().selectById(dbSession, groupId);
+      if (group == null) {
+        throw new NotFoundException(String.format("Group with id '%d' is not found", groupId));
+      }
+
+      return group.getName();
+    } finally {
+      dbClient.closeSession(dbSession);
+    }
+  }
+
+  private static void checkParameters(@Nullable String groupName, @Nullable Long groupId) {
+    if (groupName != null ^ groupId != null) {
+      return;
+    }
+
+    throw new BadRequestException("Group name or group id must be provided, not both");
+  }
+}
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 9fa7f7a28d5..737a4104efd 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
@@ -24,25 +24,33 @@ import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.core.permission.GlobalPermissions;
-import org.sonar.server.permission.PermissionService;
+import org.sonar.db.DbClient;
 import org.sonar.server.permission.PermissionChange;
+import org.sonar.server.permission.PermissionService;
+
+import static org.sonar.server.permission.ws.PermissionWsCommons.searchName;
 
 public class RemoveGroupAction implements PermissionsWsAction {
 
   public static final String ACTION = "remove_group";
   public static final String PARAM_PERMISSION = "permission";
   public static final String PARAM_GROUP_NAME = "groupName";
+  public static final String PARAM_GROUP_ID = "groupId";
 
   private final PermissionService permissionService;
+  private final DbClient dbClient;
 
-  public RemoveGroupAction(PermissionService permissionService) {
+  public RemoveGroupAction(PermissionService permissionService, DbClient dbClient) {
     this.permissionService = permissionService;
+    this.dbClient = dbClient;
   }
 
   @Override
   public void define(WebService.NewController context) {
     WebService.NewAction action = context.createAction(ACTION)
-      .setDescription("Remove permission from a group.<br /> Requires 'Administer System' permission.")
+      .setDescription("Remove permission from a group.<br /> " +
+        "The group id or group name must be provided, not both.<br />" +
+        "Requires 'Administer System' permission.")
       .setSince("5.2")
       .setPost(true)
       .setHandler(this);
@@ -53,15 +61,22 @@ public class RemoveGroupAction implements PermissionsWsAction {
       .setPossibleValues(GlobalPermissions.ALL);
 
     action.createParam(PARAM_GROUP_NAME)
-      .setRequired(true)
       .setDescription("Group name or 'anyone' (whatever the case)")
       .setExampleValue("sonar-administrators");
+
+    action.createParam(PARAM_GROUP_ID)
+      .setDescription("Group ID")
+      .setExampleValue("42");
   }
 
   @Override
   public void handle(Request request, Response response) throws Exception {
     String permission = request.mandatoryParam(PARAM_PERMISSION);
-    String groupName = request.mandatoryParam(PARAM_GROUP_NAME);
+    String groupNameParam = request.param(PARAM_GROUP_NAME);
+    Long groupId = request.paramAsLong(PARAM_GROUP_ID);
+
+    String groupName = searchName(dbClient, groupNameParam, groupId);
+
     permissionService.removePermission(
       new PermissionChange()
         .setPermission(permission)
diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/RemoveGroupActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/RemoveGroupActionTest.java
index 082f2140b71..bac3a2c2a08 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/RemoveGroupActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/RemoveGroupActionTest.java
@@ -27,6 +27,9 @@ import org.junit.rules.ExpectedException;
 import org.mockito.ArgumentCaptor;
 import org.sonar.api.utils.System2;
 import org.sonar.db.DbTester;
+import org.sonar.db.user.GroupDto;
+import org.sonar.server.exceptions.BadRequestException;
+import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.exceptions.ServerException;
 import org.sonar.server.permission.PermissionService;
 import org.sonar.server.permission.PermissionChange;
@@ -52,7 +55,7 @@ public class RemoveGroupActionTest {
   public void setUp() {
     permissionService = mock(PermissionService.class);
     ws = new WsTester(new PermissionsWs(
-      new RemoveGroupAction(permissionService)));
+      new RemoveGroupAction(permissionService, db.getDbClient())));
     userSession.login("admin").setGlobalPermissions(SYSTEM_ADMIN);
   }
 
@@ -71,6 +74,23 @@ public class RemoveGroupActionTest {
   }
 
   @Test
+  public void remove_group_by_id() throws Exception {
+    GroupDto group = db.getDbClient().groupDao().insert(db.getSession(), new GroupDto()
+      .setName("sonar-administrators"));
+    db.getSession().commit();
+
+    ws.newPostRequest(PermissionsWs.ENDPOINT, ACTION)
+      .setParam(RemoveGroupAction.PARAM_GROUP_ID, group.getId().toString())
+      .setParam(RemoveGroupAction.PARAM_PERMISSION, SYSTEM_ADMIN)
+      .execute();
+
+    ArgumentCaptor<PermissionChange> permissionChangeCaptor = ArgumentCaptor.forClass(PermissionChange.class);
+    verify(permissionService).removePermission(permissionChangeCaptor.capture());
+    PermissionChange permissionChange = permissionChangeCaptor.getValue();
+    assertThat(permissionChange.group()).isEqualTo("sonar-administrators");
+  }
+
+  @Test
   public void get_request_are_not_authorized() throws Exception {
     expectedException.expect(ServerException.class);
 
@@ -82,7 +102,7 @@ public class RemoveGroupActionTest {
 
   @Test
   public void fail_when_group_name_is_missing() throws Exception {
-    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expect(BadRequestException.class);
 
     ws.newPostRequest(PermissionsWs.ENDPOINT, ACTION)
       .setParam(RemoveGroupAction.PARAM_PERMISSION, SYSTEM_ADMIN)
@@ -90,11 +110,22 @@ public class RemoveGroupActionTest {
   }
 
   @Test
-  public void fail_when_permission_is_missing() throws Exception {
+  public void fail_when_permission_name_and_id_are_missing() throws Exception {
     expectedException.expect(IllegalArgumentException.class);
 
     ws.newPostRequest(PermissionsWs.ENDPOINT, ACTION)
       .setParam(RemoveGroupAction.PARAM_GROUP_NAME, "sonar-administrators")
       .execute();
   }
+
+  @Test
+  public void fail_when_group_id_does_not_exist() throws Exception {
+    expectedException.expect(NotFoundException.class);
+    expectedException.expectMessage("Group with id '42' is not found");
+
+    ws.newPostRequest(PermissionsWs.ENDPOINT, ACTION)
+      .setParam(RemoveGroupAction.PARAM_PERMISSION, SYSTEM_ADMIN)
+      .setParam(RemoveGroupAction.PARAM_GROUP_ID, "42")
+      .execute();
+  }
 }