From 5401ba28ca50f805f19a0e33852c5566f53f9175 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 4 Feb 2020 09:17:43 +0100 Subject: [PATCH] SONAR-12962 Compute Security Review measures on new code --- ...ProjectAnalysisTaskContainerPopulator.java | 2 + .../projectanalysis/issue/IssueCounter.java | 3 +- .../issue/NewEffortAggregator.java | 15 +- .../task/projectanalysis/period/Period.java | 6 + ...ilityAndSecurityRatingMeasuresVisitor.java | 9 +- .../NewSecurityReviewMeasuresVisitor.java | 107 +++++ .../qualitymodel/SecurityReviewCounter.java | 55 +++ .../SecurityReviewMeasuresVisitor.java | 43 +-- ...yAndSecurityRatingMeasuresVisitorTest.java | 20 +- .../NewSecurityReviewMeasuresVisitorTest.java | 364 ++++++++++++++++++ .../org/sonar/db/issue/IssueMapper.xml | 2 +- .../java/org/sonar/db/issue/IssueDaoTest.java | 5 + .../live/IssueMetricFormulaFactoryImpl.java | 12 + .../IssueMetricFormulaFactoryImplTest.java | 28 +- .../resources/org/sonar/l10n/core.properties | 4 + .../org/sonar/api/measures/CoreMetrics.java | 138 ++++--- 16 files changed, 700 insertions(+), 113 deletions(-) create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitor.java create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewCounter.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitorTest.java diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java index c5dc824f950..ed5b4fd315c 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java @@ -111,6 +111,7 @@ import org.sonar.ce.task.projectanalysis.qualitygate.QualityGateStatusHolderImpl import org.sonar.ce.task.projectanalysis.qualitymodel.MaintainabilityMeasuresVisitor; import org.sonar.ce.task.projectanalysis.qualitymodel.NewMaintainabilityMeasuresVisitor; import org.sonar.ce.task.projectanalysis.qualitymodel.NewReliabilityAndSecurityRatingMeasuresVisitor; +import org.sonar.ce.task.projectanalysis.qualitymodel.NewSecurityReviewMeasuresVisitor; import org.sonar.ce.task.projectanalysis.qualitymodel.RatingSettings; import org.sonar.ce.task.projectanalysis.qualitymodel.ReliabilityAndSecurityRatingMeasuresVisitor; import org.sonar.ce.task.projectanalysis.qualitymodel.SecurityReviewMeasuresVisitor; @@ -269,6 +270,7 @@ public final class ProjectAnalysisTaskContainerPopulator implements ContainerPop ReliabilityAndSecurityRatingMeasuresVisitor.class, NewReliabilityAndSecurityRatingMeasuresVisitor.class, SecurityReviewMeasuresVisitor.class, + NewSecurityReviewMeasuresVisitor.class, LastCommitVisitor.class, MeasureComputersVisitor.class, diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCounter.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCounter.java index 057d5bb00ca..91fadde60ca 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 @@ -76,7 +76,6 @@ import static org.sonar.api.rules.RuleType.BUG; import static org.sonar.api.rules.RuleType.CODE_SMELL; import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; import static org.sonar.api.rules.RuleType.VULNERABILITY; -import static org.sonar.api.utils.DateUtils.truncateToSeconds; /** * For each component, computes the measures related to number of issues: @@ -153,7 +152,7 @@ public class IssueCounter extends IssueVisitor { currentCounters.addOnPeriod(issue); } else if (periodHolder.hasPeriod()) { Period period = periodHolder.getPeriod(); - if (issue.creationDate().getTime() > truncateToSeconds(period.getSnapshotDate())) { + if (period.isOnPeriod(issue.creationDate())){ currentCounters.addOnPeriod(issue); } } 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 ae7c9c28ac3..1bd97ca7542 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewEffortAggregator.java @@ -22,6 +22,7 @@ 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; @@ -29,13 +30,13 @@ 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; import static org.sonar.api.measures.CoreMetrics.NEW_SECURITY_REMEDIATION_EFFORT_KEY; import static org.sonar.api.measures.CoreMetrics.NEW_TECHNICAL_DEBT_KEY; -import static org.sonar.api.utils.DateUtils.truncateToSeconds; /** * Compute new effort related measures : @@ -82,9 +83,9 @@ public class NewEffortAggregator extends IssueVisitor { public void onIssue(Component component, DefaultIssue issue) { if (issue.resolution() == null && issue.effortInMinutes() != null) { if (analysisMetadataHolder.isPullRequest()) { - counter.add(issue, 0L); + counter.add(issue, null); } else if (periodHolder.hasPeriod()) { - counter.add(issue, periodHolder.getPeriod().getSnapshotDate()); + counter.add(issue, periodHolder.getPeriod()); } } } @@ -115,8 +116,8 @@ public class NewEffortAggregator extends IssueVisitor { securitySum.add(otherCounter.securitySum); } - void add(DefaultIssue issue, long startDate) { - long newEffort = calculate(issue, startDate); + void add(DefaultIssue issue, @Nullable Period period) { + long newEffort = calculate(issue, period); switch (issue.type()) { case CODE_SMELL: maintainabilitySum.add(newEffort); @@ -135,8 +136,8 @@ public class NewEffortAggregator extends IssueVisitor { } } - long calculate(DefaultIssue issue, long startDate) { - if (issue.creationDate().getTime() > truncateToSeconds(startDate)) { + long calculate(DefaultIssue issue, @Nullable Period period) { + if (period == null || period.isOnPeriod(issue.creationDate())) { return MoreObjects.firstNonNull(issue.effortInMinutes(), 0L); } return 0L; diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/period/Period.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/period/Period.java index 3a44185505f..474adf14536 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/period/Period.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/period/Period.java @@ -19,6 +19,7 @@ */ package org.sonar.ce.task.projectanalysis.period; +import java.util.Date; import java.util.Objects; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -27,6 +28,7 @@ import javax.annotation.concurrent.Immutable; import static com.google.common.base.MoreObjects.toStringHelper; import static java.util.Objects.hash; import static java.util.Objects.requireNonNull; +import static org.sonar.api.utils.DateUtils.truncateToSeconds; @Immutable public class Period { @@ -68,6 +70,10 @@ public class Period { && Objects.equals(modeParameter, period.modeParameter); } + public boolean isOnPeriod(Date date) { + return date.getTime() > truncateToSeconds(snapshotDate); + } + @Override public int hashCode() { return hash(mode, modeParameter, snapshotDate); 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 5fc877f4ee1..c9d70906bbf 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 @@ -31,7 +31,6 @@ import org.sonar.ce.task.projectanalysis.issue.ComponentIssuesRepository; 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 org.sonar.server.measure.Rating; @@ -45,7 +44,6 @@ import static org.sonar.api.rule.Severity.MAJOR; import static org.sonar.api.rule.Severity.MINOR; import static org.sonar.api.rules.RuleType.BUG; import static org.sonar.api.rules.RuleType.VULNERABILITY; -import static org.sonar.api.utils.DateUtils.truncateToSeconds; import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order.POST_ORDER; import static org.sonar.ce.task.projectanalysis.component.CrawlerDepthLimit.LEAVES; import static org.sonar.ce.task.projectanalysis.measure.Measure.newMeasureBuilder; @@ -154,7 +152,7 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitor extends PathAwareVis } void processIssue(Issue issue, boolean isPR, PeriodHolder periodHolder) { - if (isPR || isOnPeriod((DefaultIssue) issue, periodHolder.getPeriod())) { + if (isPR || periodHolder.getPeriod().isOnPeriod(((DefaultIssue) issue).creationDate())) { Rating rating = RATING_BY_SEVERITY.get(issue.severity()); if (issue.type().equals(BUG)) { newRatingValueByMetric.get(NEW_RELIABILITY_RATING_KEY).increment(rating); @@ -163,11 +161,6 @@ public class NewReliabilityAndSecurityRatingMeasuresVisitor extends PathAwareVis } } } - - private static boolean isOnPeriod(DefaultIssue issue, Period period) { - // Add one second to not take into account issues created during current analysis - return issue.creationDate().getTime() > truncateToSeconds(period.getSnapshotDate()); - } } private static final class CounterFactory extends SimpleStackElementFactory { diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitor.java new file mode 100644 index 00000000000..5c48a164805 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitor.java @@ -0,0 +1,107 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 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.qualitymodel; + +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; +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 static org.sonar.api.measures.CoreMetrics.NEW_SECURITY_HOTSPOTS_REVIEWED_KEY; +import static org.sonar.api.measures.CoreMetrics.NEW_SECURITY_REVIEW_RATING_KEY; +import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; +import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order.POST_ORDER; +import static org.sonar.ce.task.projectanalysis.component.CrawlerDepthLimit.FILE; +import static org.sonar.server.security.SecurityReviewRating.computePercent; +import static org.sonar.server.security.SecurityReviewRating.computeRating; + +public class NewSecurityReviewMeasuresVisitor extends PathAwareVisitorAdapter { + + private final ComponentIssuesRepository componentIssuesRepository; + private final MeasureRepository measureRepository; + private final PeriodHolder periodHolder; + private final AnalysisMetadataHolder analysisMetadataHolder; + private final Metric newSecurityReviewRatingMetric; + private final Metric newSecurityHotspotsReviewedMetric; + + public NewSecurityReviewMeasuresVisitor(ComponentIssuesRepository componentIssuesRepository, MeasureRepository measureRepository, PeriodHolder periodHolder, + AnalysisMetadataHolder analysisMetadataHolder, MetricRepository metricRepository) { + 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); + } + + @Override + public void visitProject(Component project, Path path) { + computeMeasure(project, path); + } + + @Override + public void visitDirectory(Component directory, Path path) { + computeMeasure(directory, path); + } + + @Override + public void visitFile(Component file, Path path) { + computeMeasure(file, path); + } + + private void computeMeasure(Component component, Path path) { + if (!periodHolder.hasPeriod() && !analysisMetadataHolder.isPullRequest()) { + return; + } + componentIssuesRepository.getIssues(component) + .stream() + .filter(issue -> issue.type().equals(SECURITY_HOTSPOT)) + .filter(issue -> analysisMetadataHolder.isPullRequest() || periodHolder.getPeriod().isOnPeriod(issue.creationDate()) ) + .forEach(issue -> path.current().processHotspot(issue)); + + Double percent = computePercent(path.current().getHotspotsToReview(), path.current().getHotspotsReviewed()); + measureRepository.add(component, newSecurityHotspotsReviewedMetric, Measure.newMeasureBuilder().setVariation(percent).createNoValue()); + measureRepository.add(component, newSecurityReviewRatingMetric, Measure.newMeasureBuilder().setVariation(computeRating(percent).getIndex()).createNoValue()); + + if (!path.isRoot()) { + path.parent().add(path.current()); + } + } + + private static final class CounterFactory extends SimpleStackElementFactory { + public static final NewSecurityReviewMeasuresVisitor.CounterFactory INSTANCE = new NewSecurityReviewMeasuresVisitor.CounterFactory(); + + private CounterFactory() { + // prevents instantiation + } + + @Override + public SecurityReviewCounter createForAny(Component component) { + return new SecurityReviewCounter(); + } + } + +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewCounter.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewCounter.java new file mode 100644 index 00000000000..07d35e3a98c --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewCounter.java @@ -0,0 +1,55 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 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.qualitymodel; + +import org.sonar.api.ce.measure.Issue; + +import static org.sonar.api.issue.Issue.STATUS_REVIEWED; +import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; + +final class SecurityReviewCounter { + private long hotspotsReviewed; + private long hotspotsToReview; + + SecurityReviewCounter() { + // prevents instantiation + } + + void processHotspot(Issue issue) { + if (issue.status().equals(STATUS_REVIEWED)) { + hotspotsReviewed++; + } else if (issue.status().equals(STATUS_TO_REVIEW)) { + hotspotsToReview++; + } + } + + void add(SecurityReviewCounter otherCounter) { + hotspotsReviewed += otherCounter.hotspotsReviewed; + hotspotsToReview += otherCounter.hotspotsToReview; + } + + public long getHotspotsReviewed() { + return hotspotsReviewed; + } + + public long getHotspotsToReview() { + return hotspotsToReview; + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewMeasuresVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewMeasuresVisitor.java index 0f7c84b45bf..2bb1fdc3d3f 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewMeasuresVisitor.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewMeasuresVisitor.java @@ -19,7 +19,6 @@ */ package org.sonar.ce.task.projectanalysis.qualitymodel; -import org.sonar.api.ce.measure.Issue; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.PathAwareVisitor; import org.sonar.ce.task.projectanalysis.component.PathAwareVisitorAdapter; @@ -30,17 +29,15 @@ import org.sonar.ce.task.projectanalysis.measure.RatingMeasures; import org.sonar.ce.task.projectanalysis.metric.Metric; import org.sonar.ce.task.projectanalysis.metric.MetricRepository; -import static org.sonar.api.issue.Issue.STATUS_REVIEWED; import static org.sonar.api.measures.CoreMetrics.SECURITY_HOTSPOTS_REVIEWED_KEY; import static org.sonar.api.measures.CoreMetrics.SECURITY_REVIEW_RATING_KEY; import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order.POST_ORDER; import static org.sonar.ce.task.projectanalysis.component.CrawlerDepthLimit.FILE; -import static org.sonar.core.issue.DefaultIssue.STATUS_TO_REVIEW; import static org.sonar.server.security.SecurityReviewRating.computePercent; import static org.sonar.server.security.SecurityReviewRating.computeRating; -public class SecurityReviewMeasuresVisitor extends PathAwareVisitorAdapter { +public class SecurityReviewMeasuresVisitor extends PathAwareVisitorAdapter { private final ComponentIssuesRepository componentIssuesRepository; private final MeasureRepository measureRepository; @@ -56,27 +53,27 @@ public class SecurityReviewMeasuresVisitor extends PathAwareVisitorAdapter path) { + public void visitProject(Component project, Path path) { computeMeasure(project, path); } @Override - public void visitDirectory(Component directory, PathAwareVisitor.Path path) { + public void visitDirectory(Component directory, PathAwareVisitor.Path path) { computeMeasure(directory, path); } @Override - public void visitFile(Component file, PathAwareVisitor.Path path) { + public void visitFile(Component file, PathAwareVisitor.Path path) { computeMeasure(file, path); } - private void computeMeasure(Component component, PathAwareVisitor.Path path) { + private void computeMeasure(Component component, PathAwareVisitor.Path path) { componentIssuesRepository.getIssues(component) .stream() .filter(issue -> issue.type().equals(SECURITY_HOTSPOT)) .forEach(issue -> path.current().processHotspot(issue)); - Double percent = computePercent(path.current().hotspotsToReview, path.current().hotspotsReviewed); + Double percent = computePercent(path.current().getHotspotsToReview(), path.current().getHotspotsReviewed()); measureRepository.add(component, securityHotspotsReviewedMetric, Measure.newMeasureBuilder().create(percent, securityHotspotsReviewedMetric.getDecimalScale())); measureRepository.add(component, securityReviewRatingMetric, RatingMeasures.get(computeRating(percent))); @@ -85,29 +82,7 @@ public class SecurityReviewMeasuresVisitor extends PathAwareVisitorAdapter { + private static final class CounterFactory extends PathAwareVisitorAdapter.SimpleStackElementFactory { public static final SecurityReviewMeasuresVisitor.CounterFactory INSTANCE = new SecurityReviewMeasuresVisitor.CounterFactory(); private CounterFactory() { @@ -115,8 +90,8 @@ public class SecurityReviewMeasuresVisitor extends PathAwareVisitorAdapter 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; + private static final int ROOT_DIR_REF = 12; + private static final int DIRECTORY_REF = 123; + private static final int FILE_1_REF = 1231; + private static final int FILE_2_REF = 1232; + + private static final Component ROOT_PROJECT = builder(Component.Type.PROJECT, PROJECT_REF).setKey("project") + .addChildren( + builder(DIRECTORY, ROOT_DIR_REF).setKey("dir") + .addChildren( + builder(DIRECTORY, DIRECTORY_REF).setKey("directory") + .addChildren( + builder(FILE, FILE_1_REF).setFileAttributes(new FileAttributes(false, LANGUAGE_KEY_1, 1)).setKey("file1").build(), + builder(FILE, FILE_2_REF).setFileAttributes(new FileAttributes(false, LANGUAGE_KEY_1, 1)).setKey("file2").build()) + .build()) + .build()) + .build(); + + @Rule + public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule(); + @Rule + public MetricRepositoryRule metricRepository = new MetricRepositoryRule() + .add(NEW_SECURITY_REVIEW_RATING) + .add(NEW_SECURITY_HOTSPOTS_REVIEWED); + @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 NewSecurityReviewMeasuresVisitor(componentIssuesRepositoryRule, measureRepository, periodsHolder, analysisMetadataHolder, metricRepository))); + + @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), + // 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)); + 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)); + fillComponentIssuesVisitorRule.setIssues(ROOT_DIR_REF, + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED).setCreationDate(AFTER_LEAK_PERIOD_DATE)); + + underTest.visit(ROOT_PROJECT); + + verifyMeasures(FILE_1_REF, A, 100.0); + verifyMeasures(FILE_2_REF, A, 100.0); + verifyMeasures(DIRECTORY_REF, A, 100.0); + verifyMeasures(ROOT_DIR_REF, A, 100.0); + verifyMeasures(PROJECT_REF, A, 100.0); + } + + @Test + 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), + // Should not be taken into account + newIssue().setCreationDate(AFTER_LEAK_PERIOD_DATE)); + 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), + // 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()); + + underTest.visit(ROOT_PROJECT); + + verifyMeasures(FILE_1_REF, A, 100.0); + verifyMeasures(FILE_2_REF, A, 80.0); + verifyMeasures(DIRECTORY_REF, A, 87.5); + verifyMeasures(ROOT_DIR_REF, A, 87.5); + verifyMeasures(PROJECT_REF, A, 87.5); + } + + @Test + 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), + // Should not be taken into account + newIssue().setCreationDate(AFTER_LEAK_PERIOD_DATE)); + 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), + // 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()); + + underTest.visit(ROOT_PROJECT); + + verifyMeasures(FILE_1_REF, A, 100.0); + verifyMeasures(FILE_2_REF, B, 71.42); + verifyMeasures(DIRECTORY_REF, B, 75.0); + verifyMeasures(ROOT_DIR_REF, B, 75.0); + verifyMeasures(PROJECT_REF, B, 75.0); + } + + @Test + 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), + // 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), + // 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()); + + underTest.visit(ROOT_PROJECT); + + verifyMeasures(FILE_1_REF, C, 50.0); + verifyMeasures(FILE_2_REF, C, 60.0); + verifyMeasures(DIRECTORY_REF, C, 57.14); + verifyMeasures(ROOT_DIR_REF, C, 57.14); + verifyMeasures(PROJECT_REF, C, 57.14); + } + + @Test + 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), + // 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), + // 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()); + + underTest.visit(ROOT_PROJECT); + + verifyMeasures(FILE_1_REF, D, 33.33); + verifyMeasures(FILE_2_REF, D, 40.0); + verifyMeasures(DIRECTORY_REF, D, 37.5); + verifyMeasures(ROOT_DIR_REF, D, 37.5); + verifyMeasures(PROJECT_REF, D, 37.5); + } + + @Test + 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), + // 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), + // 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()); + + underTest.visit(ROOT_PROJECT); + + verifyMeasures(FILE_1_REF, D, 33.33); + verifyMeasures(FILE_2_REF, E, 0.0); + verifyMeasures(DIRECTORY_REF, E, 16.66); + verifyMeasures(ROOT_DIR_REF, E, 16.66); + verifyMeasures(PROJECT_REF, E, 16.66); + } + + @Test + public void compute_A_rating_and_100_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), + newIssue()); + + underTest.visit(ROOT_PROJECT); + + verifyMeasures(PROJECT_REF, A, 100.0); + } + + @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); + + verifyMeasures(FILE_1_REF, C, 50.0); + verifyMeasures(FILE_2_REF, C, 57.14); + verifyMeasures(DIRECTORY_REF, C, 55.55); + verifyMeasures(ROOT_DIR_REF, C, 55.55); + verifyMeasures(PROJECT_REF, C, 55.55); + } + + @Test + public void no_measure_if_there_is_no_period() { + periodsHolder.setPeriod(null); + treeRootHolder.setRoot(ROOT_PROJECT); + fillComponentIssuesVisitorRule.setIssues(FILE_1_REF, + newHotspot(STATUS_TO_REVIEW, null), + newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED)); + + underTest.visit(ROOT_PROJECT); + + assertThat(measureRepository.getAddedRawMeasures(PROJECT_REF).values()).isEmpty(); + } + + private void verifyMeasures(int componentRef, Rating expectedReviewRating, double expectedHotspotsReviewed) { + MeasureAssert.assertThat(measureRepository.getAddedRawMeasure(componentRef, NEW_SECURITY_REVIEW_RATING_KEY)).hasVariation(expectedReviewRating.getIndex()); + MeasureAssert.assertThat(measureRepository.getAddedRawMeasure(componentRef, NEW_SECURITY_HOTSPOTS_REVIEWED_KEY)).hasVariation(expectedHotspotsReviewed, + VARIATION_COMPARISON_OFFSET); + } + + private static DefaultIssue newHotspot(String status, @Nullable String resolution) { + return new DefaultIssue() + .setKey(Uuids.create()) + .setSeverity(MINOR) + .setStatus(status) + .setResolution(resolution) + .setType(RuleType.SECURITY_HOTSPOT) + .setCreationDate(DEFAULT_CREATION_DATE); + } + + private static DefaultIssue newIssue() { + return new DefaultIssue() + .setKey(Uuids.create()) + .setSeverity(MAJOR) + .setType(RuleType.BUG) + .setCreationDate(DEFAULT_CREATION_DATE); + } + +} diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml index 368414c60a7..cf6c1b87322 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml @@ -322,7 +322,7 @@