From 1655663b7e4ea2249f2d1b5f5db05e6599be783a Mon Sep 17 00:00:00 2001 From: Aurelien Poscia Date: Fri, 21 Apr 2023 21:22:42 +0200 Subject: [PATCH] SONAR-19084 Compute isManaged flag correctly for GitHub users --- .../github/GitHubManagedInstanceService.java | 23 +++++--- .../it/java/org/sonar/db/user/UserDaoIT.java | 38 +++++++++++- .../main/java/org/sonar/db/user/UserDao.java | 21 +++++-- .../java/org/sonar/db/user/UserQuery.java | 17 ++++++ .../org/sonar/db/user/UserMapper.xml | 6 ++ .../java/org/sonar/db/user/UserQueryTest.java | 58 +++++++++++++++++++ 6 files changed, 150 insertions(+), 13 deletions(-) create mode 100644 server/sonar-db-dao/src/test/java/org/sonar/db/user/UserQueryTest.java diff --git a/server/sonar-auth-github/src/main/java/org/sonar/auth/github/GitHubManagedInstanceService.java b/server/sonar-auth-github/src/main/java/org/sonar/auth/github/GitHubManagedInstanceService.java index e88da1259f9..7921892fe92 100644 --- a/server/sonar-auth-github/src/main/java/org/sonar/auth/github/GitHubManagedInstanceService.java +++ b/server/sonar-auth-github/src/main/java/org/sonar/auth/github/GitHubManagedInstanceService.java @@ -53,18 +53,27 @@ public class GitHubManagedInstanceService implements ManagedInstanceService { @Override public Map getUserUuidToManaged(DbSession dbSession, Set userUuids) { + Set gitHubUserUuids = findManagedUserUuids(dbSession, userUuids); + + return userUuids.stream() + .collect(toMap(Function.identity(), gitHubUserUuids::contains)); + } + + private Set findManagedUserUuids(DbSession dbSession, Set userUuids) { + List userDtos = findManagedUsers(dbSession, userUuids); + + return userDtos.stream() + .map(UserDto::getUuid) + .collect(toSet()); + } + + private List findManagedUsers(DbSession dbSession, Set userUuids) { UserQuery managedUsersQuery = UserQuery.builder() .userUuids(userUuids) .isManagedClause(getManagedUsersSqlFilter(true)) .build(); - List userDtos = userDao.selectUsers(dbSession, managedUsersQuery); - Set gitHubUserUuids = userDtos.stream() - .map(UserDto::getUuid) - .collect(toSet()); - - return userUuids.stream() - .collect(toMap(Function.identity(), gitHubUserUuids::contains)); + return userDao.selectUsers(dbSession, managedUsersQuery); } @Override diff --git a/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoIT.java b/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoIT.java index 42f99253f63..1befd30a3c2 100644 --- a/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoIT.java +++ b/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoIT.java @@ -30,7 +30,6 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.function.Function; -import java.util.stream.Collectors; import java.util.stream.IntStream; import org.junit.Rule; import org.junit.Test; @@ -50,6 +49,7 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static java.util.Collections.singletonList; import static java.util.stream.Collectors.toMap; +import static java.util.stream.Collectors.toSet; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.groups.Tuple.tuple; @@ -249,6 +249,40 @@ public class UserDaoIT { assertThat(users).isEmpty(); } + @Test + public void selectUsersByQuery_whenSearchingByUuids_findsTheRightResults() { + db.users().insertUser(); + UserDto userToFind1 = db.users().insertUser(); + UserDto userToFind2 = db.users().insertUser(); + + UserQuery query = UserQuery.builder().userUuids(Set.of(userToFind1.getUuid(), userToFind2.getUuid())).build(); + List users = underTest.selectUsers(session, query); + + assertThat(users).usingRecursiveFieldByFieldElementComparator().containsOnly(userToFind1, userToFind2); + assertThat(underTest.countUsers(session, query)).isEqualTo(2); + } + + @Test + public void selectUsersByQuery_whenSearchingByUuidsWithLongRange_shouldReturnTheExpectedUsers() { + db.users().insertUser(); + List users = generateAndInsertUsers(3200); + Set userUuids = users.stream() + .map(UserDto::getUuid) + .collect(toSet()); + + UserQuery query = UserQuery.builder().userUuids(userUuids).build(); + List actualUsers = underTest.selectUsers(session, query); + + assertThat(actualUsers.stream().map(UserDto::getUuid).collect(toSet())) + .containsExactlyInAnyOrderElementsOf(userUuids); + } + + private List generateAndInsertUsers(int totalUsers) { + return IntStream.range(0, totalUsers) + .mapToObj(i -> db.users().insertUser()) + .toList(); + } + @DataProvider public static Object[][] paginationTestCases() { return new Object[][] { @@ -288,7 +322,7 @@ public class UserDaoIT { } return IntStream.range(1000 + (offset - 1) * limit, 1000 + offset * limit) .mapToObj(i -> allUsers.get(i + "_name")) - .collect(Collectors.toSet()); + .collect(toSet()); } @Test diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDao.java index 57a30a3fa00..e61938ac2a9 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDao.java @@ -99,12 +99,25 @@ public class UserDao implements Dao { .toList(); } - public List selectUsers(DbSession dbSession, UserQuery query) { - return mapper(dbSession).selectUsers(query, Pagination.all()); + public List selectUsers(DbSession dbSession, UserQuery userQuery) { + return selectUsers(dbSession, userQuery, Pagination.all()); } - public List selectUsers(DbSession dbSession, UserQuery query, int offset, int limit) { - return mapper(dbSession).selectUsers(query, Pagination.forPage(offset).andSize(limit)); + public List selectUsers(DbSession dbSession, UserQuery userQuery, int offset, int limit) { + Pagination pagination = Pagination.forPage(offset).andSize(limit); + return selectUsers(dbSession, userQuery, pagination); + } + + private List selectUsers(DbSession dbSession, UserQuery userQuery, Pagination pagination) { + if (userQuery.getUserUuids() != null) { + return executeLargeInputs( + userQuery.getUserUuids(), + partialSetOfUsers -> mapper(dbSession).selectUsers( + UserQuery.copyWithNewRangeOfUserUuids(userQuery, partialSetOfUsers), + pagination) + ); + } + return mapper(dbSession).selectUsers(userQuery, pagination); } public int countUsers(DbSession dbSession, UserQuery userQuery) { diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserQuery.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserQuery.java index e14d3e72c5f..003083807a8 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserQuery.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserQuery.java @@ -21,6 +21,8 @@ package org.sonar.db.user; import java.time.OffsetDateTime; import java.time.temporal.ChronoUnit; +import java.util.Collection; +import java.util.HashSet; import java.util.Set; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -36,6 +38,21 @@ public class UserQuery { private final Long sonarLintLastConnectionDateTo; private final Set userUuids; + public static UserQuery copyWithNewRangeOfUserUuids(UserQuery userQuery, Collection userUuids) { + return new UserQuery(userQuery, userUuids); + } + + private UserQuery(UserQuery userQuery, Collection userUuids) { + this.searchText = userQuery.getSearchText(); + this.isActive = userQuery.isActive(); + this.isManagedSqlClause = userQuery.getIsManagedSqlClause(); + this.lastConnectionDateFrom = userQuery.getLastConnectionDateFrom(); + this.lastConnectionDateTo = userQuery.getLastConnectionDateTo(); + this.sonarLintLastConnectionDateTo = userQuery.getSonarLintLastConnectionDateTo(); + this.sonarLintLastConnectionDateFrom = userQuery.getSonarLintLastConnectionDateFrom(); + this.userUuids = new HashSet<>(userUuids); + } + private UserQuery(@Nullable String searchText, @Nullable Boolean isActive, @Nullable String isManagedSqlClause, @Nullable OffsetDateTime lastConnectionDateFrom, @Nullable OffsetDateTime lastConnectionDateTo, @Nullable OffsetDateTime sonarLintLastConnectionDateFrom, @Nullable OffsetDateTime sonarLintLastConnectionDateTo, @Nullable Set userUuids) { diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/user/UserMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/user/UserMapper.xml index 3f0612c39a1..4b9ff1521de 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/user/UserMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/user/UserMapper.xml @@ -174,6 +174,12 @@ AND u.active=#{query.isActive, jdbcType=BOOLEAN} + + AND u.uuid in + + #{userUuid, jdbcType=VARCHAR} + + AND ( (lower(u.login) LIKE lower(#{query.searchText, jdbcType=VARCHAR}) ESCAPE '/') diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserQueryTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserQueryTest.java new file mode 100644 index 00000000000..3e54bb32164 --- /dev/null +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserQueryTest.java @@ -0,0 +1,58 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 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.user; + +import java.time.OffsetDateTime; +import java.time.temporal.ChronoUnit; +import java.util.Set; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class UserQueryTest { + + @Test + public void copyWithNewRangeOfUserUuids_copyAllFieldsCorrectly() { + UserQuery original = createUserQueryWithAllFieldsSet(); + + Set newRangeOfUsers = Set.of("user1"); + UserQuery copy = UserQuery.copyWithNewRangeOfUserUuids(original, newRangeOfUsers); + + assertThat(copy) + .usingRecursiveComparison() + .ignoringFields("userUuids") + .isEqualTo(original); + + assertThat(copy.getUserUuids()).isEqualTo(newRangeOfUsers); + } + + private static UserQuery createUserQueryWithAllFieldsSet() { + return UserQuery.builder() + .userUuids(Set.of("user1", "user2")) + .searchText("search text") + .isActive(true) + .isManagedClause("is managed clause") + .lastConnectionDateFrom(OffsetDateTime.now().minus(1, ChronoUnit.DAYS)) + .lastConnectionDateTo(OffsetDateTime.now().plus(1, ChronoUnit.DECADES)) + .sonarLintLastConnectionDateFrom(OffsetDateTime.now().plus(2, ChronoUnit.DAYS)) + .sonarLintLastConnectionDateTo(OffsetDateTime.now().minus(2, ChronoUnit.DECADES)) + .build(); + } +} -- 2.39.5