From b4aaf8a775f69dd5f1fe1cbc0499f10057f5bf1e Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lievremont Date: Fri, 29 May 2015 16:12:23 +0200 Subject: [PATCH] SONAR-6476 SONAr-6477 Apply feedback from PR --- .../server/usergroups/ws/AddUserAction.java | 12 +- .../usergroups/ws/RemoveUserAction.java | 6 +- .../server/usergroups/ws/UsersAction.java | 28 +-- .../usergroups/ws/AddUserActionTest.java | 24 +-- .../usergroups/ws/RemoveUserActionTest.java | 20 +- .../server/usergroups/ws/UsersActionTest.java | 75 +++---- .../sonar/core/user/GroupMembershipQuery.java | 30 ++- .../sonar/core/user/UserMembershipDto.java | 3 - .../sonar/core/user/UserMembershipQuery.java | 28 ++- .../core/user/GroupMembershipDaoTest.java | 202 +++++++----------- 10 files changed, 182 insertions(+), 246 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java index 0a8ff5781d8..8753ff40025 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/AddUserAction.java @@ -37,8 +37,8 @@ import static org.sonar.core.persistence.MyBatis.closeQuietly; public class AddUserAction implements UserGroupsWsAction { - private static final String PARAM_ID = "id"; - private static final String PARAM_LOGIN = "login"; + private static final String PARAM_ID = "groupId"; + private static final String PARAM_LOGIN = "userLogin"; private final DbClient dbClient; private final UserSession userSession; @@ -72,7 +72,7 @@ public class AddUserAction implements UserGroupsWsAction { userSession.checkLoggedIn().checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN); Long groupId = request.mandatoryParamAsLong(PARAM_ID); - String login = request.param(PARAM_LOGIN); + String login = request.mandatoryParam(PARAM_LOGIN); DbSession dbSession = dbClient.openSession(false); try { @@ -82,7 +82,7 @@ public class AddUserAction implements UserGroupsWsAction { throw new NotFoundException(String.format("Could not find a user with login '%s'", login)); } - if (!userIsAlreadyMemberOf(dbSession, login, group)) { + if (userIsNotYetMemberOf(dbSession, login, group)) { UserGroupDto userGroup = new UserGroupDto().setGroupId(group.getId()).setUserId(user.getId()); dbClient.userGroupDao().insert(dbSession, userGroup); dbSession.commit(); @@ -95,7 +95,7 @@ public class AddUserAction implements UserGroupsWsAction { } - private boolean userIsAlreadyMemberOf(DbSession dbSession, String login, GroupDto group) { - return dbClient.groupMembershipDao().selectGroupsByLogins(dbSession, Arrays.asList(login)).get(login).contains(group.getName()); + private boolean userIsNotYetMemberOf(DbSession dbSession, String login, GroupDto group) { + return !dbClient.groupMembershipDao().selectGroupsByLogins(dbSession, Arrays.asList(login)).get(login).contains(group.getName()); } } 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 1e5dcc66d30..b8fc4d863b1 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 @@ -36,8 +36,8 @@ import static org.sonar.core.persistence.MyBatis.closeQuietly; public class RemoveUserAction implements UserGroupsWsAction { - private static final String PARAM_ID = "id"; - private static final String PARAM_LOGIN = "login"; + private static final String PARAM_ID = "groupId"; + private static final String PARAM_LOGIN = "userLogin"; private final DbClient dbClient; private final UserSession userSession; @@ -71,7 +71,7 @@ public class RemoveUserAction implements UserGroupsWsAction { userSession.checkLoggedIn().checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN); Long groupId = request.mandatoryParamAsLong(PARAM_ID); - String login = request.param(PARAM_LOGIN); + String login = request.mandatoryParam(PARAM_LOGIN); DbSession dbSession = dbClient.openSession(false); try { diff --git a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UsersAction.java b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UsersAction.java index c6c5d185e1f..eabc744a716 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UsersAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UsersAction.java @@ -29,12 +29,11 @@ import org.sonar.api.server.ws.WebService.Param; import org.sonar.api.utils.Paging; import org.sonar.api.utils.text.JsonWriter; import org.sonar.core.persistence.DbSession; -import org.sonar.core.user.GroupDto; +import org.sonar.core.persistence.MyBatis; import org.sonar.core.user.GroupMembershipQuery; import org.sonar.core.user.UserMembershipDto; import org.sonar.core.user.UserMembershipQuery; import org.sonar.server.db.DbClient; -import org.sonar.server.exceptions.NotFoundException; public class UsersAction implements UserGroupsWsAction { @@ -54,7 +53,7 @@ public class UsersAction implements UserGroupsWsAction { @Override public void define(NewController context) { NewAction action = context.createAction("users") - .setDescription("List the members of a group.") + .setDescription("Search for users with membership information with respect to a group.") .setHandler(this) .setResponseExample(getClass().getResource("example-users.json")) .setSince("5.2"); @@ -65,13 +64,11 @@ public class UsersAction implements UserGroupsWsAction { .setRequired(true); action.createParam(PARAM_SELECTED) - .setDescription("If specified, only show users who belong to this group (selected) or not (deselected).") + .setDescription("If specified, only show users who belong to a group (selected=selected) or only those who do not (selected=deselected).") .setPossibleValues(SELECTION_SELECTED, SELECTION_DESELECTED, SELECTION_ALL) .setDefaultValue(SELECTION_ALL); - action.createParam(Param.TEXT_QUERY) - .setDescription("If specified, only show users whose name or login contains the query.") - .setExampleValue("freddy"); + action.addSearchQuery("freddy", "names", "logins"); action.addPagingParams(25); } @@ -92,22 +89,19 @@ public class UsersAction implements UserGroupsWsAction { .pageSize(pageSize) .build(); - DbSession session = dbClient.openSession(false); + DbSession dbSession = dbClient.openSession(false); try { - GroupDto group = dbClient.groupDao().selectById(session, groupId); - if (group == null) { - throw new NotFoundException(String.format("Unable to find a group with ID '%d'", groupId)); - } - int total = dbClient.groupMembershipDao().countMembers(session, query); + dbClient.groupDao().selectById(dbSession, groupId); + int total = dbClient.groupMembershipDao().countMembers(dbSession, query); Paging paging = Paging.create(pageSize, page, total); - List users = dbClient.groupMembershipDao().selectMembers(session, query, paging.offset(), pageSize); + List users = dbClient.groupMembershipDao().selectMembers(dbSession, query, paging.offset(), paging.pageSize()); JsonWriter json = response.newJsonWriter().beginObject(); writeMembers(json, users); writePaging(json, paging); json.endObject().close(); } finally { - session.close(); + MyBatis.closeQuietly(dbSession); } } @@ -124,8 +118,8 @@ public class UsersAction implements UserGroupsWsAction { } private void writePaging(JsonWriter json, Paging paging) { - json.prop("p", paging.pageIndex()) - .prop("ps", paging.pageSize()) + json.prop(Param.PAGE, paging.pageIndex()) + .prop(Param.PAGE_SIZE, paging.pageSize()) .prop("total", paging.total()); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/AddUserActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/AddUserActionTest.java index db56d1a1afd..84d69569aef 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/AddUserActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/AddUserActionTest.java @@ -97,8 +97,8 @@ public class AddUserActionTest { userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); tester.newPostRequest("api/usergroups", "add_user") - .setParam("id", group.getId().toString()) - .setParam("login", user.getLogin()) + .setParam("groupId", group.getId().toString()) + .setParam("userLogin", user.getLogin()) .execute() .assertNoContent(); @@ -116,8 +116,8 @@ public class AddUserActionTest { userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); tester.newPostRequest("api/usergroups", "add_user") - .setParam("id", admins.getId().toString()) - .setParam("login", user.getLogin()) + .setParam("groupId", admins.getId().toString()) + .setParam("userLogin", user.getLogin()) .execute() .assertNoContent(); @@ -134,8 +134,8 @@ public class AddUserActionTest { userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); tester.newPostRequest("api/usergroups", "add_user") - .setParam("id", users.getId().toString()) - .setParam("login", user.getLogin()) + .setParam("groupId", users.getId().toString()) + .setParam("userLogin", user.getLogin()) .execute() .assertNoContent(); @@ -153,8 +153,8 @@ public class AddUserActionTest { userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); tester.newPostRequest("api/usergroups", "add_user") - .setParam("id", users.getId().toString()) - .setParam("login", user2.getLogin()) + .setParam("groupId", users.getId().toString()) + .setParam("userLogin", user2.getLogin()) .execute() .assertNoContent(); @@ -172,8 +172,8 @@ public class AddUserActionTest { userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); tester.newPostRequest("api/usergroups", "add_user") - .setParam("id", "42") - .setParam("login", user.getLogin()) + .setParam("groupId", "42") + .setParam("userLogin", user.getLogin()) .execute(); } @@ -186,8 +186,8 @@ public class AddUserActionTest { userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); tester.newPostRequest("api/usergroups", "add_user") - .setParam("id", group.getId().toString()) - .setParam("login", "my-admin") + .setParam("groupId", group.getId().toString()) + .setParam("userLogin", "my-admin") .execute(); } 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 106671a5e07..4b87e3daabe 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 @@ -96,8 +96,8 @@ public class RemoveUserActionTest { userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); tester.newPostRequest("api/usergroups", "remove_user") - .setParam("id", group.getId().toString()) - .setParam("login", user.getLogin()) + .setParam("groupId", group.getId().toString()) + .setParam("userLogin", user.getLogin()) .execute() .assertNoContent(); @@ -114,8 +114,8 @@ public class RemoveUserActionTest { userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); tester.newPostRequest("api/usergroups", "remove_user") - .setParam("id", users.getId().toString()) - .setParam("login", user.getLogin()) + .setParam("groupId", users.getId().toString()) + .setParam("userLogin", user.getLogin()) .execute() .assertNoContent(); @@ -134,8 +134,8 @@ public class RemoveUserActionTest { userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); tester.newPostRequest("api/usergroups", "remove_user") - .setParam("id", admins.getId().toString()) - .setParam("login", user.getLogin()) + .setParam("groupId", admins.getId().toString()) + .setParam("userLogin", user.getLogin()) .execute() .assertNoContent(); @@ -152,8 +152,8 @@ public class RemoveUserActionTest { userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); tester.newPostRequest("api/usergroups", "remove_user") - .setParam("id", "42") - .setParam("login", user.getLogin()) + .setParam("groupId", "42") + .setParam("userLogin", user.getLogin()) .execute(); } @@ -166,8 +166,8 @@ public class RemoveUserActionTest { userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); tester.newPostRequest("api/usergroups", "remove_user") - .setParam("id", group.getId().toString()) - .setParam("login", "my-admin") + .setParam("groupId", group.getId().toString()) + .setParam("userLogin", "my-admin") .execute(); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UsersActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UsersActionTest.java index 1540f031856..928e0c638a5 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UsersActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UsersActionTest.java @@ -20,11 +20,13 @@ package org.sonar.server.usergroups.ws; +import org.sonar.server.ws.WsTester.TestRequest; + import org.junit.After; import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; -import org.sonar.api.server.ws.WebService; +import org.junit.experimental.categories.Category; import org.sonar.api.utils.System2; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.DbTester; @@ -38,25 +40,23 @@ import org.sonar.server.user.db.GroupDao; import org.sonar.server.user.db.UserDao; import org.sonar.server.user.db.UserGroupDao; import org.sonar.server.ws.WsTester; +import org.sonar.test.DbTests; +@Category(DbTests.class) public class UsersActionTest { @ClassRule public static final DbTester dbTester = new DbTester(); - WebService.Controller controller; - - WsTester tester; - + WsTester wsTester; DbClient dbClient; - DbSession session; @Before public void setUp() { dbTester.truncateTables(); - System2 system2 = new System2(); + System2 system2 = System2.INSTANCE; UserDao userDao = new UserDao(dbTester.myBatis(), system2); GroupDao groupDao = new GroupDao(system2); UserGroupDao userGroupDao = new UserGroupDao(); @@ -66,8 +66,7 @@ public class UsersActionTest { session = dbClient.openSession(false); session.commit(); - tester = new WsTester(new UserGroupsWs(new UsersAction(dbClient))); - controller = tester.controller("api/users"); + wsTester = new WsTester(new UserGroupsWs(new UsersAction(dbClient))); } @@ -78,17 +77,21 @@ public class UsersActionTest { @Test(expected = NotFoundException.class) public void fail_on_unknown_user() throws Exception { - tester.newGetRequest("api/usergroups", "users") + newUsersRequest() .setParam("id", "42") .setParam("login", "john").execute(); } + private TestRequest newUsersRequest() { + return wsTester.newGetRequest("api/usergroups", "users"); + } + @Test public void empty_users() throws Exception { - GroupDto group = createGroup(); + GroupDto group = insertGroup(); session.commit(); - tester.newGetRequest("api/usergroups", "users") + newUsersRequest() .setParam("login", "john") .setParam("id", group.getId().toString()) .execute() @@ -97,13 +100,13 @@ public class UsersActionTest { @Test public void all_users() throws Exception { - GroupDto group = createGroup(); - UserDto groupUser = createUser("ada", "Ada Lovelace"); - createUser("grace", "Grace Hopper"); + GroupDto group = insertGroup(); + UserDto groupUser = insertUser("ada", "Ada Lovelace"); + insertUser("grace", "Grace Hopper"); addUserToGroup(groupUser, group); session.commit(); - tester.newGetRequest("api/usergroups", "users") + newUsersRequest() .setParam("id", group.getId().toString()) .execute() .assertJson(getClass(), "all.json"); @@ -111,13 +114,13 @@ public class UsersActionTest { @Test public void selected_users() throws Exception { - GroupDto group = createGroup(); - UserDto groupUser = createUser("ada", "Ada Lovelace"); - createUser("grace", "Grace Hopper"); + GroupDto group = insertGroup(); + UserDto groupUser = insertUser("ada", "Ada Lovelace"); + insertUser("grace", "Grace Hopper"); addUserToGroup(groupUser, group); session.commit(); - tester.newGetRequest("api/usergroups", "users") + newUsersRequest() .setParam("id", group.getId().toString()) .setParam("selected", "selected") .execute() @@ -126,13 +129,13 @@ public class UsersActionTest { @Test public void deselected_users() throws Exception { - GroupDto group = createGroup(); - UserDto groupUser = createUser("ada", "Ada Lovelace"); - createUser("grace", "Grace Hopper"); + GroupDto group = insertGroup(); + UserDto groupUser = insertUser("ada", "Ada Lovelace"); + insertUser("grace", "Grace Hopper"); addUserToGroup(groupUser, group); session.commit(); - tester.newGetRequest("api/usergroups", "users") + newUsersRequest() .setParam("id", group.getId().toString()) .setParam("selected", "deselected") .execute() @@ -141,19 +144,19 @@ public class UsersActionTest { @Test public void paging() throws Exception { - GroupDto group = createGroup(); - UserDto groupUser = createUser("ada", "Ada Lovelace"); - createUser("grace", "Grace Hopper"); + GroupDto group = insertGroup(); + UserDto groupUser = insertUser("ada", "Ada Lovelace"); + insertUser("grace", "Grace Hopper"); addUserToGroup(groupUser, group); session.commit(); - tester.newGetRequest("api/usergroups", "users") + newUsersRequest() .setParam("id", group.getId().toString()) .setParam("ps", "1") .execute() .assertJson(getClass(), "all_page1.json"); - tester.newGetRequest("api/usergroups", "users") + newUsersRequest() .setParam("id", group.getId().toString()) .setParam("ps", "1") .setParam("p", "2") @@ -163,31 +166,31 @@ public class UsersActionTest { @Test public void filtering() throws Exception { - GroupDto group = createGroup(); - UserDto groupUser = createUser("ada", "Ada Lovelace"); - createUser("grace", "Grace Hopper"); + GroupDto group = insertGroup(); + UserDto groupUser = insertUser("ada", "Ada Lovelace"); + insertUser("grace", "Grace Hopper"); addUserToGroup(groupUser, group); session.commit(); - tester.newGetRequest("api/usergroups", "users") + newUsersRequest() .setParam("id", group.getId().toString()) .setParam("q", "ace") .execute() .assertJson(getClass(), "all.json"); - tester.newGetRequest("api/usergroups", "users") + newUsersRequest() .setParam("id", group.getId().toString()) .setParam("q", "love") .execute() .assertJson(getClass(), "all_ada.json"); } - private GroupDto createGroup() { + private GroupDto insertGroup() { return dbClient.groupDao().insert(session, new GroupDto() .setName("sonar-users")); } - private UserDto createUser(String login, String name) { + private UserDto insertUser(String login, String name) { return dbClient.userDao().insert(session, new UserDto().setLogin(login).setName(name)); } diff --git a/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipQuery.java b/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipQuery.java index d0a55a28305..1fae7519f9d 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipQuery.java +++ b/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipQuery.java @@ -19,14 +19,15 @@ */ package org.sonar.core.user; -import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; -import org.apache.commons.lang.StringUtils; - +import java.util.Set; import javax.annotation.CheckForNull; import javax.annotation.Nullable; +import org.apache.commons.lang.StringUtils; -import java.util.Set; +import static com.google.common.base.Objects.firstNonNull; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; public class GroupMembershipQuery { @@ -139,29 +140,22 @@ public class GroupMembershipQuery { } private void initMembership() { - if (membership == null) { - membership = GroupMembershipQuery.ANY; - } else { - Preconditions.checkArgument(AVAILABLE_MEMBERSHIP.contains(membership), - "Membership is not valid (got " + membership + "). Availables values are " + AVAILABLE_MEMBERSHIP); - } + membership = firstNonNull(membership, ANY); + checkArgument(AVAILABLE_MEMBERSHIP.contains(membership), + "Membership is not valid (got " + membership + "). Availables values are " + AVAILABLE_MEMBERSHIP); } private void initPageSize() { - if (pageSize == null) { - pageSize = DEFAULT_PAGE_SIZE; - } + pageSize = firstNonNull(pageSize, DEFAULT_PAGE_SIZE); } private void initPageIndex() { - if (pageIndex == null) { - pageIndex = DEFAULT_PAGE_INDEX; - } - Preconditions.checkArgument(pageIndex > 0, "Page index must be greater than 0 (got " + pageIndex + ")"); + pageIndex = firstNonNull(pageIndex, DEFAULT_PAGE_INDEX); + checkArgument(pageIndex > 0, "Page index must be greater than 0 (got " + pageIndex + ")"); } public GroupMembershipQuery build() { - Preconditions.checkNotNull(login, "User login cant be null."); + checkNotNull(login, "User login cant be null."); initMembership(); initPageIndex(); initPageSize(); diff --git a/sonar-core/src/main/java/org/sonar/core/user/UserMembershipDto.java b/sonar-core/src/main/java/org/sonar/core/user/UserMembershipDto.java index afb6ca9584a..b7a128f1d7e 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/UserMembershipDto.java +++ b/sonar-core/src/main/java/org/sonar/core/user/UserMembershipDto.java @@ -22,9 +22,6 @@ package org.sonar.core.user; import javax.annotation.CheckForNull; import javax.annotation.Nullable; -/** - * @since 5.2 - */ public class UserMembershipDto { private Long id; diff --git a/sonar-core/src/main/java/org/sonar/core/user/UserMembershipQuery.java b/sonar-core/src/main/java/org/sonar/core/user/UserMembershipQuery.java index 209b2063c73..c54ff834be1 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/UserMembershipQuery.java +++ b/sonar-core/src/main/java/org/sonar/core/user/UserMembershipQuery.java @@ -19,13 +19,16 @@ */ package org.sonar.core.user; -import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; import java.util.Set; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.apache.commons.lang.StringUtils; +import static com.google.common.base.Objects.firstNonNull; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; + public class UserMembershipQuery { public static final int DEFAULT_PAGE_INDEX = 1; @@ -34,7 +37,7 @@ public class UserMembershipQuery { public static final String ANY = "ANY"; public static final String IN = "IN"; public static final String OUT = "OUT"; - public static final Set AVAILABLE_MEMBERSHIP = ImmutableSet.of(ANY, IN, OUT); + public static final Set AVAILABLE_MEMBERSHIPS = ImmutableSet.of(ANY, IN, OUT); private final Long groupId; private final String membership; @@ -137,29 +140,22 @@ public class UserMembershipQuery { } private void initMembership() { - if (membership == null) { - membership = UserMembershipQuery.ANY; - } else { - Preconditions.checkArgument(AVAILABLE_MEMBERSHIP.contains(membership), - "Membership is not valid (got " + membership + "). Availables values are " + AVAILABLE_MEMBERSHIP); - } + membership = firstNonNull(membership, ANY); + checkArgument(AVAILABLE_MEMBERSHIPS.contains(membership), + "Membership is not valid (got " + membership + "). Availables values are " + AVAILABLE_MEMBERSHIPS); } private void initPageSize() { - if (pageSize == null) { - pageSize = DEFAULT_PAGE_SIZE; - } + pageSize = firstNonNull(pageSize, DEFAULT_PAGE_SIZE); } private void initPageIndex() { - if (pageIndex == null) { - pageIndex = DEFAULT_PAGE_INDEX; - } - Preconditions.checkArgument(pageIndex > 0, "Page index must be greater than 0 (got " + pageIndex + ")"); + pageIndex = firstNonNull(pageIndex, DEFAULT_PAGE_INDEX); + checkArgument(pageIndex > 0, "Page index must be greater than 0 (got " + pageIndex + ")"); } public UserMembershipQuery build() { - Preconditions.checkNotNull(groupId, "Group ID cant be null."); + checkNotNull(groupId, "Group ID cant be null."); initMembership(); initPageIndex(); initPageSize(); diff --git a/sonar-core/src/test/java/org/sonar/core/user/GroupMembershipDaoTest.java b/sonar-core/src/test/java/org/sonar/core/user/GroupMembershipDaoTest.java index 8b8992f07f0..e2b45132d91 100644 --- a/sonar-core/src/test/java/org/sonar/core/user/GroupMembershipDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/user/GroupMembershipDaoTest.java @@ -23,12 +23,14 @@ package org.sonar.core.user; import com.google.common.collect.Multimap; import java.util.Arrays; import java.util.List; +import org.junit.After; import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; import org.junit.experimental.categories.Category; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.DbTester; +import org.sonar.core.persistence.MyBatis; import org.sonar.test.DbTests; import static org.assertj.core.api.Assertions.assertThat; @@ -42,10 +44,18 @@ public class GroupMembershipDaoTest { private GroupMembershipDao dao; + private DbSession dbSession; + @Before public void setUp() { dbTester.truncateTables(); dao = new GroupMembershipDao(dbTester.myBatis()); + dbSession = dbTester.myBatis().openSession(false); + } + + @After + public void tearDown() { + MyBatis.closeQuietly(dbSession); } @Test @@ -155,154 +165,108 @@ public class GroupMembershipDaoTest { @Test public void count_groups() { dbTester.prepareDbUnit(getClass(), "shared.xml"); - DbSession session = dbTester.myBatis().openSession(false); - - try { - // 200 is member of 3 groups - assertThat(dao.countGroups(session, GroupMembershipQuery.builder().login("arthur").membership(GroupMembershipQuery.IN).build(), 200L)).isEqualTo(3); - assertThat(dao.countGroups(session, GroupMembershipQuery.builder().login("arthur").membership(GroupMembershipQuery.OUT).build(), 200L)).isZero(); - // 201 is member of 1 group on 3 - assertThat(dao.countGroups(session, GroupMembershipQuery.builder().login("arthur").membership(GroupMembershipQuery.IN).build(), 201L)).isEqualTo(1); - assertThat(dao.countGroups(session, GroupMembershipQuery.builder().login("arthur").membership(GroupMembershipQuery.OUT).build(), 201L)).isEqualTo(2); - // 999 is member of 0 group - assertThat(dao.countGroups(session, GroupMembershipQuery.builder().login("arthur").membership(GroupMembershipQuery.IN).build(), 999L)).isZero(); - assertThat(dao.countGroups(session, GroupMembershipQuery.builder().login("arthur").membership(GroupMembershipQuery.OUT).build(), 2999L)).isEqualTo(3); - } finally { - session.close(); - } + + // 200 is member of 3 groups + assertThat(dao.countGroups(dbSession, GroupMembershipQuery.builder().login("arthur").membership(GroupMembershipQuery.IN).build(), 200L)).isEqualTo(3); + assertThat(dao.countGroups(dbSession, GroupMembershipQuery.builder().login("arthur").membership(GroupMembershipQuery.OUT).build(), 200L)).isZero(); + // 201 is member of 1 group on 3 + assertThat(dao.countGroups(dbSession, GroupMembershipQuery.builder().login("arthur").membership(GroupMembershipQuery.IN).build(), 201L)).isEqualTo(1); + assertThat(dao.countGroups(dbSession, GroupMembershipQuery.builder().login("arthur").membership(GroupMembershipQuery.OUT).build(), 201L)).isEqualTo(2); + // 999 is member of 0 group + assertThat(dao.countGroups(dbSession, GroupMembershipQuery.builder().login("arthur").membership(GroupMembershipQuery.IN).build(), 999L)).isZero(); + assertThat(dao.countGroups(dbSession, GroupMembershipQuery.builder().login("arthur").membership(GroupMembershipQuery.OUT).build(), 2999L)).isEqualTo(3); } @Test public void count_users_by_group() { dbTester.prepareDbUnit(getClass(), "shared_plus_empty_group.xml"); - DbSession session = dbTester.myBatis().openSession(false); - - try { - assertThat(dao.countUsersByGroups(session, Arrays.asList(100L, 101L, 102L, 103L))).containsOnly( - entry("sonar-users", 2), entry("sonar-reviewers", 1), entry("sonar-administrators", 1), entry("sonar-nobody", 0)); - assertThat(dao.countUsersByGroups(session, Arrays.asList(100L, 103L))).containsOnly( - entry("sonar-administrators", 1), entry("sonar-nobody", 0)); - } finally { - session.close(); - } + + assertThat(dao.countUsersByGroups(dbSession, Arrays.asList(100L, 101L, 102L, 103L))).containsOnly( + entry("sonar-users", 2), entry("sonar-reviewers", 1), entry("sonar-administrators", 1), entry("sonar-nobody", 0)); + assertThat(dao.countUsersByGroups(dbSession, Arrays.asList(100L, 103L))).containsOnly( + entry("sonar-administrators", 1), entry("sonar-nobody", 0)); } @Test public void count_groups_by_login() { dbTester.prepareDbUnit(getClass(), "shared.xml"); - DbSession session = dbTester.myBatis().openSession(false); - - try { - assertThat(dao.selectGroupsByLogins(session, Arrays.asList()).keys()).isEmpty(); - Multimap groupsByLogin = dao.selectGroupsByLogins(session, Arrays.asList("two-hundred", "two-hundred-one", "two-hundred-two")); - assertThat(groupsByLogin.get("two-hundred")).containsOnly("sonar-administrators", "sonar-users", "sonar-reviewers"); - assertThat(groupsByLogin.get("two-hundred-one")).containsOnly("sonar-users"); - assertThat(groupsByLogin.get("two-hundred-two")).isEmpty(); - } finally { - session.close(); - } + + assertThat(dao.selectGroupsByLogins(dbSession, Arrays.asList()).keys()).isEmpty(); + Multimap groupsByLogin = dao.selectGroupsByLogins(dbSession, Arrays.asList("two-hundred", "two-hundred-one", "two-hundred-two")); + assertThat(groupsByLogin.get("two-hundred")).containsOnly("sonar-administrators", "sonar-users", "sonar-reviewers"); + assertThat(groupsByLogin.get("two-hundred-one")).containsOnly("sonar-users"); + assertThat(groupsByLogin.get("two-hundred-two")).isEmpty(); } @Test public void count_members() { dbTester.prepareDbUnit(getClass(), "shared_plus_empty_group.xml"); - DbSession session = dbTester.myBatis().openSession(false); - try { - // 100 has 1 member and 1 non member - assertThat(dao.countMembers(session, UserMembershipQuery.builder().groupId(100L).membership(UserMembershipQuery.IN).build())).isEqualTo(1); - assertThat(dao.countMembers(session, UserMembershipQuery.builder().groupId(100L).membership(UserMembershipQuery.OUT).build())).isEqualTo(1); - // 101 has 2 members - assertThat(dao.countMembers(session, UserMembershipQuery.builder().groupId(101L).membership(UserMembershipQuery.IN).build())).isEqualTo(2); - assertThat(dao.countMembers(session, UserMembershipQuery.builder().groupId(101L).membership(UserMembershipQuery.OUT).build())).isZero(); - // 102 has 1 member and 1 non member - assertThat(dao.countMembers(session, UserMembershipQuery.builder().groupId(102L).membership(UserMembershipQuery.IN).build())).isEqualTo(1); - assertThat(dao.countMembers(session, UserMembershipQuery.builder().groupId(102L).membership(UserMembershipQuery.OUT).build())).isEqualTo(1); - // 103 has no member - assertThat(dao.countMembers(session, UserMembershipQuery.builder().groupId(103L).membership(UserMembershipQuery.IN).build())).isZero(); - assertThat(dao.countMembers(session, UserMembershipQuery.builder().groupId(103L).membership(UserMembershipQuery.OUT).build())).isEqualTo(2); - } finally { - session.close(); - } + // 100 has 1 member and 1 non member + assertThat(dao.countMembers(dbSession, UserMembershipQuery.builder().groupId(100L).membership(UserMembershipQuery.IN).build())).isEqualTo(1); + assertThat(dao.countMembers(dbSession, UserMembershipQuery.builder().groupId(100L).membership(UserMembershipQuery.OUT).build())).isEqualTo(1); + // 101 has 2 members + assertThat(dao.countMembers(dbSession, UserMembershipQuery.builder().groupId(101L).membership(UserMembershipQuery.IN).build())).isEqualTo(2); + assertThat(dao.countMembers(dbSession, UserMembershipQuery.builder().groupId(101L).membership(UserMembershipQuery.OUT).build())).isZero(); + // 102 has 1 member and 1 non member + assertThat(dao.countMembers(dbSession, UserMembershipQuery.builder().groupId(102L).membership(UserMembershipQuery.IN).build())).isEqualTo(1); + assertThat(dao.countMembers(dbSession, UserMembershipQuery.builder().groupId(102L).membership(UserMembershipQuery.OUT).build())).isEqualTo(1); + // 103 has no member + assertThat(dao.countMembers(dbSession, UserMembershipQuery.builder().groupId(103L).membership(UserMembershipQuery.IN).build())).isZero(); + assertThat(dao.countMembers(dbSession, UserMembershipQuery.builder().groupId(103L).membership(UserMembershipQuery.OUT).build())).isEqualTo(2); } @Test public void select_group_members_by_query() { dbTester.prepareDbUnit(getClass(), "shared_plus_empty_group.xml"); - DbSession session = dbTester.myBatis().openSession(false); - - try { - // 100 has 1 member - assertThat(dao.selectMembers(session, UserMembershipQuery.builder().groupId(100L).membership(UserMembershipQuery.IN).build(), 0, 10)).hasSize(1); - // 101 has 2 members - assertThat(dao.selectMembers(session, UserMembershipQuery.builder().groupId(101L).membership(UserMembershipQuery.IN).build(), 0, 10)).hasSize(2); - // 102 has 1 member - assertThat(dao.selectMembers(session, UserMembershipQuery.builder().groupId(102L).membership(UserMembershipQuery.IN).build(), 0, 10)).hasSize(1); - // 103 has no member - assertThat(dao.selectMembers(session, UserMembershipQuery.builder().groupId(103L).membership(UserMembershipQuery.IN).build(), 0, 10)).isEmpty(); - } finally { - session.close(); - } + // 100 has 1 member + assertThat(dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(100L).membership(UserMembershipQuery.IN).build(), 0, 10)).hasSize(1); + // 101 has 2 members + assertThat(dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(101L).membership(UserMembershipQuery.IN).build(), 0, 10)).hasSize(2); + // 102 has 1 member + assertThat(dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(102L).membership(UserMembershipQuery.IN).build(), 0, 10)).hasSize(1); + // 103 has no member + assertThat(dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(103L).membership(UserMembershipQuery.IN).build(), 0, 10)).isEmpty(); } @Test public void select_users_not_affected_to_a_group_by_query() { dbTester.prepareDbUnit(getClass(), "shared_plus_empty_group.xml"); - DbSession session = dbTester.myBatis().openSession(false); - - try { // 100 has 1 member - assertThat(dao.selectMembers(session, UserMembershipQuery.builder().groupId(100L).membership(UserMembershipQuery.OUT).build(), 0, 10)).hasSize(1); + assertThat(dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(100L).membership(UserMembershipQuery.OUT).build(), 0, 10)).hasSize(1); // 101 has 2 members - assertThat(dao.selectMembers(session, UserMembershipQuery.builder().groupId(101L).membership(UserMembershipQuery.OUT).build(), 0, 10)).isEmpty(); + assertThat(dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(101L).membership(UserMembershipQuery.OUT).build(), 0, 10)).isEmpty(); // 102 has 1 member - assertThat(dao.selectMembers(session, UserMembershipQuery.builder().groupId(102L).membership(UserMembershipQuery.OUT).build(), 0, 10)).hasSize(1); + assertThat(dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(102L).membership(UserMembershipQuery.OUT).build(), 0, 10)).hasSize(1); // 103 has no member - assertThat(dao.selectMembers(session, UserMembershipQuery.builder().groupId(103L).membership(UserMembershipQuery.OUT).build(), 0, 10)).hasSize(2); - } finally { - session.close(); - } + assertThat(dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(103L).membership(UserMembershipQuery.OUT).build(), 0, 10)).hasSize(2); } @Test public void search_by_user_name_or_login() { dbTester.prepareDbUnit(getClass(), "shared_plus_empty_group.xml"); - DbSession session = dbTester.myBatis().openSession(false); - - try { - - List result = dao.selectMembers(session, UserMembershipQuery.builder().groupId(100L).memberSearch("admin").build(), 0, 10); - assertThat(result).hasSize(2); + List result = dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(100L).memberSearch("admin").build(), 0, 10); + assertThat(result).hasSize(2); - assertThat(result.get(0).getName()).isEqualTo("Admin"); - assertThat(result.get(1).getName()).isEqualTo("Not Admin"); + assertThat(result.get(0).getName()).isEqualTo("Admin"); + assertThat(result.get(1).getName()).isEqualTo("Not Admin"); - result = dao.selectMembers(session, UserMembershipQuery.builder().groupId(100L).memberSearch("not").build(), 0, 10); + result = dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(100L).memberSearch("not").build(), 0, 10); assertThat(result).hasSize(1); - - } finally { - session.close(); - } } @Test public void search_by_login_or_name_with_capitalization() { dbTester.prepareDbUnit(getClass(), "shared_plus_empty_group.xml"); - DbSession session = dbTester.myBatis().openSession(false); - - try { - List result = dao.selectMembers(session, UserMembershipQuery.builder().groupId(100L).memberSearch("admin").build(), 0, 10); - assertThat(result).hasSize(2); + List result = dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(100L).memberSearch("admin").build(), 0, 10); + assertThat(result).hasSize(2); - result = dao.selectMembers(session, UserMembershipQuery.builder().groupId(100L).memberSearch("AdMiN").build(), 0, 10); - assertThat(result).hasSize(2); - } finally { - session.close(); - } + result = dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(100L).memberSearch("AdMiN").build(), 0, 10); + assertThat(result).hasSize(2); } @@ -310,38 +274,26 @@ public class GroupMembershipDaoTest { public void should_be_sorted_by_user_name() { dbTester.prepareDbUnit(getClass(), "shared_plus_empty_group.xml"); - DbSession session = dbTester.myBatis().openSession(false); - - try { - List result = dao.selectMembers(session, UserMembershipQuery.builder().groupId(100L).build(), 0, 10); - assertThat(result).hasSize(2); - assertThat(result.get(0).getName()).isEqualTo("Admin"); - assertThat(result.get(1).getName()).isEqualTo("Not Admin"); - } finally { - session.close(); - } + List result = dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(100L).build(), 0, 10); + assertThat(result).hasSize(2); + assertThat(result.get(0).getName()).isEqualTo("Admin"); + assertThat(result.get(1).getName()).isEqualTo("Not Admin"); } @Test public void members_should_be_paginated() { dbTester.prepareDbUnit(getClass(), "shared_plus_empty_group.xml"); - DbSession session = dbTester.myBatis().openSession(false); - - try { - List result = dao.selectMembers(session, UserMembershipQuery.builder().groupId(100L).build(), 0, 2); - assertThat(result).hasSize(2); - assertThat(result.get(0).getName()).isEqualTo("Admin"); - assertThat(result.get(1).getName()).isEqualTo("Not Admin"); + List result = dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(100L).build(), 0, 2); + assertThat(result).hasSize(2); + assertThat(result.get(0).getName()).isEqualTo("Admin"); + assertThat(result.get(1).getName()).isEqualTo("Not Admin"); - result = dao.selectMembers(session, UserMembershipQuery.builder().groupId(100L).build(), 1, 2); - assertThat(result).hasSize(1); - assertThat(result.get(0).getName()).isEqualTo("Not Admin"); + result = dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(100L).build(), 1, 2); + assertThat(result).hasSize(1); + assertThat(result.get(0).getName()).isEqualTo("Not Admin"); - result = dao.selectMembers(session, UserMembershipQuery.builder().groupId(100L).build(), 2, 1); - assertThat(result).isEmpty(); - } finally { - session.close(); - } + result = dao.selectMembers(dbSession, UserMembershipQuery.builder().groupId(100L).build(), 2, 1); + assertThat(result).isEmpty(); } } -- 2.39.5