SONAR-3437 Improve code coverage and fix measure data update

6 files changed, 123 insertions, 50 deletions

diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/MeasurePersister.java b/sonar-batch/src/main/java/org/sonar/batch/index/MeasurePersister.java
index d791be13e93..0c0cf825354 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/index/MeasurePersister.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/index/MeasurePersister.java
@@ -115,11 +115,15 @@ public final class MeasurePersister {
       && measure.getTendency() == null
       && measure.getUrl() == null
       && !measure.hasData()
-      && (measure.getVariation1() == null || NumberUtils.compare(measure.getVariation1().doubleValue(), 0.0) == 0)
-      && (measure.getVariation2() == null || NumberUtils.compare(measure.getVariation2().doubleValue(), 0.0) == 0)
-      && (measure.getVariation3() == null || NumberUtils.compare(measure.getVariation3().doubleValue(), 0.0) == 0)
-      && (measure.getVariation4() == null || NumberUtils.compare(measure.getVariation4().doubleValue(), 0.0) == 0)
-      && (measure.getVariation5() == null || NumberUtils.compare(measure.getVariation5().doubleValue(), 0.0) == 0);
+      && isZeroVariation(measure.getVariation1())
+      && isZeroVariation(measure.getVariation2())
+      && isZeroVariation(measure.getVariation3())
+      && isZeroVariation(measure.getVariation4())
+      && isZeroVariation(measure.getVariation5());
+  }
+
+  private static boolean isZeroVariation(Double variation) {
+    return (variation == null) || NumberUtils.compare(variation.doubleValue(), 0.0) == 0;
   }
 
   private List<MeasureModel> getMeasuresToSave() {
@@ -223,6 +227,10 @@ public final class MeasurePersister {
       MeasureMapper mapper = session.getMapper(MeasureMapper.class);
 
       mapper.update(value);
+      mapper.deleteData(value);
+      if (value.getMeasureData() != null) {
+        mapper.insertData(value);
+      }
 
       session.commit();
     } finally {
diff --git a/sonar-batch/src/test/java/org/sonar/batch/index/MeasurePersisterTest.java b/sonar-batch/src/test/java/org/sonar/batch/index/MeasurePersisterTest.java
index f1ba1fb750a..4e7f37add5d 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/index/MeasurePersisterTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/index/MeasurePersisterTest.java
@@ -45,6 +45,8 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 public class MeasurePersisterTest extends AbstractDaoTestCase {
+  private static final String SHORT = "SHORT";
+  private static final String LONG = StringUtils.repeat("0123456789", 10);
 
   public static final int PROJECT_SNAPSHOT_ID = 3001;
   public static final int PACKAGE_SNAPSHOT_ID = 3002;
@@ -64,6 +66,7 @@ public class MeasurePersisterTest extends AbstractDaoTestCase {
   @Before
   public void mockResourcePersister() {
     when(resourcePersister.getSnapshotOrFail(project)).thenReturn(projectSnapshot);
+    when(resourcePersister.getSnapshotOrFail(aPackage)).thenReturn(packageSnapshot);
     when(resourcePersister.getSnapshot(project)).thenReturn(projectSnapshot);
     when(resourcePersister.getSnapshot(aPackage)).thenReturn(packageSnapshot);
 
@@ -71,7 +74,7 @@ public class MeasurePersisterTest extends AbstractDaoTestCase {
   }
 
   @Test
-  public void shouldInsertMeasure() {
+  public void should_insert_measure() {
     setupData("empty");
 
     Measure measure = new Measure(ncloc()).setValue(1234.0);
@@ -82,7 +85,16 @@ public class MeasurePersisterTest extends AbstractDaoTestCase {
   }
 
   @Test
-  public void shouldInsertRuleMeasure() {
+  public void should_reload_measure() {
+    Measure measure = new Measure(ncloc());
+
+    measurePersister.reloadMeasure(measure);
+
+    verify(memoryOptimizer).reloadMeasure(measure);
+  }
+
+  @Test
+  public void should_insert_rule_measure() {
     setupData("empty");
 
     Rule rule = Rule.create("pmd", "key");
@@ -95,49 +107,75 @@ public class MeasurePersisterTest extends AbstractDaoTestCase {
   }
 
   @Test
-  public void shouldInsertMeasureWithTextData() {
+  public void should_insert_measure_with_text_data() {
     setupData("empty");
 
-    measurePersister.saveMeasure(project, new Measure(ncloc()).setData("SHORT"));
-    measurePersister.saveMeasure(project, new Measure(ncloc()).setData(StringUtils.repeat("0123456789", 10)));
+    measurePersister.saveMeasure(project, new Measure(ncloc()).setData(SHORT));
+    measurePersister.saveMeasure(project, new Measure(ncloc()).setData(LONG));
 
     checkTables("shouldInsertMeasureWithLargeData", "project_measures", "measure_data");
   }
 
   @Test
-  public void shouldUpdateMeasure() {
+  public void should_not_save_best_values() {
+    setupData("empty");
+
+    measurePersister.saveMeasure(aFile, new Measure(coverage()).setValue(100.0));
+
+    assertEmptyTables("project_measures");
+  }
+
+  @Test
+  public void should_not_save_memory_only_measures() {
+    setupData("empty");
+
+    measurePersister.saveMeasure(aFile, new Measure("ncloc").setPersistenceMode(PersistenceMode.MEMORY));
+
+    assertEmptyTables("project_measures");
+  }
+
+  @Test
+  public void should_always_save_non_file_measures() {
+    setupData("empty");
+
+    measurePersister.saveMeasure(project, new Measure(ncloc()).setValue(200.0));
+    measurePersister.saveMeasure(aPackage, new Measure(ncloc()).setValue(300.0));
+
+    checkTables("shouldAlwaysPersistNonFileMeasures", "project_measures");
+  }
+
+  @Test
+  public void should_update_measure() {
     setupData("data");
 
-    Measure measure = new Measure(coverage()).setValue(12.5).setId(1L);
-    measurePersister.saveMeasure(project, measure);
+    measurePersister.saveMeasure(project, new Measure(coverage()).setValue(12.5).setId(1L));
+    measurePersister.saveMeasure(project, new Measure(coverage()).setData(SHORT).setId(2L));
+    measurePersister.saveMeasure(project, new Measure(coverage()).setData(LONG).setId(3L));
 
     checkTables("shouldUpdateMeasure", "project_measures");
   }
 
   @Test
-  public void shouldAddDelayedMeasureSeveralTimes() {
+  public void should_add_delayed_measure_several_times() {
     setupData("empty");
 
     Measure measure = new Measure(ncloc());
 
-    measure.setValue(200.0);
     measurePersister.setDelayedMode(true);
-    measurePersister.saveMeasure(project, measure);
-
-    measure.setValue(300.0);
-    measurePersister.saveMeasure(project, measure);
+    measurePersister.saveMeasure(project, measure.setValue(200.0));
+    measurePersister.saveMeasure(project, measure.setValue(300.0));
     measurePersister.dump();
 
     checkTables("shouldAddDelayedMeasureSeveralTimes", "project_measures");
   }
 
   @Test
-  public void shouldDelaySaving() {
+  public void should_delay_saving() {
     setupData("empty");
 
     measurePersister.setDelayedMode(true);
-    measurePersister.saveMeasure(project, new Measure(ncloc()).setValue(1234.0));
-    measurePersister.saveMeasure(aPackage, new Measure(ncloc()).setValue(50.0));
+    measurePersister.saveMeasure(project, new Measure(ncloc()).setValue(1234.0).setData(SHORT));
+    measurePersister.saveMeasure(aPackage, new Measure(ncloc()).setValue(50.0).setData(LONG));
 
     assertEmptyTables("project_measures");
 
@@ -146,7 +184,7 @@ public class MeasurePersisterTest extends AbstractDaoTestCase {
   }
 
   @Test
-  public void shouldNotDelaySavingWithDatabaseOnlyMeasure() {
+  public void should_not_delay_saving_with_database_only_measure() {
     setupData("empty");
 
     measurePersister.setDelayedMode(true);
@@ -157,14 +195,7 @@ public class MeasurePersisterTest extends AbstractDaoTestCase {
   }
 
   @Test
-  public void shouldNotSaveBestValues() {
-    assertThat(MeasurePersister.shouldPersistMeasure(aFile, new Measure(coverage()).setValue(0.0))).isTrue();
-    assertThat(MeasurePersister.shouldPersistMeasure(aFile, new Measure(coverage()).setValue(75.8))).isTrue();
-    assertThat(MeasurePersister.shouldPersistMeasure(aFile, new Measure(coverage()).setValue(100.0))).isFalse();
-  }
-
-  @Test
-  public void shouldNotSaveBestValueMeasuresInDelayedMode() {
+  public void should_not_save_best_value_measures_in_delayed_mode() {
     setupData("empty");
 
     measurePersister.setDelayedMode(true);
@@ -178,20 +209,7 @@ public class MeasurePersisterTest extends AbstractDaoTestCase {
   }
 
   @Test
-  public void shouldNotSaveMemoryOnlyMeasures() {
-    Measure measure = new Measure("ncloc").setPersistenceMode(PersistenceMode.MEMORY);
-
-    assertThat(MeasurePersister.shouldPersistMeasure(aPackage, measure)).isFalse();
-  }
-
-  @Test
-  public void shouldAlwaysPersistNonFileMeasures() {
-    assertThat(MeasurePersister.shouldPersistMeasure(project, new Measure(CoreMetrics.LINES, 200.0))).isTrue();
-    assertThat(MeasurePersister.shouldPersistMeasure(aPackage, new Measure(CoreMetrics.LINES, 200.0))).isTrue();
-  }
-
-  @Test
-  public void shouldNotPersistSomeFileMeasuresWithBestValue() {
+  public void should_not_save_some_file_measures_with_best_value() {
     assertThat(MeasurePersister.shouldPersistMeasure(aFile, new Measure(CoreMetrics.LINES, 200.0))).isTrue();
     assertThat(MeasurePersister.shouldPersistMeasure(aFile, new Measure(CoreMetrics.DUPLICATED_LINES_DENSITY, 3.0))).isTrue();
 
@@ -206,10 +224,13 @@ public class MeasurePersisterTest extends AbstractDaoTestCase {
   }
 
   @Test
-  public void nullValueAndNullVariationsShouldBeConsideredAsBestValue() {
-    Measure measure = new Measure(CoreMetrics.NEW_VIOLATIONS_KEY);
-
-    assertThat(MeasurePersister.isBestValueMeasure(measure, CoreMetrics.NEW_VIOLATIONS)).isTrue();
+  public void null_value_and_null_variations_should_be_considered_as_best_value() {
+    assertThat(MeasurePersister.isBestValueMeasure(new Measure(CoreMetrics.NEW_VIOLATIONS_KEY).setVariation1(0.0), CoreMetrics.NEW_VIOLATIONS)).isTrue();
+    assertThat(MeasurePersister.isBestValueMeasure(new Measure(CoreMetrics.NEW_VIOLATIONS_KEY).setVariation1(1.0), CoreMetrics.NEW_VIOLATIONS)).isFalse();
+    assertThat(MeasurePersister.isBestValueMeasure(new Measure(CoreMetrics.NEW_VIOLATIONS_KEY).setVariation2(1.0), CoreMetrics.NEW_VIOLATIONS)).isFalse();
+    assertThat(MeasurePersister.isBestValueMeasure(new Measure(CoreMetrics.NEW_VIOLATIONS_KEY).setVariation3(1.0), CoreMetrics.NEW_VIOLATIONS)).isFalse();
+    assertThat(MeasurePersister.isBestValueMeasure(new Measure(CoreMetrics.NEW_VIOLATIONS_KEY).setVariation4(1.0), CoreMetrics.NEW_VIOLATIONS)).isFalse();
+    assertThat(MeasurePersister.isBestValueMeasure(new Measure(CoreMetrics.NEW_VIOLATIONS_KEY).setVariation5(1.0), CoreMetrics.NEW_VIOLATIONS)).isFalse();
   }
 
   private static Snapshot snapshot(int id) {
diff --git a/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/data.xml b/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/data.xml
index f8900a3e228..2db0aca4e4d 100644
--- a/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/data.xml
+++ b/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/data.xml
@@ -6,4 +6,18 @@
                     person_id="[null]"
                     variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"/>
 
+  <project_measures id="2" VALUE="60" METRIC_ID="2" SNAPSHOT_ID="3001" alert_text="[null]" RULES_CATEGORY_ID="[null]"
+                    RULE_ID="[null]" text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]" rule_priority="[null]" characteristic_id="[null]" url="[null]"
+                    person_id="[null]"
+                    variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"/>
+
+  <project_measures id="3" VALUE="60" METRIC_ID="2" SNAPSHOT_ID="3001" alert_text="[null]" RULES_CATEGORY_ID="[null]"
+                    RULE_ID="[null]" text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]" rule_priority="[null]" characteristic_id="[null]" url="[null]"
+                    person_id="[null]"
+                    variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"/>
+
+  <measure_data id="1" measure_id="2" snapshot_id="3001" data="MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTIzNDU2Nzg5MDEyMzQ1Njc4OQ=="/>
+
 </dataset>
diff --git a/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldAlwaysPersistNonFileMeasures-result.xml b/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldAlwaysPersistNonFileMeasures-result.xml
new file mode 100644
index 00000000000..cc956fb7770
--- /dev/null
+++ b/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldAlwaysPersistNonFileMeasures-result.xml
@@ -0,0 +1,14 @@
+<dataset>
+
+  <project_measures id="1" VALUE="200.0" METRIC_ID="1" SNAPSHOT_ID="3001" alert_text="[null]" RULES_CATEGORY_ID="[null]"
+                    RULE_ID="[null]" text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]" rule_priority="[null]" characteristic_id="[null]" url="[null]"
+                    person_id="[null]"
+                    variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"/>
+
+  <project_measures id="2" VALUE="300.0" METRIC_ID="1" SNAPSHOT_ID="3002" alert_text="[null]" RULES_CATEGORY_ID="[null]"
+                    RULE_ID="[null]" text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]" rule_priority="[null]" characteristic_id="[null]" url="[null]"
+                    person_id="[null]"
+                    variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"/>
+</dataset>
diff --git a/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldDelaySaving-result.xml b/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldDelaySaving-result.xml
index 3aecc2ea40e..97b4eb2090a 100644
--- a/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldDelaySaving-result.xml
+++ b/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldDelaySaving-result.xml
@@ -2,7 +2,7 @@
 
   <project_measures id="1" VALUE="1234.0" METRIC_ID="1" SNAPSHOT_ID="3001" alert_text="[null]"
                     RULES_CATEGORY_ID="[null]"
-                    RULE_ID="[null]" text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    RULE_ID="[null]" text_value="SHORT" tendency="[null]" measure_date="[null]" project_id="[null]"
                     alert_status="[null]" description="[null]" rule_priority="[null]" characteristic_id="[null]" url="[null]"
                     person_id="[null]"
                     variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"/>
@@ -13,4 +13,6 @@
                     person_id="[null]"
                     variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"/>
 
+  <measure_data id="1" measure_id="2" snapshot_id="3002" data="MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTIzNDU2Nzg5MDEyMzQ1Njc4OQ=="/>
+
 </dataset>
diff --git a/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldUpdateMeasure-result.xml b/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldUpdateMeasure-result.xml
index f4742847f20..e1db7336b32 100644
--- a/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldUpdateMeasure-result.xml
+++ b/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldUpdateMeasure-result.xml
@@ -6,4 +6,18 @@
                     person_id="[null]"
                     variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"/>
 
+  <project_measures id="2" VALUE="[null]" METRIC_ID="2" SNAPSHOT_ID="3001" alert_text="[null]" RULES_CATEGORY_ID="[null]"
+                    RULE_ID="[null]" text_value="SHORT" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]" rule_priority="[null]" characteristic_id="[null]" url="[null]"
+                    person_id="[null]"
+                    variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"/>
+
+  <project_measures id="3" VALUE="[null]" METRIC_ID="2" SNAPSHOT_ID="3001" alert_text="[null]" RULES_CATEGORY_ID="[null]"
+                    RULE_ID="[null]" text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]" rule_priority="[null]" characteristic_id="[null]" url="[null]"
+                    person_id="[null]"
+                    variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"/>
+
+  <measure_data id="1" measure_id="3" snapshot_id="3001" data="MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTIzNDU2Nzg5MDEyMzQ1Njc4OQ=="/>
+
 </dataset>