Use systematically a DbSession parameter in PermissionFinder

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<UserWithPermissionDto> 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<GroupWithPermission> 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<ComponentDto> 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<UserWithPermission> 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<Integer> argumentOffset = ArgumentCaptor.forClass(Integer.class);
     ArgumentCaptor<Integer> 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<Integer> argumentOffset = ArgumentCaptor.forClass(Integer.class);
     ArgumentCaptor<Integer> 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<Integer> argumentOffset = ArgumentCaptor.forClass(Integer.class);
     ArgumentCaptor<Integer> 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");
     }
   }
-
 }