SONAR-2520 Add support for RESOLVED and REOPENED statuses for reviews

- Modifications on the Web UI side to allow to resolve reviews
- Modifications on the batch side to reopen reviews that were
  specified as resolved but the violation has not been fixed
- Modifications on the WS server side to allow to change the status
  of a review to RESOLVED or REOPENED
- Modifications on the WS client to reflect those server side changes

5 files changed, 149 insertions, 8 deletions

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java
index f0fe71402de..922e3154e2d 100644
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java
@@ -61,12 +61,20 @@ public class CloseReviewsDecorator implements Decorator {
     if (currentSnapshot != null) {
       int resourceId = currentSnapshot.getResourceId();
       int snapshotId = currentSnapshot.getId();
-      Query query = databaseSession.createNativeQuery(generateUpdateOnResourceSqlRequest(resourceId, snapshotId));
+
+      // Close reviews for which violations have been fixed
+      Query query = databaseSession.createNativeQuery(generateUpdateToCloseReviewsForFixedViolation(resourceId, snapshotId));
       int rowUpdated = query.executeUpdate();
       LOG.debug("- {} reviews set to 'closed' on resource #{}", rowUpdated, resourceId);
 
+      // Reopen reviews that had been set to resolved but for which the violation is still here
+      query = databaseSession.createNativeQuery(generateUpdateToReopenResolvedReviewsForNonFixedViolation(resourceId));
+      rowUpdated = query.executeUpdate();
+      LOG.debug("- {} reviews set to 'reopened' on resource #{}", rowUpdated, resourceId);
+
+      // And close reviews that relate to resources that have been deleted or renamed
       if (ResourceUtils.isRootProject(resource)) {
-        query = databaseSession.createNativeQuery(generateUpdateOnProjectSqlRequest(resourceId, currentSnapshot.getId()));
+        query = databaseSession.createNativeQuery(generateUpdateToCloseReviewsForDeletedResources(resourceId, currentSnapshot.getId()));
         query.setParameter(1, Boolean.TRUE);
         rowUpdated = query.executeUpdate();
         LOG.debug("- {} reviews set to 'closed' on project #{}", rowUpdated, resourceId);
@@ -76,14 +84,18 @@ public class CloseReviewsDecorator implements Decorator {
     }
   }
 
-  protected String generateUpdateOnResourceSqlRequest(int resourceId, int snapshotId) {
-    return "UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP WHERE status='OPEN' AND resource_id = " + resourceId
+  protected String generateUpdateToCloseReviewsForFixedViolation(int resourceId, int snapshotId) {
+    return "UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP WHERE status!='CLOSED' AND resource_id = " + resourceId
         + " AND rule_failure_permanent_id NOT IN " + "(SELECT permanent_id FROM rule_failures WHERE snapshot_id = " + snapshotId
         + " AND permanent_id IS NOT NULL)";
   }
 
-  protected String generateUpdateOnProjectSqlRequest(int projectId, int projectSnapshotId) {
-    return "UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP WHERE status='OPEN' AND project_id=" + projectId
+  protected String generateUpdateToReopenResolvedReviewsForNonFixedViolation(int resourceId) {
+    return "UPDATE reviews SET status='REOPENED', updated_at=CURRENT_TIMESTAMP WHERE status='RESOLVED' AND resource_id = " + resourceId;
+  }
+
+  protected String generateUpdateToCloseReviewsForDeletedResources(int projectId, int projectSnapshotId) {
+    return "UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP WHERE status!='CLOSED' AND project_id=" + projectId
         + " AND resource_id IN ( SELECT prev.project_id FROM snapshots prev  WHERE prev.root_project_id=" + projectId
         + " AND prev.islast=? AND NOT EXISTS ( SELECT cur.id FROM snapshots cur WHERE cur.root_snapshot_id=" + projectSnapshotId
         + " AND cur.created_at > prev.created_at AND cur.root_project_id=" + projectId + " AND cur.project_id=prev.project_id ) )";
diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java
index 2ea8a7c1de4..17ff7a09293 100644
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java
@@ -49,12 +49,12 @@ public class CloseReviewsDecoratorTest extends DatabaseTestCase {
     setupData("fixture");
 
     CloseReviewsDecorator reviewsDecorator = new CloseReviewsDecorator(null, null);
-    String sqlRequest = reviewsDecorator.generateUpdateOnResourceSqlRequest(666, 222);
+    String sqlRequest = reviewsDecorator.generateUpdateToCloseReviewsForFixedViolation(666, 222);
 
     Statement stmt = getConnection().createStatement();
     int count = stmt.executeUpdate(sqlRequest);
 
-    assertThat(count, is(1));
+    assertThat(count, is(3));
     assertTables("shouldCloseReviewWithoutCorrespondingViolation", new String[] { "reviews" }, new String[] { "updated_at" });
 
     try {
@@ -64,4 +64,29 @@ public class CloseReviewsDecoratorTest extends DatabaseTestCase {
       // "updated_at" column must be different, so the comparison should raise this exception
     }
   }
+
+  @Test
+  public void shouldReopenResolvedReviewWithNonFixedViolation() throws Exception {
+    setupData("fixture");
+
+    CloseReviewsDecorator reviewsDecorator = new CloseReviewsDecorator(null, null);
+
+    // First we close the reviews for which the violations have been fixed (this is because we use the same "fixture"...)
+    getConnection().createStatement().executeUpdate(reviewsDecorator.generateUpdateToCloseReviewsForFixedViolation(666, 222));
+
+    // And now we reopen the reviews that still have a violation
+    String sqlRequest = reviewsDecorator.generateUpdateToReopenResolvedReviewsForNonFixedViolation(666);
+    Statement stmt = getConnection().createStatement();
+    int count = stmt.executeUpdate(sqlRequest);
+
+    assertThat(count, is(1));
+    assertTables("shouldReopenResolvedReviewWithNonFixedViolation", new String[] { "reviews" }, new String[] { "updated_at" });
+
+    try {
+      assertTables("shouldReopenResolvedReviewWithNonFixedViolation", new String[] { "reviews" });
+      fail("'updated_at' columns are identical whereas they should be different.");
+    } catch (ComparisonFailure e) {
+      // "updated_at" column must be different, so the comparison should raise this exception
+    }
+  }
 }
diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml
index 2f432d7f745..5c1b54ac173 100644
--- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml
@@ -1,5 +1,6 @@
 <dataset>
 
+  <!-- Component 555 -->
   <snapshots 
              id="11" 
              project_id="555"
@@ -8,6 +9,7 @@
              id="111" 
              project_id="555"
              status="P" islast="false"/>
+  <!-- Component 666 -->
   <snapshots 
              id="22" 
              project_id="666"
@@ -17,6 +19,7 @@
              project_id="666"
              status="P" islast="false"/>
 
+  <!-- Violations on previous analysis -->
   <rule_failures 
              id="1"
              permanent_id="1"
@@ -32,17 +35,22 @@
              permanent_id="3"
              snapshot_id="22"
              rule_id="1" failure_level="1"/>
+  <!-- Violations on new analysis -->
+  <!-- Violation #1 still exists -->
   <rule_failures 
              id="4"
              permanent_id="1"
              snapshot_id="111"
              rule_id="1" failure_level="1"/>
+  <!-- Violation #2 has been fixed -->
+  <!-- Violation #3 still exists -->
   <rule_failures 
              id="5"
              permanent_id="3"
              snapshot_id="222"
              rule_id="1" failure_level="1"/>
 
+  <!-- Existing reviews -->
   <reviews
   			id="1"
   			status="OPEN"
@@ -63,5 +71,25 @@
   			status="CLOSED"
   			rule_failure_permanent_id="2"
   			resource_id="666"/>
+  <reviews
+  			id="5"
+  			status="REOPENED"
+  			rule_failure_permanent_id="3"
+  			resource_id="666"/>
+  <reviews
+  			id="6"
+  			status="RESOLVED"
+  			rule_failure_permanent_id="3"
+  			resource_id="666"/>
+  <reviews
+  			id="7"
+  			status="REOPENED"
+  			rule_failure_permanent_id="2"
+  			resource_id="666"/>
+  <reviews
+  			id="8"
+  			status="RESOLVED"
+  			rule_failure_permanent_id="2"
+  			resource_id="666"/>
 
 </dataset>
\ No newline at end of file
diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml
index 7b3d60ec42c..f74c2d97a73 100644
--- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml
@@ -24,5 +24,29 @@
   			rule_failure_permanent_id="2"
   			resource_id="666"
   			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  <reviews
+  			id="5"
+  			status="REOPENED"
+  			rule_failure_permanent_id="3"
+  			resource_id="666"
+  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  <reviews
+  			id="6"
+  			status="RESOLVED"
+  			rule_failure_permanent_id="3"
+  			resource_id="666"
+  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  <reviews
+  			id="7"
+  			status="CLOSED"
+  			rule_failure_permanent_id="2"
+  			resource_id="666"
+  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  <reviews
+  			id="8"
+  			status="CLOSED"
+  			rule_failure_permanent_id="2"
+  			resource_id="666"
+  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
 
 </dataset>
\ No newline at end of file
diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldReopenResolvedReviewWithNonFixedViolation-result.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldReopenResolvedReviewWithNonFixedViolation-result.xml
new file mode 100644
index 00000000000..0397576d6b3
--- /dev/null
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldReopenResolvedReviewWithNonFixedViolation-result.xml
@@ -0,0 +1,52 @@
+<dataset>
+
+  <reviews
+  			id="1"
+  			status="OPEN"
+  			rule_failure_permanent_id="1"
+  			resource_id="555"
+  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  <reviews
+  			id="2"
+  			status="CLOSED"
+  			rule_failure_permanent_id="2"
+  			resource_id="666"
+  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  <reviews
+  			id="3"
+  			status="OPEN"
+  			rule_failure_permanent_id="3"
+  			resource_id="666"
+  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  <reviews
+  			id="4"
+  			status="CLOSED"
+  			rule_failure_permanent_id="2"
+  			resource_id="666"
+  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  <reviews
+  			id="5"
+  			status="REOPENED"
+  			rule_failure_permanent_id="3"
+  			resource_id="666"
+  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  <reviews
+  			id="6"
+  			status="REOPENED"
+  			rule_failure_permanent_id="3"
+  			resource_id="666"
+  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  <reviews
+  			id="7"
+  			status="CLOSED"
+  			rule_failure_permanent_id="2"
+  			resource_id="666"
+  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  <reviews
+  			id="8"
+  			status="CLOSED"
+  			rule_failure_permanent_id="2"
+  			resource_id="666"
+  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+
+</dataset>
\ No newline at end of file