From 7e47c81f79f0cd31d533a90e19264ff60afe75de Mon Sep 17 00:00:00 2001 From: Aurelien Poscia Date: Mon, 27 Mar 2023 17:36:41 +0200 Subject: [PATCH] SONAR-18728 Use table SCM_ACCOUNTS instead of SCM_ACCOUNTS column in USERS --- .../java/org/sonar/db/version/SqTables.java | 1 + .../it/java/org/sonar/db/user/UserDaoIT.java | 62 ++++++-- .../sonar/db/user/UserDaoWithPersisterIT.java | 5 +- .../sonar/db/audit/model/UserNewValue.java | 12 +- .../main/java/org/sonar/db/user/UserDao.java | 19 ++- .../main/java/org/sonar/db/user/UserDto.java | 43 ++---- .../java/org/sonar/db/user/UserMapper.java | 6 +- .../org/sonar/db/user/UserMapper.xml | 142 ++++++++++-------- server/sonar-db-dao/src/schema/schema-sq.ddl | 2 +- .../db/audit/model/UserNewValueTest.java | 40 +++-- .../java/org/sonar/db/user/UserDtoTest.java | 45 ------ .../java/org/sonar/db/user/UserTesting.java | 3 +- .../version/v100/CreateScmAccountsTable.java | 2 +- .../schema.sql | 2 +- .../schema.sql | 2 +- .../server/user/index/UserIndexerIT.java | 4 +- .../sonar/server/user/index/UserIndexer.java | 3 +- .../org/sonar/server/user/UserUpdater.java | 8 +- .../server/user/UserUpdaterCreateTest.java | 10 +- .../user/UserUpdaterReactivateTest.java | 4 +- .../server/user/UserUpdaterUpdateTest.java | 24 +-- .../server/user/ws/AnonymizeActionIT.java | 3 +- .../sonar/server/user/ws/CurrentActionIT.java | 5 +- .../server/user/ws/DeactivateActionIT.java | 2 +- .../sonar/server/user/ws/UpdateActionIT.java | 13 +- .../sonar/server/user/ws/CreateAction.java | 2 +- .../sonar/server/user/ws/CurrentAction.java | 2 +- .../sonar/server/user/ws/SearchAction.java | 4 +- .../sonar/server/user/ws/UserJsonWriter.java | 2 +- 29 files changed, 250 insertions(+), 222 deletions(-) delete mode 100644 server/sonar-db-dao/src/test/java/org/sonar/db/user/UserDtoTest.java diff --git a/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java b/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java index 16c98a0e284..c6a6b80dcc8 100644 --- a/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java +++ b/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java @@ -101,6 +101,7 @@ public final class SqTables { "schema_migrations", "scim_groups", "scim_users", + "scm_accounts", "session_tokens", "snapshots", "users", 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 bb0be8dfd00..f68c75e14ea 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 @@ -27,6 +27,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -78,6 +79,17 @@ public class UserDaoIT { assertThat(underTest.selectByUuid(session, "unknown")).isNull(); } + @Test + public void selectByUuid_withScmAccount_retrievesScmAccounts() { + List scmAccountsUser1 = List.of("account1_1", "account1_2"); + UserDto user1 = db.users().insertUser(u -> u.setScmAccounts(scmAccountsUser1)); + UserDto user2 = db.users().insertUser(u -> u.setScmAccounts(List.of("account2_1", "account2_2"))); + + UserDto userDto = underTest.selectByUuid(session, user1.getUuid()); + assertThat(userDto).isNotNull(); + assertThat(userDto.getSortedScmAccounts()).containsExactlyElementsOf(scmAccountsUser1); + } + @Test public void selectUserByLogin_ignore_inactive() { db.users().insertUser(user -> user.setLogin("user1")); @@ -282,8 +294,7 @@ public class UserDaoIT { assertThat(user.isResetPassword()).isFalse(); assertThat(user.isLocal()).isTrue(); - assertThat(user.getScmAccountsAsList()).isEmpty(); - assertThat(user.getScmAccounts()).isNull(); + assertThat(user.getSortedScmAccounts()).isEmpty(); assertThat(user.getHashMethod()).isNull(); assertThat(user.getLastConnectionDate()).isNull(); assertThat(user.getHomepageType()).isNull(); @@ -298,7 +309,7 @@ public class UserDaoIT { .setLogin("john") .setName("John") .setEmail("jo@hn.com") - .setScmAccounts(",jo.hn,john2,") + .setScmAccounts(List.of("jo.hn", "john2", "")) .setActive(true) .setResetPassword(true) .setSalt("1234") @@ -323,7 +334,7 @@ public class UserDaoIT { assertThat(user.getEmail()).isEqualTo("jo@hn.com"); assertThat(user.isActive()).isTrue(); assertThat(user.isResetPassword()).isTrue(); - assertThat(user.getScmAccounts()).isEqualTo(",jo.hn,john2,"); + assertThat(user.getSortedScmAccounts()).containsExactly("jo.hn", "john2"); assertThat(user.getSalt()).isEqualTo("1234"); assertThat(user.getCryptedPassword()).isEqualTo("abcd"); assertThat(user.getHashMethod()).isEqualTo("SHA1"); @@ -361,7 +372,7 @@ public class UserDaoIT { .setLogin("johnDoo") .setName("John Doo") .setEmail("jodoo@hn.com") - .setScmAccounts(",jo.hn,john2,johndoo,") + .setScmAccounts(List.of("jo.hn", "john2", "johndoo", "")) .setActive(false) .setResetPassword(true) .setSalt("12345") @@ -383,7 +394,7 @@ public class UserDaoIT { assertThat(reloaded.getEmail()).isEqualTo("jodoo@hn.com"); assertThat(reloaded.isActive()).isFalse(); assertThat(reloaded.isResetPassword()).isTrue(); - assertThat(reloaded.getScmAccounts()).isEqualTo(",jo.hn,john2,johndoo,"); + assertThat(reloaded.getSortedScmAccounts()).containsExactly("jo.hn", "john2", "johndoo"); assertThat(reloaded.getSalt()).isEqualTo("12345"); assertThat(reloaded.getCryptedPassword()).isEqualTo("abcde"); assertThat(reloaded.getHashMethod()).isEqualTo("BCRYPT"); @@ -396,6 +407,23 @@ public class UserDaoIT { assertThat(reloaded.getLastConnectionDate()).isEqualTo(10_000_000_000L); } + @Test + public void update_scmAccounts() { + UserDto user = db.users().insertUser(u -> u.setScmAccounts(emptyList())); + + underTest.update(db.getSession(), user.setScmAccounts(List.of("jo.hn", "john2", "johndoo", ""))); + UserDto reloaded = Objects.requireNonNull(underTest.selectByUuid(db.getSession(), user.getUuid())); + assertThat(reloaded.getSortedScmAccounts()).containsExactly("jo.hn", "john2", "johndoo"); + + underTest.update(db.getSession(), user.setScmAccounts(List.of("jo.hn", "john2"))); + reloaded = Objects.requireNonNull(underTest.selectByUuid(db.getSession(), user.getUuid())); + assertThat(reloaded.getSortedScmAccounts()).containsExactly("jo.hn", "john2"); + + underTest.update(db.getSession(), user.setScmAccounts(List.of("jo.hn", "john3", "john2"))); + reloaded = Objects.requireNonNull(underTest.selectByUuid(db.getSession(), user.getUuid())); + assertThat(reloaded.getSortedScmAccounts()).containsExactly("jo.hn", "john2", "john3"); + } + @Test public void deactivate_user() { UserDto user = insertActiveUser(); @@ -414,7 +442,7 @@ public class UserDaoIT { assertThat(userReloaded.getExternalLogin()).isEqualTo(user.getExternalLogin()); assertThat(userReloaded.getExternalIdentityProvider()).isEqualTo(user.getExternalIdentityProvider()); assertThat(userReloaded.getEmail()).isNull(); - assertThat(userReloaded.getScmAccounts()).isNull(); + assertThat(userReloaded.getSortedScmAccounts()).isEmpty(); assertThat(userReloaded.getSalt()).isNull(); assertThat(userReloaded.getCryptedPassword()).isNull(); assertThat(userReloaded.getUpdatedAt()).isEqualTo(NOW); @@ -505,7 +533,7 @@ public class UserDaoIT { .setName("Marius") .setEmail("marius@lesbronzes.fr") .setActive(true) - .setScmAccounts("\nma\nmarius33\n") + .setScmAccounts(List.of("ma", "marius33")) .setSalt("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365") .setCryptedPassword("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg") .setHomepageType("project") @@ -517,7 +545,7 @@ public class UserDaoIT { assertThat(dto.getName()).isEqualTo("Marius"); assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr"); assertThat(dto.isActive()).isTrue(); - assertThat(dto.getScmAccountsAsList()).containsOnly("ma", "marius33"); + assertThat(dto.getSortedScmAccounts()).containsExactly("ma", "marius33"); assertThat(dto.getSalt()).isEqualTo("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365"); assertThat(dto.getCryptedPassword()).isEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"); assertThat(dto.getCreatedAt()).isEqualTo(user1.getCreatedAt()); @@ -530,9 +558,12 @@ public class UserDaoIT { @Test public void select_nullable_by_scm_account() { db.users().insertUser(user -> user.setLogin("marius").setName("Marius").setEmail("marius@lesbronzes.fr").setScmAccounts(asList("ma", "marius33"))); - db.users().insertUser(user -> user.setLogin("sbrandhof").setName("Simon Brandhof").setEmail("sbrandhof@lesbronzes.fr").setScmAccounts((String) null)); + db.users().insertUser(user -> user.setLogin("sbrandhof").setName("Simon Brandhof").setEmail("sbrandhof@lesbronzes.fr").setScmAccounts(emptyList())); + + List searchByMa = underTest.selectByScmAccountOrLoginOrEmail(session, "ma"); + assertThat(searchByMa).extracting(UserDto::getLogin).containsExactly("marius"); + assertThat(searchByMa.iterator().next().getSortedScmAccounts()).containsExactly("ma", "marius33"); - assertThat(underTest.selectByScmAccountOrLoginOrEmail(session, "ma")).extracting(UserDto::getLogin).containsExactly("marius"); assertThat(underTest.selectByScmAccountOrLoginOrEmail(session, "marius")).extracting(UserDto::getLogin).containsExactly("marius"); assertThat(underTest.selectByScmAccountOrLoginOrEmail(session, "marius@lesbronzes.fr")).extracting(UserDto::getLogin).containsExactly("marius"); assertThat(underTest.selectByScmAccountOrLoginOrEmail(session, "m")).isEmpty(); @@ -542,7 +573,7 @@ public class UserDaoIT { @Test public void select_nullable_by_scm_account_return_many_results_when_same_email_is_used_by_many_users() { db.users().insertUser(user -> user.setLogin("marius").setName("Marius").setEmail("marius@lesbronzes.fr").setScmAccounts(asList("ma", "marius33"))); - db.users().insertUser(user -> user.setLogin("sbrandhof").setName("Simon Brandhof").setEmail("marius@lesbronzes.fr").setScmAccounts((String) null)); + db.users().insertUser(user -> user.setLogin("sbrandhof").setName("Simon Brandhof").setEmail("marius@lesbronzes.fr").setScmAccounts(emptyList())); List results = underTest.selectByScmAccountOrLoginOrEmail(session, "marius@lesbronzes.fr"); @@ -653,7 +684,8 @@ public class UserDaoIT { List result = underTest.selectUsersForTelemetry(db.getSession()); assertThat(result) - .extracting(UserTelemetryDto::getUuid, UserTelemetryDto::isActive, UserTelemetryDto::getLastConnectionDate, UserTelemetryDto::getLastSonarlintConnectionDate, UserTelemetryDto::getScimUuid) + .extracting(UserTelemetryDto::getUuid, UserTelemetryDto::isActive, UserTelemetryDto::getLastConnectionDate, UserTelemetryDto::getLastSonarlintConnectionDate, + UserTelemetryDto::getScimUuid) .containsExactlyInAnyOrder( tuple(u1.getUuid(), u1.isActive(), u1.getLastConnectionDate(), u1.getLastSonarlintConnectionDate(), null), tuple(u2.getUuid(), u2.isActive(), u2.getLastConnectionDate(), u2.getLastSonarlintConnectionDate(), null) @@ -690,7 +722,8 @@ public class UserDaoIT { List result = underTest.selectUsersForTelemetry(db.getSession()); assertThat(result) - .extracting(UserTelemetryDto::getUuid, UserTelemetryDto::isActive, UserTelemetryDto::getLastConnectionDate, UserTelemetryDto::getLastSonarlintConnectionDate, UserTelemetryDto::getScimUuid) + .extracting(UserTelemetryDto::getUuid, UserTelemetryDto::isActive, UserTelemetryDto::getLastConnectionDate, UserTelemetryDto::getLastSonarlintConnectionDate, + UserTelemetryDto::getScimUuid) .containsExactlyInAnyOrder( tuple(u1.getUuid(), u1.isActive(), u1.getLastConnectionDate(), u1.getLastSonarlintConnectionDate(), scimUser1.getScimUserUuid()), tuple(u2.getUuid(), u2.isActive(), u2.getLastConnectionDate(), u2.getLastSonarlintConnectionDate(), null) @@ -707,7 +740,6 @@ public class UserDaoIT { return dto; } - private ScimUserDto enableScimForUser(UserDto userDto) { return dbClient.scimUserDao().enableScimForUser(db.getSession(), userDto.getUuid()); } diff --git a/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoWithPersisterIT.java b/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoWithPersisterIT.java index 09dd9d2f3ac..17df16f7f77 100644 --- a/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoWithPersisterIT.java +++ b/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoWithPersisterIT.java @@ -20,6 +20,7 @@ package org.sonar.db.user; import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import java.util.List; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -89,7 +90,7 @@ public class UserDaoWithPersisterIT { .setLogin("johnDoo") .setName("John Doo") .setEmail("jodoo@hn.com") - .setScmAccounts(",jo.hn,john2,johndoo,") + .setScmAccounts(List.of("jo.hn","john2","johndoo")) .setActive(false) .setResetPassword(true) .setSalt("12345") @@ -111,7 +112,7 @@ public class UserDaoWithPersisterIT { UserNewValue::getScmAccounts, UserNewValue::getExternalId, UserNewValue::getExternalLogin, UserNewValue::getExternalIdentityProvider, UserNewValue::isLocal, UserNewValue::getLastConnectionDate) .containsExactly(updatedUser.getUuid(), updatedUser.getLogin(), updatedUser.getName(), updatedUser.getEmail(), updatedUser.isActive(), - updatedUser.getScmAccounts(), updatedUser.getExternalId(), updatedUser.getExternalLogin(), updatedUser.getExternalIdentityProvider(), + updatedUser.getSortedScmAccounts(), updatedUser.getExternalId(), updatedUser.getExternalLogin(), updatedUser.getExternalIdentityProvider(), updatedUser.isLocal(), updatedUser.getLastConnectionDate()); assertThat(newValue.toString()) .contains("name") diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/UserNewValue.java b/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/UserNewValue.java index dfea8d069c3..4f83bf3cc1e 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/UserNewValue.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/UserNewValue.java @@ -19,6 +19,8 @@ */ package org.sonar.db.audit.model; +import java.util.ArrayList; +import java.util.List; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.apache.commons.lang.ObjectUtils; @@ -40,8 +42,7 @@ public class UserNewValue extends NewValue { @Nullable private Boolean isActive; - @Nullable - private String scmAccounts; + private List scmAccounts = new ArrayList<>(); @Nullable private String externalId; @@ -69,7 +70,7 @@ public class UserNewValue extends NewValue { this.name = userDto.getName(); this.email = userDto.getEmail(); this.isActive = userDto.isActive(); - this.scmAccounts = userDto.getScmAccounts(); + this.scmAccounts = userDto.getSortedScmAccounts(); this.externalId = userDto.getExternalId(); this.externalLogin = userDto.getExternalLogin(); this.externalIdentityProvider = userDto.getExternalIdentityProvider(); @@ -100,8 +101,7 @@ public class UserNewValue extends NewValue { return this.isActive; } - @CheckForNull - public String getScmAccounts() { + public List getScmAccounts() { return this.scmAccounts; } @@ -138,7 +138,7 @@ public class UserNewValue extends NewValue { addField(sb, "\"name\": ", this.name, true); addField(sb, "\"email\": ", this.email, true); addField(sb, "\"isActive\": ", ObjectUtils.toString(this.isActive), false); - addField(sb, "\"scmAccounts\": ", this.scmAccounts, true); + addField(sb, "\"scmAccounts\": ", String.join(",", scmAccounts), true); addField(sb, "\"externalId\": ", this.externalId, true); addField(sb, "\"externalLogin\": ", this.externalLogin, true); addField(sb, "\"externalIdentityProvider\": ", this.externalIdentityProvider, true); 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 82b9a080e55..e42533ac9e2 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 @@ -28,6 +28,7 @@ import java.util.function.Consumer; import java.util.function.Function; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +import org.apache.commons.lang.StringUtils; import org.sonar.api.utils.System2; import org.sonar.core.util.UuidFactory; import org.sonar.db.Dao; @@ -42,7 +43,6 @@ import static java.util.Locale.ENGLISH; import static java.util.concurrent.TimeUnit.DAYS; import static org.sonar.db.DatabaseUtils.executeLargeInputs; import static org.sonar.db.DatabaseUtils.executeLargeInputsWithoutOutput; -import static org.sonar.db.user.UserDto.SCM_ACCOUNTS_SEPARATOR; public class UserDao implements Dao { private static final long WEEK_IN_MS = DAYS.toMillis(7L); @@ -78,6 +78,7 @@ public class UserDao implements Dao { /** * Select users by uuids, including disabled users. An empty list is returned * if list of uuids is empty, without any db round trips. + * * @return */ public List selectByUuids(DbSession session, Collection uuids) { @@ -117,16 +118,25 @@ public class UserDao implements Dao { public UserDto insert(DbSession session, UserDto dto) { long now = system2.now(); mapper(session).insert(dto.setUuid(uuidFactory.create()).setCreatedAt(now).setUpdatedAt(now)); + insertScmAccounts(session, dto.getUuid(), dto.getSortedScmAccounts()); auditPersister.addUser(session, new UserNewValue(dto.getUuid(), dto.getLogin())); return dto; } + private static void insertScmAccounts(DbSession session, String userUuid, List scmAccounts) { + scmAccounts.stream() + .filter(StringUtils::isNotBlank) + .forEach(scmAccount -> mapper(session).insertScmAccount(userUuid, scmAccount)); + } + public UserDto update(DbSession session, UserDto dto) { return update(session, dto, true); } public UserDto update(DbSession session, UserDto dto, boolean track) { mapper(session).update(dto.setUpdatedAt(system2.now())); + mapper(session).deleteAllScmAccounts(dto.getUuid()); + insertScmAccounts(session, dto.getUuid(), dto.getSortedScmAccounts()); if (track) { auditPersister.updateUser(session, new UserNewValue(dto)); } @@ -139,6 +149,7 @@ public class UserDao implements Dao { public void deactivateUser(DbSession dbSession, UserDto user) { mapper(dbSession).deactivateUser(user.getLogin(), system2.now()); + mapper(dbSession).deleteAllScmAccounts(user.getUuid()); auditPersister.deactivateUser(dbSession, new UserNewValue(user.getUuid(), user.getLogin())); } @@ -160,15 +171,11 @@ public class UserDao implements Dao { } public List selectByScmAccountOrLoginOrEmail(DbSession session, String scmAccountOrLoginOrEmail) { - String like = new StringBuilder().append("%") - .append(SCM_ACCOUNTS_SEPARATOR).append(scmAccountOrLoginOrEmail) - .append(SCM_ACCOUNTS_SEPARATOR).append("%").toString(); - return mapper(session).selectNullableByScmAccountOrLoginOrEmail(scmAccountOrLoginOrEmail, like); + return mapper(session).selectNullableByScmAccountOrLoginOrEmail(scmAccountOrLoginOrEmail); } /** * Search for an active user with the given emailCaseInsensitive exits in database - * * Select is case insensitive. Result for searching 'mail@emailCaseInsensitive.com' or 'Mail@Email.com' is the same */ public List selectByEmail(DbSession dbSession, String emailCaseInsensitive) { diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java index dcd31aac4ea..6ff3eab31c2 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java @@ -19,14 +19,15 @@ */ package org.sonar.db.user; -import com.google.common.base.Splitter; -import com.google.common.collect.Lists; import java.util.ArrayList; import java.util.List; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.core.user.DefaultUser; +import static java.lang.String.CASE_INSENSITIVE_ORDER; +import static java.util.Comparator.comparing; + /** * @since 3.2 */ @@ -39,7 +40,6 @@ public class UserDto implements UserId { private String name; private String email; private boolean active = true; - private String scmAccounts; private String externalId; private String externalLogin; private String externalIdentityProvider; @@ -53,6 +53,7 @@ public class UserDto implements UserId { private String homepageParameter; private boolean local = true; private boolean resetPassword = false; + private List scmAccounts = new ArrayList<>(); /** * Date of the last time the user has accessed to the server. @@ -121,41 +122,23 @@ public class UserDto implements UserId { return this; } - @CheckForNull - public String getScmAccounts() { + /** + * Used by mybatis + */ + private List getScmAccounts() { return scmAccounts; } - public List getScmAccountsAsList() { - return decodeScmAccounts(scmAccounts); - } - - public UserDto setScmAccounts(@Nullable String s) { - this.scmAccounts = s; - return this; + public List getSortedScmAccounts() { + // needs to be done when reading, as mybatis do not use the setter + return scmAccounts.stream().sorted(comparing(s -> s, CASE_INSENSITIVE_ORDER)).toList(); } - public UserDto setScmAccounts(@Nullable List list) { - this.scmAccounts = encodeScmAccounts(list); + public UserDto setScmAccounts(List scmAccounts) { + this.scmAccounts = scmAccounts; return this; } - @CheckForNull - public static String encodeScmAccounts(@Nullable List scmAccounts) { - if (scmAccounts != null && !scmAccounts.isEmpty()) { - return String.format("%s%s%s", SCM_ACCOUNTS_SEPARATOR, String.join(String.valueOf(SCM_ACCOUNTS_SEPARATOR), scmAccounts), SCM_ACCOUNTS_SEPARATOR); - } - return null; - } - - public static List decodeScmAccounts(@Nullable String dbValue) { - if (dbValue == null) { - return new ArrayList<>(); - } else { - return Lists.newArrayList(Splitter.on(SCM_ACCOUNTS_SEPARATOR).omitEmptyStrings().split(dbValue)); - } - } - public String getExternalId() { return externalId; } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserMapper.java index 58815a63abc..c0f6d6fcaee 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserMapper.java @@ -38,7 +38,7 @@ public interface UserMapper { * Can return multiple results if an email is used by many users (For instance, technical account can use the same email as a none technical account) */ @CheckForNull - List selectNullableByScmAccountOrLoginOrEmail(@Param("scmAccount") String scmAccountOrLoginOrEmail, @Param("likeScmAccount") String likeScmAccount); + List selectNullableByScmAccountOrLoginOrEmail(@Param("scmAccount") String scmAccountOrLoginOrEmail); /** * Select user by login. Note that disabled users are ignored. @@ -85,4 +85,8 @@ public interface UserMapper { long countActiveSonarlintUsers(@Param("sinceDate") long sinceDate); long countActiveUsers(); + + void insertScmAccount(@Param("userUuid") String userUuid, @Param("scmAccount") String scmAccount); + + void deleteAllScmAccounts(@Param("userUuid") String userUuid); } 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 78930597d29..8bef5ef227e 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 @@ -9,7 +9,6 @@ u.name as name, u.email as email, u.active as "active", - u.scm_accounts as "scmAccounts", u.salt as "salt", u.crypted_password as "cryptedPassword", u.hash_method as "hashMethod", @@ -26,75 +25,91 @@ u.updated_at as "updatedAt" - + WHERE u.uuid=#{uuid, jdbcType=VARCHAR} - + WHERE u.login=#{login, jdbcType=VARCHAR} - + + WHERE uuid IN ( + SELECT + u.uuid + FROM users u + + WHERE + u.login=#{scmAccount, jdbcType=VARCHAR} + OR u.email=#{scmAccount, jdbcType=VARCHAR} + OR sa.scm_account=#{scmAccount, jdbcType=VARCHAR} + ) - + WHERE u.login=#{id, jdbcType=INTEGER} AND u.active=${_true} - + WHERE u.login in #{login, jdbcType=VARCHAR} - + WHERE u.uuid in #{uuid, jdbcType=VARCHAR} - + + ORDER BY u.uuid, sa.scm_account - + ORDER BY u.login limit #{pagination.pageSize,jdbcType=INTEGER} offset #{pagination.offset,jdbcType=INTEGER} - SELECT - + FROM (SELECT @@ -104,11 +119,12 @@ offset #{pagination.offset} rows fetch next #{pagination.pageSize,jdbcType=INTEGER} rows only ) u + - SELECT - + FROM (SELECT rownum as rn, t.* from ( SELECT @@ -118,6 +134,7 @@ ORDER BY u.login ASC ) t ) u + WHERE u.rn BETWEEN #{pagination.startRowNumber,jdbcType=INTEGER} and #{pagination.endRowNumber,jdbcType=INTEGER} ORDER BY u.rn ASC @@ -184,10 +201,8 @@ ORDER BY u.uuid - + WHERE lower(u.email)=#{email, jdbcType=VARCHAR} AND u.active=${_true} @@ -196,17 +211,13 @@ SELECT distinct(external_identity_provider) from users - + WHERE u.external_id=#{externalId, jdbcType=VARCHAR} AND u.external_identity_provider=#{externalIdentityProvider, jdbcType=VARCHAR} - + WHERE u.external_identity_provider=#{externalIdentityProvider, jdbcType=VARCHAR} AND u.external_id in @@ -214,10 +225,8 @@ - + WHERE u.external_login=#{externalLogin, jdbcType=VARCHAR} AND u.external_identity_provider=#{externalIdentityProvider, jdbcType=VARCHAR} @@ -272,7 +281,6 @@ name, email, active, - scm_accounts, external_id, external_login, external_identity_provider, @@ -292,7 +300,6 @@ #{user.name,jdbcType=VARCHAR}, #{user.email,jdbcType=VARCHAR}, #{user.active,jdbcType=BOOLEAN}, - #{user.scmAccounts,jdbcType=VARCHAR}, #{user.externalId,jdbcType=VARCHAR}, #{user.externalLogin,jdbcType=VARCHAR}, #{user.externalIdentityProvider,jdbcType=VARCHAR}, @@ -315,7 +322,6 @@ name = #{user.name, jdbcType=VARCHAR}, email = #{user.email, jdbcType=VARCHAR}, active = #{user.active, jdbcType=BOOLEAN}, - scm_accounts = #{user.scmAccounts, jdbcType=VARCHAR}, external_id = #{user.externalId, jdbcType=VARCHAR}, external_login = #{user.externalLogin, jdbcType=VARCHAR}, external_identity_provider = #{user.externalIdentityProvider, jdbcType=VARCHAR}, @@ -342,4 +348,20 @@ select count(1) from users u WHERE u.active=${_true} + + insert into scm_accounts ( + user_uuid, + scm_account + ) values ( + #{userUuid,jdbcType=VARCHAR}, + #{scmAccount,jdbcType=VARCHAR} + ) + + + + delete from scm_accounts + where + user_uuid = #{userUuid,jdbcType=VARCHAR} + + diff --git a/server/sonar-db-dao/src/schema/schema-sq.ddl b/server/sonar-db-dao/src/schema/schema-sq.ddl index 371a507c20b..b07d5318d37 100644 --- a/server/sonar-db-dao/src/schema/schema-sq.ddl +++ b/server/sonar-db-dao/src/schema/schema-sq.ddl @@ -931,7 +931,7 @@ CREATE UNIQUE INDEX "UNIQ_SCIM_USERS_USER_UUID" ON "SCIM_USERS"("USER_UUID" NULL CREATE TABLE "SCM_ACCOUNTS"( "USER_UUID" CHARACTER VARYING(255) NOT NULL, - "SCM_ACCOUNT" CHARACTER VARYING(100) NOT NULL + "SCM_ACCOUNT" CHARACTER VARYING(255) NOT NULL ); ALTER TABLE "SCM_ACCOUNTS" ADD CONSTRAINT "PK_SCM_ACCOUNTS" PRIMARY KEY("USER_UUID", "SCM_ACCOUNT"); diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/audit/model/UserNewValueTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/audit/model/UserNewValueTest.java index 0fe33e64f83..46a2b5ba3e9 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/audit/model/UserNewValueTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/audit/model/UserNewValueTest.java @@ -19,10 +19,12 @@ */ package org.sonar.db.audit.model; +import java.util.List; import org.json.simple.parser.JSONParser; import org.junit.Test; import org.sonar.db.user.UserDto; +import static java.util.Collections.emptyList; import static org.junit.Assert.fail; public class UserNewValueTest { @@ -31,16 +33,18 @@ public class UserNewValueTest { @Test public void toString_givenAllFieldsWithValue_returnValidJSON() { - UserDto userDto = new UserDto(); - userDto.setName("name"); - userDto.setEmail("name@email.com"); - userDto.setActive(true); - userDto.setScmAccounts("\ngithub-account\n"); - userDto.setExternalId("name"); - userDto.setExternalLogin("name"); - userDto.setExternalIdentityProvider("github"); - userDto.setLocal(false); - userDto.setLastConnectionDate(System.currentTimeMillis()); + UserDto userDto = createUserDto(); + UserNewValue userNewValue = new UserNewValue(userDto); + + String jsonString = userNewValue.toString(); + + assertValidJSON(jsonString); + } + + @Test + public void toString_givenEmptyScmAccount_returnValidJSON() { + UserDto userDto = createUserDto(); + userDto.setScmAccounts(emptyList()); UserNewValue userNewValue = new UserNewValue(userDto); String jsonString = userNewValue.toString(); @@ -56,7 +60,21 @@ public class UserNewValueTest { assertValidJSON(jsonString); } - + + private static UserDto createUserDto() { + UserDto userDto = new UserDto(); + userDto.setName("name"); + userDto.setEmail("name@email.com"); + userDto.setActive(true); + userDto.setScmAccounts(List.of("github-account", "gitlab-account")); + userDto.setExternalId("name"); + userDto.setExternalLogin("name"); + userDto.setExternalIdentityProvider("github"); + userDto.setLocal(false); + userDto.setLastConnectionDate(System.currentTimeMillis()); + return userDto; + } + private void assertValidJSON(String jsonString) { try { jsonParser.parse(jsonString); diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserDtoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserDtoTest.java deleted file mode 100644 index 5588864a2d8..00000000000 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/user/UserDtoTest.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * 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.util.Arrays; -import java.util.Collections; -import org.junit.Test; - -import static org.assertj.core.api.Assertions.assertThat; - -public class UserDtoTest { - - - @Test - public void encode_scm_accounts() { - assertThat(UserDto.encodeScmAccounts(null)).isNull(); - assertThat(UserDto.encodeScmAccounts(Collections.emptyList())).isNull(); - assertThat(UserDto.encodeScmAccounts(Arrays.asList("foo"))).isEqualTo("\nfoo\n"); - assertThat(UserDto.encodeScmAccounts(Arrays.asList("foo", "bar"))).isEqualTo("\nfoo\nbar\n"); - } - - @Test - public void decode_scm_accounts() { - assertThat(UserDto.decodeScmAccounts(null)).isEmpty(); - assertThat(UserDto.decodeScmAccounts("\nfoo\n")).containsOnly("foo"); - assertThat(UserDto.decodeScmAccounts("\nfoo\nbar\n")).containsOnly("foo", "bar"); - } -} diff --git a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/user/UserTesting.java b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/user/UserTesting.java index a60dd160d3b..63762e339b5 100644 --- a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/user/UserTesting.java +++ b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/user/UserTesting.java @@ -21,6 +21,7 @@ package org.sonar.db.user; import javax.annotation.Nullable; +import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.apache.commons.lang.math.RandomUtils.nextBoolean; @@ -79,7 +80,7 @@ public class UserTesting { return newUserDto() .setActive(false) // All these fields are reset when disabling a user - .setScmAccounts((String) null) + .setScmAccounts(emptyList()) .setEmail(null) .setCryptedPassword(null) .setSalt(null); diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v100/CreateScmAccountsTable.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v100/CreateScmAccountsTable.java index 0cb9a6b8c50..66e11bc312e 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v100/CreateScmAccountsTable.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v100/CreateScmAccountsTable.java @@ -34,7 +34,7 @@ public class CreateScmAccountsTable extends CreateTableChange { static final String SCM_ACCOUNT_COLUMN_NAME = "scm_account"; @VisibleForTesting - static final int SCM_ACCOUNT_SIZE = 100; + static final int SCM_ACCOUNT_SIZE = 255; public CreateScmAccountsTable(Database db) { super(db, TABLE_NAME); diff --git a/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v100/CreateIndexesForScmAccountsTest/schema.sql b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v100/CreateIndexesForScmAccountsTest/schema.sql index eca4564d0de..7a113bbdda2 100644 --- a/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v100/CreateIndexesForScmAccountsTest/schema.sql +++ b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v100/CreateIndexesForScmAccountsTest/schema.sql @@ -1,5 +1,5 @@ CREATE TABLE "SCM_ACCOUNTS"( "USER_UUID" CHARACTER VARYING(255) NOT NULL, - "SCM_ACCOUNT" CHARACTER VARYING(100) NOT NULL + "SCM_ACCOUNT" CHARACTER VARYING(255) NOT NULL ); ALTER TABLE "SCM_ACCOUNTS" ADD CONSTRAINT "PK_SCM_ACCOUNTS" PRIMARY KEY("USER_UUID", "SCM_ACCOUNT"); diff --git a/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v100/MigrateScmAccountsFromUsersToScmAccountsTest/schema.sql b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v100/MigrateScmAccountsFromUsersToScmAccountsTest/schema.sql index 935cde8aa74..2f34178cb08 100644 --- a/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v100/MigrateScmAccountsFromUsersToScmAccountsTest/schema.sql +++ b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v100/MigrateScmAccountsFromUsersToScmAccountsTest/schema.sql @@ -28,6 +28,6 @@ CREATE UNIQUE INDEX "UNIQ_EXTERNAL_LOGIN" ON "USERS"("EXTERNAL_IDENTITY_PROVIDER CREATE TABLE "SCM_ACCOUNTS"( "USER_UUID" CHARACTER VARYING(255) NOT NULL, - "SCM_ACCOUNT" CHARACTER VARYING(100) NOT NULL + "SCM_ACCOUNT" CHARACTER VARYING(255) NOT NULL ); ALTER TABLE "SCM_ACCOUNTS" ADD CONSTRAINT "PK_SCM_ACCOUNTS" PRIMARY KEY("USER_UUID", "SCM_ACCOUNT"); diff --git a/server/sonar-server-common/src/it/java/org/sonar/server/user/index/UserIndexerIT.java b/server/sonar-server-common/src/it/java/org/sonar/server/user/index/UserIndexerIT.java index b42ae32968a..69d809ea6a5 100644 --- a/server/sonar-server-common/src/it/java/org/sonar/server/user/index/UserIndexerIT.java +++ b/server/sonar-server-common/src/it/java/org/sonar/server/user/index/UserIndexerIT.java @@ -65,7 +65,7 @@ public class UserIndexerIT { assertThat(doc.name()).isEqualTo(user.getName()); assertThat(doc.email()).isEqualTo(user.getEmail()); assertThat(doc.active()).isEqualTo(user.isActive()); - assertThat(doc.scmAccounts()).isEqualTo(user.getScmAccountsAsList()); + assertThat(doc.scmAccounts()).isEqualTo(user.getSortedScmAccounts()); } @Test @@ -82,7 +82,7 @@ public class UserIndexerIT { assertThat(doc.name()).isEqualTo(user.getName()); assertThat(doc.email()).isEqualTo(user.getEmail()); assertThat(doc.active()).isEqualTo(user.isActive()); - assertThat(doc.scmAccounts()).isEqualTo(user.getScmAccountsAsList()); + assertThat(doc.scmAccounts()).isEqualTo(user.getSortedScmAccounts()); } @Test diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/user/index/UserIndexer.java b/server/sonar-server-common/src/main/java/org/sonar/server/user/index/UserIndexer.java index 0b9d3953cca..41142f7a36e 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/user/index/UserIndexer.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/user/index/UserIndexer.java @@ -145,8 +145,9 @@ public class UserIndexer implements ResilientIndexer { doc.setName(user.getName()); doc.setEmail(user.getEmail()); doc.setActive(user.isActive()); - doc.setScmAccounts(UserDto.decodeScmAccounts(user.getScmAccounts())); + doc.setScmAccounts(user.getSortedScmAccounts()); return doc.toIndexRequest(); } + } diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java index 576f919d1e3..366e681e201 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java @@ -22,7 +22,6 @@ package org.sonar.server.user; import com.google.common.base.Joiner; import com.google.common.base.Strings; import java.util.ArrayList; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Objects; @@ -52,6 +51,7 @@ import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.collect.Lists.newArrayList; import static java.lang.String.format; import static java.util.Arrays.stream; +import static java.util.Collections.emptyList; import static java.util.stream.Stream.concat; import static org.sonar.api.CoreProperties.DEFAULT_ISSUE_ASSIGNEE; import static org.sonar.core.util.Slug.slugify; @@ -262,7 +262,7 @@ public class UserUpdater { private boolean updateScmAccounts(DbSession dbSession, UpdateUser updateUser, UserDto userDto, List messages) { String email = updateUser.email(); List scmAccounts = sanitizeScmAccounts(updateUser.scmAccounts()); - List existingScmAccounts = userDto.getScmAccountsAsList(); + List existingScmAccounts = userDto.getSortedScmAccounts(); if (updateUser.isScmAccountsChanged() && !(existingScmAccounts.containsAll(scmAccounts) && scmAccounts.containsAll(existingScmAccounts))) { if (!scmAccounts.isEmpty()) { String newOrOldEmail = email != null ? email : userDto.getEmail(); @@ -270,7 +270,7 @@ public class UserUpdater { userDto.setScmAccounts(scmAccounts); } } else { - userDto.setScmAccounts((String) null); + userDto.setScmAccounts(emptyList()); } return true; } @@ -411,7 +411,7 @@ public class UserUpdater { .sorted(String::compareToIgnoreCase) .collect(toList(scmAccounts.size())); } - return Collections.emptyList(); + return emptyList(); } private void checkLoginUniqueness(DbSession dbSession, String login) { diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java index 17d886d00dd..059bd7eee30 100644 --- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterCreateTest.java @@ -99,7 +99,7 @@ public class UserUpdaterCreateTest { assertThat(dto.getLogin()).isEqualTo("user"); assertThat(dto.getName()).isEqualTo("User"); assertThat(dto.getEmail()).isEqualTo("user@mail.com"); - assertThat(dto.getScmAccountsAsList()).containsOnly("u1", "u_1", "User 1"); + assertThat(dto.getSortedScmAccounts()).containsOnly("u1", "u_1", "User 1"); assertThat(dto.isActive()).isTrue(); assertThat(dto.isLocal()).isTrue(); @@ -135,7 +135,7 @@ public class UserUpdaterCreateTest { assertThat(dto.getLogin()).isEqualTo("us"); assertThat(dto.getName()).isEqualTo("User"); assertThat(dto.getEmail()).isNull(); - assertThat(dto.getScmAccounts()).isNull(); + assertThat(dto.getSortedScmAccounts()).isEmpty(); assertThat(dto.isActive()).isTrue(); } @@ -238,7 +238,7 @@ public class UserUpdaterCreateTest { .build(), u -> { }); - assertThat(dbClient.userDao().selectByLogin(session, "user").getScmAccountsAsList()).containsOnly("u1"); + assertThat(dbClient.userDao().selectByLogin(session, "user").getSortedScmAccounts()).containsOnly("u1"); } @Test @@ -253,7 +253,7 @@ public class UserUpdaterCreateTest { .build(), u -> { }); - assertThat(dbClient.userDao().selectByLogin(session, "user").getScmAccounts()).isNull(); + assertThat(dbClient.userDao().selectByLogin(session, "user").getSortedScmAccounts()).isEmpty(); } @Test @@ -268,7 +268,7 @@ public class UserUpdaterCreateTest { .build(), u -> { }); - assertThat(dbClient.userDao().selectByLogin(session, "user").getScmAccountsAsList()).containsOnly("u1"); + assertThat(dbClient.userDao().selectByLogin(session, "user").getSortedScmAccounts()).containsOnly("u1"); } @Test diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java index b71d96979aa..c5ecdee63a7 100644 --- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterReactivateTest.java @@ -87,7 +87,7 @@ public class UserUpdaterReactivateTest { assertThat(reloaded.getLogin()).isEqualTo("marius"); assertThat(reloaded.getName()).isEqualTo("Marius2"); assertThat(reloaded.getEmail()).isEqualTo("marius2@mail.com"); - assertThat(reloaded.getScmAccounts()).isNull(); + assertThat(reloaded.getSortedScmAccounts()).isEmpty(); assertThat(reloaded.isLocal()).isTrue(); assertThat(reloaded.getSalt()).isNotNull(); assertThat(reloaded.getHashMethod()).isEqualTo(HashMethod.PBKDF2.name()); @@ -129,7 +129,7 @@ public class UserUpdaterReactivateTest { assertThat(dto.isActive()).isTrue(); assertThat(dto.getName()).isEqualTo(user.getName()); - assertThat(dto.getScmAccounts()).isNull(); + assertThat(dto.getSortedScmAccounts()).isEmpty(); assertThat(dto.getSalt()).isNull(); assertThat(dto.getCryptedPassword()).isNull(); assertThat(dto.getCreatedAt()).isEqualTo(user.getCreatedAt()); diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java index d3d3cccf60f..dfdc55fd4c5 100644 --- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java @@ -100,7 +100,7 @@ public class UserUpdaterUpdateTest { assertThat(updatedUser.isActive()).isTrue(); assertThat(updatedUser.getName()).isEqualTo("Marius2"); assertThat(updatedUser.getEmail()).isEqualTo("marius2@mail.com"); - assertThat(updatedUser.getScmAccountsAsList()).containsOnly("ma2"); + assertThat(updatedUser.getSortedScmAccounts()).containsOnly("ma2"); assertThat(updatedUser.getCreatedAt()).isEqualTo(user.getCreatedAt()); assertThat(updatedUser.getUpdatedAt()).isGreaterThan(user.getCreatedAt()); @@ -167,7 +167,7 @@ public class UserUpdaterUpdateTest { }); UserDto dto = dbClient.userDao().selectByLogin(session, DEFAULT_LOGIN); - assertThat(dto.getScmAccountsAsList()).containsOnly("ma2"); + assertThat(dto.getSortedScmAccounts()).containsOnly("ma2"); verify(auditPersister, times(1)).updateUserPassword(any(), any()); } @@ -190,7 +190,7 @@ public class UserUpdaterUpdateTest { assertThat(userReloaded.isLocal()).isTrue(); assertThat(userReloaded.getName()).isEqualTo(user.getName()); assertThat(userReloaded.getEmail()).isEqualTo(user.getEmail()); - assertThat(userReloaded.getScmAccountsAsList()).containsAll(user.getScmAccountsAsList()); + assertThat(userReloaded.getSortedScmAccounts()).containsAll(user.getSortedScmAccounts()); assertThat(userReloaded.getSalt()).isEqualTo(user.getSalt()); assertThat(userReloaded.getCryptedPassword()).isEqualTo(user.getCryptedPassword()); } @@ -213,7 +213,7 @@ public class UserUpdaterUpdateTest { assertThat(userReloaded.getExternalId()).isEqualTo(user.getExternalId()); assertThat(userReloaded.getName()).isEqualTo(user.getName()); assertThat(userReloaded.getEmail()).isEqualTo(user.getEmail()); - assertThat(userReloaded.getScmAccountsAsList()).containsAll(user.getScmAccountsAsList()); + assertThat(userReloaded.getSortedScmAccounts()).containsAll(user.getSortedScmAccounts()); assertThat(userReloaded.getSalt()).isEqualTo(user.getSalt()); assertThat(userReloaded.getCryptedPassword()).isEqualTo(user.getCryptedPassword()); } @@ -280,7 +280,7 @@ public class UserUpdaterUpdateTest { // Following fields has not changed assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr"); - assertThat(dto.getScmAccountsAsList()).containsOnly("ma", "marius33"); + assertThat(dto.getSortedScmAccounts()).containsOnly("ma", "marius33"); assertThat(dto.getSalt()).isEqualTo("salt"); assertThat(dto.getCryptedPassword()).isEqualTo("crypted password"); } @@ -302,7 +302,7 @@ public class UserUpdaterUpdateTest { // Following fields has not changed assertThat(dto.getName()).isEqualTo("Marius"); - assertThat(dto.getScmAccountsAsList()).containsOnly("ma", "marius33"); + assertThat(dto.getSortedScmAccounts()).containsOnly("ma", "marius33"); assertThat(dto.getSalt()).isEqualTo("salt"); assertThat(dto.getCryptedPassword()).isEqualTo("crypted password"); } @@ -320,7 +320,7 @@ public class UserUpdaterUpdateTest { }); UserDto dto = dbClient.userDao().selectByLogin(session, DEFAULT_LOGIN); - assertThat(dto.getScmAccountsAsList()).containsOnly("ma2"); + assertThat(dto.getSortedScmAccounts()).containsOnly("ma2"); // Following fields has not changed assertThat(dto.getName()).isEqualTo("Marius"); @@ -340,7 +340,7 @@ public class UserUpdaterUpdateTest { }); UserDto dto = dbClient.userDao().selectByLogin(session, DEFAULT_LOGIN); - assertThat(dto.getScmAccountsAsList()).containsOnly("ma", "marius33"); + assertThat(dto.getSortedScmAccounts()).containsOnly("ma", "marius33"); } @Test @@ -354,7 +354,7 @@ public class UserUpdaterUpdateTest { }); UserDto dto = dbClient.userDao().selectByLogin(session, DEFAULT_LOGIN); - assertThat(dto.getScmAccounts()).isNull(); + assertThat(dto.getSortedScmAccounts()).isEmpty(); } @Test @@ -375,7 +375,7 @@ public class UserUpdaterUpdateTest { // Following fields has not changed assertThat(dto.getName()).isEqualTo("Marius"); - assertThat(dto.getScmAccountsAsList()).containsOnly("ma", "marius33"); + assertThat(dto.getSortedScmAccounts()).containsOnly("ma", "marius33"); assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr"); } @@ -399,7 +399,7 @@ public class UserUpdaterUpdateTest { // Following fields has not changed assertThat(dto.getName()).isEqualTo("Marius"); - assertThat(dto.getScmAccountsAsList()).containsOnly("ma", "marius33"); + assertThat(dto.getSortedScmAccounts()).containsOnly("ma", "marius33"); assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr"); } @@ -461,7 +461,7 @@ public class UserUpdaterUpdateTest { underTest.updateAndCommit(session, user, new UpdateUser() .setName(user.getName()) .setEmail(user.getEmail()) - .setScmAccounts(user.getScmAccountsAsList()) + .setScmAccounts(user.getSortedScmAccounts()) .setExternalIdentity(new ExternalIdentity(user.getExternalIdentityProvider(), user.getExternalLogin(), user.getExternalId())), u -> { }); diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/AnonymizeActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/AnonymizeActionIT.java index 940dfb7c156..2257f33c79c 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/AnonymizeActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/AnonymizeActionIT.java @@ -47,6 +47,7 @@ import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.TestResponse; import org.sonar.server.ws.WsActionTester; +import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.elasticsearch.index.query.QueryBuilders.boolQuery; @@ -77,7 +78,7 @@ public class AnonymizeActionIT { .setName("Ada Lovelace") .setActive(false) .setEmail(null) - .setScmAccounts((String) null) + .setScmAccounts(emptyList()) .setExternalIdentityProvider("provider") .setExternalLogin("external.login") .setExternalId("external.id")); diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/CurrentActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/CurrentActionIT.java index ea9f7f3abf4..6a78528033c 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/CurrentActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/CurrentActionIT.java @@ -42,6 +42,7 @@ import org.sonar.server.ws.WsActionTester; import org.sonarqube.ws.Users.CurrentWsResponse; import static com.google.common.collect.Lists.newArrayList; +import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; @@ -84,7 +85,7 @@ public class CurrentActionIT { CurrentWsResponse::getLocal, CurrentWsResponse::getExternalIdentity, CurrentWsResponse::getExternalProvider, CurrentWsResponse::getScmAccountsList) .containsExactly(true, "obiwan.kenobi", "Obiwan Kenobi", "obiwan.kenobi@starwars.com", "f5aa64437a1821ffe8b563099d506aef", true, "obiwan", "sonarqube", - newArrayList("obiwan:github", "obiwan:bitbucket")); + newArrayList("obiwan:bitbucket", "obiwan:github")); } @Test @@ -123,7 +124,7 @@ public class CurrentActionIT { .setLocal(true) .setExternalLogin("obiwan") .setExternalIdentityProvider("sonarqube") - .setScmAccounts((String) null)); + .setScmAccounts(emptyList())); userSession.logIn(user); CurrentWsResponse response = call(); diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/DeactivateActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/DeactivateActionIT.java index 99389de7c2f..74c0b98f237 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/DeactivateActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/DeactivateActionIT.java @@ -509,7 +509,7 @@ public class DeactivateActionIT { assertThat(user).isPresent(); assertThat(user.get().isActive()).isFalse(); assertThat(user.get().getEmail()).isNull(); - assertThat(user.get().getScmAccountsAsList()).isEmpty(); + assertThat(user.get().getSortedScmAccounts()).isEmpty(); } private void verifyThatUserIsAnomymized(String login) { diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/UpdateActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/UpdateActionIT.java index 789e3b06f47..4f013f14a42 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/UpdateActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/UpdateActionIT.java @@ -20,6 +20,7 @@ package org.sonar.server.user.ws; import java.util.Arrays; +import java.util.List; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -167,7 +168,7 @@ public class UpdateActionIT { .execute(); UserDto userDto = dbClient.userDao().selectByLogin(dbSession, "john"); - assertThat(userDto.getScmAccounts()).isNull(); + assertThat(userDto.getSortedScmAccounts()).isEmpty(); } @Test @@ -181,20 +182,20 @@ public class UpdateActionIT { .assertJson(getClass(), "update_scm_accounts.json"); UserDto user = dbClient.userDao().selectByLogin(dbSession, "john"); - assertThat(user.getScmAccountsAsList()).containsOnly("jon.snow"); + assertThat(user.getSortedScmAccounts()).containsOnly("jon.snow"); } @Test - public void update_scm_account_having_coma() { + public void update_scm_account() { createUser(); ws.newRequest() .setParam("login", "john") - .setMultiParam("scmAccount", singletonList("jon,snow")) + .setMultiParam("scmAccount", List.of("jon", "snow")) .execute(); UserDto user = dbClient.userDao().selectByLogin(dbSession, "john"); - assertThat(user.getScmAccountsAsList()).containsOnly("jon,snow"); + assertThat(user.getSortedScmAccounts()).containsExactly("jon", "snow"); } @Test @@ -237,7 +238,7 @@ public class UpdateActionIT { .execute(); UserDto user = dbClient.userDao().selectByLogin(dbSession, "john"); - assertThat(user.getScmAccountsAsList()).containsExactly("Jon.1", "JON.2", "jon.3"); + assertThat(user.getSortedScmAccounts()).containsExactly("Jon.1", "JON.2", "jon.3"); } @Test diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/CreateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/CreateAction.java index b84dc65d1d5..c7c11e916ae 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/CreateAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/CreateAction.java @@ -158,7 +158,7 @@ public class CreateAction implements UsersWsAction { .setName(userDto.getName()) .setActive(userDto.isActive()) .setLocal(userDto.isLocal()) - .addAllScmAccounts(userDto.getScmAccountsAsList()); + .addAllScmAccounts(userDto.getSortedScmAccounts()); ofNullable(emptyToNull(userDto.getEmail())).ifPresent(userBuilder::setEmail); return CreateWsResponse.newBuilder().setUser(userBuilder).build(); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/CurrentAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/CurrentAction.java index 78ed24a9357..9ce124d0d46 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/CurrentAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/CurrentAction.java @@ -120,7 +120,7 @@ public class CurrentAction implements UsersWsAction { .setName(user.getName()) .setLocal(user.isLocal()) .addAllGroups(groups) - .addAllScmAccounts(user.getScmAccountsAsList()) + .addAllScmAccounts(user.getSortedScmAccounts()) .setPermissions(Permissions.newBuilder().addAllGlobal(getGlobalPermissions()).build()) .setHomepage(buildHomepage(dbSession, user)) .setUsingSonarLintConnectedMode(user.getLastSonarlintConnectionDate() != null) diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/SearchAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/SearchAction.java index 8f9628c9bf8..51a9561443e 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/SearchAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/SearchAction.java @@ -204,8 +204,8 @@ public class SearchAction implements UsersWsAction { userBuilder.setActive(user.isActive()); userBuilder.setLocal(user.isLocal()); ofNullable(user.getExternalIdentityProvider()).ifPresent(userBuilder::setExternalProvider); - if (!user.getScmAccountsAsList().isEmpty()) { - userBuilder.setScmAccounts(ScmAccounts.newBuilder().addAllScmAccounts(user.getScmAccountsAsList())); + if (!user.getSortedScmAccounts().isEmpty()) { + userBuilder.setScmAccounts(ScmAccounts.newBuilder().addAllScmAccounts(user.getSortedScmAccounts())); } } if (userSession.isSystemAdministrator() || Objects.equals(userSession.getUuid(), user.getUuid())) { diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UserJsonWriter.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UserJsonWriter.java index 208f3ee8e42..4bec511e09b 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UserJsonWriter.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UserJsonWriter.java @@ -100,7 +100,7 @@ public class UserJsonWriter { if (isFieldNeeded(FIELD_SCM_ACCOUNTS, fields)) { json.name(FIELD_SCM_ACCOUNTS) .beginArray() - .values(user.getScmAccountsAsList()) + .values(user.getSortedScmAccounts()) .endArray(); } } -- 2.39.5