From 2a0fef63884f6bd58349bfd1e3a417b9f720f3e5 Mon Sep 17 00:00:00 2001 From: Fabrice Bellingard Date: Thu, 9 Feb 2012 17:04:26 +0100 Subject: [PATCH] Increase code coverage + fix SQL pitfall --- .../core/sensors/CloseReviewsDecorator.java | 62 +++++++++++++------ .../sensors/CloseReviewsDecoratorTest.java | 13 ++++ ...seReviewCorrespondingToDeletedResource.xml | 31 ++++++++++ 3 files changed, 86 insertions(+), 20 deletions(-) create mode 100644 plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewCorrespondingToDeletedResource.xml 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 bec450bc34b..6eae4132f76 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 @@ -45,9 +45,17 @@ import java.util.List; @DependedUpon(CloseReviewsDecorator.REVIEW_LIFECYCLE_BARRIER) public class CloseReviewsDecorator implements Decorator { - private static final Logger LOG = LoggerFactory.getLogger(CloseReviewsDecorator.class); public static final String REVIEW_LIFECYCLE_BARRIER = "REVIEW_LIFECYCLE_BARRIER"; + private static final String PROJECT_ID = "projectId"; + private static final String PROJECT_SNAPSHOT_ID = "projectSnapshotId"; + private static final String SNAPSHOT_ID = "snapshotId"; + private static final String RESOURCE_ID = "resourceId"; + private static final String IS_LAST = "isLast"; + private static final String MANUAL_VIOLATION = "manualViolation"; + private static final String AUTOMATIC_VIOLATION = "automaticViolation"; + private static final Logger LOG = LoggerFactory.getLogger(CloseReviewsDecorator.class); + private Project project; private ResourcePersister resourcePersister; private DatabaseSession databaseSession; @@ -67,6 +75,7 @@ public class CloseReviewsDecorator implements Decorator { return project.isLatestAnalysis(); } + @SuppressWarnings("rawtypes") public void decorate(Resource resource, DecoratorContext context) { Snapshot currentSnapshot = resourcePersister.getSnapshot(resource); if (currentSnapshot != null) { @@ -87,19 +96,22 @@ public class CloseReviewsDecorator implements Decorator { /** * Close reviews for which violations have been fixed. */ - protected int closeReviewsOnResolvedViolations(Resource resource, int resourceId, int snapshotId) { - String conditions = " WHERE resource_id=" + resourceId + " AND " + @SuppressWarnings({"unchecked"}) + protected int closeReviewsOnResolvedViolations(Resource resource, int resourceId, int snapshotId) { + String conditions = " WHERE resource_id=:" + RESOURCE_ID + " AND " + "( " + " ( " - + " manual_violation=:automaticViolation AND status!='CLOSED' AND " - + " rule_failure_permanent_id NOT IN (SELECT permanent_id FROM rule_failures WHERE snapshot_id=" + snapshotId + " AND permanent_id IS NOT NULL)" + + " manual_violation=:" + AUTOMATIC_VIOLATION + " AND status!='CLOSED' AND " + + " rule_failure_permanent_id NOT IN (SELECT permanent_id FROM rule_failures WHERE snapshot_id=:" + SNAPSHOT_ID + " AND permanent_id IS NOT NULL)" + " )" + " OR " - + " (manual_violation=:manualViolation AND status='RESOLVED')" + + " (manual_violation=:" + MANUAL_VIOLATION + " AND status='RESOLVED')" + ")"; List reviews = databaseSession.getEntityManager().createNativeQuery("SELECT * FROM reviews " + conditions, Review.class) - .setParameter("automaticViolation", false) - .setParameter("manualViolation", true) + .setParameter(RESOURCE_ID, resourceId) + .setParameter(SNAPSHOT_ID, snapshotId) + .setParameter(AUTOMATIC_VIOLATION, false) + .setParameter(MANUAL_VIOLATION, true) .getResultList(); for (Review review : reviews) { @@ -107,8 +119,10 @@ public class CloseReviewsDecorator implements Decorator { } int rowUpdated = databaseSession.createNativeQuery("UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP" + conditions) - .setParameter("automaticViolation", false) - .setParameter("manualViolation", true) + .setParameter(RESOURCE_ID, resourceId) + .setParameter(SNAPSHOT_ID, snapshotId) + .setParameter(AUTOMATIC_VIOLATION, false) + .setParameter(MANUAL_VIOLATION, true) .executeUpdate(); if (rowUpdated > 0) { LOG.debug("- {} reviews closed on #{}", rowUpdated, resourceId); @@ -120,17 +134,20 @@ public class CloseReviewsDecorator implements Decorator { * Reopen reviews that had been set to resolved but for which the violation is still here. * Manual violations are ignored. */ - protected int reopenReviewsOnUnresolvedViolations(Resource resource, int resourceId) { - String conditions = " WHERE status='RESOLVED' AND resolution<>'FALSE-POSITIVE' AND manual_violation=:manualViolation AND resource_id=" + resourceId; + @SuppressWarnings({"unchecked"}) + protected int reopenReviewsOnUnresolvedViolations(Resource resource, int resourceId) { + String conditions = " WHERE status='RESOLVED' AND resolution<>'FALSE-POSITIVE' AND manual_violation=:" + MANUAL_VIOLATION + " AND resource_id=:" + RESOURCE_ID; List reviews = databaseSession.getEntityManager().createNativeQuery("SELECT * FROM reviews " + conditions, Review.class) - .setParameter("manualViolation", false) + .setParameter(RESOURCE_ID, resourceId) + .setParameter(MANUAL_VIOLATION, false) .getResultList(); for (Review review : reviews) { notifyReopened(resource, review); } int rowUpdated = databaseSession.createNativeQuery("UPDATE reviews SET status='REOPENED', resolution=NULL, updated_at=CURRENT_TIMESTAMP" + conditions) - .setParameter("manualViolation", false) + .setParameter(RESOURCE_ID, resourceId) + .setParameter(MANUAL_VIOLATION, false) .executeUpdate(); if (rowUpdated > 0) { LOG.debug("- {} reviews reopened on #{}", rowUpdated, resourceId); @@ -141,19 +158,24 @@ public class CloseReviewsDecorator implements Decorator { /** * Close reviews that relate to resources that have been deleted or renamed. */ + @SuppressWarnings("unchecked") protected int closeReviewsForDeletedResources(int projectId, int projectSnapshotId) { - String conditions = " 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 ) )"; + String conditions = " WHERE status!='CLOSED' AND project_id=:" + PROJECT_ID + + " AND resource_id IN ( SELECT prev.project_id FROM snapshots prev WHERE prev.root_project_id=:" + PROJECT_ID + + " AND prev.islast=:" + IS_LAST + " AND NOT EXISTS ( SELECT cur.id FROM snapshots cur WHERE cur.root_snapshot_id=:" + PROJECT_SNAPSHOT_ID + + " AND cur.created_at > prev.created_at AND cur.root_project_id=:" + PROJECT_ID + " AND cur.project_id=prev.project_id ) )"; List reviews = databaseSession.getEntityManager().createNativeQuery("SELECT * FROM reviews " + conditions, Review.class) - .setParameter(1, Boolean.TRUE) + .setParameter(PROJECT_ID, projectId) + .setParameter(PROJECT_SNAPSHOT_ID, projectSnapshotId) + .setParameter(IS_LAST, Boolean.TRUE) .getResultList(); for (Review review : reviews) { notifyClosed(null, review); } int rowUpdated = databaseSession.createNativeQuery("UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP" + conditions) - .setParameter(1, Boolean.TRUE) + .setParameter(PROJECT_ID, projectId) + .setParameter(PROJECT_SNAPSHOT_ID, projectSnapshotId) + .setParameter(IS_LAST, Boolean.TRUE) .executeUpdate(); LOG.debug("- {} reviews set to 'closed' on project #{}", rowUpdated, projectSnapshotId); return rowUpdated; 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 27ec2e039d0..09b748903a5 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 @@ -108,4 +108,17 @@ public class CloseReviewsDecoratorTest extends AbstractDbUnitTestCase { assertThat(getSession().getSingleResult(Review.class, "id", 2L).getStatus(), is("CLOSED")); // manual violation resolved -> closed assertThat(getSession().getSingleResult(Review.class, "id", 3L).getStatus(), is("OPEN")); // manual violation not changed } + + @Test + public void shouldCloseReviewCorrespondingToDeletedResource() throws Exception { + setupData("shouldCloseReviewCorrespondingToDeletedResource"); + + int count = reviewsDecorator.closeReviewsForDeletedResources(111, 11); + assertThat(count, is(1)); + + verify(notificationManager, times(1)).scheduleForSending(any(Notification.class)); + + getSession().commit(); + assertThat(getSession().getSingleResult(Review.class, "id", 1L).getStatus(), is("CLOSED")); + } } diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewCorrespondingToDeletedResource.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewCorrespondingToDeletedResource.xml new file mode 100644 index 00000000000..847c740236d --- /dev/null +++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewCorrespondingToDeletedResource.xml @@ -0,0 +1,31 @@ + + + + + + + + \ No newline at end of file -- 2.39.5