SONAR-5830 Change the way to persist scm accounts in db in order to i…

…mprove the search

server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java

@@ -20,6 +20,7 @@
 
 package org.sonar.server.user;
 
+import com.google.common.base.CharMatcher;
 import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
 import com.google.common.collect.Iterables;
@@ -76,8 +77,7 @@ public UserUpdater(NewUserNotifier newUserNotifier, Settings settings, UserGroup
   }
 
   /**
-   * Retuen true if the user has been reactivated
-   * @return
+   * Return true if the user has been reactivated
    */
   public boolean create(NewUser newUser) {
     UserDto userDto = createNewUserDto(newUser);
@@ -256,13 +256,19 @@ private static void setEncryptedPassWord(String password, UserDto userDto) {
   @CheckForNull
   private static String convertScmAccountsToCsv(@Nullable List<String> scmAccounts) {
     if (scmAccounts != null) {
+      // Remove empty characters
       scmAccounts.removeAll(Arrays.asList(null, ""));
+      // 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();
       StringWriter writer = new StringWriter(size);
       CsvWriter csv = CsvWriter.of(writer);
       csv.values(scmAccounts.toArray(new String[size]));
       csv.close();
-      return writer.toString();
+      // Remove useless line break added by CsvWriter at this end of the text
+      return CharMatcher.BREAKING_WHITESPACE.removeFrom(writer.toString());
     }
     return null;
   }

server/sonar-server/src/main/java/org/sonar/server/user/index/UserResultSetIterator.java

@@ -19,6 +19,7 @@
  */
 package org.sonar.server.user.index;
 
+import com.google.common.base.Strings;
 import com.google.common.collect.Maps;
 import org.apache.commons.csv.CSVFormat;
 import org.apache.commons.csv.CSVParser;
@@ -106,7 +107,9 @@ private List<String> getScmAccounts(@Nullable String csv, String login) {
       csvParser = new CSVParser(reader, CSVFormat.DEFAULT);
       for (CSVRecord csvRecord : csvParser) {
         for (String aCsvRecord : csvRecord) {
-          result.add(aCsvRecord);
+          if (!Strings.isNullOrEmpty(aCsvRecord)) {
+            result.add(aCsvRecord);
+          }
         }
       }
       return result;

server/sonar-server/src/test/java/org/sonar/server/user/UserServiceMediumTest.java

@@ -33,6 +33,7 @@
 import org.sonar.server.es.EsClient;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.tester.ServerTester;
+import org.sonar.server.user.index.UserDoc;
 import org.sonar.server.user.index.UserIndexDefinition;
 
 import static com.google.common.collect.Lists.newArrayList;
@@ -79,8 +80,27 @@ public void create_user() throws Exception {
       .setScmAccounts(newArrayList("u1", "u_1")));
 
     assertThat(result).isFalse();
-    assertThat(dbClient.userDao().selectNullableByLogin(session, "user")).isNotNull();
-    assertThat(esClient.prepareGet(UserIndexDefinition.INDEX, UserIndexDefinition.TYPE_USER, "user").get().isExists()).isTrue();
+
+    UserDto userDto = dbClient.userDao().selectNullableByLogin(session, "user");
+    assertThat(userDto).isNotNull();
+    assertThat(userDto.getId()).isNotNull();
+    assertThat(userDto.getLogin()).isEqualTo("user");
+    assertThat(userDto.getName()).isEqualTo("User");
+    assertThat(userDto.getEmail()).isEqualTo("user@mail.com");
+    assertThat(userDto.getCryptedPassword()).isNotNull();
+    assertThat(userDto.getSalt()).isNotNull();
+    assertThat(userDto.getScmAccounts()).contains(",u1,u_1,");
+    assertThat(userDto.getCreatedAt()).isNotNull();
+    assertThat(userDto.getUpdatedAt()).isNotNull();
+
+    UserDoc userDoc = service.getNullableByLogin("user");
+    assertThat(userDoc).isNotNull();
+    assertThat(userDoc.login()).isEqualTo("user");
+    assertThat(userDoc.name()).isEqualTo("User");
+    assertThat(userDoc.email()).isEqualTo("user@mail.com");
+    assertThat(userDoc.scmAccounts()).containsOnly("u1", "u_1");
+    assertThat(userDoc.createdAt()).isNotNull();
+    assertThat(userDoc.updatedAt()).isNotNull();
   }
 
   @Test(expected = ForbiddenException.class)

server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java

@@ -117,7 +117,7 @@ public void create_user() throws Exception {
     assertThat(dto.getLogin()).isEqualTo("user");
     assertThat(dto.getName()).isEqualTo("User");
     assertThat(dto.getEmail()).isEqualTo("user@mail.com");
-    assertThat(dto.getScmAccounts()).contains("u1,u_1");
+    assertThat(dto.getScmAccounts()).isEqualTo(",u1,u_1,");
     assertThat(dto.isActive()).isTrue();
 
     assertThat(dto.getSalt()).isNotNull();
@@ -153,7 +153,7 @@ public void create_user_with_scm_accounts_containing_blank_entry() throws Except
       .setPasswordConfirmation("password")
       .setScmAccounts(newArrayList("u1", "", null)));
 
-    assertThat(userDao.selectNullableByLogin(session, "user").getScmAccounts()).doesNotContain(",");
+    assertThat(userDao.selectNullableByLogin(session, "user").getScmAccounts()).isEqualTo(",u1,");
   }
 
   @Test
@@ -458,7 +458,7 @@ public void update_user() throws Exception {
     assertThat(dto.isActive()).isTrue();
     assertThat(dto.getName()).isEqualTo("Marius2");
     assertThat(dto.getEmail()).isEqualTo("marius2@mail.com");
-    assertThat(dto.getScmAccounts()).contains("ma2");
+    assertThat(dto.getScmAccounts()).isEqualTo(",ma2,");
 
     assertThat(dto.getSalt()).isNotEqualTo("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365");
     assertThat(dto.getCryptedPassword()).isNotEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg");
@@ -481,7 +481,7 @@ public void update_only_user_name() throws Exception {
 
     // Following fields has not changed
     assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr");
-    assertThat(dto.getScmAccounts()).contains("ma,marius33");
+    assertThat(dto.getScmAccounts()).isEqualTo(",ma,marius33,");
     assertThat(dto.getSalt()).isEqualTo("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365");
     assertThat(dto.getCryptedPassword()).isEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg");
   }
@@ -501,13 +501,13 @@ public void update_only_user_email() throws Exception {
 
     // Following fields has not changed
     assertThat(dto.getName()).isEqualTo("Marius");
-    assertThat(dto.getScmAccounts()).contains("ma,marius33");
+    assertThat(dto.getScmAccounts()).isEqualTo(",ma,marius33,");
     assertThat(dto.getSalt()).isEqualTo("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365");
     assertThat(dto.getCryptedPassword()).isEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg");
   }
 
   @Test
-  public void update_only_scm_accounts_email() throws Exception {
+  public void update_only_scm_accounts() throws Exception {
     db.prepareDbUnit(getClass(), "update_user.xml");
     createDefaultGroup();
 
@@ -517,7 +517,7 @@ public void update_only_scm_accounts_email() throws Exception {
     session.clearCache();
 
     UserDto dto = userDao.selectNullableByLogin(session, "marius");
-    assertThat(dto.getScmAccounts()).contains("ma2");
+    assertThat(dto.getScmAccounts()).isEqualTo(",ma2,");
 
     // Following fields has not changed
     assertThat(dto.getName()).isEqualTo("Marius");
@@ -526,6 +526,20 @@ public void update_only_scm_accounts_email() throws Exception {
     assertThat(dto.getCryptedPassword()).isEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg");
   }
 
+  @Test
+  public void remove_scm_accounts() throws Exception {
+    db.prepareDbUnit(getClass(), "update_user.xml");
+    createDefaultGroup();
+
+    userUpdater.update(UpdateUser.create("marius")
+      .setScmAccounts(null));
+    session.commit();
+    session.clearCache();
+
+    UserDto dto = userDao.selectNullableByLogin(session, "marius");
+    assertThat(dto.getScmAccounts()).isNull();
+  }
+
   @Test
   public void update_only_user_password() throws Exception {
     db.prepareDbUnit(getClass(), "update_user.xml");
@@ -543,7 +557,7 @@ public void update_only_user_password() throws Exception {
 
     // Following fields has not changed
     assertThat(dto.getName()).isEqualTo("Marius");
-    assertThat(dto.getScmAccounts()).contains("ma,marius33");
+    assertThat(dto.getScmAccounts()).isEqualTo(",ma,marius33,");
     assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr");
   }
 

server/sonar-server/src/test/java/org/sonar/server/user/db/UserDaoTest.java

@@ -64,7 +64,7 @@ public void select_by_login() {
     assertThat(dto.getName()).isEqualTo("Marius");
     assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr");
     assertThat(dto.isActive()).isTrue();
-    assertThat(dto.getScmAccounts()).isEqualTo("ma,marius33");
+    assertThat(dto.getScmAccounts()).isEqualTo(",ma,marius33,");
     assertThat(dto.getSalt()).isEqualTo("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365");
     assertThat(dto.getCryptedPassword()).isEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg");
     assertThat(dto.getCreatedAt()).isEqualTo(1418215735482L);

server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/fail_to_update_user_when_scm_account_is_already_used.xml

@@ -0,0 +1,6 @@
+<dataset>
+
+  <users id="101" login="john" name="John" email="john@email.com" active="[true]" scm_accounts=",jo," created_at="1418215735482" updated_at="1418215735485"
+         salt="79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365" crypted_password="650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"/>
+
+</dataset>

server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/update_user.xml

@@ -1,6 +1,6 @@
 <dataset>
 
-  <users id="101" login="marius" name="Marius" email="marius@lesbronzes.fr" active="[true]" scm_accounts="ma,marius33" created_at="1418215735482" updated_at="1418215735485"
+  <users id="101" login="marius" name="Marius" email="marius@lesbronzes.fr" active="[true]" scm_accounts=",ma,marius33," created_at="1418215735482" updated_at="1418215735485"
          salt="79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365" crypted_password="650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"/>
 
 </dataset>

server/sonar-server/src/test/resources/org/sonar/server/user/db/UserDaoTest/select_by_login.xml

@@ -1,6 +1,6 @@
 <dataset>
 
-  <users id="101" login="marius" name="Marius" email="marius@lesbronzes.fr" active="[true]" scm_accounts="ma,marius33" created_at="1418215735482" updated_at="1418215735485"
+  <users id="101" login="marius" name="Marius" email="marius@lesbronzes.fr" active="[true]" scm_accounts=",ma,marius33," created_at="1418215735482" updated_at="1418215735485"
          salt="79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365" crypted_password="650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"/>
   <users id="102" login="sbrandhof" name="Simon Brandhof" email="marius@lesbronzes.fr" active="[true]" scm_accounts="[null]" created_at="1418215735482" updated_at="1418215735485"
          salt="79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8366" crypted_password="650d2261c98361e2f67f90ce5c65a95e7d8ea2fh"/>

server/sonar-server/src/test/resources/org/sonar/server/user/index/UserResultSetIteratorTest/shared.xml

@@ -1,14 +1,14 @@
 <dataset>
 
   <users id="1" login="user1" name="User1" email="user1@mail.com" active="[true]"
-         scm_accounts="user_1,u1"
+         scm_accounts=",user_1,u1,"
          created_at="1500000000000"
          updated_at="1500000000000"
       />
 
   <!-- scm accounts with comma -->
   <users id="2" login="user2" name="User2" email="user2@mail.com" active="[true]"
-         scm_accounts="&quot;user,2&quot;,user_2"
+         scm_accounts=",&quot;user,2&quot;,user_2,"
          created_at="1500000000000"
          updated_at="1500000000000"
       />

sonar-core/src/test/java/org/sonar/core/user/UserDaoTest.java

@@ -182,7 +182,7 @@ public void insert_user() {
       .setLogin("john")
       .setName("John")
       .setEmail("jo@hn.com")
-      .setScmAccounts("jo.hn,john2")
+      .setScmAccounts(",jo.hn,john2,")
       .setActive(true)
       .setSalt("1234")
       .setCryptedPassword("abcd")
@@ -198,7 +198,7 @@ public void insert_user() {
     assertThat(user.getName()).isEqualTo("John");
     assertThat(user.getEmail()).isEqualTo("jo@hn.com");
     assertThat(user.isActive()).isTrue();
-    assertThat(user.getScmAccounts()).isEqualTo("jo.hn,john2");
+    assertThat(user.getScmAccounts()).isEqualTo(",jo.hn,john2,");
     assertThat(user.getSalt()).isEqualTo("1234");
     assertThat(user.getCryptedPassword()).isEqualTo("abcd");
     assertThat(user.getCreatedAt()).isEqualTo(date);
@@ -216,7 +216,7 @@ public void update_user() {
       .setLogin("john")
       .setName("John Doo")
       .setEmail("jodoo@hn.com")
-      .setScmAccounts("jo.hn,john2,johndoo")
+      .setScmAccounts(",jo.hn,john2,johndoo,")
       .setActive(false)
       .setSalt("12345")
       .setCryptedPassword("abcde")
@@ -231,7 +231,7 @@ public void update_user() {
     assertThat(user.getName()).isEqualTo("John Doo");
     assertThat(user.getEmail()).isEqualTo("jodoo@hn.com");
     assertThat(user.isActive()).isFalse();
-    assertThat(user.getScmAccounts()).isEqualTo("jo.hn,john2,johndoo");
+    assertThat(user.getScmAccounts()).isEqualTo(",jo.hn,john2,johndoo,");
     assertThat(user.getSalt()).isEqualTo("12345");
     assertThat(user.getCryptedPassword()).isEqualTo("abcde");
     assertThat(user.getCreatedAt()).isEqualTo(1418215735482L);