SONAR-6477 WS usergroups/remove_user handles group name

8 files changed, 181 insertions, 77 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionDependenciesFinder.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionDependenciesFinder.java
index dbcc8f6ec96..4958fa1c812 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionDependenciesFinder.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/PermissionDependenciesFinder.java
@@ -30,6 +30,7 @@ import org.sonar.db.permission.PermissionTemplateDto;
 import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.component.ComponentFinder;
+import org.sonar.server.usergroups.ws.WsGroupRef;
 
 import static java.lang.String.format;
 import static org.sonar.api.security.DefaultGroups.ANYONE;
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 11e48d6b1be..7c1d914d8d4 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
@@ -23,6 +23,7 @@ package org.sonar.server.permission.ws;
 import com.google.common.base.Optional;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.WebService.SelectionMode;
+import org.sonar.server.usergroups.ws.WsGroupRef;
 
 import static org.sonar.api.server.ws.WebService.Param.PAGE;
 import static org.sonar.api.server.ws.WebService.Param.PAGE_SIZE;
@@ -134,7 +135,7 @@ class PermissionRequest {
 
     private void setGroup(Request request) {
       if (withGroup) {
-        this.group = WsGroupRef.fromRequest(request);
+        this.group = WsGroupRef.fromPermissionRequest(request);
       }
     }
 
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/AddGroupToTemplateAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/AddGroupToTemplateAction.java
index 7b459b8cc1d..7d40a1a5698 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/AddGroupToTemplateAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/AddGroupToTemplateAction.java
@@ -31,7 +31,7 @@ import org.sonar.db.permission.PermissionTemplateDto;
 import org.sonar.db.user.GroupDto;
 import org.sonar.server.permission.ws.PermissionDependenciesFinder;
 import org.sonar.server.permission.ws.PermissionsWsAction;
-import org.sonar.server.permission.ws.WsGroupRef;
+import org.sonar.server.usergroups.ws.WsGroupRef;
 import org.sonar.server.permission.ws.WsTemplateRef;
 import org.sonar.server.user.UserSession;
 
@@ -79,7 +79,7 @@ public class AddGroupToTemplateAction implements PermissionsWsAction {
     checkGlobalAdminUser(userSession);
 
     String permission = wsRequest.mandatoryParam(PARAM_PERMISSION);
-    WsGroupRef group = WsGroupRef.fromRequest(wsRequest);
+    WsGroupRef group = WsGroupRef.fromPermissionRequest(wsRequest);
 
     DbSession dbSession = dbClient.openSession(false);
     try {
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/RemoveGroupFromTemplateAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/RemoveGroupFromTemplateAction.java
index d86cd253b4e..06c76a2b975 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/RemoveGroupFromTemplateAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/RemoveGroupFromTemplateAction.java
@@ -29,7 +29,7 @@ import org.sonar.db.permission.PermissionTemplateDto;
 import org.sonar.db.user.GroupDto;
 import org.sonar.server.permission.ws.PermissionDependenciesFinder;
 import org.sonar.server.permission.ws.PermissionsWsAction;
-import org.sonar.server.permission.ws.WsGroupRef;
+import org.sonar.server.usergroups.ws.WsGroupRef;
 import org.sonar.server.permission.ws.WsTemplateRef;
 import org.sonar.server.user.UserSession;
 
@@ -74,7 +74,7 @@ public class RemoveGroupFromTemplateAction implements PermissionsWsAction {
     checkGlobalAdminUser(userSession);
 
     String permission = wsRequest.mandatoryParam(PARAM_PERMISSION);
-    WsGroupRef group = WsGroupRef.fromRequest(wsRequest);
+    WsGroupRef group = WsGroupRef.fromPermissionRequest(wsRequest);
 
     DbSession dbSession = dbClient.openSession(false);
     try {
diff --git a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/RemoveUserAction.java b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/RemoveUserAction.java
index 939bf546063..07e36fa6c5d 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/RemoveUserAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/RemoveUserAction.java
@@ -19,26 +19,32 @@
  */
 package org.sonar.server.usergroups.ws;
 
+import javax.annotation.CheckForNull;
+import org.sonar.api.security.DefaultGroups;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService.NewAction;
 import org.sonar.api.server.ws.WebService.NewController;
 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.db.user.UserDto;
 import org.sonar.db.user.UserGroupDto;
-import org.sonar.server.db.DbClient;
-import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.user.UserSession;
 
-import static org.sonar.db.MyBatis.closeQuietly;
+import static java.lang.String.format;
+import static org.sonar.api.security.DefaultGroups.isAnyone;
+import static org.sonar.server.usergroups.ws.UserGroupsWsParameters.PARAM_GROUP_ID;
+import static org.sonar.server.usergroups.ws.UserGroupsWsParameters.PARAM_GROUP_NAME;
+import static org.sonar.server.usergroups.ws.UserGroupsWsParameters.PARAM_LOGIN;
+import static org.sonar.server.usergroups.ws.UserGroupsWsParameters.createGroupParameters;
+import static org.sonar.server.usergroups.ws.UserGroupsWsParameters.createLoginParameter;
+import static org.sonar.server.ws.WsUtils.checkFound;
+import static org.sonar.server.ws.WsUtils.checkRequest;
 
 public class RemoveUserAction implements UserGroupsWsAction {
 
-  private static final String PARAM_ID = "id";
-  private static final String PARAM_LOGIN = "login";
-
   private final DbClient dbClient;
   private final UserSession userSession;
 
@@ -50,47 +56,69 @@ public class RemoveUserAction implements UserGroupsWsAction {
   @Override
   public void define(NewController context) {
     NewAction action = context.createAction("remove_user")
-      .setDescription("Remove a user from a group.")
+      .setDescription(format("Remove a user from a group.<br />" +
+        "'%s' or '%s' must be provided.", PARAM_GROUP_ID, PARAM_GROUP_NAME))
       .setHandler(this)
       .setPost(true)
       .setSince("5.2");
 
-    action.createParam(PARAM_ID)
-      .setDescription("ID of the group")
-      .setExampleValue("42")
-      .setRequired(true);
-
-    action.createParam(PARAM_LOGIN)
-      .setDescription("Login of the user.")
-      .setExampleValue("g.hopper")
-      .setRequired(true);
+    createGroupParameters(action);
+    createLoginParameter(action);
   }
 
   @Override
   public void handle(Request request, Response response) throws Exception {
     userSession.checkLoggedIn().checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN);
 
-    Long groupId = request.mandatoryParamAsLong(PARAM_ID);
+    WsGroupRef wsGroupRef = WsGroupRef.fromUserGroupsRequest(request);
     String login = request.mandatoryParam(PARAM_LOGIN);
 
     DbSession dbSession = dbClient.openSession(false);
     try {
-      GroupDto group = dbClient.groupDao().selectById(dbSession, groupId);
-      if (group == null) {
-        throw new NotFoundException(String.format("Could not find a user group with id '%s'", groupId));
-      }
-      UserDto user = dbClient.userDao().selectActiveUserByLogin(dbSession, login);
-      if (user == null) {
-        throw new NotFoundException(String.format("Could not find a user with login '%s'", login));
-      }
+      GroupDto group = getGroup(dbSession, wsGroupRef);
+      checkRequest(group != null, "It is not possible to remove a user from the '%s' group.", DefaultGroups.ANYONE);
+      UserDto user = getUser(dbSession, login);
 
       UserGroupDto userGroup = new UserGroupDto().setGroupId(group.getId()).setUserId(user.getId());
       dbClient.userGroupDao().delete(dbSession, userGroup);
       dbSession.commit();
       response.noContent();
     } finally {
-      closeQuietly(dbSession);
+      dbClient.closeSession(dbSession);
+    }
+
+  }
+
+  /**
+   *
+   * @return null if it's the anyone group
+   */
+  @CheckForNull
+  private GroupDto getGroup(DbSession dbSession, WsGroupRef group) {
+    Long groupId = group.id();
+    String groupName = group.name();
+
+    if (isAnyone(groupName)) {
+      return null;
     }
 
+    GroupDto groupDto = null;
+
+    if (groupId != null) {
+      groupDto = checkFound(dbClient.groupDao().selectById(dbSession, groupId),
+        format("Group with id '%d' is not found", groupId));
+    }
+
+    if (groupName != null) {
+      groupDto = checkFound(dbClient.groupDao().selectByName(dbSession, groupName),
+        format("Group with name '%s' is not found", groupName));
+    }
+
+    return groupDto;
+  }
+
+  private UserDto getUser(DbSession dbSession, String userLogin) {
+    return checkFound(dbClient.userDao().selectActiveUserByLogin(dbSession, userLogin),
+      format("User with login '%s' is not found'", userLogin));
   }
 }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UserGroupsWsParameters.java b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UserGroupsWsParameters.java
new file mode 100644
index 00000000000..98ad4fb8244
--- /dev/null
+++ b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UserGroupsWsParameters.java
@@ -0,0 +1,53 @@
+/*
+ * 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.usergroups.ws;
+
+import org.sonar.api.server.ws.WebService.NewAction;
+import org.sonar.api.server.ws.WebService.NewParam;
+
+public class UserGroupsWsParameters {
+  static final String PARAM_GROUP_NAME = "name";
+  static final String PARAM_GROUP_ID = "id";
+  static final String PARAM_LOGIN = "login";
+
+  static void createGroupParameters(NewAction action) {
+    createGroupIdParameter(action);
+    createGroupNameParameter(action);
+  }
+
+  private static void createGroupIdParameter(NewAction action) {
+    action.createParam(PARAM_GROUP_ID)
+      .setDescription("Group id")
+      .setExampleValue("42");
+  }
+
+  private static void createGroupNameParameter(NewAction action) {
+    action.createParam(PARAM_GROUP_NAME)
+      .setDescription("Group name or 'anyone' (case insensitive)")
+      .setExampleValue("sonar-administrators");
+  }
+
+  static NewParam createLoginParameter(NewAction action) {
+    return action.createParam(PARAM_LOGIN)
+      .setDescription("Login of the user.")
+      .setExampleValue("g.hopper");
+  }
+}
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/WsGroupRef.java b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/WsGroupRef.java
index 329b2d4f665..b7c7bf48aa5 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/WsGroupRef.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/WsGroupRef.java
@@ -18,14 +18,13 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
-package org.sonar.server.permission.ws;
+package org.sonar.server.usergroups.ws;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.sonar.api.server.ws.Request;
+import org.sonar.server.permission.ws.WsPermissionParameters;
 
-import static org.sonar.server.permission.ws.WsPermissionParameters.PARAM_GROUP_ID;
-import static org.sonar.server.permission.ws.WsPermissionParameters.PARAM_GROUP_NAME;
 import static org.sonar.server.ws.WsUtils.checkRequest;
 
 /**
@@ -43,9 +42,16 @@ public class WsGroupRef {
     this.name = name;
   }
 
-  public static WsGroupRef fromRequest(Request wsRequest) {
-    Long id = wsRequest.paramAsLong(PARAM_GROUP_ID);
-    String name = wsRequest.param(PARAM_GROUP_NAME);
+  public static WsGroupRef fromPermissionRequest(Request wsRequest) {
+    Long id = wsRequest.paramAsLong(WsPermissionParameters.PARAM_GROUP_ID);
+    String name = wsRequest.param(WsPermissionParameters.PARAM_GROUP_NAME);
+
+    return new WsGroupRef(id, name);
+  }
+
+  public static WsGroupRef fromUserGroupsRequest(Request wsRequest) {
+    Long id = wsRequest.paramAsLong(UserGroupsWsParameters.PARAM_GROUP_ID);
+    String name = wsRequest.param(UserGroupsWsParameters.PARAM_GROUP_NAME);
 
     return new WsGroupRef(id, name);
   }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/RemoveUserActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/RemoveUserActionTest.java
index 5eb47729cef..d710ed9a766 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/RemoveUserActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/RemoveUserActionTest.java
@@ -22,7 +22,6 @@ package org.sonar.server.usergroups.ws;
 
 import java.util.Arrays;
 import org.apache.commons.lang.StringUtils;
-import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -32,75 +31,66 @@ import org.sonar.api.utils.System2;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
+import org.sonar.db.user.GroupDao;
 import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.GroupMembershipDao;
+import org.sonar.db.user.UserDao;
 import org.sonar.db.user.UserDto;
 import org.sonar.db.user.UserGroupDao;
 import org.sonar.db.user.UserGroupDto;
-import org.sonar.server.db.DbClient;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.tester.UserSessionRule;
-import org.sonar.db.user.GroupDao;
-import org.sonar.db.user.UserDao;
 import org.sonar.server.ws.WsTester;
 import org.sonar.test.DbTests;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.server.usergroups.ws.UserGroupsWsParameters.PARAM_GROUP_NAME;
+import static org.sonar.server.usergroups.ws.UserGroupsWsParameters.PARAM_LOGIN;
 
 @Category(DbTests.class)
 public class RemoveUserActionTest {
 
   @Rule
-  public DbTester dbTester = DbTester.create(System2.INSTANCE);
-
+  public DbTester db = DbTester.create(System2.INSTANCE);
   @Rule
   public final UserSessionRule userSession = UserSessionRule.standalone();
-
   @Rule
   public final ExpectedException expectedException = ExpectedException.none();
 
-  private WsTester tester;
+  private WsTester ws;
   private GroupDao groupDao;
   private UserDao userDao;
   private GroupMembershipDao groupMembershipDao;
   private UserGroupDao userGroupDao;
-  private DbSession session;
+  private DbSession dbSession;
 
   @Before
   public void setUp() {
-    dbTester.truncateTables();
+    dbSession = db.getSession();
 
-    groupDao = new GroupDao(System2.INSTANCE);
-    userDao = new UserDao(dbTester.myBatis(), System2.INSTANCE);
-    groupMembershipDao = new GroupMembershipDao(dbTester.myBatis());
-    userGroupDao = new UserGroupDao();
+    org.sonar.db.DbClient dbClient = db.getDbClient();
+    groupDao = dbClient.groupDao();
+    userDao = dbClient.userDao();
+    groupMembershipDao = dbClient.groupMembershipDao();
+    userGroupDao = dbClient.userGroupDao();
 
-    DbClient dbClient = new DbClient(dbTester.database(), dbTester.myBatis(), groupDao, userDao, userGroupDao, groupMembershipDao);
-
-    tester = new WsTester(new UserGroupsWs(new RemoveUserAction(dbClient, userSession)));
-
-    session = dbClient.openSession(false);
-  }
-
-  @After
-  public void after() {
-    session.close();
+    ws = new WsTester(new UserGroupsWs(new RemoveUserAction(dbClient, userSession)));
   }
 
   @Test
   public void remove_user_not_in_group() throws Exception {
     GroupDto group = insertGroup("admins");
     UserDto user = insertUser("my-admin");
-    session.commit();
+    dbSession.commit();
 
     userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
-    tester.newPostRequest("api/usergroups", "remove_user")
+    newRequest()
       .setParam("id", group.getId().toString())
       .setParam("login", user.getLogin())
       .execute()
       .assertNoContent();
 
-    assertThat(groupMembershipDao.selectGroupsByLogins(session, Arrays.asList(user.getLogin())).get(user.getLogin()))
+    assertThat(groupMembershipDao.selectGroupsByLogins(dbSession, Arrays.asList(user.getLogin())).get(user.getLogin()))
       .isEmpty();
   }
 
@@ -109,16 +99,37 @@ public class RemoveUserActionTest {
     GroupDto users = insertGroup("users");
     UserDto user = insertUser("my-admin");
     insertMember(users.getId(), user.getId());
-    session.commit();
+    dbSession.commit();
 
     userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
-    tester.newPostRequest("api/usergroups", "remove_user")
+
+    newRequest()
       .setParam("id", users.getId().toString())
       .setParam("login", user.getLogin())
       .execute()
       .assertNoContent();
 
-    assertThat(groupMembershipDao.selectGroupsByLogins(session, Arrays.asList(user.getLogin())).get(user.getLogin()))
+    assertThat(groupMembershipDao.selectGroupsByLogins(dbSession, Arrays.asList(user.getLogin())).get(user.getLogin()))
+      .isEmpty();
+  }
+
+  @Test
+  public void remove_user_by_group_name() throws Exception {
+    userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
+    GroupDto group = insertGroup("group_name");
+    UserDto user = insertUser("user_login");
+    insertMember(group.getId(), user.getId());
+    assertThat(groupMembershipDao.selectGroupsByLogins(dbSession, Arrays.asList(user.getLogin())).get(user.getLogin()))
+      .isNotEmpty();
+    db.commit();
+
+    newRequest()
+      .setParam(PARAM_GROUP_NAME, group.getName())
+      .setParam(PARAM_LOGIN, user.getLogin())
+      .execute()
+      .assertNoContent();
+
+    assertThat(groupMembershipDao.selectGroupsByLogins(dbSession, Arrays.asList(user.getLogin())).get(user.getLogin()))
       .isEmpty();
   }
 
@@ -129,28 +140,28 @@ public class RemoveUserActionTest {
     UserDto user = insertUser("user");
     insertMember(users.getId(), user.getId());
     insertMember(admins.getId(), user.getId());
-    session.commit();
+    dbSession.commit();
 
     userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
-    tester.newPostRequest("api/usergroups", "remove_user")
+    newRequest()
       .setParam("id", admins.getId().toString())
       .setParam("login", user.getLogin())
       .execute()
       .assertNoContent();
 
-    assertThat(groupMembershipDao.selectGroupsByLogins(session, Arrays.asList(user.getLogin())).get(user.getLogin()))
+    assertThat(groupMembershipDao.selectGroupsByLogins(dbSession, Arrays.asList(user.getLogin())).get(user.getLogin()))
       .containsOnly(users.getName());
   }
 
   @Test
   public void unknown_group() throws Exception {
     UserDto user = insertUser("my-admin");
-    session.commit();
+    dbSession.commit();
 
     expectedException.expect(NotFoundException.class);
 
     userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
-    tester.newPostRequest("api/usergroups", "remove_user")
+    newRequest()
       .setParam("id", "42")
       .setParam("login", user.getLogin())
       .execute();
@@ -159,28 +170,32 @@ public class RemoveUserActionTest {
   @Test
   public void unknown_user() throws Exception {
     GroupDto group = insertGroup("admins");
-    session.commit();
+    dbSession.commit();
 
     expectedException.expect(NotFoundException.class);
 
     userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
-    tester.newPostRequest("api/usergroups", "remove_user")
+    newRequest()
       .setParam("id", group.getId().toString())
       .setParam("login", "my-admin")
       .execute();
   }
 
+  private WsTester.TestRequest newRequest() {
+    return ws.newPostRequest("api/usergroups", "remove_user");
+  }
+
   private GroupDto insertGroup(String groupName) {
-    return groupDao.insert(session, new GroupDto()
+    return groupDao.insert(dbSession, new GroupDto()
       .setName(groupName)
       .setDescription(StringUtils.capitalize(groupName)));
   }
 
   private UserDto insertUser(String login) {
-    return userDao.insert(session, new UserDto().setLogin(login).setName(login).setActive(true));
+    return userDao.insert(dbSession, new UserDto().setLogin(login).setName(login).setActive(true));
   }
 
   private void insertMember(long groupId, long userId) {
-    userGroupDao.insert(session, new UserGroupDto().setGroupId(groupId).setUserId(userId));
+    userGroupDao.insert(dbSession, new UserGroupDto().setGroupId(groupId).setUserId(userId));
   }
 }