From 18133988a02d80180ee55b91bc5b80807ec9f3de Mon Sep 17 00:00:00 2001 From: Mark Payne Date: Wed, 8 Jun 2016 08:57:37 -0400 Subject: [PATCH] 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 --- .../web/revision/NaiveRevisionManager.java | 16 +++- .../revision/TestNaiveRevisionManager.java | 96 +++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/main/java/org/apache/nifi/web/revision/NaiveRevisionManager.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/main/java/org/apache/nifi/web/revision/NaiveRevisionManager.java index c00661d864..7bcf9f4329 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/main/java/org/apache/nifi/web/revision/NaiveRevisionManager.java +++ b/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); diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/test/java/org/apache/nifi/web/revision/TestNaiveRevisionManager.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/test/java/org/apache/nifi/web/revision/TestNaiveRevisionManager.java index 303b450f3b..e3148d3e64 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/test/java/org/apache/nifi/web/revision/TestNaiveRevisionManager.java +++ b/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()); + } } \ No newline at end of file