SONAR-1450: Add support for incremental review of incoming violations

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<RuleFailureModel> 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;
@@ -60,14 +58,6 @@ public class ViolationPersisterTest extends AbstractDbUnitTestCase {
   }
 
   @Test
-  public void shouldLoadViolations() {
-    List<RuleFailureModel> models = violationPersister.getPreviousViolations(javaFile);
-
-    assertThat(models, notNullValue());
-    assertThat(models.size(), greaterThan(0));
-  }
-
-  @Test
   public void shouldSaveViolations() {
     Violation violation1a = Violation.create(rule1, javaFile)
         .setPriority(RulePriority.CRITICAL).setLineId(20).setCost(55.6)
@@ -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" />
 
-  <RULE_FAILURES ID="1" SNAPSHOT_ID="1000" RULE_ID="30" FAILURE_LEVEL="3" MESSAGE="the message" LINE="20" COST="[null]"/>
+  <RULE_FAILURES ID="1" SNAPSHOT_ID="1000" RULE_ID="30" FAILURE_LEVEL="3" MESSAGE="old message" LINE="10" COST="[null]"/>
 </dataset>
\ 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" />
 
-  <RULE_FAILURES ID="1" SNAPSHOT_ID="1000" RULE_ID="30" FAILURE_LEVEL="3" MESSAGE="the message" LINE="20" COST="[null]"/>
+  <RULE_FAILURES ID="1" SNAPSHOT_ID="1000" RULE_ID="30" FAILURE_LEVEL="3" MESSAGE="old message" LINE="10" COST="[null]"/>
   <RULE_FAILURES ID="2" SNAPSHOT_ID="1000" RULE_ID="30" FAILURE_LEVEL="3" MESSAGE="the message" LINE="20" COST="55.6"/>
   <RULE_FAILURES ID="3" SNAPSHOT_ID="1000" RULE_ID="30" FAILURE_LEVEL="3" MESSAGE="[null]" LINE="50" COST="80"/>
   <RULE_FAILURES ID="4" SNAPSHOT_ID="1000" RULE_ID="31" FAILURE_LEVEL="1" MESSAGE="[null]" LINE="[null]" COST="[null]"/>
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" />
 
-  <RULE_FAILURES ID="1" SNAPSHOT_ID="1000" RULE_ID="30" FAILURE_LEVEL="3" MESSAGE="the message" LINE="20" COST="55.6"/>
+  <RULE_FAILURES ID="1" SNAPSHOT_ID="1000" RULE_ID="30" FAILURE_LEVEL="3" MESSAGE="old message" LINE="10" COST="55.6"/>
 </dataset>
\ No newline at end of file