Fix bug on UpdateReviewsDecorator when line number is NULL

4 files changed, 36 insertions, 3 deletions

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 b6c6e0081e3..9951b8f6e88 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
@@ -53,6 +53,7 @@ public class UpdateReviewsDecorator implements Decorator {
   private DatabaseSession databaseSession;
   private ViolationTrackingDecorator violationTrackingDecorator;
   private Query updateReviewQuery;
+  private Query updateReviewQueryForNullLine;
   private Map<Integer, Violation> violationsPerPermanentId;
 
   public UpdateReviewsDecorator(ResourcePersister resourcePersister, DatabaseSession databaseSession,
@@ -91,6 +92,8 @@ public class UpdateReviewsDecorator implements Decorator {
     // prepare the DB native queries
     updateReviewQuery = databaseSession
         .createNativeQuery("UPDATE reviews SET title=?, resource_line=?, updated_at=CURRENT_TIMESTAMP WHERE id=?");
+    updateReviewQueryForNullLine = databaseSession
+        .createNativeQuery("UPDATE reviews SET title=?, resource_line=NULL, updated_at=CURRENT_TIMESTAMP WHERE id=?");
     Query searchReviewsQuery = databaseSession.getEntityManager().createNativeQuery(
         "SELECT * FROM reviews WHERE status!='CLOSED' AND resource_id=?", Review.class);
     // and iterate over the reviews that we find
@@ -107,7 +110,11 @@ public class UpdateReviewsDecorator implements Decorator {
       Integer line = violation.getLineId();
       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();
+        if (line == null) {
+          updateReviewQueryForNullLine.setParameter(1, message).setParameter(2, review.getId()).executeUpdate();
+        } else {
+          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 01ff4f73906..6b283b3de88 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
@@ -81,12 +81,14 @@ public class UpdateReviewsDecoratorTest extends AbstractDbUnitTestCase {
     Violation v4 = mock(Violation.class);
     when(v4.getMessage()).thenReturn("message 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);
+    Violation v6 = mock(Violation.class);
+    when(v6.getMessage()).thenReturn("message 6");
+    when(v6.getLineId()).thenReturn(null);
     DecoratorContext context = mock(DecoratorContext.class);
-    when(context.getViolations()).thenReturn(Lists.newArrayList(v1, v2, v3, v4, v5));
+    when(context.getViolations()).thenReturn(Lists.newArrayList(v1, v2, v3, v4, v5, v6));
 
     RuleFailureModel rf1 = mock(RuleFailureModel.class);
     when(rf1.getPermanentId()).thenReturn(1);
@@ -103,6 +105,9 @@ public class UpdateReviewsDecoratorTest extends AbstractDbUnitTestCase {
     RuleFailureModel rf5 = mock(RuleFailureModel.class);
     when(rf5.getPermanentId()).thenReturn(5);
     when(violationTrackingDecorator.getReferenceViolation(v5)).thenReturn(rf5);
+    RuleFailureModel rf6 = mock(RuleFailureModel.class);
+    when(rf6.getPermanentId()).thenReturn(6);
+    when(violationTrackingDecorator.getReferenceViolation(v6)).thenReturn(rf6);
 
     setupData("fixture");
 
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 71bfcf97fac..7384c0e2e5f 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
@@ -55,5 +55,15 @@
     rule_id="[null]"
     manual_violation="false"
     manual_severity="false"/>
+  <reviews
+    id="6"
+    status="OPEN"
+    rule_failure_permanent_id="6"
+    resource_id="1"
+    title="message OLD"
+    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 cb41697da88..55e6c7fc05a 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
@@ -55,5 +55,16 @@
     rule_id="[null]"
     manual_violation="false"
     manual_severity="false"/>
+  <reviews
+    id="6"
+    status="OPEN"
+    rule_failure_permanent_id="6"
+    resource_id="1"
+    title="message 6"
+    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