SONAR-3437 Improve code coverage and fix measure data update

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 d791be13e9357d2d834c22660512ca898e477a80..0c0cf8253548454b68a2ede5670a856608e40164 100644 (file)
--- 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 f1ba1fb750a4390a686d89b1ee1d387775e3cd4c..4e7f37add5d36ab5d816ead37c54aae1305e903b 100644 (file)
--- 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 f8900a3e2280904db134512ef27957686e6bdacd..2db0aca4e4d4fb5269d78f5e720f8c6d0b3b8650 100644 (file)
--- 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 (file)
index 0000000..cc956fb
--- /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 3aecc2ea40ed637e526e0cf1f596c75e6b466df1..97b4eb2090a511b4c722407748b710f8323f7a19 100644 (file)
--- 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 f4742847f205a87429943a3af54b1cbcb6df0e91..e1db7336b32081d0c5914d907217cdba890548ae 100644 (file)
--- 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>

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/database/model/MeasureMapper.java b/sonar-plugin-api/src/main/java/org/sonar/api/database/model/MeasureMapper.java
index 5995096374796e839714c2053cc3265fa3d600ac..f9362533a764b03a692561783a91122245c59673 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/database/model/MeasureMapper.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/database/model/MeasureMapper.java
@@ -24,5 +24,7 @@ public interface MeasureMapper {
 
   void insertData(MeasureModel data);
 
+  void deleteData(MeasureModel data);
+
   void update(MeasureModel measure);
 }

diff --git a/sonar-plugin-api/src/main/resources/org/sonar/api/database/model/MeasureMapper-oracle.xml b/sonar-plugin-api/src/main/resources/org/sonar/api/database/model/MeasureMapper-oracle.xml
index fa8838e35fa5089d975ea8b9e240c7160aba1e9d..380e14ffb08efea3934e3e003a04a76232207498 100644 (file)
--- a/sonar-plugin-api/src/main/resources/org/sonar/api/database/model/MeasureMapper-oracle.xml
+++ b/sonar-plugin-api/src/main/resources/org/sonar/api/database/model/MeasureMapper-oracle.xml
@@ -22,7 +22,11 @@
     INSERT INTO measure_data (id, measure_id, snapshot_id, data)
     VALUES (measure_data_seq.NEXTVAL, #{id}, #{snapshotId}, #{measureData.data})
   </insert>
-  
+
+  <update id="deleteData" parameterType="MeasureModel">
+    DELETE FROM measure_data WHERE measure_id = #{id} AND snapshot_id = #{snapshotId}
+  </update>
+
   <update id="update" parameterType="MeasureModel">
     UPDATE project_measures
     SET

diff --git a/sonar-plugin-api/src/main/resources/org/sonar/api/database/model/MeasureMapper.xml b/sonar-plugin-api/src/main/resources/org/sonar/api/database/model/MeasureMapper.xml
index b3c70bf3462c1782fad1021b4e91ce6bfd4668ad..6823b1414978491a086ee06de9147b1fdf870c24 100644 (file)
--- a/sonar-plugin-api/src/main/resources/org/sonar/api/database/model/MeasureMapper.xml
+++ b/sonar-plugin-api/src/main/resources/org/sonar/api/database/model/MeasureMapper.xml
@@ -15,12 +15,16 @@
       #{variationValue2}, #{variationValue3}, #{variationValue4}, #{variationValue5}, #{personId}
     )
   </insert>
-  
+
   <insert id="insertData" parameterType="MeasureModel" useGeneratedKeys="true">
     INSERT INTO measure_data (measure_id, snapshot_id, data)
     VALUES (#{id}, #{snapshotId}, #{measureData.data})
   </insert>
-  
+
+  <update id="deleteData" parameterType="MeasureModel">
+    DELETE FROM measure_data WHERE measure_id = #{id} AND snapshot_id = #{snapshotId}
+  </update>
+
   <update id="update" parameterType="MeasureModel">
     UPDATE project_measures
     SET