SONAR-12960 Make Security Review Rating more intuitive on Portfolios

4 files changed, 27 insertions, 226 deletions

diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewRatingVisitorForPortfolios.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewRatingVisitorForPortfolios.java
deleted file mode 100644
index a88e3d07c43..00000000000
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewRatingVisitorForPortfolios.java
+++ /dev/null
@@ -1,79 +0,0 @@
-/*
- * 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 java.util.Optional;
-import org.sonar.ce.task.projectanalysis.component.Component;
-import org.sonar.ce.task.projectanalysis.component.CrawlerDepthLimit;
-import org.sonar.ce.task.projectanalysis.component.TypeAwareVisitorAdapter;
-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;
-import org.sonar.ce.task.projectanalysis.metric.MetricRepository;
-import org.sonar.server.measure.Rating;
-import org.sonar.server.security.SecurityReviewRating;
-
-import static org.sonar.api.measures.CoreMetrics.NCLOC_KEY;
-import static org.sonar.api.measures.CoreMetrics.SECURITY_HOTSPOTS_KEY;
-import static org.sonar.api.measures.CoreMetrics.SECURITY_REVIEW_RATING_KEY;
-import static org.sonar.ce.task.projectanalysis.component.ViewAttributes.Type.APPLICATION;
-
-public class SecurityReviewRatingVisitorForPortfolios extends TypeAwareVisitorAdapter {
-
-  private final MeasureRepository measureRepository;
-  private final Metric nclocMetric;
-  private final Metric securityHostspotsMetric;
-  private final Metric securityReviewRatingMetric;
-
-  public SecurityReviewRatingVisitorForPortfolios(MeasureRepository measureRepository, MetricRepository metricRepository) {
-    super(CrawlerDepthLimit.SUBVIEW, Order.POST_ORDER);
-    this.measureRepository = measureRepository;
-    this.nclocMetric = metricRepository.getByKey(NCLOC_KEY);
-    this.securityHostspotsMetric = metricRepository.getByKey(SECURITY_HOTSPOTS_KEY);
-    this.securityReviewRatingMetric = metricRepository.getByKey(SECURITY_REVIEW_RATING_KEY);
-  }
-
-  @Override
-  public void visitView(Component view) {
-    if (view.getViewAttributes().getType().equals(APPLICATION)) {
-      return;
-    }
-    computeMeasure(view);
-  }
-
-  @Override
-  public void visitSubView(Component subView) {
-    computeMeasure(subView);
-  }
-
-  private void computeMeasure(Component component) {
-    Optional<Measure> nclocMeasure = measureRepository.getRawMeasure(component, nclocMetric);
-    Optional<Measure> securityHostspotsMeasure = measureRepository.getRawMeasure(component, securityHostspotsMetric);
-    if (!nclocMeasure.isPresent() || !securityHostspotsMeasure.isPresent()) {
-      return;
-    }
-    int ncloc = nclocMeasure.get().getIntValue();
-    int securityHotspots = securityHostspotsMeasure.get().getIntValue();
-    Rating rating = SecurityReviewRating.computeForPortfolios(ncloc, securityHotspots);
-    measureRepository.add(component, securityReviewRatingMetric, RatingMeasures.get(rating));
-  }
-
-}
diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewRatingVisitorForPortfoliosTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewRatingVisitorForPortfoliosTest.java
deleted file mode 100644
index 099113d525e..00000000000
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitymodel/SecurityReviewRatingVisitorForPortfoliosTest.java
+++ /dev/null
@@ -1,106 +0,0 @@
-/*
- * 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.junit.Rule;
-import org.junit.Test;
-import org.sonar.ce.task.projectanalysis.component.Component;
-import org.sonar.ce.task.projectanalysis.component.TreeRootHolderRule;
-import org.sonar.ce.task.projectanalysis.component.ViewAttributes;
-import org.sonar.ce.task.projectanalysis.component.ViewsComponent;
-import org.sonar.ce.task.projectanalysis.component.VisitorsCrawler;
-import org.sonar.ce.task.projectanalysis.measure.MeasureRepositoryRule;
-import org.sonar.ce.task.projectanalysis.metric.MetricRepositoryRule;
-
-import static java.util.Collections.singletonList;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.sonar.api.measures.CoreMetrics.NCLOC;
-import static org.sonar.api.measures.CoreMetrics.NCLOC_KEY;
-import static org.sonar.api.measures.CoreMetrics.SECURITY_HOTSPOTS;
-import static org.sonar.api.measures.CoreMetrics.SECURITY_HOTSPOTS_KEY;
-import static org.sonar.api.measures.CoreMetrics.SECURITY_REVIEW_RATING;
-import static org.sonar.api.measures.CoreMetrics.SECURITY_REVIEW_RATING_KEY;
-import static org.sonar.ce.task.projectanalysis.measure.Measure.newMeasureBuilder;
-import static org.sonar.server.measure.Rating.B;
-import static org.sonar.server.measure.Rating.C;
-
-public class SecurityReviewRatingVisitorForPortfoliosTest {
-
-  private static final int PORTFOLIO_REF = 10;
-  private static final int SUB_PORTFOLIO_1_REF = 11;
-  private static final int SUB_PORTFOLIO_2_REF = 12;
-  private static final Component PORTFOLIO = ViewsComponent.builder(Component.Type.VIEW, Integer.toString(PORTFOLIO_REF))
-    .setViewAttributes(new ViewAttributes(ViewAttributes.Type.PORTFOLIO))
-    .addChildren(
-      ViewsComponent.builder(Component.Type.SUBVIEW, Integer.toString(SUB_PORTFOLIO_1_REF)).build(),
-      ViewsComponent.builder(Component.Type.SUBVIEW, Integer.toString(SUB_PORTFOLIO_2_REF)).build())
-    .build();
-
-  @Rule
-  public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule();
-
-  @Rule
-  public MetricRepositoryRule metricRepository = new MetricRepositoryRule()
-    .add(NCLOC)
-    .add(SECURITY_HOTSPOTS)
-    .add(SECURITY_REVIEW_RATING);
-
-  @Rule
-  public MeasureRepositoryRule measureRepository = MeasureRepositoryRule.create(treeRootHolder, metricRepository);
-
-  private VisitorsCrawler underTest = new VisitorsCrawler(singletonList(new SecurityReviewRatingVisitorForPortfolios(measureRepository, metricRepository)));
-
-  @Test
-  public void compute_security_review_rating_on_portfolio() {
-    treeRootHolder.setRoot(PORTFOLIO);
-    measureRepository.addRawMeasure(PORTFOLIO_REF, NCLOC_KEY, newMeasureBuilder().create(2000));
-    measureRepository.addRawMeasure(PORTFOLIO_REF, SECURITY_HOTSPOTS_KEY, newMeasureBuilder().create(20));
-    measureRepository.addRawMeasure(SUB_PORTFOLIO_1_REF, NCLOC_KEY, newMeasureBuilder().create(1000));
-    measureRepository.addRawMeasure(SUB_PORTFOLIO_1_REF, SECURITY_HOTSPOTS_KEY, newMeasureBuilder().create(5));
-    measureRepository.addRawMeasure(SUB_PORTFOLIO_2_REF, NCLOC_KEY, newMeasureBuilder().create(1000));
-    measureRepository.addRawMeasure(SUB_PORTFOLIO_2_REF, SECURITY_HOTSPOTS_KEY, newMeasureBuilder().create(15));
-
-    underTest.visit(PORTFOLIO);
-
-    assertThat(measureRepository.getAddedRawMeasure(SUB_PORTFOLIO_1_REF, SECURITY_REVIEW_RATING_KEY).get().getIntValue()).isEqualTo(B.getIndex());
-    assertThat(measureRepository.getAddedRawMeasure(SUB_PORTFOLIO_2_REF, SECURITY_REVIEW_RATING_KEY).get().getIntValue()).isEqualTo(C.getIndex());
-    assertThat(measureRepository.getAddedRawMeasure(PORTFOLIO_REF, SECURITY_REVIEW_RATING_KEY).get().getIntValue()).isEqualTo(B.getIndex());
-  }
-
-  @Test
-  public void compute_nothing_when_no_ncloc() {
-    treeRootHolder.setRoot(PORTFOLIO);
-    measureRepository.addRawMeasure(PORTFOLIO_REF, SECURITY_HOTSPOTS_KEY, newMeasureBuilder().create(2));
-
-    underTest.visit(PORTFOLIO);
-
-    assertThat(measureRepository.getAddedRawMeasure(PORTFOLIO_REF, SECURITY_REVIEW_RATING_KEY)).isEmpty();
-  }
-
-  @Test
-  public void compute_nothing_when_no_security_hotspot() {
-    treeRootHolder.setRoot(PORTFOLIO);
-    measureRepository.addRawMeasure(PORTFOLIO_REF, NCLOC_KEY, newMeasureBuilder().create(1000));
-
-    underTest.visit(PORTFOLIO);
-
-    assertThat(measureRepository.getAddedRawMeasure(PORTFOLIO_REF, SECURITY_REVIEW_RATING_KEY)).isEmpty();
-  }
-}
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 f96839250fc..d2c038ccc97 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
@@ -33,28 +33,6 @@ public class SecurityReviewRating {
     // Only static method
   }
 
-  /**
-   * This code will be removed when updating computation of Security Review Rating for portfolios
-   */
-  @Deprecated
-  public static Rating computeForPortfolios(int ncloc, int securityHotspots) {
-    if (ncloc == 0) {
-      return A;
-    }
-    double ratio = (double) securityHotspots * 1000d / (double) ncloc;
-    if (ratio <= 3d) {
-      return A;
-    } else if (ratio <= 10) {
-      return B;
-    } else if (ratio <= 15) {
-      return C;
-    } else if (ratio <= 25) {
-      return D;
-    } else {
-      return E;
-    }
-  }
-
   public static double computePercent(long hotspotsToReview, long hotspotsReviewed) {
     long total = hotspotsToReview + hotspotsReviewed;
     if (total == 0) {
@@ -63,7 +41,7 @@ public class SecurityReviewRating {
     return hotspotsReviewed * 100.0 / total;
   }
 
-  public static Rating computeRating(Double percent) {
+  public static Rating computeRating(double percent) {
     if (percent >= 80.0) {
       return A;
     } else if (percent >= 70.0) {
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 cdb276aba16..7c4e5d44821 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
@@ -24,6 +24,7 @@ import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import com.tngtech.java.junit.dataprovider.UseDataProvider;
 import java.util.ArrayList;
 import java.util.List;
+import org.assertj.core.data.Offset;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.sonar.server.measure.Rating;
@@ -34,35 +35,42 @@ import static org.sonar.server.measure.Rating.B;
 import static org.sonar.server.measure.Rating.C;
 import static org.sonar.server.measure.Rating.D;
 import static org.sonar.server.measure.Rating.E;
+import static org.sonar.server.security.SecurityReviewRating.computePercent;
+import static org.sonar.server.security.SecurityReviewRating.computeRating;
 
 @RunWith(DataProviderRunner.class)
 public class SecurityReviewRatingTest {
 
+  private static final Offset<Double> DOUBLE_OFFSET = Offset.offset(0.01d);
+
   @DataProvider
   public static Object[][] values() {
     List<Object[]> res = new ArrayList<>();
-    res.add(new Object[] {1000, 0, A});
-    res.add(new Object[] {1000, 3, A});
-    res.add(new Object[] {1000, 4, B});
-    res.add(new Object[] {1000, 10, B});
-    res.add(new Object[] {1000, 11, C});
-    res.add(new Object[] {1000, 15, C});
-    res.add(new Object[] {1000, 16, D});
-    res.add(new Object[] {1000, 25, D});
-    res.add(new Object[] {1000, 26, E});
-    res.add(new Object[] {1000, 900, E});
-
-    res.add(new Object[] {0, 2, A});
-    res.add(new Object[] {1001, 3, A});
-    res.add(new Object[] {999, 3, B});
-    res.add(new Object[] {Integer.MAX_VALUE, Integer.MAX_VALUE, E});
-    return res.toArray(new Object[res.size()][3]);
+    res.add(new Object[] {100.0, A});
+    res.add(new Object[] {90.0, A});
+    res.add(new Object[] {80.0, A});
+    res.add(new Object[] {75.0, B});
+    res.add(new Object[] {70.0, B});
+    res.add(new Object[] {60, C});
+    res.add(new Object[] {50.0, C});
+    res.add(new Object[] {40.0, D});
+    res.add(new Object[] {30.0, D});
+    res.add(new Object[] {29.9, E});
+    return res.toArray(new Object[res.size()][2]);
   }
 
   @Test
   @UseDataProvider("values")
-  public void compute_security_review_rating_on_project(int ncloc, int securityHotspots, Rating expectedRating) {
-    assertThat(SecurityReviewRating.computeForPortfolios(ncloc, securityHotspots)).isEqualTo(expectedRating);
+  public void compute_rating(double percent, Rating expectedRating) {
+    assertThat(computeRating(percent)).isEqualTo(expectedRating);
   }
 
+  @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);
+  }
 }