From 856a11e8266edf43d70d3781d30ac017435685bb Mon Sep 17 00:00:00 2001
From: Michal Duda <michal.duda@sonarsource.com>
Date: Thu, 13 Jun 2019 12:24:59 +0200
Subject: [PATCH] SONAR-11560 fix api/permissions/users sorting

---
 .../sonar/db/permission/PermissionQuery.java  | 13 ++++++
 .../db/permission/UserPermissionDao.java      | 11 ++++-
 .../db/permission/UserPermissionMapper.java   |  5 +++
 .../db/permission/UserPermissionMapper.xml    | 22 ++++++++++
 .../db/permission/PermissionQueryTest.java    |  2 +
 .../db/permission/UserPermissionDaoTest.java  | 42 +++++++++++++++++++
 .../server/permission/ws/GroupsAction.java    |  1 +
 .../server/permission/ws/UsersAction.java     | 16 +++----
 .../server/permission/ws/users-example.json   |  9 +++-
 .../server/permission/ws/UsersActionTest.java |  6 ++-
 10 files changed, 116 insertions(+), 11 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 4e346ffef52..8eaab6b2b18 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
@@ -48,6 +48,7 @@ public class PermissionQuery {
   private final String permission;
   // 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
@@ -67,6 +68,7 @@ public class PermissionQuery {
     this.permission = builder.permission;
     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);
@@ -99,6 +101,11 @@ public class PermissionQuery {
     return componentUuid;
   }
 
+  @CheckForNull
+  public Long getComponentId() {
+    return componentId;
+  }
+
   @CheckForNull
   public String getSearchQuery() {
     return searchQuery;
@@ -130,6 +137,7 @@ public class PermissionQuery {
     private String permission;
     private String organizationUuid;
     private String componentUuid;
+    private Long componentId;
     private String template;
     private String searchQuery;
     private boolean withAtLeastOnePermission;
@@ -157,6 +165,11 @@ public class PermissionQuery {
       return this;
     }
 
+    public Builder setComponentId(Long componentId) {
+      this.componentId = componentId;
+      return this;
+    }
+
     public Builder setOrganizationUuid(String organizationUuid) {
       this.organizationUuid = organizationUuid;
       return this;
diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/permission/UserPermissionDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/permission/UserPermissionDao.java
index c4556579a95..f29f4ba3f97 100644
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/permission/UserPermissionDao.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/permission/UserPermissionDao.java
@@ -51,7 +51,16 @@ public class UserPermissionDao implements Dao {
   }
 
   public List<Integer> selectUserIdsByQuery(DbSession dbSession, PermissionQuery query) {
-    return mapper(dbSession).selectUserIdsByQuery(query)
+    return paginate(mapper(dbSession).selectUserIdsByQuery(query), query);
+  }
+
+  public List<Integer> selectUserIdsByQueryAndScope(DbSession dbSession, PermissionQuery query) {
+    return paginate(mapper(dbSession).selectUserIdsByQueryAndScope(query), query);
+  }
+
+
+  private static List<Integer> paginate(List<Integer> results, PermissionQuery query) {
+    return results
       .stream()
       // Pagination is done in Java because it's too complex to use SQL pagination in Oracle and MsSQL with the distinct
       .skip(query.getPageOffset())
diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/permission/UserPermissionMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/permission/UserPermissionMapper.java
index 9156aeb6a5a..164673412d9 100644
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/permission/UserPermissionMapper.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/permission/UserPermissionMapper.java
@@ -30,6 +30,11 @@ public interface UserPermissionMapper {
 
   List<Integer> selectUserIdsByQuery(@Param("query") PermissionQuery query);
 
+  /**
+   * Fetch user ids based on permission query and only in a specific scope (global permissions only, organization permissions only or project permissions only)
+   */
+  List<Integer> selectUserIdsByQueryAndScope(@Param("query") PermissionQuery query);
+
   /**
    * Count the number of distinct users returned by {@link #selectUserIdsByQuery(PermissionQuery)}
    * {@link PermissionQuery#getPageOffset()} and {@link PermissionQuery#getPageSize()} are ignored.
diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/UserPermissionMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/UserPermissionMapper.xml
index 62453effbc5..71d6b929116 100644
--- a/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/UserPermissionMapper.xml
+++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/UserPermissionMapper.xml
@@ -27,6 +27,28 @@
     order by case when (count(ur.role) > 0) then 1 else 2 end asc, lower(u.name) asc
   </select>
 
+  <select id="selectUserIdsByQueryAndScope" parameterType="map" resultType="int">
+    select
+    u.id, lower(u.name)
+    from users u
+    left join user_roles ur on ur.user_id = u.id and ur.organization_uuid=#{query.organizationUuid,jdbcType=VARCHAR}
+    <choose>
+      <when test="query.componentId == null">
+        and ur.resource_id is null
+      </when>
+      <otherwise>
+        and ur.resource_id = #{query.componentId,jdbcType=BIGINT}
+      </otherwise>
+    </choose>
+    left join projects p on ur.resource_id = p.id
+    inner join organization_members om on u.id=om.user_id and om.organization_uuid=#{query.organizationUuid,jdbcType=VARCHAR}
+    <where>
+      <include refid="sqlQueryFilters" />
+    </where>
+    group by u.id, lower(u.name)
+    order by case when (count(distinct ur.role) > 0) then 1 else 2 end asc, lower(u.name) asc
+  </select>
+
   <select id="countUsersByQuery" parameterType="map" resultType="int">
     select count(distinct(u.id))
     <include refid="sqlQueryJoins" />
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 56f574d34a7..e0c15477d03 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
@@ -34,12 +34,14 @@ public class PermissionQueryTest {
   public void create_query() {
     PermissionQuery quey = PermissionQuery.builder()
       .setComponentUuid("COMPONENT_UUID")
+      .setComponentId(1234L)
       .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");
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 9e20261b533..21538a36575 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
@@ -260,6 +260,48 @@ public class UserPermissionDaoTest {
     assertThat(underTest.selectUserIdsByQuery(dbSession, query)).isEmpty();
   }
 
+  @Test
+  public void selectUserIdsByQueryAndScope_with_organization_scope() {
+    OrganizationDto org1 = db.organizations().insert();
+    OrganizationDto org2 = db.organizations().insert();
+    UserDto user1 = insertUser(u -> u.setLogin("login1").setName("Marius").setEmail("email1@email.com"), org1, org2);
+    UserDto user2 = insertUser(u -> u.setLogin("login2").setName("Marie").setEmail("email2@email.com"), org1, org2);
+    ComponentDto project1 = db.components().insertPrivateProject(org1);
+    ComponentDto project2 = db.components().insertPrivateProject(org2);
+    addProjectPermission(org1, USER, user1, project1);
+    addGlobalPermission(org1, PROVISIONING, user1);
+    addProjectPermission(org2, ISSUE_ADMIN, user2, project2);
+    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(org1.getUuid()).build();
+
+    List<Integer> result = underTest.selectUserIdsByQueryAndScope(dbSession, query);
+
+    // users with any kind of global permissions are first on the list and then sorted by name
+    assertThat(result).containsExactly(user1.getId(), user2.getId());
+  }
+
+  @Test
+  public void selectUserIdsByQueryAndScope_with_project_scope() {
+    OrganizationDto org1 = db.organizations().insert();
+    OrganizationDto org2 = db.organizations().insert();
+    UserDto user1 = insertUser(u -> u.setLogin("login1").setName("Marius").setEmail("email1@email.com"), org1, org2);
+    UserDto user2 = insertUser(u -> u.setLogin("login2").setName("Marie").setEmail("email2@email.com"), org1, org2);
+    ComponentDto project1 = db.components().insertPrivateProject(org1);
+    ComponentDto project2 = db.components().insertPrivateProject(org2);
+    addProjectPermission(org1, USER, user1, project1);
+    addGlobalPermission(org1, PROVISIONING, user1);
+    addProjectPermission(org2, ISSUE_ADMIN, user2, project2);
+    PermissionQuery query = PermissionQuery.builder()
+      .setOrganizationUuid(org1.getUuid())
+      .setComponentUuid(project1.uuid())
+      .setComponentId(project1.getId())
+      .build();
+
+    List<Integer> result = underTest.selectUserIdsByQueryAndScope(dbSession, query);
+
+    // users with any this projects permissions are first on the list and then sorted by name
+    assertThat(result).containsExactly(user1.getId(), user2.getId());
+  }
+
   @Test
   public void selectUserIdsByQuery_is_paginated() {
     OrganizationDto organization = db.organizations().insert();
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 5c271680743..71b525b6423 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
@@ -125,6 +125,7 @@ public class GroupsAction implements PermissionsWsAction {
       .setSearchQuery(textQuery);
     if (project.isPresent()) {
       permissionQuery.setComponentUuid(project.get().getUuid());
+      permissionQuery.setComponentId(project.get().getId());
     }
 
     return permissionQuery.build();
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 7ceded5fe58..8f1a6e7003b 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
@@ -131,7 +131,10 @@ public class UsersAction implements PermissionsWsAction {
       .setPageIndex(request.mandatoryParamAsInt(Param.PAGE))
       .setPageSize(request.mandatoryParamAsInt(Param.PAGE_SIZE))
       .setSearchQuery(textQuery);
-    project.ifPresent(projectId -> permissionQuery.setComponentUuid(projectId.getUuid()));
+    project.ifPresent(projectId -> {
+      permissionQuery.setComponentUuid(projectId.getUuid());
+      permissionQuery.setComponentId(projectId.getId());
+    });
     if (permission != null) {
       if (project.isPresent()) {
         requestValidator.validateProjectPermission(permission);
@@ -167,7 +170,7 @@ public class UsersAction implements PermissionsWsAction {
   }
 
   private List<UserDto> findUsers(DbSession dbSession, PermissionQuery query) {
-    List<Integer> orderedIds = dbClient.userPermissionDao().selectUserIdsByQuery(dbSession, query);
+    List<Integer> orderedIds = dbClient.userPermissionDao().selectUserIdsByQueryAndScope(dbSession, query);
     return Ordering.explicit(orderedIds).onResultOf(UserDto::getId).immutableSortedCopy(dbClient.userDao().selectByIds(dbSession, orderedIds));
   }
 
@@ -176,11 +179,10 @@ public class UsersAction implements PermissionsWsAction {
       return emptyList();
     }
     List<Integer> userIds = users.stream().map(UserDto::getId).collect(Collectors.toList());
-    PermissionQuery query = PermissionQuery.builder()
+    PermissionQuery.Builder queryBuilder = PermissionQuery.builder()
       .setOrganizationUuid(org.getUuid())
-      .setComponentUuid(project.map(ProjectId::getUuid).orElse(null))
-      .withAtLeastOnePermission()
-      .build();
-    return dbClient.userPermissionDao().selectUserPermissionsByQuery(dbSession, query, userIds);
+      .withAtLeastOnePermission();
+    project.ifPresent(p -> queryBuilder.setComponentUuid(p.getUuid()));
+    return dbClient.userPermissionDao().selectUserPermissionsByQuery(dbSession, queryBuilder.build(), userIds);
   }
 }
diff --git a/server/sonar-server/src/main/resources/org/sonar/server/permission/ws/users-example.json b/server/sonar-server/src/main/resources/org/sonar/server/permission/ws/users-example.json
index 1f3e6ae34cb..3c4e357f176 100644
--- a/server/sonar-server/src/main/resources/org/sonar/server/permission/ws/users-example.json
+++ b/server/sonar-server/src/main/resources/org/sonar/server/permission/ws/users-example.json
@@ -2,7 +2,7 @@
   "paging": {
     "pageIndex": 1,
     "pageSize": 20,
-    "total": 2
+    "total": 3
   },
   "users": [
     {
@@ -18,6 +18,13 @@
       "email": "george.orwell@1984.net",
       "avatar": "583af86a274c1027ef078cada831babf",
       "permissions": ["scan"]
+    },
+    {
+      "login": "adam.west",
+      "name": "Adam West",
+      "email": "adamwest@adamwest.com",
+      "avatar": "9b55aba24cc5ee533294334bd20abb34",
+      "permissions": []
     }
   ]
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/UsersActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/UsersActionTest.java
index 61963cdb588..5120c032329 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/UsersActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/UsersActionTest.java
@@ -76,12 +76,14 @@ public class UsersActionTest extends BasePermissionWsTest<UsersAction> {
   public void search_for_users_with_response_example() {
     UserDto user1 = db.users().insertUser(newUserDto().setLogin("admin").setName("Administrator").setEmail("admin@admin.com"));
     db.organizations().addMember(db.getDefaultOrganization(), user1);
-    UserDto user2 = db.users().insertUser(newUserDto().setLogin("george.orwell").setName("George Orwell").setEmail("george.orwell@1984.net"));
+    UserDto user2 = db.users().insertUser(newUserDto().setLogin("adam.west").setName("Adam West").setEmail("adamwest@adamwest.com"));
     db.organizations().addMember(db.getDefaultOrganization(), user2);
+    UserDto user3 = db.users().insertUser(newUserDto().setLogin("george.orwell").setName("George Orwell").setEmail("george.orwell@1984.net"));
+    db.organizations().addMember(db.getDefaultOrganization(), user3);
     db.users().insertPermissionOnUser(user1, ADMINISTER_QUALITY_PROFILES);
     db.users().insertPermissionOnUser(user1, ADMINISTER);
     db.users().insertPermissionOnUser(user1, ADMINISTER_QUALITY_GATES);
-    db.users().insertPermissionOnUser(user2, SCAN);
+    db.users().insertPermissionOnUser(user3, SCAN);
 
     loginAsAdmin(db.getDefaultOrganization());
     String result = newRequest().execute().getInput();
-- 
2.39.5