diff options
9 files changed, 375 insertions, 2 deletions
diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java index 88035ca7..f1c8b687 100644 --- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java +++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java @@ -73,6 +73,14 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy return(tLock.lock(holdSec) ? OperResult.OPER_ACCEPTED : OperResult.OPER_DENIED); } + + @Override + public OperResult beforeRefresh(String resourceId, String owner, int holdSec) { + + TargetLock tLock = new TargetLock(resourceId, uuid, owner, dataSource); + + return(tLock.refresh(holdSec) ? OperResult.OPER_ACCEPTED : OperResult.OPER_DENIED); + } @Override public OperResult beforeUnlock(String resourceId, String owner) { diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java index fe6f2fe0..1db34538 100644 --- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java +++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java @@ -73,6 +73,17 @@ public class TargetLock { return grabLock(holdSec); } + + /** + * refresh a lock + * + * @param holdSec the amount of time, in seconds, that the lock should be held + * @return {@code true} if the lock was refreshed, {@code false} if the resource is + * not currently locked by the given owner + */ + public boolean refresh(int holdSec) { + return updateLock(holdSec); + } /** * Unlock a resource by deleting it's associated record in the db @@ -158,6 +169,36 @@ public class TargetLock { } } + + /** + * Updates the DB record associated with the lock. + * + * @param holdSec the amount of time, in seconds, that the lock should be held + * @return {@code true} if the record was updated, {@code false} otherwise + */ + private boolean updateLock(int holdSec) { + + try (Connection conn = dataSource.getConnection(); + + PreparedStatement updateStatement = conn.prepareStatement( + "UPDATE pooling.locks SET host = ?, owner = ?, expirationTime = timestampadd(second, ?, now()) WHERE resourceId = ? AND owner = ? AND expirationTime >= now()")) { + + int i = 1; + updateStatement.setString(i++, this.uuid.toString()); + updateStatement.setString(i++, this.owner); + updateStatement.setInt(i++, holdSec); + updateStatement.setString(i++, this.resourceId); + updateStatement.setString(i++, this.owner); + + // refresh succeeded iff a record was updated + return (updateStatement.executeUpdate() == 1); + + } catch (SQLException e) { + logger.error("error in TargetLock.refreshLock()", e); + return false; + } + + } /** *To remove a lock we simply delete the record from the db diff --git a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java index c1b46d67..6e33f224 100644 --- a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java +++ b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java @@ -119,6 +119,32 @@ public class TargetLockTest { assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeLock("resource1", "owner2", MAX_AGE_SEC)); } + + @Test + public void testUpdateLock() throws InterruptedException, ExecutionException { + // not locked yet - refresh should fail + assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeRefresh("resource1", "owner1", MAX_AGE_SEC)); + + // now lock it + assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC)); + + // refresh should work now + assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeRefresh("resource1", "owner1", MAX_AGE_SEC)); + + // expire the lock + try (PreparedStatement updateStatement = conn.prepareStatement("UPDATE pooling.locks SET expirationTime = timestampadd(second, -1, now()) WHERE resourceId = ?");) + { + updateStatement.setString(1, "resource1"); + updateStatement.executeUpdate(); + + } catch (SQLException e) { + logger.error("Error in TargetLockTest.testGrabLockSuccess()", e); + throw new RuntimeException(e); + } + + // refresh should fail now + assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeRefresh("resource1", "owner1", MAX_AGE_SEC)); + } @Test diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java index 683dd83d..ba6fe856 100644 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java +++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java @@ -98,6 +98,34 @@ public interface PolicyResourceLockFeatureAPI extends OrderedService { } /** + * This method is called before a lock is refreshed on a resource. It may be invoked + * repeatedly to extend the time that a lock is held. + * + * @param resourceId + * @param owner + * @param holdSec the amount of time, in seconds, that the lock should be held + * @return the result, where <b>OPER_DENIED</b> indicates that the resource is not + * currently locked by the given owner + */ + public default OperResult beforeRefresh(String resourceId, String owner, int holdSec) { + return OperResult.OPER_UNHANDLED; + } + + /** + * This method is called after a lock for a resource has been refreshed (or after the + * refresh has been denied). + * + * @param resourceId + * @param owner + * @param locked {@code true} if the lock was acquired, {@code false} if it was denied + * @return {@code true} if the implementer handled the request, {@code false} + * otherwise + */ + public default boolean afterRefresh(String resourceId, String owner, boolean locked) { + return false; + } + + /** * This method is called before a lock on a resource is released. * * @param resourceId diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java index 1a94a531..8e13ced4 100644 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java +++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java @@ -90,6 +90,28 @@ public class PolicyResourceLockManager extends SimpleLockManager { } @Override + public boolean refresh(String resourceId, String owner, int holdSec) { + if (resourceId == null) { + throw makeNullArgException(MSG_NULL_RESOURCE_ID); + } + + if (owner == null) { + throw makeNullArgException(MSG_NULL_OWNER); + } + + + return doBoolIntercept(impl -> impl.beforeRefresh(resourceId, owner, holdSec), () -> { + + // implementer didn't do the work - defer to the superclass + boolean refreshed = super.refresh(resourceId, owner, holdSec); + + doIntercept(false, impl -> impl.afterRefresh(resourceId, owner, refreshed)); + + return refreshed; + }); + } + + @Override public boolean unlock(String resourceId, String owner) { if (resourceId == null) { throw makeNullArgException(MSG_NULL_RESOURCE_ID); diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java index 2a44ddcd..081ad4c1 100644 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java +++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java @@ -118,6 +118,49 @@ public class SimpleLockManager { } /** + * Attempts to refresh a lock on a resource. + * + * @param resourceId + * @param owner + * @param holdSec the amount of time, in seconds, that the lock should be held + * @return {@code true} if locked, {@code false} if the resource is not currently + * locked by the given owner + * @throws IllegalArgumentException if the resourceId or owner is {@code null} + */ + public boolean refresh(String resourceId, String owner, int holdSec) { + + if (resourceId == null) { + throw makeNullArgException(MSG_NULL_RESOURCE_ID); + } + + if (owner == null) { + throw makeNullArgException(MSG_NULL_OWNER); + } + + boolean refreshed = false; + + synchronized(locker) { + cleanUpLocks(); + + Data existingLock = resource2data.get(resourceId); + if (existingLock != null && existingLock.getOwner().equals(owner)) { + // MUST remove the existing lock from the set + locks.remove(existingLock); + + refreshed = true; + + Data data = new Data(owner, resourceId, currentTime.getMillis() + TimeUnit.SECONDS.toMillis(holdSec)); + resource2data.put(resourceId, data); + locks.add(data); + } + } + + logger.info("refresh lock {} for resource {} owner {}", refreshed, resourceId, owner); + + return refreshed; + } + + /** * Unlocks a resource. * * @param resourceId diff --git a/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPITest.java b/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPITest.java index 0404877b..0d7d9437 100644 --- a/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPITest.java +++ b/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPITest.java @@ -55,6 +55,17 @@ public class PolicyResourceLockFeatureAPITest { } @Test + public void testBeforeRefresh() { + assertEquals(OperResult.OPER_UNHANDLED, api.beforeRefresh(RESOURCE_ID, OWNER, 0)); + } + + @Test + public void testAfterRefresh() { + assertFalse(api.afterRefresh(RESOURCE_ID, OWNER, true)); + assertFalse(api.afterRefresh(RESOURCE_ID, OWNER, false)); + } + + @Test public void testBeforeUnlock() { assertEquals(OperResult.OPER_UNHANDLED, api.beforeUnlock(RESOURCE_ID, OWNER)); } diff --git a/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockManagerTest.java b/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockManagerTest.java index 92e60268..5cbed304 100644 --- a/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockManagerTest.java +++ b/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockManagerTest.java @@ -108,6 +108,7 @@ public class PolicyResourceLockManagerTest { */ private void initImplementer(PolicyResourceLockFeatureAPI impl) { when(impl.beforeLock(anyString(), anyString(), anyInt())).thenReturn(OperResult.OPER_UNHANDLED); + when(impl.beforeRefresh(anyString(), anyString(), anyInt())).thenReturn(OperResult.OPER_UNHANDLED); when(impl.beforeUnlock(anyString(), anyString())).thenReturn(OperResult.OPER_UNHANDLED); when(impl.beforeIsLocked(anyString())).thenReturn(OperResult.OPER_UNHANDLED); when(impl.beforeIsLockedBy(anyString(), anyString())).thenReturn(OperResult.OPER_UNHANDLED); @@ -225,6 +226,125 @@ public class PolicyResourceLockManagerTest { } @Test + public void testRefresh() throws Exception { + assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + assertTrue(mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + verify(impl1).beforeRefresh(RESOURCE_A, OWNER1, MAX_AGE_SEC); + verify(impl2).beforeRefresh(RESOURCE_A, OWNER1, MAX_AGE_SEC); + verify(impl1).afterRefresh(RESOURCE_A, OWNER1, true); + verify(impl2).afterRefresh(RESOURCE_A, OWNER1, true); + + assertTrue(mgr.isLocked(RESOURCE_A)); + assertTrue(mgr.isLockedBy(RESOURCE_A, OWNER1)); + assertFalse(mgr.isLocked(RESOURCE_B)); + assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER2)); + + // different owner and resource + assertFalse(mgr.refresh(RESOURCE_C, OWNER3, MAX_AGE_SEC)); + + // different owner + assertFalse(mgr.refresh(RESOURCE_A, OWNER3, MAX_AGE_SEC)); + } + + @Test + public void testRefresh_ArgEx() { + IllegalArgumentException ex = + expectException(IllegalArgumentException.class, () -> mgr.refresh(null, OWNER1, MAX_AGE_SEC)); + assertEquals(NULL_RESOURCE_ID, ex.getMessage()); + + ex = expectException(IllegalArgumentException.class, () -> mgr.refresh(RESOURCE_A, null, MAX_AGE_SEC)); + assertEquals(NULL_OWNER, ex.getMessage()); + + // this should not throw an exception + mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC); + } + + @Test + public void testRefresh_Acquired_BeforeIntercepted() { + assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + // have impl1 intercept + when(impl1.beforeRefresh(RESOURCE_A, OWNER1, MAX_AGE_SEC)).thenReturn(OperResult.OPER_ACCEPTED); + + assertTrue(mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + verify(impl1).beforeRefresh(RESOURCE_A, OWNER1, MAX_AGE_SEC); + verify(impl2, never()).beforeRefresh(anyString(), anyString(), anyInt()); + + verify(impl1, never()).afterRefresh(anyString(), anyString(), anyBoolean()); + verify(impl2, never()).afterRefresh(anyString(), anyString(), anyBoolean()); + } + + @Test + public void testRefresh_Denied_BeforeIntercepted() { + assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + // have impl1 intercept + when(impl1.beforeRefresh(RESOURCE_A, OWNER1, MAX_AGE_SEC)).thenReturn(OperResult.OPER_DENIED); + + assertFalse(mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + verify(impl1).beforeRefresh(RESOURCE_A, OWNER1, MAX_AGE_SEC); + verify(impl2, never()).beforeRefresh(anyString(), anyString(), anyInt()); + + verify(impl1, never()).afterRefresh(anyString(), anyString(), anyBoolean()); + verify(impl2, never()).afterRefresh(anyString(), anyString(), anyBoolean()); + } + + @Test + public void testRefresh_Acquired_AfterIntercepted() throws Exception { + assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + // impl1 intercepts during afterRefresh() + when(impl1.afterRefresh(RESOURCE_A, OWNER1, true)).thenReturn(true); + + assertTrue(mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + // impl1 sees it, but impl2 does not + verify(impl1).afterRefresh(RESOURCE_A, OWNER1, true); + verify(impl2, never()).afterRefresh(anyString(), anyString(), anyBoolean()); + } + + @Test + public void testRefresh_Acquired() throws Exception { + assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + assertTrue(mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + verify(impl1).afterRefresh(RESOURCE_A, OWNER1, true); + verify(impl2).afterRefresh(RESOURCE_A, OWNER1, true); + } + + @Test + public void testRefresh_Denied_AfterIntercepted() throws Exception { + + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); + + // impl1 intercepts during afterRefresh() + when(impl1.afterRefresh(RESOURCE_A, OWNER2, false)).thenReturn(true); + + // owner2 tries to lock + assertFalse(mgr.refresh(RESOURCE_A, OWNER2, MAX_AGE_SEC)); + + // impl1 sees it, but impl2 does not + verify(impl1).afterRefresh(RESOURCE_A, OWNER2, false); + verify(impl2, never()).afterRefresh(RESOURCE_A, OWNER2, false); + } + + @Test + public void testRefresh_Denied() { + + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); + + // owner2 tries to lock + mgr.refresh(RESOURCE_A, OWNER2, MAX_AGE_SEC); + + verify(impl1).afterRefresh(RESOURCE_A, OWNER2, false); + verify(impl2).afterRefresh(RESOURCE_A, OWNER2, false); + } + + @Test public void testUnlock() throws Exception { mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); mgr.lock(RESOURCE_B, OWNER1, MAX_AGE_SEC); diff --git a/policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java b/policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java index 14964e0e..df6fb10a 100644 --- a/policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java +++ b/policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java @@ -100,10 +100,10 @@ public class SimpleLockManagerTest { assertFalse(mgr.isLocked(RESOURCE_B)); assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER2)); - // null callback - not locked yet + // different owner and resource - should succeed assertTrue(mgr.lock(RESOURCE_C, OWNER3, MAX_AGE_SEC)); - // null callback - already locked + // different owner - already locked assertFalse(mgr.lock(RESOURCE_A, OWNER3, MAX_AGE_SEC)); } @@ -152,6 +152,80 @@ public class SimpleLockManagerTest { } @Test + public void testRefresh() throws Exception { + // don't own the lock yet - cannot refresh + assertFalse(mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + // now the lock is owned + assertTrue(mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + // refresh again + assertTrue(mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC + 1)); + + assertTrue(mgr.isLocked(RESOURCE_A)); + assertTrue(mgr.isLockedBy(RESOURCE_A, OWNER1)); + assertFalse(mgr.isLocked(RESOURCE_B)); + assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER2)); + + // different owner + assertFalse(mgr.refresh(RESOURCE_A, OWNER3, MAX_AGE_SEC)); + + // different resource + assertFalse(mgr.refresh(RESOURCE_C, OWNER1, MAX_AGE_SEC)); + } + + @Test + public void testRefresh_ExtendLock() throws Exception { + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); + + // sleep half of the cycle + testTime.sleep(MAX_AGE_MS / 2); + assertTrue(mgr.isLockedBy(RESOURCE_A, OWNER1)); + + // extend the lock + mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC); + + // verify still locked after sleeping the other half of the cycle + testTime.sleep(MAX_AGE_MS / 2 + 1); + assertTrue(mgr.isLockedBy(RESOURCE_A, OWNER1)); + + // and should release after another half cycle + testTime.sleep(MAX_AGE_MS / 2); + + // cannot refresh expired lock + assertFalse(mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER1)); + } + + @Test + public void testRefresh_AlreadyLocked() throws Exception { + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); + + // same owner + assertTrue(mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + + // different owner + assertFalse(mgr.refresh(RESOURCE_A, OWNER2, MAX_AGE_SEC)); + assertFalse(mgr.lock(RESOURCE_A, OWNER2, MAX_AGE_SEC)); + } + + @Test + public void testRefresh_ArgEx() { + IllegalArgumentException ex = + expectException(IllegalArgumentException.class, () -> mgr.refresh(null, OWNER1, MAX_AGE_SEC)); + assertEquals(NULL_RESOURCE_ID, ex.getMessage()); + + ex = expectException(IllegalArgumentException.class, () -> mgr.refresh(RESOURCE_A, null, MAX_AGE_SEC)); + assertEquals(NULL_OWNER, ex.getMessage()); + + // this should not throw an exception + mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC); + } + + @Test public void testUnlock() throws Exception { mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); |