SONAR-7837 Clean up PermissionTemplateDao code

3 files changed, 38 insertions, 66 deletions

diff --git a/sonar-db/src/main/java/org/sonar/db/permission/template/PermissionTemplateDao.java b/sonar-db/src/main/java/org/sonar/db/permission/template/PermissionTemplateDao.java
index 27608d23acf..667b708533c 100644
--- a/sonar-db/src/main/java/org/sonar/db/permission/template/PermissionTemplateDao.java
+++ b/sonar-db/src/main/java/org/sonar/db/permission/template/PermissionTemplateDao.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.db.permission.template;
 
-import com.google.common.annotations.VisibleForTesting;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
@@ -30,9 +29,7 @@ import javax.annotation.Nullable;
 import org.apache.ibatis.session.ResultHandler;
 import org.apache.ibatis.session.RowBounds;
 import org.apache.ibatis.session.SqlSession;
-import org.sonar.api.security.DefaultGroups;
 import org.sonar.api.utils.System2;
-import org.sonar.api.web.UserRole;
 import org.sonar.db.Dao;
 import org.sonar.db.DbSession;
 import org.sonar.db.MyBatis;
@@ -41,14 +38,13 @@ import org.sonar.db.permission.GroupWithPermissionDto;
 import org.sonar.db.permission.OldPermissionQuery;
 import org.sonar.db.permission.UserWithPermissionDto;
 
-import static com.google.common.collect.Maps.newHashMap;
 import static java.lang.String.format;
+import static org.sonar.api.security.DefaultGroups.ANYONE;
+import static org.sonar.api.web.UserRole.ADMIN;
 import static org.sonar.db.DatabaseUtils.executeLargeInputsWithoutOutput;
 
 public class PermissionTemplateDao implements Dao {
 
-  public static final String QUERY_PARAMETER = "query";
-  public static final String TEMPLATE_ID_PARAMETER = "templateId";
   private static final String ANYONE_GROUP_PARAMETER = "anyoneGroup";
 
   private final MyBatis myBatis;
@@ -62,35 +58,12 @@ public class PermissionTemplateDao implements Dao {
   /**
    * @return a paginated list of users.
    */
-  public List<UserWithPermissionDto> selectUsers(OldPermissionQuery query, Long templateId, int offset, int limit) {
-    DbSession session = myBatis.openSession(false);
-    try {
-      return selectUsers(session, query, templateId, offset, limit);
-    } finally {
-      MyBatis.closeQuietly(session);
-    }
-  }
-
-  /**
-   * @return a paginated list of users.
-   */
   public List<UserWithPermissionDto> selectUsers(DbSession session, OldPermissionQuery query, Long templateId, int offset, int limit) {
-    Map<String, Object> params = newHashMap();
-    params.put(QUERY_PARAMETER, query);
-    params.put(TEMPLATE_ID_PARAMETER, templateId);
-    return mapper(session).selectUsers(params, new RowBounds(offset, limit));
+    return mapper(session).selectUsers(query, templateId, new RowBounds(offset, limit));
   }
 
   public int countUsers(DbSession session, OldPermissionQuery query, Long templateId) {
-    Map<String, Object> params = newHashMap();
-    params.put(QUERY_PARAMETER, query);
-    params.put(TEMPLATE_ID_PARAMETER, templateId);
-    return mapper(session).countUsers(params);
-  }
-
-  @VisibleForTesting
-  List<UserWithPermissionDto> selectUsers(OldPermissionQuery query, Long templateId) {
-    return selectUsers(query, templateId, 0, Integer.MAX_VALUE);
+    return mapper(session).countUsers(query, templateId);
   }
 
   /**
@@ -103,8 +76,7 @@ public class PermissionTemplateDao implements Dao {
   }
 
   public List<GroupWithPermissionDto> selectGroups(DbSession session, OldPermissionQuery query, Long templateId, int offset, int limit) {
-    Map<String, Object> params = groupsParameters(query, templateId);
-    return mapper(session).selectGroups(params, new RowBounds(offset, limit));
+    return mapper(session).selectGroups(query, templateId, ANYONE, ADMIN, new RowBounds(offset, limit));
   }
 
   public List<GroupWithPermissionDto> selectGroups(OldPermissionQuery query, Long templateId) {
@@ -121,26 +93,13 @@ public class PermissionTemplateDao implements Dao {
   }
 
   private static int countGroups(DbSession session, OldPermissionQuery query, long templateId, @Nullable String groupName) {
-    Map<String, Object> parameters = groupsParameters(query, templateId);
-    if (groupName != null) {
-      parameters.put("groupName", groupName.toUpperCase(Locale.ENGLISH));
-    }
-    return mapper(session).countGroups(parameters);
+    return mapper(session).countGroups(query, templateId, ANYONE, ADMIN, groupName != null ? groupName.toUpperCase(Locale.ENGLISH) : null);
   }
 
   public boolean hasGroup(DbSession session, OldPermissionQuery query, long templateId, String groupName) {
     return countGroups(session, query, templateId, groupName) > 0;
   }
 
-  private static Map<String, Object> groupsParameters(OldPermissionQuery query, Long templateId) {
-    Map<String, Object> params = newHashMap();
-    params.put(QUERY_PARAMETER, query);
-    params.put(TEMPLATE_ID_PARAMETER, templateId);
-    params.put("anyoneGroup", DefaultGroups.ANYONE);
-    params.put("projectAdminPermission", UserRole.ADMIN);
-    return params;
-  }
-
   @CheckForNull
   public PermissionTemplateDto selectByUuid(DbSession session, String templateUuid) {
     return mapper(session).selectByUuid(templateUuid);
@@ -240,8 +199,7 @@ public class PermissionTemplateDao implements Dao {
    */
   public void groupsCountByTemplateIdAndPermission(DbSession dbSession, List<Long> templateIds, ResultHandler resultHandler) {
     Map<String, Object> parameters = new HashMap<>(2);
-    parameters.put(ANYONE_GROUP_PARAMETER, DefaultGroups.ANYONE);
-
+    parameters.put(ANYONE_GROUP_PARAMETER, ANYONE);
     executeLargeInputsWithoutOutput(
       templateIds,
       partitionedTemplateIds -> {
diff --git a/sonar-db/src/main/java/org/sonar/db/permission/template/PermissionTemplateMapper.java b/sonar-db/src/main/java/org/sonar/db/permission/template/PermissionTemplateMapper.java
index 12515af01e3..6f3e5e2ad12 100644
--- a/sonar-db/src/main/java/org/sonar/db/permission/template/PermissionTemplateMapper.java
+++ b/sonar-db/src/main/java/org/sonar/db/permission/template/PermissionTemplateMapper.java
@@ -26,6 +26,7 @@ import org.apache.ibatis.annotations.Param;
 import org.apache.ibatis.session.ResultHandler;
 import org.apache.ibatis.session.RowBounds;
 import org.sonar.db.permission.GroupWithPermissionDto;
+import org.sonar.db.permission.OldPermissionQuery;
 import org.sonar.db.permission.UserWithPermissionDto;
 
 /**
@@ -59,15 +60,17 @@ public interface PermissionTemplateMapper {
 
   void deleteByGroupId(long groupId);
 
-  List<GroupWithPermissionDto> selectGroups(Map<String, Object> params, RowBounds rowBounds);
+  List<GroupWithPermissionDto> selectGroups(@Param("query") OldPermissionQuery query, @Param("templateId") long templateId, @Param("anyoneGroup") String anyoneGroup,
+    @Param("projectAdminPermission") String projectAdminPermission, RowBounds rowBounds);
 
-  List<UserWithPermissionDto> selectUsers(Map<String, Object> params, RowBounds rowBounds);
+  List<UserWithPermissionDto> selectUsers(@Param("query") OldPermissionQuery query, @Param("templateId") long templateId, RowBounds rowBounds);
 
   PermissionTemplateDto selectByName(String name);
 
-  int countUsers(Map<String, Object> params);
+  int countUsers(@Param("query") OldPermissionQuery query, @Param("templateId") long templateId);
 
-  int countGroups(Map<String, Object> parameters);
+  int countGroups(@Param("query") OldPermissionQuery query, @Param("templateId") long templateId, @Param("anyoneGroup") String anyoneGroup,
+    @Param("projectAdminPermission") String projectAdminPermission, @Nullable @Param("groupName") String groupName);
 
   List<PermissionTemplateDto> selectAll(@Param("nameMatch") String nameMatch);
 
diff --git a/sonar-db/src/test/java/org/sonar/db/permission/template/UserWithPermissionTemplateDaoTest.java b/sonar-db/src/test/java/org/sonar/db/permission/template/UserWithPermissionTemplateDaoTest.java
index 6a32e78e415..aa54fa656fb 100644
--- a/sonar-db/src/test/java/org/sonar/db/permission/template/UserWithPermissionTemplateDaoTest.java
+++ b/sonar-db/src/test/java/org/sonar/db/permission/template/UserWithPermissionTemplateDaoTest.java
@@ -26,14 +26,13 @@ import javax.annotation.Nullable;
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.api.utils.System2;
+import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.permission.OldPermissionQuery;
 import org.sonar.db.permission.UserWithPermissionDto;
-import org.sonar.db.permission.template.PermissionTemplateDao;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-
 public class UserWithPermissionTemplateDaoTest {
 
   private static final Long TEMPLATE_ID = 50L;
@@ -41,6 +40,8 @@ public class UserWithPermissionTemplateDaoTest {
   @Rule
   public DbTester dbTester = DbTester.create(System2.INSTANCE);
 
+  DbSession dbSession = dbTester.getSession();
+
   PermissionTemplateDao dao = dbTester.getDbClient().permissionTemplateDao();
 
   @Test
@@ -48,7 +49,7 @@ public class UserWithPermissionTemplateDaoTest {
     dbTester.prepareDbUnit(getClass(), "users_with_permissions.xml");
 
     OldPermissionQuery query = OldPermissionQuery.builder().permission("user").build();
-    List<UserWithPermissionDto> result = dao.selectUsers(query, TEMPLATE_ID);
+    List<UserWithPermissionDto> result = dao.selectUsers(dbSession, query, TEMPLATE_ID, 0, 10);
     assertThat(result).hasSize(3);
 
     UserWithPermissionDto user1 = result.get(0);
@@ -72,7 +73,7 @@ public class UserWithPermissionTemplateDaoTest {
     dbTester.prepareDbUnit(getClass(), "users_with_permissions.xml");
 
     OldPermissionQuery query = OldPermissionQuery.builder().permission("user").build();
-    List<UserWithPermissionDto> result = dao.selectUsers(query, 999L);
+    List<UserWithPermissionDto> result = dao.selectUsers(dbSession, query, 999L, 0, 10);
     assertThat(result).hasSize(3);
 
     UserWithPermissionDto user1 = result.get(0);
@@ -90,7 +91,10 @@ public class UserWithPermissionTemplateDaoTest {
     dbTester.prepareDbUnit(getClass(), "users_with_permissions.xml");
 
     // user1 and user2 have permission user
-    assertThat(dao.selectUsers(OldPermissionQuery.builder().permission("user").membership(OldPermissionQuery.IN).build(), TEMPLATE_ID)).hasSize(2);
+    assertThat(dao.selectUsers(
+      dbSession,
+      OldPermissionQuery.builder().permission("user").membership(OldPermissionQuery.IN).build(),
+      TEMPLATE_ID, 0, 10)).hasSize(2);
   }
 
   @Test
@@ -98,7 +102,10 @@ public class UserWithPermissionTemplateDaoTest {
     dbTester.prepareDbUnit(getClass(), "users_with_permissions.xml");
 
     // Only user3 has not the user permission
-    assertThat(dao.selectUsers(OldPermissionQuery.builder().permission("user").membership(OldPermissionQuery.OUT).build(), TEMPLATE_ID)).hasSize(1);
+    assertThat(dao.selectUsers(
+      dbSession,
+      OldPermissionQuery.builder().permission("user").membership(OldPermissionQuery.OUT).build(),
+      TEMPLATE_ID, 0, 10)).hasSize(1);
   }
 
   @Test
@@ -106,7 +113,7 @@ public class UserWithPermissionTemplateDaoTest {
     dbTester.prepareDbUnit(getClass(), "select_only_enable_users.xml");
 
     OldPermissionQuery query = OldPermissionQuery.builder().permission("user").build();
-    List<UserWithPermissionDto> result = dao.selectUsers(query, 999L);
+    List<UserWithPermissionDto> result = dao.selectUsers(dbSession, query, 999L, 0, 10);
     assertThat(result).hasSize(3);
 
     // Disabled user should not be returned
@@ -122,11 +129,15 @@ public class UserWithPermissionTemplateDaoTest {
   public void search_by_user_name() {
     dbTester.prepareDbUnit(getClass(), "users_with_permissions.xml");
 
-    List<UserWithPermissionDto> result = dao.selectUsers(OldPermissionQuery.builder().permission("user").search("SEr1").build(), TEMPLATE_ID);
+    List<UserWithPermissionDto> result = dao.selectUsers(
+      dbSession,
+      OldPermissionQuery.builder().permission("user").search("SEr1").build(),
+      TEMPLATE_ID, 0, 10);
     assertThat(result).hasSize(1);
     assertThat(result.get(0).getName()).isEqualTo("User1");
 
-    result = dao.selectUsers(OldPermissionQuery.builder().permission("user").search("user").build(), TEMPLATE_ID);
+    result = dao.selectUsers(
+      dbSession, OldPermissionQuery.builder().permission("user").search("user").build(), TEMPLATE_ID, 0, 10);
     assertThat(result).hasSize(3);
   }
 
@@ -134,7 +145,7 @@ public class UserWithPermissionTemplateDaoTest {
   public void should_be_sorted_by_user_name() {
     dbTester.prepareDbUnit(getClass(), "users_with_permissions_should_be_sorted_by_user_name.xml");
 
-    List<UserWithPermissionDto> result = dao.selectUsers(OldPermissionQuery.builder().permission("user").build(), TEMPLATE_ID);
+    List<UserWithPermissionDto> result = dao.selectUsers(dbSession, OldPermissionQuery.builder().permission("user").build(), TEMPLATE_ID, 0, 10);
     assertThat(result).hasSize(3);
     assertThat(result.get(0).getName()).isEqualTo("User1");
     assertThat(result.get(1).getName()).isEqualTo("User2");
@@ -145,17 +156,17 @@ public class UserWithPermissionTemplateDaoTest {
   public void should_be_paginated() {
     dbTester.prepareDbUnit(getClass(), "users_with_permissions.xml");
 
-    List<UserWithPermissionDto> result = dao.selectUsers(OldPermissionQuery.builder().permission("user").build(), TEMPLATE_ID, 0, 2);
+    List<UserWithPermissionDto> result = dao.selectUsers(dbSession, OldPermissionQuery.builder().permission("user").build(), TEMPLATE_ID, 0, 2);
     assertThat(result).hasSize(2);
     assertThat(result.get(0).getName()).isEqualTo("User1");
     assertThat(result.get(1).getName()).isEqualTo("User2");
 
-    result = dao.selectUsers(OldPermissionQuery.builder().permission("user").build(), TEMPLATE_ID, 1, 2);
+    result = dao.selectUsers(dbSession, OldPermissionQuery.builder().permission("user").build(), TEMPLATE_ID, 1, 2);
     assertThat(result).hasSize(2);
     assertThat(result.get(0).getName()).isEqualTo("User2");
     assertThat(result.get(1).getName()).isEqualTo("User3");
 
-    result = dao.selectUsers(OldPermissionQuery.builder().permission("user").build(), TEMPLATE_ID, 2, 1);
+    result = dao.selectUsers(dbSession, OldPermissionQuery.builder().permission("user").build(), TEMPLATE_ID, 2, 1);
     assertThat(result).hasSize(1);
     assertThat(result.get(0).getName()).isEqualTo("User3");
   }