From 3fe62fecd08570c507506bebddf96ef47d3923a5 Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Tue, 12 Jan 2021 15:22:28 -0600 Subject: [PATCH] SONAR-14257 Pull requests should inherit issue state from its target branch --- ...ProjectAnalysisTaskContainerPopulator.java | 5 +- .../issue/PullRequestTrackerExecution.java | 32 ++++- .../ReferenceBranchTrackerExecution.java | 3 +- .../issue/TargetBranchComponentUuids.java | 89 +++++++++++++ .../TrackerTargetBranchInputFactory.java | 90 +++++++++++++ .../issue/IntegrateIssuesVisitorTest.java | 5 +- .../PullRequestTrackerExecutionTest.java | 60 ++++++--- .../issue/TargetBranchComponentUuidsTest.java | 126 ++++++++++++++++++ .../TrackerTargetBranchInputFactoryTest.java | 111 +++++++++++++++ .../core/issue/tracking/AbstractTracker.java | 20 +-- .../sonar/core/issue/tracking/Tracker.java | 29 ++-- 11 files changed, 510 insertions(+), 60 deletions(-) create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TargetBranchComponentUuids.java create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactory.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TargetBranchComponentUuidsTest.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactoryTest.java diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java index 52637abdf53..89f83b97594 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java @@ -81,11 +81,13 @@ import org.sonar.ce.task.projectanalysis.issue.ScmAccountToUserLoader; import org.sonar.ce.task.projectanalysis.issue.SiblingsIssueMerger; import org.sonar.ce.task.projectanalysis.issue.SiblingsIssuesLoader; import org.sonar.ce.task.projectanalysis.issue.SourceBranchComponentUuids; +import org.sonar.ce.task.projectanalysis.issue.TargetBranchComponentUuids; import org.sonar.ce.task.projectanalysis.issue.TrackerBaseInputFactory; import org.sonar.ce.task.projectanalysis.issue.TrackerExecution; import org.sonar.ce.task.projectanalysis.issue.TrackerRawInputFactory; import org.sonar.ce.task.projectanalysis.issue.TrackerReferenceBranchInputFactory; import org.sonar.ce.task.projectanalysis.issue.TrackerSourceBranchInputFactory; +import org.sonar.ce.task.projectanalysis.issue.TrackerTargetBranchInputFactory; import org.sonar.ce.task.projectanalysis.issue.UpdateConflictResolver; import org.sonar.ce.task.projectanalysis.issue.commonrule.BranchCoverageRule; import org.sonar.ce.task.projectanalysis.issue.commonrule.CommentDensityRule; @@ -273,9 +275,10 @@ public final class ProjectAnalysisTaskContainerPopulator implements ContainerPop NewSecurityReviewMeasuresVisitor.class, LastCommitVisitor.class, MeasureComputersVisitor.class, - + TargetBranchComponentUuids.class, UpdateConflictResolver.class, TrackerBaseInputFactory.class, + TrackerTargetBranchInputFactory.class, TrackerRawInputFactory.class, TrackerReferenceBranchInputFactory.class, TrackerSourceBranchInputFactory.class, diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/PullRequestTrackerExecution.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/PullRequestTrackerExecution.java index eb9dbfe9e3b..5d51346180e 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/PullRequestTrackerExecution.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/PullRequestTrackerExecution.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import org.sonar.api.issue.Issue; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.source.NewLinesRepository; import org.sonar.core.issue.DefaultIssue; @@ -36,23 +37,40 @@ public class PullRequestTrackerExecution { private final TrackerBaseInputFactory baseInputFactory; private final Tracker tracker; private final NewLinesRepository newLinesRepository; + private final TrackerTargetBranchInputFactory targetInputFactory; - public PullRequestTrackerExecution(TrackerBaseInputFactory baseInputFactory, Tracker tracker, - NewLinesRepository newLinesRepository) { + public PullRequestTrackerExecution(TrackerBaseInputFactory baseInputFactory, TrackerTargetBranchInputFactory targetInputFactory, + Tracker tracker, NewLinesRepository newLinesRepository) { this.baseInputFactory = baseInputFactory; + this.targetInputFactory = targetInputFactory; this.tracker = tracker; this.newLinesRepository = newLinesRepository; } public Tracking track(Component component, Input rawInput) { - Input previousAnalysisInput = baseInputFactory.create(component); - // Step 1: only keep issues on changed lines List filteredRaws = keepIssuesHavingAtLeastOneLocationOnChangedLines(component, rawInput.getIssues()); - Input unmatchedRawsAfterChangedLineFiltering = new DefaultTrackingInput(filteredRaws, rawInput.getLineHashSequence(), rawInput.getBlockHashSequence()); + Input unmatchedRawsAfterChangedLineFiltering = createInput(rawInput, filteredRaws); + + // Step 2: remove issues that are resolved in the target branch + Input unmatchedRawsAfterTargetResolvedTracking; + if (targetInputFactory.hasTargetBranchAnalysis()) { + Input targetInput = targetInputFactory.createForTargetBranch(component); + List resolvedTargetIssues = targetInput.getIssues().stream().filter(i -> Issue.STATUS_RESOLVED.equals(i.status())).collect(Collectors.toList()); + Input resolvedTargetInput = createInput(targetInput, resolvedTargetIssues); + Tracking prResolvedTracking = tracker.trackNonClosed(unmatchedRawsAfterChangedLineFiltering, resolvedTargetInput); + unmatchedRawsAfterTargetResolvedTracking = createInput(rawInput, prResolvedTracking.getUnmatchedRaws().collect(Collectors.toList())); + } else { + unmatchedRawsAfterTargetResolvedTracking = unmatchedRawsAfterChangedLineFiltering; + } + + // Step 3: track issues with previous analysis of the current PR + Input previousAnalysisInput = baseInputFactory.create(component); + return tracker.trackNonClosed(unmatchedRawsAfterTargetResolvedTracking, previousAnalysisInput); + } - // Step 2: track issues with previous analysis of the current PR - return tracker.trackNonClosed(unmatchedRawsAfterChangedLineFiltering, previousAnalysisInput); + private static Input createInput(Input input, Collection issues) { + return new DefaultTrackingInput(issues, input.getLineHashSequence(), input.getBlockHashSequence()); } private List keepIssuesHavingAtLeastOneLocationOnChangedLines(Component component, Collection issues) { diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ReferenceBranchTrackerExecution.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ReferenceBranchTrackerExecution.java index 080f3ed704c..ab7735f1ff2 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ReferenceBranchTrackerExecution.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ReferenceBranchTrackerExecution.java @@ -29,8 +29,7 @@ public class ReferenceBranchTrackerExecution { private final TrackerReferenceBranchInputFactory referenceBranchInputFactory; private final Tracker tracker; - public ReferenceBranchTrackerExecution(TrackerReferenceBranchInputFactory referenceBranchInputFactory, - Tracker tracker) { + public ReferenceBranchTrackerExecution(TrackerReferenceBranchInputFactory referenceBranchInputFactory, Tracker tracker) { this.referenceBranchInputFactory = referenceBranchInputFactory; this.tracker = tracker; } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TargetBranchComponentUuids.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TargetBranchComponentUuids.java new file mode 100644 index 00000000000..88866706e87 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TargetBranchComponentUuids.java @@ -0,0 +1,89 @@ +/* + * SonarQube + * Copyright (C) 2009-2021 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.ce.task.projectanalysis.issue; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import javax.annotation.CheckForNull; +import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.component.BranchDto; +import org.sonar.db.component.ComponentDto; + +import static org.sonar.db.component.ComponentDto.removeBranchAndPullRequestFromKey; + +/** + * Cache a map between component keys and uuids in the merge branch and optionally the target branch (for PR and SLB, and only if this target branch is analyzed) + */ +public class TargetBranchComponentUuids { + private final AnalysisMetadataHolder analysisMetadataHolder; + private final DbClient dbClient; + private Map targetBranchComponentsUuidsByKey; + private boolean hasTargetBranchAnalysis; + @CheckForNull + private String targetBranchUuid; + + public TargetBranchComponentUuids(AnalysisMetadataHolder analysisMetadataHolder, DbClient dbClient) { + this.analysisMetadataHolder = analysisMetadataHolder; + this.dbClient = dbClient; + } + + private void lazyInit() { + if (targetBranchComponentsUuidsByKey == null) { + targetBranchComponentsUuidsByKey = new HashMap<>(); + + if (analysisMetadataHolder.isPullRequest()) { + try (DbSession dbSession = dbClient.openSession(false)) { + initForTargetBranch(dbSession); + } + } else { + hasTargetBranchAnalysis = false; + } + } + } + + private void initForTargetBranch(DbSession dbSession) { + Optional branchDtoOpt = dbClient.branchDao().selectByBranchKey(dbSession, analysisMetadataHolder.getProject().getUuid(), + analysisMetadataHolder.getBranch().getTargetBranchName()); + targetBranchUuid = branchDtoOpt.map(BranchDto::getUuid).orElse(null); + hasTargetBranchAnalysis = targetBranchUuid != null && dbClient.snapshotDao().selectLastAnalysisByRootComponentUuid(dbSession, targetBranchUuid).isPresent(); + if (hasTargetBranchAnalysis) { + List targetComponents = dbClient.componentDao().selectByProjectUuid(targetBranchUuid, dbSession); + for (ComponentDto dto : targetComponents) { + targetBranchComponentsUuidsByKey.put(dto.getKey(), dto.uuid()); + } + } + } + + public boolean hasTargetBranchAnalysis() { + lazyInit(); + return hasTargetBranchAnalysis; + } + + @CheckForNull + public String getTargetBranchComponentUuid(String dbKey) { + lazyInit(); + String cleanComponentKey = removeBranchAndPullRequestFromKey(dbKey); + return targetBranchComponentsUuidsByKey.get(cleanComponentKey); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactory.java new file mode 100644 index 00000000000..d5197a3f893 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactory.java @@ -0,0 +1,90 @@ +/* + * SonarQube + * Copyright (C) 2009-2021 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.ce.task.projectanalysis.issue; + +import java.util.Collections; +import java.util.List; +import javax.annotation.Nullable; +import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.tracking.Input; +import org.sonar.core.issue.tracking.LazyInput; +import org.sonar.core.issue.tracking.LineHashSequence; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; + +public class TrackerTargetBranchInputFactory { + private static final LineHashSequence EMPTY_LINE_HASH_SEQUENCE = new LineHashSequence(Collections.emptyList()); + + private final ComponentIssuesLoader componentIssuesLoader; + private final DbClient dbClient; + private final TargetBranchComponentUuids targetBranchComponentUuids; + + public TrackerTargetBranchInputFactory(ComponentIssuesLoader componentIssuesLoader, TargetBranchComponentUuids targetBranchComponentUuids, + DbClient dbClient) { + this.componentIssuesLoader = componentIssuesLoader; + this.targetBranchComponentUuids = targetBranchComponentUuids; + this.dbClient = dbClient; + // TODO detect file moves? + } + + public boolean hasTargetBranchAnalysis() { + return targetBranchComponentUuids.hasTargetBranchAnalysis(); + } + + public Input createForTargetBranch(Component component) { + String targetBranchComponentUuid = targetBranchComponentUuids.getTargetBranchComponentUuid(component.getDbKey()); + return new TargetLazyInput(component.getType(), targetBranchComponentUuid); + } + + private class TargetLazyInput extends LazyInput { + private final Component.Type type; + private final String targetBranchComponentUuid; + + private TargetLazyInput(Component.Type type, @Nullable String targetBranchComponentUuid) { + this.type = type; + this.targetBranchComponentUuid = targetBranchComponentUuid; + } + + @Override + protected LineHashSequence loadLineHashSequence() { + if (targetBranchComponentUuid == null || type != Component.Type.FILE) { + return EMPTY_LINE_HASH_SEQUENCE; + } + + try (DbSession session = dbClient.openSession(false)) { + List hashes = dbClient.fileSourceDao().selectLineHashes(session, targetBranchComponentUuid); + if (hashes == null || hashes.isEmpty()) { + return EMPTY_LINE_HASH_SEQUENCE; + } + return new LineHashSequence(hashes); + } + } + + @Override + protected List loadIssues() { + if (targetBranchComponentUuid == null) { + return Collections.emptyList(); + } + return componentIssuesLoader.loadOpenIssuesWithChanges(targetBranchComponentUuid); + } + } + +} 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 618e7ab7a0f..b68348ecc41 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 @@ -127,7 +127,7 @@ public class IntegrateIssuesVisitorTest { private final ReferenceBranchComponentUuids referenceBranchComponentUuids = mock(ReferenceBranchComponentUuids.class); private final SourceLinesHashRepository sourceLinesHash = mock(SourceLinesHashRepository.class); private final NewLinesRepository newLinesRepository = mock(NewLinesRepository.class); - + private TargetBranchComponentUuids targetBranchComponentUuids = mock(TargetBranchComponentUuids.class); private ArgumentCaptor defaultIssueCaptor; private final ComponentIssuesLoader issuesLoader = new ComponentIssuesLoader(dbTester.getDbClient(), ruleRepositoryRule, activeRulesHolderRule, new MapSettings().asConfig(), @@ -153,11 +153,12 @@ public class IntegrateIssuesVisitorTest { issueFilter, ruleRepositoryRule, activeRulesHolder); TrackerBaseInputFactory baseInputFactory = new TrackerBaseInputFactory(issuesLoader, dbClient, movedFilesRepository, mock(ReportModulesPath.class), analysisMetadataHolder, new IssueFieldsSetter(), mock(ComponentsWithUnprocessedIssues.class)); + TrackerTargetBranchInputFactory targetInputFactory = new TrackerTargetBranchInputFactory(issuesLoader, targetBranchComponentUuids, dbClient); TrackerReferenceBranchInputFactory mergeInputFactory = new TrackerReferenceBranchInputFactory(issuesLoader, mergeBranchComponentsUuids, dbClient); ClosedIssuesInputFactory closedIssuesInputFactory = new ClosedIssuesInputFactory(issuesLoader, dbClient, movedFilesRepository); tracker = new TrackerExecution(baseInputFactory, closedIssuesInputFactory, new Tracker<>(), issuesLoader, analysisMetadataHolder); - prBranchTracker = new PullRequestTrackerExecution(baseInputFactory, new Tracker<>(), newLinesRepository); mergeBranchTracker = new ReferenceBranchTrackerExecution(mergeInputFactory, new Tracker<>()); + prBranchTracker = new PullRequestTrackerExecution(baseInputFactory, targetInputFactory, new Tracker<>(), newLinesRepository); trackingDelegator = new IssueTrackingDelegator(prBranchTracker, mergeBranchTracker, tracker, analysisMetadataHolder); treeRootHolder.setRoot(PROJECT); protoIssueCache = new ProtoIssueCache(temp.newFile(), System2.INSTANCE); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/PullRequestTrackerExecutionTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/PullRequestTrackerExecutionTest.java index 0f0f13015b8..07591fe172b 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/PullRequestTrackerExecutionTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/PullRequestTrackerExecutionTest.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Optional; import org.junit.Before; import org.junit.Test; +import org.sonar.api.issue.Issue; import org.sonar.api.rule.RuleKey; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.source.NewLinesRepository; @@ -60,26 +61,26 @@ public class PullRequestTrackerExecutionTest { private final NewLinesRepository newLinesRepository = mock(NewLinesRepository.class); private final List rawIssues = new ArrayList<>(); private final List baseIssues = new ArrayList<>(); + private final List targetIssues = new ArrayList<>(); private PullRequestTrackerExecution underTest; + private TrackerTargetBranchInputFactory targetFactory = mock(TrackerTargetBranchInputFactory.class); + @Before public void setUp() { when(baseFactory.create(FILE)).thenReturn(createInput(baseIssues)); + when(targetFactory.createForTargetBranch(FILE)).thenReturn(createInput(targetIssues)); Tracker tracker = new Tracker<>(); - underTest = new PullRequestTrackerExecution(baseFactory, tracker, newLinesRepository); + underTest = new PullRequestTrackerExecution(baseFactory, targetFactory, tracker, newLinesRepository); } @Test public void simple_tracking_keep_only_issues_having_location_on_changed_lines() { - final DefaultIssue issue1 = createIssue(2, RuleTesting.XOO_X1); - issue1.setLocations(DbIssues.Locations.newBuilder() - .setTextRange(DbCommons.TextRange.newBuilder().setStartLine(2).setEndLine(3)).build()); + final DefaultIssue issue1 = createIssue(2, 3, RuleTesting.XOO_X1); rawIssues.add(issue1); - final DefaultIssue issue2 = createIssue(2, RuleTesting.XOO_X1); - issue2.setLocations(DbIssues.Locations.newBuilder().setTextRange(DbCommons.TextRange.newBuilder().setStartLine(2).setEndLine(2)).build()); - rawIssues.add(issue2); + rawIssues.add(createIssue(2, RuleTesting.XOO_X1)); when(newLinesRepository.getNewLines(FILE)).thenReturn(Optional.of(new HashSet<>(Arrays.asList(1, 3)))); @@ -127,16 +128,36 @@ public class PullRequestTrackerExecutionTest { } @Test - public void track_and_ignore_issues_from_previous_analysis() { + public void track_and_ignore_resolved_issues_from_target_branch() { when(newLinesRepository.getNewLines(FILE)).thenReturn(Optional.of(new HashSet<>(Arrays.asList(1, 2, 3)))); - rawIssues.add(createIssue(1, RuleTesting.XOO_X1) - .setLocations(DbIssues.Locations.newBuilder().setTextRange(DbCommons.TextRange.newBuilder().setStartLine(1).setEndLine(1)).build())); - rawIssues.add(createIssue(2, RuleTesting.XOO_X2) - .setLocations(DbIssues.Locations.newBuilder().setTextRange(DbCommons.TextRange.newBuilder().setStartLine(2).setEndLine(2)).build())); - rawIssues.add(createIssue(3, RuleTesting.XOO_X3) - .setLocations(DbIssues.Locations.newBuilder().setTextRange(DbCommons.TextRange.newBuilder().setStartLine(3).setEndLine(3)).build())); + rawIssues.add(createIssue(1, RuleTesting.XOO_X1)); + rawIssues.add(createIssue(2, RuleTesting.XOO_X2)); + rawIssues.add(createIssue(3, RuleTesting.XOO_X3)); + + when(targetFactory.hasTargetBranchAnalysis()).thenReturn(true); + DefaultIssue resolvedIssue = createIssue(1, RuleTesting.XOO_X1).setStatus(Issue.STATUS_RESOLVED).setResolution(Issue.RESOLUTION_FALSE_POSITIVE); + // will cause rawIssue0 to be ignored + targetIssues.add(resolvedIssue); + // not ignored since it's not resolved + targetIssues.add(rawIssues.get(1)); + baseIssues.add(rawIssues.get(0)); + // should be matched + baseIssues.add(rawIssues.get(1)); + + Tracking tracking = underTest.track(FILE, createInput(rawIssues)); + assertThat(tracking.getMatchedRaws()).isEqualTo(Collections.singletonMap(rawIssues.get(1), rawIssues.get(1))); + assertThat(tracking.getUnmatchedRaws()).containsOnly(rawIssues.get(2)); + } + + @Test + public void track_and_ignore_issues_from_previous_analysis() { + when(newLinesRepository.getNewLines(FILE)).thenReturn(Optional.of(new HashSet<>(Arrays.asList(1, 2, 3)))); + + rawIssues.add(createIssue(1, RuleTesting.XOO_X1)); + rawIssues.add(createIssue(2, RuleTesting.XOO_X2)); + rawIssues.add(createIssue(3, RuleTesting.XOO_X3)); baseIssues.add(rawIssues.get(0)); Tracking tracking = underTest.track(FILE, createInput(rawIssues)); @@ -145,13 +166,18 @@ public class PullRequestTrackerExecutionTest { } private DefaultIssue createIssue(int line, RuleKey ruleKey) { + return createIssue(line, line, ruleKey); + } + + private DefaultIssue createIssue(int startLine, int endLine, RuleKey ruleKey) { return new DefaultIssue() .setRuleKey(ruleKey) - .setLine(line) - .setMessage("msg" + line); + .setLine(startLine) + .setLocations(DbIssues.Locations.newBuilder().setTextRange(DbCommons.TextRange.newBuilder().setStartLine(startLine).setEndLine(endLine)).build()) + .setMessage("msg" + startLine); } - private Input createInput(Collection issues) { + private static Input createInput(Collection issues) { return new Input() { @Override public LineHashSequence getLineHashSequence() { diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TargetBranchComponentUuidsTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TargetBranchComponentUuidsTest.java new file mode 100644 index 00000000000..23bcc027f1e --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TargetBranchComponentUuidsTest.java @@ -0,0 +1,126 @@ +/* + * SonarQube + * Copyright (C) 2009-2021 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.ce.task.projectanalysis.issue; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; +import org.sonar.ce.task.projectanalysis.analysis.Branch; +import org.sonar.db.DbTester; +import org.sonar.db.component.BranchType; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.component.ComponentTesting; +import org.sonar.db.protobuf.DbProjectBranches; +import org.sonar.server.project.Project; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.sonar.db.component.SnapshotTesting.newAnalysis; + +public class TargetBranchComponentUuidsTest { + private static final String BRANCH_KEY = "branch1"; + private static final String PR_KEY = "pr1"; + + @org.junit.Rule + public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule(); + + @Rule + public DbTester db = DbTester.create(); + + private TargetBranchComponentUuids underTest; + private final Branch branch = mock(Branch.class); + private ComponentDto branch1; + private ComponentDto branch1File; + private ComponentDto pr1File; + + @Before + public void setup() { + underTest = new TargetBranchComponentUuids(analysisMetadataHolder, db.getDbClient()); + Project project = mock(Project.class); + analysisMetadataHolder.setProject(project); + analysisMetadataHolder.setBranch(branch); + + ComponentDto projectDto = db.components().insertPublicProject(); + when(project.getUuid()).thenReturn(projectDto.uuid()); + branch1 = db.components().insertProjectBranch(projectDto, b -> b.setKey(BRANCH_KEY)); + ComponentDto pr1branch = db.components().insertProjectBranch(projectDto, b -> b.setKey(PR_KEY) + .setBranchType(BranchType.PULL_REQUEST) + .setPullRequestData(DbProjectBranches.PullRequestData.newBuilder().setTarget(BRANCH_KEY).build()) + .setMergeBranchUuid(projectDto.getMainBranchProjectUuid())); + branch1File = ComponentTesting.newFileDto(branch1, null, "file").setUuid("branch1File"); + pr1File = ComponentTesting.newFileDto(pr1branch, null, "file").setUuid("file1"); + db.components().insertComponents(branch1File, pr1File); + } + + @Test + public void should_support_db_key_when_looking_for_target_branch_component() { + when(branch.getType()).thenReturn(BranchType.PULL_REQUEST); + when(branch.getName()).thenReturn("prBranch"); + when(branch.getTargetBranchName()).thenReturn(BRANCH_KEY); + + when(branch.getPullRequestKey()).thenReturn(PR_KEY); + db.components().insertSnapshot(newAnalysis(branch1)); + + assertThat(underTest.getTargetBranchComponentUuid(pr1File.getDbKey())).isEqualTo(branch1File.uuid()); + assertThat(underTest.hasTargetBranchAnalysis()).isTrue(); + } + + @Test + public void should_support_key_when_looking_for_target_branch_component() { + when(branch.getType()).thenReturn(BranchType.PULL_REQUEST); + when(branch.getName()).thenReturn("prBranch"); + when(branch.getTargetBranchName()).thenReturn(BRANCH_KEY); + when(branch.getPullRequestKey()).thenReturn(PR_KEY); + db.components().insertSnapshot(newAnalysis(branch1)); + + assertThat(underTest.getTargetBranchComponentUuid(pr1File.getKey())).isEqualTo(branch1File.uuid()); + } + + @Test + public void return_null_if_file_doesnt_exist() { + when(branch.getType()).thenReturn(BranchType.PULL_REQUEST); + when(branch.getName()).thenReturn("prBranch"); + when(branch.getTargetBranchName()).thenReturn(BRANCH_KEY); + when(branch.getPullRequestKey()).thenReturn(PR_KEY); + db.components().insertSnapshot(newAnalysis(branch1)); + + assertThat(underTest.getTargetBranchComponentUuid("doesnt exist")).isNull(); + } + + @Test + public void skip_init_if_not_a_pull_request() { + when(branch.getType()).thenReturn(BranchType.BRANCH); + when(branch.getName()).thenReturn("prBranch"); + when(branch.getTargetBranchName()).thenReturn(BRANCH_KEY); + + assertThat(underTest.getTargetBranchComponentUuid(pr1File.getDbKey())).isNull(); + } + + @Test + public void skip_init_if_no_target_branch_analysis() { + when(branch.getType()).thenReturn(BranchType.PULL_REQUEST); + when(branch.getName()).thenReturn("prBranch"); + when(branch.getTargetBranchName()).thenReturn(BRANCH_KEY); + + assertThat(underTest.getTargetBranchComponentUuid(pr1File.getDbKey())).isNull(); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactoryTest.java new file mode 100644 index 00000000000..3d2367902dc --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactoryTest.java @@ -0,0 +1,111 @@ +/* + * SonarQube + * Copyright (C) 2009-2021 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.ce.task.projectanalysis.issue; + +import java.util.Collections; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.tracking.Input; +import org.sonar.db.DbTester; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.component.ComponentTesting; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class TrackerTargetBranchInputFactoryTest { + private final static String COMPONENT_KEY = "file1"; + private final static String COMPONENT_UUID = "uuid1"; + + @Rule + public DbTester db = DbTester.create(); + + private final ComponentIssuesLoader componentIssuesLoader = mock(ComponentIssuesLoader.class); + private final TargetBranchComponentUuids targetBranchComponentUuids = mock(TargetBranchComponentUuids.class); + private TrackerTargetBranchInputFactory underTest; + + @Before + public void setUp() { + underTest = new TrackerTargetBranchInputFactory(componentIssuesLoader, targetBranchComponentUuids, db.getDbClient()); + } + + @Test + public void gets_issues_and_hashes_in_matching_component() { + DefaultIssue issue1 = new DefaultIssue(); + when(targetBranchComponentUuids.getTargetBranchComponentUuid(COMPONENT_KEY)).thenReturn(COMPONENT_UUID); + when(componentIssuesLoader.loadOpenIssuesWithChanges(COMPONENT_UUID)).thenReturn(Collections.singletonList(issue1)); + ComponentDto fileDto = ComponentTesting.newFileDto(ComponentTesting.newPublicProjectDto()).setUuid(COMPONENT_UUID); + db.fileSources().insertFileSource(fileDto, 3); + + Component component = mock(Component.class); + when(component.getDbKey()).thenReturn(COMPONENT_KEY); + when(component.getType()).thenReturn(Component.Type.FILE); + Input input = underTest.createForTargetBranch(component); + + assertThat(input.getIssues()).containsOnly(issue1); + assertThat(input.getLineHashSequence().length()).isEqualTo(3); + } + + @Test + public void get_issues_without_line_hashes() { + DefaultIssue issue1 = new DefaultIssue(); + when(targetBranchComponentUuids.getTargetBranchComponentUuid(COMPONENT_KEY)).thenReturn(COMPONENT_UUID); + when(componentIssuesLoader.loadOpenIssuesWithChanges(COMPONENT_UUID)).thenReturn(Collections.singletonList(issue1)); + ComponentDto fileDto = ComponentTesting.newFileDto(ComponentTesting.newPublicProjectDto()).setUuid(COMPONENT_UUID); + db.fileSources().insertFileSource(fileDto, 0); + + Component component = mock(Component.class); + when(component.getDbKey()).thenReturn(COMPONENT_KEY); + when(component.getType()).thenReturn(Component.Type.FILE); + Input input = underTest.createForTargetBranch(component); + + assertThat(input.getIssues()).containsOnly(issue1); + assertThat(input.getLineHashSequence().length()).isZero(); + } + + @Test + public void gets_nothing_when_there_is_no_matching_component() { + Component component = mock(Component.class); + when(component.getDbKey()).thenReturn(COMPONENT_KEY); + when(component.getType()).thenReturn(Component.Type.FILE); + Input input = underTest.createForTargetBranch(component); + + assertThat(input.getIssues()).isEmpty(); + assertThat(input.getLineHashSequence().length()).isZero(); + } + + @Test + public void hasTargetBranchAnalysis_returns_true_if_source_branch_of_pr_was_analysed() { + when(targetBranchComponentUuids.hasTargetBranchAnalysis()).thenReturn(true); + + assertThat(underTest.hasTargetBranchAnalysis()).isTrue(); + } + + @Test + public void hasTargetBranchAnalysis_returns_false_if_no_target_branch_analysis() { + when(targetBranchComponentUuids.hasTargetBranchAnalysis()).thenReturn(false); + + assertThat(underTest.hasTargetBranchAnalysis()).isFalse(); + } +} 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 de82e76dd47..429017f1f05 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 @@ -76,9 +76,7 @@ public class AbstractTracker { } LineAndLineHashKey that = (LineAndLineHashKey) o; // start with most discriminant field - return Objects.equals(line, that.line) - && lineHash.equals(that.lineHash) - && ruleKey.equals(that.ruleKey); + return Objects.equals(line, that.line) && lineHash.equals(that.lineHash) && ruleKey.equals(that.ruleKey); } @Override @@ -110,10 +108,7 @@ public class AbstractTracker { } LineAndLineHashAndMessage that = (LineAndLineHashAndMessage) o; // start with most discriminant field - return Objects.equals(line, that.line) - && lineHash.equals(that.lineHash) - && message.equals(that.message) - && ruleKey.equals(that.ruleKey); + return Objects.equals(line, that.line) && lineHash.equals(that.lineHash) && message.equals(that.message) && ruleKey.equals(that.ruleKey); } @Override @@ -143,9 +138,7 @@ public class AbstractTracker { } LineHashAndMessageKey that = (LineHashAndMessageKey) o; // start with most discriminant field - return lineHash.equals(that.lineHash) - && message.equals(that.message) - && ruleKey.equals(that.ruleKey); + return lineHash.equals(that.lineHash) && message.equals(that.message) && ruleKey.equals(that.ruleKey); } @Override @@ -175,9 +168,7 @@ public class AbstractTracker { } LineAndMessageKey that = (LineAndMessageKey) o; // start with most discriminant field - return Objects.equals(line, that.line) - && message.equals(that.message) - && ruleKey.equals(that.ruleKey); + return Objects.equals(line, that.line) && message.equals(that.message) && ruleKey.equals(that.ruleKey); } @Override @@ -205,8 +196,7 @@ public class AbstractTracker { } LineHashKey that = (LineHashKey) o; // start with most discriminant field - return lineHash.equals(that.lineHash) - && ruleKey.equals(that.ruleKey); + return lineHash.equals(that.lineHash) && ruleKey.equals(that.ruleKey); } @Override 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 a0cd8ea1ce8..c876377bd91 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 @@ -22,8 +22,8 @@ package org.sonar.core.issue.tracking; import java.util.Collection; import java.util.List; import java.util.stream.Stream; -import org.sonar.api.scanner.ScannerSide; import org.sonar.api.issue.Issue; +import org.sonar.api.scanner.ScannerSide; import static org.sonar.core.util.stream.MoreCollectors.toList; @@ -59,7 +59,7 @@ public class Tracker extends Abst ClosedTracking closedTracking = ClosedTracking.of(nonClosedTracking, baseInput); match(closedTracking, LineAndLineHashAndMessage::new); - return new MergedTracking<>(nonClosedTracking, closedTracking); + return mergedTracking(nonClosedTracking, closedTracking); } private void detectCodeMoves(Input rawInput, Input baseInput, Tracking tracking) { @@ -86,20 +86,17 @@ public class Tracker extends Abst } } - private static class MergedTracking extends Tracking { - private MergedTracking(NonClosedTracking nonClosedTracking, ClosedTracking closedTracking) { - super( - nonClosedTracking.getRawInput().getIssues(), - concatIssues(nonClosedTracking, closedTracking), - closedTracking.rawToBase, closedTracking.baseToRaw); - } + private Tracking mergedTracking(NonClosedTracking nonClosedTracking, ClosedTracking closedTracking) { + return new Tracking<>(nonClosedTracking.getRawInput().getIssues(), + concatIssues(nonClosedTracking, closedTracking), + closedTracking.rawToBase, closedTracking.baseToRaw); + } - private static List concatIssues( - NonClosedTracking nonClosedTracking, ClosedTracking closedTracking) { - Collection nonClosedIssues = nonClosedTracking.getBaseInput().getIssues(); - Collection closeIssues = closedTracking.getBaseInput().getIssues(); - return Stream.concat(nonClosedIssues.stream(), closeIssues.stream()) - .collect(toList(nonClosedIssues.size() + closeIssues.size())); - } + private static List concatIssues( + NonClosedTracking nonClosedTracking, ClosedTracking closedTracking) { + Collection nonClosedIssues = nonClosedTracking.getBaseInput().getIssues(); + Collection closeIssues = closedTracking.getBaseInput().getIssues(); + return Stream.concat(nonClosedIssues.stream(), closeIssues.stream()) + .collect(toList(nonClosedIssues.size() + closeIssues.size())); } } -- 2.39.5