@@ -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. | |||
*/ |
@@ -48,7 +48,6 @@ public class AnalysisMetadataHolderImpl implements MutableAnalysisMetadataHolder | |||
private final InitializedProperty<Map<String, QualityProfile>> qProfilesPerLanguage = new InitializedProperty<>(); | |||
private final InitializedProperty<Map<String, ScannerPlugin>> pluginsByKey = new InitializedProperty<>(); | |||
private final InitializedProperty<String> scmRevision = new InitializedProperty<>(); | |||
private final InitializedProperty<Long> 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; |
@@ -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 | |||
*/ |
@@ -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, |
@@ -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<String, Counters> 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; |
@@ -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())); | |||
} |
@@ -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<String, NewEffortCounter> 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; | |||
} | |||
} |
@@ -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<Set<Integer>> newLinesOpt = newLinesRepository.getNewLines(component); | |||
if (newLinesOpt.isEmpty()) { | |||
return false; | |||
} | |||
Set<Integer> newLines = newLinesOpt.get(); | |||
return IssueLocations.allLinesFor(issue, component.getUuid()).anyMatch(newLines::contains); | |||
} | |||
} |
@@ -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) { |
@@ -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<String, String> 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<BranchDto> 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<ComponentDto> 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); | |||
} | |||
} |
@@ -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<String, Metric> 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<Counter> 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<Counter> path) { | |||
@@ -138,21 +134,24 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitor extends PathAwareVis | |||
} | |||
} | |||
static final class Counter { | |||
private Map<String, RatingValue> newRatingValueByMetric = ImmutableMap.of( | |||
static class Counter { | |||
private final Map<String, RatingValue> 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<NewReliabilityAndSecurityRatingMeasuresVisitor.Counter> { | |||
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); | |||
} | |||
} | |||
} |
@@ -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<Se | |||
private final ComponentIssuesRepository componentIssuesRepository; | |||
private final MeasureRepository measureRepository; | |||
private final PeriodHolder periodHolder; | |||
private final AnalysisMetadataHolder analysisMetadataHolder; | |||
private final Metric newSecurityReviewRatingMetric; | |||
private final Metric newSecurityHotspotsReviewedMetric; | |||
private final Metric newSecurityHotspotsReviewedStatusMetric; | |||
private final Metric newSecurityHotspotsToReviewStatusMetric; | |||
private final NewIssueClassifier newIssueClassifier; | |||
public NewSecurityReviewMeasuresVisitor(ComponentIssuesRepository componentIssuesRepository, MeasureRepository measureRepository, PeriodHolder periodHolder, | |||
AnalysisMetadataHolder analysisMetadataHolder, MetricRepository metricRepository) { | |||
public NewSecurityReviewMeasuresVisitor(ComponentIssuesRepository componentIssuesRepository, MeasureRepository measureRepository, MetricRepository metricRepository, | |||
NewIssueClassifier newIssueClassifier) { | |||
super(FILE, POST_ORDER, NewSecurityReviewMeasuresVisitor.CounterFactory.INSTANCE); | |||
this.componentIssuesRepository = componentIssuesRepository; | |||
this.measureRepository = measureRepository; | |||
this.periodHolder = periodHolder; | |||
this.analysisMetadataHolder = analysisMetadataHolder; | |||
this.newSecurityReviewRatingMetric = metricRepository.getByKey(NEW_SECURITY_REVIEW_RATING_KEY); | |||
this.newSecurityHotspotsReviewedMetric = metricRepository.getByKey(NEW_SECURITY_HOTSPOTS_REVIEWED_KEY); | |||
this.newSecurityHotspotsReviewedStatusMetric = metricRepository.getByKey(NEW_SECURITY_HOTSPOTS_REVIEWED_STATUS_KEY); | |||
this.newSecurityHotspotsToReviewStatusMetric = metricRepository.getByKey(NEW_SECURITY_HOTSPOTS_TO_REVIEW_STATUS_KEY); | |||
this.newIssueClassifier = newIssueClassifier; | |||
} | |||
@Override | |||
public void visitProject(Component project, Path<SecurityReviewCounter> 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<Se | |||
} | |||
private void computeMeasure(Component component, Path<SecurityReviewCounter> 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<Double> percent = computePercent(path.current().getHotspotsToReview(), path.current().getHotspotsReviewed()); |
@@ -49,7 +49,7 @@ public class ScmInfoDbLoader { | |||
public Optional<DbScmInfo> getScmInfo(Component file) { | |||
Optional<String> uuid = getFileUUid(file); | |||
if (!uuid.isPresent()) { | |||
if (uuid.isEmpty()) { | |||
return Optional.empty(); | |||
} | |||
@@ -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<Set<Integer>> 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<Set<Integer>> 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<Set<Integer>> computeNewLinesFromScm(Component component) { | |||
if (!periodHolder.hasPeriodDate() && !analysisMetadataHolder.isPullRequest()) { | |||
return Optional.empty(); | |||
} | |||
Optional<ScmInfo> scmInfoOpt = scmInfoRepository.getScmInfo(component); | |||
if (!scmInfoOpt.isPresent()) { | |||
if (scmInfoOpt.isEmpty()) { | |||
return Optional.empty(); | |||
} | |||
@@ -80,16 +79,20 @@ public class NewLinesRepository { | |||
Set<Integer> 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<allChangesets.length; i++) { | |||
long referenceDate = useAnalysisDateAsReferenceDate() ? (analysisMetadataHolder.getAnalysisDate() - 1) : periodHolder.getPeriod().getDate(); | |||
for (int i = 0; i < allChangesets.length; i++) { | |||
if (isLineInPeriod(allChangesets[i].getDate(), referenceDate)) { | |||
lines.add(i+1); | |||
lines.add(i + 1); | |||
} | |||
} | |||
return Optional.of(lines); | |||
} | |||
private boolean useAnalysisDateAsReferenceDate() { | |||
return analysisMetadataHolder.isPullRequest() || NewCodePeriodType.REFERENCE_BRANCH.name().equals(periodHolder.getPeriod().getMode()); | |||
} | |||
/** | |||
* A line belongs to a Period if its date is older than the SNAPSHOT's date of the period. | |||
*/ | |||
@@ -98,13 +101,17 @@ public class NewLinesRepository { | |||
} | |||
private Optional<Set<Integer>> 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<Set<Integer>> readFromReport(Component file) { | |||
return reportReader.readComponentChangedLines(file.getReportAttributes().getRef()) | |||
.map(c -> new HashSet<>(c.getLineList())); |
@@ -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<MovedFilesRepository.OriginalFile> originalFile = movedFilesRepository.getOriginalFile(component); | |||
uuid = originalFile.map(MovedFilesRepository.OriginalFile::getUuid).orElse(component.getUuid()); |
@@ -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()); |
@@ -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; |
@@ -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); | |||
} |
@@ -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) { |
@@ -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() { |
@@ -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; | |||
} | |||
} |
@@ -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); | |||
} | |||
} |
@@ -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); |
@@ -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<Double> 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; | |||
} | |||
} |
@@ -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<Set<Integer>> 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 |
@@ -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); |
@@ -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); |
@@ -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( |
@@ -38,7 +38,6 @@ public class AnalysisMetadataHolderRule extends ExternalResource implements Muta | |||
private final InitializedProperty<String> uuid = new InitializedProperty<>(); | |||
private final InitializedProperty<Long> analysisDate = new InitializedProperty<>(); | |||
private final InitializedProperty<Long> forkDate = new InitializedProperty<>(); | |||
private final InitializedProperty<Analysis> baseAnalysis = new InitializedProperty<>(); | |||
private final InitializedProperty<Boolean> crossProjectDuplicationEnabled = new InitializedProperty<>(); | |||
private final InitializedProperty<Branch> 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(); |
@@ -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(); |
@@ -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; | |||
} |
@@ -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<String> 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<String> 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<Integer> 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 { |
@@ -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<DefaultInputModule> queue = new LinkedList<>(); | |||
queue.add(moduleHierarchy.root()); |
@@ -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; | |||
} | |||
} |
@@ -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, |
@@ -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<RevCommit> 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; | |||
} | |||
@@ -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<SVNLogEntryPath> 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; | |||
} | |||
} | |||
} |
@@ -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<Path> changedFiles) { |
@@ -55,8 +55,7 @@ public class SvnScmSupport { | |||
public static List<Object> getObjects() { | |||
return Arrays.asList(SvnScmProvider.class, | |||
SvnBlameCommand.class, | |||
SvnConfiguration.class, | |||
FindFork.class | |||
SvnConfiguration.class | |||
); | |||
} | |||
} |
@@ -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() { |
@@ -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", "")); |
@@ -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) { |
@@ -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(); | |||
} | |||
@@ -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"); | |||
} | |||
} |
@@ -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<Path> 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)); | |||
} | |||
} |
@@ -56,7 +56,7 @@ message Metadata { | |||
string target_branch_name = 18; | |||
int64 forkDate = 19; | |||
reserved 19; // forkDate (no longer used) | |||
map<string, int32> not_analyzed_files_by_language = 20; | |||