From 694f338829f274917811f490c3b300e71ce8fe77 Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Tue, 5 Feb 2019 20:22:02 +0100 Subject: [PATCH] SONAR-11677 Simple issue reporting on PR/SLB with no reference analysis Only keep issues having at least a location on a changed line --- .../component/MergeBranchComponentUuids.java | 10 ++- ...ProjectAnalysisTaskContainerPopulator.java | 4 +- .../FormulaExecutorComponentVisitor.java | 16 ++-- .../issue/IssueTrackingDelegator.java | 10 +-- ...rtBranchOrPullRequestTrackerExecution.java | 87 +++++++++++++++++++ .../issue/ShortBranchTrackerExecution.java | 57 ------------ .../issue/TrackerMergeBranchInputFactory.java | 4 + .../source/NewLinesRepository.java | 16 ++-- .../step/BuildComponentTreeStep.java | 34 +++----- .../step/NewCoverageMeasuresStep.java | 7 +- .../step/NewSizeMeasuresStep.java | 3 + .../issue/IntegrateIssuesVisitorTest.java | 6 +- .../issue/IssueTrackingDelegatorTest.java | 2 +- ...nchOrPullRequestTrackerExecutionTest.java} | 68 +++++++++++++-- 14 files changed, 209 insertions(+), 115 deletions(-) create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchOrPullRequestTrackerExecution.java delete mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchTrackerExecution.java rename server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/{ShortBranchTrackerExecutionTest.java => ShortBranchOrPullRequestTrackerExecutionTest.java} (54%) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/MergeBranchComponentUuids.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/MergeBranchComponentUuids.java index 228cb651f88..84d5d066f0f 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/MergeBranchComponentUuids.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/MergeBranchComponentUuids.java @@ -24,11 +24,11 @@ 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 org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import static com.google.common.base.Preconditions.checkState; import static org.sonar.db.component.ComponentDto.removeBranchAndPullRequestFromKey; @@ -41,6 +41,7 @@ public class MergeBranchComponentUuids { private final DbClient dbClient; private Map uuidsByKey; private String mergeBranchName; + private boolean hasMergeBranchAnalysis; public MergeBranchComponentUuids(AnalysisMetadataHolder analysisMetadataHolder, DbClient dbClient) { this.analysisMetadataHolder = analysisMetadataHolder; @@ -62,10 +63,17 @@ public class MergeBranchComponentUuids { Optional opt = dbClient.branchDao().selectByUuid(dbSession, mergeBranchUuid); checkState(opt.isPresent(), "Merge branch '%s' does not exist", mergeBranchUuid); mergeBranchName = opt.get().getKey(); + + hasMergeBranchAnalysis = dbClient.snapshotDao().selectLastAnalysisByRootComponentUuid(dbSession, mergeBranchUuid).isPresent(); } } } + public boolean hasMergeBranchAnalysis() { + lazyInit(); + return hasMergeBranchAnalysis; + } + public String getMergeBranchName() { lazyInit(); return mergeBranchName; 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 3d67ddb385f..5013704bd6d 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 @@ -80,7 +80,7 @@ import org.sonar.ce.task.projectanalysis.issue.ScmAccountToUser; import org.sonar.ce.task.projectanalysis.issue.ScmAccountToUserLoader; import org.sonar.ce.task.projectanalysis.issue.ShortBranchIssueMerger; import org.sonar.ce.task.projectanalysis.issue.ShortBranchIssuesLoader; -import org.sonar.ce.task.projectanalysis.issue.ShortBranchTrackerExecution; +import org.sonar.ce.task.projectanalysis.issue.ShortBranchOrPullRequestTrackerExecution; import org.sonar.ce.task.projectanalysis.issue.TrackerBaseInputFactory; import org.sonar.ce.task.projectanalysis.issue.TrackerExecution; import org.sonar.ce.task.projectanalysis.issue.TrackerMergeBranchInputFactory; @@ -276,7 +276,7 @@ public final class ProjectAnalysisTaskContainerPopulator implements ContainerPop ClosedIssuesInputFactory.class, Tracker.class, TrackerExecution.class, - ShortBranchTrackerExecution.class, + ShortBranchOrPullRequestTrackerExecution.class, MergeBranchTrackerExecution.class, ComponentIssuesLoader.class, BaseIssuesLoader.class, diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/formula/FormulaExecutorComponentVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/formula/FormulaExecutorComponentVisitor.java index 1580d0d7a03..7c6ba352e59 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/formula/FormulaExecutorComponentVisitor.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/formula/FormulaExecutorComponentVisitor.java @@ -141,13 +141,13 @@ public class FormulaExecutorComponentVisitor extends PathAwareVisitorAdapter path) { - CounterInitializationContext counterContext = new CounterInitializationContextImpl(file); + private void processLeaf(Component component, Path path) { + CounterInitializationContext counterContext = new CounterInitializationContextImpl(component); for (Formula formula : formulas) { Counter counter = formula.createNewCounter(); counter.initialize(counterContext); for (String metricKey : formula.getOutputMetricKeys()) { - addNewMeasure(file, metricKey, formula, counter); + addNewMeasure(component, metricKey, formula, counter); } aggregateToParent(path, formula, counter); } @@ -172,20 +172,20 @@ public class FormulaExecutorComponentVisitor extends PathAwareVisitorAdapter getMeasure(String metricKey) { - return measureRepository.getRawMeasure(file, metricRepository.getByKey(metricKey)); + return measureRepository.getRawMeasure(component, metricRepository.getByKey(metricKey)); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueTrackingDelegator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueTrackingDelegator.java index bab0696905b..fcde1d5941b 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueTrackingDelegator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueTrackingDelegator.java @@ -30,14 +30,14 @@ import org.sonar.db.component.BranchType; import static java.util.Collections.emptyMap; public class IssueTrackingDelegator { - private final ShortBranchTrackerExecution shortBranchTracker; + private final ShortBranchOrPullRequestTrackerExecution shortBranchOrPullRequestTracker; private final TrackerExecution tracker; private final AnalysisMetadataHolder analysisMetadataHolder; private final MergeBranchTrackerExecution mergeBranchTracker; - public IssueTrackingDelegator(ShortBranchTrackerExecution shortBranchTracker, MergeBranchTrackerExecution longBranchTracker, - TrackerExecution tracker, AnalysisMetadataHolder analysisMetadataHolder) { - this.shortBranchTracker = shortBranchTracker; + public IssueTrackingDelegator(ShortBranchOrPullRequestTrackerExecution shortBranchOrPullRequestTracker, MergeBranchTrackerExecution longBranchTracker, + TrackerExecution tracker, AnalysisMetadataHolder analysisMetadataHolder) { + this.shortBranchOrPullRequestTracker = shortBranchOrPullRequestTracker; this.mergeBranchTracker = longBranchTracker; this.tracker = tracker; this.analysisMetadataHolder = analysisMetadataHolder; @@ -45,7 +45,7 @@ public class IssueTrackingDelegator { public TrackingResult track(Component component) { if (analysisMetadataHolder.isShortLivingBranch() || analysisMetadataHolder.isPullRequest()) { - return standardResult(shortBranchTracker.track(component)); + return standardResult(shortBranchOrPullRequestTracker.track(component)); } else if (isFirstAnalysisSecondaryLongLivingBranch()) { Tracking tracking = mergeBranchTracker.track(component); return new TrackingResult(tracking.getMatchedRaws(), emptyMap(), Stream.empty(), tracking.getUnmatchedRaws()); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchOrPullRequestTrackerExecution.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchOrPullRequestTrackerExecution.java new file mode 100644 index 00000000000..3bc412a5c5c --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchOrPullRequestTrackerExecution.java @@ -0,0 +1,87 @@ +/* + * SonarQube + * Copyright (C) 2009-2019 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.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.source.NewLinesRepository; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.tracking.Input; +import org.sonar.core.issue.tracking.Tracker; +import org.sonar.core.issue.tracking.Tracking; +import org.sonar.core.util.stream.MoreCollectors; + +public class ShortBranchOrPullRequestTrackerExecution { + private final TrackerBaseInputFactory baseInputFactory; + private final TrackerRawInputFactory rawInputFactory; + private final TrackerMergeBranchInputFactory mergeInputFactory; + private final Tracker tracker; + private final NewLinesRepository newLinesRepository; + + public ShortBranchOrPullRequestTrackerExecution(TrackerBaseInputFactory baseInputFactory, TrackerRawInputFactory rawInputFactory, + TrackerMergeBranchInputFactory mergeInputFactory, + Tracker tracker, NewLinesRepository newLinesRepository) { + this.baseInputFactory = baseInputFactory; + this.rawInputFactory = rawInputFactory; + this.mergeInputFactory = mergeInputFactory; + this.tracker = tracker; + this.newLinesRepository = newLinesRepository; + } + + public Tracking track(Component component) { + Input rawInput = rawInputFactory.create(component); + Input baseInput = baseInputFactory.create(component); + + Input unmatchedRawInput; + if (mergeInputFactory.hasMergeBranchAnalysis()) { + Input mergeInput = mergeInputFactory.create(component); + Tracking mergeTracking = tracker.trackNonClosed(rawInput, mergeInput); + List unmatchedRaws = mergeTracking.getUnmatchedRaws().collect(MoreCollectors.toList()); + unmatchedRawInput = new DefaultTrackingInput(unmatchedRaws, rawInput.getLineHashSequence(), rawInput.getBlockHashSequence()); + } else { + List filteredRaws = keepIssuesHavingAtLeastOneLocationOnChangedLines(component, rawInput.getIssues()); + unmatchedRawInput = new DefaultTrackingInput(filteredRaws, rawInput.getLineHashSequence(), rawInput.getBlockHashSequence()); + } + + // do second tracking with base branch using raws issues that are still unmatched + return tracker.trackNonClosed(unmatchedRawInput, baseInput); + } + + private List keepIssuesHavingAtLeastOneLocationOnChangedLines(Component component, Collection issues) { + if (component.getType() != Component.Type.FILE) { + return Collections.emptyList(); + } + final Optional> newLinesOpt = newLinesRepository.getNewLines(component); + if (!newLinesOpt.isPresent()) { + return Collections.emptyList(); + } + final Set newLines = newLinesOpt.get(); + return issues + .stream() + .filter(i -> IssueLocations.allLinesFor(i, component.getUuid()).anyMatch(newLines::contains)) + .collect(Collectors.toList()); + } + +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchTrackerExecution.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchTrackerExecution.java deleted file mode 100644 index 6599ca2fb77..00000000000 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchTrackerExecution.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2019 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.List; -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.Tracker; -import org.sonar.core.issue.tracking.Tracking; -import org.sonar.core.util.stream.MoreCollectors; - -public class ShortBranchTrackerExecution { - private final TrackerBaseInputFactory baseInputFactory; - private final TrackerRawInputFactory rawInputFactory; - private final TrackerMergeBranchInputFactory mergeInputFactory; - private final Tracker tracker; - - public ShortBranchTrackerExecution(TrackerBaseInputFactory baseInputFactory, TrackerRawInputFactory rawInputFactory, TrackerMergeBranchInputFactory mergeInputFactory, - Tracker tracker) { - this.baseInputFactory = baseInputFactory; - this.rawInputFactory = rawInputFactory; - this.mergeInputFactory = mergeInputFactory; - this.tracker = tracker; - } - - public Tracking track(Component component) { - Input rawInput = rawInputFactory.create(component); - Input baseInput = baseInputFactory.create(component); - Input mergeInput = mergeInputFactory.create(component); - - Tracking mergeTracking = tracker.trackNonClosed(rawInput, mergeInput); - List unmatchedRaws = mergeTracking.getUnmatchedRaws().collect(MoreCollectors.toList()); - Input unmatchedRawInput = new DefaultTrackingInput(unmatchedRaws, rawInput.getLineHashSequence(), rawInput.getBlockHashSequence()); - - // do second tracking with base branch using raws issues that are still unmatched - return tracker.trackNonClosed(unmatchedRawInput, baseInput); - } - -} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerMergeBranchInputFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerMergeBranchInputFactory.java index 37af0b88257..8690f1bd8dd 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerMergeBranchInputFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerMergeBranchInputFactory.java @@ -45,6 +45,10 @@ public class TrackerMergeBranchInputFactory { // TODO detect file moves? } + public boolean hasMergeBranchAnalysis() { + return mergeBranchComponentUuids.hasMergeBranchAnalysis(); + } + public Input create(Component component) { String mergeBranchComponentUuid = mergeBranchComponentUuids.getUuid(component.getDbKey()); return new MergeLazyInput(component.getType(), mergeBranchComponentUuid); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/NewLinesRepository.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/NewLinesRepository.java index 8ec485f5f4f..c59b4ee12a2 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/NewLinesRepository.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/NewLinesRepository.java @@ -19,6 +19,7 @@ */ package org.sonar.ce.task.projectanalysis.source; +import com.google.common.base.Preconditions; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -51,12 +52,13 @@ public class NewLinesRepository { return isPullRequestOrShortLivedBranch() || periodHolder.hasPeriod(); } - public Optional> getNewLines(Component component) { - Optional> reportChangedLines = getChangedLinesFromReport(component); + public Optional> getNewLines(Component file) { + Preconditions.checkArgument(file.getType() == Component.Type.FILE, "Changed lines are only available on files, but was: " + file.getType().name()); + Optional> reportChangedLines = getChangedLinesFromReport(file); if (reportChangedLines.isPresent()) { return reportChangedLines; } - return computeNewLinesFromScm(component); + return computeNewLinesFromScm(file); } /** @@ -93,9 +95,9 @@ public class NewLinesRepository { return lineDate > period.getSnapshotDate(); } - private Optional> getChangedLinesFromReport(Component component) { + private Optional> getChangedLinesFromReport(Component file) { if (isPullRequestOrShortLivedBranch()) { - return reportChangedLinesCache.computeIfAbsent(component, this::readFromReport); + return reportChangedLinesCache.computeIfAbsent(file, this::readFromReport); } return Optional.empty(); @@ -105,8 +107,8 @@ public class NewLinesRepository { return analysisMetadataHolder.isPullRequest() || analysisMetadataHolder.isShortLivingBranch(); } - private Optional> readFromReport(Component component) { - return reportReader.readComponentChangedLines(component.getReportAttributes().getRef()) + private Optional> readFromReport(Component file) { + return reportReader.readComponentChangedLines(file.getReportAttributes().getRef()) .map(c -> new HashSet<>(c.getLineList())); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/BuildComponentTreeStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/BuildComponentTreeStep.java index 87412dcf332..09925d5656e 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/BuildComponentTreeStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/BuildComponentTreeStep.java @@ -19,8 +19,8 @@ */ package org.sonar.ce.task.projectanalysis.step; +import java.util.Optional; import java.util.function.Function; -import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.ce.task.projectanalysis.analysis.Analysis; import org.sonar.ce.task.projectanalysis.analysis.Branch; @@ -39,7 +39,6 @@ import org.sonar.ce.task.step.ComputationStep; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.SnapshotDto; -import org.sonar.db.component.SnapshotQuery; import org.sonar.scanner.protocol.output.ScannerReport; import static com.google.common.base.MoreObjects.firstNonNull; @@ -89,14 +88,14 @@ public class BuildComponentTreeStep implements ComputationStep { ComponentUuidFactoryWithMigration componentUuidFactoryWithMigration = new ComponentUuidFactoryWithMigration(dbClient, dbSession, rootKey, pathToKey, reportModulesPath.get()); String rootUuid = componentUuidFactoryWithMigration.getOrCreateForKey(rootKey); - SnapshotDto baseAnalysis = loadBaseAnalysis(dbSession, rootUuid); + Optional baseAnalysis = dbClient.snapshotDao().selectLastAnalysisByRootComponentUuid(dbSession, rootUuid); ComponentTreeBuilder builder = new ComponentTreeBuilder(keyGenerator, publicKeyGenerator, componentUuidFactoryWithMigration::getOrCreateForKey, reportReader::readComponent, analysisMetadataHolder.getProject(), analysisMetadataHolder.getBranch(), - createProjectAttributes(metadata, baseAnalysis), + createProjectAttributes(metadata, baseAnalysis.orElse(null)), issueRelocationToRoot); String relativePathFromScmRoot = metadata.getRelativePathFromScmRoot(); @@ -109,7 +108,7 @@ public class BuildComponentTreeStep implements ComputationStep { treeRootHolder.setRoots(reportTreeRoot, reportTreeRoot); } - analysisMetadataHolder.setBaseAnalysis(toAnalysis(baseAnalysis)); + analysisMetadataHolder.setBaseAnalysis(baseAnalysis.map(BuildComponentTreeStep::toAnalysis).orElse(null)); context.getStatistics().add("components", treeRootHolder.getSize()); } @@ -150,25 +149,12 @@ public class BuildComponentTreeStep implements ComputationStep { return branch; } - @CheckForNull - private SnapshotDto loadBaseAnalysis(DbSession dbSession, String rootUuid) { - return dbClient.snapshotDao().selectAnalysisByQuery( - dbSession, - new SnapshotQuery() - .setComponentUuid(rootUuid) - .setIsLast(true)); - } - - @CheckForNull - private static Analysis toAnalysis(@Nullable SnapshotDto dto) { - if (dto != null) { - return new Analysis.Builder() - .setId(dto.getId()) - .setUuid(dto.getUuid()) - .setCreatedAt(dto.getCreatedAt()) - .build(); - } - return null; + private static Analysis toAnalysis(SnapshotDto dto) { + return new Analysis.Builder() + .setId(dto.getId()) + .setUuid(dto.getUuid()) + .setCreatedAt(dto.getCreatedAt()) + .build(); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/NewCoverageMeasuresStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/NewCoverageMeasuresStep.java index 0e934381179..64bce16d146 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/NewCoverageMeasuresStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/NewCoverageMeasuresStep.java @@ -241,8 +241,11 @@ public class NewCoverageMeasuresStep implements ComputationStep { @Override public void initialize(CounterInitializationContext context) { - Component fileComponent = context.getLeaf(); - Optional> newLinesSet = newLinesRepository.getNewLines(fileComponent); + Component component = context.getLeaf(); + if (component.getType() != Component.Type.FILE) { + return; + } + Optional> newLinesSet = newLinesRepository.getNewLines(component); if (!newLinesSet.isPresent()) { return; } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/NewSizeMeasuresStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/NewSizeMeasuresStep.java index 5161a42c6e2..3961b4342e3 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/NewSizeMeasuresStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/NewSizeMeasuresStep.java @@ -102,6 +102,9 @@ public class NewSizeMeasuresStep implements ComputationStep { @Override public void initialize(CounterInitializationContext context) { Component leaf = context.getLeaf(); + if (leaf.getType() != Component.Type.FILE) { + return; + } Optional> changedLines = newLinesRepository.getNewLines(leaf); if (!changedLines.isPresent()) { return; 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 a1600f1a4c8..1fbad25ab55 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 @@ -47,6 +47,7 @@ import org.sonar.ce.task.projectanalysis.issue.filter.IssueFilter; import org.sonar.ce.task.projectanalysis.qualityprofile.ActiveRulesHolder; import org.sonar.ce.task.projectanalysis.qualityprofile.ActiveRulesHolderRule; import org.sonar.ce.task.projectanalysis.qualityprofile.AlwaysActiveRulesHolderImpl; +import org.sonar.ce.task.projectanalysis.source.NewLinesRepository; import org.sonar.ce.task.projectanalysis.source.SourceLinesHashRepository; import org.sonar.ce.task.projectanalysis.source.SourceLinesRepositoryRule; import org.sonar.core.issue.DefaultIssue; @@ -120,13 +121,14 @@ public class IntegrateIssuesVisitorTest { private MergeBranchComponentUuids mergeBranchComponentUuids = mock(MergeBranchComponentUuids.class); private SourceLinesHashRepository sourceLinesHash = mock(SourceLinesHashRepository.class); private IssueRelocationToRoot issueRelocationToRoot = mock(IssueRelocationToRoot.class); + private NewLinesRepository newLinesRepository = mock(NewLinesRepository.class); private ArgumentCaptor defaultIssueCaptor; private ComponentIssuesLoader issuesLoader = new ComponentIssuesLoader(dbTester.getDbClient(), ruleRepositoryRule, activeRulesHolderRule, new MapSettings().asConfig(), System2.INSTANCE); private IssueTrackingDelegator trackingDelegator; private TrackerExecution tracker; - private ShortBranchTrackerExecution shortBranchTracker; + private ShortBranchOrPullRequestTrackerExecution shortBranchTracker; private MergeBranchTrackerExecution mergeBranchTracker; private ActiveRulesHolder activeRulesHolder = new AlwaysActiveRulesHolderImpl(); private IssueCache issueCache; @@ -147,7 +149,7 @@ public class IntegrateIssuesVisitorTest { TrackerMergeBranchInputFactory mergeInputFactory = new TrackerMergeBranchInputFactory(issuesLoader, mergeBranchComponentsUuids, dbClient); ClosedIssuesInputFactory closedIssuesInputFactory = new ClosedIssuesInputFactory(issuesLoader, dbClient, movedFilesRepository); tracker = new TrackerExecution(baseInputFactory, rawInputFactory, closedIssuesInputFactory, new Tracker<>(), issuesLoader, analysisMetadataHolder); - shortBranchTracker = new ShortBranchTrackerExecution(baseInputFactory, rawInputFactory, mergeInputFactory, new Tracker<>()); + shortBranchTracker = new ShortBranchOrPullRequestTrackerExecution(baseInputFactory, rawInputFactory, mergeInputFactory, new Tracker<>(), newLinesRepository); mergeBranchTracker = new MergeBranchTrackerExecution(rawInputFactory, mergeInputFactory, new Tracker<>()); trackingDelegator = new IssueTrackingDelegator(shortBranchTracker, mergeBranchTracker, tracker, analysisMetadataHolder); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueTrackingDelegatorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueTrackingDelegatorTest.java index 4e4b52b7457..ac60f9964a9 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueTrackingDelegatorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueTrackingDelegatorTest.java @@ -37,7 +37,7 @@ import static org.mockito.Mockito.when; public class IssueTrackingDelegatorTest { @Mock - private ShortBranchTrackerExecution shortBranchTracker; + private ShortBranchOrPullRequestTrackerExecution shortBranchTracker; @Mock private MergeBranchTrackerExecution mergeBranchTracker; @Mock diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchTrackerExecutionTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchOrPullRequestTrackerExecutionTest.java similarity index 54% rename from server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchTrackerExecutionTest.java rename to server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchOrPullRequestTrackerExecutionTest.java index ddbd4f187b1..94fc7b5334b 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchTrackerExecutionTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchOrPullRequestTrackerExecutionTest.java @@ -23,26 +23,31 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Optional; 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.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.source.NewLinesRepository; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.tracking.BlockHashSequence; import org.sonar.core.issue.tracking.Input; import org.sonar.core.issue.tracking.LineHashSequence; import org.sonar.core.issue.tracking.Tracker; import org.sonar.core.issue.tracking.Tracking; +import org.sonar.db.protobuf.DbCommons; +import org.sonar.db.protobuf.DbIssues; import org.sonar.db.rule.RuleTesting; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; import static org.sonar.ce.task.projectanalysis.component.ReportComponent.builder; -public class ShortBranchTrackerExecutionTest { +public class ShortBranchOrPullRequestTrackerExecutionTest { static final String FILE_UUID = "FILE_UUID"; static final String FILE_KEY = "FILE_KEY"; static final int FILE_REF = 2; @@ -58,8 +63,10 @@ public class ShortBranchTrackerExecutionTest { private TrackerBaseInputFactory baseFactory; @Mock private TrackerMergeBranchInputFactory mergeFactory; + @Mock + private NewLinesRepository newLinesRepository; - private ShortBranchTrackerExecution underTest; + private ShortBranchOrPullRequestTrackerExecution underTest; private List rawIssues = new ArrayList<>(); private List baseIssues = new ArrayList<>(); @@ -74,16 +81,64 @@ public class ShortBranchTrackerExecutionTest { when(mergeFactory.create(FILE)).thenReturn(createInput(mergeBranchIssues)); Tracker tracker = new Tracker<>(); - underTest = new ShortBranchTrackerExecution(baseFactory, rawFactory, mergeFactory, tracker); + underTest = new ShortBranchOrPullRequestTrackerExecution(baseFactory, rawFactory, mergeFactory, tracker, newLinesRepository); } @Test - public void simple_tracking() { - rawIssues.add(createIssue(1, RuleTesting.XOO_X1)); + 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()); + 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); + + when(mergeFactory.hasMergeBranchAnalysis()).thenReturn(false); + when(newLinesRepository.getNewLines(FILE)).thenReturn(Optional.of(new HashSet<>(Arrays.asList(1, 3)))); + + Tracking tracking = underTest.track(FILE); + + assertThat(tracking.getUnmatchedBases()).isEmpty(); + assertThat(tracking.getMatchedRaws()).isEmpty(); + assertThat(tracking.getUnmatchedRaws()).containsOnly(issue1); + } + + @Test + public void simple_tracking_keep_also_issues_having_secondary_locations_on_changed_lines() { + final DefaultIssue issueWithSecondaryLocationOnAChangedLine = createIssue(2, RuleTesting.XOO_X1); + issueWithSecondaryLocationOnAChangedLine.setLocations(DbIssues.Locations.newBuilder() + .setTextRange(DbCommons.TextRange.newBuilder().setStartLine(2).setEndLine(3)) + .addFlow(DbIssues.Flow.newBuilder().addLocation(DbIssues.Location.newBuilder() + .setComponentId(FILE.getUuid()) + .setTextRange(DbCommons.TextRange.newBuilder().setStartLine(6).setEndLine(8)).build()).build()) + .build()); + rawIssues.add(issueWithSecondaryLocationOnAChangedLine); + final DefaultIssue issueWithNoLocationsOnChangedLines = createIssue(2, RuleTesting.XOO_X1); + issueWithNoLocationsOnChangedLines.setLocations(DbIssues.Locations.newBuilder() + .setTextRange(DbCommons.TextRange.newBuilder().setStartLine(2).setEndLine(2)) + .addFlow(DbIssues.Flow.newBuilder().addLocation(DbIssues.Location.newBuilder() + .setComponentId(FILE.getUuid()) + .setTextRange(DbCommons.TextRange.newBuilder().setStartLine(11).setEndLine(12)).build()).build()) + .build()); + rawIssues.add(issueWithNoLocationsOnChangedLines); + final DefaultIssue issueWithALocationOnADifferentFile = createIssue(2, RuleTesting.XOO_X1); + issueWithALocationOnADifferentFile.setLocations(DbIssues.Locations.newBuilder() + .setTextRange(DbCommons.TextRange.newBuilder().setStartLine(2).setEndLine(3)) + .addFlow(DbIssues.Flow.newBuilder().addLocation(DbIssues.Location.newBuilder() + .setComponentId("anotherUuid") + .setTextRange(DbCommons.TextRange.newBuilder().setStartLine(6).setEndLine(8)).build()).build()) + .build()); + rawIssues.add(issueWithALocationOnADifferentFile); + + when(mergeFactory.hasMergeBranchAnalysis()).thenReturn(false); + when(newLinesRepository.getNewLines(FILE)).thenReturn(Optional.of(new HashSet<>(Arrays.asList(7, 10)))); + Tracking tracking = underTest.track(FILE); + assertThat(tracking.getUnmatchedBases()).isEmpty(); assertThat(tracking.getMatchedRaws()).isEmpty(); - assertThat(tracking.getUnmatchedRaws()).containsOnly(rawIssues.get(0)); + assertThat(tracking.getUnmatchedRaws()).containsOnly(issueWithSecondaryLocationOnAChangedLine); } @Test @@ -92,6 +147,7 @@ public class ShortBranchTrackerExecutionTest { rawIssues.add(createIssue(2, RuleTesting.XOO_X2)); rawIssues.add(createIssue(3, RuleTesting.XOO_X3)); + when(mergeFactory.hasMergeBranchAnalysis()).thenReturn(true); mergeBranchIssues.add(rawIssues.get(0)); baseIssues.add(rawIssues.get(0)); -- 2.39.5