From: Julien Lancelot Date: Fri, 7 Feb 2020 10:16:05 +0000 (+0100) Subject: SONAR-12960 Make Security Review Rating more intuitive on Portfolios X-Git-Tag: 8.2.0.32929~84 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=3ebf6d2303098f010cff38df15a4ae5fb7a737f4;p=sonarqube.git SONAR-12960 Make Security Review Rating more intuitive on Portfolios --- 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 nclocMeasure = measureRepository.getRawMeasure(component, nclocMetric); - Optional 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_OFFSET = Offset.offset(0.01d); + @DataProvider public static Object[][] values() { List 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); + } }