SONAR-5830 Fix issue when twice scm account was added and when no scm account was...

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 8542278a95145ad4b7ef1edc5d23ccd91dbd4ff2..13ba63edc1a301554ee94e0306c78f36eebf980c 100644 (file)
--- 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<String> scmAccounts = newUser.scmAccounts();
+    List<String> 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<String> scmAccounts = updateUser.scmAccounts();
+      List<String> 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<String> sanitizeScmAccounts(List<String> scmAccounts) {
-    scmAccounts.removeAll(Arrays.asList(null, ""));
+  @CheckForNull
+  private static List<String> sanitizeScmAccounts(@Nullable List<String> 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<String> scmAccounts) {
+    // Use a set to remove duplication, then use a list to add empty characters
+    List<String> 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 996d7cbb6d380fcf4d875766e9a63d7009f3fb3d..91b845e769002958f2d2699e232ee3db48fedc0f 100644 (file)
--- 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 8172bbae517456847bf248692f8b5e96fe673caf..34a9a9415fe86dc8625154ad84fd95cf395008a7 100644 (file)
--- 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");