@@ -19,6 +19,7 @@ | |||
*/ | |||
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; | |||
@@ -94,9 +95,9 @@ public class NewSecurityReviewMeasuresVisitor extends PathAwareVisitorAdapter<Se | |||
.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()); | |||
Optional<Double> percent = computePercent(path.current().getHotspotsToReview(), path.current().getHotspotsReviewed()); | |||
measureRepository.add(component, newSecurityReviewRatingMetric, Measure.newMeasureBuilder().setVariation(computeRating(percent.orElse(null)).getIndex()).createNoValue()); | |||
percent.ifPresent(p -> measureRepository.add(component, newSecurityHotspotsReviewedMetric, Measure.newMeasureBuilder().setVariation(p).createNoValue())); | |||
if (!path.isRoot()) { | |||
path.parent().add(path.current()); |
@@ -19,11 +19,11 @@ | |||
*/ | |||
package org.sonar.ce.task.projectanalysis.qualitymodel; | |||
import java.util.Optional; | |||
import org.sonar.ce.task.projectanalysis.component.Component; | |||
import org.sonar.ce.task.projectanalysis.component.PathAwareVisitor; | |||
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.measure.RatingMeasures; | |||
import org.sonar.ce.task.projectanalysis.metric.Metric; | |||
@@ -36,6 +36,7 @@ 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.ce.task.projectanalysis.measure.Measure.newMeasureBuilder; | |||
import static org.sonar.server.security.SecurityReviewRating.computePercent; | |||
import static org.sonar.server.security.SecurityReviewRating.computeRating; | |||
@@ -62,8 +63,8 @@ public class SecurityReviewMeasuresVisitor extends PathAwareVisitorAdapter<Secur | |||
public void visitProject(Component project, Path<SecurityReviewCounter> path) { | |||
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, securityHotspotsReviewedStatusMetric, Measure.newMeasureBuilder().create(path.current().getHotspotsReviewed())); | |||
measureRepository.add(project, securityHotspotsToReviewStatusMetric, Measure.newMeasureBuilder().create(path.current().getHotspotsToReview())); | |||
measureRepository.add(project, securityHotspotsReviewedStatusMetric, newMeasureBuilder().create(path.current().getHotspotsReviewed())); | |||
measureRepository.add(project, securityHotspotsToReviewStatusMetric, newMeasureBuilder().create(path.current().getHotspotsToReview())); | |||
} | |||
@Override | |||
@@ -82,9 +83,9 @@ public class SecurityReviewMeasuresVisitor extends PathAwareVisitorAdapter<Secur | |||
.filter(issue -> issue.type().equals(SECURITY_HOTSPOT)) | |||
.forEach(issue -> path.current().processHotspot(issue)); | |||
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))); | |||
Optional<Double> percent = computePercent(path.current().getHotspotsToReview(), path.current().getHotspotsReviewed()); | |||
measureRepository.add(component, securityReviewRatingMetric, RatingMeasures.get(computeRating(percent.orElse(null)))); | |||
percent.ifPresent(p -> measureRepository.add(component, securityHotspotsReviewedMetric, newMeasureBuilder().create(p, securityHotspotsReviewedMetric.getDecimalScale()))); | |||
if (!path.isRoot()) { | |||
path.parent().add(path.current()); |
@@ -288,7 +288,7 @@ public class NewSecurityReviewMeasuresVisitorTest { | |||
} | |||
@Test | |||
public void compute_A_rating_and_100_percent_when_no_new_hotspot_on_new_code() { | |||
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), | |||
@@ -297,7 +297,7 @@ public class NewSecurityReviewMeasuresVisitorTest { | |||
underTest.visit(ROOT_PROJECT); | |||
verifyRatingAndReviewedMeasures(PROJECT_REF, A, 100.0); | |||
verifyRatingAndReviewedMeasures(PROJECT_REF, A, null); | |||
} | |||
@Test | |||
@@ -378,10 +378,14 @@ public class NewSecurityReviewMeasuresVisitorTest { | |||
assertThat(measureRepository.getAddedRawMeasures(PROJECT_REF).values()).isEmpty(); | |||
} | |||
private void verifyRatingAndReviewedMeasures(int componentRef, Rating expectedReviewRating, double expectedHotspotsReviewed) { | |||
private void verifyRatingAndReviewedMeasures(int componentRef, Rating expectedReviewRating, @Nullable Double expectedHotspotsReviewed) { | |||
assertThat(measureRepository.getAddedRawMeasure(componentRef, NEW_SECURITY_REVIEW_RATING_KEY)).hasVariation(expectedReviewRating.getIndex()); | |||
assertThat(measureRepository.getAddedRawMeasure(componentRef, NEW_SECURITY_HOTSPOTS_REVIEWED_KEY)).hasVariation(expectedHotspotsReviewed, | |||
VARIATION_COMPARISON_OFFSET); | |||
if (expectedHotspotsReviewed != null){ | |||
assertThat(measureRepository.getAddedRawMeasure(componentRef, NEW_SECURITY_HOTSPOTS_REVIEWED_KEY)).hasVariation(expectedHotspotsReviewed, | |||
VARIATION_COMPARISON_OFFSET); | |||
} else { | |||
assertThat(measureRepository.getAddedRawMeasure(componentRef, NEW_SECURITY_HOTSPOTS_REVIEWED_KEY)).isAbsent(); | |||
} | |||
} | |||
private void verifyHotspotStatusMeasures(int componentRef, @Nullable Integer hotspotsReviewed, @Nullable Integer hotspotsToReview) { |
@@ -247,12 +247,12 @@ public class SecurityReviewMeasuresVisitorTest { | |||
} | |||
@Test | |||
public void compute_A_rating_and_100_percent_when_no_hotspot() { | |||
public void compute_A_rating_and_no_reviewed_when_no_hotspot() { | |||
treeRootHolder.setRoot(ROOT_PROJECT); | |||
underTest.visit(ROOT_PROJECT); | |||
verifyRatingAndReviewedMeasures(PROJECT_REF, A, 100.0); | |||
verifyRatingAndReviewedMeasures(PROJECT_REF, A, null); | |||
} | |||
@Test | |||
@@ -289,9 +289,13 @@ public class SecurityReviewMeasuresVisitorTest { | |||
verifyHotspotStatusMeasures(PROJECT_REF, 0, 0); | |||
} | |||
private void verifyRatingAndReviewedMeasures(int componentRef, Rating expectedReviewRating, double expectedHotspotsReviewed) { | |||
private void verifyRatingAndReviewedMeasures(int componentRef, Rating expectedReviewRating, @Nullable Double expectedHotspotsReviewed) { | |||
verifySecurityReviewRating(componentRef, expectedReviewRating); | |||
verifySecurityHotspotsReviewed(componentRef, expectedHotspotsReviewed); | |||
if (expectedHotspotsReviewed != null) { | |||
verifySecurityHotspotsReviewed(componentRef, expectedHotspotsReviewed); | |||
} else { | |||
assertThat(measureRepository.getAddedRawMeasure(componentRef, SECURITY_HOTSPOTS_REVIEWED_KEY)).isEmpty(); | |||
} | |||
} | |||
private void verifySecurityReviewRating(int componentRef, Rating rating) { |
@@ -19,6 +19,8 @@ | |||
*/ | |||
package org.sonar.server.security; | |||
import java.util.Optional; | |||
import javax.annotation.Nullable; | |||
import org.sonar.server.measure.Rating; | |||
import static org.sonar.server.measure.Rating.A; | |||
@@ -33,16 +35,16 @@ public class SecurityReviewRating { | |||
// Only static method | |||
} | |||
public static double computePercent(long hotspotsToReview, long hotspotsReviewed) { | |||
public static Optional<Double> computePercent(long hotspotsToReview, long hotspotsReviewed) { | |||
long total = hotspotsToReview + hotspotsReviewed; | |||
if (total == 0) { | |||
return 100.0; | |||
return Optional.empty(); | |||
} | |||
return hotspotsReviewed * 100.0 / total; | |||
return Optional.of(hotspotsReviewed * 100.0 / total); | |||
} | |||
public static Rating computeRating(double percent) { | |||
if (percent >= 80.0) { | |||
public static Rating computeRating(@Nullable Double percent) { | |||
if (percent == null || percent >= 80.0) { | |||
return A; | |||
} else if (percent >= 70.0) { | |||
return B; |
@@ -67,10 +67,11 @@ public class SecurityReviewRatingTest { | |||
@Test | |||
public void compute_percent() { | |||
assertThat(computePercent(0, 10)).isEqualTo(100.0); | |||
assertThat(computePercent(1, 3)).isEqualTo(75.0); | |||
assertThat(computePercent(3, 4)).isEqualTo(57.14, DOUBLE_OFFSET); | |||
assertThat(computePercent(10, 10)).isEqualTo(50.0); | |||
assertThat(computePercent(10, 0)).isEqualTo(0.0); | |||
assertThat(computePercent(0, 0)).isEmpty(); | |||
assertThat(computePercent(0, 10).get()).isEqualTo(100.0); | |||
assertThat(computePercent(1, 3).get()).isEqualTo(75.0); | |||
assertThat(computePercent(3, 4).get()).isEqualTo(57.14, DOUBLE_OFFSET); | |||
assertThat(computePercent(10, 10).get()).isEqualTo(50.0); | |||
assertThat(computePercent(10, 0).get()).isEqualTo(0.0); | |||
} | |||
} |
@@ -110,12 +110,14 @@ public class IssueMetricFormulaFactoryImpl implements IssueMetricFormulaFactory | |||
(context, issues) -> context.setValue(RATING_BY_SEVERITY.get(issues.getHighestSeverityOfUnresolved(RuleType.VULNERABILITY, false).orElse(Severity.INFO)))), | |||
new IssueMetricFormula(CoreMetrics.SECURITY_REVIEW_RATING, false, | |||
(context, issues) -> context | |||
.setValue(computeRating(computePercent(issues.countHotspotsByStatus(Issue.STATUS_TO_REVIEW, false), issues.countHotspotsByStatus(Issue.STATUS_REVIEWED, false))))), | |||
(context, issues) -> { | |||
Optional<Double> percent = computePercent(issues.countHotspotsByStatus(Issue.STATUS_TO_REVIEW, false), issues.countHotspotsByStatus(Issue.STATUS_REVIEWED, false)); | |||
context.setValue(computeRating(percent.orElse(null))); | |||
}), | |||
new IssueMetricFormula(CoreMetrics.SECURITY_HOTSPOTS_REVIEWED, false, | |||
(context, issues) -> context | |||
.setValue(computePercent(issues.countHotspotsByStatus(Issue.STATUS_TO_REVIEW, false), issues.countHotspotsByStatus(Issue.STATUS_REVIEWED, false)))), | |||
(context, issues) -> computePercent(issues.countHotspotsByStatus(Issue.STATUS_TO_REVIEW, false), issues.countHotspotsByStatus(Issue.STATUS_REVIEWED, false)) | |||
.ifPresent(context::setValue)), | |||
new IssueMetricFormula(CoreMetrics.SECURITY_HOTSPOTS_REVIEWED_STATUS, false, | |||
(context, issues) -> context.setValue(issues.countHotspotsByStatus(Issue.STATUS_REVIEWED, false))), | |||
@@ -176,15 +178,13 @@ public class IssueMetricFormulaFactoryImpl implements IssueMetricFormulaFactory | |||
new IssueMetricFormula(CoreMetrics.NEW_SECURITY_REVIEW_RATING, true, | |||
(context, issues) -> { | |||
Rating rating = computeRating(computePercent(issues.countHotspotsByStatus(Issue.STATUS_TO_REVIEW, true), issues.countHotspotsByStatus(Issue.STATUS_REVIEWED, true))); | |||
context.setLeakValue(rating); | |||
Optional<Double> percent = computePercent(issues.countHotspotsByStatus(Issue.STATUS_TO_REVIEW, true), issues.countHotspotsByStatus(Issue.STATUS_REVIEWED, true)); | |||
context.setLeakValue(computeRating(percent.orElse(null))); | |||
}), | |||
new IssueMetricFormula(CoreMetrics.NEW_SECURITY_HOTSPOTS_REVIEWED, true, | |||
(context, issues) -> { | |||
double percent = computePercent(issues.countHotspotsByStatus(Issue.STATUS_TO_REVIEW, true), issues.countHotspotsByStatus(Issue.STATUS_REVIEWED, true)); | |||
context.setLeakValue(percent); | |||
}), | |||
(context, issues) -> computePercent(issues.countHotspotsByStatus(Issue.STATUS_TO_REVIEW, true), issues.countHotspotsByStatus(Issue.STATUS_REVIEWED, true)) | |||
.ifPresent(context::setLeakValue)), | |||
new IssueMetricFormula(CoreMetrics.NEW_SECURITY_HOTSPOTS_REVIEWED_STATUS, true, | |||
(context, issues) -> context.setLeakValue(issues.countHotspotsByStatus(Issue.STATUS_REVIEWED, true))), |
@@ -142,7 +142,7 @@ public class IssueMetricFormulaFactoryImplTest { | |||
.assertThatValueIs(CoreMetrics.SECURITY_HOTSPOTS_REVIEWED, 75.0); | |||
withNoIssues() | |||
.assertThatValueIs(CoreMetrics.SECURITY_HOTSPOTS_REVIEWED, 100.0); | |||
.assertNoValue(CoreMetrics.SECURITY_HOTSPOTS_REVIEWED); | |||
} | |||
@Test | |||
@@ -697,7 +697,7 @@ public class IssueMetricFormulaFactoryImplTest { | |||
.assertThatLeakValueIs(CoreMetrics.NEW_SECURITY_HOTSPOTS_REVIEWED, 75.0); | |||
withNoIssues() | |||
.assertThatLeakValueIs(CoreMetrics.NEW_SECURITY_HOTSPOTS_REVIEWED, 100.0); | |||
.assertNoLeakValue(CoreMetrics.NEW_SECURITY_HOTSPOTS_REVIEWED); | |||
} | |||
@Test | |||
@@ -850,12 +850,24 @@ public class IssueMetricFormulaFactoryImplTest { | |||
return this; | |||
} | |||
Verifier assertNoLeakValue(Metric metric) { | |||
TestContext context = run(metric, true); | |||
assertThat(context.ratingLeakValue).isNull(); | |||
return this; | |||
} | |||
Verifier assertThatValueIs(Metric metric, Rating expectedValue) { | |||
TestContext context = run(metric, false); | |||
assertThat(context.ratingValue).isNotNull().isEqualTo(expectedValue); | |||
return this; | |||
} | |||
Verifier assertNoValue(Metric metric) { | |||
TestContext context = run(metric, false); | |||
assertThat(context.ratingValue).isNull(); | |||
return this; | |||
} | |||
private TestContext run(Metric metric, boolean expectLeakFormula) { | |||
IssueMetricFormula formula = underTest.getFormulas().stream() | |||
.filter(f -> f.getMetric().getKey().equals(metric.getKey())) |