From 111cf7219f5a4439687f2cd836fb8e498e545773 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Mon, 9 Jul 2012 10:16:32 +0200 Subject: [PATCH] SONAR-3437 Prepare for refactoring to MyBatis --- .../sonar/batch/index/MeasurePersister.java | 99 ++++++------ .../batch/index/MeasurePersisterTest.java | 141 +++++++++--------- .../index/MeasurePersisterTest/shared.xml | 30 ---- .../shouldDelaySaving-result.xml | 31 ---- .../shouldInsertMeasure-result.xml | 30 ---- ...aySavingWithDatabaseOnlyMeasure-result.xml | 38 ----- .../shouldUpdateMeasure-result.xml | 30 ---- 7 files changed, 119 insertions(+), 280 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 ddc1df16b7e..3b967d89e38 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 @@ -19,6 +19,7 @@ */ package org.sonar.batch.index; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.SetMultimap; import org.apache.commons.lang.math.NumberUtils; @@ -39,14 +40,12 @@ import java.util.Collection; import java.util.Map; public final class MeasurePersister { - + private final DatabaseSession session; + private final ResourcePersister resourcePersister; + private final RuleFinder ruleFinder; + private final MemoryOptimizer memoryOptimizer; + private final SetMultimap unsavedMeasuresByResource = LinkedHashMultimap.create(); private boolean delayedMode = false; - private SetMultimap unsavedMeasuresByResource = LinkedHashMultimap.create(); - private DatabaseSession session; - private ResourcePersister resourcePersister; - private RuleFinder ruleFinder; - private MemoryOptimizer memoryOptimizer; - public MeasurePersister(DatabaseSession session, ResourcePersister resourcePersister, RuleFinder ruleFinder, MemoryOptimizer memoryOptimizer) { this.session = session; @@ -60,29 +59,23 @@ public final class MeasurePersister { } public void saveMeasure(Resource resource, Measure measure) { - boolean saveLater = (measure.getPersistenceMode().useMemory() && delayedMode); - if (saveLater) { + if (shouldSaveLater(measure)) { unsavedMeasuresByResource.put(resource, measure); + return; + } - } else { + if (measure.getId() != null) { // update + MeasureModel model = session.reattach(MeasureModel.class, measure.getId()); + model = mergeModel(measure, model); + + model.save(session); + memoryOptimizer.evictDataMeasure(measure, model); + } else if (shouldPersistMeasure(resource, measure)) { // insert Snapshot snapshot = resourcePersister.getSnapshotOrFail(resource); - MeasureModel model = null; - if (measure.getId() != null) { - // update - model = session.reattach(MeasureModel.class, measure.getId()); - model = mergeModel(measure, model); - model.save(session); - - } else if (shouldPersistMeasure(resource, measure)) { - // insert - model = createModel(measure); - model.setSnapshotId(snapshot.getId()); - model.save(session); - measure.setId(model.getId()); // could be removed - } - if (model != null) { - memoryOptimizer.evictDataMeasure(measure, model); - } + MeasureModel model = createModel(measure).setSnapshotId(snapshot.getId()); + + model.save(session); + memoryOptimizer.evictDataMeasure(measure, model); } } @@ -90,39 +83,44 @@ public final class MeasurePersister { return memoryOptimizer.reloadMeasure(measure); } + private boolean shouldSaveLater(Measure measure) { + return delayedMode && measure.getPersistenceMode().useMemory(); + } + + @VisibleForTesting static boolean shouldPersistMeasure(Resource resource, Measure measure) { - Metric metric = measure.getMetric(); return measure.getPersistenceMode().useDatabase() && - !(ResourceUtils.isEntity(resource) && isBestValueMeasure(measure, metric)); + !(ResourceUtils.isEntity(resource) && isBestValueMeasure(measure, measure.getMetric())); } + @VisibleForTesting static boolean isBestValueMeasure(Measure measure, Metric metric) { - return measure.getId() == null && - metric.isOptimizedBestValue() == Boolean.TRUE && - metric.getBestValue() != null && - (measure.getValue() == null || NumberUtils.compare(metric.getBestValue(), measure.getValue()) == 0) && - measure.getAlertStatus() == null && - measure.getDescription() == null && - 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); + return measure.getId() == null + && metric.isOptimizedBestValue() == Boolean.TRUE + && metric.getBestValue() != null + && (measure.getValue() == null || NumberUtils.compare(metric.getBestValue(), measure.getValue()) == 0) + && measure.getAlertStatus() == null + && measure.getDescription() == null + && 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); } public void dump() { LoggerFactory.getLogger(getClass()).debug("{} measures to dump", unsavedMeasuresByResource.size()); + Map> map = unsavedMeasuresByResource.asMap(); for (Map.Entry> entry : map.entrySet()) { Resource resource = entry.getKey(); Snapshot snapshot = resourcePersister.getSnapshot(entry.getKey()); for (Measure measure : entry.getValue()) { if (shouldPersistMeasure(resource, measure)) { - MeasureModel model = createModel(measure); - model.setSnapshotId(snapshot.getId()); + MeasureModel model = createModel(measure).setSnapshotId(snapshot.getId()); model.save(session); } } @@ -132,12 +130,12 @@ public final class MeasurePersister { unsavedMeasuresByResource.clear(); } - MeasureModel createModel(Measure measure) { + private MeasureModel createModel(Measure measure) { return mergeModel(measure, new MeasureModel()); } - MeasureModel mergeModel(Measure measure, MeasureModel merge) { - merge.setMetricId(measure.getMetric().getId());// we assume that the index has updated the metric + private MeasureModel mergeModel(Measure measure, MeasureModel merge) { + merge.setMetricId(measure.getMetric().getId()); // we assume that the index has updated the metric merge.setDescription(measure.getDescription()); merge.setData(measure.getData()); merge.setAlertStatus(measure.getAlertStatus()); @@ -161,11 +159,10 @@ public final class MeasurePersister { merge.setRulePriority(ruleMeasure.getSeverity()); if (ruleMeasure.getRule() != null) { Rule ruleWithId = ruleFinder.findByKey(ruleMeasure.getRule().getRepositoryKey(), ruleMeasure.getRule().getKey()); - if (ruleWithId != null) { - merge.setRuleId(ruleWithId.getId()); - } else { + if (ruleWithId == null) { throw new SonarException("Can not save a measure with unknown rule " + ruleMeasure); } + merge.setRuleId(ruleWithId.getId()); } } return merge; 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 b37166c3d94..4d13104ba42 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 @@ -30,16 +30,16 @@ import org.sonar.api.measures.PersistenceMode; import org.sonar.api.resources.JavaFile; import org.sonar.api.resources.JavaPackage; import org.sonar.api.resources.Project; -import org.sonar.core.rule.DefaultRuleFinder; import org.sonar.jpa.test.AbstractDbUnitTestCase; import java.util.List; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; -import static org.mockito.Matchers.anyObject; +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class MeasurePersisterTest extends AbstractDbUnitTestCase { @@ -48,38 +48,29 @@ public class MeasurePersisterTest extends AbstractDbUnitTestCase { public static final int FILE_SNAPSHOT_ID = 3003; public static final int COVERAGE_METRIC_ID = 2; - private ResourcePersister resourcePersister; private MeasurePersister measurePersister; + private ResourcePersister resourcePersister = mock(ResourcePersister.class); + private MemoryOptimizer memoryOptimizer = mock(MemoryOptimizer.class); private Project project = new Project("foo"); private JavaPackage aPackage = new JavaPackage("org.foo"); private JavaFile aFile = new JavaFile("org.foo.Bar"); - private Snapshot projectSnapshot, packageSnapshot, fileSnapshot; - private Metric ncloc, coverage; - private MemoryOptimizer memoryOptimizer; + private Snapshot projectSnapshot = snapshot(PROJECT_SNAPSHOT_ID); + private Snapshot packageSnapshot = snapshot(PACKAGE_SNAPSHOT_ID); @Before public void mockResourcePersister() { - setupData("shared"); - resourcePersister = mock(ResourcePersister.class); - projectSnapshot = getSession().getSingleResult(Snapshot.class, "id", PROJECT_SNAPSHOT_ID); - packageSnapshot = getSession().getSingleResult(Snapshot.class, "id", PACKAGE_SNAPSHOT_ID); - fileSnapshot = getSession().getSingleResult(Snapshot.class, "id", FILE_SNAPSHOT_ID); - ncloc = getSession().getSingleResult(Metric.class, "key", "ncloc"); - coverage = getSession().getSingleResult(Metric.class, "key", "coverage"); - when(resourcePersister.getSnapshotOrFail(eq(project))).thenReturn(projectSnapshot); - when(resourcePersister.getSnapshotOrFail(eq(aPackage))).thenReturn(packageSnapshot); - when(resourcePersister.getSnapshotOrFail(eq(aFile))).thenReturn(fileSnapshot); + when(resourcePersister.getSnapshotOrFail(project)).thenReturn(projectSnapshot); when(resourcePersister.getSnapshot(project)).thenReturn(projectSnapshot); when(resourcePersister.getSnapshot(aPackage)).thenReturn(packageSnapshot); - when(resourcePersister.getSnapshot(aFile)).thenReturn(fileSnapshot); - memoryOptimizer = mock(MemoryOptimizer.class); - measurePersister = new MeasurePersister(getSession(), resourcePersister, new DefaultRuleFinder(getSessionFactory()), memoryOptimizer); + + measurePersister = new MeasurePersister(getSession(), resourcePersister, null, memoryOptimizer); } @Test public void shouldInsertMeasure() { - Measure measure = new Measure(ncloc).setValue(1234.0); + setupData("shared"); + Measure measure = new Measure(ncloc()).setValue(1234.0); measurePersister.saveMeasure(project, measure); checkTables("shouldInsertMeasure", "project_measures"); @@ -87,19 +78,17 @@ public class MeasurePersisterTest extends AbstractDbUnitTestCase { @Test public void shouldRegisterPersistedMeasureToMemoryOptimizer() { - Measure measure = new Measure(ncloc).setValue(1234.0); - + Measure measure = new Measure(ncloc()).setValue(1234.0); measurePersister.saveMeasure(project, measure); - verify(memoryOptimizer).evictDataMeasure(eq(measure), (MeasureModel)anyObject()); + verify(memoryOptimizer).evictDataMeasure(eq(measure), any(MeasureModel.class)); } - @Test public void shouldUpdateMeasure() { - Measure measure = new Measure(coverage).setValue(12.5); - measure.setId(1L); + setupData("shared"); + Measure measure = new Measure(coverage()).setValue(12.5).setId(1L); measurePersister.saveMeasure(project, measure); checkTables("shouldUpdateMeasure", "project_measures"); @@ -107,28 +96,28 @@ public class MeasurePersisterTest extends AbstractDbUnitTestCase { @Test public void shouldAddDelayedMeasureSeveralTimes() { + Measure measure = new Measure(ncloc()).setValue(200.0); + measurePersister.setDelayedMode(true); - Measure measure = new Measure(ncloc).setValue(200.0); measurePersister.saveMeasure(project, measure); measure.setValue(300.0); measurePersister.saveMeasure(project, measure); - measurePersister.dump(); List coverageMeasures = getSession().getResults(MeasureModel.class, "snapshotId", PROJECT_SNAPSHOT_ID, "metricId", 1); - assertThat(coverageMeasures.size(), is(1)); - assertThat(coverageMeasures.get(0).getValue(), is(300.0)); + assertThat(coverageMeasures).onProperty("value").containsExactly(300.0); } @Test public void shouldDelaySaving() { - measurePersister.setDelayedMode(true); + setupData("shared"); - measurePersister.saveMeasure(project, new Measure(ncloc).setValue(1234.0)); - measurePersister.saveMeasure(aPackage, new Measure(ncloc).setValue(50.0)); + measurePersister.setDelayedMode(true); + measurePersister.saveMeasure(project, new Measure(ncloc()).setValue(1234.0)); + measurePersister.saveMeasure(aPackage, new Measure(ncloc()).setValue(50.0)); - assertThat(getSession().getResults(MeasureModel.class, "metricId", 1).size(), is(0)); + assertThat(getSession().getResults(MeasureModel.class, "metricId", 1)).isEmpty(); measurePersister.dump(); checkTables("shouldDelaySaving", "project_measures"); @@ -136,80 +125,92 @@ public class MeasurePersisterTest extends AbstractDbUnitTestCase { @Test public void shouldNotDelaySavingWithDatabaseOnlyMeasure() { - measurePersister.setDelayedMode(true); - - measurePersister.saveMeasure(project, new Measure(ncloc).setValue(1234.0).setPersistenceMode(PersistenceMode.DATABASE)); // database only - measurePersister.saveMeasure(aPackage, new Measure(ncloc).setValue(50.0)); // database + memory + setupData("shared"); - // no dump => the db-only measure is saved + measurePersister.setDelayedMode(true); + measurePersister.saveMeasure(project, new Measure(ncloc()).setValue(1234.0).setPersistenceMode(PersistenceMode.DATABASE)); // database + measurePersister.saveMeasure(aPackage, new Measure(ncloc()).setValue(50.0)); // database + memory checkTables("shouldNotDelaySavingWithDatabaseOnlyMeasure", "project_measures"); } @Test public void shouldNotSaveBestValues() { - JavaFile file = new JavaFile("org.foo.MyClass"); - - Measure measure = new Measure(coverage).setValue(0.0); - assertThat(MeasurePersister.shouldPersistMeasure(file, measure), is(true)); + Measure measure = new Measure(coverage()).setValue(0.0); + assertThat(MeasurePersister.shouldPersistMeasure(aFile, measure)).isTrue(); - measure = new Measure(coverage).setValue(75.8); - assertThat(MeasurePersister.shouldPersistMeasure(file, measure), is(true)); + measure = new Measure(coverage()).setValue(75.8); + assertThat(MeasurePersister.shouldPersistMeasure(aFile, measure)).isTrue(); - measure = new Measure(coverage).setValue(100.0); - assertThat(MeasurePersister.shouldPersistMeasure(file, measure), is(false)); + measure = new Measure(coverage()).setValue(100.0); + assertThat(MeasurePersister.shouldPersistMeasure(aFile, measure)).isFalse(); } @Test public void shouldNotSaveBestValueMeasuresInDelayedMode() { measurePersister.setDelayedMode(true); + measurePersister.saveMeasure(aFile, new Measure(coverage()).setValue(100.0)); - measurePersister.saveMeasure(aFile, new Measure(coverage).setValue(100.0)); - - assertThat(getSession().getResults(MeasureModel.class, "metricId", COVERAGE_METRIC_ID, "snapshotId", FILE_SNAPSHOT_ID).size(), is(0)); + assertThat(getSession().getResults(MeasureModel.class, "metricId", COVERAGE_METRIC_ID, "snapshotId", FILE_SNAPSHOT_ID)).isEmpty(); measurePersister.dump(); - // not saved because it's a best value measure - assertThat(getSession().getResults(MeasureModel.class, "metricId", COVERAGE_METRIC_ID, "snapshotId", FILE_SNAPSHOT_ID).size(), is(0)); + assertThat(getSession().getResults(MeasureModel.class, "metricId", COVERAGE_METRIC_ID, "snapshotId", FILE_SNAPSHOT_ID)).isEmpty(); } - @Test public void shouldNotSaveMemoryOnlyMeasures() { Measure measure = new Measure("ncloc").setPersistenceMode(PersistenceMode.MEMORY); - assertThat(MeasurePersister.shouldPersistMeasure(aPackage, measure), is(false)); + assertThat(MeasurePersister.shouldPersistMeasure(aPackage, measure)).isFalse(); } @Test public void shouldAlwaysPersistNonFileMeasures() { - assertThat(MeasurePersister.shouldPersistMeasure(project, new Measure(CoreMetrics.LINES, 200.0)), is(true)); - assertThat(MeasurePersister.shouldPersistMeasure(aPackage, new Measure(CoreMetrics.LINES, 200.0)), is(true)); + 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() { - JavaFile file = new JavaFile("org.foo.Bar"); + assertThat(MeasurePersister.shouldPersistMeasure(aFile, new Measure(CoreMetrics.LINES, 200.0))).isTrue(); + assertThat(MeasurePersister.shouldPersistMeasure(aFile, new Measure(CoreMetrics.DUPLICATED_LINES_DENSITY, 3.0))).isTrue(); - // must persist: - assertThat(MeasurePersister.shouldPersistMeasure(file, new Measure(CoreMetrics.LINES, 200.0)), is(true)); - assertThat(MeasurePersister.shouldPersistMeasure(file, new Measure(CoreMetrics.DUPLICATED_LINES_DENSITY, 3.0)), is(true)); - - - // must not persist: Measure duplicatedLines = new Measure(CoreMetrics.DUPLICATED_LINES_DENSITY, 0.0); - assertThat(MeasurePersister.shouldPersistMeasure(file, duplicatedLines), is(false)); + assertThat(MeasurePersister.shouldPersistMeasure(aFile, duplicatedLines)).isFalse(); duplicatedLines.setVariation1(0.0); - assertThat(MeasurePersister.shouldPersistMeasure(file, duplicatedLines), is(false)); + assertThat(MeasurePersister.shouldPersistMeasure(aFile, duplicatedLines)).isFalse(); duplicatedLines.setVariation1(-3.0); - assertThat(MeasurePersister.shouldPersistMeasure(file, duplicatedLines), is(true)); + assertThat(MeasurePersister.shouldPersistMeasure(aFile, duplicatedLines)).isTrue(); } @Test public void nullValueAndNullVariationsShouldBeConsideredAsBestValue() { Measure measure = new Measure(CoreMetrics.NEW_VIOLATIONS_KEY); - assertThat(MeasurePersister.isBestValueMeasure(measure, CoreMetrics.NEW_VIOLATIONS), is(true)); + + assertThat(MeasurePersister.isBestValueMeasure(measure, CoreMetrics.NEW_VIOLATIONS)).isTrue(); + } + + private static Snapshot snapshot(int id) { + Snapshot snapshot = mock(Snapshot.class); + when(snapshot.getId()).thenReturn(id); + return snapshot; + } + + private static Metric ncloc() { + Metric ncloc = mock(Metric.class); + when(ncloc.getId()).thenReturn(1); + when(ncloc.getKey()).thenReturn("ncloc"); + return ncloc; + } + + private static Metric coverage() { + Metric coverage = mock(Metric.class); + when(coverage.getId()).thenReturn(COVERAGE_METRIC_ID); + when(coverage.getKey()).thenReturn("coverage"); + when(coverage.isOptimizedBestValue()).thenReturn(true); + when(coverage.getBestValue()).thenReturn(100.0); + return coverage; } } diff --git a/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shared.xml b/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shared.xml index 2b8c2b91a70..f8900a3e228 100644 --- a/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shared.xml +++ b/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shared.xml @@ -1,35 +1,5 @@ - diff --git a/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldInsertMeasure-result.xml b/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldInsertMeasure-result.xml index 933b0fff827..1cd6e6d10c4 100644 --- a/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldInsertMeasure-result.xml +++ b/sonar-batch/src/test/resources/org/sonar/batch/index/MeasurePersisterTest/shouldInsertMeasure-result.xml @@ -1,35 +1,5 @@ - 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 4758f158973..f4742847f20 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 @@ -1,35 +1,5 @@ -