SONAR-12666 Fix groups sorting when high number of groups

6 files changed, 147 insertions, 41 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 8eaab6b2b18..d82ddf94dc3 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
@@ -24,6 +24,7 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 import org.sonar.db.WildcardPosition;
+import org.sonar.db.component.ComponentDto;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
 import static com.google.common.base.Preconditions.checkArgument;
@@ -160,12 +161,12 @@ public class PermissionQuery {
       return this;
     }
 
-    public Builder setComponentUuid(String componentUuid) {
-      this.componentUuid = componentUuid;
-      return this;
+    public Builder setComponent(ComponentDto component) {
+      return setComponent(component.uuid(), component.getId());
     }
 
-    public Builder setComponentId(Long componentId) {
+    public Builder setComponent(String componentUuid, long componentId) {
+      this.componentUuid = componentUuid;
       this.componentId = componentId;
       return this;
     }
diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/GroupPermissionMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/GroupPermissionMapper.xml
index 0c66ab4a2ef..11cc02aca6c 100644
--- a/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/GroupPermissionMapper.xml
+++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/GroupPermissionMapper.xml
@@ -58,18 +58,31 @@
       select g.id as groupId, g.name as name, gr.role as permission, gr.resource_id as componentId, gr.id as id
       from groups g
       left join group_roles gr on g.id = gr.group_id
+      <if test="query.componentId == null">
+        and gr.resource_id is null
+      </if>
+      <if test="query.componentId != null">
+        and gr.resource_id = #{query.componentId,jdbcType=BIGINT}
+      </if>
       where
-      g.organization_uuid = #{query.organizationUuid,jdbcType=VARCHAR}
+        g.organization_uuid = #{query.organizationUuid,jdbcType=VARCHAR}
 
     union all
 
     select 0 as groupId, 'Anyone' as name, gr.role as permission, gr.resource_id as componentId, gr.id as id
     from group_roles gr
-    <if test="query.withAtLeastOnePermission()">
-      where
-      gr.organization_uuid = #{query.organizationUuid,jdbcType=VARCHAR} and
-      gr.group_id is null
-    </if>
+    <where>
+      <if test="query.componentId == null">
+        and gr.resource_id is null
+      </if>
+      <if test="query.componentId != null">
+        and gr.resource_id = #{query.componentId,jdbcType=BIGINT}
+      </if>
+      <if test="query.withAtLeastOnePermission()">
+        and gr.organization_uuid = #{query.organizationUuid,jdbcType=VARCHAR} and
+        gr.group_id is null
+      </if>
+    </where>
 
     ) sub
     left join projects p on sub.componentId = p.id
diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/GroupPermissionDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/GroupPermissionDaoTest.java
index a996a1b8ea3..8f25059396d 100644
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/GroupPermissionDaoTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/GroupPermissionDaoTest.java
@@ -51,6 +51,7 @@ import static org.sonar.core.permission.GlobalPermissions.SCAN_EXECUTION;
 import static org.sonar.db.permission.OrganizationPermission.ADMINISTER;
 import static org.sonar.db.permission.OrganizationPermission.PROVISION_PROJECTS;
 import static org.sonar.db.permission.OrganizationPermission.SCAN;
+import static org.sonar.db.permission.PermissionQuery.DEFAULT_PAGE_SIZE;
 
 public class GroupPermissionDaoTest {
 
@@ -137,6 +138,44 @@ public class GroupPermissionDaoTest {
   }
 
   @Test
+  public void selectGroupNamesByQuery_is_ordered_by_permissions_then_by_group_when_many_groups_for_global_permissions() {
+    OrganizationDto organizationDto = db.organizations().insert();
+    ComponentDto project = db.components().insertPrivateProject(organizationDto);
+    IntStream.rangeClosed(1, DEFAULT_PAGE_SIZE + 1).forEach(i -> {
+      GroupDto group = db.users().insertGroup(organizationDto, "Group-" + i);
+      // Add permission on project to be sure projects are excluded
+      db.users().insertProjectPermissionOnGroup(group, SCAN.getKey(), project);
+    });
+    String lastGroupName = "Group-" + (DEFAULT_PAGE_SIZE + 1);
+    db.users().insertPermissionOnGroup(db.users().selectGroup(organizationDto, lastGroupName).get(), SCAN);
+
+    assertThat(underTest.selectGroupNamesByQuery(dbSession, newQuery()
+      .setOrganizationUuid(organizationDto.getUuid()).build()))
+        .hasSize(DEFAULT_PAGE_SIZE)
+        .startsWith(ANYONE, lastGroupName, "Group-1");
+  }
+
+  @Test
+  public void selectGroupNamesByQuery_is_ordered_by_global_permissions_then_by_group_when_many_groups_for_project_permissions() {
+    OrganizationDto organizationDto = db.organizations().insert();
+    IntStream.rangeClosed(1, DEFAULT_PAGE_SIZE + 1).forEach(i -> {
+      GroupDto group = db.users().insertGroup(organizationDto, "Group-" + i);
+      // Add global permission to be sure they are excluded
+      db.users().insertPermissionOnGroup(group, SCAN.getKey());
+    });
+    ComponentDto project = db.components().insertPrivateProject(organizationDto);
+    String lastGroupName = "Group-" + (DEFAULT_PAGE_SIZE + 1);
+    db.users().insertProjectPermissionOnGroup(db.users().selectGroup(organizationDto, lastGroupName).get(), SCAN.getKey(), project);
+
+    assertThat(underTest.selectGroupNamesByQuery(dbSession, newQuery()
+      .setOrganizationUuid(organizationDto.getUuid())
+      .setComponent(project)
+      .build()))
+        .hasSize(DEFAULT_PAGE_SIZE)
+        .startsWith(ANYONE, lastGroupName, "Group-1");
+  }
+
+  @Test
   public void countGroupsByQuery() {
     OrganizationDto organizationDto = db.getDefaultOrganization();
     GroupDto group1 = db.users().insertGroup(organizationDto, "Group-1");
@@ -200,7 +239,8 @@ public class GroupPermissionDaoTest {
     db.users().insertProjectPermissionOnGroup(group3, "p1", anotherProject);
     db.users().insertPermissionOnGroup(group2, "p5");
 
-    PermissionQuery.Builder builderOnComponent = newQuery().setComponentUuid(project.uuid());
+    PermissionQuery.Builder builderOnComponent = newQuery()
+      .setComponent(project);
     assertThat(underTest.selectGroupNamesByQuery(dbSession,
       builderOnComponent.withAtLeastOnePermission().build())).containsOnlyOnce(group1.getName());
     assertThat(underTest.selectGroupNamesByQuery(dbSession,
@@ -225,7 +265,8 @@ public class GroupPermissionDaoTest {
     db.users().insertProjectPermissionOnGroup(group3, UserRole.SCAN, anotherProject);
     db.users().insertPermissionOnGroup(group2, SCAN);
 
-    PermissionQuery.Builder builderOnComponent = newQuery().setComponentUuid(project.uuid());
+    PermissionQuery.Builder builderOnComponent = newQuery()
+      .setComponent(project);
     assertThat(underTest.selectGroupNamesByQuery(dbSession,
       builderOnComponent.withAtLeastOnePermission().build())).containsOnlyOnce(group1.getName());
     assertThat(underTest.selectGroupNamesByQuery(dbSession,
diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/PermissionQueryTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/PermissionQueryTest.java
index e0c15477d03..d59256bae54 100644
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/PermissionQueryTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/PermissionQueryTest.java
@@ -22,8 +22,12 @@ package org.sonar.db.permission;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.sonar.db.component.ComponentDto;
+import org.sonar.db.organization.OrganizationDto;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.db.component.ComponentTesting.newPublicProjectDto;
+import static org.sonar.db.organization.OrganizationTesting.newOrganizationDto;
 
 public class PermissionQueryTest {
 
@@ -32,41 +36,42 @@ public class PermissionQueryTest {
 
   @Test
   public void create_query() {
-    PermissionQuery quey = PermissionQuery.builder()
-      .setComponentUuid("COMPONENT_UUID")
-      .setComponentId(1234L)
+    OrganizationDto organization = newOrganizationDto();
+    ComponentDto project= newPublicProjectDto(organization);
+    PermissionQuery query = PermissionQuery.builder()
+      .setComponent(project)
       .setOrganizationUuid("ORGANIZATION_UUID")
       .setPermission("user")
       .setSearchQuery("sonar")
       .build();
 
-    assertThat(quey.getComponentUuid()).isEqualTo("COMPONENT_UUID");
-    assertThat(quey.getComponentId()).isEqualTo(1234L);
-    assertThat(quey.getOrganizationUuid()).isEqualTo("ORGANIZATION_UUID");
-    assertThat(quey.getPermission()).isEqualTo("user");
-    assertThat(quey.getSearchQuery()).isEqualTo("sonar");
+    assertThat(query.getComponentUuid()).isEqualTo(project.uuid());
+    assertThat(query.getComponentId()).isEqualTo(project.getId());
+    assertThat(query.getOrganizationUuid()).isEqualTo("ORGANIZATION_UUID");
+    assertThat(query.getPermission()).isEqualTo("user");
+    assertThat(query.getSearchQuery()).isEqualTo("sonar");
   }
 
   @Test
   public void create_query_with_pagination() {
-    PermissionQuery quey = PermissionQuery.builder()
+    PermissionQuery query = PermissionQuery.builder()
       .setOrganizationUuid("ORGANIZATION_UUID")
       .setPageSize(10)
       .setPageIndex(5)
       .build();
 
-    assertThat(quey.getPageOffset()).isEqualTo(40);
-    assertThat(quey.getPageSize()).isEqualTo(10);
+    assertThat(query.getPageOffset()).isEqualTo(40);
+    assertThat(query.getPageSize()).isEqualTo(10);
   }
 
   @Test
   public void create_query_with_default_pagination() {
-    PermissionQuery quey = PermissionQuery.builder()
+    PermissionQuery query = PermissionQuery.builder()
       .setOrganizationUuid("ORGANIZATION_UUID")
       .build();
 
-    assertThat(quey.getPageOffset()).isEqualTo(0);
-    assertThat(quey.getPageSize()).isEqualTo(20);
+    assertThat(query.getPageOffset()).isEqualTo(0);
+    assertThat(query.getPageSize()).isEqualTo(20);
   }
 
   @Test
diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/UserPermissionDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/UserPermissionDaoTest.java
index 21538a36575..b78fa9cf962 100644
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/UserPermissionDaoTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/UserPermissionDaoTest.java
@@ -26,6 +26,7 @@ import java.util.List;
 import java.util.Random;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 import org.assertj.core.groups.Tuple;
 import org.junit.Rule;
 import org.junit.Test;
@@ -48,11 +49,14 @@ import static org.sonar.api.web.UserRole.CODEVIEWER;
 import static org.sonar.api.web.UserRole.ISSUE_ADMIN;
 import static org.sonar.api.web.UserRole.USER;
 import static org.sonar.core.permission.GlobalPermissions.PROVISIONING;
+import static org.sonar.core.permission.GlobalPermissions.QUALITY_PROFILE_ADMIN;
 import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN;
+import static org.sonar.db.component.ComponentTesting.newPrivateProjectDto;
 import static org.sonar.db.permission.OrganizationPermission.ADMINISTER;
 import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_GATES;
 import static org.sonar.db.permission.OrganizationPermission.PROVISION_PROJECTS;
 import static org.sonar.db.permission.OrganizationPermission.SCAN;
+import static org.sonar.db.permission.PermissionQuery.DEFAULT_PAGE_SIZE;
 
 public class UserPermissionDaoTest {
 
@@ -132,27 +136,27 @@ public class UserPermissionDaoTest {
     addProjectPermission(organization, ISSUE_ADMIN, user3, project2);
 
     // project permissions of users who has at least one permission on this project
-    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().setComponentUuid(project1.uuid()).build();
+    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().setComponent(project1).build();
     expectPermissions(query, asList(user2.getId(), user1.getId()), perm3, perm2, perm1);
 
     // empty if nobody has the specified global permission
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setPermission("missing").setComponentUuid(project1.uuid()).build();
+    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setPermission("missing").setComponent(project1).build();
     expectPermissions(query, emptyList());
 
     // search by user name (matches 2 users), users with at least one permission
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setSearchQuery("Mari").withAtLeastOnePermission().setComponentUuid(project1.uuid()).build();
+    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setSearchQuery("Mari").withAtLeastOnePermission().setComponent(project1).build();
     expectPermissions(query, asList(user2.getId(), user1.getId()), perm3, perm2, perm1);
 
     // search by user name (matches 2 users) and project permission
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setSearchQuery("Mari").setPermission(ISSUE_ADMIN).setComponentUuid(project1.uuid()).build();
+    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setSearchQuery("Mari").setPermission(ISSUE_ADMIN).setComponent(project1).build();
     expectPermissions(query, asList(user2.getId(), user1.getId()), perm3, perm2);
 
     // search by user name (no match)
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setSearchQuery("Unknown").setComponentUuid(project1.uuid()).build();
+    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setSearchQuery("Unknown").setComponent(project1).build();
     expectPermissions(query, emptyList());
 
     // permissions of unknown project
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setComponentUuid("missing").withAtLeastOnePermission().build();
+    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setComponent(newPrivateProjectDto(organization)).withAtLeastOnePermission().build();
     expectPermissions(query, emptyList());
   }
 
@@ -164,16 +168,57 @@ public class UserPermissionDaoTest {
     UserDto user3 = insertUser(u -> u.setLogin("login3").setName("Z").setEmail("zanother3@another.com"), organization);
     UserDto user4 = insertUser(u -> u.setLogin("login4").setName("A").setEmail("zanother3@another.com"), organization);
     addGlobalPermission(organization, SYSTEM_ADMIN, user1);
-    ComponentDto project1 = db.components().insertPrivateProject(organization);
-    addProjectPermission(organization, USER, user2, project1);
+    addGlobalPermission(organization, QUALITY_PROFILE_ADMIN, user2);
 
     PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).build();
 
-    assertThat(underTest.selectUserIdsByQuery(dbSession, query))
+    assertThat(underTest.selectUserIdsByQueryAndScope(dbSession, query))
       .containsExactly(user2.getId(), user1.getId(), user4.getId(), user3.getId());
   }
 
   @Test
+  public void selectUserIdsByQuery_is_ordering_by_users_having_permissions_first_then_by_name_lowercase_when_high_number_of_users_for_global_permissions() {
+    OrganizationDto organization = db.organizations().insert();
+    ComponentDto project = db.components().insertPrivateProject(organization);
+    IntStream.rangeClosed(1, DEFAULT_PAGE_SIZE + 1).forEach(i -> {
+      UserDto user = insertUser(u -> u.setLogin("login" + i).setName("" + i), organization);
+      // Add permission on project to be sure projects are excluded
+      db.users().insertProjectPermissionOnUser(user, SCAN.getKey(), project);
+    });
+    String lastLogin = "login" + (DEFAULT_PAGE_SIZE + 1);
+    UserDto lastUser = db.getDbClient().userDao().selectByLogin(dbSession, lastLogin);
+    addGlobalPermission(organization, SYSTEM_ADMIN, lastUser);
+
+    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).build();
+
+    assertThat(underTest.selectUserIdsByQueryAndScope(dbSession, query))
+      .hasSize(DEFAULT_PAGE_SIZE)
+      .startsWith(lastUser.getId());
+  }
+
+  @Test
+  public void selectUserIdsByQuery_is_ordering_by_users_having_permissions_first_then_by_name_lowercase_when_high_number_of_users_for_project_permissions() {
+    OrganizationDto organization = db.organizations().insert();
+    IntStream.rangeClosed(1, DEFAULT_PAGE_SIZE + 1).forEach(i -> {
+      UserDto user = insertUser(u -> u.setLogin("login" + i).setName("" + i), organization);
+      // Add global permission to be sure they are excluded
+      addGlobalPermission(organization, SYSTEM_ADMIN, user);
+    });
+    String lastLogin = "login" + (DEFAULT_PAGE_SIZE + 1);
+    UserDto lastUser = db.getDbClient().userDao().selectByLogin(dbSession, lastLogin);
+    ComponentDto project = db.components().insertPrivateProject(organization);
+    db.users().insertProjectPermissionOnUser(lastUser, SCAN.getKey(), project);
+
+    PermissionQuery query = PermissionQuery.builder()
+      .setComponent(project)
+      .setOrganizationUuid(organization.getUuid()).build();
+
+    assertThat(underTest.selectUserIdsByQueryAndScope(dbSession, query))
+      .hasSize(DEFAULT_PAGE_SIZE)
+      .startsWith(lastUser.getId());
+  }
+
+  @Test
   public void selectUserIdsByQuery_is_not_ordering_by_number_of_permissions() {
     OrganizationDto organization = db.organizations().insert();
     UserDto user1 = insertUser(u -> u.setLogin("login1").setName("Z").setEmail("email1@email.com"), organization);
@@ -234,13 +279,13 @@ public class UserPermissionDaoTest {
     addProjectPermission(org2, ISSUE_ADMIN, user2, project2);
 
     // logins are ordered by user name: user2 ("Marie") then user1 ("Marius")
-    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(project1.getOrganizationUuid()).setComponentUuid(project1.uuid()).withAtLeastOnePermission().build();
+    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(project1.getOrganizationUuid()).setComponent(project1).withAtLeastOnePermission().build();
     assertThat(underTest.selectUserIdsByQuery(dbSession, query)).containsExactly(user2.getId(), user1.getId());
-    query = PermissionQuery.builder().setOrganizationUuid("anotherOrg").setComponentUuid(project1.uuid()).withAtLeastOnePermission().build();
+    query = PermissionQuery.builder().setOrganizationUuid("anotherOrg").setComponent(project1).withAtLeastOnePermission().build();
     assertThat(underTest.selectUserIdsByQuery(dbSession, query)).isEmpty();
 
     // on a project without permissions
-    query = PermissionQuery.builder().setOrganizationUuid(org1.getUuid()).setComponentUuid("missing").withAtLeastOnePermission().build();
+    query = PermissionQuery.builder().setOrganizationUuid(org1.getUuid()).setComponent(newPrivateProjectDto(org1)).withAtLeastOnePermission().build();
     assertThat(underTest.selectUserIdsByQuery(dbSession, query)).isEmpty();
 
     // search all users whose name matches "mar", whatever the permissions
@@ -252,7 +297,7 @@ public class UserPermissionDaoTest {
     assertThat(underTest.selectUserIdsByQuery(dbSession, query)).containsExactly(user1.getId());
 
     // search all users whose name matches "mariu", whatever the permissions
-    query = PermissionQuery.builder().setOrganizationUuid(org1.getUuid()).setSearchQuery("mariu").setComponentUuid(project1.uuid()).build();
+    query = PermissionQuery.builder().setOrganizationUuid(org1.getUuid()).setSearchQuery("mariu").setComponent(project1).build();
     assertThat(underTest.selectUserIdsByQuery(dbSession, query)).containsExactly(user1.getId());
 
     // search all users whose name matches "mariu", whatever the organization
@@ -292,8 +337,7 @@ public class UserPermissionDaoTest {
     addProjectPermission(org2, ISSUE_ADMIN, user2, project2);
     PermissionQuery query = PermissionQuery.builder()
       .setOrganizationUuid(org1.getUuid())
-      .setComponentUuid(project1.uuid())
-      .setComponentId(project1.getId())
+      .setComponent(project1)
       .build();
 
     List<Integer> result = underTest.selectUserIdsByQueryAndScope(dbSession, query);
diff --git a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/component/ComponentTesting.java b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/component/ComponentTesting.java
index f11c0d50eb3..95f6c9aa54e 100644
--- a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/component/ComponentTesting.java
+++ b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/component/ComponentTesting.java
@@ -29,6 +29,7 @@ import org.sonar.db.organization.OrganizationDto;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
+import static org.apache.commons.lang.math.RandomUtils.nextLong;
 import static org.sonar.db.component.BranchType.PULL_REQUEST;
 import static org.sonar.db.component.ComponentDto.BRANCH_KEY_SEPARATOR;
 import static org.sonar.db.component.ComponentDto.PULL_REQUEST_SEPARATOR;
@@ -139,6 +140,7 @@ public class ComponentTesting {
 
   private static ComponentDto newProjectDto(String organizationUuid, String uuid, boolean isPrivate) {
     return new ComponentDto()
+      .setId(nextLong())
       .setOrganizationUuid(organizationUuid)
       .setUuid(uuid)
       .setUuidPath(UUID_PATH_OF_ROOT)