From a9cb1fca0c0b6ec2fa1f11a6a9068469fe955173 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Beno=C3=AEt=20Gianinetti?= Date: Wed, 24 Apr 2019 16:43:17 +0200 Subject: [PATCH] SONAR-11966 Add security hotspot to Github Checks --- .../issue/IntegrateIssuesVisitor.java | 32 ++++--------- .../issue/IntegrateIssuesVisitorTest.java | 2 +- .../rule/DefaultActiveRulesLoader.java | 26 ++-------- .../rule/DefaultActiveRulesLoaderTest.java | 47 ++++--------------- 4 files changed, 23 insertions(+), 84 deletions(-) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java index 2c51dff13ef..f6c6d5f4127 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java @@ -19,18 +19,16 @@ */ package org.sonar.ce.task.projectanalysis.issue; -import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.stream.Stream; -import org.sonar.api.rules.RuleType; -import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.CrawlerDepthLimit; import org.sonar.ce.task.projectanalysis.component.MergeAndTargetBranchComponentUuids; import org.sonar.ce.task.projectanalysis.component.TypeAwareVisitorAdapter; import org.sonar.ce.task.projectanalysis.util.cache.DiskCache; import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.util.stream.MoreCollectors; import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order.POST_ORDER; @@ -41,17 +39,14 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { private final IssueVisitors issueVisitors; private final IssueTrackingDelegator issueTracking; private final SiblingsIssueMerger issueStatusCopier; - private final AnalysisMetadataHolder analysisMetadataHolder; private final MergeAndTargetBranchComponentUuids mergeAndTargetBranchComponentUuids; - public IntegrateIssuesVisitor(IssueCache issueCache, IssueLifecycle issueLifecycle, IssueVisitors issueVisitors, - AnalysisMetadataHolder analysisMetadataHolder, IssueTrackingDelegator issueTracking, SiblingsIssueMerger issueStatusCopier, - MergeAndTargetBranchComponentUuids mergeAndTargetBranchComponentUuids) { + public IntegrateIssuesVisitor(IssueCache issueCache, IssueLifecycle issueLifecycle, IssueVisitors issueVisitors, IssueTrackingDelegator issueTracking, + SiblingsIssueMerger issueStatusCopier, MergeAndTargetBranchComponentUuids mergeAndTargetBranchComponentUuids) { super(CrawlerDepthLimit.FILE, POST_ORDER); this.issueCache = issueCache; this.issueLifecycle = issueLifecycle; this.issueVisitors = issueVisitors; - this.analysisMetadataHolder = analysisMetadataHolder; this.issueTracking = issueTracking; this.issueStatusCopier = issueStatusCopier; this.mergeAndTargetBranchComponentUuids = mergeAndTargetBranchComponentUuids; @@ -73,22 +68,17 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { } private void fillNewOpenIssues(Component component, Stream newIssues, DiskCache.DiskAppender cacheAppender) { - List list = new ArrayList<>(); + List newIssuesList = newIssues + .peek(issueLifecycle::initNewOpenIssue) + .collect(MoreCollectors.toList()); - newIssues.forEach(issue -> { - issueLifecycle.initNewOpenIssue(issue); - if (analysisMetadataHolder.isLongLivingBranch() || issue.type() != RuleType.SECURITY_HOTSPOT) { - list.add(issue); - } - }); - - if (list.isEmpty()) { + if (newIssuesList.isEmpty()) { return; } - issueStatusCopier.tryMerge(component, list); + issueStatusCopier.tryMerge(component, newIssuesList); - for (DefaultIssue issue : list) { + for (DefaultIssue issue : newIssuesList) { process(component, issue, cacheAppender); } } @@ -107,9 +97,7 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { DefaultIssue raw = entry.getKey(); DefaultIssue base = entry.getValue(); issueLifecycle.mergeExistingOpenIssue(raw, base); - if (analysisMetadataHolder.isLongLivingBranch() || raw.type() != RuleType.SECURITY_HOTSPOT) { - process(component, raw, cacheAppender); - } + process(component, raw, cacheAppender); } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java index 90853ca12bc..4c4ba86943e 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java @@ -155,7 +155,7 @@ public class IntegrateIssuesVisitorTest { treeRootHolder.setRoot(PROJECT); issueCache = new IssueCache(temp.newFile(), System2.INSTANCE); when(issueFilter.accept(any(DefaultIssue.class), eq(FILE))).thenReturn(true); - underTest = new IntegrateIssuesVisitor(issueCache, issueLifecycle, issueVisitors, analysisMetadataHolder, trackingDelegator, issueStatusCopier, mergeAndTargetBranchComponentUuids); + underTest = new IntegrateIssuesVisitor(issueCache, issueLifecycle, issueVisitors, trackingDelegator, issueStatusCopier, mergeAndTargetBranchComponentUuids); } @Test diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/rule/DefaultActiveRulesLoader.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/rule/DefaultActiveRulesLoader.java index dd5be5b771c..3f6cb897aaf 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/rule/DefaultActiveRulesLoader.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/rule/DefaultActiveRulesLoader.java @@ -21,17 +21,13 @@ package org.sonar.scanner.rule; import java.io.IOException; import java.io.InputStream; -import java.util.Arrays; import java.util.HashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import org.apache.commons.io.IOUtils; import org.sonar.api.rule.RuleKey; -import org.sonar.api.rules.RuleType; import org.sonar.scanner.bootstrap.ScannerWsClient; -import org.sonar.scanner.scan.branch.BranchConfiguration; import org.sonar.scanner.util.ScannerUtils; import org.sonarqube.ws.Rules; import org.sonarqube.ws.Rules.Active; @@ -46,23 +42,11 @@ import static org.sonar.api.utils.DateUtils.parseDateTime; public class DefaultActiveRulesLoader implements ActiveRulesLoader { private static final String RULES_SEARCH_URL = "/api/rules/search.protobuf?f=repo,name,severity,lang,internalKey,templateKey,params,actives,createdAt,updatedAt&activation=true"; - private static final String RULES_SEARCH_NO_HOTSPOT_URL; - - static { - // need to use static initializer because of https://bugs.openjdk.java.net/browse/JDK-8077605 - RULES_SEARCH_NO_HOTSPOT_URL = RULES_SEARCH_URL + "&types=" - + Arrays.stream(RuleType.values()) - .filter(t -> t != RuleType.SECURITY_HOTSPOT) - .map(Enum::name) - .collect(Collectors.joining(",")); - } private final ScannerWsClient wsClient; - private final BranchConfiguration branchConfiguration; - public DefaultActiveRulesLoader(ScannerWsClient wsClient, BranchConfiguration branchConfiguration) { + public DefaultActiveRulesLoader(ScannerWsClient wsClient) { this.wsClient = wsClient; - this.branchConfiguration = branchConfiguration; } @Override @@ -90,14 +74,10 @@ public class DefaultActiveRulesLoader implements ActiveRulesLoader { private String getUrl(String qualityProfileKey, int page, int pageSize) { StringBuilder builder = new StringBuilder(1024); - if (branchConfiguration.isShortOrPullRequest()) { - builder.append(RULES_SEARCH_NO_HOTSPOT_URL); - } else { - builder.append(RULES_SEARCH_URL); - } + builder.append(RULES_SEARCH_URL); builder.append("&qprofile=").append(ScannerUtils.encodeForUrl(qualityProfileKey)); - builder.append("&p=").append(page); builder.append("&ps=").append(pageSize); + builder.append("&p=").append(page); return builder.toString(); } diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/rule/DefaultActiveRulesLoaderTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/rule/DefaultActiveRulesLoaderTest.java index 1d35d6a8b78..ad46a6851ed 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/rule/DefaultActiveRulesLoaderTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/rule/DefaultActiveRulesLoaderTest.java @@ -21,7 +21,6 @@ package org.sonar.scanner.rule; import com.google.common.collect.ImmutableSortedMap; import java.io.ByteArrayInputStream; -import java.io.IOException; import java.io.InputStream; import java.util.Collection; import java.util.stream.IntStream; @@ -56,22 +55,21 @@ public class DefaultActiveRulesLoaderTest { private DefaultActiveRulesLoader loader; private ScannerWsClient wsClient; - private BranchConfiguration branchConfig; @Before public void setUp() { wsClient = mock(ScannerWsClient.class); - branchConfig = mock(BranchConfiguration.class); + BranchConfiguration branchConfig = mock(BranchConfiguration.class); when(branchConfig.isShortOrPullRequest()).thenReturn(false); - loader = new DefaultActiveRulesLoader(wsClient, branchConfig); + loader = new DefaultActiveRulesLoader(wsClient); } @Test - public void feed_real_response_encode_qp() throws IOException { + public void feed_real_response_encode_qp() { int total = PAGE_SIZE_1 + PAGE_SIZE_2; - WsTestUtil.mockStream(wsClient, urlOfPage(1, false), responseOfSize(PAGE_SIZE_1, total)); - WsTestUtil.mockStream(wsClient, urlOfPage(2, false), responseOfSize(PAGE_SIZE_2, total)); + WsTestUtil.mockStream(wsClient, urlOfPage(1), responseOfSize(PAGE_SIZE_1, total)); + WsTestUtil.mockStream(wsClient, urlOfPage(2), responseOfSize(PAGE_SIZE_2, total)); Collection activeRules = loader.load("c+-test_c+-values-17445"); assertThat(activeRules).hasSize(total); @@ -85,42 +83,15 @@ public class DefaultActiveRulesLoaderTest { .extracting(LoadedActiveRule::getSeverity) .containsExactly(SEVERITY_VALUE); - WsTestUtil.verifyCall(wsClient, urlOfPage(1, false)); - WsTestUtil.verifyCall(wsClient, urlOfPage(2, false)); + WsTestUtil.verifyCall(wsClient, urlOfPage(1)); + WsTestUtil.verifyCall(wsClient, urlOfPage(2)); verifyNoMoreInteractions(wsClient); } - @Test - public void no_hotspots_on_pr_or_short_branches() throws IOException { - when(branchConfig.isShortOrPullRequest()).thenReturn(true); - int total = PAGE_SIZE_1 + PAGE_SIZE_2; - - WsTestUtil.mockStream(wsClient, urlOfPage(1, true), responseOfSize(PAGE_SIZE_1, total)); - WsTestUtil.mockStream(wsClient, urlOfPage(2, true), responseOfSize(PAGE_SIZE_2, total)); - - Collection activeRules = loader.load("c+-test_c+-values-17445"); - assertThat(activeRules).hasSize(total); - assertThat(activeRules) - .filteredOn(r -> r.getRuleKey().equals(EXAMPLE_KEY)) - .extracting(LoadedActiveRule::getParams) - .extracting(p -> p.get(FORMAT_KEY)) - .containsExactly(FORMAT_VALUE); - assertThat(activeRules) - .filteredOn(r -> r.getRuleKey().equals(EXAMPLE_KEY)) - .extracting(LoadedActiveRule::getSeverity) - .containsExactly(SEVERITY_VALUE); - - WsTestUtil.verifyCall(wsClient, urlOfPage(1, true)); - WsTestUtil.verifyCall(wsClient, urlOfPage(2, true)); - - verifyNoMoreInteractions(wsClient); - } - - private String urlOfPage(int page, boolean noHotspots) { + private String urlOfPage(int page) { return "/api/rules/search.protobuf?f=repo,name,severity,lang,internalKey,templateKey,params,actives,createdAt,updatedAt&activation=true" - + (noHotspots ? "&types=CODE_SMELL,BUG,VULNERABILITY" : "") + "&qprofile=c%2B-test_c%2B-values-17445&p=" + page - + "&ps=500"; + + ("") + "&qprofile=c%2B-test_c%2B-values-17445&ps=500&p=" + page + ""; } /** -- 2.39.5