From: Fabrice Bellingard <bellingard@gmail.com>
Date: Wed, 4 Jan 2012 08:56:50 +0000 (+0100)
Subject: Fix bug on UpdateReviewsDecorator when a violation has no line number
X-Git-Tag: 2.14~357
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=95ff06f763abc141fe16eb197f936279afd2e11c;p=sonarqube.git

Fix bug on UpdateReviewsDecorator when a violation has no line number
---

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/UpdateReviewsDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/UpdateReviewsDecorator.java
index 790f0929511..b6c6e0081e3 100644
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/UpdateReviewsDecorator.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/UpdateReviewsDecorator.java
@@ -105,7 +105,8 @@ public class UpdateReviewsDecorator implements Decorator {
     if (violation != null) {
       String message = violation.getMessage();
       Integer line = violation.getLineId();
-      if ( !review.getTitle().equals(message) || !review.getResourceLine().equals(line)) {
+      if ( !review.getTitle().equals(message) || (review.getResourceLine() == null && line != null)
+          || (review.getResourceLine() != null && !review.getResourceLine().equals(line))) {
         updateReviewQuery.setParameter(1, message).setParameter(2, line).setParameter(3, review.getId()).executeUpdate();
       }
     }
diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/UpdateReviewsDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/UpdateReviewsDecoratorTest.java
index 2a8797cf979..01ff4f73906 100644
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/UpdateReviewsDecoratorTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/UpdateReviewsDecoratorTest.java
@@ -51,12 +51,12 @@ public class UpdateReviewsDecoratorTest extends AbstractDbUnitTestCase {
     Snapshot snapshot = new Snapshot();
     snapshot.setResourceId(1);
     when(resourcePersister.getSnapshot(resource)).thenReturn(snapshot);
-    
+
     Project project = mock(Project.class);
     when(project.getRoot()).thenReturn(project);
-    
+
     violationTrackingDecorator = mock(ViolationTrackingDecorator.class);
-    
+
     reviewsDecorator = new UpdateReviewsDecorator(resourcePersister, getSession(), violationTrackingDecorator);
   }
 
@@ -80,10 +80,14 @@ public class UpdateReviewsDecoratorTest extends AbstractDbUnitTestCase {
     when(v3.getLineId()).thenReturn(3);
     Violation v4 = mock(Violation.class);
     when(v4.getMessage()).thenReturn("message 4");
-    when(v4.getLineId()).thenReturn(4);    
+    when(v4.getLineId()).thenReturn(4);
+    // specific case when a violation has no line number
+    Violation v5 = mock(Violation.class);
+    when(v5.getMessage()).thenReturn("message 5");
+    when(v5.getLineId()).thenReturn(null);
     DecoratorContext context = mock(DecoratorContext.class);
-    when(context.getViolations()).thenReturn(Lists.newArrayList(v1,v2,v3,v4));
-    
+    when(context.getViolations()).thenReturn(Lists.newArrayList(v1, v2, v3, v4, v5));
+
     RuleFailureModel rf1 = mock(RuleFailureModel.class);
     when(rf1.getPermanentId()).thenReturn(1);
     when(violationTrackingDecorator.getReferenceViolation(v1)).thenReturn(rf1);
@@ -96,12 +100,14 @@ public class UpdateReviewsDecoratorTest extends AbstractDbUnitTestCase {
     RuleFailureModel rf4 = mock(RuleFailureModel.class);
     when(rf4.getPermanentId()).thenReturn(4);
     when(violationTrackingDecorator.getReferenceViolation(v4)).thenReturn(rf4);
-    
+    RuleFailureModel rf5 = mock(RuleFailureModel.class);
+    when(rf5.getPermanentId()).thenReturn(5);
+    when(violationTrackingDecorator.getReferenceViolation(v5)).thenReturn(rf5);
+
     setupData("fixture");
-    
+
     reviewsDecorator.decorate(resource, context);
-    
-    
+
     checkTables("shouldUpdateReviews", new String[] { "updated_at" }, new String[] { "reviews" });
   }
 }
diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/UpdateReviewsDecoratorTest/fixture.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/UpdateReviewsDecoratorTest/fixture.xml
index 62e91386059..71bfcf97fac 100644
--- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/UpdateReviewsDecoratorTest/fixture.xml
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/UpdateReviewsDecoratorTest/fixture.xml
@@ -45,5 +45,15 @@
     rule_id="[null]"
     manual_violation="false"
     manual_severity="false"/>
+  <reviews
+    id="5"
+    status="OPEN"
+    rule_failure_permanent_id="5"
+    resource_id="1"
+    title="message 5"
+    resource_line="[null]"
+    rule_id="[null]"
+    manual_violation="false"
+    manual_severity="false"/>
 
 </dataset>
\ No newline at end of file
diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/UpdateReviewsDecoratorTest/shouldUpdateReviews-result.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/UpdateReviewsDecoratorTest/shouldUpdateReviews-result.xml
index 83dd96b6121..cb41697da88 100644
--- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/UpdateReviewsDecoratorTest/shouldUpdateReviews-result.xml
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/UpdateReviewsDecoratorTest/shouldUpdateReviews-result.xml
@@ -44,5 +44,16 @@
     rule_id="[null]"
     manual_violation="false"
     manual_severity="false"/>
+  <reviews
+    id="5"
+    status="OPEN"
+    rule_failure_permanent_id="5"
+    resource_id="1"
+    title="message 5"
+    resource_line="[null]"
+    created_at="[null]" user_id="[null]" assignee_id="[null]" resolution="[null]" severity="[null]" project_id="[null]"
+    rule_id="[null]"
+    manual_violation="false"
+    manual_severity="false"/>
 
 </dataset>
\ No newline at end of file