SONAR-7237 Variation measures are set to current value when no past measures

2 files changed, 110 insertions, 53 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/ComputeMeasureVariationsStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/ComputeMeasureVariationsStep.java
index ba3b831a3e2..0cc9a4ddd7a 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/ComputeMeasureVariationsStep.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/ComputeMeasureVariationsStep.java
@@ -20,6 +20,7 @@
 package org.sonar.server.computation.step;
 
 import com.google.common.base.Function;
+import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
 import java.util.Collection;
 import java.util.HashMap;
@@ -48,6 +49,7 @@ import org.sonar.server.computation.period.PeriodsHolder;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.FluentIterable.from;
+import static java.lang.String.format;
 import static org.sonar.server.computation.component.Component.Type.DIRECTORY;
 import static org.sonar.server.computation.component.Component.Type.SUBVIEW;
 import static org.sonar.server.computation.component.ComponentVisitor.Order.PRE_ORDER;
@@ -100,14 +102,14 @@ public class ComputeMeasureVariationsStep implements ComputationStep {
 
     private final DbSession session;
     private final Set<Integer> metricIds;
-    private final Map<String, Metric> metricByKeys;
+    private final List<Metric> metrics;
 
-    public VariationMeasuresVisitor(DbSession session, Iterable<Metric> metrics) {
+    VariationMeasuresVisitor(DbSession session, List<Metric> metrics) {
       // measures on files are currently purged, so past measures are not available on files
       super(CrawlerDepthLimit.reportMaxDepth(DIRECTORY).withViewsMaxDepth(SUBVIEW), PRE_ORDER);
       this.session = session;
       this.metricIds = from(metrics).transform(MetricDtoToMetricId.INSTANCE).toSet();
-      this.metricByKeys = from(metrics).uniqueIndex(MetricToKey.INSTANCE);
+      this.metrics = metrics;
     }
 
     @Override
@@ -126,35 +128,19 @@ public class ComputeMeasureVariationsStep implements ComputationStep {
       return measuresWithVariationRepository;
     }
 
-    private void processMeasuresWithVariation(Component component, MeasuresWithVariationRepository measuresWithVariationRepository) {
-      for (MeasureWithVariations measureWithVariations : measuresWithVariationRepository.measures()) {
-        Metric metric = measureWithVariations.getMetric();
-        Measure measure = Measure.updatedMeasureBuilder(measureWithVariations.getMeasure())
-          .setVariations(new MeasureVariations(
-            measureWithVariations.getVariation(1),
-            measureWithVariations.getVariation(2),
-            measureWithVariations.getVariation(3),
-            measureWithVariations.getVariation(4),
-            measureWithVariations.getVariation(5)))
-          .create();
-        measureRepository.update(component, metric, measure);
-      }
-    }
-
     private void setVariationMeasures(Component component, List<PastMeasureDto> pastMeasures, int period, MeasuresWithVariationRepository measuresWithVariationRepository) {
       Map<MeasureKey, PastMeasureDto> pastMeasuresByMeasureKey = from(pastMeasures).uniqueIndex(pastMeasureToMeasureKey);
-      for (Map.Entry<String, Measure> entry : from(measureRepository.getRawMeasures(component).entries()).filter(NotDeveloperMeasure.INSTANCE)) {
-        String metricKey = entry.getKey();
-        Measure measure = entry.getValue();
-        PastMeasureDto pastMeasure = pastMeasuresByMeasureKey.get(new MeasureKey(metricKey, null));
-        if (pastMeasure != null && pastMeasure.hasValue()) {
-          Metric metric = metricByKeys.get(metricKey);
-          measuresWithVariationRepository.add(metric, measure, period, computeVariation(measure, pastMeasure.getValue()));
+      for (Metric metric : metrics) {
+        Optional<Measure> measure = measureRepository.getRawMeasure(component, metric);
+        if (measure.isPresent() && !measure.get().hasVariations()) {
+          PastMeasureDto pastMeasure = pastMeasuresByMeasureKey.get(new MeasureKey(metric.getKey(), null));
+          double pastValue = (pastMeasure != null && pastMeasure.hasValue()) ? pastMeasure.getValue() : 0d;
+          measuresWithVariationRepository.add(metric, measure.get(), period, computeVariation(measure.get(), pastValue));
         }
       }
     }
 
-    private double computeVariation(Measure measure, Double pastValue) {
+    private double computeVariation(Measure measure, double pastValue) {
       switch (measure.getValueType()) {
         case INT:
           return measure.getIntValue() - pastValue;
@@ -165,7 +151,22 @@ public class ComputeMeasureVariationsStep implements ComputationStep {
         case BOOLEAN:
           return (measure.getBooleanValue() ? 1d : 0d) - pastValue;
         default:
-          throw new IllegalArgumentException("Unsupported Measure.ValueType " + measure.getValueType());
+          throw new IllegalArgumentException(format("Unsupported Measure.ValueType on measure '%s'", measure));
+      }
+    }
+
+    private void processMeasuresWithVariation(Component component, MeasuresWithVariationRepository measuresWithVariationRepository) {
+      for (MeasureWithVariations measureWithVariations : measuresWithVariationRepository.measures()) {
+        Metric metric = measureWithVariations.getMetric();
+        Measure measure = Measure.updatedMeasureBuilder(measureWithVariations.getMeasure())
+          .setVariations(new MeasureVariations(
+            measureWithVariations.getVariation(1),
+            measureWithVariations.getVariation(2),
+            measureWithVariations.getVariation(3),
+            measureWithVariations.getVariation(4),
+            measureWithVariations.getVariation(5)))
+          .create();
+        measureRepository.update(component, metric, measure);
       }
     }
   }
@@ -195,7 +196,7 @@ public class ComputeMeasureVariationsStep implements ComputationStep {
     private final Measure measure;
     private final Double[] variations = new Double[5];
 
-    public MeasureWithVariations(Metric metric, Measure measure) {
+    MeasureWithVariations(Metric metric, Measure measure) {
       this.metric = metric;
       this.measure = measure;
     }
@@ -228,16 +229,6 @@ public class ComputeMeasureVariationsStep implements ComputationStep {
     }
   }
 
-  private enum MetricToKey implements Function<Metric, String> {
-    INSTANCE;
-
-    @Nullable
-    @Override
-    public String apply(@Nonnull Metric metric) {
-      return metric.getKey();
-    }
-  }
-
   private enum NumericMetric implements Predicate<Metric> {
     INSTANCE;
 
@@ -251,15 +242,6 @@ public class ComputeMeasureVariationsStep implements ComputationStep {
     }
   }
 
-  private enum NotDeveloperMeasure implements Predicate<Map.Entry<String, Measure>> {
-    INSTANCE;
-
-    @Override
-    public boolean apply(@Nonnull Map.Entry<String, Measure> input) {
-      return input.getValue().getDeveloper() == null;
-    }
-  }
-
   @Override
   public String getDescription() {
     return "Compute measure variations";
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/ReportComputeMeasureVariationsStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/ReportComputeMeasureVariationsStepTest.java
index dabf56743ec..972a45df642 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/step/ReportComputeMeasureVariationsStepTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/ReportComputeMeasureVariationsStepTest.java
@@ -37,6 +37,7 @@ import org.sonar.server.computation.component.DumbDeveloper;
 import org.sonar.server.computation.component.ReportComponent;
 import org.sonar.server.computation.measure.Measure;
 import org.sonar.server.computation.measure.MeasureRepositoryRule;
+import org.sonar.server.computation.measure.MeasureVariations;
 import org.sonar.server.computation.metric.Metric;
 import org.sonar.server.computation.metric.MetricImpl;
 import org.sonar.server.computation.metric.MetricRepositoryRule;
@@ -48,13 +49,13 @@ import static org.sonar.db.component.SnapshotTesting.createForComponent;
 import static org.sonar.db.component.SnapshotTesting.newSnapshotForProject;
 import static org.sonar.server.computation.measure.Measure.newMeasureBuilder;
 
-
 public class ReportComputeMeasureVariationsStepTest {
 
   static final Metric ISSUES_METRIC = new MetricImpl(1, "violations", "violations", Metric.MetricType.INT);
   static final Metric DEBT_METRIC = new MetricImpl(2, "sqale_index", "sqale_index", Metric.MetricType.WORK_DUR);
   static final Metric FILE_COMPLEXITY_METRIC = new MetricImpl(3, "file_complexity", "file_complexity", Metric.MetricType.FLOAT);
   static final Metric BUILD_BREAKER_METRIC = new MetricImpl(4, "build_breaker", "build_breaker", Metric.MetricType.BOOL);
+  static final Metric NEW_DEBT = new MetricImpl(5, "new_debt", "new_debt", Metric.MetricType.WORK_DUR);
 
   static final ComponentDto PROJECT_DTO = ComponentTesting.newProjectDto();
 
@@ -74,7 +75,8 @@ public class ReportComputeMeasureVariationsStepTest {
     .add(ISSUES_METRIC)
     .add(DEBT_METRIC)
     .add(FILE_COMPLEXITY_METRIC)
-    .add(BUILD_BREAKER_METRIC);
+    .add(BUILD_BREAKER_METRIC)
+    .add(NEW_DEBT);
   @Rule
   public MeasureRepositoryRule measureRepository = MeasureRepositoryRule.create(treeRootHolder, metricRepository);
 
@@ -151,6 +153,64 @@ public class ReportComputeMeasureVariationsStepTest {
   }
 
   @Test
+  public void set_zero_variation_when_no_change() {
+    // Project
+    SnapshotDto period1ProjectSnapshot = newSnapshotForProject(PROJECT_DTO);
+    dbClient.snapshotDao().insert(session, period1ProjectSnapshot);
+    dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 60d));
+
+    // Directory
+    ComponentDto directoryDto = ComponentTesting.newDirectory(PROJECT_DTO, "dir");
+    dbClient.componentDao().insert(session, directoryDto);
+    SnapshotDto period1DirectorySnapshot = createForComponent(directoryDto, period1ProjectSnapshot);
+    dbClient.snapshotDao().insert(session, period1DirectorySnapshot);
+    dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), directoryDto.getId(), period1DirectorySnapshot.getId(), 10d));
+    session.commit();
+
+    periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot));
+
+    Component directory = ReportComponent.builder(Component.Type.DIRECTORY, 2).setUuid(directoryDto.uuid()).build();
+    Component project = ReportComponent.builder(Component.Type.PROJECT, 1).setUuid(PROJECT_DTO.uuid()).addChildren(directory).build();
+    treeRootHolder.setRoot(project);
+
+    addRawMeasure(project, ISSUES_METRIC, newMeasureBuilder().create(60, null));
+    addRawMeasure(directory, ISSUES_METRIC, newMeasureBuilder().create(10, null));
+
+    underTest.execute();
+
+    assertThat(measureRepository.getRawMeasure(project, ISSUES_METRIC).get().getVariations().getVariation1()).isEqualTo(0d);
+    assertThat(measureRepository.getRawMeasure(directory, ISSUES_METRIC).get().getVariations().getVariation1()).isEqualTo(0d);
+  }
+
+  @Test
+  public void set_variation_to_raw_value_on_new_component() throws Exception {
+    // Project
+    SnapshotDto past1ProjectSnapshot = newSnapshotForProject(PROJECT_DTO).setCreatedAt(1000_000_000L);
+    SnapshotDto currentProjectSnapshot = newSnapshotForProject(PROJECT_DTO).setCreatedAt(2000_000_000L);
+    dbClient.snapshotDao().insert(session, past1ProjectSnapshot);
+    dbClient.snapshotDao().insert(session, currentProjectSnapshot);
+    dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), past1ProjectSnapshot.getId(), 60d));
+    dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), currentProjectSnapshot.getId(), 60d));
+    session.commit();
+
+    periodsHolder.setPeriods(newPeriod(1, past1ProjectSnapshot));
+
+    // Directory has just been added => no snapshot
+    Component directory = ReportComponent.builder(Component.Type.DIRECTORY, 2).setUuid("DIRECTORY").build();
+    Component project = ReportComponent.builder(Component.Type.PROJECT, 1).setUuid(PROJECT_DTO.uuid()).addChildren(directory).build();
+    treeRootHolder.setRoot(project);
+
+    addRawMeasure(project, ISSUES_METRIC, newMeasureBuilder().create(90, null));
+    addRawMeasure(directory, ISSUES_METRIC, newMeasureBuilder().create(10, null));
+
+    underTest.execute();
+
+    assertThat(measureRepository.getRawMeasure(project, ISSUES_METRIC).get().getVariations().getVariation1()).isEqualTo(30d);
+    // Variation should be the raw value
+    assertThat(measureRepository.getRawMeasure(directory, ISSUES_METRIC).get().getVariations().getVariation1()).isEqualTo(10d);
+  }
+
+  @Test
   public void set_variations_on_all_periods() {
     SnapshotDto period1ProjectSnapshot = newSnapshotForProject(PROJECT_DTO).setLast(false);
     SnapshotDto period2ProjectSnapshot = newSnapshotForProject(PROJECT_DTO).setLast(false);
@@ -198,8 +258,7 @@ public class ReportComputeMeasureVariationsStepTest {
       newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 60d),
       newMeasureDto(DEBT_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 10d),
       newMeasureDto(FILE_COMPLEXITY_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 2d),
-      newMeasureDto(BUILD_BREAKER_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 1d)
-      );
+      newMeasureDto(BUILD_BREAKER_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 1d));
     session.commit();
 
     periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot));
@@ -226,8 +285,7 @@ public class ReportComputeMeasureVariationsStepTest {
     SnapshotDto period1ProjectSnapshot = newSnapshotForProject(PROJECT_DTO);
     dbClient.snapshotDao().insert(session, period1ProjectSnapshot);
     dbClient.measureDao().insert(session,
-      newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 60d)
-      );
+      newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 60d));
     session.commit();
 
     periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot));
@@ -244,6 +302,23 @@ public class ReportComputeMeasureVariationsStepTest {
     assertThat(measureRepository.getRawMeasure(PROJECT, ISSUES_METRIC, developer).get().hasVariations()).isFalse();
   }
 
+  @Test
+  public void does_not_update_existing_variations() throws Exception {
+    SnapshotDto period1ProjectSnapshot = newSnapshotForProject(PROJECT_DTO);
+    dbClient.snapshotDao().insert(session, period1ProjectSnapshot);
+    dbClient.measureDao().insert(session, newMeasureDto(NEW_DEBT.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 60d));
+    session.commit();
+    periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot));
+    treeRootHolder.setRoot(PROJECT);
+
+    addRawMeasure(PROJECT, NEW_DEBT, newMeasureBuilder().setVariations(new MeasureVariations(10d)).createNoValue());
+
+    underTest.execute();
+
+    // As the measure has already variations it has been ignored, then variations will be the same
+    assertThat(measureRepository.getRawMeasure(PROJECT, NEW_DEBT).get().getVariations().getVariation1()).isEqualTo(10d);
+  }
+
   private static MeasureDto newMeasureDto(int metricId, long projectId, long snapshotId, double value) {
     return new MeasureDto().setMetricId(metricId).setComponentId(projectId).setSnapshotId(snapshotId).setValue(value);
   }