From 8ff07c056e873426723912c9484cdcb9f411d8d1 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Wed, 11 Jul 2012 11:07:32 +0200 Subject: [PATCH] SONAR-3437 Improve code coverage and fix measure data update --- .../sonar/batch/index/MeasurePersister.java | 18 ++- .../batch/index/MeasurePersisterTest.java | 109 +++++++++++------- .../batch/index/MeasurePersisterTest/data.xml | 14 +++ ...uldAlwaysPersistNonFileMeasures-result.xml | 14 +++ .../shouldDelaySaving-result.xml | 4 +- .../shouldUpdateMeasure-result.xml | 14 +++ .../api/database/model/MeasureMapper.java | 2 + .../database/model/MeasureMapper-oracle.xml | 6 +- .../api/database/model/MeasureMapper.xml | 8 +- 9 files changed, 136 insertions(+), 53 deletions(-) create mode 100644 sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldAlwaysPersistNonFileMeasures-result.xml 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 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]"/> + + + + + + 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 @@ + + + + + + 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 @@ @@ -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]"/> + + 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]"/> + + + + + + 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 59950963747..f9362533a76 100644 --- 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 fa8838e35fa..380e14ffb08 100644 --- 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}) - + + + DELETE FROM measure_data WHERE measure_id = #{id} AND snapshot_id = #{snapshotId} + + 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 b3c70bf3462..6823b141497 100644 --- 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 INTO measure_data (measure_id, snapshot_id, data) VALUES (#{id}, #{snapshotId}, #{measureData.data}) - + + + DELETE FROM measure_data WHERE measure_id = #{id} AND snapshot_id = #{snapshotId} + + UPDATE project_measures SET -- 2.39.5