From 4a5eaf2240d43f26441b0028ebd7e753ade22293 Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Wed, 26 Apr 2017 13:24:40 +0200 Subject: [PATCH] SONAR-9141 Failure to create reports in issues mode when using branches --- .../sonar/scanner/issue/IssueTransformer.java | 6 +- .../issue/tracking/LocalIssueTracking.java | 18 +-- .../scanner/issue/tracking/TrackedIssue.java | 9 ++ .../repository/ProjectRepositories.java | 4 +- .../scan/filesystem/StatusDetection.java | 4 +- .../issuesmode/ScanOnlyChangedTest.java | 107 ++++++++++++------ 6 files changed, 97 insertions(+), 51 deletions(-) diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/IssueTransformer.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/IssueTransformer.java index 5896c4555c3..d1052cd8bee 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/IssueTransformer.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/IssueTransformer.java @@ -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); diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/LocalIssueTracking.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/LocalIssueTracking.java index 80cfbe0caff..758f6eebc16 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/LocalIssueTracking.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/LocalIssueTracking.java @@ -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 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 serverIssues, List trackedIssues) { + private void copyServerIssues(Collection serverIssues, List 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 unmatchedIssues, Collection mergeTo) { + private void addUnmatchedFromServer(Iterable unmatchedIssues, Collection 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 issues) { + private void addIssuesOnDeletedComponents(Collection 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); } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/TrackedIssue.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/TrackedIssue.java index e3f39d7029d..31bf62f0143 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/TrackedIssue.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/TrackedIssue.java @@ -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; diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositories.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositories.java index 70012bcd2e6..6c9921c53a2 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositories.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositories.java @@ -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 diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/StatusDetection.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/StatusDetection.java index e415cdb8d10..851332f663d 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/StatusDetection.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/StatusDetection.java @@ -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; } diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issuesmode/ScanOnlyChangedTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issuesmode/ScanOnlyChangedTest.java index 7d8ebd5331d..dafd2fedfb2 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issuesmode/ScanOnlyChangedTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issuesmode/ScanOnlyChangedTest.java @@ -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 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() { @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); + } + } } -- 2.39.5