From: Julien HENRY Date: Wed, 11 Oct 2017 10:07:37 +0000 (+0200) Subject: SONAR-9913 Match issues from short living branches more aggressively X-Git-Tag: 6.7-RC1~154 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=777701594f14b0886684d1d81bf32539a7522146;p=sonarqube.git SONAR-9913 Match issues from short living branches more aggressively When there are multiple candidates (from multiple branches), prefer any of the RESOLVED one, before taking any of the CONFIRMED ones. --- diff --git a/server/sonar-db-dao/src/main/java/org/sonar/core/issue/ShortBranchIssue.java b/server/sonar-db-dao/src/main/java/org/sonar/core/issue/ShortBranchIssue.java index f13c0167797..0e2f88f94b9 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/core/issue/ShortBranchIssue.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/core/issue/ShortBranchIssue.java @@ -65,6 +65,7 @@ public class ShortBranchIssue implements Trackable { return ruleKey; } + @Override public String getStatus() { return status; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java index 267a4523e0f..2e5be262543 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java @@ -64,7 +64,7 @@ import org.sonar.server.computation.task.projectanalysis.issue.IssueCache; import org.sonar.server.computation.task.projectanalysis.issue.IssueCounter; import org.sonar.server.computation.task.projectanalysis.issue.IssueCreationDateCalculator; import org.sonar.server.computation.task.projectanalysis.issue.IssueLifecycle; -import org.sonar.server.computation.task.projectanalysis.issue.IssueStatusCopier; +import org.sonar.server.computation.task.projectanalysis.issue.ShortBranchIssueStatusCopier; import org.sonar.server.computation.task.projectanalysis.issue.IssueTrackingDelegator; import org.sonar.server.computation.task.projectanalysis.issue.IssueVisitors; import org.sonar.server.computation.task.projectanalysis.issue.IssuesRepositoryVisitor; @@ -73,7 +73,7 @@ import org.sonar.server.computation.task.projectanalysis.issue.MergeBranchTracke import org.sonar.server.computation.task.projectanalysis.issue.MovedIssueVisitor; import org.sonar.server.computation.task.projectanalysis.issue.NewEffortAggregator; import org.sonar.server.computation.task.projectanalysis.issue.RemoveProcessedComponentsVisitor; -import org.sonar.server.computation.task.projectanalysis.issue.ResolvedShortBranchIssuesLoader; +import org.sonar.server.computation.task.projectanalysis.issue.ShortBranchIssuesLoader; import org.sonar.server.computation.task.projectanalysis.issue.RuleRepositoryImpl; import org.sonar.server.computation.task.projectanalysis.issue.RuleTagsCopier; import org.sonar.server.computation.task.projectanalysis.issue.RuleTypeCopier; @@ -252,8 +252,8 @@ public final class ProjectAnalysisTaskContainerPopulator implements ContainerPop BaseIssuesLoader.class, IssueTrackingDelegator.class, BranchPersister.class, - ResolvedShortBranchIssuesLoader.class, - IssueStatusCopier.class, + ShortBranchIssuesLoader.class, + ShortBranchIssueStatusCopier.class, // filemove SourceSimilarityImpl.class, diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IntegrateIssuesVisitor.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IntegrateIssuesVisitor.java index 76cbed32dcf..26db336fcf5 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IntegrateIssuesVisitor.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IntegrateIssuesVisitor.java @@ -37,11 +37,11 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { private final IssueLifecycle issueLifecycle; private final IssueVisitors issueVisitors; private final IssueTrackingDelegator issueTracking; - private final IssueStatusCopier issueStatusCopier; + private final ShortBranchIssueStatusCopier issueStatusCopier; private final AnalysisMetadataHolder analysisMetadataHolder; public IntegrateIssuesVisitor(IssueCache issueCache, IssueLifecycle issueLifecycle, IssueVisitors issueVisitors, - AnalysisMetadataHolder analysisMetadataHolder, IssueTrackingDelegator issueTracking, IssueStatusCopier issueStatusCopier) { + AnalysisMetadataHolder analysisMetadataHolder, IssueTrackingDelegator issueTracking, ShortBranchIssueStatusCopier issueStatusCopier) { super(CrawlerDepthLimit.FILE, POST_ORDER); this.issueCache = issueCache; this.issueLifecycle = issueLifecycle; diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueStatusCopier.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueStatusCopier.java deleted file mode 100644 index 40f50c1d90c..00000000000 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueStatusCopier.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2017 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.server.computation.task.projectanalysis.issue; - -import java.util.Collection; -import java.util.Map; -import org.sonar.core.issue.DefaultIssue; -import org.sonar.core.issue.ShortBranchIssue; -import org.sonar.core.issue.tracking.SimpleTracker; -import org.sonar.core.issue.tracking.Tracking; -import org.sonar.server.computation.task.projectanalysis.component.Component; - -public class IssueStatusCopier { - private final ResolvedShortBranchIssuesLoader resolvedShortBranchIssuesLoader; - private final SimpleTracker tracker; - private final IssueLifecycle issueLifecycle; - - public IssueStatusCopier(ResolvedShortBranchIssuesLoader resolvedShortBranchIssuesLoader, IssueLifecycle issueLifecycle) { - this(resolvedShortBranchIssuesLoader, new SimpleTracker<>(), issueLifecycle); - } - - public IssueStatusCopier(ResolvedShortBranchIssuesLoader resolvedShortBranchIssuesLoader, SimpleTracker tracker, IssueLifecycle issueLifecycle) { - this.resolvedShortBranchIssuesLoader = resolvedShortBranchIssuesLoader; - this.tracker = tracker; - this.issueLifecycle = issueLifecycle; - } - - public void updateStatus(Component component, Collection newIssues) { - Collection shortBranchIssues = resolvedShortBranchIssuesLoader.create(component); - Tracking tracking = tracker.track(newIssues, shortBranchIssues); - - for (Map.Entry e : tracking.getMatchedRaws().entrySet()) { - ShortBranchIssue issue = e.getValue(); - issueLifecycle.copyResolution(e.getKey(), issue.getStatus(), issue.getResolution()); - } - } -} diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/ResolvedShortBranchIssuesLoader.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/ResolvedShortBranchIssuesLoader.java deleted file mode 100644 index 4e3e5c33af9..00000000000 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/ResolvedShortBranchIssuesLoader.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2017 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.server.computation.task.projectanalysis.issue; - -import java.util.Collection; -import java.util.Collections; -import java.util.Set; -import java.util.stream.Collectors; -import org.sonar.core.issue.ShortBranchIssue; -import org.sonar.db.DbClient; -import org.sonar.db.DbSession; -import org.sonar.db.component.ComponentDto; -import org.sonar.db.issue.ShortBranchIssueDto; -import org.sonar.server.computation.task.projectanalysis.component.Component; -import org.sonar.server.computation.task.projectanalysis.component.ShortBranchComponentsWithIssues; - -public class ResolvedShortBranchIssuesLoader { - - private final ShortBranchComponentsWithIssues shortBranchComponentsWithIssues; - private final DbClient dbClient; - - public ResolvedShortBranchIssuesLoader(ShortBranchComponentsWithIssues shortBranchComponentsWithIssues, DbClient dbClient) { - this.shortBranchComponentsWithIssues = shortBranchComponentsWithIssues; - this.dbClient = dbClient; - } - - public Collection create(Component component) { - String componentKey = ComponentDto.removeBranchFromKey(component.getKey()); - Set uuids = shortBranchComponentsWithIssues.getUuids(componentKey); - if (uuids.isEmpty()) { - return Collections.emptyList(); - } - try (DbSession session = dbClient.openSession(false)) { - return dbClient.issueDao().selectResolvedOrConfirmedByComponentUuids(session, uuids) - .stream() - .map(ShortBranchIssueDto::toShortBranchIssue) - .collect(Collectors.toList()); - } - } -} diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/ShortBranchIssueStatusCopier.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/ShortBranchIssueStatusCopier.java new file mode 100644 index 00000000000..8b49759aae6 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/ShortBranchIssueStatusCopier.java @@ -0,0 +1,55 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.computation.task.projectanalysis.issue; + +import java.util.Collection; +import java.util.Map; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.ShortBranchIssue; +import org.sonar.core.issue.tracking.SimpleTracker; +import org.sonar.core.issue.tracking.Tracking; +import org.sonar.server.computation.task.projectanalysis.component.Component; + +public class ShortBranchIssueStatusCopier { + private final ShortBranchIssuesLoader resolvedShortBranchIssuesLoader; + private final SimpleTracker tracker; + private final IssueLifecycle issueLifecycle; + + public ShortBranchIssueStatusCopier(ShortBranchIssuesLoader resolvedShortBranchIssuesLoader, IssueLifecycle issueLifecycle) { + this(resolvedShortBranchIssuesLoader, new SimpleTracker<>(), issueLifecycle); + } + + public ShortBranchIssueStatusCopier(ShortBranchIssuesLoader resolvedShortBranchIssuesLoader, SimpleTracker tracker, + IssueLifecycle issueLifecycle) { + this.resolvedShortBranchIssuesLoader = resolvedShortBranchIssuesLoader; + this.tracker = tracker; + this.issueLifecycle = issueLifecycle; + } + + public void updateStatus(Component component, Collection newIssues) { + Collection shortBranchIssues = resolvedShortBranchIssuesLoader.loadCandidateIssuesForMergingInTargetBranch(component); + Tracking tracking = tracker.track(newIssues, shortBranchIssues); + + for (Map.Entry e : tracking.getMatchedRaws().entrySet()) { + ShortBranchIssue issue = e.getValue(); + issueLifecycle.copyResolution(e.getKey(), issue.getStatus(), issue.getResolution()); + } + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/ShortBranchIssuesLoader.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/ShortBranchIssuesLoader.java new file mode 100644 index 00000000000..ebac0c2f982 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/ShortBranchIssuesLoader.java @@ -0,0 +1,57 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.computation.task.projectanalysis.issue; + +import java.util.Collection; +import java.util.Collections; +import java.util.Set; +import java.util.stream.Collectors; +import org.sonar.core.issue.ShortBranchIssue; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.issue.ShortBranchIssueDto; +import org.sonar.server.computation.task.projectanalysis.component.Component; +import org.sonar.server.computation.task.projectanalysis.component.ShortBranchComponentsWithIssues; + +public class ShortBranchIssuesLoader { + + private final ShortBranchComponentsWithIssues shortBranchComponentsWithIssues; + private final DbClient dbClient; + + public ShortBranchIssuesLoader(ShortBranchComponentsWithIssues shortBranchComponentsWithIssues, DbClient dbClient) { + this.shortBranchComponentsWithIssues = shortBranchComponentsWithIssues; + this.dbClient = dbClient; + } + + public Collection loadCandidateIssuesForMergingInTargetBranch(Component component) { + String componentKey = ComponentDto.removeBranchFromKey(component.getKey()); + Set uuids = shortBranchComponentsWithIssues.getUuids(componentKey); + if (uuids.isEmpty()) { + return Collections.emptyList(); + } + try (DbSession session = dbClient.openSession(false)) { + return dbClient.issueDao().selectResolvedOrConfirmedByComponentUuids(session, uuids) + .stream() + .map(ShortBranchIssueDto::toShortBranchIssue) + .collect(Collectors.toList()); + } + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java index 6dd63fa7f24..8f30a438333 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java @@ -113,7 +113,7 @@ public class IntegrateIssuesVisitorTest { @Mock private MergeBranchComponentUuids mergeBranchComponentsUuids; @Mock - private IssueStatusCopier issueStatusCopier; + private ShortBranchIssueStatusCopier issueStatusCopier; ArgumentCaptor defaultIssueCaptor; diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueStatusCopierTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueStatusCopierTest.java deleted file mode 100644 index 9716ed4dbca..00000000000 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueStatusCopierTest.java +++ /dev/null @@ -1,96 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2017 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.server.computation.task.projectanalysis.issue; - -import java.util.Collections; -import org.junit.Before; -import org.junit.Test; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; -import org.sonar.api.rule.RuleKey; -import org.sonar.core.issue.DefaultIssue; -import org.sonar.core.issue.ShortBranchIssue; -import org.sonar.core.issue.tracking.SimpleTracker; -import org.sonar.server.computation.task.projectanalysis.component.Component; - -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; -import static org.mockito.Mockito.when; - -public class IssueStatusCopierTest { - @Mock - private ResolvedShortBranchIssuesLoader resolvedShortBranchIssuesLoader; - @Mock - private IssueLifecycle issueLifecycle; - @Mock - private Component component; - - private SimpleTracker tracker = new SimpleTracker<>(); - private IssueStatusCopier copier; - - @Before - public void setUp() { - MockitoAnnotations.initMocks(this); - copier = new IssueStatusCopier(resolvedShortBranchIssuesLoader, tracker, issueLifecycle); - } - - @Test - public void do_nothing_if_no_match() { - when(resolvedShortBranchIssuesLoader.create(component)).thenReturn(Collections.emptyList()); - DefaultIssue i = createIssue("issue1", "rule1"); - copier.updateStatus(component, Collections.singleton(i)); - - verify(resolvedShortBranchIssuesLoader).create(component); - verifyZeroInteractions(issueLifecycle); - } - - @Test - public void do_nothing_if_no_new_issue() { - DefaultIssue i = createIssue("issue1", "rule1"); - when(resolvedShortBranchIssuesLoader.create(component)).thenReturn(Collections.singleton(newShortBranchIssue(i))); - copier.updateStatus(component, Collections.emptyList()); - - verify(resolvedShortBranchIssuesLoader).create(component); - verifyZeroInteractions(issueLifecycle); - } - - @Test - public void update_status_on_matches() { - ShortBranchIssue shortBranchIssue = newShortBranchIssue(createIssue("issue1", "rule1")); - DefaultIssue newIssue = createIssue("issue2", "rule1"); - - when(resolvedShortBranchIssuesLoader.create(component)).thenReturn(Collections.singleton(shortBranchIssue)); - copier.updateStatus(component, Collections.singleton(newIssue)); - verify(issueLifecycle).copyResolution(newIssue, shortBranchIssue.getStatus(), shortBranchIssue.getResolution()); - } - - private static DefaultIssue createIssue(String key, String ruleKey) { - DefaultIssue issue = new DefaultIssue(); - issue.setKey(key); - issue.setRuleKey(RuleKey.of("repo", ruleKey)); - issue.setMessage("msg"); - issue.setLine(1); - return issue; - } - - private ShortBranchIssue newShortBranchIssue(DefaultIssue i) { - return new ShortBranchIssue(i.line(), i.message(), i.getLineHash(), i.ruleKey(), i.status(), i.resolution()); - } -} diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/ShortBranchIssueStatusCopierTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/ShortBranchIssueStatusCopierTest.java new file mode 100644 index 00000000000..c5948cfc60b --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/ShortBranchIssueStatusCopierTest.java @@ -0,0 +1,113 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.computation.task.projectanalysis.issue; + +import java.util.Arrays; +import java.util.Collections; +import javax.annotation.Nullable; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.sonar.api.issue.Issue; +import org.sonar.api.rule.RuleKey; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.ShortBranchIssue; +import org.sonar.core.issue.tracking.SimpleTracker; +import org.sonar.server.computation.task.projectanalysis.component.Component; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + +public class ShortBranchIssueStatusCopierTest { + @Mock + private ShortBranchIssuesLoader resolvedShortBranchIssuesLoader; + @Mock + private IssueLifecycle issueLifecycle; + @Mock + private Component component; + + private SimpleTracker tracker = new SimpleTracker<>(); + private ShortBranchIssueStatusCopier copier; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + copier = new ShortBranchIssueStatusCopier(resolvedShortBranchIssuesLoader, tracker, issueLifecycle); + } + + @Test + public void do_nothing_if_no_match() { + when(resolvedShortBranchIssuesLoader.loadCandidateIssuesForMergingInTargetBranch(component)).thenReturn(Collections.emptyList()); + DefaultIssue i = createIssue("issue1", "rule1", Issue.STATUS_CONFIRMED, null); + copier.updateStatus(component, Collections.singleton(i)); + + verify(resolvedShortBranchIssuesLoader).loadCandidateIssuesForMergingInTargetBranch(component); + verifyZeroInteractions(issueLifecycle); + } + + @Test + public void do_nothing_if_no_new_issue() { + DefaultIssue i = createIssue("issue1", "rule1", Issue.STATUS_CONFIRMED, null); + when(resolvedShortBranchIssuesLoader.loadCandidateIssuesForMergingInTargetBranch(component)).thenReturn(Collections.singleton(newShortBranchIssue(i))); + copier.updateStatus(component, Collections.emptyList()); + + verify(resolvedShortBranchIssuesLoader).loadCandidateIssuesForMergingInTargetBranch(component); + verifyZeroInteractions(issueLifecycle); + } + + @Test + public void update_status_on_matches() { + ShortBranchIssue shortBranchIssue = newShortBranchIssue(createIssue("issue1", "rule1", Issue.STATUS_CONFIRMED, null)); + DefaultIssue newIssue = createIssue("issue2", "rule1", Issue.STATUS_OPEN, null); + + when(resolvedShortBranchIssuesLoader.loadCandidateIssuesForMergingInTargetBranch(component)).thenReturn(Collections.singleton(shortBranchIssue)); + copier.updateStatus(component, Collections.singleton(newIssue)); + verify(issueLifecycle).copyResolution(newIssue, shortBranchIssue.getStatus(), shortBranchIssue.getResolution()); + } + + @Test + public void prefer_resolved_issues() { + ShortBranchIssue shortBranchIssue1 = newShortBranchIssue(createIssue("issue1", "rule1", Issue.STATUS_CONFIRMED, null)); + ShortBranchIssue shortBranchIssue2 = newShortBranchIssue(createIssue("issue2", "rule1", Issue.STATUS_CONFIRMED, null)); + ShortBranchIssue shortBranchIssue3 = newShortBranchIssue(createIssue("issue3", "rule1", Issue.STATUS_RESOLVED, Issue.RESOLUTION_FALSE_POSITIVE)); + DefaultIssue newIssue = createIssue("newIssue", "rule1", Issue.STATUS_OPEN, null); + + when(resolvedShortBranchIssuesLoader.loadCandidateIssuesForMergingInTargetBranch(component)).thenReturn(Arrays.asList(shortBranchIssue1, shortBranchIssue2, shortBranchIssue3)); + copier.updateStatus(component, Collections.singleton(newIssue)); + verify(issueLifecycle).copyResolution(newIssue, Issue.STATUS_RESOLVED, Issue.RESOLUTION_FALSE_POSITIVE); + } + + private static DefaultIssue createIssue(String key, String ruleKey, String status, @Nullable String resolution) { + DefaultIssue issue = new DefaultIssue(); + issue.setKey(key); + issue.setRuleKey(RuleKey.of("repo", ruleKey)); + issue.setMessage("msg"); + issue.setLine(1); + issue.setStatus(status); + issue.setResolution(resolution); + return issue; + } + + private ShortBranchIssue newShortBranchIssue(DefaultIssue i) { + return new ShortBranchIssue(i.line(), i.message(), i.getLineHash(), i.ruleKey(), i.status(), i.resolution()); + } +} diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java index 198515da6d0..fd729489a70 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java @@ -648,4 +648,9 @@ public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure. public RuleKey getRuleKey() { return ruleKey; } + + @Override + public String getStatus() { + return status; + } } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/AbstractTracker.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/AbstractTracker.java index bdf7dd75d5d..69096993324 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/AbstractTracker.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/AbstractTracker.java @@ -22,19 +22,20 @@ package org.sonar.core.issue.tracking; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.Multimap; import java.util.Collection; -import java.util.Iterator; import java.util.Objects; +import java.util.function.Function; import javax.annotation.Nonnull; import org.apache.commons.lang.StringUtils; +import org.sonar.api.issue.Issue; import org.sonar.api.rule.RuleKey; public class AbstractTracker { - protected void match(Tracking tracking, SearchKeyFactory factory) { - match(tracking, factory, false); + protected void match(Tracking tracking, Function searchKeyFactory) { + match(tracking, searchKeyFactory, false); } - protected void match(Tracking tracking, SearchKeyFactory factory, boolean rejectMultipleMatches) { + protected void match(Tracking tracking, Function searchKeyFactory, boolean preferResolved) { if (tracking.isComplete()) { return; @@ -42,39 +43,39 @@ public class AbstractTracker { Multimap baseSearch = ArrayListMultimap.create(); for (BASE base : tracking.getUnmatchedBases()) { - baseSearch.put(factory.create(base), base); + baseSearch.put(searchKeyFactory.apply(base), base); } for (RAW raw : tracking.getUnmatchedRaws()) { - SearchKey rawKey = factory.create(raw); + SearchKey rawKey = searchKeyFactory.apply(raw); Collection bases = baseSearch.get(rawKey); if (!bases.isEmpty()) { - Iterator it = bases.iterator(); - BASE match = it.next(); - if (rejectMultipleMatches && it.hasNext()) { - continue; + BASE match; + if (preferResolved) { + match = bases.stream() + .filter(i -> Issue.STATUS_RESOLVED.equals(i.getStatus())) + .findFirst() + .orElse(bases.iterator().next()); + } else { + // TODO taking the first one. Could be improved if there are more than 2 issues on the same line. + // Message could be checked to take the best one. + match = bases.iterator().next(); } - // TODO taking the first one. Could be improved if there are more than 2 issues on the same line. - // Message could be checked to take the best one. tracking.match(raw, match); baseSearch.remove(rawKey, match); } } } - private interface SearchKey { + protected interface SearchKey { } - private interface SearchKeyFactory { - SearchKey create(Trackable trackable); - } - - private static class LineAndLineHashKey implements SearchKey { + protected static class LineAndLineHashKey implements SearchKey { private final RuleKey ruleKey; private final String lineHash; private final Integer line; - LineAndLineHashKey(Trackable trackable) { + protected LineAndLineHashKey(Trackable trackable) { this.ruleKey = trackable.getRuleKey(); this.line = trackable.getLine(); this.lineHash = StringUtils.defaultString(trackable.getLineHash(), ""); @@ -98,15 +99,7 @@ public class AbstractTracker { } } - protected enum LineAndLineHashKeyFactory implements SearchKeyFactory { - INSTANCE; - @Override - public SearchKey create(Trackable t) { - return new LineAndLineHashKey(t); - } - } - - private static class LineHashAndMessageKey implements SearchKey { + protected static class LineHashAndMessageKey implements SearchKey { private final RuleKey ruleKey; private final String message; private final String lineHash; @@ -135,15 +128,7 @@ public class AbstractTracker { } } - protected enum LineHashAndMessageKeyFactory implements SearchKeyFactory { - INSTANCE; - @Override - public SearchKey create(Trackable t) { - return new LineHashAndMessageKey(t); - } - } - - private static class LineAndMessageKey implements SearchKey { + protected static class LineAndMessageKey implements SearchKey { private final RuleKey ruleKey; private final String message; private final Integer line; @@ -172,15 +157,7 @@ public class AbstractTracker { } } - protected enum LineAndMessageKeyFactory implements SearchKeyFactory { - INSTANCE; - @Override - public SearchKey create(Trackable t) { - return new LineAndMessageKey(t); - } - } - - private static class LineHashKey implements SearchKey { + protected static class LineHashKey implements SearchKey { private final RuleKey ruleKey; private final String lineHash; @@ -206,11 +183,4 @@ public class AbstractTracker { } } - protected enum LineHashKeyFactory implements SearchKeyFactory { - INSTANCE; - @Override - public SearchKey create(Trackable t) { - return new LineHashKey(t); - } - } } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/SimpleTracker.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/SimpleTracker.java index 4ae1833c8f4..54fd8ea2bba 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/SimpleTracker.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/SimpleTracker.java @@ -31,10 +31,10 @@ public class SimpleTracker extend Tracking tracking = new Tracking<>(rawInput, baseInput); // 1. match issues with same rule, same line and same line hash, but not necessarily with same message - match(tracking, LineAndLineHashKeyFactory.INSTANCE, true); + match(tracking, LineAndLineHashKey::new, true); // 2. match issues with same rule, same message and same line hash - match(tracking, LineHashAndMessageKeyFactory.INSTANCE, true); + match(tracking, LineHashAndMessageKey::new, true); return tracking; } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Trackable.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Trackable.java index c723ef06f27..0d04b81e366 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Trackable.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Trackable.java @@ -40,4 +40,6 @@ public interface Trackable { String getLineHash(); RuleKey getRuleKey(); + + String getStatus(); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java index 2039219ff89..dcbbb869ac1 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java @@ -19,8 +19,8 @@ */ package org.sonar.core.issue.tracking; -import org.sonar.api.batch.ScannerSide; import org.sonar.api.batch.InstantiationStrategy; +import org.sonar.api.batch.ScannerSide; @InstantiationStrategy(InstantiationStrategy.PER_BATCH) @ScannerSide @@ -30,20 +30,20 @@ public class Tracker extends Abst Tracking tracking = new Tracking<>(rawInput.getIssues(), baseInput.getIssues()); // 1. match issues with same rule, same line and same line hash, but not necessarily with same message - match(tracking, LineAndLineHashKeyFactory.INSTANCE); + match(tracking, LineAndLineHashKey::new); // 2. detect code moves by comparing blocks of codes detectCodeMoves(rawInput, baseInput, tracking); // 3. match issues with same rule, same message and same line hash - match(tracking, LineHashAndMessageKeyFactory.INSTANCE); + match(tracking, LineHashAndMessageKey::new); // 4. match issues with same rule, same line and same message - match(tracking, LineAndMessageKeyFactory.INSTANCE); + match(tracking, LineAndMessageKey::new); // 5. match issues with same rule and same line hash but different line and different message. // See SONAR-2812 - match(tracking, LineHashKeyFactory.INSTANCE); + match(tracking, LineHashKey::new); return tracking; } diff --git a/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java b/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java index 3f5b28cdba4..59e106f1433 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java @@ -186,7 +186,7 @@ public class TrackerTest { @Test public void do_not_fail_if_raw_line_does_not_exist() { FakeInput baseInput = new FakeInput(); - FakeInput rawInput = new FakeInput("H1").addIssue(new Issue(200, "H200", RULE_SYSTEM_PRINT, "msg")); + FakeInput rawInput = new FakeInput("H1").addIssue(new Issue(200, "H200", RULE_SYSTEM_PRINT, "msg", org.sonar.api.issue.Issue.STATUS_OPEN)); Tracking tracking = tracker.track(rawInput, baseInput); @@ -385,11 +385,13 @@ public class TrackerTest { private final RuleKey ruleKey; private final Integer line; private final String message, lineHash; + private final String status; - Issue(@Nullable Integer line, String lineHash, RuleKey ruleKey, String message) { + Issue(@Nullable Integer line, String lineHash, RuleKey ruleKey, String message, String status) { this.line = line; this.lineHash = lineHash; this.ruleKey = ruleKey; + this.status = status; this.message = trim(message); } @@ -412,6 +414,11 @@ public class TrackerTest { public RuleKey getRuleKey() { return ruleKey; } + + @Override + public String getStatus() { + return status; + } } private static class FakeInput implements Input { @@ -431,7 +438,7 @@ public class TrackerTest { } Issue createIssueOnLine(int line, RuleKey ruleKey, String message) { - Issue issue = new Issue(line, lineHashes.get(line - 1), ruleKey, message); + Issue issue = new Issue(line, lineHashes.get(line - 1), ruleKey, message, org.sonar.api.issue.Issue.STATUS_OPEN); issues.add(issue); return issue; } @@ -440,7 +447,7 @@ public class TrackerTest { * No line (line 0) */ Issue createIssue(RuleKey ruleKey, String message) { - Issue issue = new Issue(null, "", ruleKey, message); + Issue issue = new Issue(null, "", ruleKey, message, org.sonar.api.issue.Issue.STATUS_OPEN); issues.add(issue); return issue; } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/ServerIssueFromWs.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/ServerIssueFromWs.java index 914f8af2c88..a0888d20341 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/ServerIssueFromWs.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/ServerIssueFromWs.java @@ -63,4 +63,9 @@ public class ServerIssueFromWs implements Trackable { return dto.hasMsg() ? trim(dto.getMsg()) : ""; } + @Override + public String getStatus() { + throw new UnsupportedOperationException(); + } + } 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 d1b68964093..aae406f92a0 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 @@ -180,6 +180,11 @@ public class TrackedIssue implements Trackable, Serializable { return ruleKey; } + @Override + public String getStatus() { + return status; + } + public String severity() { return severity; }