From: Duarte Meneses Date: Thu, 14 Feb 2019 21:43:26 +0000 (-0600) Subject: SONAR-11707 Create new metrics for issue count and fix ITs X-Git-Tag: 7.7~82 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=b0eda21c2748bb7df7d32768fea264a5122a221d;p=sonarqube.git SONAR-11707 Create new metrics for issue count and fix ITs --- diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCounter.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCounter.java index 6f33a516d07..1cb8d4fd058 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCounter.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCounter.java @@ -28,6 +28,7 @@ import java.util.Map; import javax.annotation.Nullable; import org.sonar.api.measures.CoreMetrics; 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; @@ -104,15 +105,17 @@ public class IssueCounter extends IssueVisitor { .build(); private final PeriodHolder periodHolder; + private final AnalysisMetadataHolder analysisMetadataHolder; private final MetricRepository metricRepository; private final MeasureRepository measureRepository; private final Map countersByComponentUuid = new HashMap<>(); private Counters currentCounters; - public IssueCounter(PeriodHolder periodHolder, + public IssueCounter(PeriodHolder periodHolder, AnalysisMetadataHolder analysisMetadataHolder, MetricRepository metricRepository, MeasureRepository measureRepository) { this.periodHolder = periodHolder; + this.analysisMetadataHolder = analysisMetadataHolder; this.metricRepository = metricRepository; this.measureRepository = measureRepository; } @@ -137,13 +140,13 @@ public class IssueCounter extends IssueVisitor { } currentCounters.add(issue); - if (!periodHolder.hasPeriod()) { - return; - } - Period period = periodHolder.getPeriod(); - // Add one second to not take into account issues created during current analysis - if (issue.creationDate().getTime() > truncateToSeconds(period.getSnapshotDate())) { + if (analysisMetadataHolder.isSLBorPR()) { currentCounters.addOnPeriod(issue); + } else if (periodHolder.hasPeriod()) { + Period period = periodHolder.getPeriod(); + if (issue.creationDate().getTime() > truncateToSeconds(period.getSnapshotDate())) { + currentCounters.addOnPeriod(issue); + } } } @@ -152,7 +155,7 @@ public class IssueCounter extends IssueVisitor { addMeasuresBySeverity(component); addMeasuresByStatus(component); addMeasuresByType(component); - addMeasuresByPeriod(component); + addNewMeasures(component); currentCounters = null; } @@ -184,8 +187,8 @@ public class IssueCounter extends IssueVisitor { measureRepository.add(component, metric, Measure.newMeasureBuilder().create(value)); } - private void addMeasuresByPeriod(Component component) { - if (!periodHolder.hasPeriod()) { + private void addNewMeasures(Component component) { + if (!periodHolder.hasPeriod() && !analysisMetadataHolder.isSLBorPR()) { return; } double unresolvedVariations = (double) currentCounters.counterForPeriod().unresolved; diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregator.java index 30376875414..2f15cdebe22 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregator.java @@ -23,12 +23,12 @@ import com.google.common.base.MoreObjects; import java.util.HashMap; import java.util.Map; 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; @@ -46,6 +46,7 @@ import static org.sonar.api.utils.DateUtils.truncateToSeconds; public class NewEffortAggregator extends IssueVisitor { private final Map counterByComponentUuid = new HashMap<>(); private final PeriodHolder periodHolder; + private final AnalysisMetadataHolder analysisMetadataHolder; private final MeasureRepository measureRepository; private final Metric newMaintainabilityEffortMetric; @@ -54,8 +55,10 @@ public class NewEffortAggregator extends IssueVisitor { private NewEffortCounter counter = null; - public NewEffortAggregator(PeriodHolder periodHolder, MetricRepository metricRepository, MeasureRepository measureRepository) { + public NewEffortAggregator(PeriodHolder periodHolder, AnalysisMetadataHolder analysisMetadataHolder, MetricRepository metricRepository, + MeasureRepository measureRepository) { this.periodHolder = periodHolder; + this.analysisMetadataHolder = analysisMetadataHolder; this.measureRepository = measureRepository; this.newMaintainabilityEffortMetric = metricRepository.getByKey(NEW_TECHNICAL_DEBT_KEY); @@ -77,14 +80,18 @@ public class NewEffortAggregator extends IssueVisitor { @Override public void onIssue(Component component, DefaultIssue issue) { - if (issue.resolution() == null && issue.effortInMinutes() != null && periodHolder.hasPeriod()) { - counter.add(issue, periodHolder.getPeriod()); + if (issue.resolution() == null && issue.effortInMinutes() != null) { + if (analysisMetadataHolder.isSLBorPR()) { + counter.add(issue, 0L); + } else if (periodHolder.hasPeriod()) { + counter.add(issue, periodHolder.getPeriod().getSnapshotDate()); + } } } @Override public void afterComponent(Component component) { - if (periodHolder.hasPeriod()) { + if (periodHolder.hasPeriod() || analysisMetadataHolder.isSLBorPR()) { computeMeasure(component, newMaintainabilityEffortMetric, counter.maintainabilitySum); computeMeasure(component, newReliabilityEffortMetric, counter.reliabilitySum); computeMeasure(component, newSecurityEffortMetric, counter.securitySum); @@ -108,8 +115,8 @@ public class NewEffortAggregator extends IssueVisitor { securitySum.add(otherCounter.securitySum); } - void add(DefaultIssue issue, Period period) { - long newEffort = calculate(issue, period); + void add(DefaultIssue issue, long startDate) { + long newEffort = calculate(issue, startDate); switch (issue.type()) { case CODE_SMELL: maintainabilitySum.add(newEffort); @@ -128,8 +135,8 @@ public class NewEffortAggregator extends IssueVisitor { } } - long calculate(DefaultIssue issue, Period period) { - if (issue.creationDate().getTime() > truncateToSeconds(period.getSnapshotDate())) { + long calculate(DefaultIssue issue, long startDate) { + if (issue.creationDate().getTime() > truncateToSeconds(startDate)) { return MoreObjects.firstNonNull(issue.effortInMinutes(), 0L); } return 0L; diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitor.java index 5b8090c3391..772f2b848d1 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitor.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitor.java @@ -23,6 +23,7 @@ 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; @@ -73,9 +74,10 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitor extends PathAwareVis private final PeriodHolder periodHolder; private final Map metricsByKey; + private final AnalysisMetadataHolder analysisMetadataHolder; - public NewReliabilityAndSecurityRatingMeasuresVisitor(MetricRepository metricRepository, MeasureRepository measureRepository, ComponentIssuesRepository componentIssuesRepository, - PeriodHolder periodHolder) { + public NewReliabilityAndSecurityRatingMeasuresVisitor(MetricRepository metricRepository, MeasureRepository measureRepository, + ComponentIssuesRepository componentIssuesRepository, PeriodHolder periodHolder, AnalysisMetadataHolder analysisMetadataHolder) { super(LEAVES, POST_ORDER, CounterFactory.INSTANCE); this.measureRepository = measureRepository; this.componentIssuesRepository = componentIssuesRepository; @@ -85,6 +87,7 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitor extends PathAwareVis 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; } @Override @@ -103,7 +106,7 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitor extends PathAwareVis } private void computeAndSaveMeasures(Component component, Path path) { - if (!periodHolder.hasPeriod()) { + if (!periodHolder.hasPeriod() && !analysisMetadataHolder.isSLBorPR()) { return; } initRatingsToA(path); @@ -128,7 +131,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, periodHolder.getPeriod())); + .forEach(issue -> path.current().processIssue(issue, analysisMetadataHolder.isSLBorPR(), periodHolder)); } private static void addToParent(Path path) { @@ -147,11 +150,11 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitor extends PathAwareVis } void add(Counter otherCounter) { - newRatingValueByMetric.entrySet().forEach(e -> e.getValue().increment(otherCounter.newRatingValueByMetric.get(e.getKey()))); + newRatingValueByMetric.forEach((metric, rating) -> rating.increment(otherCounter.newRatingValueByMetric.get(metric))); } - void processIssue(Issue issue, Period period) { - if (isOnPeriod((DefaultIssue) issue, period)) { + void processIssue(Issue issue, boolean isSLBorPR, PeriodHolder periodHolder) { + if (isSLBorPR || isOnPeriod((DefaultIssue) issue, periodHolder.getPeriod())) { Rating rating = RATING_BY_SEVERITY.get(issue.severity()); if (issue.type().equals(BUG)) { newRatingValueByMetric.get(NEW_RELIABILITY_RATING_KEY).increment(rating); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderImplTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderImplTest.java index a96565ad165..b303628de23 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderImplTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/analysis/AnalysisMetadataHolderImplTest.java @@ -349,6 +349,28 @@ public class AnalysisMetadataHolderImplTest { assertThat(underTest.isShortLivingBranch()).isTrue(); } + @Test + public void getIsSLBorPR_returns_true() { + Branch branch = mock(Branch.class); + when(branch.getType()).thenReturn(BranchType.SHORT); + + AnalysisMetadataHolderImpl underTest = new AnalysisMetadataHolderImpl(); + underTest.setBranch(branch); + + assertThat(underTest.isSLBorPR()).isTrue(); + } + + @Test + public void getIsSLBorPR_returns_false() { + Branch branch = mock(Branch.class); + when(branch.getType()).thenReturn(BranchType.LONG); + + AnalysisMetadataHolderImpl underTest = new AnalysisMetadataHolderImpl(); + underTest.setBranch(branch); + + assertThat(underTest.isSLBorPR()).isFalse(); + } + @Test public void getPullRequestBranch_returns_true() { Branch branch = mock(Branch.class); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueCounterTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueCounterTest.java index f787db4ec07..4dcd5044223 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueCounterTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueCounterTest.java @@ -27,6 +27,8 @@ import org.junit.Rule; import org.junit.Test; import org.sonar.api.measures.CoreMetrics; 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,9 +40,12 @@ 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 org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FALSE_POSITIVE; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; import static org.sonar.api.issue.Issue.RESOLUTION_WONT_FIX; @@ -111,6 +116,9 @@ public class IssueCounterTest { @Rule public PeriodHolderRule periodsHolder = new PeriodHolderRule(); + @Rule + public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule(); + @Rule public MetricRepositoryRule metricRepository = new MetricRepositoryRule() .add(ISSUES_METRIC) @@ -140,7 +148,7 @@ public class IssueCounterTest { @Rule public MeasureRepositoryRule measureRepository = MeasureRepositoryRule.create(treeRootHolder, metricRepository); - private IssueCounter underTest = new IssueCounter(periodsHolder, metricRepository, measureRepository); + private IssueCounter underTest = new IssueCounter(periodsHolder, analysisMetadataHolder, metricRepository, measureRepository); @Test public void count_issues_by_status() { @@ -282,7 +290,7 @@ public class IssueCounterTest { } @Test - public void count_new_issues() { + public void count_new_issues_if_period_exists() { Period period = newPeriod(1500000000000L); periodsHolder.setPeriod(period); @@ -320,6 +328,44 @@ public class IssueCounterTest { assertVariation(PROJECT, NEW_VULNERABILITIES_METRIC, 0); } + @Test + public void count_all_issues_as_new_issues_if_pr_or_slb() { + periodsHolder.setPeriod(null); + Branch branch = mock(Branch.class); + when(branch.getType()).thenReturn(BranchType.SHORT); + 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); + + assertVariation(FILE1, NEW_ISSUES_METRIC, 4); + assertVariation(FILE1, NEW_CRITICAL_ISSUES_METRIC, 2); + assertVariation(FILE1, NEW_BLOCKER_ISSUES_METRIC, 2); + assertVariation(FILE1, NEW_MAJOR_ISSUES_METRIC, 0); + assertVariation(FILE1, NEW_CODE_SMELLS_METRIC, 2); + assertVariation(FILE1, NEW_BUGS_METRIC, 2); + assertVariation(FILE1, NEW_VULNERABILITIES_METRIC, 0); + + assertVariation(PROJECT, NEW_ISSUES_METRIC, 4); + assertVariation(PROJECT, NEW_CRITICAL_ISSUES_METRIC, 2); + assertVariation(PROJECT, NEW_BLOCKER_ISSUES_METRIC, 2); + assertVariation(PROJECT, NEW_MAJOR_ISSUES_METRIC, 0); + assertVariation(PROJECT, NEW_CODE_SMELLS_METRIC, 2); + assertVariation(PROJECT, NEW_BUGS_METRIC, 2); + assertVariation(PROJECT, NEW_VULNERABILITIES_METRIC, 0); + } + @Test public void exclude_hotspots_from_issue_counts() { periodsHolder.setPeriod(null); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregatorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregatorTest.java index 272fc1bd249..2431763a984 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregatorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregatorTest.java @@ -24,7 +24,10 @@ 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.DefaultBranchImpl; import org.sonar.ce.task.projectanalysis.component.ReportComponent; import org.sonar.ce.task.projectanalysis.measure.Measure; import org.sonar.ce.task.projectanalysis.measure.MeasureRepositoryRule; @@ -32,9 +35,12 @@ 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 static org.assertj.core.api.Assertions.assertThat; import static org.assertj.guava.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; import static org.sonar.api.measures.CoreMetrics.NEW_RELIABILITY_REMEDIATION_EFFORT; import static org.sonar.api.measures.CoreMetrics.NEW_RELIABILITY_REMEDIATION_EFFORT_KEY; @@ -68,9 +74,11 @@ 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, metricRepository, measureRepository); + private NewEffortAggregator underTest = new NewEffortAggregator(periodsHolder, analysisMetadataHolder, metricRepository, measureRepository); @Test public void sum_new_maintainability_effort_of_issues() { @@ -255,6 +263,9 @@ public class NewEffortAggregatorTest { @Test public void no_measures_if_no_periods() { + Branch branch = mock(Branch.class); + when(branch.getType()).thenReturn(BranchType.LONG); + analysisMetadataHolder.setBranch(branch); periodsHolder.setPeriod(null); DefaultIssue unresolved = newCodeSmellIssue(10); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitorTest.java index f76530bf921..aa29d30d742 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitorTest.java @@ -25,6 +25,8 @@ 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; @@ -39,9 +41,12 @@ 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.server.measure.Rating; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; import static org.sonar.api.measures.CoreMetrics.NEW_RELIABILITY_RATING; import static org.sonar.api.measures.CoreMetrics.NEW_RELIABILITY_RATING_KEY; @@ -106,6 +111,8 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { @Rule public PeriodHolderRule periodsHolder = new PeriodHolderRule().setPeriod(new Period("mode", null, LEAK_PERIOD_SNAPSHOT_IN_MILLISEC, "UUID")); + @Rule + public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule(); @Rule public ComponentIssuesRepositoryRule componentIssuesRepositoryRule = new ComponentIssuesRepositoryRule(treeRootHolder); @@ -113,7 +120,8 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitorTest { public FillComponentIssuesVisitorRule fillComponentIssuesVisitorRule = new FillComponentIssuesVisitorRule(componentIssuesRepositoryRule, treeRootHolder); private VisitorsCrawler underTest = new VisitorsCrawler(Arrays.asList(fillComponentIssuesVisitorRule, - new NewReliabilityAndSecurityRatingMeasuresVisitor(metricRepository, measureRepository, componentIssuesRepositoryRule, periodsHolder))); + new NewReliabilityAndSecurityRatingMeasuresVisitor(metricRepository, measureRepository, componentIssuesRepositoryRule, + periodsHolder, analysisMetadataHolder))); @Test public void measures_created_for_project_are_all_A_when_they_have_no_FILE_child() { @@ -276,6 +284,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), diff --git a/server/sonar-server/src/main/java/org/sonar/server/measure/live/LiveMeasureComputerImpl.java b/server/sonar-server/src/main/java/org/sonar/server/measure/live/LiveMeasureComputerImpl.java index d53cf0cf717..f46daa3255e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/measure/live/LiveMeasureComputerImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/measure/live/LiveMeasureComputerImpl.java @@ -34,6 +34,7 @@ import org.sonar.api.utils.log.Loggers; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.BranchDto; +import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; import org.sonar.db.component.SnapshotDto; import org.sonar.db.measure.LiveMeasureComparator; @@ -100,7 +101,6 @@ public class LiveMeasureComputerImpl implements LiveMeasureComputer { if (!lastAnalysis.isPresent()) { return Optional.empty(); } - Optional beginningOfLeakPeriod = lastAnalysis.map(SnapshotDto::getPeriodDate); QualityGate qualityGate = qGateComputer.loadQualityGate(dbSession, organization, project, branch); Collection metricKeys = getKeysOfAllInvolvedMetrics(qualityGate); @@ -118,11 +118,13 @@ public class LiveMeasureComputerImpl implements LiveMeasureComputer { MeasureMatrix matrix = new MeasureMatrix(components, metricsPerId.values(), dbMeasures); FormulaContextImpl context = new FormulaContextImpl(matrix, debtRatingGrid); + long beginningOfLeak = getBeginningOfLeakPeriod(lastAnalysis, branch); + components.forEach(c -> { - IssueCounter issueCounter = new IssueCounter(dbClient.issueDao().selectIssueGroupsByBaseComponent(dbSession, c, beginningOfLeakPeriod.orElse(Long.MAX_VALUE))); + IssueCounter issueCounter = new IssueCounter(dbClient.issueDao().selectIssueGroupsByBaseComponent(dbSession, c, beginningOfLeak)); for (IssueMetricFormula formula : formulaFactory.getFormulas()) { - // exclude leak formulas when leak period is not defined - if (beginningOfLeakPeriod.isPresent() || !formula.isOnLeak()) { + // use formulas when the leak period is defined, it's a PR/SLB, or the formula is not about the leak period + if (shouldUseLeakFormulas(lastAnalysis.get(), branch) || !formula.isOnLeak()) { context.change(c, formula); try { formula.compute(context, issueCounter); @@ -144,6 +146,23 @@ public class LiveMeasureComputerImpl implements LiveMeasureComputer { new QGChangeEvent(project, branch, lastAnalysis.get(), config, previousStatus, () -> Optional.of(evaluatedQualityGate))); } + private static long getBeginningOfLeakPeriod(Optional lastAnalysis, BranchDto branch) { + if (isSLBorPR(branch)) { + return 0L; + } else { + Optional beginningOfLeakPeriod = lastAnalysis.map(SnapshotDto::getPeriodDate); + return beginningOfLeakPeriod.orElse(Long.MAX_VALUE); + } + } + + private static boolean isSLBorPR(BranchDto branch) { + return branch.getBranchType() == BranchType.SHORT || branch.getBranchType() == BranchType.PULL_REQUEST; + } + + private static boolean shouldUseLeakFormulas(SnapshotDto lastAnalysis, BranchDto branch) { + return lastAnalysis.getPeriodDate() != null || isSLBorPR(branch); + } + @CheckForNull private static Metric.Level loadPreviousStatus(List metrics, List dbMeasures) { MetricDto alertStatusMetric = metrics.stream() diff --git a/server/sonar-server/src/test/java/org/sonar/server/measure/live/LiveMeasureComputerImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/measure/live/LiveMeasureComputerImplTest.java index be254749f57..df8fff10315 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/measure/live/LiveMeasureComputerImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/measure/live/LiveMeasureComputerImplTest.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; +import javax.annotation.Nullable; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -42,14 +43,15 @@ import org.sonar.core.config.CorePropertyDefinitions; import org.sonar.db.DbSession; import org.sonar.db.DbTester; import org.sonar.db.component.BranchDto; +import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; import org.sonar.db.component.ComponentTesting; import org.sonar.db.measure.LiveMeasureDto; import org.sonar.db.metric.MetricDto; import org.sonar.db.organization.OrganizationDto; -import org.sonar.server.measure.Rating; import org.sonar.server.es.ProjectIndexer; import org.sonar.server.es.TestProjectIndexers; +import org.sonar.server.measure.Rating; import org.sonar.server.qualitygate.EvaluatedQualityGate; import org.sonar.server.qualitygate.QualityGate; import org.sonar.server.qualitygate.changeevent.QGChangeEvent; @@ -85,6 +87,8 @@ public class LiveMeasureComputerImplTest { private ComponentDto dir; private ComponentDto file1; private ComponentDto file2; + private ComponentDto branch; + private ComponentDto branchFile; private LiveQualityGateComputer qGateComputer = mock(LiveQualityGateComputer.class); private QualityGate qualityGate = mock(QualityGate.class); private EvaluatedQualityGate newQualityGate = mock(EvaluatedQualityGate.class); @@ -99,6 +103,8 @@ public class LiveMeasureComputerImplTest { dir = db.components().insertComponent(ComponentTesting.newDirectory(project, "src/main/java")); file1 = db.components().insertComponent(ComponentTesting.newFileDto(project, dir)); file2 = db.components().insertComponent(ComponentTesting.newFileDto(project, dir)); + branch = db.components().insertProjectBranch(project, b -> b.setBranchType(BranchType.PULL_REQUEST)); + branchFile = db.components().insertComponent(ComponentTesting.newFileDto(branch)); } @Test @@ -216,11 +222,29 @@ public class LiveMeasureComputerImplTest { assertThatProjectChanged(result, project); } + @Test + public void calculate_new_metrics_if_it_is_pr_or_branch() { + markProjectAsAnalyzed(branch, null); + db.measures().insertLiveMeasure(branch, intMetric, m -> m.setVariation(42.0).setValue(null)); + db.measures().insertLiveMeasure(branchFile, intMetric, m -> m.setVariation(42.0).setValue(null)); + + // generates values 1, 2, 3 on leak measures + List result = run(branchFile, newQualifierBasedIntLeakFormula(), newRatingLeakFormula(Rating.B)); + + assertThat(db.countRowsOfTable(db.getSession(), "live_measures")).isEqualTo(4); + + // Numeric value depends on qualifier (see newQualifierBasedIntLeakFormula()) + assertThatIntMeasureHasLeakValue(branchFile, ORDERED_BOTTOM_UP.indexOf(Qualifiers.FILE)); + assertThatRatingMeasureHasLeakValue(branchFile, Rating.B); + assertThatIntMeasureHasLeakValue(branch, ORDERED_BOTTOM_UP.indexOf(Qualifiers.PROJECT)); + assertThatRatingMeasureHasLeakValue(branch, Rating.B); + assertThatProjectChanged(result, branch); + } + @Test public void do_nothing_if_project_has_not_been_analyzed() { // project has no snapshots List result = run(file1, newIncrementalFormula()); - assertThat(db.countRowsOfTable(db.getSession(), "live_measures")).isEqualTo(0); assertThatProjectNotChanged(result, project); } @@ -380,8 +404,12 @@ public class LiveMeasureComputerImplTest { } private void markProjectAsAnalyzed(ComponentDto p) { + markProjectAsAnalyzed(p, 1_490_000_000L); + } + + private void markProjectAsAnalyzed(ComponentDto p, @Nullable Long periodDate) { assertThat(p.qualifier()).isEqualTo(Qualifiers.PROJECT); - db.components().insertSnapshot(p, s -> s.setPeriodDate(1_490_000_000L)); + db.components().insertSnapshot(p, s -> s.setPeriodDate(periodDate)); } private LiveMeasureDto assertThatIntMeasureHasValue(ComponentDto component, double expectedValue) {