Skip to content

Commit

Permalink
SONAR-9287 Fix limited number of returned permissions in api/permissi…
Browse files Browse the repository at this point in the history 
…ons/users
  • Loading branch information
@julienlancelot
julienlancelot committed May 30, 2017
1 parent 98695ad commit f91190b
Show file tree
Hide file tree
Showing 12 changed files with 345 additions and 179 deletions.
4 changes: 2 additions & 2 deletions server/sonar-db-dao/src/main/java/org/sonar/db/permission/PermissionQuery.java
View file
Expand Up @@ -134,8 +134,8 @@ public static class Builder {
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
Expand Down
51 changes: 21 additions & 30 deletions server/sonar-db-dao/src/main/java/org/sonar/db/permission/UserPermissionDao.java
View file
Expand Up @@ -22,12 +22,11 @@
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;
Expand All @@ -37,40 +36,32 @@
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);
}

/**
Expand Down
14 changes: 5 additions & 9 deletions server/sonar-db-dao/src/main/java/org/sonar/db/permission/UserPermissionMapper.java
View file
Expand Up @@ -22,23 +22,19 @@
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.
Expand Down
73 changes: 42 additions & 31 deletions server/sonar-db-dao/src/main/resources/org/sonar/db/permission/UserPermissionMapper.xml
View file
Expand Up @@ -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">
Expand Down
88 changes: 88 additions & 0 deletions server/sonar-db-dao/src/test/java/org/sonar/db/permission/PermissionQueryTest.java
View file
@@ -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();
}
}

0 comments on commit f91190b

Please sign in to comment.