SONAR-12960 Do not compute Security Hotspots Reviewed when 0 hotspots

8 files changed, 65 insertions, 40 deletions

diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitor.java
index 806915dded5..cbd94f0f20b 100644
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitor.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitor.java
@@ -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());
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 574d53a74e3..2cb2741724d 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,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());
diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitorTest.java
index c39752eb775..bf873feccda 100644
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitorTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/NewSecurityReviewMeasuresVisitorTest.java
@@ -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) {
diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewMeasuresVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewMeasuresVisitorTest.java
index 9780f4e72f3..cf47649b6e5 100644
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewMeasuresVisitorTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewMeasuresVisitorTest.java
@@ -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) {
diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityReviewRating.java b/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityReviewRating.java
index d2c038ccc97..06999dd581d 100644
--- a/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityReviewRating.java
+++ b/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityReviewRating.java
@@ -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;
diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/security/SecurityReviewRatingTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/security/SecurityReviewRatingTest.java
index 7c4e5d44821..188401492e8 100644
--- a/server/sonar-server-common/src/test/java/org/sonar/server/security/SecurityReviewRatingTest.java
+++ b/server/sonar-server-common/src/test/java/org/sonar/server/security/SecurityReviewRatingTest.java
@@ -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);
   }
 }
diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/IssueMetricFormulaFactoryImpl.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/IssueMetricFormulaFactoryImpl.java
index d4a63507f33..1d9cbfd14de 100644
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/IssueMetricFormulaFactoryImpl.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/IssueMetricFormulaFactoryImpl.java
@@ -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))),
diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/measure/live/IssueMetricFormulaFactoryImplTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/measure/live/IssueMetricFormulaFactoryImplTest.java
index 7f92edb76e0..aec34528be5 100644
--- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/measure/live/IssueMetricFormulaFactoryImplTest.java
+++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/measure/live/IssueMetricFormulaFactoryImplTest.java
@@ -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()))