From e7af578fb13af943b5beb6f10299cdd5ec4d876e Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Mon, 7 Sep 2015 17:39:26 +0200 Subject: [PATCH] Use systematically a DbSession parameter in PermissionFinder --- .../server/permission/PermissionFinder.java | 43 ++++---------- .../server/permission/ws/GroupsAction.java | 6 +- .../server/permission/ws/UsersAction.java | 6 +- .../permission/PermissionFinderTest.java | 59 ++++++++----------- 4 files changed, 39 insertions(+), 75 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionFinder.java b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionFinder.java index 393d594d963..fc13eee3374 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionFinder.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionFinder.java @@ -51,67 +51,44 @@ import static org.sonar.api.utils.Paging.forPageIndex; @ServerSide public class PermissionFinder { - private final DbClient dbClient; - private final PermissionDao permissionDao; private final ResourceDao resourceDao; private final PermissionTemplateDao permissionTemplateDao; public PermissionFinder(DbClient dbClient) { - this.dbClient = dbClient; this.resourceDao = dbClient.resourceDao(); this.permissionDao = dbClient.permissionDao(); this.permissionTemplateDao = dbClient.permissionTemplateDao(); } - public UserWithPermissionQueryResult findUsersWithPermission(PermissionQuery query) { + public UserWithPermissionQueryResult findUsersWithPermission(DbSession dbSession, PermissionQuery query) { Long componentId = componentId(query.component()); int limit = query.pageSize(); - DbSession dbSession = dbClient.openSession(false); - try { - int total = permissionDao.countUsers(dbSession, query, componentId); - return toUserQueryResult(permissionDao.selectUsers(dbSession, query, componentId, offset(query), limit), total); - } finally { - dbClient.closeSession(dbSession); - } + int total = permissionDao.countUsers(dbSession, query, componentId); + return toUserQueryResult(permissionDao.selectUsers(dbSession, query, componentId, offset(query), limit), total); } - public UserWithPermissionQueryResult findUsersWithPermissionTemplate(PermissionQuery query) { + public UserWithPermissionQueryResult findUsersWithPermissionTemplate(DbSession dbSession, PermissionQuery query) { Long permissionTemplateId = templateId(query.template()); int limit = query.pageSize(); - DbSession dbSession = dbClient.openSession(false); - try { - int total = permissionTemplateDao.countUsers(dbSession, query, permissionTemplateId); - return toUserQueryResult(permissionTemplateDao.selectUsers(dbSession, query, permissionTemplateId, offset(query), limit), total); - } finally { - dbClient.closeSession(dbSession); - } + int total = permissionTemplateDao.countUsers(dbSession, query, permissionTemplateId); + return toUserQueryResult(permissionTemplateDao.selectUsers(dbSession, query, permissionTemplateId, offset(query), limit), total); } /** * Paging for groups search is done in Java in order to correctly handle the 'Anyone' group */ - public GroupWithPermissionQueryResult findGroupsWithPermission(PermissionQuery query) { + public GroupWithPermissionQueryResult findGroupsWithPermission(DbSession dbSession, PermissionQuery query) { Long componentId = componentId(query.component()); - DbSession dbSession = dbClient.openSession(false); - try { - return toGroupQueryResult(permissionDao.selectGroups(dbSession, query, componentId), query); - } finally { - dbClient.closeSession(dbSession); - } + return toGroupQueryResult(permissionDao.selectGroups(dbSession, query, componentId), query); } /** * Paging for groups search is done in Java in order to correctly handle the 'Anyone' group */ - public GroupWithPermissionQueryResult findGroupsWithPermissionTemplate(PermissionQuery query) { + public GroupWithPermissionQueryResult findGroupsWithPermissionTemplate(DbSession dbSession, PermissionQuery query) { Long permissionTemplateId = templateId(query.template()); - DbSession dbSession = dbClient.openSession(false); - try { - return toGroupQueryResult(permissionTemplateDao.selectGroups(dbSession, query, permissionTemplateId), query); - } finally { - dbClient.closeSession(dbSession); - } + return toGroupQueryResult(permissionTemplateDao.selectGroups(dbSession, query, permissionTemplateId), query); } private static UserWithPermissionQueryResult toUserQueryResult(List dtos, int total) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/GroupsAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/GroupsAction.java index 198b3da2355..7cddc442a8e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/GroupsAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/GroupsAction.java @@ -89,7 +89,8 @@ public class GroupsAction implements PermissionsWsAction { checkProjectAdminUserByComponentDto(userSession, project); PermissionQuery permissionQuery = buildPermissionQuery(request, project); - WsGroupsResponse groupsResponse = buildResponse(permissionQuery, request); + GroupWithPermissionQueryResult groupsResult = permissionFinder.findGroupsWithPermission(dbSession, permissionQuery); + WsGroupsResponse groupsResponse = buildResponse(groupsResult, request); writeProtobuf(groupsResponse, wsRequest, wsResponse); } finally { @@ -97,8 +98,7 @@ public class GroupsAction implements PermissionsWsAction { } } - private WsGroupsResponse buildResponse(PermissionQuery permissionQuery, PermissionRequest permissionRequest) { - GroupWithPermissionQueryResult groupsResult = permissionFinder.findGroupsWithPermission(permissionQuery); + private WsGroupsResponse buildResponse(GroupWithPermissionQueryResult groupsResult, PermissionRequest permissionRequest) { List groupsWithPermission = groupsResult.groups(); WsGroupsResponse.Builder groupsResponse = WsGroupsResponse.newBuilder(); 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 83d48ebfb6e..c359deacfa6 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 @@ -89,7 +89,8 @@ public class UsersAction implements PermissionsWsAction { Optional project = dependenciesFinder.searchProject(dbSession, request); checkProjectAdminUserByComponentDto(userSession, project); PermissionQuery permissionQuery = buildPermissionQuery(request, project); - WsUsersResponse wsUsersResponse = wsUsersResponse(permissionQuery, request.page(), request.pageSize()); + UserWithPermissionQueryResult usersResult = permissionFinder.findUsersWithPermission(dbSession, permissionQuery); + WsUsersResponse wsUsersResponse = buildResponse(usersResult, request.page(), request.pageSize()); writeProtobuf(wsUsersResponse, wsRequest, wsResponse); } finally { @@ -97,8 +98,7 @@ public class UsersAction implements PermissionsWsAction { } } - private WsUsersResponse wsUsersResponse(PermissionQuery permissionQuery, int page, int pageSize) { - UserWithPermissionQueryResult usersResult = permissionFinder.findUsersWithPermission(permissionQuery); + private WsUsersResponse buildResponse(UserWithPermissionQueryResult usersResult, int page, int pageSize) { List usersWithPermission = usersResult.users(); WsUsersResponse.Builder userResponse = WsUsersResponse.newBuilder(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionFinderTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionFinderTest.java index a3d6ae9d177..5c350e0a857 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionFinderTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionFinderTest.java @@ -22,10 +22,7 @@ package org.sonar.server.permission; import java.util.List; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; import org.sonar.core.permission.GroupWithPermission; import org.sonar.db.DbClient; import org.sonar.db.DbSession; @@ -51,17 +48,12 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -@RunWith(MockitoJUnitRunner.class) public class PermissionFinderTest { - @Mock - PermissionDao permissionDao; - - @Mock - ResourceDao resourceDao; - - @Mock - PermissionTemplateDao permissionTemplateDao; + PermissionDao permissionDao = mock(PermissionDao.class); + ResourceDao resourceDao = mock(ResourceDao.class); + PermissionTemplateDao permissionTemplateDao = mock(PermissionTemplateDao.class); + DbSession dbSession = mock(DbSession.class); PermissionFinder underTest; @@ -82,7 +74,7 @@ public class PermissionFinderTest { ); when(permissionDao.countUsers(any(DbSession.class), any(PermissionQuery.class), anyLong())).thenReturn(1); - UserWithPermissionQueryResult result = underTest.findUsersWithPermission(PermissionQuery.builder().permission("user").build()); + UserWithPermissionQueryResult result = underTest.findUsersWithPermission(dbSession, PermissionQuery.builder().permission("user").build()); assertThat(result.users()).hasSize(1); assertThat(result.total()).isEqualTo(1); } @@ -92,7 +84,7 @@ public class PermissionFinderTest { when(resourceDao.selectResource(any(ResourceQuery.class))).thenReturn(null); try { - underTest.findUsersWithPermission(PermissionQuery.builder().permission("user").component("Unknown").build()); + underTest.findUsersWithPermission(dbSession, PermissionQuery.builder().permission("user").component("Unknown").build()); fail(); } catch (Exception e) { assertThat(e).isInstanceOf(NotFoundException.class).hasMessage("Project 'Unknown' does not exist"); @@ -101,7 +93,7 @@ public class PermissionFinderTest { @Test public void find_users_with_paging() { - underTest.findUsersWithPermission(PermissionQuery.builder().permission("user").pageIndex(3).pageSize(10).build()); + underTest.findUsersWithPermission(dbSession, PermissionQuery.builder().permission("user").pageIndex(3).pageSize(10).build()); ArgumentCaptor argumentOffset = ArgumentCaptor.forClass(Integer.class); ArgumentCaptor argumentLimit = ArgumentCaptor.forClass(Integer.class); @@ -119,7 +111,7 @@ public class PermissionFinderTest { new UserWithPermissionDto().setName("user3").setPermission("user")) ); when(permissionDao.countUsers(any(DbSession.class), any(PermissionQuery.class), anyLong())).thenReturn(3); - UserWithPermissionQueryResult result = underTest.findUsersWithPermission(PermissionQuery.builder().permission("user").pageIndex(1).pageSize(2).build()); + UserWithPermissionQueryResult result = underTest.findUsersWithPermission(dbSession, PermissionQuery.builder().permission("user").pageIndex(1).pageSize(2).build()); ArgumentCaptor argumentOffset = ArgumentCaptor.forClass(Integer.class); ArgumentCaptor argumentLimit = ArgumentCaptor.forClass(Integer.class); @@ -140,7 +132,7 @@ public class PermissionFinderTest { ); when(permissionDao.countUsers(any(DbSession.class), any(PermissionQuery.class), anyLong())).thenReturn(4); - UserWithPermissionQueryResult result = underTest.findUsersWithPermission(PermissionQuery.builder().permission("user").pageIndex(1).pageSize(10).build()); + UserWithPermissionQueryResult result = underTest.findUsersWithPermission(dbSession, PermissionQuery.builder().permission("user").pageIndex(1).pageSize(10).build()); ArgumentCaptor argumentOffset = ArgumentCaptor.forClass(Integer.class); ArgumentCaptor argumentLimit = ArgumentCaptor.forClass(Integer.class); @@ -157,8 +149,7 @@ public class PermissionFinderTest { newArrayList(new GroupWithPermissionDto().setName("users").setPermission("user")) ); - GroupWithPermissionQueryResult result = underTest.findGroupsWithPermission( - PermissionQuery.builder().permission("user").membership(PermissionQuery.IN).build()); + GroupWithPermissionQueryResult result = underTest.findGroupsWithPermission(dbSession, PermissionQuery.builder().permission("user").membership(PermissionQuery.IN).build()); assertThat(result.groups()).hasSize(1); } @@ -173,7 +164,7 @@ public class PermissionFinderTest { new GroupWithPermissionDto().setName("Other").setPermission(null) )); - GroupWithPermissionQueryResult result = underTest.findGroupsWithPermission( + GroupWithPermissionQueryResult result = underTest.findGroupsWithPermission(dbSession, PermissionQuery.builder() .permission("user") .pageSize(2) @@ -196,12 +187,9 @@ public class PermissionFinderTest { new GroupWithPermissionDto().setName("Other").setPermission(null) )); - assertThat(underTest.findGroupsWithPermission( - PermissionQuery.builder().permission("user").membership(PermissionQuery.IN).build()).groups()).hasSize(2); - assertThat(underTest.findGroupsWithPermission( - PermissionQuery.builder().permission("user").membership(PermissionQuery.OUT).build()).groups()).hasSize(3); - assertThat(underTest.findGroupsWithPermission( - PermissionQuery.builder().permission("user").membership(PermissionQuery.ANY).build()).groups()).hasSize(5); + assertThat(underTest.findGroupsWithPermission(dbSession, PermissionQuery.builder().permission("user").membership(PermissionQuery.IN).build()).groups()).hasSize(2); + assertThat(underTest.findGroupsWithPermission(dbSession, PermissionQuery.builder().permission("user").membership(PermissionQuery.OUT).build()).groups()).hasSize(3); + assertThat(underTest.findGroupsWithPermission(dbSession, PermissionQuery.builder().permission("user").membership(PermissionQuery.ANY).build()).groups()).hasSize(5); } @Test @@ -210,8 +198,8 @@ public class PermissionFinderTest { newArrayList(new GroupWithPermissionDto().setName("users").setPermission("user")) ); - GroupWithPermissionQueryResult result = underTest.findGroupsWithPermission(PermissionQuery.builder().permission("user") - .pageIndex(1).membership(PermissionQuery.ANY).build()); + GroupWithPermissionQueryResult result = underTest.findGroupsWithPermission(dbSession, PermissionQuery.builder().permission("user").pageIndex(1).membership(PermissionQuery.ANY) + .build()); assertThat(result.groups()).hasSize(2); GroupWithPermission first = result.groups().get(0); assertThat(first.name()).isEqualTo("Anyone"); @@ -224,7 +212,7 @@ public class PermissionFinderTest { newArrayList(new GroupWithPermissionDto().setName("users").setPermission("user")) ); - GroupWithPermissionQueryResult result = underTest.findGroupsWithPermission(PermissionQuery.builder().permission("user").search("other") + GroupWithPermissionQueryResult result = underTest.findGroupsWithPermission(dbSession, PermissionQuery.builder().permission("user").search("other") .pageIndex(1).membership(PermissionQuery.ANY).build()); // Anyone group should not be added assertThat(result.groups()).hasSize(1); @@ -236,7 +224,7 @@ public class PermissionFinderTest { newArrayList(new GroupWithPermissionDto().setName("MyAnyGroup").setPermission("user")) ); - GroupWithPermissionQueryResult result = underTest.findGroupsWithPermission(PermissionQuery.builder().permission("user").search("any") + GroupWithPermissionQueryResult result = underTest.findGroupsWithPermission(dbSession, PermissionQuery.builder().permission("user").search("any") .pageIndex(1).membership(PermissionQuery.ANY).build()); assertThat(result.groups()).hasSize(2); } @@ -247,7 +235,7 @@ public class PermissionFinderTest { newArrayList(new GroupWithPermissionDto().setName("users").setPermission("user")) ); - GroupWithPermissionQueryResult result = underTest.findGroupsWithPermission(PermissionQuery.builder().permission("user") + GroupWithPermissionQueryResult result = underTest.findGroupsWithPermission(dbSession, PermissionQuery.builder().permission("user") .pageIndex(1).membership(PermissionQuery.OUT).build()); // Anyone group should not be added assertThat(result.groups()).hasSize(1); @@ -263,7 +251,7 @@ public class PermissionFinderTest { when(permissionTemplateDao.countUsers(any(DbSession.class), any(PermissionQuery.class), anyLong())).thenReturn(1); - UserWithPermissionQueryResult result = underTest.findUsersWithPermissionTemplate(PermissionQuery.builder().permission("user").template("my_template").build()); + UserWithPermissionQueryResult result = underTest.findUsersWithPermissionTemplate(dbSession, PermissionQuery.builder().permission("user").template("my_template").build()); assertThat(result.users()).hasSize(1); assertThat(result.total()).isEqualTo(1); } @@ -273,7 +261,7 @@ public class PermissionFinderTest { when(permissionTemplateDao.selectByUuid(anyString())).thenReturn(null); try { - underTest.findUsersWithPermissionTemplate(PermissionQuery.builder().permission("user").template("Unknown").build()); + underTest.findUsersWithPermissionTemplate(dbSession, PermissionQuery.builder().permission("user").template("Unknown").build()); fail(); } catch (Exception e) { assertThat(e).isInstanceOf(NotFoundException.class).hasMessage("Template 'Unknown' does not exist"); @@ -288,7 +276,7 @@ public class PermissionFinderTest { newArrayList(new GroupWithPermissionDto().setName("users").setPermission("user")) ); - GroupWithPermissionQueryResult result = underTest.findGroupsWithPermissionTemplate( + GroupWithPermissionQueryResult result = underTest.findGroupsWithPermissionTemplate(dbSession, PermissionQuery.builder().permission("user").template("my_template").membership(PermissionQuery.OUT).build()); assertThat(result.groups()).hasSize(1); } @@ -298,11 +286,10 @@ public class PermissionFinderTest { when(permissionTemplateDao.selectByUuid(anyString())).thenReturn(null); try { - underTest.findGroupsWithPermissionTemplate(PermissionQuery.builder().permission("user").template("Unknown").build()); + underTest.findGroupsWithPermissionTemplate(dbSession, PermissionQuery.builder().permission("user").template("Unknown").build()); fail(); } catch (Exception e) { assertThat(e).isInstanceOf(NotFoundException.class).hasMessage("Template 'Unknown' does not exist"); } } - } -- 2.39.5