NIFI-1984: Ensure that if an Exception is thrown by the 'Deletion Task' when calling NaiveRevisionManager.deleteRevision() that the locking is appropriately cleaned up

This closes #510

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/main/java/org/apache/nifi/web/revision/NaiveRevisionManager.java

@@ -209,7 +209,21 @@ public class NaiveRevisionManager implements RevisionManager {
         if (successCount == revisionList.size()) {
             logger.debug("Successfully verified Revision Claim for all revisions {}", claim);
 
-            final T taskValue = task.performTask();
+            final T taskValue;
+            try {
+                taskValue = task.performTask();
+            } catch (final Exception e) {
+                logger.debug("Failed to perform Claim Deletion task. Will relinquish the Revision Claims for the following revisions: {}", revisionList);
+
+                for (final Revision revision : revisionList) {
+                    final RevisionLock revisionLock = getRevisionLock(revision);
+                    revisionLock.unlock(revision, revision, user.getUserName());
+                    logger.debug("Relinquished lock for {}", revision);
+                }
+
+                throw e;
+            }
+
             for (final Revision revision : revisionList) {
                 deleteRevisionLock(revision);
                 logger.debug("Deleted Revision {}", revision);

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/test/java/org/apache/nifi/web/revision/TestNaiveRevisionManager.java

@@ -615,4 +615,100 @@ public class TestNaiveRevisionManager {
         assertTrue(component1Seen);
         assertTrue(component2Seen);
     }
+
+    @Test(timeout = 5000)
+    public void testWriteLockReleasedWhenClaimCanceledByRevision() {
+        final RevisionManager revisionManager = new NaiveRevisionManager(2, TimeUnit.SECONDS);
+        final Revision component1V1 = new Revision(1L, CLIENT_1, COMPONENT_1);
+
+        final RevisionClaim claim = revisionManager.requestClaim(component1V1, USER_1);
+        assertNotNull(claim);
+        assertEquals(1, claim.getRevisions().size());
+        assertEquals(component1V1, claim.getRevisions().iterator().next());
+
+        assertTrue(revisionManager.cancelClaim(component1V1));
+
+        final RevisionClaim claim2 = revisionManager.requestClaim(component1V1, USER_1);
+        assertNotNull(claim2);
+        assertEquals(1, claim2.getRevisions().size());
+        assertEquals(component1V1, claim2.getRevisions().iterator().next());
+    }
+
+    @Test(timeout = 5000)
+    public void testWriteLockReleasedWhenClaimCanceledByComponentId() {
+        final RevisionManager revisionManager = new NaiveRevisionManager(2, TimeUnit.SECONDS);
+        final Revision component1V1 = new Revision(1L, CLIENT_1, COMPONENT_1);
+
+        final RevisionClaim claim = revisionManager.requestClaim(component1V1, USER_1);
+        assertNotNull(claim);
+        assertEquals(1, claim.getRevisions().size());
+        assertEquals(component1V1, claim.getRevisions().iterator().next());
+
+        assertTrue(revisionManager.cancelClaim(COMPONENT_1));
+
+        final RevisionClaim claim2 = revisionManager.requestClaim(component1V1, USER_1);
+        assertNotNull(claim2);
+        assertEquals(1, claim2.getRevisions().size());
+        assertEquals(component1V1, claim2.getRevisions().iterator().next());
+    }
+
+    @Test(timeout = 5000)
+    public void testDeleteRevisionUnlocksClaimIfExceptionThrown() {
+        final RevisionManager revisionManager = new NaiveRevisionManager(2, TimeUnit.MINUTES);
+        final Revision component1V1 = new Revision(1L, CLIENT_1, COMPONENT_1);
+
+        final RevisionClaim claim = revisionManager.requestClaim(component1V1, USER_1);
+        assertNotNull(claim);
+        assertEquals(1, claim.getRevisions().size());
+        assertEquals(component1V1, claim.getRevisions().iterator().next());
+
+        final RuntimeException re = new RuntimeException("Intentional Unit Test Exception");
+        try {
+            revisionManager.deleteRevision(claim, USER_1, () -> {
+                throw re;
+            });
+
+            Assert.fail("deleteRevision() method did not propagate Exception thrown");
+        } catch (final RuntimeException e) {
+            assertTrue(re == e);
+        }
+
+        // Ensure that we can obtain a read lock
+        revisionManager.get(COMPONENT_1, rev -> rev);
+
+        final RevisionClaim claim2 = revisionManager.requestClaim(component1V1, USER_1);
+        assertNotNull(claim2);
+        assertEquals(1, claim2.getRevisions().size());
+        assertEquals(component1V1, claim2.getRevisions().iterator().next());
+    }
+
+    @Test(timeout = 5000)
+    public void testUpdateRevisionUnlocksClaimIfExceptionThrown() {
+        final RevisionManager revisionManager = new NaiveRevisionManager(2, TimeUnit.MINUTES);
+        final Revision component1V1 = new Revision(1L, CLIENT_1, COMPONENT_1);
+
+        final RevisionClaim claim = revisionManager.requestClaim(component1V1, USER_1);
+        assertNotNull(claim);
+        assertEquals(1, claim.getRevisions().size());
+        assertEquals(component1V1, claim.getRevisions().iterator().next());
+
+        final RuntimeException re = new RuntimeException("Intentional Unit Test Exception");
+        try {
+            revisionManager.updateRevision(claim, USER_1, () -> {
+                throw re;
+            });
+
+            Assert.fail("updateRevision() method did not propagate Exception thrown");
+        } catch (final RuntimeException e) {
+            assertTrue(re == e);
+        }
+
+        // Ensure that we can obtain a read lock
+        revisionManager.get(COMPONENT_1, rev -> rev);
+
+        final RevisionClaim claim2 = revisionManager.requestClaim(component1V1, USER_1);
+        assertNotNull(claim2);
+        assertEquals(1, claim2.getRevisions().size());
+        assertEquals(component1V1, claim2.getRevisions().iterator().next());
+    }
 }