SONAR-13089 Fix permission template issue when lot of groups

* Remove PermissionQuery#template
* SONAR-13089 Fix api/templates/template_groups WS
* Add UTs for api/templates/template_users WS

5 files changed, 77 insertions, 27 deletions

diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/permission/PermissionQuery.java b/server/sonar-db-dao/src/main/java/org/sonar/db/permission/PermissionQuery.java
index 5fcc09aaa9b..e71aa775780 100644
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/permission/PermissionQuery.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/permission/PermissionQuery.java
@@ -50,7 +50,6 @@ public class PermissionQuery {
   // filter on project, else filter org permissions
   private final String componentUuid;
   private final Long componentId;
-  private final String template;
 
   // filter on login, email or name of users or groups
   private final String searchQuery;
@@ -70,7 +69,6 @@ public class PermissionQuery {
     this.withAtLeastOnePermission = builder.withAtLeastOnePermission;
     this.componentUuid = builder.componentUuid;
     this.componentId = builder.componentId;
-    this.template = builder.template;
     this.searchQuery = builder.searchQuery;
     this.searchQueryToSql = builder.searchQuery == null ? null : buildLikeValue(builder.searchQuery, WildcardPosition.BEFORE_AND_AFTER);
     this.searchQueryToSqlLowercase = searchQueryToSql == null ? null : searchQueryToSql.toLowerCase(Locale.ENGLISH);
@@ -91,12 +89,6 @@ public class PermissionQuery {
     return withAtLeastOnePermission;
   }
 
-  // TODO remove it, it should not be in the query, but set as a separate parameter
-  @Deprecated
-  public String template() {
-    return template;
-  }
-
   @CheckForNull
   public String getComponentUuid() {
     return componentUuid;
@@ -139,7 +131,6 @@ public class PermissionQuery {
     private String organizationUuid;
     private String componentUuid;
     private Long componentId;
-    private String template;
     private String searchQuery;
     private boolean withAtLeastOnePermission;
 
@@ -156,11 +147,6 @@ public class PermissionQuery {
       return this;
     }
 
-    public Builder setTemplate(@Nullable String template) {
-      this.template = template;
-      return this;
-    }
-
     public Builder setComponent(ComponentDto component) {
       return setComponent(component.uuid(), component.getId());
     }
diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/template/PermissionTemplateMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/template/PermissionTemplateMapper.xml
index 57ceee66ec0..a5cd5db918a 100644
--- a/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/template/PermissionTemplateMapper.xml
+++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/template/PermissionTemplateMapper.xml
@@ -139,8 +139,10 @@
 
   <sql id="userLoginsByQueryAndTemplate">
     FROM users u
-    LEFT JOIN perm_templates_users ptu ON ptu.user_id=u.id AND ptu.template_id=#{templateId}
-    INNER JOIN organization_members om ON u.id=om.user_id AND om.organization_uuid=#{query.organizationUuid}
+    LEFT JOIN perm_templates_users ptu ON ptu.user_id=u.id
+      AND ptu.template_id=#{templateId}
+    INNER JOIN organization_members om ON u.id=om.user_id
+      AND om.organization_uuid=#{query.organizationUuid}
     <where>
       u.active = ${_true}
       <if test="query.getSearchQueryToSql() != null">
@@ -179,6 +181,7 @@
     FROM groups g
     LEFT JOIN perm_templates_groups ptg ON
       ptg.group_id=g.id
+      AND ptg.template_id=#{templateId}
     where
       g.organization_uuid=#{query.organizationUuid,jdbcType=VARCHAR}
     UNION ALL
@@ -187,8 +190,11 @@
       'Anyone' AS name,
       ptg.permission_reference AS permission,
       ptg.template_id AS templateId
-    FROM perm_templates_groups ptg
+    FROM groups g
+    LEFT JOIN perm_templates_groups ptg ON
+      ptg.template_id=#{templateId}
     <where>
+      g.organization_uuid=#{query.organizationUuid,jdbcType=VARCHAR}
       <if test="query.withAtLeastOnePermission()">
         AND ptg.group_id IS NULL
       </if>
diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/GroupWithPermissionTemplateDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/GroupWithPermissionTemplateDaoTest.java
index 7bb1bbb9457..29165c65c62 100644
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/GroupWithPermissionTemplateDaoTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/GroupWithPermissionTemplateDaoTest.java
@@ -38,6 +38,7 @@ import static org.assertj.core.api.Assertions.tuple;
 import static org.sonar.api.web.UserRole.ADMIN;
 import static org.sonar.api.web.UserRole.USER;
 import static org.sonar.core.permission.GlobalPermissions.PROVISIONING;
+import static org.sonar.db.permission.PermissionQuery.DEFAULT_PAGE_SIZE;
 import static org.sonar.db.permission.PermissionQuery.builder;
 import static org.sonar.db.user.GroupTesting.newGroupDto;
 
@@ -91,7 +92,6 @@ public class GroupWithPermissionTemplateDaoTest {
   public void selectGroupNamesByQueryAndTemplate_is_ordering_results_by_groups_with_permission_then_by_name() {
     OrganizationDto organization = db.organizations().insert();
     PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(organization);
-
     GroupDto group1 = db.users().insertGroup(organization, "A");
     GroupDto group2 = db.users().insertGroup(organization, "B");
     GroupDto group3 = db.users().insertGroup(organization, "C");
@@ -104,6 +104,42 @@ public class GroupWithPermissionTemplateDaoTest {
   }
 
   @Test
+  public void selectGroupNamesByQueryAndTemplate_is_order_by_groups_with_permission_then_by_name_when_many_groups() {
+    OrganizationDto organization = db.organizations().insert();
+    PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(organization);
+    IntStream.rangeClosed(1, DEFAULT_PAGE_SIZE + 1).forEach(i -> {
+      db.users().insertGroup(organization, "Group-" + i);
+    });
+
+    String lastGroupName = "Group-" + (DEFAULT_PAGE_SIZE + 1);
+    permissionTemplateDbTester.addGroupToTemplate(template, db.users().selectGroup(organization, lastGroupName).get(), UserRole.USER);
+
+    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).build();
+    assertThat(underTest.selectGroupNamesByQueryAndTemplate(db.getSession(), query, template.getId()))
+      .hasSize(DEFAULT_PAGE_SIZE)
+      .startsWith("Anyone", lastGroupName, "Group-1");
+  }
+
+  @Test
+  public void selectGroupNamesByQueryAndTemplate_ignores_other_template_and_is_ordered_by_groups_with_permission_then_by_name_when_many_groups() {
+    OrganizationDto organization = db.organizations().insert();
+    PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(organization);
+    PermissionTemplateDto otherTemplate = permissionTemplateDbTester.insertTemplate(organization);
+    IntStream.rangeClosed(1, DEFAULT_PAGE_SIZE + 1).forEach(i -> {
+      GroupDto group = db.users().insertGroup(organization, "Group-" + i);
+      permissionTemplateDbTester.addGroupToTemplate(otherTemplate, group, UserRole.USER);
+    });
+
+    String lastGroupName = "Group-" + (DEFAULT_PAGE_SIZE + 1);
+    permissionTemplateDbTester.addGroupToTemplate(template, db.users().selectGroup(organization, lastGroupName).get(), UserRole.USER);
+
+    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).build();
+    assertThat(underTest.selectGroupNamesByQueryAndTemplate(db.getSession(), query, template.getId()))
+      .hasSize(DEFAULT_PAGE_SIZE)
+      .startsWith("Anyone", lastGroupName, "Group-1");
+  }
+
+  @Test
   public void select_group_names_by_query_and_template_is_paginated() {
     OrganizationDto organization = db.organizations().insert();
     IntStream.rangeClosed(0, 9).forEach(i -> db.users().insertGroup(organization, i + "-name"));
@@ -121,7 +157,7 @@ public class GroupWithPermissionTemplateDaoTest {
     OrganizationDto organization = db.organizations().insert();
     PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(organization);
 
-    GroupDto group = db.users().insertGroup(newGroupDto().setName("Group"));
+    GroupDto group = db.users().insertGroup(organization, "Group");
     PermissionTemplateDto otherTemplate = permissionTemplateDbTester.insertTemplate(organization);
     permissionTemplateDbTester.addGroupToTemplate(otherTemplate.getId(), group.getId(), USER);
 
diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/PermissionTemplateDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/PermissionTemplateDaoTest.java
index b9bcc3381e8..6839386155c 100644
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/PermissionTemplateDaoTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/PermissionTemplateDaoTest.java
@@ -54,7 +54,6 @@ public class PermissionTemplateDaoTest {
 
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
-
   @Rule
   public DbTester db = DbTester.create();
 
@@ -70,7 +69,7 @@ public class PermissionTemplateDaoTest {
   }
 
   @Test
-  public void should_create_permission_template() {
+  public void create_permission_template() {
     PermissionTemplateDto permissionTemplate = underTest.insert(db.getSession(), newPermissionTemplateDto()
       .setUuid("ABCD")
       .setName("my template")
@@ -88,7 +87,7 @@ public class PermissionTemplateDaoTest {
   }
 
   @Test
-  public void should_select_permission_template_by_uuid() {
+  public void select_permission_template_by_uuid() {
     templateDb.insertTemplate(newPermissionTemplateDto()
       .setUuid("ABCD")
       .setName("my template")
@@ -169,7 +168,7 @@ public class PermissionTemplateDaoTest {
   }
 
   @Test
-  public void should_delete_permission_template() {
+  public void delete_permission_template() {
     UserDto user1 = db.users().insertUser();
     UserDto user2 = db.users().insertUser();
     GroupDto group1 = db.users().insertGroup();
@@ -203,7 +202,7 @@ public class PermissionTemplateDaoTest {
   }
 
   @Test
-  public void should_add_user_permission_to_template() {
+  public void add_user_permission_to_template() {
     PermissionTemplateDto permissionTemplate = templateDb.insertTemplate(db.getDefaultOrganization());
     UserDto user = db.users().insertUser();
 
@@ -216,7 +215,7 @@ public class PermissionTemplateDaoTest {
   }
 
   @Test
-  public void should_remove_user_permission_from_template() {
+  public void remove_user_permission_from_template() {
     PermissionTemplateDto permissionTemplate = templateDb.insertTemplate(db.getDefaultOrganization());
     UserDto user1 = db.users().insertUser();
     UserDto user2 = db.users().insertUser();
@@ -232,7 +231,7 @@ public class PermissionTemplateDaoTest {
   }
 
   @Test
-  public void should_add_group_permission_to_template() {
+  public void add_group_permission_to_template() {
     PermissionTemplateDto permissionTemplate = templateDb.insertTemplate(db.getDefaultOrganization());
     GroupDto group = db.users().insertGroup();
 
@@ -264,7 +263,7 @@ public class PermissionTemplateDaoTest {
   }
 
   @Test
-  public void should_add_group_permission_to_anyone() {
+  public void add_group_permission_to_anyone() {
     PermissionTemplateDto permissionTemplate = templateDb.insertTemplate(db.getDefaultOrganization());
 
     underTest.insertGroupPermission(dbSession, permissionTemplate.getId(), null, "user");
diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/UserWithPermissionTemplateDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/UserWithPermissionTemplateDaoTest.java
index 2944a8f507f..22e6cf453ba 100644
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/UserWithPermissionTemplateDaoTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/UserWithPermissionTemplateDaoTest.java
@@ -20,6 +20,7 @@
 package org.sonar.db.permission.template;
 
 import java.util.Collections;
+import java.util.stream.IntStream;
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.api.utils.System2;
@@ -37,6 +38,7 @@ import static org.assertj.core.api.Assertions.tuple;
 import static org.sonar.api.web.UserRole.ADMIN;
 import static org.sonar.api.web.UserRole.CODEVIEWER;
 import static org.sonar.api.web.UserRole.USER;
+import static org.sonar.db.permission.PermissionQuery.DEFAULT_PAGE_SIZE;
 import static org.sonar.db.permission.PermissionQuery.builder;
 
 public class UserWithPermissionTemplateDaoTest {
@@ -160,6 +162,26 @@ public class UserWithPermissionTemplateDaoTest {
   }
 
   @Test
+  public void selectUserLoginsByQueryAndTemplate_is_order_by_groups_with_permission_when_many_users() {
+    OrganizationDto organization = db.organizations().insert();
+    PermissionTemplateDto template = db.permissionTemplates().insertTemplate(organization);
+    // Add another template having some users with permission to make sure it's correctly ignored
+    PermissionTemplateDto otherTemplate = db.permissionTemplates().insertTemplate(organization);
+    IntStream.rangeClosed(1, DEFAULT_PAGE_SIZE + 1).forEach(i -> {
+      UserDto user = db.users().insertUser("User-" + i);
+      db.organizations().addMember(organization, user);
+      db.permissionTemplates().addUserToTemplate(otherTemplate, user, UserRole.USER);
+    });
+    String lastLogin = "User-" + (DEFAULT_PAGE_SIZE + 1);
+    db.permissionTemplates().addUserToTemplate(template, db.users().selectUserByLogin(lastLogin).get(), UserRole.USER);
+
+    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).build();
+    assertThat(underTest.selectUserLoginsByQueryAndTemplate(db.getSession(), query, template.getId()))
+      .hasSize(DEFAULT_PAGE_SIZE)
+      .startsWith(lastLogin);
+  }
+
+  @Test
   public void should_be_paginated() {
     OrganizationDto organization = db.organizations().insert();
     UserDto user1 = db.users().insertUser(u -> u.setName("User1"));
@@ -234,4 +256,5 @@ public class UserWithPermissionTemplateDaoTest {
     assertThat(underTest.selectUserPermissionsByTemplateIdAndUserLogins(dbSession, permissionTemplate.getId(), Collections.emptyList())).isEmpty();
     assertThat(underTest.selectUserPermissionsByTemplateIdAndUserLogins(dbSession, 123L, singletonList(user1.getLogin()))).isEmpty();
   }
+
 }