From f91190bf830b3f79fb30a74b087d89dba213762d Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 29 May 2017 10:39:00 +0200 Subject: [PATCH] SONAR-9287 Fix limited number of returned permissions in api/permissions/users --- .../sonar/db/permission/PermissionQuery.java | 4 +- .../db/permission/UserPermissionDao.java | 51 ++--- .../db/permission/UserPermissionMapper.java | 14 +- .../db/permission/UserPermissionMapper.xml | 73 ++++--- .../db/permission/PermissionQueryTest.java | 88 +++++++++ .../db/permission/UserPermissionDaoTest.java | 180 ++++++++---------- .../ws/SearchGlobalPermissionsAction.java | 6 +- .../server/permission/ws/UsersAction.java | 10 +- .../server/permission/ws/UsersActionTest.java | 66 ++++++- .../ws/template/ApplyTemplateActionTest.java | 2 +- .../template/BulkApplyTemplateActionTest.java | 2 +- .../search_for_users_is_paginated.json | 28 +++ 12 files changed, 345 insertions(+), 179 deletions(-) create mode 100644 server/sonar-db-dao/src/test/java/org/sonar/db/permission/PermissionQueryTest.java create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/permission/ws/UsersActionTest/search_for_users_is_paginated.json 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 54788cecaba..d70c5f1994c 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 @@ -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 8ffeb772d29..7f74b5ee284 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 @@ -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 select(DbSession dbSession, PermissionQuery query, @Nullable Collection 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 selectUserPermissionsByQuery(DbSession dbSession, PermissionQuery query, Collection 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 selectUserIds(DbSession dbSession, PermissionQuery query) { - List dtos = select(dbSession, query, null); - return dtos.stream() - .map(UserPermissionDto::getUserId) - .distinct() - .collect(MoreCollectors.toList(dtos.size())); + public List 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 5c78b79d38a..57b90c9f51a 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 @@ -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 selectByQuery(@Param("query") PermissionQuery query, @Nullable @Param("userLogins") Collection userLogins, RowBounds rowBounds); + List selectUserPermissionsByQueryAndUserIds(@Param("query") PermissionQuery query, @Param("userIds") Collection userIds); + + List 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 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 bfe02d6267d..4c2b0c2a719 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 @@ -3,55 +3,66 @@ - select u.id as userId, ur.organization_uuid as organizationUuid, ur.resource_id as componentId, ur.role as permission - - order by lower(u.name), u.name, ur.role + + + u.id in #{userId,jdbcType=INTEGER} + + + + + - + 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} - - - and u.active = ${_true} - - - and u.login in #{userLogin,jdbcType=VARCHAR} - - - 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 '/') + + + and u.active = ${_true} + + 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 '/') + + + + and ur.organization_uuid = om.organization_uuid + and ur.role is not null + + and ur.resource_id is null - - - and ur.organization_uuid = #{query.organizationUuid,jdbcType=VARCHAR} - and ur.role is not null - - and ur.resource_id is null - - - and p.uuid = #{query.componentUuid,jdbcType=VARCHAR} - - - and ur.role = #{query.permission,jdbcType=VARCHAR} - + + and p.uuid = #{query.componentUuid,jdbcType=VARCHAR} - + + and ur.role = #{query.permission,jdbcType=VARCHAR} + +