]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-9141 Failure to create reports in issues mode when using branches
authorDuarte Meneses <duarte.meneses@sonarsource.com>
Wed, 26 Apr 2017 11:24:40 +0000 (13:24 +0200)
committerdbmeneses <duarte.meneses@sonarsource.com>
Fri, 28 Apr 2017 11:39:52 +0000 (12:39 +0100)
sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/IssueTransformer.java
sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/LocalIssueTracking.java
sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/TrackedIssue.java
sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositories.java
sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/StatusDetection.java
sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issuesmode/ScanOnlyChangedTest.java

index 5896c4555c33c16a0d6028cdfe309b6c6472673c..d1052cd8beef58c87da241ed9229985a41103155 100644 (file)
@@ -29,7 +29,6 @@ import org.apache.commons.lang.StringUtils;
 import org.sonar.api.batch.fs.InputComponent;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.rule.RuleKey;
-import org.sonar.core.component.ComponentKeys;
 import org.sonar.core.util.Uuids;
 import org.sonar.scanner.issue.tracking.SourceHashHolder;
 import org.sonar.scanner.issue.tracking.TrackedIssue;
@@ -42,7 +41,7 @@ public class IssueTransformer {
     // static only
   }
 
-  public static TrackedIssue toTrackedIssue(ServerIssue serverIssue) {
+  public static TrackedIssue toTrackedIssue(ServerIssue serverIssue, String componentKey) {
     TrackedIssue issue = new TrackedIssue();
     issue.setKey(serverIssue.getKey());
     issue.setStatus(serverIssue.getStatus());
@@ -52,7 +51,8 @@ public class IssueTransformer {
     issue.setEndLine(serverIssue.hasLine() ? serverIssue.getLine() : null);
     issue.setSeverity(serverIssue.getSeverity().name());
     issue.setAssignee(serverIssue.hasAssigneeLogin() ? serverIssue.getAssigneeLogin() : null);
-    issue.setComponentKey(ComponentKeys.createEffectiveKey(serverIssue.getModuleKey(), serverIssue.hasPath() ? serverIssue.getPath() : null));
+    // key in serverIssue might have branch, so don't use it
+    issue.setComponentKey(componentKey);
     issue.setCreationDate(new Date(serverIssue.getCreationDate()));
     issue.setRuleKey(RuleKey.of(serverIssue.getRuleRepository(), serverIssue.getRuleKey()));
     issue.setNew(false);
index 80cfbe0caffc32b51508d9d0f2846be80c2c67c9..758f6eebc16ca5dea540db85b18d0cbabafefe5f 100644 (file)
@@ -84,7 +84,7 @@ public class LocalIssueTracking {
 
       if (shouldCopyServerIssues(component)) {
         // raw issues should be empty, we just need to deal with server issues (SONAR-6931)
-        copyServerIssues(serverIssues, trackedIssues);
+        copyServerIssues(serverIssues, trackedIssues, component.key());
       } else {
 
         SourceHashHolder sourceHashHolder = loadSourceHashes(component);
@@ -95,7 +95,7 @@ public class LocalIssueTracking {
 
         Tracking<TrackedIssue, ServerIssueFromWs> track = tracker.track(rawIssues, baseIssues);
 
-        addUnmatchedFromServer(track.getUnmatchedBases(), trackedIssues);
+        addUnmatchedFromServer(track.getUnmatchedBases(), trackedIssues, component.key());
         mergeMatched(track, trackedIssues, rIssues);
         addUnmatchedFromReport(track.getUnmatchedRaws(), trackedIssues, analysisDate);
       }
@@ -104,7 +104,7 @@ public class LocalIssueTracking {
     if (hasServerAnalysis && componentTree.getParent(component) == null) {
       Preconditions.checkState(component instanceof InputModule, "Object without parent is of type: " + component.getClass());
       // issues that relate to deleted components
-      addIssuesOnDeletedComponents(trackedIssues);
+      addIssuesOnDeletedComponents(trackedIssues, component.key());
     }
 
     return trackedIssues;
@@ -143,10 +143,10 @@ public class LocalIssueTracking {
     return false;
   }
 
-  private void copyServerIssues(Collection<ServerIssueFromWs> serverIssues, List<TrackedIssue> trackedIssues) {
+  private void copyServerIssues(Collection<ServerIssueFromWs> serverIssues, List<TrackedIssue> trackedIssues, String componentKey) {
     for (ServerIssueFromWs serverIssue : serverIssues) {
       org.sonar.scanner.protocol.input.ScannerInput.ServerIssue unmatchedPreviousIssue = serverIssue.getDto();
-      TrackedIssue unmatched = IssueTransformer.toTrackedIssue(unmatchedPreviousIssue);
+      TrackedIssue unmatched = IssueTransformer.toTrackedIssue(unmatchedPreviousIssue, componentKey);
 
       ActiveRule activeRule = activeRules.find(unmatched.getRuleKey());
       unmatched.setNew(false);
@@ -205,10 +205,10 @@ public class LocalIssueTracking {
     }
   }
 
-  private void addUnmatchedFromServer(Iterable<ServerIssueFromWs> unmatchedIssues, Collection<TrackedIssue> mergeTo) {
+  private void addUnmatchedFromServer(Iterable<ServerIssueFromWs> unmatchedIssues, Collection<TrackedIssue> mergeTo, String componentKey) {
     for (ServerIssueFromWs unmatchedIssue : unmatchedIssues) {
       org.sonar.scanner.protocol.input.ScannerInput.ServerIssue unmatchedPreviousIssue = unmatchedIssue.getDto();
-      TrackedIssue unmatched = IssueTransformer.toTrackedIssue(unmatchedPreviousIssue);
+      TrackedIssue unmatched = IssueTransformer.toTrackedIssue(unmatchedPreviousIssue, componentKey);
       updateUnmatchedIssue(unmatched);
       mergeTo.add(unmatched);
     }
@@ -221,9 +221,9 @@ public class LocalIssueTracking {
     }
   }
 
-  private void addIssuesOnDeletedComponents(Collection<TrackedIssue> issues) {
+  private void addIssuesOnDeletedComponents(Collection<TrackedIssue> issues, String componentKey) {
     for (org.sonar.scanner.protocol.input.ScannerInput.ServerIssue previous : serverIssueRepository.issuesOnMissingComponents()) {
-      TrackedIssue dead = IssueTransformer.toTrackedIssue(previous);
+      TrackedIssue dead = IssueTransformer.toTrackedIssue(previous, componentKey);
       updateUnmatchedIssue(dead);
       issues.add(dead);
     }
index e3f39d7029dc783ca8e54e5407ff9d584952ff22..31bf62f014371f5dd66ce6f6b3fde0199bf9d7b0 100644 (file)
@@ -24,6 +24,7 @@ import java.io.Serializable;
 import java.util.Date;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
+
 import org.apache.commons.lang.builder.ToStringBuilder;
 import org.apache.commons.lang.builder.ToStringStyle;
 import org.sonar.api.rule.RuleKey;
@@ -81,10 +82,18 @@ public class TrackedIssue implements Trackable, Serializable {
     return this;
   }
 
+  /**
+   * Component key shared by all part of SonarQube (batch, server, WS...). 
+   * It doesn't include the branch.
+   */
   public String componentKey() {
     return componentKey;
   }
 
+  /**
+   * Component key shared by all part of SonarQube (batch, server, WS...). 
+   * It doesn't include the branch.
+   */
   public TrackedIssue setComponentKey(String componentKey) {
     this.componentKey = componentKey;
     return this;
index 70012bcd2e63096e1a4d2016ab3dcd39d6f51154..6c9921c53a20355073ab6df80f58a3bf9e5680e3 100644 (file)
@@ -68,8 +68,8 @@ public class ProjectRepositories {
   }
 
   @CheckForNull
-  public FileData fileData(String projectKey, String path) {
-    return fileDataByModuleAndPath.get(projectKey, path);
+  public FileData fileData(String projectKeyWithBranch, String path) {
+    return fileDataByModuleAndPath.get(projectKeyWithBranch, path);
   }
 
   @CheckForNull
index e415cdb8d10e9d29b438292bb04254229b9cbd05..851332f663d4a799a1a6d5738b1a907da1d0236c 100644 (file)
@@ -32,8 +32,8 @@ class StatusDetection {
     this.projectSettings = projectSettings;
   }
 
-  InputFile.Status status(String projectKey, String relativePath, String hash) {
-    FileData fileDataPerPath = projectSettings.fileData(projectKey, relativePath);
+  InputFile.Status status(String projectKeyWithBranch, String relativePath, String hash) {
+    FileData fileDataPerPath = projectSettings.fileData(projectKeyWithBranch, relativePath);
     if (fileDataPerPath == null) {
       return InputFile.Status.ADDED;
     }
index 7d8ebd5331d43deceebabcb4379d29120d066f80..dafd2fedfb27d21217dea3773c36aeeea656eccc 100644 (file)
@@ -30,6 +30,8 @@ import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.Date;
 import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.commons.io.FileUtils;
@@ -37,14 +39,20 @@ import org.apache.commons.io.filefilter.FileFilterUtils;
 import org.assertj.core.api.Condition;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 import org.sonar.api.CoreProperties;
 import org.sonar.api.batch.fs.internal.FileMetadata;
 import org.sonar.api.utils.log.LogTester;
 import org.sonar.scanner.mediumtest.ScannerMediumTester;
 import org.sonar.scanner.issue.tracking.TrackedIssue;
 import org.sonar.scanner.mediumtest.TaskResult;
+import org.sonar.scanner.mediumtest.ScannerMediumTester.TaskBuilder;
 import org.sonar.scanner.protocol.Constants.Severity;
 import org.sonar.scanner.protocol.input.ScannerInput.ServerIssue;
 import org.sonar.scanner.repository.FileData;
@@ -53,30 +61,39 @@ import org.sonar.xoo.rule.XooRulesDefinition;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+@RunWith(Parameterized.class)
 public class ScanOnlyChangedTest {
-  @org.junit.Rule
+  private static SimpleDateFormat sdf = new SimpleDateFormat("dd/MM/yyyy");
+
+  /*
+   * Run with branch too to reproduce SONAR-9141 and SONAR-9160
+   */
+  @Parameters(name = "branch enabled:{0} projectKey: {1}")
+  public static Collection<Object[]> data() {
+    return Arrays.asList(new Object[][] {
+      {false, "sample"}, {true, "sample:branch"}, {false, "sample:project"}
+    });
+  }
+
+  @Parameter(0)
+  public boolean branch;
+
+  @Parameter(1)
+  public String projectKey;
+
+  @Rule
   public TemporaryFolder temp = new TemporaryFolder();
 
-  @org.junit.Rule
+  @Rule
   public LogTester logTester = new LogTester();
 
-  private static SimpleDateFormat sdf = new SimpleDateFormat("dd/MM/yyyy");
-
   private ScannerMediumTester tester;
 
-  private static Long date(String date) {
-    try {
-      return sdf.parse(date).getTime();
-    } catch (ParseException e) {
-      throw new IllegalStateException(e);
-    }
-  }
-
   @Before
   public void prepare() throws IOException, URISyntaxException {
     String filePath = "xources/hello/HelloJava.xoo";
     Path path = Paths.get(Resources.getResource("mediumtest/xoo/sample/" + filePath).toURI());
-    String hash = new FileMetadata()
+    String md5sum = new FileMetadata()
       .readMetadata(Files.newInputStream(path), StandardCharsets.UTF_8, filePath)
       .hash();
 
@@ -89,11 +106,11 @@ public class ScanOnlyChangedTest {
       .addActiveRule("xoo", "OneIssueOnDirPerFile", null, "OneIssueOnDirPerFile", "MAJOR", null, "xoo")
       .addActiveRule("xoo", "OneIssuePerModule", null, "OneIssuePerModule", "MAJOR", null, "xoo")
       // this will cause the file to have status==SAME
-      .addFileData("sample", filePath, new FileData(hash, null))
+      .addFileData(projectKey, filePath, new FileData(md5sum, null))
       .setPreviousAnalysisDate(new Date())
       // Existing issue that is copied
       .mockServerIssue(ServerIssue.newBuilder().setKey("xyz")
-        .setModuleKey("sample")
+        .setModuleKey(projectKey)
         .setMsg("One issue per Line copied")
         .setPath("xources/hello/HelloJava.xoo")
         .setRuleRepository("xoo")
@@ -106,7 +123,7 @@ public class ScanOnlyChangedTest {
         .build())
       // Existing issue on project that is still detected
       .mockServerIssue(ServerIssue.newBuilder().setKey("resolved-on-project")
-        .setModuleKey("sample")
+        .setModuleKey(projectKey)
         .setRuleRepository("xoo")
         .setRuleKey("OneIssuePerModule")
         .setSeverity(Severity.CRITICAL)
@@ -129,31 +146,20 @@ public class ScanOnlyChangedTest {
     return projectDir;
   }
 
-  @Test
-  public void testScanAll() throws Exception {
-    File projectDir = copyProject("/mediumtest/xoo/sample");
-
-    TaskResult result = tester
-      .newScanTask(new File(projectDir, "sonar-project.properties"))
-      .property("sonar.scanAllFiles", "true")
-      .start();
-
-    assertNumberIssues(result, 16, 2, 0);
-
-    /*
-     * 8 new per line
-     */
-    assertNumberIssuesOnFile(result, "HelloJava.xoo", 8);
-  }
-
   @Test
   public void testScanOnlyChangedFiles() throws Exception {
     File projectDir = copyProject("/mediumtest/xoo/sample");
 
-    TaskResult result = tester
+    TaskBuilder taskBuilder = tester
       .newScanTask(new File(projectDir, "sonar-project.properties"))
-      .start();
+      .property("sonar.report.export.path", "report.json");
+    if (branch) {
+      taskBuilder.property("sonar.branch", "branch");
+    } else {
+      taskBuilder.property("sonar.projectKey", projectKey);
+    }
 
+    TaskResult result = taskBuilder.start();
     /*
      * We have:
      * 6 new issues per line (open) in helloscala.xoo
@@ -168,6 +174,29 @@ public class ScanOnlyChangedTest {
     assertNumberIssuesOnFile(result, "HelloJava.xoo", 1);
   }
 
+  @Test
+  public void testScanAll() throws Exception {
+    File projectDir = copyProject("/mediumtest/xoo/sample");
+
+    TaskBuilder taskBuilder = tester
+      .newScanTask(new File(projectDir, "sonar-project.properties"))
+      .property("sonar.scanAllFiles", "true");
+    if (branch) {
+      taskBuilder.property("sonar.branch", "branch");
+    } else {
+      taskBuilder.property("sonar.projectKey", projectKey);
+    }
+
+    TaskResult result = taskBuilder.start();
+
+    assertNumberIssues(result, 16, 2, 0);
+
+    /*
+     * 8 new per line
+     */
+    assertNumberIssuesOnFile(result, "HelloJava.xoo", 8);
+  }
+
   private static void assertNumberIssuesOnFile(TaskResult result, final String fileNameEndsWith, int issues) {
     assertThat(result.trackedIssues()).haveExactly(issues, new Condition<TrackedIssue>() {
       @Override
@@ -199,4 +228,12 @@ public class ScanOnlyChangedTest {
     assertThat(openIssues).isEqualTo(expectedOpen);
     assertThat(resolvedIssue).isEqualTo(expectedResolved);
   }
+
+  private static Long date(String date) {
+    try {
+      return sdf.parse(date).getTime();
+    } catch (ParseException e) {
+      throw new IllegalStateException(e);
+    }
+  }
 }