SONAR-9287 Fix limited number of returned permissions in api/permissions/users

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 54788cecaba164b6766ed6a7272529503eff190f..d70c5f1994c10eac4e8f77c07c9b62c08cf825d0 100644 (file)
--- 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
@@ -134,8 +134,8 @@ public class PermissionQuery {
     private String searchQuery;
     private boolean withAtLeastOnePermission;
 
-    private Integer pageIndex = DEFAULT_PAGE_INDEX;
-    private Integer pageSize = DEFAULT_PAGE_SIZE;
+    private Integer pageIndex;
+    private Integer pageSize;
 
     private Builder() {
       // enforce method constructor

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 8ffeb772d2908d009d5064de822d508655f206fd..7f74b5ee284329d20ab5794cabac6befb38c0c5b 100644 (file)
--- 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
@@ -22,12 +22,11 @@ package org.sonar.db.permission;
 import java.util.Collection;
 import java.util.List;
 import java.util.Set;
-import javax.annotation.Nullable;
-import org.apache.ibatis.session.RowBounds;
 import org.sonar.core.util.stream.MoreCollectors;
 import org.sonar.db.Dao;
 import org.sonar.db.DatabaseUtils;
 import org.sonar.db.DbSession;
+import org.sonar.db.Pagination;
 import org.sonar.db.component.ComponentMapper;
 
 import static com.google.common.base.Preconditions.checkArgument;
@@ -37,40 +36,32 @@ import static org.sonar.db.DatabaseUtils.executeLargeInputs;
 public class UserPermissionDao implements Dao {
 
   /**
-   * List of user permissions ordered by alphabetical order of user names
-   *  @param query non-null query including optional filters.
-   * @param userLogins if null, then filter on all active users. If not null, then filter on logins, including disabled users.
-   *                   Must not be empty. If not null then maximum size is {@link DatabaseUtils#PARTITION_SIZE_FOR_ORACLE}.
+   * List of user permissions ordered by alphabetical order of user names.
+   * Pagination is NOT applied.
+   * No sort is done.
+   *
+   * @param query non-null query including optional filters.
+   * @param userIds Filter on user ids, including disabled users. Must not be empty and maximum size is {@link DatabaseUtils#PARTITION_SIZE_FOR_ORACLE}.
    */
-  public List<UserPermissionDto> select(DbSession dbSession, PermissionQuery query, @Nullable Collection<String> userLogins) {
-    if (userLogins != null) {
-      if (userLogins.isEmpty()) {
-        return emptyList();
-      }
-      checkArgument(userLogins.size() <= DatabaseUtils.PARTITION_SIZE_FOR_ORACLE, "Maximum 1'000 users are accepted");
+  public List<UserPermissionDto> selectUserPermissionsByQuery(DbSession dbSession, PermissionQuery query, Collection<Integer> userIds) {
+    if (userIds.isEmpty()) {
+      return emptyList();
     }
-
-    RowBounds rowBounds = new RowBounds(query.getPageOffset(), query.getPageSize());
-    return mapper(dbSession).selectByQuery(query, userLogins, rowBounds);
+    checkArgument(userIds.size() <= DatabaseUtils.PARTITION_SIZE_FOR_ORACLE, "Maximum 1'000 users are accepted");
+    return mapper(dbSession).selectUserPermissionsByQueryAndUserIds(query, userIds);
   }
 
-  /**
-   * Shortcut over {@link #select(DbSession, PermissionQuery, Collection)} to return only distinct user
-   * ids, keeping the same order.
-   */
-  public List<Integer> selectUserIds(DbSession dbSession, PermissionQuery query) {
-    List<UserPermissionDto> dtos = select(dbSession, query, null);
-    return dtos.stream()
-      .map(UserPermissionDto::getUserId)
-      .distinct()
-      .collect(MoreCollectors.toList(dtos.size()));
+  public List<Integer> selectUserIdsByQuery(DbSession dbSession, PermissionQuery query) {
+    return mapper(dbSession).selectUserIdsByQuery(query)
+      .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())
+      .limit(query.getPageSize())
+      .collect(MoreCollectors.toList());
   }
 
-  /**
-   * @see UserPermissionMapper#countUsersByQuery(String, PermissionQuery, Collection)
-   */
-  public int countUsers(DbSession dbSession, String organizationUuid, PermissionQuery query) {
-    return mapper(dbSession).countUsersByQuery(organizationUuid, query, null);
+  public int countUsersByQuery(DbSession dbSession, PermissionQuery query) {
+    return mapper(dbSession).countUsersByQuery(query);
   }
 
   /**

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 5c78b79d38a389941fb4b800af28d9c7a83ebe1a..57b90c9f51af17f8a886255554aa6bbe7f73381c 100644 (file)
--- 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
@@ -22,23 +22,19 @@ package org.sonar.db.permission;
 import java.util.Collection;
 import java.util.List;
 import java.util.Set;
-import javax.annotation.Nullable;
 import org.apache.ibatis.annotations.Param;
-import org.apache.ibatis.session.RowBounds;
 
 public interface UserPermissionMapper {
 
-  List<UserPermissionDto> selectByQuery(@Param("query") PermissionQuery query, @Nullable @Param("userLogins") Collection<String> userLogins, RowBounds rowBounds);
+  List<UserPermissionDto> selectUserPermissionsByQueryAndUserIds(@Param("query") PermissionQuery query, @Param("userIds") Collection<Integer> userIds);
+
+  List<Integer> selectUserIdsByQuery(@Param("query") PermissionQuery query);
 
   /**
-   * Count the number of distinct users returned by {@link #selectByQuery(PermissionQuery, Collection, RowBounds)}
+   * Count the number of distinct users returned by {@link #selectUserIdsByQuery(PermissionQuery)}
    * {@link PermissionQuery#getPageOffset()} and {@link PermissionQuery#getPageSize()} are ignored.
-   *
-   * @param useNull must always be null. It is needed for using the sql of 
-   * {@link #selectByQuery(PermissionQuery, Collection, RowBounds)}
    */
-  int countUsersByQuery(@Param("organizationUuid") String organizationUuid, @Param("query") PermissionQuery query,
-    @Nullable @Param("userLogins") Collection<String> useNull);
+  int countUsersByQuery(@Param("query") PermissionQuery query);
 
   /**
    * Count the number of users per permission for a given list of projects.

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 bfe02d6267db1dd77f988e8ef047eadf42db3783..4c2b0c2a719ccfb15cfc5ffedd83e4da0724bea5 100644 (file)
--- 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
@@ -3,55 +3,66 @@
 
 <mapper namespace="org.sonar.db.permission.UserPermissionMapper">
 
-  <select id="selectByQuery" parameterType="map" resultType="org.sonar.db.permission.UserPermissionDto">
+  <select id="selectUserPermissionsByQueryAndUserIds" parameterType="map" resultType="org.sonar.db.permission.UserPermissionDto">
     select
       u.id as userId,
       ur.organization_uuid as organizationUuid,
       ur.resource_id as componentId,
       ur.role as permission
-    <include refid="sqlQuery" />
-    order by lower(u.name), u.name, ur.role
+    <include refid="sqlQueryJoins" />
+    <where>
+      u.id in <foreach collection="userIds" open="(" close=")" item="userId" separator=",">#{userId,jdbcType=INTEGER}</foreach>
+      <include refid="sqlQueryFilters" />
+    </where>
+  </select>
+
+  <select id="selectUserIdsByQuery" parameterType="map" resultType="int">
+    select
+      distinct u.id, lower(u.name) as lowerName
+      <include refid="sqlQueryJoins" />
+      <where>
+        <include refid="sqlQueryFilters" />
+      </where>
+    order by lowerName asc
   </select>
 
   <select id="countUsersByQuery" parameterType="map" resultType="int">
     select count(distinct(u.id))
-    <include refid="sqlQuery" />
+    <include refid="sqlQueryJoins" />
+    <where>
+      <include refid="sqlQueryFilters" />
+    </where>
   </select>
 
-  <sql id="sqlQuery">
+  <sql id="sqlQueryJoins">
     from users u
     left join user_roles ur on ur.user_id = u.id
     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>
-      <if test="userLogins == null">
-        and u.active = ${_true}
-      </if>
-      <if test="userLogins != null">
-        and u.login in <foreach collection="userLogins" open="(" close=")" item="userLogin" separator=",">#{userLogin,jdbcType=VARCHAR}</foreach>
-      </if>
-      <if test="query.searchQueryToSql != null">
-        and (
-        lower(u.name) like #{query.searchQueryToSqlLowercase,jdbcType=VARCHAR} ESCAPE '/'
-        or u.email like #{query.searchQueryToSql,jdbcType=VARCHAR} ESCAPE '/'
-        or u.login like #{query.searchQueryToSql,jdbcType=VARCHAR} ESCAPE '/')
+  </sql>
 
+  <sql id="sqlQueryFilters">
+    and u.active = ${_true}
+    <if test="query.searchQueryToSql != null">
+      and (
+      lower(u.name) like #{query.searchQueryToSqlLowercase,jdbcType=VARCHAR} ESCAPE '/'
+      or u.email like #{query.searchQueryToSql,jdbcType=VARCHAR} ESCAPE '/'
+      or u.login like #{query.searchQueryToSql,jdbcType=VARCHAR} ESCAPE '/')
+    </if>
+    <!-- filter rows with user permissions -->
+    <if test="query.withAtLeastOnePermission()">
+      and ur.organization_uuid = om.organization_uuid
+      and ur.role is not null
+      <if test="query.componentUuid==null">
+        and ur.resource_id is null
       </if>
-      <!-- filter rows with user permissions -->
-      <if test="query.withAtLeastOnePermission()">
-        and ur.organization_uuid = #{query.organizationUuid,jdbcType=VARCHAR}
-        and ur.role is not null
-        <if test="query.componentUuid==null">
-          and ur.resource_id is null
-        </if>
-        <if test="query.componentUuid!=null">
-          and p.uuid = #{query.componentUuid,jdbcType=VARCHAR}
-        </if>
-        <if test="query.permission!=null">
-          and ur.role = #{query.permission,jdbcType=VARCHAR}
-        </if>
+      <if test="query.componentUuid!=null">
+        and p.uuid = #{query.componentUuid,jdbcType=VARCHAR}
       </if>
-    </where>
+      <if test="query.permission!=null">
+        and ur.role = #{query.permission,jdbcType=VARCHAR}
+      </if>
+    </if>
   </sql>
 
   <select id="selectGlobalPermissionsOfUser" parameterType="map" resultType="string">

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
new file mode 100644 (file)
index 0000000..1645cc8
--- /dev/null
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/PermissionQueryTest.java
@@ -0,0 +1,88 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2017 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.db.permission;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import static org.assertj.core.api.Java6Assertions.assertThat;
+
+public class PermissionQueryTest {
+
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+
+  @Test
+  public void create_query() {
+    PermissionQuery quey = PermissionQuery.builder()
+      .setComponentUuid("COMPONENT_UUID")
+      .setOrganizationUuid("ORGANIZATION_UUID")
+      .setPermission("user")
+      .setSearchQuery("sonar")
+      .build();
+
+    assertThat(quey.getComponentUuid()).isEqualTo("COMPONENT_UUID");
+    assertThat(quey.getOrganizationUuid()).isEqualTo("ORGANIZATION_UUID");
+    assertThat(quey.getPermission()).isEqualTo("user");
+    assertThat(quey.getSearchQuery()).isEqualTo("sonar");
+  }
+
+  @Test
+  public void create_query_with_pagination() {
+    PermissionQuery quey = PermissionQuery.builder()
+      .setOrganizationUuid("ORGANIZATION_UUID")
+      .setPageSize(10)
+      .setPageIndex(5)
+      .build();
+
+    assertThat(quey.getPageOffset()).isEqualTo(40);
+    assertThat(quey.getPageSize()).isEqualTo(10);
+  }
+
+  @Test
+  public void create_query_with_default_pagination() {
+    PermissionQuery quey = PermissionQuery.builder()
+      .setOrganizationUuid("ORGANIZATION_UUID")
+      .build();
+
+    assertThat(quey.getPageOffset()).isEqualTo(0);
+    assertThat(quey.getPageSize()).isEqualTo(20);
+  }
+
+  @Test
+  public void fail_when_no_organization() {
+    expectedException.expect(NullPointerException.class);
+    expectedException.expectMessage("Organization UUID cannot be null");
+
+    PermissionQuery.builder().setOrganizationUuid(null).build();
+  }
+
+  @Test
+  public void fail_when_search_query_length_is_less_than_3_characters() {
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Search query should contains at least 3 characters");
+
+    PermissionQuery.builder()
+      .setOrganizationUuid("ORGANIZATION_UUID")
+      .setSearchQuery("so")
+      .build();
+  }
+}

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 9b7c64124e31248ae519590fd958d05149706d9a..429a31eaa66ad5d669acf7175718bd63834f53e1 100644 (file)
--- 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
@@ -19,13 +19,14 @@
  */
 package org.sonar.db.permission;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 import java.util.Random;
 import java.util.function.Consumer;
-import javax.annotation.Nullable;
+import java.util.stream.Collectors;
+import org.assertj.core.groups.Tuple;
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.api.utils.System2;
@@ -37,6 +38,9 @@ import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.user.UserDto;
 
 import static java.util.Arrays.asList;
+import static java.util.Arrays.stream;
+import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.tuple;
 import static org.sonar.api.web.UserRole.CODEVIEWER;
@@ -75,68 +79,41 @@ public class UserPermissionDaoTest {
 
     // global permissions of users who has at least one global permission, ordered by user name then permission
     PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().build();
-    expectPermissions(organization, query, null, global2, global3, global1);
+    expectPermissions(query, asList(user2.getId(), user1.getId()), global2, global3, global1);
 
     // default query returns all users, whatever their permissions nor organizations
     // (that's a non-sense, but still this is required for api/permissions/groups
     // when filtering users by name)
     query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).build();
-    expectPermissions(organization, query, null, global2, global3, org2Global2, global1, org2Global1, project1Perm);
-
-    // return empty list if non-null but empty logins
-    expectPermissions(organization, query, Collections.emptyList());
-
-    // global permissions of user1
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().build();
-    expectPermissions(organization, query, asList(user1.getLogin()), global1);
-
-    // global permissions of user2
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().build();
-    expectPermissions(organization, query, asList(user2.getLogin()), global2, global3);
-
-    // global permissions of user1, user2 and another one
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().build();
-    expectPermissions(organization, query, asList(user1.getLogin(), user2.getLogin(), "missing"), global2, global3, global1);
-
-    // empty global permissions if login does not exist
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().build();
-    expectPermissions(organization, query, asList("missing"));
-
-    // empty global permissions if user does not have any
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().build();
-    expectPermissions(organization, query, asList(user3.getLogin()));
-
-    // user3 has no global permissions
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().build();
-    expectPermissions(organization, query, asList(user3.getLogin()));
+    expectPermissions(query, asList(user2.getId(), user1.getId(), user3.getId()), global2, global3, org2Global2, global1, org2Global1, project1Perm);
 
     // global permissions "admin"
     query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setPermission(SYSTEM_ADMIN).build();
-    expectPermissions(organization, query, null, global2, global1);
+    expectPermissions(query, asList(user2.getId(), user1.getId()), global2, global1);
 
     // empty if nobody has the specified global permission
     query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setPermission("missing").build();
-    expectPermissions(organization, query, null);
+    expectPermissions(query, emptyList());
 
     // search by user name (matches 2 users)
     query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().setSearchQuery("mari").build();
-    expectPermissions(organization, query, null, global2, global3, global1);
+    expectPermissions(query, asList(user2.getId(), user1.getId()), global2, global3, global1);
 
     // search by user login
     query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().setSearchQuery("ogin2").build();
-    expectPermissions(organization, query, null, global2, global3);
+    expectPermissions(query, singletonList(user2.getId()), global2, global3);
 
     // search by user email
     query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().setSearchQuery("mail2").build();
-    expectPermissions(organization, query, null, global2, global3);
+    expectPermissions(query, singletonList(user2.getId()), global2, global3);
 
     // search by user name (matches 2 users) and global permission
     query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setSearchQuery("Mari").setPermission(PROVISIONING).build();
-    expectPermissions(organization, query, null, global3);
+    expectPermissions(query, singletonList(user2.getId()), global3);
 
     // search by user name (no match)
     query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setSearchQuery("Unknown").build();
-    expectPermissions(organization, query, null);
+    expectPermissions(query, emptyList());
   }
 
   @Test
@@ -155,47 +132,27 @@ public class UserPermissionDaoTest {
 
     // 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();
-    expectPermissions(organization, query, null, perm3, perm2, perm1);
-
-    // project permissions of user1
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().setComponentUuid(project1.uuid()).build();
-    expectPermissions(organization, query, asList(user1.getLogin()), perm2, perm1);
-
-    // project permissions of user2
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().setComponentUuid(project1.uuid()).build();
-    expectPermissions(organization, query, asList(user2.getLogin()), perm3);
-
-    // project permissions of user2 and another one
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().setComponentUuid(project1.uuid()).build();
-    expectPermissions(organization, query, asList(user2.getLogin(), "missing"), perm3);
-
-    // empty project permissions if login does not exist
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().setComponentUuid(project1.uuid()).build();
-    expectPermissions(organization, query, asList("missing"));
-
-    // empty project permissions if user does not have any
-    query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).withAtLeastOnePermission().setComponentUuid(project1.uuid()).build();
-    expectPermissions(organization, query, asList(user3.getLogin()));
+    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();
-    expectPermissions(organization, query, null);
+    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();
-    expectPermissions(organization, query, null, perm3, perm2, perm1);
+    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();
-    expectPermissions(organization, query, null, perm3, perm2);
+    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();
-    expectPermissions(organization, query, null);
+    expectPermissions(query, emptyList());
 
     // permissions of unknown project
     query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).setComponentUuid("missing").withAtLeastOnePermission().build();
-    expectPermissions(organization, query, null);
+    expectPermissions(query, emptyList());
   }
 
   @Test
@@ -212,10 +169,10 @@ public class UserPermissionDaoTest {
     addProjectPermission(organization, ISSUE_ADMIN, user2, project2);
 
     // no projects -> return empty list
-    assertThat(underTest.countUsersByProjectPermission(dbSession, Collections.emptyList())).isEmpty();
+    assertThat(underTest.countUsersByProjectPermission(dbSession, emptyList())).isEmpty();
 
     // one project
-    expectCount(asList(project1.getId()),
+    expectCount(singletonList(project1.getId()),
       new CountPerProjectPermission(project1.getId(), USER, 1),
       new CountPerProjectPermission(project1.getId(), ISSUE_ADMIN, 2));
 
@@ -227,7 +184,7 @@ public class UserPermissionDaoTest {
   }
 
   @Test
-  public void selectUserIds() {
+  public void selectUserIdsByQuery() {
     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);
@@ -242,29 +199,66 @@ public class UserPermissionDaoTest {
 
     // logins are ordered by user name: user2 ("Marie") then user1 ("Marius")
     PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(project1.getOrganizationUuid()).setComponentUuid(project1.uuid()).withAtLeastOnePermission().build();
-    assertThat(underTest.selectUserIds(dbSession, query)).containsExactly(user2.getId(), user1.getId());
+    assertThat(underTest.selectUserIdsByQuery(dbSession, query)).containsExactly(user2.getId(), user1.getId());
     query = PermissionQuery.builder().setOrganizationUuid("anotherOrg").setComponentUuid(project1.uuid()).withAtLeastOnePermission().build();
-    assertThat(underTest.selectUserIds(dbSession, query)).isEmpty();
+    assertThat(underTest.selectUserIdsByQuery(dbSession, query)).isEmpty();
 
     // on a project without permissions
     query = PermissionQuery.builder().setOrganizationUuid(org1.getUuid()).setComponentUuid("missing").withAtLeastOnePermission().build();
-    assertThat(underTest.selectUserIds(dbSession, query)).isEmpty();
+    assertThat(underTest.selectUserIdsByQuery(dbSession, query)).isEmpty();
 
     // search all users whose name matches "mar", whatever the permissions
     query = PermissionQuery.builder().setOrganizationUuid(org1.getUuid()).setSearchQuery("mar").build();
-    assertThat(underTest.selectUserIds(dbSession, query)).containsExactly(user2.getId(), user1.getId());
+    assertThat(underTest.selectUserIdsByQuery(dbSession, query)).containsExactly(user2.getId(), user1.getId());
 
     // search all users whose name matches "mariu", whatever the permissions
     query = PermissionQuery.builder().setOrganizationUuid(org1.getUuid()).setSearchQuery("mariu").build();
-    assertThat(underTest.selectUserIds(dbSession, query)).containsExactly(user1.getId());
+    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();
-    assertThat(underTest.selectUserIds(dbSession, query)).containsExactly(user1.getId());
+    assertThat(underTest.selectUserIdsByQuery(dbSession, query)).containsExactly(user1.getId());
 
     // search all users whose name matches "mariu", whatever the organization
     query = PermissionQuery.builder().setOrganizationUuid("missingOrg").setSearchQuery("mariu").build();
-    assertThat(underTest.selectUserIds(dbSession, query)).isEmpty();
+    assertThat(underTest.selectUserIdsByQuery(dbSession, query)).isEmpty();
+  }
+
+  @Test
+  public void selectUserIdsByQuery_is_paginated() {
+    OrganizationDto organization = db.organizations().insert();
+    List<Integer> userIds = new ArrayList<>();
+    for (int i = 0; i < 10; i++) {
+      String name = "user-" + i;
+      UserDto user = insertUser(u -> u.setName(name), organization);
+      addGlobalPermission(organization, PROVISIONING, user);
+      addGlobalPermission(organization, SYSTEM_ADMIN, user);
+      userIds.add(user.getId());
+    }
+
+    assertThat(underTest.selectUserIdsByQuery(dbSession, PermissionQuery.builder().setOrganizationUuid(organization.getUuid())
+      .setPageSize(3).setPageIndex(1).build()))
+        .containsExactly(userIds.get(0), userIds.get(1), userIds.get(2));
+    assertThat(underTest.selectUserIdsByQuery(dbSession, PermissionQuery.builder().setOrganizationUuid(organization.getUuid())
+      .setPageSize(2).setPageIndex(3).build()))
+        .containsExactly(userIds.get(4), userIds.get(5));
+    assertThat(underTest.selectUserIdsByQuery(dbSession, PermissionQuery.builder().setOrganizationUuid(organization.getUuid())
+      .setPageSize(50).setPageIndex(1).build()))
+        .hasSize(10);
+  }
+
+  @Test
+  public void selectUserIdsByQuery_is_sorted_by_insensitive_name() {
+    OrganizationDto organization = db.organizations().insert();
+    UserDto user1 = insertUser(u -> u.setName("user1"), organization);
+    addGlobalPermission(organization, PROVISIONING, user1);
+    UserDto user3 = insertUser(u -> u.setName("user3"), organization);
+    addGlobalPermission(organization, SYSTEM_ADMIN, user3);
+    UserDto user2 = insertUser(u -> u.setName("User2"), organization);
+    addGlobalPermission(organization, PROVISIONING, user2);
+
+    assertThat(underTest.selectUserIdsByQuery(dbSession, PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).build()))
+      .containsExactly(user1.getId(), user2.getId(), user3.getId());
   }
 
   @Test
@@ -597,13 +591,13 @@ public class UserPermissionDaoTest {
 
   private UserDto insertUser(Consumer<UserDto> populateUserDto, OrganizationDto... organizations) {
     UserDto user = db.users().insertUser(populateUserDto);
-    Arrays.stream(organizations).forEach(organization -> db.organizations().addMember(organization, user));
+    stream(organizations).forEach(organization -> db.organizations().addMember(organization, user));
     return user;
   }
 
   private UserDto insertUser(OrganizationDto... organizations) {
     UserDto user = db.users().insertUser();
-    Arrays.stream(organizations).forEach(organization -> db.organizations().addMember(organization, user));
+    stream(organizations).forEach(organization -> db.organizations().addMember(organization, user));
     return user;
   }
 
@@ -625,23 +619,21 @@ public class UserPermissionDaoTest {
     }
   }
 
-  private void expectPermissions(OrganizationDto org, PermissionQuery query, @Nullable Collection<String> logins, UserPermissionDto... expected) {
-    // test method "select()"
-    List<UserPermissionDto> permissions = underTest.select(dbSession, query, logins);
-    assertThat(permissions).hasSize(expected.length);
-    for (int i = 0; i < expected.length; i++) {
-      UserPermissionDto got = permissions.get(i);
-      UserPermissionDto expect = expected[i];
-      assertThat(got.getUserId()).isEqualTo(expect.getUserId());
-      assertThat(got.getPermission()).isEqualTo(expect.getPermission());
-      assertThat(got.getComponentId()).isEqualTo(expect.getComponentId());
-    }
+  private void expectPermissions(PermissionQuery query, Collection<Integer> expectedUserIds, UserPermissionDto... expectedPermissions) {
+    assertThat(underTest.selectUserIdsByQuery(dbSession, query)).containsExactly(expectedUserIds.toArray(new Integer[0]));
+    List<UserPermissionDto> currentPermissions = underTest.selectUserPermissionsByQuery(dbSession, query, expectedUserIds);
+    assertThat(currentPermissions).hasSize(expectedPermissions.length);
+    List<Tuple> expectedPermissionsAsTuple = Arrays.stream(expectedPermissions)
+      .map(expectedPermission -> tuple(expectedPermission.getUserId(), expectedPermission.getPermission(), expectedPermission.getComponentId(),
+        expectedPermission.getOrganizationUuid()))
+      .collect(Collectors.toList());
+    assertThat(currentPermissions)
+      .extracting(UserPermissionDto::getUserId, UserPermissionDto::getPermission, UserPermissionDto::getComponentId, UserPermissionDto::getOrganizationUuid)
+      .containsOnly(expectedPermissionsAsTuple.toArray(new Tuple[0]));
 
-    if (logins == null) {
-      // test method "countUsers()", which does not make sense if users are filtered
-      long distinctUsers = Arrays.stream(expected).mapToLong(p -> p.getUserId()).distinct().count();
-      assertThat((long) underTest.countUsers(dbSession, org.getUuid(), query)).isEqualTo(distinctUsers);
-    }
+    // test method "countUsers()"
+    long distinctUsers = stream(expectedPermissions).mapToLong(UserPermissionDto::getUserId).distinct().count();
+    assertThat((long) underTest.countUsersByQuery(dbSession, query)).isEqualTo(distinctUsers);
   }
 
   private UserPermissionDto addGlobalPermission(OrganizationDto org, String permission, UserDto user) {
@@ -658,10 +650,6 @@ public class UserPermissionDaoTest {
     return dto;
   }
 
-  private void addOrganizationMember(OrganizationDto org, UserDto user) {
-    db.organizations().addMember(org, user);
-  }
-
   private void assertThatProjectPermissionDoesNotExist(UserDto user, String permission, ComponentDto project) {
     assertThat(db.countSql(dbSession, "select count(id) from user_roles where role='" + permission + "' and user_id=" + user.getId() + " and resource_id=" + project.getId()))
       .isEqualTo(0);

diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/SearchGlobalPermissionsAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/SearchGlobalPermissionsAction.java
index 13b75549f99fa5b897ff4c5e298fd7028658e357..b74bb1d452134d917101bc44dae969a5b13fbb4a 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/SearchGlobalPermissionsAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/SearchGlobalPermissionsAction.java
@@ -94,7 +94,7 @@ public class SearchGlobalPermissionsAction implements PermissionsWsAction {
             .setKey(permissionKey)
             .setName(i18nName(permissionKey))
             .setDescription(i18nDescriptionMessage(permissionKey))
-            .setUsersCount(countUsers(dbSession, org, query))
+            .setUsersCount(countUsers(dbSession, query))
             .setGroupsCount(countGroups(dbSession, org, permissionKey)));
       });
 
@@ -114,8 +114,8 @@ public class SearchGlobalPermissionsAction implements PermissionsWsAction {
     return dbClient.groupPermissionDao().countGroupsByQuery(dbSession, query);
   }
 
-  private int countUsers(DbSession dbSession, OrganizationDto org, PermissionQuery permissionQuery) {
-    return dbClient.userPermissionDao().countUsers(dbSession, org.getUuid(), permissionQuery);
+  private int countUsers(DbSession dbSession, PermissionQuery permissionQuery) {
+    return dbClient.userPermissionDao().countUsersByQuery(dbSession, permissionQuery);
   }
 
   private static PermissionQuery permissionQuery(String permissionKey, OrganizationDto org) {

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 2398832dd18aa95d1b9bd35985b2929bb59f90f5..8c6f146ca520298bce0a5dc9887454c7e11ab518 100644 (file)
--- 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
@@ -105,7 +105,7 @@ public class UsersAction implements PermissionsWsAction {
 
       PermissionQuery query = buildPermissionQuery(request, org, projectId);
       List<UserDto> users = findUsers(dbSession, query);
-      int total = dbClient.userPermissionDao().countUsers(dbSession, org.getUuid(), query);
+      int total = dbClient.userPermissionDao().countUsersByQuery(dbSession, query);
       List<UserPermissionDto> userPermissions = findUserPermissions(dbSession, org, users, projectId);
       Paging paging = Paging.forPageIndex(request.mandatoryParamAsInt(Param.PAGE)).withPageSize(query.getPageSize()).andTotal(total);
       UsersWsResponse usersWsResponse = buildResponse(users, userPermissions, paging);
@@ -162,7 +162,7 @@ public class UsersAction implements PermissionsWsAction {
   }
 
   private List<UserDto> findUsers(DbSession dbSession, PermissionQuery query) {
-    List<Integer> orderedIds = dbClient.userPermissionDao().selectUserIds(dbSession, query);
+    List<Integer> orderedIds = dbClient.userPermissionDao().selectUserIdsByQuery(dbSession, query);
     return Ordering.explicit(orderedIds).onResultOf(UserDto::getId).immutableSortedCopy(dbClient.userDao().selectByIds(dbSession, orderedIds));
   }
 
@@ -170,12 +170,12 @@ public class UsersAction implements PermissionsWsAction {
     if (users.isEmpty()) {
       return emptyList();
     }
-    List<String> logins = users.stream().map(UserDto::getLogin).collect(Collectors.toList());
+    List<Integer> userIds = users.stream().map(UserDto::getId).collect(Collectors.toList());
     PermissionQuery query = PermissionQuery.builder()
       .setOrganizationUuid(org.getUuid())
-      .setComponentUuid(project.isPresent() ? project.get().getUuid() : null)
+      .setComponentUuid(project.map(ProjectId::getUuid).orElse(null))
       .withAtLeastOnePermission()
       .build();
-    return dbClient.userPermissionDao().select(dbSession, query, logins);
+    return dbClient.userPermissionDao().selectUserPermissionsByQuery(dbSession, query, userIds);
   }
 }

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 45fed320124973ad7e5bf91a7566a4db11516213..092802f619df32ba220261e34fa6a44f86b2b5d8 100644 (file)
--- 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
@@ -30,7 +30,10 @@ import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.UnauthorizedException;
 
+import static org.apache.commons.lang.StringUtils.countMatches;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.api.server.ws.WebService.Param.PAGE;
+import static org.sonar.api.server.ws.WebService.Param.PAGE_SIZE;
 import static org.sonar.api.server.ws.WebService.Param.TEXT_QUERY;
 import static org.sonar.api.web.UserRole.ISSUE_ADMIN;
 import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN;
@@ -38,6 +41,7 @@ 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.ADMINISTER_QUALITY_PROFILES;
+import static org.sonar.db.permission.OrganizationPermission.PROVISION_PROJECTS;
 import static org.sonar.db.permission.OrganizationPermission.SCAN;
 import static org.sonar.db.user.UserTesting.newUserDto;
 import static org.sonar.test.JsonAssert.assertJson;
@@ -58,9 +62,9 @@ public class UsersActionTest extends BasePermissionWsTest<UsersAction> {
     db.organizations().addMember(db.getDefaultOrganization(), user1);
     UserDto user2 = db.users().insertUser(newUserDto().setLogin("george.orwell").setName("George Orwell").setEmail("george.orwell@1984.net"));
     db.organizations().addMember(db.getDefaultOrganization(), user2);
+    db.users().insertPermissionOnUser(user1, ADMINISTER_QUALITY_PROFILES);
     db.users().insertPermissionOnUser(user1, ADMINISTER);
     db.users().insertPermissionOnUser(user1, ADMINISTER_QUALITY_GATES);
-    db.users().insertPermissionOnUser(user1, ADMINISTER_QUALITY_PROFILES);
     db.users().insertPermissionOnUser(user2, SCAN);
 
     loginAsAdmin(db.getDefaultOrganization());
@@ -224,6 +228,66 @@ public class UsersActionTest extends BasePermissionWsTest<UsersAction> {
     assertThat(result).contains("login-1", "login-2", "login-3");
   }
 
+  @Test
+  public void search_for_users_is_paginated() throws Exception {
+    for (int i = 9; i >= 0; i--) {
+      UserDto user = db.users().insertUser(newUserDto().setName("user-" + i));
+      db.organizations().addMember(db.getDefaultOrganization(), user);
+      db.users().insertPermissionOnUser(user, ADMINISTER);
+      db.users().insertPermissionOnUser(user, ADMINISTER_QUALITY_GATES);
+    }
+    loginAsAdmin(db.getDefaultOrganization());
+
+    assertJson(newRequest().setParam(PAGE, "1").setParam(PAGE_SIZE, "2").execute().getInput()).withStrictArrayOrder().isSimilarTo("{\n" +
+      "  \"paging\": {\n" +
+      "    \"pageIndex\": 1,\n" +
+      "    \"pageSize\": 2,\n" +
+      "    \"total\": 10\n" +
+      "  },\n" +
+      "  \"users\": [\n" +
+      "    {\n" +
+      "      \"name\": \"user-0\"\n" +
+      "    },\n" +
+      "    {\n" +
+      "      \"name\": \"user-1\"\n" +
+      "    }\n" +
+      "  ]\n" +
+      "}");
+    assertJson(newRequest().setParam(PAGE, "3").setParam(PAGE_SIZE, "4").execute().getInput()).withStrictArrayOrder().isSimilarTo("{\n" +
+      "  \"paging\": {\n" +
+      "    \"pageIndex\": 3,\n" +
+      "    \"pageSize\": 4,\n" +
+      "    \"total\": 10\n" +
+      "  },\n" +
+      "  \"users\": [\n" +
+      "    {\n" +
+      "      \"name\": \"user-8\"\n" +
+      "    },\n" +
+      "    {\n" +
+      "      \"name\": \"user-9\"\n" +
+      "    }\n" +
+      "  ]\n" +
+      "}");
+  }
+
+  @Test
+  public void return_more_than_20_permissions() {
+    loginAsAdmin(db.getDefaultOrganization());
+    for (int i = 0; i < 30; i++) {
+      UserDto user = db.users().insertUser(newUserDto().setLogin("user-" + i));
+      db.organizations().addMember(db.getDefaultOrganization(), user);
+      db.users().insertPermissionOnUser(user, SCAN);
+      db.users().insertPermissionOnUser(user, PROVISION_PROJECTS);
+    }
+
+    String result = newRequest()
+      .setParam(PAGE_SIZE, "100")
+      .execute()
+      .getInput();
+
+    assertThat(countMatches(result, "scan")).isEqualTo(30);
+  }
+
   @Test
   public void fail_if_project_permission_without_project() throws Exception {
     loginAsAdmin(db.getDefaultOrganization());

diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/ApplyTemplateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/ApplyTemplateActionTest.java
index e085cf10041b6c0b17566de2ae30e1f813034fc8..7a546890321b6992fbbc3902d6ccc8f33fc8fb5d 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/ApplyTemplateActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/ApplyTemplateActionTest.java
@@ -224,6 +224,6 @@ public class ApplyTemplateActionTest extends BasePermissionWsTest<ApplyTemplateA
 
   private List<Integer> selectProjectPermissionUsers(ComponentDto project, String permission) {
     PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(project.getOrganizationUuid()).setPermission(permission).setComponentUuid(project.uuid()).build();
-    return db.getDbClient().userPermissionDao().selectUserIds(db.getSession(), query);
+    return db.getDbClient().userPermissionDao().selectUserIdsByQuery(db.getSession(), query);
   }
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java
index 929eb39b4647e2cacc7759a2a539a20a3d9d7a19..1011572417e451bcd1dfc2a830d95e80a2c676d2 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/BulkApplyTemplateActionTest.java
@@ -263,6 +263,6 @@ public class BulkApplyTemplateActionTest extends BasePermissionWsTest<BulkApplyT
 
   private List<Integer> selectProjectPermissionUsers(ComponentDto project, String permission) {
     PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(project.getOrganizationUuid()).setPermission(permission).setComponentUuid(project.uuid()).build();
-    return db.getDbClient().userPermissionDao().selectUserIds(db.getSession(), query);
+    return db.getDbClient().userPermissionDao().selectUserIdsByQuery(db.getSession(), query);
   }
 }

diff --git a/server/sonar-server/src/test/resources/org/sonar/server/permission/ws/UsersActionTest/search_for_users_is_paginated.json b/server/sonar-server/src/test/resources/org/sonar/server/permission/ws/UsersActionTest/search_for_users_is_paginated.json
new file mode 100644 (file)
index 0000000..eef7bf0
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/permission/ws/UsersActionTest/search_for_users_is_paginated.json
@@ -0,0 +1,28 @@
+{
+  "paging": {
+    "pageIndex": 1,
+    "pageSize": 20,
+    "total": 2
+  },
+  "users": [
+    {
+      "login": "admin",
+      "name": "Administrator",
+      "email": "admin@admin.com",
+      "permissions": [
+        "admin",
+        "gateadmin",
+        "profileadmin"
+      ]
+    },
+    {
+      "login": "george.orwell",
+      "name": "George Orwell",
+      "email": "george.orwell@1984.net",
+      "permissions": [
+        "scan"
+      ]
+    }
+  ]
+}
+