From: Julien HENRY Date: Wed, 18 Oct 2017 12:03:28 +0000 (+0200) Subject: SONAR-8727 Do not fail when SCM login is longer than 255 characters X-Git-Tag: 6.7-RC1~145 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=bf2b385a2e8b9bfbc5cc66efb1356394c431b3e2;p=sonarqube.git 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 2dbc1c731aa..4c6c929d2d6 100644 --- 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 bf92afba817..27daa839bf2 100644 --- 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 5b29e39e914..2ae45777283 100644 --- 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 5cb2b79f548..d3b76382b4f 100644 --- 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 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); }