From ac4ed3112b7921709c39f30fe125a518a79a366c Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Mon, 23 Dec 2019 17:44:49 +0100 Subject: [PATCH] SONAR-12723 default status of Hotspot is TO_REVIEW even when external --- .../projectanalysis/issue/IssueLifecycle.java | 6 +-- .../issue/TrackerRawInputFactory.java | 16 ++++--- .../issue/TrackerRawInputFactoryTest.java | 48 ++++++++++++------- 3 files changed, 43 insertions(+), 27 deletions(-) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java index 7c01c43f013..72526f97d2e 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java @@ -88,10 +88,10 @@ public class IssueLifecycle { } private void setStatus(DefaultIssue issue, Rule rule) { - if (issue.isFromExternalRuleEngine() || rule.getType() != RuleType.SECURITY_HOTSPOT) { - issue.setStatus(Issue.STATUS_OPEN); - } else { + if (rule.getType() == RuleType.SECURITY_HOTSPOT || issue.type() == RuleType.SECURITY_HOTSPOT) { issue.setStatus(Issue.STATUS_TO_REVIEW); + } else { + issue.setStatus(Issue.STATUS_OPEN); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java index ec10804b1a1..c0229977074 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java @@ -26,7 +26,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import javax.annotation.Nullable; -import org.sonar.api.issue.Issue; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.Duration; @@ -51,6 +50,8 @@ import org.sonar.scanner.protocol.output.ScannerReport.IssueType; import org.sonar.server.rule.CommonRuleKeys; import static org.apache.commons.lang.StringUtils.isNotEmpty; +import static org.sonar.api.issue.Issue.STATUS_OPEN; +import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; public class TrackerRawInputFactory { private static final long DEFAULT_EXTERNAL_ISSUE_EFFORT = 0l; @@ -100,7 +101,7 @@ public class TrackerRawInputFactory { for (DefaultIssue commonRuleIssue : commonRuleEngine.process(component)) { if (issueFilter.accept(commonRuleIssue, component)) { - result.add(init(commonRuleIssue)); + result.add(init(commonRuleIssue, STATUS_OPEN)); } } @@ -160,7 +161,7 @@ public class TrackerRawInputFactory { private DefaultIssue toIssue(LineHashSequence lineHashSeq, ScannerReport.Issue reportIssue) { DefaultIssue issue = new DefaultIssue(); - init(issue); + init(issue, STATUS_OPEN); RuleKey ruleKey = RuleKey.of(reportIssue.getRuleRepository(), reportIssue.getRuleKey()); issue.setRuleKey(ruleKey); if (reportIssue.hasTextRange()) { @@ -202,7 +203,8 @@ public class TrackerRawInputFactory { private DefaultIssue toExternalIssue(LineHashSequence lineHashSeq, ScannerReport.ExternalIssue reportExternalIssue, Map adHocRuleMap) { DefaultIssue issue = new DefaultIssue(); - init(issue); + RuleType type = toRuleType(reportExternalIssue.getType()); + init(issue, type == RuleType.SECURITY_HOTSPOT ? STATUS_TO_REVIEW : STATUS_OPEN); RuleKey ruleKey = RuleKey.of(RuleKey.EXTERNAL_RULE_REPO_PREFIX + reportExternalIssue.getEngineId(), reportExternalIssue.getRuleId()); issue.setRuleKey(ruleKey); @@ -235,7 +237,7 @@ public class TrackerRawInputFactory { } issue.setIsFromExternalRuleEngine(true); issue.setLocations(dbLocationsBuilder.build()); - issue.setType(toRuleType(reportExternalIssue.getType())); + issue.setType(type); ruleRepository.addOrUpdateAddHocRuleIfNeeded(ruleKey, () -> toAdHocRule(reportExternalIssue, adHocRuleMap.get(issue.ruleKey()))); return issue; @@ -264,9 +266,9 @@ public class TrackerRawInputFactory { } } - private DefaultIssue init(DefaultIssue issue) { + private DefaultIssue init(DefaultIssue issue, String initialStatus) { + issue.setStatus(initialStatus); issue.setResolution(null); - issue.setStatus(Issue.STATUS_OPEN); issue.setComponentUuid(component.getUuid()); issue.setComponentKey(component.getKey()); issue.setProjectUuid(treeRootHolder.getRoot().getUuid()); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java index bfeac612398..26136619f1e 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java @@ -20,11 +20,14 @@ package org.sonar.ce.task.projectanalysis.issue; import com.google.common.collect.Iterators; +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; import java.util.Collection; import java.util.Collections; import org.junit.Rule; import org.junit.Test; -import org.sonar.api.issue.Issue; +import org.junit.runner.RunWith; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.Severity; import org.sonar.api.rules.RuleType; @@ -43,6 +46,7 @@ import org.sonar.core.issue.tracking.Input; import org.sonar.db.protobuf.DbIssues; import org.sonar.scanner.protocol.Constants; import org.sonar.scanner.protocol.output.ScannerReport; +import org.sonar.scanner.protocol.output.ScannerReport.IssueType; import org.sonar.scanner.protocol.output.ScannerReport.TextRange; import org.sonar.server.rule.CommonRuleKeys; @@ -54,7 +58,10 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.sonar.api.issue.Issue.STATUS_OPEN; +import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; +@RunWith(DataProviderRunner.class) public class TrackerRawInputFactoryTest { private static final String FILE_UUID = "fake_uuid"; @@ -213,7 +220,8 @@ public class TrackerRawInputFactoryTest { } @Test - public void load_external_issues_from_report() { + @UseDataProvider("ruleTypeAndStatusByIssueType") + public void load_external_issues_from_report(IssueType issueType, RuleType expectedRuleType, String expectedStatus) { when(sourceLinesHash.getLineHashesMatchingDBVersion(FILE)).thenReturn(Collections.singletonList("line")); ScannerReport.ExternalIssue reportIssue = ScannerReport.ExternalIssue.newBuilder() .setTextRange(TextRange.newBuilder().setStartLine(2).build()) @@ -222,7 +230,7 @@ public class TrackerRawInputFactoryTest { .setRuleId("S001") .setSeverity(Constants.Severity.BLOCKER) .setEffort(20l) - .setType(ScannerReport.IssueType.SECURITY_HOTSPOT) + .setType(issueType) .build(); reportReader.putExternalIssues(FILE.getReportAttributes().getRef(), asList(reportIssue)); Input input = underTest.create(FILE); @@ -237,16 +245,27 @@ public class TrackerRawInputFactoryTest { assertThat(issue.line()).isEqualTo(2); assertThat(issue.effort()).isEqualTo(Duration.create(20l)); assertThat(issue.message()).isEqualTo("the message"); - assertThat(issue.type()).isEqualTo(RuleType.SECURITY_HOTSPOT); + assertThat(issue.type()).isEqualTo(expectedRuleType); // fields set by compute engine assertThat(issue.checksum()).isEqualTo(input.getLineHashSequence().getHashForLine(2)); assertThat(issue.tags()).isEmpty(); - assertInitializedExternalIssue(issue); + assertInitializedExternalIssue(issue, expectedStatus); + } + + @DataProvider + public static Object[][] ruleTypeAndStatusByIssueType() { + return new Object[][] { + {IssueType.CODE_SMELL, RuleType.CODE_SMELL, STATUS_OPEN}, + {IssueType.BUG, RuleType.BUG, STATUS_OPEN}, + {IssueType.VULNERABILITY, RuleType.VULNERABILITY, STATUS_OPEN}, + {IssueType.SECURITY_HOTSPOT, RuleType.SECURITY_HOTSPOT, STATUS_TO_REVIEW} + }; } @Test - public void load_external_issues_from_report_with_default_effort() { + @UseDataProvider("ruleTypeAndStatusByIssueType") + public void load_external_issues_from_report_with_default_effort(IssueType issueType, RuleType expectedRuleType, String expectedStatus) { when(sourceLinesHash.getLineHashesMatchingDBVersion(FILE)).thenReturn(Collections.singletonList("line")); ScannerReport.ExternalIssue reportIssue = ScannerReport.ExternalIssue.newBuilder() .setTextRange(TextRange.newBuilder().setStartLine(2).build()) @@ -254,7 +273,7 @@ public class TrackerRawInputFactoryTest { .setEngineId("eslint") .setRuleId("S001") .setSeverity(Constants.Severity.BLOCKER) - .setType(ScannerReport.IssueType.BUG) + .setType(issueType) .build(); reportReader.putExternalIssues(FILE.getReportAttributes().getRef(), asList(reportIssue)); Input input = underTest.create(FILE); @@ -269,11 +288,12 @@ public class TrackerRawInputFactoryTest { assertThat(issue.line()).isEqualTo(2); assertThat(issue.effort()).isEqualTo(Duration.create(0l)); assertThat(issue.message()).isEqualTo("the message"); + assertThat(issue.type()).isEqualTo(expectedRuleType); // fields set by compute engine assertThat(issue.checksum()).isEqualTo(input.getLineHashSequence().getHashForLine(2)); assertThat(issue.tags()).isEmpty(); - assertInitializedExternalIssue(issue); + assertInitializedExternalIssue(issue, expectedStatus); } @Test @@ -372,23 +392,17 @@ public class TrackerRawInputFactoryTest { } private void assertInitializedIssue(DefaultIssue issue) { - assertThat(issue.projectKey()).isEqualTo(PROJECT.getKey()); - assertThat(issue.componentKey()).isEqualTo(FILE.getKey()); - assertThat(issue.componentUuid()).isEqualTo(FILE.getUuid()); - assertThat(issue.resolution()).isNull(); - assertThat(issue.status()).isEqualTo(Issue.STATUS_OPEN); - assertThat(issue.key()).isNull(); - assertThat(issue.authorLogin()).isNull(); + assertInitializedExternalIssue(issue, STATUS_OPEN); assertThat(issue.effort()).isNull(); assertThat(issue.effortInMinutes()).isNull(); } - private void assertInitializedExternalIssue(DefaultIssue issue) { + private void assertInitializedExternalIssue(DefaultIssue issue, String expectedStatus) { assertThat(issue.projectKey()).isEqualTo(PROJECT.getKey()); assertThat(issue.componentKey()).isEqualTo(FILE.getKey()); assertThat(issue.componentUuid()).isEqualTo(FILE.getUuid()); assertThat(issue.resolution()).isNull(); - assertThat(issue.status()).isEqualTo(Issue.STATUS_OPEN); + assertThat(issue.status()).isEqualTo(expectedStatus); assertThat(issue.key()).isNull(); assertThat(issue.authorLogin()).isNull(); } -- 2.39.5