From 3ec97d1a4368fdd3212d524aacfc73403ed709ce Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Mon, 10 Jan 2022 17:09:39 +0100 Subject: [PATCH] SONAR-14929 New Code using a 'reference branch' doesn't detect changed code with git merge workflow --- .../analysis/AnalysisMetadataHolder.java | 7 - .../analysis/AnalysisMetadataHolderImpl.java | 15 -- .../MutableAnalysisMetadataHolder.java | 5 - ...ProjectAnalysisTaskContainerPopulator.java | 4 + .../projectanalysis/issue/IssueCounter.java | 22 +- .../projectanalysis/issue/IssueLocations.java | 6 +- .../issue/NewEffortAggregator.java | 32 +-- .../issue/NewIssueClassifier.java | 76 ++++++ .../period/NewCodePeriodResolver.java | 4 +- .../NewCodeReferenceBranchComponentUuids.java | 84 +++++++ ...ilityAndSecurityRatingMeasuresVisitor.java | 41 ++-- .../NewSecurityReviewMeasuresVisitor.java | 23 +- .../projectanalysis/scm/ScmInfoDbLoader.java | 2 +- .../source/NewLinesRepository.java | 33 ++- .../source/SourceLinesDiffImpl.java | 14 +- .../LoadReportAnalysisMetadataHolderStep.java | 1 - .../step/PersistAnalysisStep.java | 3 +- .../AnalysisMetadataHolderImplTest.java | 33 +-- .../issue/IssueCounterTest.java | 112 +++------ .../issue/NewEffortAggregatorTest.java | 103 ++++---- .../issue/NewIssueClassifierTest.java | 149 ++++++++++++ ...CodeReferenceBranchComponentUuidsTest.java | 116 +++++++++ ...yAndSecurityRatingMeasuresVisitorTest.java | 126 +++++----- .../NewSecurityReviewMeasuresVisitorTest.java | 225 ++++++++---------- .../source/NewLinesRepositoryTest.java | 17 +- .../source/SourceLinesDiffImplTest.java | 27 ++- .../step/LoadPeriodsStepTest.java | 4 +- ...dReportAnalysisMetadataHolderStepTest.java | 12 - .../analysis/AnalysisMetadataHolderRule.java | 13 - .../MutableAnalysisMetadataHolderRule.java | 11 - .../org/sonar/api/batch/scm/ScmProvider.java | 2 + .../scanner/report/ChangedLinesPublisher.java | 37 ++- .../scanner/report/MetadataPublisher.java | 14 +- ...lier.java => ReferenceBranchSupplier.java} | 32 +-- .../scanner/scan/ProjectScanContainer.java | 4 +- .../org/sonar/scm/git/GitScmProvider.java | 24 -- .../main/java/org/sonar/scm/svn/FindFork.java | 118 --------- .../org/sonar/scm/svn/SvnScmProvider.java | 11 +- .../java/org/sonar/scm/svn/SvnScmSupport.java | 3 +- .../report/ChangedLinesPublisherTest.java | 4 +- .../scanner/report/MetadataPublisherTest.java | 8 +- ....java => ReferenceBranchSupplierTest.java} | 62 ++--- .../org/sonar/scm/git/GitScmProviderTest.java | 79 +----- .../java/org/sonar/scm/svn/FindForkTest.java | 143 ----------- .../org/sonar/scm/svn/SvnScmProviderTest.java | 37 +-- .../src/main/protobuf/scanner_report.proto | 2 +- 46 files changed, 858 insertions(+), 1042 deletions(-) create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewIssueClassifier.java create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/period/NewCodeReferenceBranchComponentUuids.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewIssueClassifierTest.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/period/NewCodeReferenceBranchComponentUuidsTest.java rename sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/{ForkDateSupplier.java => ReferenceBranchSupplier.java} (62%) delete mode 100644 sonar-scanner-engine/src/main/java/org/sonar/scm/svn/FindFork.java rename sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/{ForkDateSupplierTest.java => ReferenceBranchSupplierTest.java} (64%) delete mode 100644 sonar-scanner-engine/src/test/java/org/sonar/scm/svn/FindForkTest.java diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolder.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolder.java index 93e6af53c6f..b55ec2b0da6 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolder.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolder.java @@ -38,13 +38,6 @@ public interface AnalysisMetadataHolder { */ long getAnalysisDate(); - /** - * @throws IllegalStateException if no fork date has been set - */ - @CheckForNull - Long getForkDate(); - - /** * Tell whether the analysisDate has been set. */ diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderImpl.java index 9ed1a0c6527..3979edbc6b9 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderImpl.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderImpl.java @@ -48,7 +48,6 @@ public class AnalysisMetadataHolderImpl implements MutableAnalysisMetadataHolder private final InitializedProperty> qProfilesPerLanguage = new InitializedProperty<>(); private final InitializedProperty> pluginsByKey = new InitializedProperty<>(); private final InitializedProperty scmRevision = new InitializedProperty<>(); - private final InitializedProperty forkDate = new InitializedProperty<>(); private final PlatformEditionProvider editionProvider; @@ -88,20 +87,6 @@ public class AnalysisMetadataHolderImpl implements MutableAnalysisMetadataHolder return analysisDate.isInitialized(); } - @Override - public MutableAnalysisMetadataHolder setForkDate(@Nullable Long date) { - checkState(!forkDate.isInitialized(), "Fork date has already been set"); - this.forkDate.setProperty(date); - return this; - } - - @Override - @CheckForNull - public Long getForkDate() { - checkState(forkDate.isInitialized(), "Fork date has not been set"); - return this.forkDate.getProperty(); - } - @Override public boolean isFirstAnalysis() { return getBaseAnalysis() == null; diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/analysis/MutableAnalysisMetadataHolder.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/analysis/MutableAnalysisMetadataHolder.java index 9469369b718..a9859b7245d 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/analysis/MutableAnalysisMetadataHolder.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/analysis/MutableAnalysisMetadataHolder.java @@ -36,11 +36,6 @@ public interface MutableAnalysisMetadataHolder extends AnalysisMetadataHolder { */ MutableAnalysisMetadataHolder setAnalysisDate(long date); - /** - * @throws IllegalStateException if the fork date has already been set - */ - MutableAnalysisMetadataHolder setForkDate(@Nullable Long date); - /** * @throws IllegalStateException if baseAnalysis has already been set */ 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 89f83b97594..ec409d28df6 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 @@ -33,6 +33,7 @@ import org.sonar.ce.task.projectanalysis.component.BranchLoader; import org.sonar.ce.task.projectanalysis.component.BranchPersisterImpl; import org.sonar.ce.task.projectanalysis.component.ConfigurationRepositoryImpl; import org.sonar.ce.task.projectanalysis.component.DisabledComponentsHolderImpl; +import org.sonar.ce.task.projectanalysis.period.NewCodeReferenceBranchComponentUuids; import org.sonar.ce.task.projectanalysis.component.ProjectPersister; import org.sonar.ce.task.projectanalysis.component.ReferenceBranchComponentUuids; import org.sonar.ce.task.projectanalysis.component.ReportModulesPath; @@ -104,6 +105,7 @@ import org.sonar.ce.task.projectanalysis.measure.MeasureRepositoryImpl; import org.sonar.ce.task.projectanalysis.measure.MeasureToMeasureDto; import org.sonar.ce.task.projectanalysis.metric.MetricModule; import org.sonar.ce.task.projectanalysis.notification.NotificationFactory; +import org.sonar.ce.task.projectanalysis.issue.NewIssueClassifier; import org.sonar.ce.task.projectanalysis.period.NewCodePeriodResolver; import org.sonar.ce.task.projectanalysis.period.PeriodHolderImpl; import org.sonar.ce.task.projectanalysis.qualitygate.EvaluationResultTextConverterImpl; @@ -204,6 +206,7 @@ public final class ProjectAnalysisTaskContainerPopulator implements ContainerPop MutableTaskResultHolderImpl.class, BatchReportReaderImpl.class, ReferenceBranchComponentUuids.class, + NewCodeReferenceBranchComponentUuids.class, SiblingComponentsWithOpenIssues.class, // repositories @@ -237,6 +240,7 @@ public final class ProjectAnalysisTaskContainerPopulator implements ContainerPop DefaultAssignee.class, IssueVisitors.class, IssueLifecycle.class, + NewIssueClassifier.class, ComponentsWithUnprocessedIssues.class, ComponentIssuesRepositoryImpl.class, IssueFilter.class, diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCounter.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCounter.java index 04e232b47b5..24d71cbf8c8 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCounter.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCounter.java @@ -27,14 +27,11 @@ import java.util.HashMap; import java.util.Map; import javax.annotation.Nullable; import org.sonar.api.rules.RuleType; -import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.measure.Measure; import org.sonar.ce.task.projectanalysis.measure.MeasureRepository; import org.sonar.ce.task.projectanalysis.metric.Metric; import org.sonar.ce.task.projectanalysis.metric.MetricRepository; -import org.sonar.ce.task.projectanalysis.period.Period; -import org.sonar.ce.task.projectanalysis.period.PeriodHolder; import org.sonar.core.issue.DefaultIssue; import static org.sonar.api.issue.Issue.RESOLUTION_FALSE_POSITIVE; @@ -116,20 +113,17 @@ public class IssueCounter extends IssueVisitor { .put(SECURITY_HOTSPOT, NEW_SECURITY_HOTSPOTS_KEY) .build(); - private final PeriodHolder periodHolder; - private final AnalysisMetadataHolder analysisMetadataHolder; private final MetricRepository metricRepository; private final MeasureRepository measureRepository; + private final NewIssueClassifier newIssueClassifier; private final Map countersByComponentUuid = new HashMap<>(); private Counters currentCounters; - public IssueCounter(PeriodHolder periodHolder, AnalysisMetadataHolder analysisMetadataHolder, - MetricRepository metricRepository, MeasureRepository measureRepository) { - this.periodHolder = periodHolder; - this.analysisMetadataHolder = analysisMetadataHolder; + public IssueCounter(MetricRepository metricRepository, MeasureRepository measureRepository, NewIssueClassifier newIssueClassifier) { this.metricRepository = metricRepository; this.measureRepository = measureRepository; + this.newIssueClassifier = newIssueClassifier; } @Override @@ -139,7 +133,6 @@ public class IssueCounter extends IssueVisitor { // aggregate children counters for (Component child : component.getChildren()) { - // no need to keep the children in memory. They can be garbage-collected. Counters childCounters = countersByComponentUuid.remove(child.getUuid()); currentCounters.add(childCounters); } @@ -148,13 +141,8 @@ public class IssueCounter extends IssueVisitor { @Override public void onIssue(Component component, DefaultIssue issue) { currentCounters.add(issue); - if (analysisMetadataHolder.isPullRequest()) { + if (newIssueClassifier.isNew(component, issue)) { currentCounters.addOnPeriod(issue); - } else if (periodHolder.hasPeriodDate()) { - Period period = periodHolder.getPeriod(); - if (period.isOnPeriod(issue.creationDate())){ - currentCounters.addOnPeriod(issue); - } } } @@ -196,7 +184,7 @@ public class IssueCounter extends IssueVisitor { } private void addNewMeasures(Component component) { - if (!periodHolder.hasPeriodDate() && !analysisMetadataHolder.isPullRequest()) { + if (!newIssueClassifier.isEnabled()) { return; } double unresolvedVariations = currentCounters.counterForPeriod().unresolved; diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLocations.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLocations.java index 603b87b75c1..5f4ee28f49f 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLocations.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLocations.java @@ -27,7 +27,7 @@ import org.sonar.core.issue.DefaultIssue; import org.sonar.db.protobuf.DbCommons; import org.sonar.db.protobuf.DbIssues; -class IssueLocations { +public class IssueLocations { private IssueLocations() { // do not instantiate @@ -41,7 +41,7 @@ class IssueLocations { * TODO should be a method of DefaultIssue, as soon as it's no * longer in sonar-core and can access sonar-db-dao. */ - public static IntStream allLinesFor(DefaultIssue issue, String componentId) { + public static IntStream allLinesFor(DefaultIssue issue, String componentUuid) { DbIssues.Locations locations = issue.getLocations(); if (locations == null) { return IntStream.empty(); @@ -51,7 +51,7 @@ class IssueLocations { locations.hasTextRange() ? Stream.of(locations.getTextRange()) : Stream.empty(), locations.getFlowList().stream() .flatMap(f -> f.getLocationList().stream()) - .filter(l -> Objects.equals(componentIdOf(issue, l), componentId)) + .filter(l -> Objects.equals(componentIdOf(issue, l), componentUuid)) .map(DbIssues.Location::getTextRange)); return textRanges.flatMapToInt(range -> IntStream.rangeClosed(range.getStartLine(), range.getEndLine())); } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregator.java index 706223e4464..695f768f4f7 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregator.java @@ -22,16 +22,12 @@ package org.sonar.ce.task.projectanalysis.issue; import com.google.common.base.MoreObjects; import java.util.HashMap; import java.util.Map; -import javax.annotation.Nullable; import org.sonar.api.measures.CoreMetrics; -import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.measure.Measure; import org.sonar.ce.task.projectanalysis.measure.MeasureRepository; import org.sonar.ce.task.projectanalysis.metric.Metric; import org.sonar.ce.task.projectanalysis.metric.MetricRepository; -import org.sonar.ce.task.projectanalysis.period.Period; -import org.sonar.ce.task.projectanalysis.period.PeriodHolder; import org.sonar.core.issue.DefaultIssue; import static org.sonar.api.measures.CoreMetrics.NEW_RELIABILITY_REMEDIATION_EFFORT_KEY; @@ -46,25 +42,22 @@ import static org.sonar.api.measures.CoreMetrics.NEW_TECHNICAL_DEBT_KEY; */ public class NewEffortAggregator extends IssueVisitor { private final Map counterByComponentUuid = new HashMap<>(); - private final PeriodHolder periodHolder; - private final AnalysisMetadataHolder analysisMetadataHolder; private final MeasureRepository measureRepository; private final Metric newMaintainabilityEffortMetric; private final Metric newReliabilityEffortMetric; private final Metric newSecurityEffortMetric; + private final NewIssueClassifier newIssueClassifier; private NewEffortCounter counter = null; - public NewEffortAggregator(PeriodHolder periodHolder, AnalysisMetadataHolder analysisMetadataHolder, MetricRepository metricRepository, - MeasureRepository measureRepository) { - this.periodHolder = periodHolder; - this.analysisMetadataHolder = analysisMetadataHolder; + public NewEffortAggregator(MetricRepository metricRepository, MeasureRepository measureRepository, NewIssueClassifier newIssueClassifier) { this.measureRepository = measureRepository; this.newMaintainabilityEffortMetric = metricRepository.getByKey(NEW_TECHNICAL_DEBT_KEY); this.newReliabilityEffortMetric = metricRepository.getByKey(NEW_RELIABILITY_REMEDIATION_EFFORT_KEY); this.newSecurityEffortMetric = metricRepository.getByKey(NEW_SECURITY_REMEDIATION_EFFORT_KEY); + this.newIssueClassifier = newIssueClassifier; } @Override @@ -82,17 +75,13 @@ public class NewEffortAggregator extends IssueVisitor { @Override public void onIssue(Component component, DefaultIssue issue) { if (issue.resolution() == null && issue.effortInMinutes() != null) { - if (analysisMetadataHolder.isPullRequest()) { - counter.add(issue, null); - } else if (periodHolder.hasPeriodDate()) { - counter.add(issue, periodHolder.getPeriod()); - } + counter.add(component, issue); } } @Override public void afterComponent(Component component) { - if (periodHolder.hasPeriodDate() || analysisMetadataHolder.isPullRequest()) { + if (newIssueClassifier.isEnabled()) { computeMeasure(component, newMaintainabilityEffortMetric, counter.maintainabilitySum); computeMeasure(component, newReliabilityEffortMetric, counter.reliabilitySum); computeMeasure(component, newSecurityEffortMetric, counter.securitySum); @@ -105,7 +94,7 @@ public class NewEffortAggregator extends IssueVisitor { measureRepository.add(component, metric, Measure.newMeasureBuilder().setVariation(variation).createNoValue()); } - private static class NewEffortCounter { + private class NewEffortCounter { private final EffortSum maintainabilitySum = new EffortSum(); private final EffortSum reliabilitySum = new EffortSum(); private final EffortSum securitySum = new EffortSum(); @@ -116,8 +105,8 @@ public class NewEffortAggregator extends IssueVisitor { securitySum.add(otherCounter.securitySum); } - void add(DefaultIssue issue, @Nullable Period period) { - long newEffort = calculate(issue, period); + void add(Component component, DefaultIssue issue) { + long newEffort = calculate(component, issue); switch (issue.type()) { case CODE_SMELL: maintainabilitySum.add(newEffort); @@ -136,10 +125,11 @@ public class NewEffortAggregator extends IssueVisitor { } } - long calculate(DefaultIssue issue, @Nullable Period period) { - if (period == null || period.isOnPeriod(issue.creationDate())) { + long calculate(Component component, DefaultIssue issue) { + if (newIssueClassifier.isNew(component, issue)) { return MoreObjects.firstNonNull(issue.effortInMinutes(), 0L); } + return 0L; } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewIssueClassifier.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewIssueClassifier.java new file mode 100644 index 00000000000..3d4506dfae7 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewIssueClassifier.java @@ -0,0 +1,76 @@ +/* + * 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.Optional; +import java.util.Set; +import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; +import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.period.PeriodHolder; +import org.sonar.ce.task.projectanalysis.source.NewLinesRepository; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.db.newcodeperiod.NewCodePeriodType; + +public class NewIssueClassifier { + private final NewLinesRepository newLinesRepository; + private final PeriodHolder periodHolder; + private final AnalysisMetadataHolder analysisMetadataHolder; + + public NewIssueClassifier(NewLinesRepository newLinesRepository, PeriodHolder periodHolder, AnalysisMetadataHolder analysisMetadataHolder) { + this.newLinesRepository = newLinesRepository; + this.periodHolder = periodHolder; + this.analysisMetadataHolder = analysisMetadataHolder; + } + + public boolean isEnabled() { + return analysisMetadataHolder.isPullRequest() || periodHolder.hasPeriodDate() || + (periodHolder.hasPeriod() && periodHolder.getPeriod().getMode().equals(NewCodePeriodType.REFERENCE_BRANCH.name())); + } + + public boolean isNew(Component component, DefaultIssue issue) { + if (analysisMetadataHolder.isPullRequest()) { + return true; + } + + if (periodHolder.hasPeriod()) { + if (periodHolder.hasPeriodDate()) { + return periodHolder.getPeriod().isOnPeriod(issue.creationDate()); + } + + if (periodHolder.getPeriod().getMode().equals(NewCodePeriodType.REFERENCE_BRANCH.name())) { + return hasAtLeastOneLocationOnChangedLines(component, issue); + } + } + return false; + } + + private boolean hasAtLeastOneLocationOnChangedLines(Component component, DefaultIssue issue) { + if (component.getType() != Component.Type.FILE) { + return false; + } + final Optional> newLinesOpt = newLinesRepository.getNewLines(component); + if (newLinesOpt.isEmpty()) { + return false; + } + Set newLines = newLinesOpt.get(); + return IssueLocations.allLinesFor(issue, component.getUuid()).anyMatch(newLines::contains); + } + +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/period/NewCodePeriodResolver.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/period/NewCodePeriodResolver.java index fc35dddb8b3..257c3fd02f2 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/period/NewCodePeriodResolver.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/period/NewCodePeriodResolver.java @@ -85,9 +85,7 @@ public class NewCodePeriodResolver { } private Period resolveByReferenceBranch(String value) { - Long forkDate = analysisMetadataHolder.getForkDate(); - // forkDate can be null if the scanner failed to find it - return newPeriod(NewCodePeriodType.REFERENCE_BRANCH, value, forkDate); + return newPeriod(NewCodePeriodType.REFERENCE_BRANCH, value, null); } private Period resolveBySpecificAnalysis(DbSession dbSession, String rootUuid, String value) { diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/period/NewCodeReferenceBranchComponentUuids.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/period/NewCodeReferenceBranchComponentUuids.java new file mode 100644 index 00000000000..4b9eb9a5c52 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/period/NewCodeReferenceBranchComponentUuids.java @@ -0,0 +1,84 @@ +/* + * 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.period; + +import com.google.common.base.Preconditions; +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 org.sonar.db.newcodeperiod.NewCodePeriodType; + +import static org.sonar.db.component.ComponentDto.removeBranchAndPullRequestFromKey; + +/** + * Cache a map between component keys and uuids in the reference branch + */ +public class NewCodeReferenceBranchComponentUuids { + private final DbClient dbClient; + private final AnalysisMetadataHolder analysisMetadataHolder; + private final PeriodHolder periodHolder; + private Map referenceBranchComponentsUuidsByKey; + + public NewCodeReferenceBranchComponentUuids(AnalysisMetadataHolder analysisMetadataHolder, PeriodHolder periodHolder, DbClient dbClient) { + this.analysisMetadataHolder = analysisMetadataHolder; + this.periodHolder = periodHolder; + this.dbClient = dbClient; + } + + private void lazyInit() { + if (referenceBranchComponentsUuidsByKey == null) { + Preconditions.checkState(periodHolder.hasPeriod() && periodHolder.getPeriod().getMode().equals(NewCodePeriodType.REFERENCE_BRANCH.name())); + referenceBranchComponentsUuidsByKey = new HashMap<>(); + + try (DbSession dbSession = dbClient.openSession(false)) { + String referenceKey = periodHolder.getPeriod().getModeParameter() != null ? periodHolder.getPeriod().getModeParameter() : ""; + Optional opt = dbClient.branchDao().selectByBranchKey(dbSession, analysisMetadataHolder.getProject().getUuid(), referenceKey); + if (opt.isPresent()) { + init(opt.get().getUuid(), dbSession); + } + } + } + } + + private void init(String referenceBranchUuid, DbSession dbSession) { + boolean hasReferenceBranchAnalysis = dbClient.snapshotDao().selectLastAnalysisByRootComponentUuid(dbSession, referenceBranchUuid).isPresent(); + + if (hasReferenceBranchAnalysis) { + List components = dbClient.componentDao().selectByProjectUuid(referenceBranchUuid, dbSession); + for (ComponentDto dto : components) { + referenceBranchComponentsUuidsByKey.put(dto.getKey(), dto.uuid()); + } + } + } + + @CheckForNull + public String getComponentUuid(String dbKey) { + lazyInit(); + String cleanComponentKey = removeBranchAndPullRequestFromKey(dbKey); + return referenceBranchComponentsUuidsByKey.get(cleanComponentKey); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitor.java index 926c7713974..35ee3e3aa2e 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitor.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitor.java @@ -23,15 +23,14 @@ import com.google.common.collect.ImmutableMap; import java.util.Map; import org.sonar.api.ce.measure.Issue; import org.sonar.api.measures.CoreMetrics; -import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.PathAwareVisitorAdapter; import org.sonar.ce.task.projectanalysis.formula.counter.RatingValue; import org.sonar.ce.task.projectanalysis.issue.ComponentIssuesRepository; +import org.sonar.ce.task.projectanalysis.issue.NewIssueClassifier; import org.sonar.ce.task.projectanalysis.measure.MeasureRepository; import org.sonar.ce.task.projectanalysis.metric.Metric; import org.sonar.ce.task.projectanalysis.metric.MetricRepository; -import org.sonar.ce.task.projectanalysis.period.PeriodHolder; import org.sonar.core.issue.DefaultIssue; import org.sonar.server.measure.Rating; @@ -69,23 +68,20 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitor extends PathAwareVis private final MeasureRepository measureRepository; private final ComponentIssuesRepository componentIssuesRepository; - private final PeriodHolder periodHolder; - private final Map metricsByKey; - private final AnalysisMetadataHolder analysisMetadataHolder; + private final NewIssueClassifier newIssueClassifier; public NewReliabilityAndSecurityRatingMeasuresVisitor(MetricRepository metricRepository, MeasureRepository measureRepository, - ComponentIssuesRepository componentIssuesRepository, PeriodHolder periodHolder, AnalysisMetadataHolder analysisMetadataHolder) { - super(LEAVES, POST_ORDER, CounterFactory.INSTANCE); + ComponentIssuesRepository componentIssuesRepository, NewIssueClassifier newIssueClassifier) { + super(LEAVES, POST_ORDER, new CounterFactory(newIssueClassifier)); this.measureRepository = measureRepository; this.componentIssuesRepository = componentIssuesRepository; - this.periodHolder = periodHolder; // Output metrics this.metricsByKey = ImmutableMap.of( NEW_RELIABILITY_RATING_KEY, metricRepository.getByKey(NEW_RELIABILITY_RATING_KEY), NEW_SECURITY_RATING_KEY, metricRepository.getByKey(NEW_SECURITY_RATING_KEY)); - this.analysisMetadataHolder = analysisMetadataHolder; + this.newIssueClassifier = newIssueClassifier; } @Override @@ -104,7 +100,7 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitor extends PathAwareVis } private void computeAndSaveMeasures(Component component, Path path) { - if (!periodHolder.hasPeriodDate() && !analysisMetadataHolder.isPullRequest()) { + if (!newIssueClassifier.isEnabled()) { return; } initRatingsToA(path); @@ -129,7 +125,7 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitor extends PathAwareVis .stream() .filter(issue -> issue.resolution() == null) .filter(issue -> issue.type().equals(BUG) || issue.type().equals(VULNERABILITY)) - .forEach(issue -> path.current().processIssue(issue, analysisMetadataHolder.isPullRequest(), periodHolder)); + .forEach(issue -> path.current().processIssue(issue)); } private static void addToParent(Path path) { @@ -138,21 +134,24 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitor extends PathAwareVis } } - static final class Counter { - private Map newRatingValueByMetric = ImmutableMap.of( + static class Counter { + private final Map newRatingValueByMetric = Map.of( NEW_RELIABILITY_RATING_KEY, new RatingValue(), NEW_SECURITY_RATING_KEY, new RatingValue()); + private final NewIssueClassifier newIssueClassifier; + private final Component component; - private Counter() { - // prevents instantiation + public Counter(NewIssueClassifier newIssueClassifier, Component component) { + this.newIssueClassifier = newIssueClassifier; + this.component = component; } void add(Counter otherCounter) { newRatingValueByMetric.forEach((metric, rating) -> rating.increment(otherCounter.newRatingValueByMetric.get(metric))); } - void processIssue(Issue issue, boolean isPR, PeriodHolder periodHolder) { - if (isPR || periodHolder.getPeriod().isOnPeriod(((DefaultIssue) issue).creationDate())) { + void processIssue(Issue issue) { + if (newIssueClassifier.isNew(component, (DefaultIssue) issue)) { Rating rating = RATING_BY_SEVERITY.get(issue.severity()); if (issue.type().equals(BUG)) { newRatingValueByMetric.get(NEW_RELIABILITY_RATING_KEY).increment(rating); @@ -164,15 +163,15 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitor extends PathAwareVis } private static final class CounterFactory extends SimpleStackElementFactory { - public static final CounterFactory INSTANCE = new CounterFactory(); + private final NewIssueClassifier newIssueClassifier; - private CounterFactory() { - // prevents instantiation + private CounterFactory(NewIssueClassifier newIssueClassifier) { + this.newIssueClassifier = newIssueClassifier; } @Override public Counter createForAny(Component component) { - return new Counter(); + return new Counter(newIssueClassifier, component); } } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitor.java index 3d94f89b97b..0e699c86e7b 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitor.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitor.java @@ -20,7 +20,6 @@ package org.sonar.ce.task.projectanalysis.qualitymodel; import java.util.Optional; -import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.PathAwareVisitorAdapter; import org.sonar.ce.task.projectanalysis.issue.ComponentIssuesRepository; @@ -28,7 +27,7 @@ import org.sonar.ce.task.projectanalysis.measure.Measure; import org.sonar.ce.task.projectanalysis.measure.MeasureRepository; import org.sonar.ce.task.projectanalysis.metric.Metric; import org.sonar.ce.task.projectanalysis.metric.MetricRepository; -import org.sonar.ce.task.projectanalysis.period.PeriodHolder; +import org.sonar.ce.task.projectanalysis.issue.NewIssueClassifier; import static org.sonar.api.measures.CoreMetrics.NEW_SECURITY_HOTSPOTS_REVIEWED_KEY; import static org.sonar.api.measures.CoreMetrics.NEW_SECURITY_HOTSPOTS_REVIEWED_STATUS_KEY; @@ -44,32 +43,31 @@ public class NewSecurityReviewMeasuresVisitor extends PathAwareVisitorAdapter path) { - computeMeasure(project, path); - if (!periodHolder.hasPeriodDate() && !analysisMetadataHolder.isPullRequest()) { + if (!newIssueClassifier.isEnabled()) { return; } + computeMeasure(project, path); + // The following measures are only computed on projects level as they are required to compute the others measures on applications measureRepository.add(project, newSecurityHotspotsReviewedStatusMetric, Measure.newMeasureBuilder().setVariation(path.current().getHotspotsReviewed()).createNoValue()); measureRepository.add(project, newSecurityHotspotsToReviewStatusMetric, Measure.newMeasureBuilder().setVariation(path.current().getHotspotsToReview()).createNoValue()); @@ -86,13 +84,10 @@ public class NewSecurityReviewMeasuresVisitor extends PathAwareVisitorAdapter path) { - if (!periodHolder.hasPeriodDate() && !analysisMetadataHolder.isPullRequest()) { - return; - } componentIssuesRepository.getIssues(component) .stream() .filter(issue -> issue.type().equals(SECURITY_HOTSPOT)) - .filter(issue -> analysisMetadataHolder.isPullRequest() || periodHolder.getPeriod().isOnPeriod(issue.creationDate())) + .filter(issue -> newIssueClassifier.isNew(component, issue)) .forEach(issue -> path.current().processHotspot(issue)); Optional percent = computePercent(path.current().getHotspotsToReview(), path.current().getHotspotsReviewed()); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/scm/ScmInfoDbLoader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/scm/ScmInfoDbLoader.java index 8511a1ec14d..d9cf18167b5 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/scm/ScmInfoDbLoader.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/scm/ScmInfoDbLoader.java @@ -49,7 +49,7 @@ public class ScmInfoDbLoader { public Optional getScmInfo(Component file) { Optional uuid = getFileUUid(file); - if (!uuid.isPresent()) { + if (uuid.isEmpty()) { return Optional.empty(); } 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 5f4cdf13479..1759a5df80d 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 @@ -32,6 +32,7 @@ import org.sonar.ce.task.projectanalysis.period.PeriodHolder; import org.sonar.ce.task.projectanalysis.scm.Changeset; import org.sonar.ce.task.projectanalysis.scm.ScmInfo; import org.sonar.ce.task.projectanalysis.scm.ScmInfoRepository; +import org.sonar.db.newcodeperiod.NewCodePeriodType; public class NewLinesRepository { private final BatchReportReader reportReader; @@ -48,11 +49,14 @@ public class NewLinesRepository { } public boolean newLinesAvailable() { - return analysisMetadataHolder.isPullRequest() || periodHolder.hasPeriodDate(); + return analysisMetadataHolder.isPullRequest() || periodHolder.hasPeriodDate() || isReferenceBranch(); } public Optional> getNewLines(Component file) { Preconditions.checkArgument(file.getType() == Component.Type.FILE, "Changed lines are only available on files, but was: " + file.getType().name()); + if (!newLinesAvailable()) { + return Optional.empty(); + } Optional> reportChangedLines = getChangedLinesFromReport(file); if (reportChangedLines.isPresent()) { return reportChangedLines; @@ -61,17 +65,12 @@ public class NewLinesRepository { } /** - * If the changed lines are not in the report or if we are not analyzing a P/R we fall back to this method. - * If there is a period and SCM information, we compare the change dates of each line with the start of the period to figure out - * if a line is new or not. + * If the changed lines are not in the report or if we are not analyzing a P/R or a branch using a "reference branch", we fall back to this method. + * If there is a period and SCM information, we compare the change dates of each line with the start of the period to figure out if a line is new or not. */ private Optional> computeNewLinesFromScm(Component component) { - if (!periodHolder.hasPeriodDate() && !analysisMetadataHolder.isPullRequest()) { - return Optional.empty(); - } - Optional scmInfoOpt = scmInfoRepository.getScmInfo(component); - if (!scmInfoOpt.isPresent()) { + if (scmInfoOpt.isEmpty()) { return Optional.empty(); } @@ -80,16 +79,20 @@ public class NewLinesRepository { Set lines = new HashSet<>(); // in PRs, we consider changes introduced in this analysis as new, hence subtracting 1. - long referenceDate = analysisMetadataHolder.isPullRequest() ? analysisMetadataHolder.getAnalysisDate() - 1 : periodHolder.getPeriod().getDate(); - for (int i=0; i> getChangedLinesFromReport(Component file) { - if (analysisMetadataHolder.isPullRequest()) { + if (analysisMetadataHolder.isPullRequest() || isReferenceBranch()) { return reportChangedLinesCache.computeIfAbsent(file, this::readFromReport); } return Optional.empty(); } + private boolean isReferenceBranch() { + return periodHolder.hasPeriod() && periodHolder.getPeriod().getMode().equals(NewCodePeriodType.REFERENCE_BRANCH.name()); + } + 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/source/SourceLinesDiffImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/SourceLinesDiffImpl.java index dd116bf3f7a..cac6f05e7cd 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/SourceLinesDiffImpl.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/SourceLinesDiffImpl.java @@ -24,10 +24,13 @@ import java.util.List; import java.util.Optional; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.period.NewCodeReferenceBranchComponentUuids; import org.sonar.ce.task.projectanalysis.component.ReferenceBranchComponentUuids; import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository; +import org.sonar.ce.task.projectanalysis.period.PeriodHolder; import org.sonar.db.DbClient; import org.sonar.db.DbSession; +import org.sonar.db.newcodeperiod.NewCodePeriodType; import org.sonar.db.source.FileSourceDao; public class SourceLinesDiffImpl implements SourceLinesDiff { @@ -38,15 +41,20 @@ public class SourceLinesDiffImpl implements SourceLinesDiff { private final ReferenceBranchComponentUuids referenceBranchComponentUuids; private final MovedFilesRepository movedFilesRepository; private final AnalysisMetadataHolder analysisMetadataHolder; + private final PeriodHolder periodHolder; + private final NewCodeReferenceBranchComponentUuids newCodeReferenceBranchComponentUuids; - public SourceLinesDiffImpl(DbClient dbClient, FileSourceDao fileSourceDao, SourceLinesHashRepository sourceLinesHash, - ReferenceBranchComponentUuids referenceBranchComponentUuids, MovedFilesRepository movedFilesRepository, AnalysisMetadataHolder analysisMetadataHolder) { + public SourceLinesDiffImpl(DbClient dbClient, FileSourceDao fileSourceDao, SourceLinesHashRepository sourceLinesHash, ReferenceBranchComponentUuids referenceBranchComponentUuids, + MovedFilesRepository movedFilesRepository, AnalysisMetadataHolder analysisMetadataHolder, PeriodHolder periodHolder, + NewCodeReferenceBranchComponentUuids newCodeReferenceBranchComponentUuids) { this.dbClient = dbClient; this.fileSourceDao = fileSourceDao; this.sourceLinesHash = sourceLinesHash; this.referenceBranchComponentUuids = referenceBranchComponentUuids; this.movedFilesRepository = movedFilesRepository; this.analysisMetadataHolder = analysisMetadataHolder; + this.periodHolder = periodHolder; + this.newCodeReferenceBranchComponentUuids = newCodeReferenceBranchComponentUuids; } @Override @@ -62,6 +70,8 @@ public class SourceLinesDiffImpl implements SourceLinesDiff { String uuid; if (analysisMetadataHolder.isPullRequest()) { uuid = referenceBranchComponentUuids.getComponentUuid(component.getDbKey()); + } else if (periodHolder.hasPeriod() && periodHolder.getPeriod().getMode().equals(NewCodePeriodType.REFERENCE_BRANCH.name())) { + uuid = newCodeReferenceBranchComponentUuids.getComponentUuid(component.getDbKey()); } else { Optional originalFile = movedFilesRepository.getOriginalFile(component); uuid = originalFile.map(MovedFilesRepository.OriginalFile::getUuid).orElse(component.getUuid()); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/LoadReportAnalysisMetadataHolderStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/LoadReportAnalysisMetadataHolderStep.java index 00e713bfcc7..b1b991bfe01 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/LoadReportAnalysisMetadataHolderStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/LoadReportAnalysisMetadataHolderStep.java @@ -76,7 +76,6 @@ public class LoadReportAnalysisMetadataHolderStep implements ComputationStep { } private void loadMetadata(ScannerReport.Metadata reportMetadata) { - analysisMetadata.setForkDate(reportMetadata.getForkDate() > 0 ? reportMetadata.getForkDate() : null); analysisMetadata.setAnalysisDate(reportMetadata.getAnalysisDate()); analysisMetadata.setRootComponentRef(reportMetadata.getRootComponentRef()); analysisMetadata.setCrossProjectDuplicationEnabled(reportMetadata.getCrossProjectDuplicationActivated()); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistAnalysisStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistAnalysisStep.java index 06bbdd163f5..b39c61da91b 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistAnalysisStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistAnalysisStep.java @@ -46,8 +46,7 @@ public class PersistAnalysisStep implements ComputationStep { private final AnalysisMetadataHolder analysisMetadataHolder; private final PeriodHolder periodHolder; - public PersistAnalysisStep(System2 system2, DbClient dbClient, TreeRootHolder treeRootHolder, - AnalysisMetadataHolder analysisMetadataHolder, PeriodHolder periodHolder) { + public PersistAnalysisStep(System2 system2, DbClient dbClient, TreeRootHolder treeRootHolder, AnalysisMetadataHolder analysisMetadataHolder, PeriodHolder periodHolder) { this.system2 = system2; this.dbClient = dbClient; this.treeRootHolder = treeRootHolder; diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderImplTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderImplTest.java index a08f58076c7..60413e97b12 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderImplTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderImplTest.java @@ -94,31 +94,6 @@ public class AnalysisMetadataHolderImplTest { .hasMessage("Analysis date has already been set"); } - @Test - public void getForkDate_returns_date_with_same_time_as_the_one_set_with_setForkDate() { - - underTest.setForkDate(SOME_DATE); - - assertThat(underTest.getForkDate()).isEqualTo(SOME_DATE); - } - - @Test - public void getForkDate_throws_ISE_when_holder_is_not_initialized() { - assertThatThrownBy(() -> new AnalysisMetadataHolderImpl(editionProvider).getForkDate()) - .isInstanceOf(IllegalStateException.class) - .hasMessage("Fork date has not been set"); - } - - @Test - public void setForkDate_throws_ISE_when_called_twice() { - AnalysisMetadataHolderImpl underTest = new AnalysisMetadataHolderImpl(editionProvider); - underTest.setForkDate(SOME_DATE); - - assertThatThrownBy(() -> underTest.setForkDate(SOME_DATE)) - .isInstanceOf(IllegalStateException.class) - .hasMessage("Fork date has already been set"); - } - @Test public void hasAnalysisDateBeenSet_returns_false_when_holder_is_not_initialized() { assertThat(new AnalysisMetadataHolderImpl(editionProvider).hasAnalysisDateBeenSet()).isFalse(); @@ -273,8 +248,8 @@ public class AnalysisMetadataHolderImplTest { @DataProvider public static Object[][] anyEditionIncludingNone() { return Stream.concat( - Stream.of((Edition) null), - Arrays.stream(Edition.values())) + Stream.of((Edition) null), + Arrays.stream(Edition.values())) .map(t -> new Object[] {t}) .toArray(Object[][]::new); } @@ -282,8 +257,8 @@ public class AnalysisMetadataHolderImplTest { @DataProvider public static Object[][] anyEditionIncludingNoneButCommunity() { return Stream.concat( - Stream.of((Edition) null), - Arrays.stream(Edition.values())).filter(t -> t != Edition.COMMUNITY) + Stream.of((Edition) null), + Arrays.stream(Edition.values())).filter(t -> t != Edition.COMMUNITY) .map(t -> new Object[] {t}) .toArray(Object[][]::new); } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueCounterTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueCounterTest.java index 5a2ee297a3f..f29c120b94b 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueCounterTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueCounterTest.java @@ -20,7 +20,6 @@ package org.sonar.ce.task.projectanalysis.issue; import java.util.Arrays; -import java.util.Date; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -29,8 +28,6 @@ import org.assertj.core.data.MapEntry; import org.junit.Rule; import org.junit.Test; import org.sonar.api.rules.RuleType; -import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; -import org.sonar.ce.task.projectanalysis.analysis.Branch; import org.sonar.ce.task.projectanalysis.batch.BatchReportReaderRule; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.TreeRootHolderRule; @@ -38,14 +35,14 @@ import org.sonar.ce.task.projectanalysis.measure.MeasureRepoEntry; import org.sonar.ce.task.projectanalysis.measure.MeasureRepositoryRule; import org.sonar.ce.task.projectanalysis.metric.MetricRepositoryRule; import org.sonar.ce.task.projectanalysis.period.Period; -import org.sonar.ce.task.projectanalysis.period.PeriodHolderRule; import org.sonar.core.issue.DefaultIssue; -import org.sonar.db.component.BranchType; import org.sonar.db.rule.RuleTesting; import static java.util.Arrays.stream; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FALSE_POSITIVE; @@ -120,12 +117,6 @@ public class IssueCounterTest { @Rule public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule(); - @Rule - public PeriodHolderRule periodsHolder = new PeriodHolderRule(); - - @Rule - public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule(); - @Rule public MetricRepositoryRule metricRepository = new MetricRepositoryRule() .add(VIOLATIONS) @@ -156,13 +147,11 @@ public class IssueCounterTest { @Rule public MeasureRepositoryRule measureRepository = MeasureRepositoryRule.create(treeRootHolder, metricRepository); - - private IssueCounter underTest = new IssueCounter(periodsHolder, analysisMetadataHolder, metricRepository, measureRepository); + private NewIssueClassifier newIssueClassifier = mock(NewIssueClassifier.class); + private IssueCounter underTest = new IssueCounter(metricRepository, measureRepository, newIssueClassifier); @Test public void count_issues_by_status() { - periodsHolder.setPeriod(null); - // bottom-up traversal -> from files to project underTest.beforeComponent(FILE1); underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, BLOCKER)); @@ -191,8 +180,6 @@ public class IssueCounterTest { @Test public void count_issues_by_resolution() { - periodsHolder.setPeriod(null); - // bottom-up traversal -> from files to project underTest.beforeComponent(FILE1); underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, BLOCKER)); @@ -223,8 +210,6 @@ public class IssueCounterTest { @Test public void count_unresolved_issues_by_severity() { - periodsHolder.setPeriod(null); - // bottom-up traversal -> from files to project underTest.beforeComponent(FILE1); underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, BLOCKER)); @@ -249,8 +234,6 @@ public class IssueCounterTest { @Test public void count_unresolved_issues_by_type() { - periodsHolder.setPeriod(null); - // bottom-up traversal -> from files to project // file1 : one open code smell, one closed code smell (which will be excluded from metric) underTest.beforeComponent(FILE1); @@ -279,21 +262,20 @@ public class IssueCounterTest { } @Test - public void count_new_issues_if_period_exists() { - Period period = newPeriod(1500000000000L); - periodsHolder.setPeriod(period); + public void count_new_issues() { + when(newIssueClassifier.isEnabled()).thenReturn(true); underTest.beforeComponent(FILE1); // created before -> existing issues (so ignored) - underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, BLOCKER, period.getDate() - 1000000L).setType(RuleType.CODE_SMELL)); - // created during the first analysis starting the period -> existing issues (so ignored) - underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, BLOCKER, period.getDate()).setType(RuleType.BUG)); + underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, BLOCKER).setType(RuleType.CODE_SMELL)); + underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, BLOCKER).setType(RuleType.BUG)); + // created after -> 4 new issues but 1 is closed - underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, CRITICAL, period.getDate() + 100000L).setType(RuleType.CODE_SMELL)); - underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, CRITICAL, period.getDate() + 100000L).setType(RuleType.BUG)); - underTest.onIssue(FILE1, createIssue(RESOLUTION_FIXED, STATUS_CLOSED, MAJOR, period.getDate() + 200000L).setType(RuleType.BUG)); - underTest.onIssue(FILE1, createSecurityHotspot(period.getDate() + 100000L)); - underTest.onIssue(FILE1, createSecurityHotspot(period.getDate() + 100000L).setResolution(RESOLUTION_WONT_FIX).setStatus(STATUS_CLOSED)); + underTest.onIssue(FILE1, createNewIssue(null, STATUS_OPEN, CRITICAL).setType(RuleType.CODE_SMELL)); + underTest.onIssue(FILE1, createNewIssue(null, STATUS_OPEN, CRITICAL).setType(RuleType.BUG)); + underTest.onIssue(FILE1, createNewIssue(RESOLUTION_FIXED, STATUS_CLOSED, MAJOR).setType(RuleType.BUG)); + underTest.onIssue(FILE1, createNewSecurityHotspot()); + underTest.onIssue(FILE1, createNewSecurityHotspot().setResolution(RESOLUTION_WONT_FIX).setStatus(STATUS_CLOSED)); underTest.afterComponent(FILE1); underTest.beforeComponent(FILE2); @@ -308,37 +290,8 @@ public class IssueCounterTest { entry(NEW_CODE_SMELLS_KEY, 1), entry(NEW_BUGS_KEY, 1), entry(NEW_VULNERABILITIES_KEY, 0), entry(NEW_SECURITY_HOTSPOTS_KEY, 1)); } - @Test - public void count_all_issues_as_new_issues_if_pr() { - periodsHolder.setPeriod(null); - Branch branch = mock(Branch.class); - when(branch.getType()).thenReturn(BranchType.PULL_REQUEST); - analysisMetadataHolder.setBranch(branch); - - underTest.beforeComponent(FILE1); - underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, BLOCKER, 1000000L).setType(RuleType.CODE_SMELL)); - underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, BLOCKER, 0L).setType(RuleType.BUG)); - underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, CRITICAL, 100000L).setType(RuleType.CODE_SMELL)); - underTest.onIssue(FILE1, createIssue(null, STATUS_OPEN, CRITICAL, 100000L).setType(RuleType.BUG)); - underTest.onIssue(FILE1, createIssue(RESOLUTION_FIXED, STATUS_CLOSED, MAJOR, 200000L).setType(RuleType.BUG)); - underTest.afterComponent(FILE1); - - underTest.beforeComponent(FILE2); - underTest.afterComponent(FILE2); - - underTest.beforeComponent(PROJECT); - underTest.afterComponent(PROJECT); - - assertVariations(FILE1, entry(NEW_VIOLATIONS_KEY, 4), entry(NEW_CRITICAL_VIOLATIONS_KEY, 2), entry(NEW_BLOCKER_VIOLATIONS_KEY, 2), entry(NEW_MAJOR_VIOLATIONS_KEY, 0), - entry(NEW_CODE_SMELLS_KEY, 2), entry(NEW_BUGS_KEY, 2), entry(NEW_VULNERABILITIES_KEY, 0)); - assertVariations(PROJECT, entry(NEW_VIOLATIONS_KEY, 4), entry(NEW_CRITICAL_VIOLATIONS_KEY, 2), entry(NEW_BLOCKER_VIOLATIONS_KEY, 2), entry(NEW_MAJOR_VIOLATIONS_KEY, 0), - entry(NEW_CODE_SMELLS_KEY, 2), entry(NEW_BUGS_KEY, 2), entry(NEW_VULNERABILITIES_KEY, 0)); - } - @Test public void exclude_hotspots_from_issue_counts() { - periodsHolder.setPeriod(null); - // bottom-up traversal -> from files to project underTest.beforeComponent(FILE1); underTest.onIssue(FILE1, createSecurityHotspot()); @@ -363,20 +316,18 @@ public class IssueCounterTest { @Test public void exclude_new_hotspots_from_issue_counts() { - Period period = newPeriod(1500000000000L); - periodsHolder.setPeriod(period); + when(newIssueClassifier.isEnabled()).thenReturn(true); underTest.beforeComponent(FILE1); // created before -> existing issues (so ignored) - underTest.onIssue(FILE1, createSecurityHotspot(period.getDate() - 1000000L)); - // created during the first analysis starting the period -> existing issues (so ignored) - underTest.onIssue(FILE1, createSecurityHotspot(period.getDate())); + underTest.onIssue(FILE1, createSecurityHotspot()); + underTest.onIssue(FILE1, createSecurityHotspot()); // created after, but closed - underTest.onIssue(FILE1, createSecurityHotspot(period.getDate() + 100000L).setStatus(STATUS_RESOLVED).setResolution(RESOLUTION_WONT_FIX)); + underTest.onIssue(FILE1, createNewSecurityHotspot().setStatus(STATUS_RESOLVED).setResolution(RESOLUTION_WONT_FIX)); for (String severity : Arrays.asList(CRITICAL, BLOCKER, MAJOR)) { - DefaultIssue issue = createSecurityHotspot(period.getDate() + 100000L); + DefaultIssue issue = createNewSecurityHotspot(); issue.setSeverity(severity); underTest.onIssue(FILE1, issue); } @@ -413,28 +364,33 @@ public class IssueCounterTest { .containsAll(expected); } - private static DefaultIssue createIssue(@Nullable String resolution, String status, String severity) { - return createIssue(resolution, status, severity, RuleType.CODE_SMELL, new Date().getTime()); + private DefaultIssue createNewIssue(@Nullable String resolution, String status, String severity) { + return createNewIssue(resolution, status, severity, RuleType.CODE_SMELL); } - private static DefaultIssue createIssue(@Nullable String resolution, String status, String severity, long creationDate) { - return createIssue(resolution, status, severity, RuleType.CODE_SMELL, creationDate); + private DefaultIssue createNewIssue(@Nullable String resolution, String status, String severity, RuleType ruleType) { + DefaultIssue issue = createIssue(resolution, status, severity, ruleType); + when(newIssueClassifier.isNew(any(), eq(issue))).thenReturn(true); + return issue; + } + + private static DefaultIssue createIssue(@Nullable String resolution, String status, String severity) { + return createIssue(resolution, status, severity, RuleType.CODE_SMELL); } - private static DefaultIssue createIssue(@Nullable String resolution, String status, String severity, RuleType ruleType, long creationDate) { + private static DefaultIssue createIssue(@Nullable String resolution, String status, String severity, RuleType ruleType) { return new DefaultIssue() .setResolution(resolution).setStatus(status) .setSeverity(severity).setRuleKey(RuleTesting.XOO_X1) - .setType(ruleType) - .setCreationDate(new Date(creationDate)); + .setType(ruleType); } private static DefaultIssue createSecurityHotspot() { - return createSecurityHotspot(new Date().getTime()); + return createIssue(null, STATUS_OPEN, "MAJOR", RuleType.SECURITY_HOTSPOT); } - private static DefaultIssue createSecurityHotspot(long creationDate) { - return createIssue(null, STATUS_OPEN, "MAJOR", RuleType.SECURITY_HOTSPOT, creationDate); + private DefaultIssue createNewSecurityHotspot() { + return createNewIssue(null, STATUS_OPEN, "MAJOR", RuleType.SECURITY_HOTSPOT); } private static Period newPeriod(long date) { diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregatorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregatorTest.java index 7af69174a21..0b1ba5a08f6 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregatorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregatorTest.java @@ -19,25 +19,23 @@ */ package org.sonar.ce.task.projectanalysis.issue; -import java.util.Date; -import java.util.Random; import org.junit.Test; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.Duration; -import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; import org.sonar.ce.task.projectanalysis.analysis.Branch; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.ReportComponent; import org.sonar.ce.task.projectanalysis.measure.Measure; import org.sonar.ce.task.projectanalysis.measure.MeasureRepositoryRule; import org.sonar.ce.task.projectanalysis.metric.MetricRepositoryRule; -import org.sonar.ce.task.projectanalysis.period.Period; import org.sonar.ce.task.projectanalysis.period.PeriodHolderRule; import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.util.UuidFactoryFast; import org.sonar.db.component.BranchType; -import org.sonar.db.newcodeperiod.NewCodePeriodType; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; @@ -52,14 +50,6 @@ import static org.sonar.api.rules.RuleType.CODE_SMELL; import static org.sonar.api.rules.RuleType.VULNERABILITY; public class NewEffortAggregatorTest { - - private static final Period PERIOD = new Period(NewCodePeriodType.PREVIOUS_VERSION.name(), null, 1_500_000_000L); - private static final long[] OLD_ISSUES_DATES = new long[]{ - PERIOD.getDate(), - PERIOD.getDate() - 1, - PERIOD.getDate() - 1_200_000L, - }; - private static final Component FILE = ReportComponent.builder(Component.Type.FILE, 1).setUuid("FILE").build(); private static final Component PROJECT = ReportComponent.builder(Component.Type.PROJECT, 2).addChildren(FILE).build(); @@ -72,15 +62,13 @@ public class NewEffortAggregatorTest { .add(NEW_SECURITY_REMEDIATION_EFFORT); @org.junit.Rule public MeasureRepositoryRule measureRepository = MeasureRepositoryRule.create(); - @org.junit.Rule - public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule(); - - private final Random random = new Random(); - private NewEffortAggregator underTest = new NewEffortAggregator(periodsHolder, analysisMetadataHolder, metricRepository, measureRepository); + private final NewIssueClassifier newIssueClassifier = mock(NewIssueClassifier.class); + private final NewEffortAggregator underTest = new NewEffortAggregator(metricRepository, measureRepository, newIssueClassifier); @Test public void sum_new_maintainability_effort_of_issues() { - periodsHolder.setPeriod(PERIOD); + when(newIssueClassifier.isEnabled()).thenReturn(true); + when(newIssueClassifier.isNew(any(), any())).thenReturn(true); DefaultIssue unresolved1 = newCodeSmellIssue(10L); DefaultIssue old1 = oldCodeSmellIssue(100L); DefaultIssue unresolved2 = newCodeSmellIssue(30L); @@ -102,7 +90,8 @@ public class NewEffortAggregatorTest { @Test public void new_maintainability_effort_is_only_computed_using_code_smell_issues() { - periodsHolder.setPeriod(PERIOD); + when(newIssueClassifier.isEnabled()).thenReturn(true); + when(newIssueClassifier.isNew(any(), any())).thenReturn(true); DefaultIssue codeSmellIssue = newCodeSmellIssue(10); DefaultIssue oldSmellIssue = oldCodeSmellIssue(100); // Issues of type BUG and VULNERABILITY should be ignored @@ -126,7 +115,8 @@ public class NewEffortAggregatorTest { @Test public void sum_new_reliability_effort_of_issues() { - periodsHolder.setPeriod(PERIOD); + when(newIssueClassifier.isEnabled()).thenReturn(true); + when(newIssueClassifier.isNew(any(), any())).thenReturn(true); DefaultIssue unresolved1 = newBugIssue(10L); DefaultIssue old1 = oldBugIssue(100L); DefaultIssue unresolved2 = newBugIssue(30L); @@ -149,7 +139,8 @@ public class NewEffortAggregatorTest { @Test public void new_reliability_effort_is_only_computed_using_bug_issues() { - periodsHolder.setPeriod(PERIOD); + when(newIssueClassifier.isEnabled()).thenReturn(true); + when(newIssueClassifier.isNew(any(), any())).thenReturn(true); DefaultIssue bugIssue = newBugIssue(15); DefaultIssue oldBugIssue = oldBugIssue(150); // Issues of type CODE SMELL and VULNERABILITY should be ignored @@ -173,7 +164,7 @@ public class NewEffortAggregatorTest { @Test public void sum_new_vulnerability_effort_of_issues() { - periodsHolder.setPeriod(PERIOD); + when(newIssueClassifier.isEnabled()).thenReturn(true); DefaultIssue unresolved1 = newVulnerabilityIssue(10L); DefaultIssue old1 = oldVulnerabilityIssue(100L); DefaultIssue unresolved2 = newVulnerabilityIssue(30L); @@ -197,7 +188,8 @@ public class NewEffortAggregatorTest { @Test public void new_security_effort_is_only_computed_using_vulnerability_issues() { - periodsHolder.setPeriod(PERIOD); + when(newIssueClassifier.isEnabled()).thenReturn(true); + when(newIssueClassifier.isNew(any(), any())).thenReturn(true); DefaultIssue vulnerabilityIssue = newVulnerabilityIssue(12); DefaultIssue oldVulnerabilityIssue = oldVulnerabilityIssue(120); // Issues of type CODE SMELL and BUG should be ignored @@ -221,7 +213,8 @@ public class NewEffortAggregatorTest { @Test public void aggregate_new_characteristic_measures_of_children() { - periodsHolder.setPeriod(PERIOD); + when(newIssueClassifier.isEnabled()).thenReturn(true); + when(newIssueClassifier.isNew(any(), any())).thenReturn(true); DefaultIssue codeSmellIssue = newCodeSmellIssue(10); DefaultIssue oldCodeSmellIssue = oldCodeSmellIssue(100); @@ -261,9 +254,9 @@ public class NewEffortAggregatorTest { @Test public void no_measures_if_no_periods() { + when(newIssueClassifier.isEnabled()).thenReturn(false); Branch branch = mock(Branch.class); when(branch.getType()).thenReturn(BranchType.BRANCH); - analysisMetadataHolder.setBranch(branch); periodsHolder.setPeriod(null); DefaultIssue unresolved = newCodeSmellIssue(10); @@ -276,7 +269,8 @@ public class NewEffortAggregatorTest { @Test public void should_have_empty_measures_if_no_issues() { - periodsHolder.setPeriod(PERIOD); + when(newIssueClassifier.isEnabled()).thenReturn(true); + when(newIssueClassifier.isNew(any(), any())).thenReturn(true); underTest.beforeComponent(FILE); underTest.afterComponent(FILE); @@ -292,52 +286,45 @@ public class NewEffortAggregatorTest { assertThat(newMeasure.getValueType()).isEqualTo(Measure.ValueType.NO_VALUE); } - private static DefaultIssue newCodeSmellIssue(long effort) { - return newCodeSmellIssueWithoutEffort() - .setEffort(Duration.create(effort)) - .setType(RuleType.CODE_SMELL) - .setCreationDate(new Date(PERIOD.getDate() + 10_000L)); + private DefaultIssue newCodeSmellIssue(long effort) { + return createIssue(CODE_SMELL, effort, true); } private DefaultIssue oldCodeSmellIssue(long effort) { - return newCodeSmellIssueWithoutEffort() - .setEffort(Duration.create(effort)) - .setType(RuleType.CODE_SMELL) - .setCreationDate(new Date(OLD_ISSUES_DATES[random.nextInt(OLD_ISSUES_DATES.length)])); + return createIssue(CODE_SMELL, effort, false); } - private static DefaultIssue newBugIssue(long effort) { - return newCodeSmellIssueWithoutEffort() - .setEffort(Duration.create(effort)) - .setType(RuleType.BUG) - .setCreationDate(new Date(PERIOD.getDate() + 10_000L)); + private DefaultIssue newBugIssue(long effort) { + return createIssue(BUG, effort, true); } private DefaultIssue oldBugIssue(long effort) { - return newCodeSmellIssueWithoutEffort() - .setEffort(Duration.create(effort)) - .setType(RuleType.BUG) - .setCreationDate(new Date(OLD_ISSUES_DATES[random.nextInt(OLD_ISSUES_DATES.length)])); + return createIssue(BUG, effort, false); } - private static DefaultIssue newVulnerabilityIssue(long effort) { - return newCodeSmellIssueWithoutEffort() - .setEffort(Duration.create(effort)) - .setType(RuleType.VULNERABILITY) - .setCreationDate(new Date(PERIOD.getDate() + 10_000L)); + private DefaultIssue newVulnerabilityIssue(long effort) { + return createIssue(VULNERABILITY, effort, true); } private DefaultIssue oldVulnerabilityIssue(long effort) { - return newCodeSmellIssueWithoutEffort() - .setEffort(Duration.create(effort)) - .setType(RuleType.VULNERABILITY) - .setCreationDate(new Date(OLD_ISSUES_DATES[random.nextInt(OLD_ISSUES_DATES.length)])); + return createIssue(VULNERABILITY, effort, false); } - private static DefaultIssue newCodeSmellIssueWithoutEffort() { - return new DefaultIssue() - .setType(CODE_SMELL) - .setCreationDate(new Date(PERIOD.getDate() + 10_000L)); + private DefaultIssue newCodeSmellIssueWithoutEffort() { + DefaultIssue defaultIssue = new DefaultIssue() + .setKey(UuidFactoryFast.getInstance().create()) + .setType(CODE_SMELL); + when(newIssueClassifier.isNew(any(), eq(defaultIssue))).thenReturn(true); + return defaultIssue; + } + + private DefaultIssue createIssue(RuleType type, long effort, boolean isNew) { + DefaultIssue defaultIssue = new DefaultIssue() + .setKey(UuidFactoryFast.getInstance().create()) + .setEffort(Duration.create(effort)) + .setType(type); + when(newIssueClassifier.isNew(any(), eq(defaultIssue))).thenReturn(isNew); + return defaultIssue; } private static DefaultIssue newBugIssueWithoutEffort() { diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewIssueClassifierTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewIssueClassifierTest.java new file mode 100644 index 00000000000..95584a702d3 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewIssueClassifierTest.java @@ -0,0 +1,149 @@ +/* + * 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.Date; +import java.util.Optional; +import java.util.Set; +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.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.period.Period; +import org.sonar.ce.task.projectanalysis.period.PeriodHolderRule; +import org.sonar.ce.task.projectanalysis.source.NewLinesRepository; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.db.component.BranchType; +import org.sonar.db.newcodeperiod.NewCodePeriodType; +import org.sonar.db.protobuf.DbCommons; +import org.sonar.db.protobuf.DbIssues; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class NewIssueClassifierTest { + @Rule + public PeriodHolderRule periodHolder = new PeriodHolderRule(); + @Rule + public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule(); + private final NewLinesRepository newLinesRepository = mock(NewLinesRepository.class); + private final NewIssueClassifier newIssueClassifier = new NewIssueClassifier(newLinesRepository, periodHolder, analysisMetadataHolder); + + @Test + public void isEnabled_returns_false() { + periodHolder.setPeriod(null); + assertThat(newIssueClassifier.isEnabled()).isFalse(); + } + + @Test + public void isEnabled_returns_true_when_pull_request() { + periodHolder.setPeriod(null); + analysisMetadataHolder.setBranch(newPr()); + assertThat(newIssueClassifier.isEnabled()).isTrue(); + } + + @Test + public void isEnabled_returns_true_when_periodDate_present() { + periodHolder.setPeriod(new Period(NewCodePeriodType.NUMBER_OF_DAYS.name(), "10", 1000L)); + assertThat(newIssueClassifier.isEnabled()).isTrue(); + } + + @Test + public void isEnabled_returns_true_when_reference_period_present() { + periodHolder.setPeriod(new Period(NewCodePeriodType.REFERENCE_BRANCH.name(), "master", null)); + assertThat(newIssueClassifier.isEnabled()).isTrue(); + } + + @Test + public void isNew_returns_true_for_any_issue_if_pull_request() { + periodHolder.setPeriod(null); + analysisMetadataHolder.setBranch(newPr()); + assertThat(newIssueClassifier.isNew(mock(Component.class), mock(DefaultIssue.class))).isTrue(); + } + + @Test + public void isNew_returns_true_if_issue_is_on_period() { + periodHolder.setPeriod(new Period(NewCodePeriodType.NUMBER_OF_DAYS.name(), "10", 1000L)); + DefaultIssue issue = mock(DefaultIssue.class); + when(issue.creationDate()).thenReturn(new Date(2000L)); + assertThat(newIssueClassifier.isNew(mock(Component.class), issue)).isTrue(); + } + + @Test + public void isNew_returns_true_for_issue_located_on_changed_lines() { + periodHolder.setPeriod(new Period(NewCodePeriodType.REFERENCE_BRANCH.name(), "master", null)); + Component file = mock(Component.class); + DefaultIssue issue = mock(DefaultIssue.class); + when(file.getType()).thenReturn(Component.Type.FILE); + when(file.getUuid()).thenReturn("fileUuid"); + when(newLinesRepository.getNewLines(file)).thenReturn(Optional.of(Set.of(2, 3))); + when(issue.getLocations()).thenReturn(DbIssues.Locations.newBuilder() + .setTextRange(DbCommons.TextRange.newBuilder() + .setStartLine(2) + .setStartOffset(1) + .setEndLine(2) + .setEndOffset(2) + .build()) + .build()); + assertThat(newIssueClassifier.isNew(file, issue)).isTrue(); + } + + @Test + public void isNew_returns_false_for_issue_not_located_on_changed_lines() { + periodHolder.setPeriod(new Period(NewCodePeriodType.REFERENCE_BRANCH.name(), "master", null)); + Component file = mock(Component.class); + DefaultIssue issue = mock(DefaultIssue.class); + when(file.getType()).thenReturn(Component.Type.FILE); + when(file.getUuid()).thenReturn("fileUuid"); + when(newLinesRepository.getNewLines(file)).thenReturn(Optional.of(Set.of(2, 3))); + when(issue.getLocations()).thenReturn(DbIssues.Locations.newBuilder() + .setTextRange(DbCommons.TextRange.newBuilder() + .setStartLine(10) + .setStartOffset(1) + .setEndLine(10) + .setEndOffset(2) + .build()) + .build()); + assertThat(newIssueClassifier.isNew(file, issue)).isFalse(); + } + + @Test + public void isNew_returns_false_if_issue_is_not_on_period() { + periodHolder.setPeriod(new Period(NewCodePeriodType.NUMBER_OF_DAYS.name(), "10", 1000L)); + DefaultIssue issue = mock(DefaultIssue.class); + when(issue.creationDate()).thenReturn(new Date(500L)); + assertThat(newIssueClassifier.isNew(mock(Component.class), issue)).isFalse(); + } + + @Test + public void isNew_returns_false_if_period_without_date() { + periodHolder.setPeriod(new Period(NewCodePeriodType.NUMBER_OF_DAYS.name(), "10", null)); + assertThat(newIssueClassifier.isNew(mock(Component.class), mock(DefaultIssue.class))).isFalse(); + } + + private Branch newPr() { + Branch nonMainBranch = mock(Branch.class); + when(nonMainBranch.isMain()).thenReturn(false); + when(nonMainBranch.getType()).thenReturn(BranchType.PULL_REQUEST); + return nonMainBranch; + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/period/NewCodeReferenceBranchComponentUuidsTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/period/NewCodeReferenceBranchComponentUuidsTest.java new file mode 100644 index 00000000000..e9352a996e4 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/period/NewCodeReferenceBranchComponentUuidsTest.java @@ -0,0 +1,116 @@ +/* + * 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.period; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; +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.newcodeperiod.NewCodePeriodType; +import org.sonar.server.project.Project; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.sonar.db.component.SnapshotTesting.newAnalysis; + +public class NewCodeReferenceBranchComponentUuidsTest { + @Rule + public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule(); + + @Rule + public PeriodHolderRule periodHolder = new PeriodHolderRule(); + @Rule + public DbTester db = DbTester.create(); + + private NewCodeReferenceBranchComponentUuids underTest = new NewCodeReferenceBranchComponentUuids(analysisMetadataHolder, periodHolder, db.getDbClient()); + + private ComponentDto branch1; + private ComponentDto branch1File; + private ComponentDto pr1File; + private ComponentDto pr2File; + private Project project = mock(Project.class); + private ComponentDto pr1; + private ComponentDto pr2; + private ComponentDto branch2; + private ComponentDto branch2File; + + @Before + public void setUp() { + analysisMetadataHolder.setProject(project); + + ComponentDto projectDto = db.components().insertPublicProject(); + when(project.getUuid()).thenReturn(projectDto.uuid()); + branch1 = db.components().insertProjectBranch(projectDto, b -> b.setKey("branch1")); + branch2 = db.components().insertProjectBranch(projectDto, b -> b.setKey("branch2")); + pr1 = db.components().insertProjectBranch(projectDto, b -> b.setKey("pr1").setBranchType(BranchType.PULL_REQUEST).setMergeBranchUuid(branch1.uuid())); + pr2 = db.components().insertProjectBranch(projectDto, b -> b.setKey("pr2").setBranchType(BranchType.PULL_REQUEST).setMergeBranchUuid(branch1.uuid())); + branch1File = ComponentTesting.newFileDto(branch1, null, "file").setUuid("branch1File"); + branch2File = ComponentTesting.newFileDto(branch2, null, "file").setUuid("branch2File"); + pr1File = ComponentTesting.newFileDto(pr1, null, "file").setUuid("file1"); + pr2File = ComponentTesting.newFileDto(pr2, null, "file").setUuid("file2"); + db.components().insertComponents(branch1File, pr1File, pr2File, branch2File); + } + + @Test + public void should_support_db_key_when_looking_for_reference_component() { + periodHolder.setPeriod(new Period(NewCodePeriodType.REFERENCE_BRANCH.name(), "branch1", null)); + db.components().insertSnapshot(newAnalysis(branch1)); + assertThat(underTest.getComponentUuid(pr1File.getDbKey())).isEqualTo(branch1File.uuid()); + } + + @Test + public void should_support_key_when_looking_for_reference_component() { + periodHolder.setPeriod(new Period(NewCodePeriodType.REFERENCE_BRANCH.name(), "branch1", null)); + db.components().insertSnapshot(newAnalysis(branch1)); + assertThat(underTest.getComponentUuid(pr1File.getKey())).isEqualTo(branch1File.uuid()); + } + + @Test + public void return_null_if_file_doesnt_exist() { + periodHolder.setPeriod(new Period(NewCodePeriodType.REFERENCE_BRANCH.name(), "branch1", null)); + db.components().insertSnapshot(newAnalysis(branch1)); + assertThat(underTest.getComponentUuid("doesnt exist")).isNull(); + } + + @Test + public void skip_init_if_no_reference_branch_analysis() { + periodHolder.setPeriod(new Period(NewCodePeriodType.REFERENCE_BRANCH.name(), "branch1", null)); + assertThat(underTest.getComponentUuid(pr1File.getDbKey())).isNull(); + } + + @Test + public void skip_init_if_branch_not_found() { + periodHolder.setPeriod(new Period(NewCodePeriodType.REFERENCE_BRANCH.name(), "unknown", null)); + assertThat(underTest.getComponentUuid(pr1File.getDbKey())).isNull(); + } + + @Test + public void throw_ise_if_mode_is_not_reference_branch() { + periodHolder.setPeriod(new Period(NewCodePeriodType.NUMBER_OF_DAYS.name(), "10", 1000L)); + assertThatThrownBy(() -> underTest.getComponentUuid(pr1File.getDbKey())) + .isInstanceOf(IllegalStateException.class); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitorTest.java index dc715492f20..56325609157 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitorTest.java @@ -21,12 +21,11 @@ package org.sonar.ce.task.projectanalysis.qualitymodel; import java.util.Arrays; import java.util.Date; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.Duration; -import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; -import org.sonar.ce.task.projectanalysis.analysis.Branch; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.FileAttributes; import org.sonar.ce.task.projectanalysis.component.ReportComponent; @@ -34,17 +33,17 @@ import org.sonar.ce.task.projectanalysis.component.TreeRootHolderRule; import org.sonar.ce.task.projectanalysis.component.VisitorsCrawler; import org.sonar.ce.task.projectanalysis.issue.ComponentIssuesRepositoryRule; import org.sonar.ce.task.projectanalysis.issue.FillComponentIssuesVisitorRule; +import org.sonar.ce.task.projectanalysis.issue.NewIssueClassifier; import org.sonar.ce.task.projectanalysis.measure.MeasureAssert; import org.sonar.ce.task.projectanalysis.measure.MeasureRepositoryRule; import org.sonar.ce.task.projectanalysis.metric.MetricRepositoryRule; -import org.sonar.ce.task.projectanalysis.period.Period; -import org.sonar.ce.task.projectanalysis.period.PeriodHolderRule; import org.sonar.core.issue.DefaultIssue; -import org.sonar.core.util.Uuids; -import org.sonar.db.component.BranchType; +import org.sonar.core.util.UuidFactoryFast; import org.sonar.server.measure.Rating; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; @@ -74,7 +73,6 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { private static final long LEAK_PERIOD_SNAPSHOT_IN_MILLISEC = 12323L; private static final Date DEFAULT_ISSUE_CREATION_DATE = new Date(1000L); - private static final Date BEFORE_LEAK_PERIOD_DATE = new Date(LEAK_PERIOD_SNAPSHOT_IN_MILLISEC - 5000L); private static final Date AFTER_LEAK_PERIOD_DATE = new Date(LEAK_PERIOD_SNAPSHOT_IN_MILLISEC + 5000L); static final String LANGUAGE_KEY_1 = "lKey1"; @@ -108,20 +106,19 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { @Rule public MeasureRepositoryRule measureRepository = MeasureRepositoryRule.create(treeRootHolder, metricRepository); - @Rule - public PeriodHolderRule periodsHolder = new PeriodHolderRule().setPeriod(new Period("mode", null, LEAK_PERIOD_SNAPSHOT_IN_MILLISEC)); - - @Rule - public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule(); @Rule public ComponentIssuesRepositoryRule componentIssuesRepositoryRule = new ComponentIssuesRepositoryRule(treeRootHolder); - @Rule public FillComponentIssuesVisitorRule fillComponentIssuesVisitorRule = new FillComponentIssuesVisitorRule(componentIssuesRepositoryRule, treeRootHolder); - private VisitorsCrawler underTest = new VisitorsCrawler(Arrays.asList(fillComponentIssuesVisitorRule, - new NewReliabilityAndSecurityRatingMeasuresVisitor(metricRepository, measureRepository, componentIssuesRepositoryRule, - periodsHolder, analysisMetadataHolder))); + private final NewIssueClassifier newIssueClassifier = mock(NewIssueClassifier.class); + private final VisitorsCrawler underTest = new VisitorsCrawler(Arrays.asList(fillComponentIssuesVisitorRule, + new NewReliabilityAndSecurityRatingMeasuresVisitor(metricRepository, measureRepository, componentIssuesRepositoryRule, newIssueClassifier))); + + @Before + public void before() { + when(newIssueClassifier.isEnabled()).thenReturn(true); + } @Test public void measures_created_for_project_are_all_A_when_they_have_no_FILE_child() { @@ -136,7 +133,7 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { @Test public void no_measure_if_there_is_no_period() { - periodsHolder.setPeriod(null); + when(newIssueClassifier.isEnabled()).thenReturn(false); treeRootHolder.setRoot(builder(PROJECT, 1).build()); underTest.visit(treeRootHolder.getRoot()); @@ -148,17 +145,16 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { public void compute_new_security_rating() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newVulnerabilityIssue(10L, MAJOR).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newVulnerabilityIssue(10L, MAJOR), // Should not be taken into account - newVulnerabilityIssue(1L, MAJOR).setCreationDate(BEFORE_LEAK_PERIOD_DATE), - newBugIssue(1L, MAJOR).setCreationDate(AFTER_LEAK_PERIOD_DATE)); + oldVulnerabilityIssue(1L, MAJOR), + newBugIssue(1L, MAJOR)); fillComponentIssuesVisitorRule.setIssues(FILE_2_REF, - newVulnerabilityIssue(2L, CRITICAL).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newVulnerabilityIssue(3L, MINOR).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newVulnerabilityIssue(2L, CRITICAL), + newVulnerabilityIssue(3L, MINOR), // Should not be taken into account - newVulnerabilityIssue(10L, BLOCKER).setCreationDate(AFTER_LEAK_PERIOD_DATE).setResolution(RESOLUTION_FIXED)); - fillComponentIssuesVisitorRule.setIssues(ROOT_DIR_REF, - newVulnerabilityIssue(7L, BLOCKER).setCreationDate(AFTER_LEAK_PERIOD_DATE)); + newVulnerabilityIssue(10L, BLOCKER).setResolution(RESOLUTION_FIXED)); + fillComponentIssuesVisitorRule.setIssues(ROOT_DIR_REF, newVulnerabilityIssue(7L, BLOCKER)); underTest.visit(ROOT_PROJECT); @@ -186,8 +182,7 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { @Test public void compute_new_security_rating_to_A_when_no_new_issue() { treeRootHolder.setRoot(ROOT_PROJECT); - fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newVulnerabilityIssue(1L, MAJOR).setCreationDate(BEFORE_LEAK_PERIOD_DATE)); + fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, oldVulnerabilityIssue(1L, MAJOR)); underTest.visit(ROOT_PROJECT); @@ -202,17 +197,17 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { public void compute_new_reliability_rating() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newBugIssue(10L, MAJOR).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newBugIssue(10L, MAJOR), // Should not be taken into account - newBugIssue(1L, MAJOR).setCreationDate(BEFORE_LEAK_PERIOD_DATE), - newVulnerabilityIssue(1L, MAJOR).setCreationDate(AFTER_LEAK_PERIOD_DATE)); + oldBugIssue(1L, MAJOR), + newVulnerabilityIssue(1L, MAJOR)); fillComponentIssuesVisitorRule.setIssues(FILE_2_REF, - newBugIssue(2L, CRITICAL).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newBugIssue(3L, MINOR).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newBugIssue(2L, CRITICAL), + newBugIssue(3L, MINOR), // Should not be taken into account - newBugIssue(10L, BLOCKER).setCreationDate(AFTER_LEAK_PERIOD_DATE).setResolution(RESOLUTION_FIXED)); + newBugIssue(10L, BLOCKER).setResolution(RESOLUTION_FIXED)); fillComponentIssuesVisitorRule.setIssues(ROOT_DIR_REF, - newBugIssue(7L, BLOCKER).setCreationDate(AFTER_LEAK_PERIOD_DATE)); + newBugIssue(7L, BLOCKER)); underTest.visit(ROOT_PROJECT); @@ -240,8 +235,7 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { @Test public void compute_new_reliability_rating_to_A_when_no_new_issue() { treeRootHolder.setRoot(ROOT_PROJECT); - fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newBugIssue(1L, MAJOR).setCreationDate(BEFORE_LEAK_PERIOD_DATE)); + fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, oldBugIssue(1L, MAJOR)); underTest.visit(ROOT_PROJECT); @@ -256,10 +250,10 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { public void compute_E_reliability_and_security_rating_on_blocker_issue() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newBugIssue(10L, BLOCKER).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newVulnerabilityIssue(1L, BLOCKER).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newBugIssue(10L, BLOCKER), + newVulnerabilityIssue(1L, BLOCKER), // Should not be taken into account - newBugIssue(1L, MAJOR).setCreationDate(AFTER_LEAK_PERIOD_DATE)); + newBugIssue(1L, MAJOR)); underTest.visit(ROOT_PROJECT); @@ -271,10 +265,10 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { public void compute_D_reliability_and_security_rating_on_critical_issue() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newBugIssue(10L, CRITICAL).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newVulnerabilityIssue(15L, CRITICAL).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newBugIssue(10L, CRITICAL), + newVulnerabilityIssue(15L, CRITICAL), // Should not be taken into account - newCodeSmellIssue(1L, MAJOR).setCreationDate(AFTER_LEAK_PERIOD_DATE)); + newCodeSmellIssue(1L, MAJOR)); underTest.visit(ROOT_PROJECT); @@ -284,18 +278,12 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { @Test public void compute_C_reliability_and_security_rating_on_major_issue() { - // Calculate metric not because a period is set, but because it is a PR - periodsHolder.setPeriod(null); - Branch b = mock(Branch.class); - when(b.getType()).thenReturn(BranchType.PULL_REQUEST); - analysisMetadataHolder.setBranch(b); - treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newBugIssue(10L, MAJOR).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newVulnerabilityIssue(15L, MAJOR).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newBugIssue(10L, MAJOR), + newVulnerabilityIssue(15L, MAJOR), // Should not be taken into account - newCodeSmellIssue(1L, MAJOR).setCreationDate(AFTER_LEAK_PERIOD_DATE)); + newCodeSmellIssue(1L, MAJOR)); underTest.visit(ROOT_PROJECT); @@ -307,10 +295,10 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { public void compute_B_reliability_and_security_rating_on_minor_issue() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newBugIssue(10L, MINOR).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newVulnerabilityIssue(15L, MINOR).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newBugIssue(10L, MINOR), + newVulnerabilityIssue(15L, MINOR), // Should not be taken into account - newCodeSmellIssue(1L, MAJOR).setCreationDate(AFTER_LEAK_PERIOD_DATE)); + newCodeSmellIssue(1L, MAJOR)); underTest.visit(ROOT_PROJECT); @@ -338,26 +326,36 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { .hasVariation(rating.getIndex()); } - private static DefaultIssue newBugIssue(long effort, String severity) { - return newIssue(effort, severity, BUG); + private DefaultIssue newBugIssue(long effort, String severity) { + return createIssue(effort, severity, BUG, true); + } + + private DefaultIssue oldBugIssue(long effort, String severity) { + return createIssue(effort, severity, BUG, false); + } + + private DefaultIssue newVulnerabilityIssue(long effort, String severity) { + return createIssue(effort, severity, VULNERABILITY, true); } - private static DefaultIssue newVulnerabilityIssue(long effort, String severity) { - return newIssue(effort, severity, VULNERABILITY); + private DefaultIssue oldVulnerabilityIssue(long effort, String severity) { + return createIssue(effort, severity, VULNERABILITY, false); } - private static DefaultIssue newCodeSmellIssue(long effort, String severity) { - return newIssue(effort, severity, CODE_SMELL); + private DefaultIssue newCodeSmellIssue(long effort, String severity) { + return createIssue(effort, severity, CODE_SMELL, true); } - private static DefaultIssue newIssue(long effort, String severity, RuleType type) { - return newIssue(severity, type) + private DefaultIssue createIssue(long effort, String severity, RuleType type, boolean isNew) { + DefaultIssue issue = createIssue(severity, type) .setEffort(Duration.create(effort)); + when(newIssueClassifier.isNew(any(), eq(issue))).thenReturn(isNew); + return issue; } - private static DefaultIssue newIssue(String severity, RuleType type) { + private static DefaultIssue createIssue(String severity, RuleType type) { return new DefaultIssue() - .setKey(Uuids.create()) + .setKey(UuidFactoryFast.getInstance().create()) .setSeverity(severity) .setType(type) .setCreationDate(DEFAULT_ISSUE_CREATION_DATE); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitorTest.java index 7f873c6ebf8..5db31cfa634 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitorTest.java @@ -20,15 +20,13 @@ package org.sonar.ce.task.projectanalysis.qualitymodel; import java.util.Arrays; -import java.util.Date; import javax.annotation.Nullable; import org.assertj.core.api.Assertions; import org.assertj.core.data.Offset; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.sonar.api.rules.RuleType; -import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; -import org.sonar.ce.task.projectanalysis.analysis.Branch; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.FileAttributes; import org.sonar.ce.task.projectanalysis.component.TreeRootHolderRule; @@ -37,14 +35,15 @@ import org.sonar.ce.task.projectanalysis.issue.ComponentIssuesRepositoryRule; import org.sonar.ce.task.projectanalysis.issue.FillComponentIssuesVisitorRule; import org.sonar.ce.task.projectanalysis.measure.MeasureRepositoryRule; import org.sonar.ce.task.projectanalysis.metric.MetricRepositoryRule; -import org.sonar.ce.task.projectanalysis.period.Period; -import org.sonar.ce.task.projectanalysis.period.PeriodHolderRule; +import org.sonar.ce.task.projectanalysis.issue.NewIssueClassifier; import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.util.UuidFactoryFast; import org.sonar.core.util.Uuids; -import org.sonar.db.component.BranchType; import org.sonar.server.measure.Rating; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; @@ -71,14 +70,7 @@ import static org.sonar.server.measure.Rating.D; import static org.sonar.server.measure.Rating.E; public class NewSecurityReviewMeasuresVisitorTest { - private static final Offset VARIATION_COMPARISON_OFFSET = Offset.offset(0.01); - - private static final long LEAK_PERIOD_SNAPSHOT_IN_MILLISEC = 12323L; - private static final Date DEFAULT_CREATION_DATE = new Date(1000L); - private static final Date BEFORE_LEAK_PERIOD_DATE = new Date(LEAK_PERIOD_SNAPSHOT_IN_MILLISEC - 5000L); - private static final Date AFTER_LEAK_PERIOD_DATE = new Date(LEAK_PERIOD_SNAPSHOT_IN_MILLISEC + 5000L); - private static final String LANGUAGE_KEY_1 = "lKey1"; private static final int PROJECT_REF = 1; @@ -110,31 +102,32 @@ public class NewSecurityReviewMeasuresVisitorTest { @Rule public MeasureRepositoryRule measureRepository = MeasureRepositoryRule.create(treeRootHolder, metricRepository); @Rule - public PeriodHolderRule periodsHolder = new PeriodHolderRule().setPeriod(new Period("mode", null, LEAK_PERIOD_SNAPSHOT_IN_MILLISEC)); - @Rule - public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule(); - @Rule public ComponentIssuesRepositoryRule componentIssuesRepositoryRule = new ComponentIssuesRepositoryRule(treeRootHolder); @Rule public FillComponentIssuesVisitorRule fillComponentIssuesVisitorRule = new FillComponentIssuesVisitorRule(componentIssuesRepositoryRule, treeRootHolder); + private final NewIssueClassifier newIssueClassifier = mock(NewIssueClassifier.class); + private final VisitorsCrawler underTest = new VisitorsCrawler(Arrays.asList(fillComponentIssuesVisitorRule, + new NewSecurityReviewMeasuresVisitor(componentIssuesRepositoryRule, measureRepository, metricRepository, newIssueClassifier))); - private VisitorsCrawler underTest = new VisitorsCrawler(Arrays.asList(fillComponentIssuesVisitorRule, - new NewSecurityReviewMeasuresVisitor(componentIssuesRepositoryRule, measureRepository, periodsHolder, analysisMetadataHolder, metricRepository))); + @Before + public void setup() { + when(newIssueClassifier.isEnabled()).thenReturn(true); + } @Test public void compute_measures_when_100_percent_hotspots_reviewed() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), // Should not be taken into account - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(BEFORE_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(BEFORE_LEAK_PERIOD_DATE), - newIssue().setCreationDate(AFTER_LEAK_PERIOD_DATE)); + oldHotspot(STATUS_TO_REVIEW, null), + oldHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newIssue()); fillComponentIssuesVisitorRule.setIssues(FILE_2_REF, - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE)); + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED)); fillComponentIssuesVisitorRule.setIssues(ROOT_DIR_REF, - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE)); + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED)); underTest.visit(ROOT_PROJECT); @@ -149,20 +142,20 @@ public class NewSecurityReviewMeasuresVisitorTest { public void compute_measures_when_more_than_80_percent_hotspots_reviewed() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), // Should not be taken into account - newIssue().setCreationDate(AFTER_LEAK_PERIOD_DATE)); + newIssue()); fillComponentIssuesVisitorRule.setIssues(FILE_2_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), // Should not be taken into account - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(BEFORE_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(BEFORE_LEAK_PERIOD_DATE), + oldHotspot(STATUS_TO_REVIEW, null), + oldHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), newIssue()); underTest.visit(ROOT_PROJECT); @@ -178,20 +171,20 @@ public class NewSecurityReviewMeasuresVisitorTest { public void compute_measures_when_more_than_70_percent_hotspots_reviewed() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), // Should not be taken into account - newIssue().setCreationDate(AFTER_LEAK_PERIOD_DATE)); + newIssue()); fillComponentIssuesVisitorRule.setIssues(FILE_2_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), // Should not be taken into account - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(BEFORE_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(BEFORE_LEAK_PERIOD_DATE), + oldHotspot(STATUS_TO_REVIEW, null), + oldHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), newIssue()); underTest.visit(ROOT_PROJECT); @@ -207,19 +200,19 @@ public class NewSecurityReviewMeasuresVisitorTest { public void compute_measures_when_more_than_50_percent_hotspots_reviewed() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), // Should not be taken into account newIssue()); fillComponentIssuesVisitorRule.setIssues(FILE_2_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), // Should not be taken into account - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(BEFORE_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(BEFORE_LEAK_PERIOD_DATE), + oldHotspot(STATUS_TO_REVIEW, null), + oldHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), newIssue()); underTest.visit(ROOT_PROJECT); @@ -235,20 +228,20 @@ public class NewSecurityReviewMeasuresVisitorTest { public void compute_measures_when_more_30_than_percent_hotspots_reviewed() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), // Should not be taken into account newIssue()); fillComponentIssuesVisitorRule.setIssues(FILE_2_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), // Should not be taken into account - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(BEFORE_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(BEFORE_LEAK_PERIOD_DATE), + oldHotspot(STATUS_TO_REVIEW, null), + oldHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), newIssue()); underTest.visit(ROOT_PROJECT); @@ -264,18 +257,18 @@ public class NewSecurityReviewMeasuresVisitorTest { public void compute_measures_when_less_than_30_percent_hotspots_reviewed() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), // Should not be taken into account newIssue()); fillComponentIssuesVisitorRule.setIssues(FILE_2_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_TO_REVIEW, null), // Should not be taken into account - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(BEFORE_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(BEFORE_LEAK_PERIOD_DATE), + oldHotspot(STATUS_TO_REVIEW, null), + oldHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), newIssue()); underTest.visit(ROOT_PROJECT); @@ -291,8 +284,8 @@ public class NewSecurityReviewMeasuresVisitorTest { public void compute_A_rating_and_no_percent_when_no_new_hotspot_on_new_code() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(BEFORE_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(BEFORE_LEAK_PERIOD_DATE), + oldHotspot(STATUS_TO_REVIEW, null), + oldHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), newIssue()); underTest.visit(ROOT_PROJECT); @@ -300,51 +293,20 @@ public class NewSecurityReviewMeasuresVisitorTest { verifyRatingAndReviewedMeasures(PROJECT_REF, A, null); } - @Test - public void compute_measures_on_pr() { - periodsHolder.setPeriod(null); - Branch b = mock(Branch.class); - when(b.getType()).thenReturn(BranchType.PULL_REQUEST); - analysisMetadataHolder.setBranch(b); - treeRootHolder.setRoot(ROOT_PROJECT); - fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - // Should not be taken into account - newIssue()); - fillComponentIssuesVisitorRule.setIssues(FILE_2_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - // Dates is not taken into account on PR - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(BEFORE_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(BEFORE_LEAK_PERIOD_DATE)); - - underTest.visit(ROOT_PROJECT); - - verifyRatingAndReviewedMeasures(FILE_1_REF, C, 50.0); - verifyRatingAndReviewedMeasures(FILE_2_REF, C, 57.14); - verifyRatingAndReviewedMeasures(DIRECTORY_REF, C, 55.55); - verifyRatingAndReviewedMeasures(ROOT_DIR_REF, C, 55.55); - verifyRatingAndReviewedMeasures(PROJECT_REF, C, 55.55); - } - @Test public void compute_status_related_measures() { treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), // Should not be taken into account newIssue()); fillComponentIssuesVisitorRule.setIssues(FILE_2_REF, - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_TO_REVIEW, null).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), - newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED), newIssue()); underTest.visit(ROOT_PROJECT); @@ -367,7 +329,7 @@ public class NewSecurityReviewMeasuresVisitorTest { @Test public void no_measure_if_there_is_no_period() { - periodsHolder.setPeriod(null); + when(newIssueClassifier.isEnabled()).thenReturn(false); treeRootHolder.setRoot(ROOT_PROJECT); fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, newHotspot(STATUS_TO_REVIEW, null), @@ -380,7 +342,7 @@ public class NewSecurityReviewMeasuresVisitorTest { private void verifyRatingAndReviewedMeasures(int componentRef, Rating expectedReviewRating, @Nullable Double expectedHotspotsReviewed) { assertThat(measureRepository.getAddedRawMeasure(componentRef, NEW_SECURITY_REVIEW_RATING_KEY)).hasVariation(expectedReviewRating.getIndex()); - if (expectedHotspotsReviewed != null){ + if (expectedHotspotsReviewed != null) { assertThat(measureRepository.getAddedRawMeasure(componentRef, NEW_SECURITY_HOTSPOTS_REVIEWED_KEY)).hasVariation(expectedHotspotsReviewed, VARIATION_COMPARISON_OFFSET); } else { @@ -401,22 +363,33 @@ public class NewSecurityReviewMeasuresVisitorTest { } } - private static DefaultIssue newHotspot(String status, @Nullable String resolution) { - return new DefaultIssue() - .setKey(Uuids.create()) + private DefaultIssue newHotspot(String status, @Nullable String resolution) { + return createHotspot(status, resolution, true); + } + + private DefaultIssue oldHotspot(String status, @Nullable String resolution) { + return createHotspot(status, resolution, false); + } + + private DefaultIssue createHotspot(String status, @Nullable String resolution, boolean isNew) { + DefaultIssue issue = new DefaultIssue() + .setKey(UuidFactoryFast.getInstance().create()) .setSeverity(MINOR) .setStatus(status) .setResolution(resolution) - .setType(RuleType.SECURITY_HOTSPOT) - .setCreationDate(DEFAULT_CREATION_DATE); + .setType(RuleType.SECURITY_HOTSPOT); + when(newIssueClassifier.isNew(any(), eq(issue))).thenReturn(isNew); + return issue; } - private static DefaultIssue newIssue() { - return new DefaultIssue() + private DefaultIssue newIssue() { + DefaultIssue issue = new DefaultIssue() .setKey(Uuids.create()) .setSeverity(MAJOR) - .setType(RuleType.BUG) - .setCreationDate(DEFAULT_CREATION_DATE); + .setType(RuleType.BUG); + when(newIssueClassifier.isNew(any(), eq(issue))).thenReturn(false); + return issue; + } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/NewLinesRepositoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/NewLinesRepositoryTest.java index f64a5c9a635..cf083226f66 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/NewLinesRepositoryTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/NewLinesRepositoryTest.java @@ -34,6 +34,7 @@ import org.sonar.ce.task.projectanalysis.period.PeriodHolderRule; import org.sonar.ce.task.projectanalysis.scm.Changeset; import org.sonar.ce.task.projectanalysis.scm.ScmInfoRepositoryRule; import org.sonar.db.component.BranchType; +import org.sonar.db.newcodeperiod.NewCodePeriodType; import org.sonar.scanner.protocol.output.ScannerReport; import static org.assertj.core.api.Assertions.assertThat; @@ -52,7 +53,7 @@ public class NewLinesRepositoryTest { @Rule public ScmInfoRepositoryRule scmInfoRepository = new ScmInfoRepositoryRule(); - private NewLinesRepository repository = new NewLinesRepository(reader, analysisMetadataHolder, periodHolder, scmInfoRepository); + private final NewLinesRepository repository = new NewLinesRepository(reader, analysisMetadataHolder, periodHolder, scmInfoRepository); @Test public void load_new_lines_from_report_when_available_and_pullrequest() { @@ -66,6 +67,18 @@ public class NewLinesRepositoryTest { assertThat(repository.newLinesAvailable()).isTrue(); } + @Test + public void load_new_lines_from_report_when_available_and_using_reference_branch() { + periodHolder.setPeriod(new Period(NewCodePeriodType.REFERENCE_BRANCH.name(), null, null)); + createChangedLinesInReport(1, 2, 5); + + Optional> newLines = repository.getNewLines(FILE); + + assertThat(newLines).isPresent(); + assertThat(newLines.get()).containsOnly(1, 2, 5); + assertThat(repository.newLinesAvailable()).isTrue(); + } + @Test public void compute_new_lines_using_scm_info_for_period() { periodHolder.setPeriod(new Period("", null, 1000L)); @@ -93,7 +106,7 @@ public class NewLinesRepositoryTest { } @Test - public void return_empty_if_no_period_and_not_pullrequest() { + public void return_empty_if_no_period_no_pullrequest_and_no_reference_branch() { periodHolder.setPeriod(null); // even though we have lines in the report and scm data, nothing should be returned since we have no period diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/SourceLinesDiffImplTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/SourceLinesDiffImplTest.java index a88145244d7..6116ec27575 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/SourceLinesDiffImplTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/SourceLinesDiffImplTest.java @@ -25,8 +25,10 @@ import org.junit.Rule; import org.junit.Test; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.period.NewCodeReferenceBranchComponentUuids; import org.sonar.ce.task.projectanalysis.component.ReferenceBranchComponentUuids; import org.sonar.ce.task.projectanalysis.filemove.MutableMovedFilesRepositoryRule; +import org.sonar.ce.task.projectanalysis.period.PeriodHolderRule; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDao; @@ -40,18 +42,22 @@ import static org.sonar.ce.task.projectanalysis.component.ReportComponent.builde public class SourceLinesDiffImplTest { - private DbClient dbClient = mock(DbClient.class); - private DbSession dbSession = mock(DbSession.class); - private ComponentDao componentDao = mock(ComponentDao.class); - private FileSourceDao fileSourceDao = mock(FileSourceDao.class); - private SourceLinesHashRepository sourceLinesHash = mock(SourceLinesHashRepository.class); - private AnalysisMetadataHolder analysisMetadataHolder = mock(AnalysisMetadataHolder.class); - private ReferenceBranchComponentUuids referenceBranchComponentUuids = mock(ReferenceBranchComponentUuids.class); + private final DbClient dbClient = mock(DbClient.class); + private final DbSession dbSession = mock(DbSession.class); + private final ComponentDao componentDao = mock(ComponentDao.class); + private final FileSourceDao fileSourceDao = mock(FileSourceDao.class); + private final SourceLinesHashRepository sourceLinesHash = mock(SourceLinesHashRepository.class); + private final AnalysisMetadataHolder analysisMetadataHolder = mock(AnalysisMetadataHolder.class); + private final ReferenceBranchComponentUuids referenceBranchComponentUuids = mock(ReferenceBranchComponentUuids.class); + private final NewCodeReferenceBranchComponentUuids newCodeReferenceBranchComponentUuids = mock(NewCodeReferenceBranchComponentUuids.class); + @Rule + public PeriodHolderRule periodHolder = new PeriodHolderRule(); + @Rule public MutableMovedFilesRepositoryRule movedFiles = new MutableMovedFilesRepositoryRule(); - private SourceLinesDiffImpl underTest = new SourceLinesDiffImpl(dbClient, fileSourceDao, sourceLinesHash, - referenceBranchComponentUuids, movedFiles, analysisMetadataHolder); + private final SourceLinesDiffImpl underTest = new SourceLinesDiffImpl(dbClient, fileSourceDao, sourceLinesHash, + referenceBranchComponentUuids, movedFiles, analysisMetadataHolder, periodHolder, newCodeReferenceBranchComponentUuids); private static final int FILE_REF = 1; @@ -74,6 +80,7 @@ public class SourceLinesDiffImplTest { @Test public void should_find_diff_with_reference_branch_for_prs() { + periodHolder.setPeriod(null); Component component = fileComponent(FILE_REF); mockLineHashesInDb(2, CONTENT); @@ -87,6 +94,7 @@ public class SourceLinesDiffImplTest { @Test public void all_file_is_modified_if_no_source_in_db() { + periodHolder.setPeriod(null); Component component = fileComponent(FILE_REF); setLineHashesInReport(component, CONTENT); @@ -96,6 +104,7 @@ public class SourceLinesDiffImplTest { @Test public void should_find_no_diff_when_report_and_db_content_are_identical() { + periodHolder.setPeriod(null); Component component = fileComponent(FILE_REF); mockLineHashesInDb(FILE_REF, CONTENT); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/LoadPeriodsStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/LoadPeriodsStepTest.java index 21b35f7ea81..ac37432ca69 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/LoadPeriodsStepTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/LoadPeriodsStepTest.java @@ -174,10 +174,9 @@ public class LoadPeriodsStepTest extends BaseStepTest { setupRoot(branch); setProjectPeriod(project.uuid(), NewCodePeriodType.REFERENCE_BRANCH, "master"); - when(analysisMetadataHolder.getForkDate()).thenReturn(123456789L); underTest.execute(new TestComputationStepContext()); - assertPeriod(NewCodePeriodType.REFERENCE_BRANCH, "master", 123456789L); + assertPeriod(NewCodePeriodType.REFERENCE_BRANCH, "master", null); } @Test @@ -186,7 +185,6 @@ public class LoadPeriodsStepTest extends BaseStepTest { setupRoot(branch); setProjectPeriod(project.uuid(), NewCodePeriodType.REFERENCE_BRANCH, "master"); - when(analysisMetadataHolder.getForkDate()).thenReturn(null); underTest.execute(new TestComputationStepContext()); assertPeriod(NewCodePeriodType.REFERENCE_BRANCH, "master", null); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/LoadReportAnalysisMetadataHolderStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/LoadReportAnalysisMetadataHolderStepTest.java index 782a83ef415..6f8b4eb5990 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/LoadReportAnalysisMetadataHolderStepTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/LoadReportAnalysisMetadataHolderStepTest.java @@ -103,18 +103,6 @@ public class LoadReportAnalysisMetadataHolderStepTest { assertThat(analysisMetadataHolder.getAnalysisDate()).isEqualTo(ANALYSIS_DATE); } - @Test - public void set_fork_date() { - reportReader.setMetadata( - newBatchReportBuilder() - .setForkDate(ANALYSIS_DATE) - .build()); - - underTest.execute(new TestComputationStepContext()); - - assertThat(analysisMetadataHolder.getForkDate()).isEqualTo(ANALYSIS_DATE); - } - @Test public void set_project_from_dto() { reportReader.setMetadata( diff --git a/server/sonar-ce-task-projectanalysis/src/testFixtures/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderRule.java b/server/sonar-ce-task-projectanalysis/src/testFixtures/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderRule.java index d7610680681..0975c901319 100644 --- a/server/sonar-ce-task-projectanalysis/src/testFixtures/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderRule.java +++ b/server/sonar-ce-task-projectanalysis/src/testFixtures/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderRule.java @@ -38,7 +38,6 @@ public class AnalysisMetadataHolderRule extends ExternalResource implements Muta private final InitializedProperty uuid = new InitializedProperty<>(); private final InitializedProperty analysisDate = new InitializedProperty<>(); - private final InitializedProperty forkDate = new InitializedProperty<>(); private final InitializedProperty baseAnalysis = new InitializedProperty<>(); private final InitializedProperty crossProjectDuplicationEnabled = new InitializedProperty<>(); private final InitializedProperty branch = new InitializedProperty<>(); @@ -74,24 +73,12 @@ public class AnalysisMetadataHolderRule extends ExternalResource implements Muta return this; } - @Override - public MutableAnalysisMetadataHolder setForkDate(@Nullable Long date) { - forkDate.setProperty(date); - return this; - } - @Override public long getAnalysisDate() { checkState(analysisDate.isInitialized(), "Analysis date has not been set"); return this.analysisDate.getProperty(); } - @CheckForNull - @Override - public Long getForkDate() { - return forkDate.getProperty(); - } - @Override public boolean hasAnalysisDateBeenSet() { return analysisDate.isInitialized(); diff --git a/server/sonar-ce-task-projectanalysis/src/testFixtures/java/org/sonar/ce/task/projectanalysis/analysis/MutableAnalysisMetadataHolderRule.java b/server/sonar-ce-task-projectanalysis/src/testFixtures/java/org/sonar/ce/task/projectanalysis/analysis/MutableAnalysisMetadataHolderRule.java index 4c3bb76a7e1..c2638f4c80b 100644 --- a/server/sonar-ce-task-projectanalysis/src/testFixtures/java/org/sonar/ce/task/projectanalysis/analysis/MutableAnalysisMetadataHolderRule.java +++ b/server/sonar-ce-task-projectanalysis/src/testFixtures/java/org/sonar/ce/task/projectanalysis/analysis/MutableAnalysisMetadataHolderRule.java @@ -55,22 +55,11 @@ public class MutableAnalysisMetadataHolderRule extends ExternalResource implemen return this; } - @Override - public MutableAnalysisMetadataHolder setForkDate(@Nullable Long date) { - return delegate.setForkDate(date); - } - @Override public long getAnalysisDate() { return delegate.getAnalysisDate(); } - @CheckForNull - @Override - public Long getForkDate() { - return delegate.getForkDate(); - } - @Override public boolean hasAnalysisDateBeenSet() { return delegate.hasAnalysisDateBeenSet(); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/scm/ScmProvider.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/scm/ScmProvider.java index 6dfbe796073..5e173433bf0 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/scm/ScmProvider.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/scm/ScmProvider.java @@ -87,8 +87,10 @@ public abstract class ScmProvider { * * @return null if the SCM provider was not able to compute the date * @since 8.4 + * @deprecated Not used anymore since 9.3 */ @CheckForNull + @Deprecated public Instant forkDate(String referenceBranchName, Path rootBaseDir) { return null; } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java index 396dacbf759..16267c27e55 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java @@ -22,6 +22,7 @@ package org.sonar.scanner.report; import java.nio.file.Path; import java.util.Collections; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -34,10 +35,13 @@ import org.sonar.api.utils.log.Loggers; import org.sonar.api.utils.log.Profiler; import org.sonar.scanner.protocol.output.ScannerReport; import org.sonar.scanner.protocol.output.ScannerReportWriter; +import org.sonar.scanner.repository.ReferenceBranchSupplier; import org.sonar.scanner.scan.branch.BranchConfiguration; import org.sonar.scanner.scan.filesystem.InputComponentStore; import org.sonar.scanner.scm.ScmConfiguration; +import static java.util.Optional.empty; + public class ChangedLinesPublisher implements ReportPublisherStep { private static final Logger LOG = Loggers.get(ChangedLinesPublisher.class); private static final String LOG_MSG = "SCM writing changed lines"; @@ -46,31 +50,38 @@ public class ChangedLinesPublisher implements ReportPublisherStep { private final DefaultInputProject project; private final InputComponentStore inputComponentStore; private final BranchConfiguration branchConfiguration; + private final ReferenceBranchSupplier referenceBranchSupplier; public ChangedLinesPublisher(ScmConfiguration scmConfiguration, DefaultInputProject project, InputComponentStore inputComponentStore, - BranchConfiguration branchConfiguration) { + BranchConfiguration branchConfiguration, ReferenceBranchSupplier referenceBranchSupplier) { this.scmConfiguration = scmConfiguration; this.project = project; this.inputComponentStore = inputComponentStore; this.branchConfiguration = branchConfiguration; + this.referenceBranchSupplier = referenceBranchSupplier; } @Override public void publish(ScannerReportWriter writer) { - String targetBranchName = branchConfiguration.targetBranchName(); - if (scmConfiguration.isDisabled() || !branchConfiguration.isPullRequest() || targetBranchName == null) { - return; + Optional targetBranch = getTargetBranch(); + if (targetBranch.isPresent()) { + Profiler profiler = Profiler.create(LOG).startInfo(LOG_MSG); + int count = writeChangedLines(scmConfiguration.provider(), writer, targetBranch.get()); + LOG.debug("SCM reported changed lines for {} {} in the branch", count, ScannerUtils.pluralize("file", count)); + profiler.stopInfo(); } + } - ScmProvider provider = scmConfiguration.provider(); - if (provider == null) { - return; + private Optional getTargetBranch() { + if (scmConfiguration.isDisabled() || scmConfiguration.provider() == null) { + return empty(); } - Profiler profiler = Profiler.create(LOG).startInfo(LOG_MSG); - int count = writeChangedLines(provider, writer, targetBranchName); - LOG.debug("SCM reported changed lines for {} {} in the branch", count, ScannerUtils.pluralize("file", count)); - profiler.stopInfo(); + String targetBranchName = branchConfiguration.targetBranchName(); + if (branchConfiguration.isPullRequest() && targetBranchName != null) { + return Optional.of(targetBranchName); + } + return Optional.ofNullable(referenceBranchSupplier.get()); } private int writeChangedLines(ScmProvider provider, ScannerReportWriter writer, String targetScmBranch) { @@ -92,7 +103,9 @@ public class ChangedLinesPublisher implements ReportPublisherStep { Set changedLines = pathSetMap.get(e.getKey()); if (changedLines == null) { - LOG.warn("File '{}' was detected as changed but without having changed lines", e.getKey().toAbsolutePath()); + if (branchConfiguration.isPullRequest()) { + LOG.warn("File '{}' was detected as changed but without having changed lines", e.getKey().toAbsolutePath()); + } // assume that no line was changed writeChangedLines(writer, e.getValue().scannerId(), Collections.emptySet()); } else { diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/MetadataPublisher.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/MetadataPublisher.java index 53efb72218b..4b5e46b2862 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/MetadataPublisher.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/MetadataPublisher.java @@ -21,7 +21,6 @@ package org.sonar.scanner.report; import java.io.File; import java.nio.file.Path; -import java.time.Instant; import java.util.LinkedList; import java.util.Map.Entry; import java.util.regex.Pattern; @@ -38,7 +37,6 @@ import org.sonar.scanner.fs.InputModuleHierarchy; import org.sonar.scanner.protocol.output.ScannerReport; import org.sonar.scanner.protocol.output.ScannerReport.Metadata.BranchType; import org.sonar.scanner.protocol.output.ScannerReportWriter; -import org.sonar.scanner.repository.ForkDateSupplier; import org.sonar.scanner.rule.QProfile; import org.sonar.scanner.rule.QualityProfiles; import org.sonar.scanner.scan.branch.BranchConfiguration; @@ -57,13 +55,12 @@ public class MetadataPublisher implements ReportPublisherStep { private final ScannerPluginRepository pluginRepository; private final BranchConfiguration branchConfiguration; private final ScmRevision scmRevision; - private final ForkDateSupplier forkDateSupplier; private final InputComponentStore componentStore; private final ScmConfiguration scmConfiguration; public MetadataPublisher(ProjectInfo projectInfo, InputModuleHierarchy moduleHierarchy, QualityProfiles qProfiles, CpdSettings cpdSettings, ScannerPluginRepository pluginRepository, BranchConfiguration branchConfiguration, - ScmRevision scmRevision, ForkDateSupplier forkDateSupplier, InputComponentStore componentStore, ScmConfiguration scmConfiguration) { + ScmRevision scmRevision, InputComponentStore componentStore, ScmConfiguration scmConfiguration) { this.projectInfo = projectInfo; this.moduleHierarchy = moduleHierarchy; this.qProfiles = qProfiles; @@ -71,7 +68,6 @@ public class MetadataPublisher implements ReportPublisherStep { this.pluginRepository = pluginRepository; this.branchConfiguration = branchConfiguration; this.scmRevision = scmRevision; - this.forkDateSupplier = forkDateSupplier; this.componentStore = componentStore; this.scmConfiguration = scmConfiguration; } @@ -93,7 +89,6 @@ public class MetadataPublisher implements ReportPublisherStep { } addScmInformation(builder); - addForkPoint(builder); addNotAnalyzedFileCountsByLanguage(builder); for (QProfile qp : qProfiles.findAll()) { @@ -114,13 +109,6 @@ public class MetadataPublisher implements ReportPublisherStep { writer.writeMetadata(builder.build()); } - private void addForkPoint(ScannerReport.Metadata.Builder builder) { - Instant date = forkDateSupplier.get(); - if (date != null) { - builder.setForkDate(date.toEpochMilli()); - } - } - private void addModulesRelativePaths(ScannerReport.Metadata.Builder builder) { LinkedList queue = new LinkedList<>(); queue.add(moduleHierarchy.root()); diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ForkDateSupplier.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ReferenceBranchSupplier.java similarity index 62% rename from sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ForkDateSupplier.java rename to sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ReferenceBranchSupplier.java index 18180ab0f53..48c526c1e01 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ForkDateSupplier.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ReferenceBranchSupplier.java @@ -19,41 +19,33 @@ */ package org.sonar.scanner.repository; -import java.time.Instant; import javax.annotation.CheckForNull; import org.sonar.api.batch.fs.internal.DefaultInputProject; -import org.sonar.api.notifications.AnalysisWarnings; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.api.utils.log.Profiler; import org.sonar.scanner.scan.branch.BranchConfiguration; import org.sonar.scanner.scan.branch.ProjectBranches; -import org.sonar.scanner.scm.ScmConfiguration; import org.sonarqube.ws.NewCodePeriods; -public class ForkDateSupplier { - private static final Logger LOG = Loggers.get(ForkDateSupplier.class); +public class ReferenceBranchSupplier { + private static final Logger LOG = Loggers.get(ReferenceBranchSupplier.class); private static final String LOG_MSG_WS = "Load New Code definition"; private final NewCodePeriodLoader newCodePeriodLoader; private final BranchConfiguration branchConfiguration; private final DefaultInputProject project; - private final ScmConfiguration scmConfiguration; private final ProjectBranches branches; - private final AnalysisWarnings analysisWarnings; - public ForkDateSupplier(NewCodePeriodLoader newCodePeriodLoader, BranchConfiguration branchConfiguration, - DefaultInputProject project, ScmConfiguration scmConfiguration, ProjectBranches branches, AnalysisWarnings analysisWarnings) { + public ReferenceBranchSupplier(NewCodePeriodLoader newCodePeriodLoader, BranchConfiguration branchConfiguration, DefaultInputProject project, ProjectBranches branches) { this.newCodePeriodLoader = newCodePeriodLoader; this.branchConfiguration = branchConfiguration; this.project = project; - this.scmConfiguration = scmConfiguration; this.branches = branches; - this.analysisWarnings = analysisWarnings; } @CheckForNull - public Instant get() { + public String get() { // branches will be empty in CE if (branchConfiguration.isPullRequest() || branches.isEmpty()) { return null; @@ -73,20 +65,6 @@ public class ForkDateSupplier { return null; } - LOG.info("Computing New Code since fork with '{}'", referenceBranchName); - if (scmConfiguration.isDisabled() || scmConfiguration.provider() == null) { - LOG.warn("SCM provider is disabled. No New Code will be computed."); - analysisWarnings.addUnique("The scanner failed to compute New Code because no SCM provider was found. Please check your scanner logs."); - return null; - } - - Instant forkdate = scmConfiguration.provider().forkDate(referenceBranchName, project.getBaseDir()); - if (forkdate != null) { - LOG.debug("Fork detected at '{}'", referenceBranchName, forkdate); - } else { - analysisWarnings.addUnique("The scanner failed to compute New Code. Please check your scanner logs."); - LOG.warn("Failed to detect fork date. No New Code will be computed.", referenceBranchName); - } - return forkdate; + return referenceBranchName; } } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectScanContainer.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectScanContainer.java index 94c44b23a73..4df53478096 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectScanContainer.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectScanContainer.java @@ -89,7 +89,7 @@ import org.sonar.scanner.report.TestExecutionPublisher; import org.sonar.scanner.repository.ContextPropertiesCache; import org.sonar.scanner.repository.DefaultProjectRepositoriesLoader; import org.sonar.scanner.repository.DefaultQualityProfileLoader; -import org.sonar.scanner.repository.ForkDateSupplier; +import org.sonar.scanner.repository.ReferenceBranchSupplier; import org.sonar.scanner.repository.ProjectRepositoriesLoader; import org.sonar.scanner.repository.ProjectRepositoriesSupplier; import org.sonar.scanner.repository.QualityProfileLoader; @@ -230,7 +230,7 @@ public class ProjectScanContainer extends ComponentContainer { ProjectCoverageAndDuplicationExclusions.class, // Report - ForkDateSupplier.class, + ReferenceBranchSupplier.class, ScannerMetrics.class, ReportPublisher.class, AnalysisContextReportPublisher.class, diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitScmProvider.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitScmProvider.java index 1fb9d8eda24..d9157b5a3ce 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitScmProvider.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitScmProvider.java @@ -214,30 +214,6 @@ public class GitScmProvider extends ScmProvider { @Override @CheckForNull public Instant forkDate(String referenceBranchName, Path projectBaseDir) { - try (Repository repo = buildRepo(projectBaseDir)) { - Ref targetRef = resolveTargetRef(referenceBranchName, repo); - if (targetRef == null) { - LOG.warn("Branch '{}' not found in git", referenceBranchName); - return null; - } - - if (isDiffAlgoInvalid(repo.getConfig())) { - LOG.warn("The diff algorithm configured in git is not supported. " - + "No information regarding changes in the branch will be collected, which can lead to unexpected results."); - return null; - } - - Optional mergeBaseCommit = findMergeBase(repo, targetRef); - if (!mergeBaseCommit.isPresent()) { - LOG.warn("No fork point found between HEAD and " + targetRef.getName()); - return null; - } - - return Instant.ofEpochSecond(mergeBaseCommit.get().getCommitTime()); - } catch (Exception e) { - LOG.warn("Failed to find fork point with git", e); - } - return null; } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/FindFork.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/FindFork.java deleted file mode 100644 index 5a2ea83ad3b..00000000000 --- a/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/FindFork.java +++ /dev/null @@ -1,118 +0,0 @@ -/* - * 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.scm.svn; - -import java.io.File; -import java.nio.file.Path; -import java.time.Instant; -import java.util.Optional; -import javax.annotation.CheckForNull; -import org.sonar.api.scanner.ScannerSide; -import org.sonar.api.utils.log.Logger; -import org.sonar.api.utils.log.Loggers; -import org.tmatesoft.svn.core.ISVNLogEntryHandler; -import org.tmatesoft.svn.core.SVNException; -import org.tmatesoft.svn.core.SVNLogEntry; -import org.tmatesoft.svn.core.SVNLogEntryPath; -import org.tmatesoft.svn.core.wc.SVNClientManager; -import org.tmatesoft.svn.core.wc.SVNRevision; -import org.tmatesoft.svn.core.wc.SVNStatus; - -import static org.sonar.scm.svn.SvnScmSupport.newSvnClientManager; - -@ScannerSide -public class FindFork { - private static final Logger LOG = Loggers.get(FindFork.class); - - private final SvnConfiguration configuration; - - public FindFork(SvnConfiguration configuration) { - this.configuration = configuration; - } - - @CheckForNull - public Instant findDate(Path location, String referenceBranch) throws SVNException { - ForkPoint forkPoint = find(location, referenceBranch); - if (forkPoint != null) { - return forkPoint.date(); - } - return null; - } - - @CheckForNull - public ForkPoint find(Path location, String referenceBranch) throws SVNException { - SVNClientManager clientManager = newSvnClientManager(configuration); - SVNRevision revision = getSvnRevision(location, clientManager); - LOG.debug("latest revision is " + revision); - String svnRefBranch = "/" + referenceBranch; - - SVNLogEntryHolder handler = new SVNLogEntryHolder(); - SVNRevision endRevision = SVNRevision.create(1); - SVNRevision startRevision = SVNRevision.create(revision.getNumber()); - - do { - clientManager.getLogClient().doLog(new File[] {location.toFile()}, startRevision, endRevision, true, true, -1, handler); - SVNLogEntry lastEntry = handler.getLastEntry(); - Optional copyFromReference = lastEntry.getChangedPaths().values().stream() - .filter(e -> e.getCopyPath() != null && e.getCopyPath().equals(svnRefBranch)) - .findFirst(); - - if (copyFromReference.isPresent()) { - return new ForkPoint(String.valueOf(copyFromReference.get().getCopyRevision()), Instant.ofEpochMilli(lastEntry.getDate().getTime())); - } - - if (lastEntry.getChangedPaths().isEmpty()) { - // shouldn't happen since it should only stop in revisions with changed paths - return null; - } - - SVNLogEntryPath firstChangedPath = lastEntry.getChangedPaths().values().iterator().next(); - if (firstChangedPath.getCopyPath() == null) { - // we walked the history to the root, and the last commit found had no copy reference. Must be the trunk, there is no fork point - return null; - } - - // TODO Looks like a revision can have multiple changed paths. Should we iterate through all of them? - startRevision = SVNRevision.create(firstChangedPath.getCopyRevision()); - } while (true); - - } - - private static SVNRevision getSvnRevision(Path location, SVNClientManager clientManager) throws SVNException { - SVNStatus svnStatus = clientManager.getStatusClient().doStatus(location.toFile(), false); - return svnStatus.getRevision(); - } - - /** - * Handler keeping only the last entry, and count how many entries have been seen. - */ - private static class SVNLogEntryHolder implements ISVNLogEntryHandler { - SVNLogEntry value; - - public SVNLogEntry getLastEntry() { - return value; - } - - @Override - public void handleLogEntry(SVNLogEntry svnLogEntry) { - this.value = svnLogEntry; - } - } -} diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnScmProvider.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnScmProvider.java index 535ee8b2993..57bdbd4edd3 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnScmProvider.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnScmProvider.java @@ -54,12 +54,10 @@ public class SvnScmProvider extends ScmProvider { private final SvnConfiguration configuration; private final SvnBlameCommand blameCommand; - private final FindFork findFork; - public SvnScmProvider(SvnConfiguration configuration, SvnBlameCommand blameCommand, FindFork findFork) { + public SvnScmProvider(SvnConfiguration configuration, SvnBlameCommand blameCommand) { this.configuration = configuration; this.blameCommand = blameCommand; - this.findFork = findFork; } @Override @@ -202,12 +200,7 @@ public class SvnScmProvider extends ScmProvider { @CheckForNull @Override public Instant forkDate(String referenceBranch, Path rootBaseDir) { - try { - return findFork.findDate(rootBaseDir, referenceBranch); - } catch (SVNException e) { - LOG.warn("Unable to find fork date with '" + referenceBranch + "'", e); - return null; - } + return null; } ChangedLinesComputer newChangedLinesComputer(Path rootBaseDir, Set changedFiles) { diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnScmSupport.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnScmSupport.java index 8a664d1f0ef..9ee3f8fb4c6 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnScmSupport.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnScmSupport.java @@ -55,8 +55,7 @@ public class SvnScmSupport { public static List getObjects() { return Arrays.asList(SvnScmProvider.class, SvnBlameCommand.class, - SvnConfiguration.class, - FindFork.class + SvnConfiguration.class ); } } diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java index f19917876e4..e13e116c0d2 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java @@ -39,6 +39,7 @@ import org.sonar.api.utils.log.LogTester; import org.sonar.scanner.fs.InputModuleHierarchy; import org.sonar.scanner.protocol.output.ScannerReportReader; import org.sonar.scanner.protocol.output.ScannerReportWriter; +import org.sonar.scanner.repository.ReferenceBranchSupplier; import org.sonar.scanner.scan.branch.BranchConfiguration; import org.sonar.scanner.scan.filesystem.InputComponentStore; import org.sonar.scanner.scm.ScmConfiguration; @@ -57,6 +58,7 @@ public class ChangedLinesPublisherTest { private InputModuleHierarchy inputModuleHierarchy = mock(InputModuleHierarchy.class); private InputComponentStore inputComponentStore = mock(InputComponentStore.class); private BranchConfiguration branchConfiguration = mock(BranchConfiguration.class); + private ReferenceBranchSupplier referenceBranchSupplier = mock(ReferenceBranchSupplier.class); private ScannerReportWriter writer; private ScmProvider provider = mock(ScmProvider.class); private DefaultInputProject project = mock(DefaultInputProject.class); @@ -67,7 +69,7 @@ public class ChangedLinesPublisherTest { @Rule public LogTester logTester = new LogTester(); - private ChangedLinesPublisher publisher = new ChangedLinesPublisher(scmConfiguration, project, inputComponentStore, branchConfiguration); + private ChangedLinesPublisher publisher = new ChangedLinesPublisher(scmConfiguration, project, inputComponentStore, branchConfiguration, referenceBranchSupplier); @Before public void setUp() { diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/MetadataPublisherTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/MetadataPublisherTest.java index a7bda1c9992..b7f8356ac46 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/MetadataPublisherTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/MetadataPublisherTest.java @@ -50,7 +50,7 @@ import org.sonar.scanner.fs.InputModuleHierarchy; import org.sonar.scanner.protocol.output.ScannerReport; import org.sonar.scanner.protocol.output.ScannerReportReader; import org.sonar.scanner.protocol.output.ScannerReportWriter; -import org.sonar.scanner.repository.ForkDateSupplier; +import org.sonar.scanner.repository.ReferenceBranchSupplier; import org.sonar.scanner.rule.QProfile; import org.sonar.scanner.rule.QualityProfiles; import org.sonar.scanner.scan.branch.BranchConfiguration; @@ -77,7 +77,7 @@ public class MetadataPublisherTest { private final QualityProfiles qProfiles = mock(QualityProfiles.class); private final ProjectInfo projectInfo = mock(ProjectInfo.class); private final CpdSettings cpdSettings = mock(CpdSettings.class); - private final ForkDateSupplier forkDateSupplier = mock(ForkDateSupplier.class); + private final ReferenceBranchSupplier referenceBranchSupplier = mock(ReferenceBranchSupplier.class); private final ScannerPluginRepository pluginRepository = mock(ScannerPluginRepository.class); private BranchConfiguration branches; private ScmConfiguration scmConfiguration; @@ -116,13 +116,12 @@ public class MetadataPublisherTest { scmConfiguration = mock(ScmConfiguration.class); when(scmConfiguration.provider()).thenReturn(scmProvider); underTest = new MetadataPublisher(projectInfo, inputModuleHierarchy, qProfiles, cpdSettings, - pluginRepository, branches, scmRevision, forkDateSupplier, componentStore, scmConfiguration); + pluginRepository, branches, scmRevision, componentStore, scmConfiguration); } @Test public void write_metadata() throws Exception { Date date = new Date(); - when(forkDateSupplier.get()).thenReturn(Instant.ofEpochMilli(123456789L)); when(qProfiles.findAll()).thenReturn(Collections.singletonList(new QProfile("q1", "Q1", "java", date))); when(pluginRepository.getPluginsByKey()).thenReturn(ImmutableMap.of( "java", new ScannerPlugin("java", 12345L, null), @@ -134,7 +133,6 @@ public class MetadataPublisherTest { ScannerReportReader reader = new ScannerReportReader(outputDir); ScannerReport.Metadata metadata = reader.readMetadata(); - assertThat(metadata.getForkDate()).isEqualTo(123456789L); assertThat(metadata.getAnalysisDate()).isEqualTo(1234567L); assertThat(metadata.getProjectKey()).isEqualTo("root"); assertThat(metadata.getModulesProjectRelativePathByKeyMap()).containsOnly(entry("module", "modulePath"), entry("root", "")); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/ForkDateSupplierTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/ReferenceBranchSupplierTest.java similarity index 64% rename from sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/ForkDateSupplierTest.java rename to sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/ReferenceBranchSupplierTest.java index 3ad2ebc69b4..4804d62e01c 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/ForkDateSupplierTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/ReferenceBranchSupplierTest.java @@ -42,82 +42,53 @@ import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; -public class ForkDateSupplierTest { +public class ReferenceBranchSupplierTest { private final static String PROJECT_KEY = "project"; private final static String BRANCH_KEY = "branch"; private final static Path BASE_DIR = Paths.get("root"); - private NewCodePeriodLoader newCodePeriodLoader = mock(NewCodePeriodLoader.class); - private BranchConfiguration branchConfiguration = mock(BranchConfiguration.class); - private DefaultInputProject project = mock(DefaultInputProject.class); - private ScmConfiguration scmConfiguration = mock(ScmConfiguration.class); - private ScmProvider scmProvider = mock(ScmProvider.class); - private ProjectBranches projectBranches = mock(ProjectBranches.class); - private AnalysisWarnings analysisWarnings = mock(AnalysisWarnings.class); - private ForkDateSupplier forkDateSupplier = new ForkDateSupplier(newCodePeriodLoader, branchConfiguration, project, scmConfiguration, projectBranches, analysisWarnings); + private final NewCodePeriodLoader newCodePeriodLoader = mock(NewCodePeriodLoader.class); + private final BranchConfiguration branchConfiguration = mock(BranchConfiguration.class); + private final DefaultInputProject project = mock(DefaultInputProject.class); + private final ProjectBranches projectBranches = mock(ProjectBranches.class); + private final ReferenceBranchSupplier referenceBranchSupplier = new ReferenceBranchSupplier(newCodePeriodLoader, branchConfiguration, project, projectBranches); @Before public void setUp() { when(projectBranches.isEmpty()).thenReturn(false); when(project.key()).thenReturn(PROJECT_KEY); when(project.getBaseDir()).thenReturn(BASE_DIR); - when(scmConfiguration.isDisabled()).thenReturn(false); - when(scmConfiguration.provider()).thenReturn(scmProvider); } @Test - public void returns_forkDate_for_branches_with_ref() { - Instant date = Instant.now(); + public void returns_reference_branch_when_set() { when(branchConfiguration.branchType()).thenReturn(BranchType.BRANCH); when(branchConfiguration.branchName()).thenReturn(BRANCH_KEY); - when(scmProvider.forkDate("master", BASE_DIR)).thenReturn(date); when(newCodePeriodLoader.load(PROJECT_KEY, BRANCH_KEY)).thenReturn(createResponse(NewCodePeriods.NewCodePeriodType.REFERENCE_BRANCH, "master")); - assertThat(forkDateSupplier.get()).isEqualTo(date); + assertThat(referenceBranchSupplier.get()).isEqualTo("master"); } @Test public void uses_default_branch_if_no_branch_specified() { - Instant date = Instant.now(); when(branchConfiguration.branchType()).thenReturn(BranchType.BRANCH); when(branchConfiguration.branchName()).thenReturn(null); when(projectBranches.defaultBranchName()).thenReturn("default"); when(newCodePeriodLoader.load(PROJECT_KEY, "default")).thenReturn(createResponse(NewCodePeriods.NewCodePeriodType.REFERENCE_BRANCH, "master")); - when(scmProvider.forkDate("master", BASE_DIR)).thenReturn(date); - assertThat(forkDateSupplier.get()).isEqualTo(date); - - verifyNoInteractions(analysisWarnings); + assertThat(referenceBranchSupplier.get()).isEqualTo("master"); } @Test public void returns_null_if_no_branches() { when(projectBranches.isEmpty()).thenReturn(true); - assertThat(forkDateSupplier.get()).isNull(); + assertThat(referenceBranchSupplier.get()).isNull(); verify(branchConfiguration).isPullRequest(); verify(projectBranches).isEmpty(); verifyNoMoreInteractions(branchConfiguration); - verifyNoInteractions(scmConfiguration, scmProvider, analysisWarnings, newCodePeriodLoader); - } - - @Test - public void returns_null_if_scm_disabled() { - when(branchConfiguration.branchType()).thenReturn(BranchType.BRANCH); - when(branchConfiguration.branchName()).thenReturn(BRANCH_KEY); - when(scmConfiguration.isDisabled()).thenReturn(true); - when(newCodePeriodLoader.load(PROJECT_KEY, BRANCH_KEY)).thenReturn(createResponse(NewCodePeriods.NewCodePeriodType.REFERENCE_BRANCH, "master")); - - assertThat(forkDateSupplier.get()).isNull(); - - verify(scmConfiguration).isDisabled(); - verify(branchConfiguration, times(2)).branchName(); - verify(branchConfiguration).isPullRequest(); - verify(analysisWarnings).addUnique(anyString()); - - verifyNoInteractions(scmProvider); - verifyNoMoreInteractions(branchConfiguration); + verifyNoInteractions(newCodePeriodLoader); } @Test @@ -126,24 +97,23 @@ public class ForkDateSupplierTest { when(branchConfiguration.branchName()).thenReturn(BRANCH_KEY); when(newCodePeriodLoader.load(PROJECT_KEY, BRANCH_KEY)).thenReturn(createResponse(NewCodePeriods.NewCodePeriodType.REFERENCE_BRANCH, BRANCH_KEY)); - assertThat(forkDateSupplier.get()).isNull(); + assertThat(referenceBranchSupplier.get()).isNull(); verify(branchConfiguration, times(2)).branchName(); verify(branchConfiguration).isPullRequest(); verify(newCodePeriodLoader).load(PROJECT_KEY, BRANCH_KEY); - verifyNoInteractions(scmProvider, analysisWarnings, scmConfiguration); verifyNoMoreInteractions(branchConfiguration); } @Test public void returns_null_if_pull_request() { when(branchConfiguration.isPullRequest()).thenReturn(true); - assertThat(forkDateSupplier.get()).isNull(); + assertThat(referenceBranchSupplier.get()).isNull(); verify(branchConfiguration).isPullRequest(); - verifyNoInteractions(newCodePeriodLoader, analysisWarnings, scmProvider, scmConfiguration); + verifyNoInteractions(newCodePeriodLoader); verifyNoMoreInteractions(branchConfiguration); } @@ -153,9 +123,7 @@ public class ForkDateSupplierTest { when(branchConfiguration.branchName()).thenReturn(BRANCH_KEY); when(newCodePeriodLoader.load(PROJECT_KEY, BRANCH_KEY)).thenReturn(createResponse(NewCodePeriods.NewCodePeriodType.NUMBER_OF_DAYS, "2")); - assertThat(forkDateSupplier.get()).isNull(); - - verifyNoInteractions(scmProvider, analysisWarnings, scmConfiguration); + assertThat(referenceBranchSupplier.get()).isNull(); } private NewCodePeriods.ShowWSResponse createResponse(NewCodePeriods.NewCodePeriodType type, String value) { diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitScmProviderTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitScmProviderTest.java index a3c9f88a0e0..daac3162570 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitScmProviderTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitScmProviderTest.java @@ -316,84 +316,7 @@ public class GitScmProviderTest { } @Test - public void forkDate_from_diverged() throws IOException, GitAPIException { - createAndCommitFile("file-m1.xoo", Instant.now().minus(8, ChronoUnit.DAYS)); - createAndCommitFile("file-m2.xoo", Instant.now().minus(7, ChronoUnit.DAYS)); - Instant expectedForkDate = Instant.now().minus(6, ChronoUnit.DAYS); - createAndCommitFile("file-m3.xoo", expectedForkDate); - ObjectId forkPoint = git.getRepository().exactRef("HEAD").getObjectId(); - - appendToAndCommitFile("file-m3.xoo"); - createAndCommitFile("file-m4.xoo"); - - git.branchCreate().setName("b1").setStartPoint(forkPoint.getName()).call(); - git.checkout().setName("b1").call(); - createAndCommitFile("file-b1.xoo"); - appendToAndCommitFile("file-m1.xoo"); - deleteAndCommitFile("file-m2.xoo"); - - assertThat(newScmProvider().forkDate("master", worktree)) - .isEqualTo(expectedForkDate.truncatedTo(ChronoUnit.SECONDS)); - } - - @Test - public void forkDate_should_not_fail_if_reference_is_the_same_branch() throws IOException, GitAPIException { - createAndCommitFile("file-m1.xoo", Instant.now().minus(8, ChronoUnit.DAYS)); - createAndCommitFile("file-m2.xoo", Instant.now().minus(7, ChronoUnit.DAYS)); - - ObjectId forkPoint = git.getRepository().exactRef("HEAD").getObjectId(); - git.branchCreate().setName("b1").setStartPoint(forkPoint.getName()).call(); - git.checkout().setName("b1").call(); - - Instant expectedForkDate = Instant.now().minus(6, ChronoUnit.DAYS); - createAndCommitFile("file-m3.xoo", expectedForkDate); - - assertThat(newScmProvider().forkDate("b1", worktree)) - .isEqualTo(expectedForkDate.truncatedTo(ChronoUnit.SECONDS)); - } - - @Test - public void forkDate_should_not_fail_with_patience_diff_algo() throws IOException { - Path gitConfig = worktree.resolve(".git").resolve("config"); - Files.write(gitConfig, "[diff]\nalgorithm = patience\n".getBytes(StandardCharsets.UTF_8)); - Repository repo = FileRepositoryBuilder.create(worktree.resolve(".git").toFile()); - git = new Git(repo); - - assertThat(newScmProvider().forkDate("master", worktree)).isNull(); - } - - @Test - public void forkDate_should_not_fail_with_invalid_basedir() throws IOException { - assertThat(newScmProvider().forkDate("master", temp.newFolder().toPath())).isNull(); - } - - @Test - public void forkDate_should_not_fail_when_no_merge_base_is_found() throws IOException, GitAPIException { - createAndCommitFile("file-m1.xoo", Instant.now().minus(8, ChronoUnit.DAYS)); - - git.checkout().setOrphan(true).setName("b1").call(); - createAndCommitFile("file-b1.xoo"); - - assertThat(newScmProvider().forkDate("master", worktree)).isNull(); - } - - @Test - public void forkDate_without_target_branch() throws IOException, GitAPIException { - createAndCommitFile("file-m1.xoo", Instant.now().minus(8, ChronoUnit.DAYS)); - createAndCommitFile("file-m2.xoo", Instant.now().minus(7, ChronoUnit.DAYS)); - Instant expectedForkDate = Instant.now().minus(6, ChronoUnit.DAYS); - createAndCommitFile("file-m3.xoo", expectedForkDate); - ObjectId forkPoint = git.getRepository().exactRef("HEAD").getObjectId(); - - appendToAndCommitFile("file-m3.xoo"); - createAndCommitFile("file-m4.xoo"); - - git.branchCreate().setName("b1").setStartPoint(forkPoint.getName()).call(); - git.checkout().setName("b1").call(); - createAndCommitFile("file-b1.xoo"); - appendToAndCommitFile("file-m1.xoo"); - deleteAndCommitFile("file-m2.xoo"); - + public void forkDate_returns_null() { assertThat(newScmProvider().forkDate("unknown", worktree)).isNull(); } diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/FindForkTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/FindForkTest.java deleted file mode 100644 index 7f7ae16ee1a..00000000000 --- a/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/FindForkTest.java +++ /dev/null @@ -1,143 +0,0 @@ -/* - * 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.scm.svn; - -import java.io.IOException; -import java.nio.file.Path; -import java.nio.file.Paths; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.ClassRule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; -import org.tmatesoft.svn.core.SVNException; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; - -public class FindForkTest { - - @ClassRule - public static TemporaryFolder temp = new TemporaryFolder(); - - private static SvnTester svnTester; - - private static Path trunk; - private static Path b1; - private static Path b2; - - private FindFork findFork; - - @BeforeClass - public static void before() throws IOException, SVNException { - svnTester = new SvnTester(temp.newFolder().toPath()); - - trunk = temp.newFolder("trunk").toPath(); - svnTester.checkout(trunk, "trunk"); - createAndCommitFile(trunk, "file-1-commit-in-trunk.xoo"); - createAndCommitFile(trunk, "file-2-commit-in-trunk.xoo"); - createAndCommitFile(trunk, "file-3-commit-in-trunk.xoo"); - svnTester.checkout(trunk, "trunk"); - - svnTester.createBranch("b1"); - b1 = temp.newFolder("branches", "b1").toPath(); - svnTester.checkout(b1, "branches/b1"); - createAndCommitFile(b1, "file-1-commit-in-b1.xoo"); - createAndCommitFile(b1, "file-2-commit-in-b1.xoo"); - createAndCommitFile(b1, "file-3-commit-in-b1.xoo"); - svnTester.checkout(b1, "branches/b1"); - - svnTester.createBranch("branches/b1", "b2"); - b2 = temp.newFolder("branches", "b2").toPath(); - svnTester.checkout(b2, "branches/b2"); - - createAndCommitFile(b2, "file-1-commit-in-b2.xoo"); - createAndCommitFile(b2, "file-2-commit-in-b2.xoo"); - createAndCommitFile(b2, "file-3-commit-in-b2.xoo"); - svnTester.checkout(b2, "branches/b2"); - } - - @Before - public void setUp() { - SvnConfiguration configurationMock = mock(SvnConfiguration.class); - findFork = new FindFork(configurationMock); - } - - @Test - public void testEmptyBranch() throws SVNException, IOException { - svnTester.createBranch("empty"); - Path empty = temp.newFolder("branches", "empty").toPath(); - - svnTester.checkout(empty, "branches/empty"); - ForkPoint forkPoint = findFork.find(empty, "unknown"); - assertThat(forkPoint).isNull(); - } - - @Test - public void returnNoDate() throws SVNException { - FindFork findFork = new FindFork(mock(SvnConfiguration.class)) { - @Override - public ForkPoint find(Path location, String referenceBranch) { - return null; - } - }; - - assertThat(findFork.findDate(Paths.get(""), "branch")).isNull(); - } - - @Test - public void testTrunk() throws SVNException { - ForkPoint forkPoint = findFork.find(trunk, "unknown"); - assertThat(forkPoint).isNull(); - } - - @Test - public void testB1() throws SVNException { - ForkPoint forkPoint = findFork.find(b1, "trunk"); - assertThat(forkPoint.commit()).isEqualTo("5"); - } - - @Test - public void testB2() throws SVNException { - ForkPoint forkPoint = findFork.find(b2, "branches/b1"); - assertThat(forkPoint.commit()).isEqualTo("9"); - } - - @Test - public void testB2Date() throws SVNException { - assertThat(findFork.findDate(b2, "branches/b1")).isNotNull(); - } - - @Test - public void testB2FromTrunk() throws SVNException { - ForkPoint forkPoint = findFork.find(b2, "trunk"); - assertThat(forkPoint.commit()).isEqualTo("5"); - } - - private static void createAndCommitFile(Path worktree, String filename, String content) throws IOException, SVNException { - svnTester.createFile(worktree, filename, content); - svnTester.add(worktree, filename); - svnTester.commit(worktree); - } - - private static void createAndCommitFile(Path worktree, String filename) throws IOException, SVNException { - createAndCommitFile(worktree, filename, filename + "\n"); - } -} diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/SvnScmProviderTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/SvnScmProviderTest.java index ae10963a47d..0a5c3c6ce19 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/SvnScmProviderTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/SvnScmProviderTest.java @@ -24,7 +24,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.time.Instant; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -36,7 +35,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.sonar.api.batch.scm.ScmProvider; -import org.tmatesoft.svn.core.SVNCancelException; import org.tmatesoft.svn.core.SVNException; import org.tmatesoft.svn.core.SVNURL; import org.tmatesoft.svn.core.wc.SVNClientManager; @@ -82,7 +80,6 @@ public class SvnScmProviderTest { @Rule public TemporaryFolder temp = new TemporaryFolder(); - private FindFork findFork = mock(FindFork.class); private SvnConfiguration config = mock(SvnConfiguration.class); private SvnTester svnTester; @@ -98,7 +95,7 @@ public class SvnScmProviderTest { @Test public void sanityCheck() { SvnBlameCommand blameCommand = new SvnBlameCommand(config); - SvnScmProvider svnScmProvider = new SvnScmProvider(config, blameCommand, findFork); + SvnScmProvider svnScmProvider = new SvnScmProvider(config, blameCommand); assertThat(svnScmProvider.key()).isEqualTo("svn"); assertThat(svnScmProvider.blameCommand()).isEqualTo(blameCommand); } @@ -231,7 +228,7 @@ public class SvnScmProviderTest { svnTester.createBranch("b1"); svnTester.checkout(b1, "branches/b1"); - SvnScmProvider scmProvider = new SvnScmProvider(config, new SvnBlameCommand(config), findFork) { + SvnScmProvider scmProvider = new SvnScmProvider(config, new SvnBlameCommand(config)) { @Override ChangedLinesComputer newChangedLinesComputer(Path rootBaseDir, Set changedFiles) { throw new IllegalStateException("crash"); @@ -241,29 +238,9 @@ public class SvnScmProviderTest { } @Test - public void forkDate_returns_null_if_no_fork_found() { - assertThat(new SvnScmProvider(config, new SvnBlameCommand(config), findFork).forkDate("branch", Paths.get(""))).isNull(); - } - - @Test - public void forkDate_returns_instant_if_fork_found() throws SVNException { - Path rootBaseDir = Paths.get(""); - String referenceBranch = "branch"; - Instant forkDate = Instant.ofEpochMilli(123456789L); - SvnScmProvider provider = new SvnScmProvider(config, new SvnBlameCommand(config), findFork); - when(findFork.findDate(rootBaseDir, referenceBranch)).thenReturn(forkDate); - - assertThat(provider.forkDate(referenceBranch, rootBaseDir)).isEqualTo(forkDate); - } - - @Test - public void forkDate_returns_null_if_exception_occurs() throws SVNException { - Path rootBaseDir = Paths.get(""); - String referenceBranch = "branch"; - SvnScmProvider provider = new SvnScmProvider(config, new SvnBlameCommand(config), findFork); - when(findFork.findDate(rootBaseDir, referenceBranch)).thenThrow(new SVNCancelException()); - - assertThat(provider.forkDate(referenceBranch, rootBaseDir)).isNull(); + public void forkDate_returns_null() throws SVNException { + SvnScmProvider provider = new SvnScmProvider(config, new SvnBlameCommand(config)); + assertThat(provider.forkDate("", Paths.get(""))).isNull(); } @Test @@ -305,7 +282,7 @@ public class SvnScmProviderTest { svnTester.commit(worktree); } - private void deleteAndCommitFile(Path worktree, String filename) throws IOException, SVNException { + private void deleteAndCommitFile(Path worktree, String filename) throws SVNException { svnTester.deleteFile(worktree, filename); svnTester.commit(worktree); } @@ -316,6 +293,6 @@ public class SvnScmProviderTest { } private SvnScmProvider newScmProvider() { - return new SvnScmProvider(config, new SvnBlameCommand(config), findFork); + return new SvnScmProvider(config, new SvnBlameCommand(config)); } } diff --git a/sonar-scanner-protocol/src/main/protobuf/scanner_report.proto b/sonar-scanner-protocol/src/main/protobuf/scanner_report.proto index cf13d10f9d1..a6fc6398869 100644 --- a/sonar-scanner-protocol/src/main/protobuf/scanner_report.proto +++ b/sonar-scanner-protocol/src/main/protobuf/scanner_report.proto @@ -56,7 +56,7 @@ message Metadata { string target_branch_name = 18; - int64 forkDate = 19; + reserved 19; // forkDate (no longer used) map not_analyzed_files_by_language = 20; -- 2.39.5