SONAR-2251 Unstable state of violations when stopping a build

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecorator.java
index e1299a013651e6322b339cb06c611e709a5e7b6e..57460299a54a92808b2c4e9d57d6f7c5ab1f301e 100644 (file)
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecorator.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecorator.java
@@ -93,11 +93,14 @@ public class ViolationPersisterDecorator implements Decorator {
     for (Violation violation : context.getViolations()) {
       RuleFailureModel pastViolation = selectPastViolation(violation, pastViolationsByRule);
       if (pastViolation != null) {
-        // remove violation from past, since would be updated and shouldn't affect other violations anymore
+        // remove violation, since would be updated and shouldn't affect other violations anymore
         pastViolationsByRule.remove(violation.getRule(), pastViolation);
+        violation.setCreatedAt(pastViolation.getCreatedAt());
+      } else {
+        violation.setCreatedAt(null);//avoid plugins to set date
       }
       String checksum = getChecksumForLine(checksums, violation.getLineId());
-      violationPersister.saveOrUpdateViolation(context.getProject(), violation, pastViolation, checksum);
+      violationPersister.saveViolation(context.getProject(), violation, checksum);
     }
   }
 

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 c840adbfa7836cdc2e907a80da1eda56a5f31e6e..d6cbcf1ac27a1befd88ff0695454356a0abd1cc0 100644 (file)
--- a/sonar-batch/src/main/java/org/sonar/batch/index/ViolationPersister.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/index/ViolationPersister.java
@@ -40,38 +40,30 @@ public final class ViolationPersister {
   }
 
   void saveViolation(Project project, Violation violation) {
-    saveOrUpdateViolation(project, violation, null, null);
+    saveViolation(project, violation, null);
   }
 
-  public void saveOrUpdateViolation(Project project, Violation violation, RuleFailureModel model, String checksum) {
+  public void saveViolation(Project project, Violation violation, String checksum) {
     Snapshot snapshot = resourcePersister.saveResource(project, violation.getResource());
-    if (model != null) {
-      // update
-      model = session.reattach(RuleFailureModel.class, model.getId());
-      model = mergeModel(violation, model);
-    } else {
-      // insert
-      model = createModel(violation);
-      model.setCreatedAt(snapshot.getCreatedAt());
+    if (violation.getCreatedAt()==null) {
+      violation.setCreatedAt(snapshot.getCreatedAt());
     }
+    RuleFailureModel model = createModel(violation);
     model.setSnapshotId(snapshot.getId());
     model.setChecksum(checksum);
     session.save(model);
     violation.setMessage(model.getMessage());// the message can be changed in the class RuleFailure (truncate + trim)
-    violation.setCreatedAt(model.getCreatedAt());
   }
 
   private RuleFailureModel createModel(Violation violation) {
-    return mergeModel(violation, new RuleFailureModel());
-  }
-
-  private RuleFailureModel mergeModel(Violation violation, RuleFailureModel merge) {
+    RuleFailureModel model = new RuleFailureModel();
     Rule rule = ruleFinder.findByKey(violation.getRule().getRepositoryKey(), violation.getRule().getKey());
-    merge.setRuleId(rule.getId());
-    merge.setPriority(violation.getSeverity());
-    merge.setLine(violation.getLineId());
-    merge.setMessage(violation.getMessage());
-    merge.setCost(violation.getCost());
-    return merge;
+    model.setRuleId(rule.getId());
+    model.setPriority(violation.getSeverity());
+    model.setCreatedAt(violation.getCreatedAt());
+    model.setLine(violation.getLineId());
+    model.setMessage(violation.getMessage());
+    model.setCost(violation.getCost());
+    return model;
   }
 }

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 7d90a707941fd38c0a294922afa09d0fd88b52cc..9b27d33b2517c5f9dbc421b4032527763730b383 100644 (file)
--- a/sonar-batch/src/test/java/org/sonar/batch/index/ViolationPersisterTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/index/ViolationPersisterTest.java
@@ -73,15 +73,4 @@ public class ViolationPersisterTest extends AbstractDbUnitTestCase {
     checkTables("shouldInsertViolations", "rule_failures");
   }
 
-  @Test
-  public void shouldUpdateViolation() {
-    Violation violation = Violation.create(rule1, javaFile)
-        .setLineId(20).setCost(55.6).setSeverity(RulePriority.MINOR);
-    RuleFailureModel model = getSession().getSingleResult(RuleFailureModel.class, "id", 1);
-
-    violationPersister.saveOrUpdateViolation(new Project("project"), violation, model, null);
-
-    assertThat(violation.getCreatedAt(), notNullValue());
-    checkTables("shouldUpdateViolation", "rule_failures");
-  }
 }

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
deleted file mode 100644 (file)
index b54b33d..0000000
--- a/sonar-batch/src/test/resources/org/sonar/batch/index/ViolationPersisterTest/shouldUpdateViolation-result.xml
+++ /dev/null
@@ -1,23 +0,0 @@
-<dataset>
-  <rules_categories id="1" name="Efficiency" description="[null]"/>
-  <rules_categories id="6" name="Usability" description="[null]"/>
-
-  <rules id="30" name="Check Header" plugin_rule_key="com.puppycrawl.tools.checkstyle.checks.header.HeaderCheck"
-         plugin_config_key="Checker/Treewalker/HeaderCheck" plugin_name="checkstyle" description="[null]" priority="4" enabled="true"
-         cardinality="SINGLE" parent_id="[null]"/>
-
-  <rules id="31" name="Equals Avoid Null" plugin_rule_key="com.puppycrawl.tools.checkstyle.checks.coding.EqualsAvoidNullCheck"
-         plugin_config_key="Checker/TreeWalker/EqualsAvoidNull" plugin_name="checkstyle" description="[null]" priority="4" enabled="true"
-         cardinality="SINGLE" parent_id="[null]"/>
-
-  <projects id="200" scope="FIL" qualifier="CLA" kee="project:org.foo.Bar" root_id="[null]"
-            name="Bar" long_name="org.foo.Bar" description="[null]"
-            enabled="true" language="java" copy_resource_id="[null]" profile_id="[null]"/>
-
-  <snapshots period1_mode="[null]" period1_param="[null]" period1_date="[null]" period2_mode="[null]" period2_param="[null]" period2_date="[null]" period3_mode="[null]" period3_param="[null]" period3_date="[null]" period4_mode="[null]" period4_param="[null]" period4_date="[null]" period5_mode="[null]" period5_param="[null]" period5_date="[null]" id="1000" project_id="200" parent_snapshot_id="[null]" root_project_id="100" root_snapshot_id="[null]"
-             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="1" MESSAGE="old message" LINE="20" COST="55.6" created_at="2008-11-01 13:58:00.00" checksum="[null]"/>
-  <RULE_FAILURES ID="2" SNAPSHOT_ID="1000" RULE_ID="30" FAILURE_LEVEL="3" MESSAGE="old message" LINE="10" COST="[null]" created_at="2008-11-01 13:58:00.00" checksum="[null]"/>
-</dataset>

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/database/model/RuleFailureModel.java b/sonar-plugin-api/src/main/java/org/sonar/api/database/model/RuleFailureModel.java
index ae0f9531c891c7e56a12bb43343f2bbef945c30e..fdcd25583e079e2c6236f35340f6398b1f2143a9 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/database/model/RuleFailureModel.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/database/model/RuleFailureModel.java
@@ -74,10 +74,18 @@ public class RuleFailureModel extends BaseIdentifiable {
     return StringUtils.abbreviate(StringUtils.trim(message), MESSAGE_COLUMN_SIZE);
   }
 
+  /**
+   * @deprecated since 2.7. Replace by getPriority()
+   */
+  @Deprecated
   public RulePriority getLevel() {
     return priority;
   }
 
+  /**
+   * @deprecated since 2.7. Replace by setPriority()
+   */
+  @Deprecated
   public void setLevel(RulePriority priority) {
     this.priority = priority;
   }