From cd45a1067c9f249f9fd3e28a4f4fb957c846906a Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 3 Jan 2018 14:23:10 +0100 Subject: [PATCH] SONAR-10193 Align Technical Debt rating calculation with docs --- .../qualitymodel/DebtRatingGrid.java | 63 +++++++++++++++---- .../qualitymodel/DebtRatingGridTest.java | 38 +++++++---- .../IssueMetricFormulaFactoryImplTest.java | 8 +-- 3 files changed, 81 insertions(+), 28 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/qualitymodel/DebtRatingGrid.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/qualitymodel/DebtRatingGrid.java index 9114db76556..30e0cf2519e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/qualitymodel/DebtRatingGrid.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/qualitymodel/DebtRatingGrid.java @@ -21,19 +21,24 @@ package org.sonar.server.computation.task.projectanalysis.qualitymodel; import com.google.common.annotations.VisibleForTesting; import java.util.Arrays; +import java.util.EnumMap; +import java.util.Map; import org.sonar.api.config.Configuration; -import org.sonar.api.utils.MessageException; +import static com.google.common.base.Preconditions.checkState; +import static java.lang.String.format; import static org.sonar.api.CoreProperties.RATING_GRID; import static org.sonar.api.CoreProperties.RATING_GRID_DEF_VALUES; +import static org.sonar.server.computation.task.projectanalysis.qualitymodel.Rating.A; +import static org.sonar.server.computation.task.projectanalysis.qualitymodel.Rating.B; +import static org.sonar.server.computation.task.projectanalysis.qualitymodel.Rating.C; +import static org.sonar.server.computation.task.projectanalysis.qualitymodel.Rating.D; +import static org.sonar.server.computation.task.projectanalysis.qualitymodel.Rating.E; public class DebtRatingGrid { private final double[] gridValues; - - public DebtRatingGrid(double[] gridValues) { - this.gridValues = Arrays.copyOf(gridValues, gridValues.length); - } + private final EnumMap ratingBounds; public DebtRatingGrid(Configuration config) { try { @@ -42,20 +47,35 @@ public class DebtRatingGrid { for (int i = 0; i < 4; i++) { gridValues[i] = Double.parseDouble(grades[i]); } + this.ratingBounds = buildRatingBounds(gridValues); } catch (Exception e) { throw new IllegalArgumentException("The rating grid is incorrect. Expected something similar to '" + RATING_GRID_DEF_VALUES + "' and got '" + config.get(RATING_GRID).orElse("") + "'", e); } } - public Rating getRatingForDensity(double density) { - for (Rating rating : Rating.values()) { - double lowerBound = getGradeLowerBound(rating); - if (density >= lowerBound) { - return rating; - } - } - throw MessageException.of("The rating density value should be between 0 and " + Double.MAX_VALUE + " and got " + density); + public DebtRatingGrid(double[] gridValues) { + this.gridValues = Arrays.copyOf(gridValues, gridValues.length); + this.ratingBounds = buildRatingBounds(gridValues); + } + + private static EnumMap buildRatingBounds(double[] gridValues) { + checkState(gridValues.length == 4, "Rating grid should contains 4 values"); + EnumMap ratingBounds = new EnumMap<>(Rating.class); + ratingBounds.put(A, new Bounds(0d, gridValues[0])); + ratingBounds.put(B, new Bounds(gridValues[0], gridValues[1])); + ratingBounds.put(C, new Bounds(gridValues[1], gridValues[2])); + ratingBounds.put(D, new Bounds(gridValues[2], gridValues[3])); + ratingBounds.put(E, new Bounds(gridValues[3], Double.MAX_VALUE)); + return ratingBounds; + } + + public Rating getRatingForDensity(double value) { + return ratingBounds.entrySet().stream() + .filter(e -> e.getValue().match(value)) + .map(Map.Entry::getKey) + .findFirst() + .orElseThrow(() -> new IllegalArgumentException(format("Invalid value '%s'", value))); } public double getGradeLowerBound(Rating rating) { @@ -70,4 +90,21 @@ public class DebtRatingGrid { return gridValues; } + private static class Bounds { + private final double lowerBound; + private final double higherBound; + private final boolean isLowerBoundInclusive; + + private Bounds(double lowerBound, double higherBound) { + this.lowerBound = lowerBound; + this.higherBound = higherBound; + this.isLowerBoundInclusive = lowerBound == 0; + } + + boolean match(double value) { + boolean lowerBoundMatch = isLowerBoundInclusive ? (value >= lowerBound) : (value > lowerBound); + return lowerBoundMatch && value <= higherBound; + } + } + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/qualitymodel/DebtRatingGridTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/qualitymodel/DebtRatingGridTest.java index 596697900d9..8f5b2e7ad12 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/qualitymodel/DebtRatingGridTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/qualitymodel/DebtRatingGridTest.java @@ -25,7 +25,6 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import static org.assertj.core.api.Assertions.assertThat; - import static org.sonar.server.computation.task.projectanalysis.qualitymodel.Rating.A; import static org.sonar.server.computation.task.projectanalysis.qualitymodel.Rating.B; import static org.sonar.server.computation.task.projectanalysis.qualitymodel.Rating.C; @@ -50,25 +49,26 @@ public class DebtRatingGridTest { assertThat(ratingGrid.getRatingForDensity(0)).isEqualTo(A); assertThat(ratingGrid.getRatingForDensity(0.05)).isEqualTo(A); assertThat(ratingGrid.getRatingForDensity(0.09999999)).isEqualTo(A); - assertThat(ratingGrid.getRatingForDensity(0.1)).isEqualTo(B); + assertThat(ratingGrid.getRatingForDensity(0.1)).isEqualTo(A); assertThat(ratingGrid.getRatingForDensity(0.15)).isEqualTo(B); - assertThat(ratingGrid.getRatingForDensity(0.2)).isEqualTo(C); + assertThat(ratingGrid.getRatingForDensity(0.2)).isEqualTo(B); assertThat(ratingGrid.getRatingForDensity(0.25)).isEqualTo(C); - assertThat(ratingGrid.getRatingForDensity(0.5)).isEqualTo(D); + assertThat(ratingGrid.getRatingForDensity(0.5)).isEqualTo(C); assertThat(ratingGrid.getRatingForDensity(0.65)).isEqualTo(D); - assertThat(ratingGrid.getRatingForDensity(1)).isEqualTo(E); + assertThat(ratingGrid.getRatingForDensity(1)).isEqualTo(D); assertThat(ratingGrid.getRatingForDensity(1.01)).isEqualTo(E); } @Test - public void fail_on_invalid_density() { - throwable.expect(RuntimeException.class); - - ratingGrid.getRatingForDensity(-1); + public void density_matching_exact_grid_values() { + assertThat(ratingGrid.getRatingForDensity(0.1)).isEqualTo(A); + assertThat(ratingGrid.getRatingForDensity(0.2)).isEqualTo(B); + assertThat(ratingGrid.getRatingForDensity(0.5)).isEqualTo(C); + assertThat(ratingGrid.getRatingForDensity(1)).isEqualTo(D); } @Test - public void convert_int_to_rating() throws Exception { + public void convert_int_to_rating() { assertThat(Rating.valueOf(1)).isEqualTo(A); assertThat(Rating.valueOf(2)).isEqualTo(B); assertThat(Rating.valueOf(3)).isEqualTo(C); @@ -77,8 +77,24 @@ public class DebtRatingGridTest { } @Test - public void fail_to_concert_invalid_value() throws Exception { + public void fail_on_invalid_density() { + throwable.expect(IllegalArgumentException.class); + throwable.expectMessage("Invalid value '-1.0'"); + + ratingGrid.getRatingForDensity(-1); + } + + @Test + public void fail_to_concert_invalid_value() { throwable.expect(IllegalArgumentException.class); Rating.valueOf(10); } + + @Test + public void fail_on_invalid_grid() { + throwable.expect(IllegalStateException.class); + throwable.expectMessage("Rating grid should contains 4 values"); + + ratingGrid = new DebtRatingGrid(new double[] {0.1, 0.2, 0.5}); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/measure/live/IssueMetricFormulaFactoryImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/measure/live/IssueMetricFormulaFactoryImplTest.java index 8e7d34e9f53..ca7fb566f65 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/measure/live/IssueMetricFormulaFactoryImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/measure/live/IssueMetricFormulaFactoryImplTest.java @@ -254,11 +254,11 @@ public class IssueMetricFormulaFactoryImplTest { .assertThatValueIs(CoreMetrics.SQALE_DEBT_RATIO, 200.0) .assertThatValueIs(CoreMetrics.SQALE_RATING, Rating.E); - // B is 5% --> min debt is exactly 200*0.05=10 + // A is 5% --> min debt is exactly 200*0.05=10 with(CoreMetrics.DEVELOPMENT_COST, 200.0) .and(CoreMetrics.TECHNICAL_DEBT, 10.0) .assertThatValueIs(CoreMetrics.SQALE_DEBT_RATIO, 5.0) - .assertThatValueIs(CoreMetrics.SQALE_RATING, Rating.B); + .assertThatValueIs(CoreMetrics.SQALE_RATING, Rating.A); with(CoreMetrics.TECHNICAL_DEBT, 0.0) .and(CoreMetrics.DEVELOPMENT_COST, 0.0) @@ -630,11 +630,11 @@ public class IssueMetricFormulaFactoryImplTest { .assertThatLeakValueIs(CoreMetrics.NEW_SQALE_DEBT_RATIO, 200.0) .assertThatLeakValueIs(CoreMetrics.NEW_MAINTAINABILITY_RATING, Rating.E); - // B is 5% --> min debt is exactly 200*0.05=10 + // A is 5% --> min debt is exactly 200*0.05=10 withLeak(CoreMetrics.NEW_DEVELOPMENT_COST, 200.0) .andLeak(CoreMetrics.NEW_TECHNICAL_DEBT, 10.0) .assertThatLeakValueIs(CoreMetrics.NEW_SQALE_DEBT_RATIO, 5.0) - .assertThatLeakValueIs(CoreMetrics.NEW_MAINTAINABILITY_RATING, Rating.B); + .assertThatLeakValueIs(CoreMetrics.NEW_MAINTAINABILITY_RATING, Rating.A); withLeak(CoreMetrics.NEW_TECHNICAL_DEBT, 0.0) .andLeak(CoreMetrics.NEW_DEVELOPMENT_COST, 0.0) -- 2.39.5