SONAR-10193 Align Technical Debt rating calculation with docs

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 9114db765562623083de9e83adbe8fd51a5a7c7a..30e0cf2519e6d790b5ce1bcf4f47815838e4c100 100644 (file)
--- 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<Rating, Bounds> 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<Rating, Bounds> buildRatingBounds(double[] gridValues) {
+    checkState(gridValues.length == 4, "Rating grid should contains 4 values");
+    EnumMap<Rating, Bounds> 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 596697900d9837666c3f1a08b833eed45f6989e1..8f5b2e7ad12310bddc0d525ce82dedbd0847623d 100644 (file)
--- 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 8e7d34e9f535af91cb232a697b67b54e29828637..ca7fb566f65133812b26e6de12d07f65a93c1bcf 100644 (file)
--- 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)