Distinguish lock from refresh

This is the first step of separating the lock "refresh" operation
from the original "lock" operation.  This step entails adding the
refresh() method to both the default and the feature-distriubted
locking mechanisms.
Change method call, in junit test, from lock to refresh.
Change branch name in git review.

Change-Id: I506de7a96cb3ee786839aca04ad67cdd7378832c
Issue-ID: POLICY-872
Signed-off-by: Jim Hahn <jrh3@att.com>

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);