SONAR-8727 Do not fail when SCM login is longer than 255 characters

diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java
index 2dbc1c731aa4a38f4371bf8ddb05092dae1e90ab..4c6c929d2d6798ed8a982bb3eac2d56b617e49f6 100644 (file)
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java
@@ -51,6 +51,7 @@ import static org.sonar.api.utils.DateUtils.longToDate;
 
 public final class IssueDto implements Serializable {
 
+  public static final int AUTHOR_MAX_SIZE = 255;
   private static final char TAGS_SEPARATOR = ',';
   private static final Joiner TAGS_JOINER = Joiner.on(TAGS_SEPARATOR).skipNulls();
   private static final Splitter TAGS_SPLITTER = Splitter.on(',').trimResults().omitEmptyStrings();
@@ -350,7 +351,7 @@ public final class IssueDto implements Serializable {
   }
 
   public IssueDto setAuthorLogin(@Nullable String s) {
-    checkArgument(s == null || s.length() <= 255, "Value is too long for issue author login: %s", s);
+    checkArgument(s == null || s.length() <= AUTHOR_MAX_SIZE, "Value is too long for issue author login: %s", s);
     this.authorLogin = s;
     return this;
   }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssigner.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssigner.java
index bf92afba81797ec0572dd25acea5a0b88690e1f1..27daa839bf25f7a1ebc336e2aa2ff98b493f1bb4 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssigner.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssigner.java
@@ -28,6 +28,7 @@ import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.IssueChangeContext;
+import org.sonar.db.issue.IssueDto;
 import org.sonar.server.computation.task.projectanalysis.analysis.AnalysisMetadataHolder;
 import org.sonar.server.computation.task.projectanalysis.component.Component;
 import org.sonar.server.computation.task.projectanalysis.scm.ScmInfo;
@@ -75,11 +76,15 @@ public class IssueAssigner extends IssueVisitor {
       String scmAuthor = guessScmAuthor(issue);
 
       if (!Strings.isNullOrEmpty(scmAuthor)) {
-        issueUpdater.setNewAuthor(issue, scmAuthor, changeContext);
+        if (scmAuthor.length() <= IssueDto.AUTHOR_MAX_SIZE) {
+          issueUpdater.setNewAuthor(issue, scmAuthor, changeContext);
+        } else {
+          LOGGER.debug("SCM account '{}' is too long to be stored as issue author", scmAuthor);
+        }
       }
 
       if (issue.assignee() == null) {
-        String author = issue.authorLogin() == null ? null : scmAccountToUser.getNullable(issue.authorLogin());
+        String author = Strings.isNullOrEmpty(scmAuthor) ? null : scmAccountToUser.getNullable(scmAuthor);
         String assigneeLogin = StringUtils.defaultIfEmpty(author, defaultAssignee.loadDefaultAssigneeLogin());
 
         issueUpdater.setNewAssignee(issue, assigneeLogin, changeContext);

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssignerTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssignerTest.java
index 5b29e39e9146aadec777386bcc59942a7c6acdf5..2ae45777283455d855ec5f4ad79823904d4839ff 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssignerTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssignerTest.java
@@ -19,8 +19,8 @@
  */
 package org.sonar.server.computation.task.projectanalysis.issue;
 
-import org.junit.Test;
 import org.junit.Rule;
+import org.junit.Test;
 import org.sonar.api.rules.RuleType;
 import org.sonar.api.utils.log.LogTester;
 import org.sonar.api.utils.log.LoggerLevel;
@@ -31,6 +31,8 @@ import org.sonar.server.computation.task.projectanalysis.scm.Changeset;
 import org.sonar.server.computation.task.projectanalysis.scm.ScmInfoRepositoryRule;
 import org.sonar.server.issue.IssueFieldsSetter;
 
+import static java.util.stream.Collectors.joining;
+import static java.util.stream.IntStream.range;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -97,6 +99,21 @@ public class IssueAssignerTest {
     assertThat(issue.assignee()).isEqualTo("John C");
   }
 
+  @Test
+  public void dont_store_author_too_long() throws Exception {
+    String scmAuthor = range(0, 256).mapToObj(i -> "s").collect(joining());
+    addScmUser(scmAuthor, "John C");
+    setSingleChangeset(scmAuthor, 123456789L, "rev-1");
+    DefaultIssue issue = new DefaultIssue().setLine(1);
+
+    underTest.onIssue(FILE, issue);
+
+    assertThat(issue.authorLogin()).isNull();
+    assertThat(issue.assignee()).isEqualTo("John C");
+
+    assertThat(logTester.logs(LoggerLevel.DEBUG)).contains("SCM account '" + scmAuthor + "' is too long to be stored as issue author");
+  }
+
   @Test
   public void set_default_assignee_if_author_not_found() throws Exception {
     addScmUser("john", null);
@@ -168,7 +185,6 @@ public class IssueAssignerTest {
     assertThat(issue.assignee()).isEqualTo("DefaultAssignee");
   }
 
-
   @Test
   public void set_last_committer_when_line_is_bigger_than_changeset_size() throws Exception {
     addScmUser("john", "John C");

diff --git a/tests/src/test/java/org/sonarqube/tests/issue/AutoAssignTest.java b/tests/src/test/java/org/sonarqube/tests/issue/AutoAssignTest.java
index 5cb2b79f54867354a8702e9c327b3441db21733c..d3b76382b4f6df1d55e2ec9401f1101a82d38edf 100644 (file)
--- a/tests/src/test/java/org/sonarqube/tests/issue/AutoAssignTest.java
+++ b/tests/src/test/java/org/sonarqube/tests/issue/AutoAssignTest.java
@@ -39,6 +39,7 @@ import util.ProjectAnalysis;
 import util.ProjectAnalysisRule;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.groups.Tuple.tuple;
 import static util.ItUtils.newAdminWsClient;
 import static util.ItUtils.setServerProperty;
 
@@ -81,8 +82,8 @@ public class AutoAssignTest extends AbstractIssueTest {
     // verify that SCM account matches, case-insensitive
     createUser("user7", "User 7", "user7@email.com", "user7ScmAccount");
     createUser("user8", "User 8", "user8@email.com", "user8SCMaccOUNT");
-    // SCM accounts long then 255 chars will be ignored
-    createUser("user9", "User 9", "user9@email.com", IntStream.range(0,256).mapToObj(i -> "s").collect(Collectors.joining()));
+    // SCM accounts longer than 255
+    createUser("user9", "User 9", "user9@email.com", IntStream.range(0, 256).mapToObj(i -> "s").collect(Collectors.joining()));
 
     projectAnalysis.run();
 
@@ -99,8 +100,8 @@ public class AutoAssignTest extends AbstractIssueTest {
     // SCM account match, case-insensitive
     verifyIssueAssignee(issues, 7, "user7");
     verifyIssueAssignee(issues, 8, "user8");
-    // SCM accounts long then 255 chars will be ignored
-    verifyIssueAssignee(issues, 10, null);
+    // SCM accounts longer than 255 chars
+    verifyIssueAssignee(issues, 10, "user9");
   }
 
   private static void verifyIssueAssignee(List<Issue> issues, int line, @Nullable String expectedAssignee) {
@@ -116,7 +117,7 @@ public class AutoAssignTest extends AbstractIssueTest {
 
     // user1 is assigned to his issues. All other issues are assigned to the default assignee.
     assertThat(search(IssueQuery.create().assignees("user1")).list()).hasSize(1);
-    assertThat(search(IssueQuery.create().assignees("user2")).list()).hasSize(8);
+    assertThat(search(IssueQuery.create().assignees("user2")).list()).hasSize(9);
     // No unassigned issues
     assertThat(search(IssueQuery.create().assigned(false)).list()).isEmpty();
   }
@@ -139,9 +140,19 @@ public class AutoAssignTest extends AbstractIssueTest {
     assertThat(issues).isNotEmpty();
 
     // No author and assignee are set
-    for (Issue issue : issues) {
-      assertThat(issue.author()).isEmpty();
-    }
+    assertThat(issues)
+      .extracting(Issue::line, Issue::author)
+      .containsExactlyInAnyOrder(
+        tuple(1, ""),
+        tuple(2, ""),
+        tuple(3, ""),
+        tuple(4, ""),
+        tuple(5, ""),
+        tuple(6, ""),
+        tuple(7, ""),
+        tuple(8, ""),
+        tuple(9, ""),
+        tuple(10, ""));
     assertThat(search(IssueQuery.create().assigned(true)).list()).isEmpty();
 
     // Run a second analysis with SCM
@@ -150,9 +161,20 @@ public class AutoAssignTest extends AbstractIssueTest {
     assertThat(issues).isNotEmpty();
 
     // Authors and assignees are set
-    for (Issue issue : issues) {
-      assertThat(issue.author()).isNotEmpty();
-    }
+    assertThat(issues)
+      .extracting(Issue::line, Issue::author)
+      .containsExactlyInAnyOrder(
+        tuple(1, "user1"),
+        tuple(2, "user2"),
+        tuple(3, "user3name"),
+        tuple(4, "user4name"),
+        tuple(5, "user5@email.com"),
+        tuple(6, "user6@email.com"),
+        tuple(7, "user7scmaccount"),
+        tuple(8, "user8scmaccount"),
+        tuple(9, "user8scmaccount"),
+        // SONAR-8727
+        tuple(10, ""));
     assertThat(search(IssueQuery.create().assignees("user1")).list()).hasSize(1);
   }