From 23a105115a33c691bfcc7ae2866e042e1ce2290d Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 8 Jan 2015 12:12:03 +0100 Subject: [PATCH] SONAR-5830 Fix issue when twice scm account was added and when no scm account was added --- .../org/sonar/server/user/UserUpdater.java | 27 +++++++----- .../sonar/server/user/UserUpdaterTest.java | 44 +++++++++++++++++-- .../server/user/index/UserIndexTest.java | 8 ++++ 3 files changed, 64 insertions(+), 15 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java index 8542278a951..13ba63edc1a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java @@ -43,6 +43,7 @@ import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.user.db.UserGroupDao; import org.sonar.server.util.Validation; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; import java.io.StringWriter; @@ -52,6 +53,7 @@ import java.util.List; import java.util.Random; import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Sets.newLinkedHashSet; public class UserUpdater implements ServerComponent { @@ -150,9 +152,8 @@ public class UserUpdater implements ServerComponent { validatePasswords(password, passwordConfirmation, messages); setEncryptedPassWord(password, userDto); - List scmAccounts = newUser.scmAccounts(); + List scmAccounts = sanitizeScmAccounts(newUser.scmAccounts()); if (scmAccounts != null && !scmAccounts.isEmpty()) { - scmAccounts = sanitizeScmAccounts(scmAccounts); validateScmAccounts(dbSession, scmAccounts, login, email, null, messages); userDto.setScmAccounts(convertScmAccountsToCsv(scmAccounts)); } @@ -186,9 +187,8 @@ public class UserUpdater implements ServerComponent { } if (updateUser.isScmAccountsChanged()) { - List scmAccounts = updateUser.scmAccounts(); + List scmAccounts = sanitizeScmAccounts(updateUser.scmAccounts()); if (scmAccounts != null && !scmAccounts.isEmpty()) { - scmAccounts = sanitizeScmAccounts(scmAccounts); validateScmAccounts(dbSession, scmAccounts, userDto.getLogin(), email != null ? email : userDto.getEmail(), userDto, messages); userDto.setScmAccounts(convertScmAccountsToCsv(scmAccounts)); } else { @@ -255,8 +255,11 @@ public class UserUpdater implements ServerComponent { } } - private static List sanitizeScmAccounts(List scmAccounts) { - scmAccounts.removeAll(Arrays.asList(null, "")); + @CheckForNull + private static List sanitizeScmAccounts(@Nullable List scmAccounts) { + if (scmAccounts != null) { + scmAccounts.removeAll(Arrays.asList(null, "")); + } return scmAccounts; } @@ -284,17 +287,19 @@ public class UserUpdater implements ServerComponent { } private static String convertScmAccountsToCsv(List scmAccounts) { + // Use a set to remove duplication, then use a list to add empty characters + List uniqueScmAccounts = newArrayList(newLinkedHashSet(scmAccounts)); // Add one empty character at the begin and at the end of the list to be able to generate a coma at the begin and at the end of the // text - scmAccounts.add(0, ""); - scmAccounts.add(""); - int size = scmAccounts.size(); + uniqueScmAccounts.add(0, ""); + uniqueScmAccounts.add(""); + int size = uniqueScmAccounts.size(); StringWriter writer = new StringWriter(size); CsvWriter csv = CsvWriter.of(writer); - csv.values(scmAccounts.toArray(new String[size])); + csv.values(uniqueScmAccounts.toArray(new String[size])); csv.close(); // Remove useless line break added by CsvWriter at this end of the text - return CharMatcher.BREAKING_WHITESPACE.removeFrom(writer.toString()); + return CharMatcher.JAVA_ISO_CONTROL.removeFrom(writer.toString()); } private void notifyNewUser(String login, String name, String email) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java index 996d7cbb6d3..91b845e7690 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java @@ -110,14 +110,14 @@ public class UserUpdaterTest { .setEmail("user@mail.com") .setPassword("password") .setPasswordConfirmation("password") - .setScmAccounts(newArrayList("u1", "u_1"))); + .setScmAccounts(newArrayList("u1", "u_1", "User 1"))); UserDto dto = userDao.selectNullableByLogin(session, "user"); assertThat(dto.getId()).isNotNull(); assertThat(dto.getLogin()).isEqualTo("user"); assertThat(dto.getName()).isEqualTo("User"); assertThat(dto.getEmail()).isEqualTo("user@mail.com"); - assertThat(dto.getScmAccounts()).isEqualTo(",u1,u_1,"); + assertThat(dto.getScmAccounts()).isEqualTo(",u1,u_1,User 1,"); assertThat(dto.isActive()).isTrue(); assertThat(dto.getSalt()).isNotNull(); @@ -138,11 +138,17 @@ public class UserUpdaterTest { .setPassword("password") .setPasswordConfirmation("password")); - assertThat(userDao.selectNullableByLogin(session, "user")).isNotNull(); + UserDto dto = userDao.selectNullableByLogin(session, "user"); + assertThat(dto.getId()).isNotNull(); + assertThat(dto.getLogin()).isEqualTo("user"); + assertThat(dto.getName()).isEqualTo("User"); + assertThat(dto.getEmail()).isNull(); + assertThat(dto.getScmAccounts()).isNull(); + assertThat(dto.isActive()).isTrue(); } @Test - public void create_user_with_scm_accounts_containing_blank_entry() throws Exception { + public void create_user_with_scm_accounts_containing_blank_or_null_entries() throws Exception { when(system2.now()).thenReturn(1418215735482L); createDefaultGroup(); @@ -156,6 +162,36 @@ public class UserUpdaterTest { assertThat(userDao.selectNullableByLogin(session, "user").getScmAccounts()).isEqualTo(",u1,"); } + @Test + public void create_user_with_scm_accounts_containing_one_blank_entry() throws Exception { + when(system2.now()).thenReturn(1418215735482L); + createDefaultGroup(); + + userUpdater.create(NewUser.create() + .setLogin("user") + .setName("User") + .setPassword("password") + .setPasswordConfirmation("password") + .setScmAccounts(newArrayList(""))); + + assertThat(userDao.selectNullableByLogin(session, "user").getScmAccounts()).isNull(); + } + + @Test + public void create_user_with_scm_accounts_containing_duplications() throws Exception { + when(system2.now()).thenReturn(1418215735482L); + createDefaultGroup(); + + userUpdater.create(NewUser.create() + .setLogin("user") + .setName("User") + .setPassword("password") + .setPasswordConfirmation("password") + .setScmAccounts(newArrayList("u1", "u1"))); + + assertThat(userDao.selectNullableByLogin(session, "user").getScmAccounts()).isEqualTo(",u1,"); + } + @Test public void fail_to_create_user_with_missing_login() throws Exception { try { diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java index 8172bbae517..34a9a9415fe 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java @@ -59,6 +59,14 @@ public class UserIndexTest { assertThat(index.getNullableByLogin("unknown")).isNull(); } + @Test + public void get_nullable_by_login_should_be_case_sensitive() throws Exception { + esTester.putDocuments(UserIndexDefinition.INDEX, UserIndexDefinition.TYPE_USER, this.getClass(), "get_nullable_by_login.json"); + + assertThat(index.getNullableByLogin("user1")).isNotNull(); + assertThat(index.getNullableByLogin("User1")).isNull(); + } + @Test public void get_by_login() throws Exception { esTester.putDocuments(UserIndexDefinition.INDEX, UserIndexDefinition.TYPE_USER, this.getClass(), "get_nullable_by_login.json"); -- 2.39.5