From 4cc9bec445ec46161c8693d8125525cf0e1bb478 Mon Sep 17 00:00:00 2001 From: Godin Date: Tue, 30 Nov 2010 13:40:58 +0000 Subject: [PATCH] SONAR-1450: Add support for incremental review of incoming violations --- .../sonar/batch/index/ViolationPersister.java | 31 ++++------ .../batch/index/ViolationPersisterTest.java | 60 +++++++++++-------- .../index/ViolationPersisterTest/shared.xml | 2 +- .../shouldInsertViolations-result.xml | 2 +- .../shouldUpdateViolation-result.xml | 2 +- 5 files changed, 52 insertions(+), 45 deletions(-) diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/ViolationPersister.java b/sonar-batch/src/main/java/org/sonar/batch/index/ViolationPersister.java index 2a799a7632e..2ec89741ded 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/index/ViolationPersister.java +++ b/sonar-batch/src/main/java/org/sonar/batch/index/ViolationPersister.java @@ -23,14 +23,10 @@ import org.sonar.api.database.DatabaseSession; import org.sonar.api.database.model.RuleFailureModel; import org.sonar.api.database.model.Snapshot; import org.sonar.api.resources.Project; -import org.sonar.api.resources.Resource; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; import org.sonar.api.rules.Violation; -import java.util.Collections; -import java.util.List; - public final class ViolationPersister { private DatabaseSession session; @@ -43,26 +39,25 @@ public final class ViolationPersister { this.ruleFinder = ruleFinder; } - public List getPreviousViolations(Resource resource) { - Snapshot snapshot = resourcePersister.getSnapshot(resource); - Snapshot previousLastSnapshot = resourcePersister.getLastSnapshot(snapshot, true); - if (previousLastSnapshot == null) { - return Collections.emptyList(); - } - return session.getResults(RuleFailureModel.class, "snapshotId", previousLastSnapshot.getId()); + public void saveViolation(Project project, Violation violation) { + saveOrUpdateViolation(project, violation); } - public void saveViolation(Project project, Violation violation) { - saveOrUpdateViolation(project, violation, null); + public RuleFailureModel selectPreviousViolation(Violation violation) { + Snapshot snapshot = resourcePersister.getSnapshot(violation.getResource()); + Snapshot previousLastSnapshot = resourcePersister.getLastSnapshot(snapshot, true); + return session.getSingleResult(RuleFailureModel.class, + "snapshotId", previousLastSnapshot.getId(), + "line", violation.getLineId(), + "message", violation.getMessage()); } - public void saveOrUpdateViolation(Project project, Violation violation, RuleFailureModel oldModel) { + public void saveOrUpdateViolation(Project project, Violation violation) { Snapshot snapshot = resourcePersister.saveResource(project, violation.getResource()); - RuleFailureModel model; - if (oldModel != null) { + RuleFailureModel model = selectPreviousViolation(violation); + if (model != null) { // update - model = session.reattach(RuleFailureModel.class, oldModel.getId()); - model = mergeModel(violation, oldModel); + model = mergeModel(violation, model); } else { // insert model = createModel(violation); diff --git a/sonar-batch/src/test/java/org/sonar/batch/index/ViolationPersisterTest.java b/sonar-batch/src/test/java/org/sonar/batch/index/ViolationPersisterTest.java index d4e60a18baf..d4b93acb3ae 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/index/ViolationPersisterTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/index/ViolationPersisterTest.java @@ -19,6 +19,14 @@ */ package org.sonar.batch.index; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Matchers.anyObject; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + import org.junit.Before; import org.junit.Test; import org.sonar.api.database.model.RuleFailureModel; @@ -31,16 +39,6 @@ import org.sonar.api.rules.Violation; import org.sonar.core.components.DefaultRuleFinder; import org.sonar.jpa.test.AbstractDbUnitTestCase; -import java.util.List; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.notNullValue; -import static org.mockito.Matchers.anyObject; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - public class ViolationPersisterTest extends AbstractDbUnitTestCase { private ViolationPersister violationPersister; @@ -59,14 +57,6 @@ public class ViolationPersisterTest extends AbstractDbUnitTestCase { violationPersister = new ViolationPersister(getSession(), resourcePersister, new DefaultRuleFinder(getSessionFactory())); } - @Test - public void shouldLoadViolations() { - List models = violationPersister.getPreviousViolations(javaFile); - - assertThat(models, notNullValue()); - assertThat(models.size(), greaterThan(0)); - } - @Test public void shouldSaveViolations() { Violation violation1a = Violation.create(rule1, javaFile) @@ -77,20 +67,42 @@ public class ViolationPersisterTest extends AbstractDbUnitTestCase { Violation violation2 = Violation.create(rule2, javaFile) .setPriority(RulePriority.MINOR); - violationPersister.saveOrUpdateViolation(new Project("project"), violation1a, null); - violationPersister.saveOrUpdateViolation(new Project("project"), violation1b, null); - violationPersister.saveOrUpdateViolation(new Project("project"), violation2, null); + violationPersister.saveViolation(new Project("project"), violation1a); + violationPersister.saveViolation(new Project("project"), violation1b); + violationPersister.saveViolation(new Project("project"), violation2); checkTables("shouldInsertViolations", "rule_failures"); } + @Test + public void shouldSelectPreviousViolation() { + Violation violation = Violation.create(rule1, javaFile) + .setPriority(RulePriority.CRITICAL).setLineId(10) + .setMessage("old message"); + + RuleFailureModel model = violationPersister.selectPreviousViolation(violation); + + assertThat(model, notNullValue()); + } + + @Test + public void noPreviousViolation() { + Violation violation = Violation.create(rule1, javaFile) + .setPriority(RulePriority.CRITICAL).setLineId(10) + .setMessage("new message"); + + RuleFailureModel model = violationPersister.selectPreviousViolation(violation); + + assertThat(model, nullValue()); + } + @Test public void shouldUpdateViolation() { Violation violation = Violation.create(rule1, javaFile) - .setPriority(RulePriority.CRITICAL).setLineId(20).setCost(55.6); - RuleFailureModel oldModel = violationPersister.getPreviousViolations(javaFile).iterator().next(); + .setPriority(RulePriority.CRITICAL).setLineId(10).setCost(55.6) + .setMessage("old message"); - violationPersister.saveOrUpdateViolation(new Project("project"), violation, oldModel); + violationPersister.saveOrUpdateViolation(new Project("project"), violation); checkTables("shouldUpdateViolation", "rule_failures"); } diff --git a/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shared.xml b/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shared.xml index 0d7a84ff870..bdaac3f6b2f 100644 --- a/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shared.xml +++ b/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shared.xml @@ -19,5 +19,5 @@ scope="FIL" qualifier="CLA" created_at="2008-11-01 13:58:00.00" version="[null]" path="" status="U" islast="false" depth="3" /> - + \ No newline at end of file diff --git a/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shouldInsertViolations-result.xml b/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shouldInsertViolations-result.xml index c28d5fd6403..47117589804 100644 --- a/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shouldInsertViolations-result.xml +++ b/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shouldInsertViolations-result.xml @@ -18,7 +18,7 @@ scope="FIL" qualifier="CLA" created_at="2008-11-01 13:58:00.00" version="[null]" path="" status="U" islast="false" depth="3" /> - + diff --git a/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shouldUpdateViolation-result.xml b/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shouldUpdateViolation-result.xml index 150433b43af..0ec12a8e75e 100644 --- a/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shouldUpdateViolation-result.xml +++ b/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shouldUpdateViolation-result.xml @@ -18,5 +18,5 @@ scope="FIL" qualifier="CLA" created_at="2008-11-01 13:58:00.00" version="[null]" path="" status="U" islast="false" depth="3" /> - + \ No newline at end of file -- 2.39.5