SONAR-9904 guess author of issue based on all its locations

diff --git a/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/MultilineIssuesSensor.java b/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/MultilineIssuesSensor.java
index bf1d64b44a3a9232f01c86c762f4cd70aae4a86f..41c4fd973ef24ad155df1bc595971378ba9c5f57 100644 (file)
--- a/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/MultilineIssuesSensor.java
+++ b/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/MultilineIssuesSensor.java
@@ -76,11 +76,11 @@ public class MultilineIssuesSensor implements Sensor {
     Map<Integer, Table<Integer, Integer, TextPointer>> endFlowsPositions = Maps.newHashMap();
 
     parseIssues(file, context, startIssuesPositions, endIssuesPositions);
-    parseFlows(file, context, startFlowsPositions, endFlowsPositions);
+    parseFlows(file, startFlowsPositions, endFlowsPositions);
     createIssues(file, context, startIssuesPositions, endIssuesPositions, startFlowsPositions, endFlowsPositions);
   }
 
-  private static void parseFlows(InputFile file, SensorContext context, Map<Integer, Table<Integer, Integer, TextPointer>> startFlowsPositions,
+  private static void parseFlows(InputFile file, Map<Integer, Table<Integer, Integer, TextPointer>> startFlowsPositions,
     Map<Integer, Table<Integer, Integer, TextPointer>> endFlowsPositions) {
     int currentLine = 0;
     try {

diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueAssigner.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueAssigner.java
index f5a1556157827bcecc1088503dddc5d3d3cf046a..2c913fedbc5a0e58c0a8a7f801dd6c16fa1aa7e0 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueAssigner.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueAssigner.java
@@ -19,22 +19,22 @@
  */
 package org.sonar.ce.task.projectanalysis.issue;
 
+import java.util.Comparator;
 import java.util.Date;
 import java.util.Optional;
-import javax.annotation.CheckForNull;
+import org.apache.commons.lang.StringUtils;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
-import org.sonar.ce.task.projectanalysis.component.Component;
-import org.sonar.core.issue.DefaultIssue;
-import org.sonar.core.issue.IssueChangeContext;
-import org.sonar.db.issue.IssueDto;
 import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder;
 import org.sonar.ce.task.projectanalysis.component.Component;
+import org.sonar.ce.task.projectanalysis.scm.Changeset;
 import org.sonar.ce.task.projectanalysis.scm.ScmInfo;
 import org.sonar.ce.task.projectanalysis.scm.ScmInfoRepository;
+import org.sonar.core.issue.DefaultIssue;
+import org.sonar.core.issue.IssueChangeContext;
+import org.sonar.db.issue.IssueDto;
 import org.sonar.server.issue.IssueFieldsSetter;
 
-import static com.google.common.base.Strings.isNullOrEmpty;
 import static org.apache.commons.lang.StringUtils.defaultIfEmpty;
 import static org.sonar.core.issue.IssueChangeContext.createScan;
 
@@ -67,23 +67,24 @@ public class IssueAssigner extends IssueVisitor {
 
   @Override
   public void onIssue(Component component, DefaultIssue issue) {
-    if (issue.authorLogin() == null) {
-      loadScmChangesets(component);
-      String scmAuthor = guessScmAuthor(issue);
+    if (issue.authorLogin() != null) {
+      return;
+    }
+    loadScmChangesets(component);
+    Optional<String> scmAuthor = guessScmAuthor(issue, component);
 
-      if (!isNullOrEmpty(scmAuthor)) {
-        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 (scmAuthor.isPresent()) {
+      if (scmAuthor.get().length() <= IssueDto.AUTHOR_MAX_SIZE) {
+        issueUpdater.setNewAuthor(issue, scmAuthor.get(), changeContext);
+      } else {
+        LOGGER.debug("SCM account '{}' is too long to be stored as issue author", scmAuthor.get());
       }
+    }
 
-      if (issue.assignee() == null) {
-        String assigneeUuid = isNullOrEmpty(scmAuthor) ? null : scmAccountToUser.getNullable(scmAuthor);
-        assigneeUuid = defaultIfEmpty(assigneeUuid, defaultAssignee.loadDefaultAssigneeUuid());
-        issueUpdater.setNewAssignee(issue, assigneeUuid, changeContext);
-      }
+    if (issue.assignee() == null) {
+      String assigneeUuid = scmAuthor.map(scmAccountToUser::getNullable).orElse(null);
+      assigneeUuid = defaultIfEmpty(assigneeUuid, defaultAssignee.loadDefaultAssigneeUuid());
+      issueUpdater.setNewAssignee(issue, assigneeUuid, changeContext);
     }
   }
 
@@ -104,20 +105,21 @@ public class IssueAssigner extends IssueVisitor {
   }
 
   /**
-   * Get the SCM login of the last committer on the line. When line is zero,
-   * then get the last committer on the file.
+   * Author of the latest change on the lines involved by the issue.
+   * If no authors are set on the lines, then the author of the latest change on the file
+   * is returned.
    */
-  @CheckForNull
-  private String guessScmAuthor(DefaultIssue issue) {
-    Integer line = issue.line();
+  private Optional<String> guessScmAuthor(DefaultIssue issue, Component component) {
     String author = null;
-    if (line != null && scmChangesets != null) {
-      if (scmChangesets.hasChangesetForLine(line)) {
-        author = scmChangesets.getChangesetForLine(line).getAuthor();
-      } else {
-        LOGGER.warn("No SCM info has been found for issue {}", issue);
-      }
+    if (scmChangesets != null) {
+      author = IssueLocations.allLinesFor(issue, component.getUuid())
+        .filter(scmChangesets::hasChangesetForLine)
+        .mapToObj(scmChangesets::getChangesetForLine)
+        .filter(c -> StringUtils.isNotEmpty(c.getAuthor()))
+        .max(Comparator.comparingLong(Changeset::getDate))
+        .map(Changeset::getAuthor)
+        .orElse(null);
     }
-    return defaultIfEmpty(author, lastCommitAuthor);
+    return Optional.ofNullable(defaultIfEmpty(author, lastCommitAuthor));
   }
 }

diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueAssignerTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueAssignerTest.java
index dce1261831381cf59999b79e35f31e6924b15bda..7a127fcab348d657a76ade0fdf379f2c8dafe8c1 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueAssignerTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueAssignerTest.java
@@ -19,9 +19,10 @@
  */
 package org.sonar.ce.task.projectanalysis.issue;
 
+import java.util.Arrays;
+import javax.annotation.Nullable;
 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;
 import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule;
@@ -29,6 +30,8 @@ import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.ce.task.projectanalysis.scm.Changeset;
 import org.sonar.ce.task.projectanalysis.scm.ScmInfoRepositoryRule;
 import org.sonar.core.issue.DefaultIssue;
+import org.sonar.db.protobuf.DbCommons;
+import org.sonar.db.protobuf.DbIssues;
 import org.sonar.server.issue.IssueFieldsSetter;
 
 import static java.util.stream.Collectors.joining;
@@ -58,7 +61,7 @@ public class IssueAssignerTest {
 
   @Test
   public void do_not_set_author_if_no_changeset() {
-    DefaultIssue issue = new DefaultIssue().setLine(1);
+    DefaultIssue issue = newIssueOnLines(1);
 
     underTest.onIssue(FILE, issue);
 
@@ -68,7 +71,7 @@ public class IssueAssignerTest {
   @Test
   public void set_author_of_new_issue_if_changeset() {
     setSingleChangeset("john", 123456789L, "rev-1");
-    DefaultIssue issue = new DefaultIssue().setLine(1);
+    DefaultIssue issue = newIssueOnLines(1);
 
     underTest.onIssue(FILE, issue);
 
@@ -78,8 +81,7 @@ public class IssueAssignerTest {
   @Test
   public void do_not_reset_author_if_already_set() {
     setSingleChangeset("john", 123456789L, "rev-1");
-    DefaultIssue issue = new DefaultIssue()
-      .setLine(1)
+    DefaultIssue issue = newIssueOnLines(1)
       .setAuthorLogin("jane");
 
     underTest.onIssue(FILE, issue);
@@ -92,7 +94,7 @@ public class IssueAssignerTest {
     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);
+    DefaultIssue issue = newIssueOnLines(1);
 
     underTest.onIssue(FILE, issue);
 
@@ -106,7 +108,7 @@ public class IssueAssignerTest {
   public void assign_new_issue_to_author_of_change() {
     addScmUser("john", "u123");
     setSingleChangeset("john", 123456789L, "rev-1");
-    DefaultIssue issue = new DefaultIssue().setLine(1);
+    DefaultIssue issue = newIssueOnLines(1);
 
     underTest.onIssue(FILE, issue);
 
@@ -117,7 +119,7 @@ public class IssueAssignerTest {
   public void assign_new_issue_to_default_assignee_if_author_not_found() {
     setSingleChangeset("john", 123456789L, "rev-1");
     when(defaultAssignee.loadDefaultAssigneeUuid()).thenReturn("u1234");
-    DefaultIssue issue = new DefaultIssue().setLine(1);
+    DefaultIssue issue = newIssueOnLines(1);
 
     underTest.onIssue(FILE, issue);
 
@@ -127,7 +129,7 @@ public class IssueAssignerTest {
   @Test
   public void do_not_assign_new_issue_if_no_author_in_changeset() {
     setSingleChangeset(null, 123456789L, "rev-1");
-    DefaultIssue issue = new DefaultIssue().setLine(1);
+    DefaultIssue issue = newIssueOnLines(1);
 
     underTest.onIssue(FILE, issue);
 
@@ -139,7 +141,7 @@ public class IssueAssignerTest {
   public void do_not_assign_issue_if_unassigned_but_already_authored() {
     addScmUser("john", "u1234");
     setSingleChangeset("john", 123456789L, "rev-1");
-    DefaultIssue issue = new DefaultIssue().setLine(1)
+    DefaultIssue issue = newIssueOnLines(1)
       .setAuthorLogin("john")
       .setAssigneeUuid(null);
 
@@ -154,18 +156,18 @@ public class IssueAssignerTest {
     addScmUser("henry", "Henry V");
     Changeset changeset1 = Changeset.newChangesetBuilder()
       .setAuthor("john")
-      .setDate(123456789L)
+      .setDate(1_000L)
       .setRevision("rev-1")
       .build();
     // Latest changeset
     Changeset changeset2 = Changeset.newChangesetBuilder()
       .setAuthor("henry")
-      .setDate(1234567810L)
+      .setDate(2_000L)
       .setRevision("rev-2")
       .build();
     scmInfoRepository.setScmInfo(FILE_REF, changeset1, changeset2, changeset1);
 
-    DefaultIssue issue = new DefaultIssue().setLine(null);
+    DefaultIssue issue = newIssueOnLines();
 
     underTest.onIssue(FILE, issue);
 
@@ -174,7 +176,7 @@ public class IssueAssignerTest {
 
   @Test
   public void assign_to_default_assignee_if_no_author() {
-    DefaultIssue issue = new DefaultIssue().setLine(null);
+    DefaultIssue issue = newIssueOnLines();
 
     when(defaultAssignee.loadDefaultAssigneeUuid()).thenReturn("u123");
     underTest.onIssue(FILE, issue);
@@ -183,7 +185,7 @@ public class IssueAssignerTest {
   }
 
   @Test
-  public void assign_to_default_assignee_if_scm_on_issue_locations() {
+  public void assign_to_default_assignee_if_no_scm_on_issue_locations() {
     addScmUser("john", "John C");
     Changeset changeset = Changeset.newChangesetBuilder()
       .setAuthor("john")
@@ -191,7 +193,7 @@ public class IssueAssignerTest {
       .setRevision("rev-1")
       .build();
     scmInfoRepository.setScmInfo(FILE_REF, changeset, changeset);
-    DefaultIssue issue = new DefaultIssue().setLine(3);
+    DefaultIssue issue = newIssueOnLines(3);
 
     underTest.onIssue(FILE, issue);
 
@@ -199,22 +201,29 @@ public class IssueAssignerTest {
   }
 
   @Test
-  public void display_warning_when_line_is_above_max_size() {
-    setSingleChangeset("john", 123456789L, "rev-1");
-    DefaultIssue issue = new DefaultIssue().setLine(2).setType(RuleType.VULNERABILITY);
+  public void assign_to_author_of_the_most_recent_change_in_all_issue_locations() {
+    addScmUser("john", "u1");
+    addScmUser("jane", "u2");
+    Changeset commit1 = Changeset.newChangesetBuilder()
+      .setAuthor("john")
+      .setDate(1_000L)
+      .setRevision("rev-1")
+      .build();
+    Changeset commit2 = Changeset.newChangesetBuilder()
+      .setAuthor("jane")
+      .setDate(2_000L)
+      .setRevision("rev-2")
+      .build();
+    scmInfoRepository.setScmInfo(FILE_REF, commit1, commit1, commit2, commit1);
+    DefaultIssue issue = newIssueOnLines(2, 3, 4);
 
     underTest.onIssue(FILE, issue);
 
-    assertThat(logTester.logs(LoggerLevel.WARN)).containsOnly(
-      "No SCM info has been found for issue DefaultIssue[key=<null>,type=VULNERABILITY,componentUuid=<null>,componentKey=<null>," +
-        "moduleUuid=<null>,moduleUuidPath=<null>,projectUuid=<null>,projectKey=<null>,ruleKey=<null>,language=<null>,severity=<null>," +
-        "manualSeverity=false,message=<null>,line=2,gap=<null>,effort=<null>,status=<null>,resolution=<null>," +
-        "assigneeUuid=<null>,checksum=<null>,attributes=<null>,authorLogin=<null>,comments=<null>,tags=<null>," +
-        "locations=<null>,isFromExternalRuleEngine=false,creationDate=<null>,updateDate=<null>,closeDate=<null>,isFromHotspot=false,currentChange=<null>,changes=<null>,isNew=true,isCopied=false," +
-        "beingClosed=false,onDisabledRule=false,isChanged=false,sendNotifications=false,selectedAt=<null>]");
+    assertThat(issue.authorLogin()).isEqualTo("jane");
+    assertThat(issue.assignee()).isEqualTo("u2");
   }
 
-  private void setSingleChangeset(String author, Long date, String revision) {
+  private void setSingleChangeset(@Nullable String author, Long date, String revision) {
     scmInfoRepository.setScmInfo(FILE_REF,
       Changeset.newChangesetBuilder()
         .setAuthor(author)
@@ -227,4 +236,21 @@ public class IssueAssignerTest {
     when(scmAccountToUser.getNullable(scmAccount)).thenReturn(userUuid);
   }
 
+  private static DefaultIssue newIssueOnLines(int... lines) {
+    DefaultIssue issue = new DefaultIssue();
+    issue.setComponentUuid(FILE.getUuid());
+    DbIssues.Locations.Builder locations = DbIssues.Locations.newBuilder();
+    DbIssues.Flow.Builder flow = DbIssues.Flow.newBuilder();
+    Arrays.stream(lines).forEach(line -> flow.addLocation(newLocation(line)));
+    locations.addFlow(flow.build());
+    issue.setLocations(locations.build());
+    return issue;
+  }
+
+  private static DbIssues.Location newLocation(int line) {
+    return DbIssues.Location.newBuilder()
+      .setComponentId(FILE.getUuid())
+      .setTextRange(DbCommons.TextRange.newBuilder().setStartLine(line).setEndLine(line).build()).build();
+  }
+
 }