merge 2.5: fix tracking of violations when message contains whitespace or is too long - fix a bug + add unit tests

2 files changed, 57 insertions, 15 deletions

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 28b33e281bd..2c1c31fe9e4 100644
--- 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
@@ -45,8 +45,7 @@ import java.util.Collections;
 import java.util.List;
 
 @DependsUpon(DecoratorBarriers.END_OF_VIOLATIONS_GENERATION)
-@DependedUpon("ViolationPersisterDecorator")
-/* temporary workaround - see NewViolationsDecorator */
+@DependedUpon("ViolationPersisterDecorator") /* temporary workaround - see NewViolationsDecorator */
 public class ViolationPersisterDecorator implements Decorator {
 
   /**
@@ -119,7 +118,7 @@ public class ViolationPersisterDecorator implements Decorator {
       }
     } catch (IOException e) {
       throw new SonarException("Unable to calculate checksums", e);
-
+      
     } finally {
       IOUtils.closeQuietly(stream);
     }
@@ -146,13 +145,14 @@ public class ViolationPersisterDecorator implements Decorator {
    */
   RuleFailureModel selectPastViolation(Violation violation, Multimap<Rule, RuleFailureModel> pastViolationsByRule) {
     Collection<RuleFailureModel> pastViolations = pastViolationsByRule.get(violation.getRule());
-    if (pastViolations == null || pastViolations.isEmpty()) {
+    if (pastViolations==null || pastViolations.isEmpty()) {
       // skip violation, if there is no past violations with same rule
       return null;
     }
-    RuleFailureModel found = selectPastViolationUsingLine(violation, pastViolations);
+    String dbFormattedMessage = RuleFailureModel.abbreviateMessage(violation.getMessage());
+    RuleFailureModel found = selectPastViolationUsingLine(violation, dbFormattedMessage, pastViolations);
     if (found == null) {
-      found = selectPastViolationUsingChecksum(violation, pastViolations);
+      found = selectPastViolationUsingChecksum(violation, dbFormattedMessage, pastViolations);
     }
     return found;
   }
@@ -160,9 +160,9 @@ public class ViolationPersisterDecorator implements Decorator {
   /**
    * Search for past violation with same message and line.
    */
-  private RuleFailureModel selectPastViolationUsingLine(Violation violation, Collection<RuleFailureModel> pastViolations) {
+  private RuleFailureModel selectPastViolationUsingLine(Violation violation, String dbFormattedMessage, Collection<RuleFailureModel> pastViolations) {
     for (RuleFailureModel pastViolation : pastViolations) {
-      if (ObjectUtils.equals(violation.getLineId(), pastViolation.getLine()) && StringUtils.equals(violation.getMessage(), pastViolation.getMessage())) {
+      if (ObjectUtils.equals(violation.getLineId(), pastViolation.getLine()) && StringUtils.equals(dbFormattedMessage, pastViolation.getMessage())) {
         return pastViolation;
       }
     }
@@ -172,7 +172,7 @@ public class ViolationPersisterDecorator implements Decorator {
   /**
    * Search for past violation with same message and checksum.
    */
-  private RuleFailureModel selectPastViolationUsingChecksum(Violation violation, Collection<RuleFailureModel> pastViolations) {
+  private RuleFailureModel selectPastViolationUsingChecksum(Violation violation, String dbFormattedMessage, Collection<RuleFailureModel> pastViolations) {
     String checksum = getChecksumForLine(checksums, violation.getLineId());
     // skip violation, which not attached to line
     if (checksum == null) {
@@ -180,7 +180,7 @@ public class ViolationPersisterDecorator implements Decorator {
     }
     for (RuleFailureModel pastViolation : pastViolations) {
       String pastChecksum = pastViolation.getChecksum();
-      if (StringUtils.equals(checksum, pastChecksum) && StringUtils.equals(violation.getMessage(), pastViolation.getMessage())) {
+      if (StringUtils.equals(checksum, pastChecksum) && StringUtils.equals(dbFormattedMessage, pastViolation.getMessage())) {
         return pastViolation;
       }
     }
diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecoratorTest.java
index 12feac164f6..741496af6a4 100644
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecoratorTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecoratorTest.java
@@ -1,3 +1,22 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2009 SonarSource SA
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * Sonar is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
 package org.sonar.plugins.core.timemachine;
 
 import com.google.common.collect.LinkedHashMultimap;
@@ -40,6 +59,7 @@ public class ViolationPersisterDecoratorTest {
   @Test
   public void sameRuleLineMessage() {
     Rule rule = Rule.create().setKey("rule");
+    rule.setId(50);
     Violation violation = Violation.create(rule, null)
         .setLineId(1).setMessage("message");
 
@@ -55,6 +75,7 @@ public class ViolationPersisterDecoratorTest {
   @Test
   public void sameRuleAndMessageButDifferentLine() {
     Rule rule = Rule.create().setKey("rule");
+    rule.setId(50);
     Violation violation = Violation.create(rule, null)
         .setLineId(1).setMessage("message");
     decorator.checksums = ViolationPersisterDecorator.getChecksums("violation");
@@ -70,7 +91,7 @@ public class ViolationPersisterDecoratorTest {
   }
 
   @Test
-  public void newViolation() {
+  public void shouldCreateNewViolation() {
     Rule rule = Rule.create().setKey("rule");
     Violation violation = Violation.create(rule, null)
         .setLineId(1).setMessage("message");
@@ -85,25 +106,46 @@ public class ViolationPersisterDecoratorTest {
   }
 
   @Test
-  public void differentRule() {
+  public void shouldNotTrackViolationIfDifferentRule() {
     Rule rule = Rule.create().setKey("rule");
+    rule.setId(50);
     Violation violation = Violation.create(rule, null)
         .setLineId(1).setMessage("message");
 
-    Rule anotherRule = Rule.create().setKey("anotherRule");
-    RuleFailureModel pastViolation = newPastViolation(anotherRule, 1, "message");
+    Rule otherRule = Rule.create().setKey("anotherRule");
+    otherRule.setId(244);
+    RuleFailureModel pastViolationOnOtherRule = newPastViolation(otherRule, 1, "message");
 
     Multimap<Rule, RuleFailureModel> pastViolationsByRule = LinkedHashMultimap.create();
-    pastViolationsByRule.put(anotherRule, pastViolation);
+    pastViolationsByRule.put(otherRule, pastViolationOnOtherRule);
 
     RuleFailureModel found = decorator.selectPastViolation(violation, pastViolationsByRule);
     assertThat(found, nullValue());
   }
 
+  @Test
+  public void shouldCompareViolationsWithDatabaseFormat() {
+    // violation messages are trimmed and can be abbreviated when persisted in database.
+    // Comparing violation messages must use the same format.
+    Rule rule = Rule.create().setKey("rule");
+    rule.setId(50);
+    Violation violation = Violation.create(rule, null)
+        .setLineId(30).setMessage("   message     "); // starts and ends with whitespaces
+
+    RuleFailureModel pastViolation = newPastViolation(rule, 30, "message"); // trimmed in database
+
+    Multimap<Rule, RuleFailureModel> pastViolationsByRule = LinkedHashMultimap.create();
+    pastViolationsByRule.put(rule, pastViolation);
+
+    RuleFailureModel found = decorator.selectPastViolation(violation, pastViolationsByRule);
+    assertThat(found, equalTo(pastViolation));
+  }
+
   private RuleFailureModel newPastViolation(Rule rule, Integer line, String message) {
     RuleFailureModel pastViolation = new RuleFailureModel();
     pastViolation.setLine(line);
     pastViolation.setMessage(message);
+    pastViolation.setRuleId(rule.getId());
     return pastViolation;
   }