From: Sébastien Lesaint <sebastien.lesaint@sonarsource.com>
Date: Thu, 14 Jan 2016 10:05:29 +0000 (+0100)
Subject: SONAR-7103 new_debt_ratio not based on new_technical_debt on non-file
X-Git-Tag: 5.4-M7
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=366217e521da38a4b17f7e6ada0f4d6612475580;p=sonarqube.git

SONAR-7103 new_debt_ratio not based on new_technical_debt on non-file
---

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/sqale/SqaleNewMeasuresVisitor.java b/server/sonar-server/src/main/java/org/sonar/server/computation/sqale/SqaleNewMeasuresVisitor.java
index 473eef51101..531137e8c93 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/sqale/SqaleNewMeasuresVisitor.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/sqale/SqaleNewMeasuresVisitor.java
@@ -54,7 +54,7 @@ import static org.sonar.server.computation.measure.MeasureVariations.newMeasureV
  * This visitor depends on {@link org.sonar.server.computation.issue.IntegrateIssuesVisitor} for the computation of
  * metric {@link CoreMetrics#NEW_TECHNICAL_DEBT}.
  */
-public class SqaleNewMeasuresVisitor extends PathAwareVisitorAdapter<SqaleNewMeasuresVisitor.NewDevelopmentCostCounter> {
+public class SqaleNewMeasuresVisitor extends PathAwareVisitorAdapter<SqaleNewMeasuresVisitor.NewTechDebtRatioCounter> {
   private static final Logger LOG = Loggers.get(SqaleNewMeasuresVisitor.class);
 
   private final ScmInfoRepository scmInfoRepository;
@@ -83,38 +83,37 @@ public class SqaleNewMeasuresVisitor extends PathAwareVisitorAdapter<SqaleNewMea
   }
 
   @Override
-  public void visitProject(Component project, Path<NewDevelopmentCostCounter> path) {
+  public void visitProject(Component project, Path<NewTechDebtRatioCounter> path) {
     computeAndSaveNewDebtRatioMeasure(project, path);
   }
 
   @Override
-  public void visitModule(Component module, Path<NewDevelopmentCostCounter> path) {
+  public void visitModule(Component module, Path<NewTechDebtRatioCounter> path) {
     computeAndSaveNewDebtRatioMeasure(module, path);
-    increaseNewDevCostOfParent(path);
+    increaseNewDebtAndDevCostOfParent(path);
   }
 
   @Override
-  public void visitDirectory(Component directory, Path<NewDevelopmentCostCounter> path) {
+  public void visitDirectory(Component directory, Path<NewTechDebtRatioCounter> path) {
     computeAndSaveNewDebtRatioMeasure(directory, path);
-    increaseNewDevCostOfParent(path);
+    increaseNewDebtAndDevCostOfParent(path);
   }
 
   @Override
-  public void visitFile(Component file, Path<NewDevelopmentCostCounter> path) {
+  public void visitFile(Component file, Path<NewTechDebtRatioCounter> path) {
     if (file.getFileAttributes().isUnitTest()) {
       return;
     }
 
     initNewDebtRatioCounter(file, path);
     computeAndSaveNewDebtRatioMeasure(file, path);
-    increaseNewDevCostOfParent(path);
+    increaseNewDebtAndDevCostOfParent(path);
   }
 
-  private void computeAndSaveNewDebtRatioMeasure(Component component, Path<NewDevelopmentCostCounter> path) {
+  private void computeAndSaveNewDebtRatioMeasure(Component component, Path<NewTechDebtRatioCounter> path) {
     MeasureVariations.Builder builder = newMeasureVariationsBuilder();
     for (Period period : periodsHolder.getPeriods()) {
-      long newDevCost = path.current().getValue(period).getValue();
-      double density = computeDensity(component, period, newDevCost);
+      double density = computeDensity(path.current(), period);
       builder.setVariation(period, 100.0 * density);
     }
     if (!builder.isEmpty()) {
@@ -123,10 +122,13 @@ public class SqaleNewMeasuresVisitor extends PathAwareVisitorAdapter<SqaleNewMea
     }
   }
 
-  private double computeDensity(Component component, Period period, long developmentCost) {
-    double debt = getLongValue(measureRepository.getRawMeasure(component, this.newDebtMetric), period);
-    if (developmentCost != 0L) {
-      return debt / (double) developmentCost;
+  private double computeDensity(NewTechDebtRatioCounter counter, Period period) {
+    LongVariationValue newDebt = counter.getNewDebt(period);
+    if (newDebt.isSet()) {
+      long developmentCost = counter.getDevCost(period).getValue();
+      if (developmentCost != 0L) {
+        return newDebt.getValue() / (double) developmentCost;
+      }
     }
     return 0d;
   }
@@ -145,7 +147,7 @@ public class SqaleNewMeasuresVisitor extends PathAwareVisitorAdapter<SqaleNewMea
     return 0L;
   }
 
-  private void initNewDebtRatioCounter(Component file, Path<NewDevelopmentCostCounter> path) {
+  private void initNewDebtRatioCounter(Component file, Path<NewTechDebtRatioCounter> path) {
     // first analysis, no period, no differential value to compute, save processing time and return now
     if (periodsHolder.getPeriods().isEmpty()) {
       return;
@@ -163,22 +165,31 @@ public class SqaleNewMeasuresVisitor extends PathAwareVisitorAdapter<SqaleNewMea
     }
 
     ScmInfo scmInfo = scmInfoOptional.get();
-    initNewDebtRatioCounter(path.current(), file.getFileAttributes().getLanguageKey(), nclocDataMeasure.get(), scmInfo);
+    initNewDebtRatioCounter(path.current(), file, nclocDataMeasure.get(), scmInfo);
   }
 
-  private void initNewDebtRatioCounter(NewDevelopmentCostCounter devCostCounter, String languageKey, Measure nclocDataMeasure, ScmInfo scmInfo) {
-    long lineDevCost = sqaleRatingSettings.getDevCost(languageKey);
+  private void initNewDebtRatioCounter(NewTechDebtRatioCounter devCostCounter, Component file, Measure nclocDataMeasure, ScmInfo scmInfo) {
+    boolean[] hasDevCost = new boolean[PeriodsHolder.MAX_NUMBER_OF_PERIODS];
+
+    long lineDevCost = sqaleRatingSettings.getDevCost(file.getFileAttributes().getLanguageKey());
     for (Integer nclocLineIndex : nclocLineIndexes(nclocDataMeasure)) {
       Changeset changeset = scmInfo.getChangesetForLine(nclocLineIndex);
       for (Period period : periodsHolder.getPeriods()) {
         if (isLineInPeriod(changeset.getDate(), period)) {
-          devCostCounter.increment(period, lineDevCost);
+          devCostCounter.incrementDevCost(period, lineDevCost);
+          hasDevCost[period.getIndex()] = true;
         }
       }
     }
+    for (Period period : periodsHolder.getPeriods()) {
+      if (hasDevCost[period.getIndex()]) {
+        long newDebt = getLongValue(measureRepository.getRawMeasure(file, this.newDebtMetric), period);
+        devCostCounter.incrementNewDebt(period, newDebt);
+      }
+    }
   }
 
-  private static void increaseNewDevCostOfParent(Path<NewDevelopmentCostCounter> path) {
+  private static void increaseNewDebtAndDevCostOfParent(Path<NewTechDebtRatioCounter> path) {
     path.parent().add(path.current());
   }
 
@@ -202,19 +213,29 @@ public class SqaleNewMeasuresVisitor extends PathAwareVisitorAdapter<SqaleNewMea
       .transform(MapEntryToKey.INSTANCE);
   }
 
-  public static final class NewDevelopmentCostCounter {
-    private final LongVariationValue.Array devCosts = LongVariationValue.newArray();
+  public static final class NewTechDebtRatioCounter {
+    private final LongVariationValue.Array newDebt = LongVariationValue.newArray();
+    private final LongVariationValue.Array devCost = LongVariationValue.newArray();
+
+    public void add(NewTechDebtRatioCounter counter) {
+      this.newDebt.incrementAll(counter.newDebt);
+      this.devCost.incrementAll(counter.devCost);
+    }
+
+    public LongVariationValue.Array incrementNewDebt(Period period, long value) {
+      return newDebt.increment(period, value);
+    }
 
-    public void add(NewDevelopmentCostCounter counter) {
-      this.devCosts.incrementAll(counter.devCosts);
+    public LongVariationValue.Array incrementDevCost(Period period, long value) {
+      return devCost.increment(period, value);
     }
 
-    public LongVariationValue.Array increment(Period period, long value) {
-      return devCosts.increment(period, value);
+    public LongVariationValue getNewDebt(Period period) {
+      return this.newDebt.get(period);
     }
 
-    public LongVariationValue getValue(Period period) {
-      return this.devCosts.get(period);
+    public LongVariationValue getDevCost(Period period) {
+      return this.devCost.get(period);
     }
 
   }
@@ -238,12 +259,12 @@ public class SqaleNewMeasuresVisitor extends PathAwareVisitorAdapter<SqaleNewMea
     }
   }
 
-  private static class NewDevelopmentCostCounterFactory extends SimpleStackElementFactory<NewDevelopmentCostCounter> {
+  private static class NewDevelopmentCostCounterFactory extends SimpleStackElementFactory<NewTechDebtRatioCounter> {
     public static final NewDevelopmentCostCounterFactory INSTANCE = new NewDevelopmentCostCounterFactory();
 
     @Override
-    public NewDevelopmentCostCounter createForAny(Component component) {
-      return new NewDevelopmentCostCounter();
+    public NewTechDebtRatioCounter createForAny(Component component) {
+      return new NewTechDebtRatioCounter();
     }
   }
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/sqale/SqaleNewMeasuresVisitorTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/sqale/SqaleNewMeasuresVisitorTest.java
index 71a8e277578..44ea83c022d 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/sqale/SqaleNewMeasuresVisitorTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/sqale/SqaleNewMeasuresVisitorTest.java
@@ -118,6 +118,7 @@ public class SqaleNewMeasuresVisitorTest {
     periodsHolder.setPeriods();
     when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST);
     setupOneFileAloneInAProject(50, 12, Flag.SRC_FILE, Flag.WITH_NCLOC, Flag.WITH_CHANGESET);
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(50, 12));
 
     underTest.visit(treeRootHolder.getRoot());
 
@@ -148,6 +149,7 @@ public class SqaleNewMeasuresVisitorTest {
   public void file_has_new_debt_ratio_if_some_scm_dates_are_after_snapshot_dates() {
     when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST);
     setupOneFileAloneInAProject(50, 12, Flag.SRC_FILE, Flag.WITH_NCLOC, Flag.WITH_CHANGESET);
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(50, 12));
 
     underTest.visit(treeRootHolder.getRoot());
 
@@ -159,6 +161,7 @@ public class SqaleNewMeasuresVisitorTest {
   public void new_debt_ratio_changes_with_language_cost() {
     when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST * 10);
     setupOneFileAloneInAProject(50, 12, Flag.SRC_FILE, Flag.WITH_NCLOC, Flag.WITH_CHANGESET);
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(50, 12));
 
     underTest.visit(treeRootHolder.getRoot());
 
@@ -170,6 +173,19 @@ public class SqaleNewMeasuresVisitorTest {
   public void new_debt_ratio_changes_with_new_technical_debt() {
     when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST);
     setupOneFileAloneInAProject(500, 120, Flag.SRC_FILE, Flag.WITH_NCLOC, Flag.WITH_CHANGESET);
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(500, 120));
+
+    underTest.visit(treeRootHolder.getRoot());
+
+    assertNewDebtRatioValues(LANGUAGE_1_FILE_REF, 833.33, 0);
+    assertNewDebtRatioValues(ROOT_REF, 833.33, 0);
+  }
+
+  @Test
+  public void new_debt_ratio_on_non_file_level_is_based_on_new_technical_debt_of_that_level() {
+    when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST);
+    setupOneFileAloneInAProject(500, 120, Flag.SRC_FILE, Flag.WITH_NCLOC, Flag.WITH_CHANGESET);
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(1200, 820));
 
     underTest.visit(treeRootHolder.getRoot());
 
@@ -181,6 +197,19 @@ public class SqaleNewMeasuresVisitorTest {
   public void no_new_debt_ratio_when_file_is_unit_test() {
     when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST);
     setupOneFileAloneInAProject(50, 12, Flag.UT_FILE, Flag.WITH_NCLOC, Flag.WITH_CHANGESET);
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(50, 12));
+
+    underTest.visit(treeRootHolder.getRoot());
+
+    assertNoNewDebtRatioMeasure(LANGUAGE_1_FILE_REF);
+    assertNewDebtRatioValues(ROOT_REF, 0, 0);
+  }
+
+  @Test
+  public void new_debt_ratio_is_0_on_non_file_level_when_all_files_are_unit_test() {
+    when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST);
+    setupOneFileAloneInAProject(50, 12, Flag.UT_FILE, Flag.WITH_NCLOC, Flag.WITH_CHANGESET);
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(200, 162));
 
     underTest.visit(treeRootHolder.getRoot());
 
@@ -192,6 +221,19 @@ public class SqaleNewMeasuresVisitorTest {
   public void new_debt_ratio_is_0_when_file_has_no_changesets() {
     when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST);
     setupOneFileAloneInAProject(50, 12, Flag.SRC_FILE, Flag.WITH_NCLOC, Flag.NO_CHANGESET);
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(50, 12));
+
+    underTest.visit(treeRootHolder.getRoot());
+
+    assertNewDebtRatioValues(LANGUAGE_1_FILE_REF, 0, 0);
+    assertNewDebtRatioValues(ROOT_REF, 0, 0);
+  }
+
+  @Test
+  public void new_debt_ratio_is_0_on_non_file_level_when_no_file_has_changesets() {
+    when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST);
+    setupOneFileAloneInAProject(50, 12, Flag.SRC_FILE, Flag.WITH_NCLOC, Flag.NO_CHANGESET);
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(200, 162));
 
     underTest.visit(treeRootHolder.getRoot());
 
@@ -203,6 +245,19 @@ public class SqaleNewMeasuresVisitorTest {
   public void new_debt_ratio_is_0_when_there_is_no_ncloc_in_file() {
     when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST);
     setupOneFileAloneInAProject(50, 12, Flag.SRC_FILE, Flag.NO_NCLOC, Flag.WITH_CHANGESET);
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(50, 12));
+
+    underTest.visit(treeRootHolder.getRoot());
+
+    assertNewDebtRatioValues(LANGUAGE_1_FILE_REF, 0, 0);
+    assertNewDebtRatioValues(ROOT_REF, 0, 0);
+  }
+
+  @Test
+  public void new_debt_ratio_is_0_on_non_file_level_when_one_file_has_zero_new_debt_because_of_no_changeset() {
+    when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST);
+    setupOneFileAloneInAProject(50, 12, Flag.SRC_FILE, Flag.NO_NCLOC, Flag.WITH_CHANGESET);
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(200, 162));
 
     underTest.visit(treeRootHolder.getRoot());
 
@@ -214,6 +269,7 @@ public class SqaleNewMeasuresVisitorTest {
   public void new_debt_ratio_is_0_when_ncloc_measure_is_missing() {
     when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST);
     setupOneFileAloneInAProject(50, 12, Flag.SRC_FILE, Flag.MISSING_MEASURE_NCLOC, Flag.WITH_CHANGESET);
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(50, 12));
 
     underTest.visit(treeRootHolder.getRoot());
 
@@ -222,7 +278,7 @@ public class SqaleNewMeasuresVisitorTest {
   }
 
   @Test
-  public void no_leaf_components_always_have_a_measure_when_at_least_one_period_exist() {
+  public void leaf_components_always_have_a_measure_when_at_least_one_period_exist_and_ratio_is_computed_from_current_level_new_debt() {
     when(sqaleRatingSettings.getDevCost(LANGUAGE_1_KEY)).thenReturn(LANGUAGE_1_DEV_COST);
     treeRootHolder.setRoot(
       builder(PROJECT, ROOT_REF)
@@ -239,9 +295,9 @@ public class SqaleNewMeasuresVisitorTest {
 
     Measure newDebtMeasure = createNewDebtMeasure(50, 12);
     measureRepository.addRawMeasure(LANGUAGE_1_FILE_REF, NEW_TECHNICAL_DEBT_KEY, newDebtMeasure);
-    measureRepository.addRawMeasure(111, NEW_TECHNICAL_DEBT_KEY, newDebtMeasure);
-    measureRepository.addRawMeasure(11, NEW_TECHNICAL_DEBT_KEY, newDebtMeasure);
-    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, newDebtMeasure);
+    measureRepository.addRawMeasure(111, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(150, 112));
+    measureRepository.addRawMeasure(11, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(200, 112));
+    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, createNewDebtMeasure(250, 212));
     // 4 lines file, only first one is not ncloc
     measureRepository.addRawMeasure(LANGUAGE_1_FILE_REF, NCLOC_DATA_KEY, createNclocDataMeasure(2, 3, 4));
     // first 2 lines are before all snapshots, 2 last lines are after PERIOD 2's snapshot date
@@ -270,7 +326,6 @@ public class SqaleNewMeasuresVisitorTest {
 
     Measure newDebtMeasure = createNewDebtMeasure(newDebtPeriod2, newDebtPeriod4);
     measureRepository.addRawMeasure(LANGUAGE_1_FILE_REF, NEW_TECHNICAL_DEBT_KEY, newDebtMeasure);
-    measureRepository.addRawMeasure(ROOT_REF, NEW_TECHNICAL_DEBT_KEY, newDebtMeasure);
     if (withNclocLines == Flag.WITH_NCLOC) {
       // 4 lines file, only first one is not ncloc
       measureRepository.addRawMeasure(LANGUAGE_1_FILE_REF, NCLOC_DATA_KEY, createNclocDataMeasure(2, 3, 4));