Use refresh() instead of lock() in guard

This is the second step of separating the lock "refresh" operation
from the original "lock" operation.
Modified PolicyGuard to use refresh() to extend a lock.  Also removed
the host name from the owner String, as it will no be longer needed
(once the final step is done).

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

diff --git a/controlloop/common/guard/src/main/java/org/onap/policy/guard/PolicyGuard.java b/controlloop/common/guard/src/main/java/org/onap/policy/guard/PolicyGuard.java
index 73baf20..e9ff010 100644 (file)
--- a/controlloop/common/guard/src/main/java/org/onap/policy/guard/PolicyGuard.java
+++ b/controlloop/common/guard/src/main/java/org/onap/policy/guard/PolicyGuard.java
@@ -23,7 +23,6 @@ package org.onap.policy.guard;
 import java.util.UUID;
 import org.onap.policy.controlloop.policy.TargetType;
 import org.onap.policy.drools.core.lock.PolicyResourceLockManager;
-import org.onap.policy.drools.utils.NetworkUtil;
 import org.onap.policy.guard.impl.PNFTargetLock;
 import org.onap.policy.guard.impl.VMTargetLock;
 import org.onap.policy.guard.impl.VNFTargetLock;
@@ -94,10 +93,10 @@ public class PolicyGuard {
             UUID requestID, LockCallback callback, int holdSec) {
         
         String owner = makeOwner(targetType, requestID);
-        
-        GuardResult guardResult = managerLockTarget(targetInstance, owner, holdSec);
-        if(guardResult != GuardResult.LOCK_ACQUIRED) {
-            return LockResult.createLockResult(guardResult, null);
+
+        boolean result = factory.getManager().lock(targetInstance, owner, holdSec);
+        if(!result) {
+            return LockResult.createLockResult(GuardResult.LOCK_DENIED, null);
         }
         
         TargetLock lock = null;
@@ -142,21 +141,10 @@ public class PolicyGuard {
      */
     public static GuardResult lockTarget(TargetLock lock, int holdSec) {
         String owner = makeOwner(lock.getTargetType(), lock.getRequestID());
-        GuardResult result = managerLockTarget(lock.getTargetInstance(), owner, holdSec);
+        
+        boolean result = factory.getManager().refresh(lock.getTargetInstance(), owner, holdSec);
         
         logger.debug("Lock {} extend {}", lock, result);
-        return result;
-    }
-
-    /**
-     * Asks the manager to lock the given target.
-     * @param targetInstance
-     * @param owner
-     * @param holdSec maximum number of seconds to hold the lock
-     * @return the result: acquired or denied
-     */
-    private static GuardResult managerLockTarget(String targetInstance, String owner, int holdSec) {
-        boolean result = factory.getManager().lock(targetInstance, owner, holdSec);
         return(result ? GuardResult.LOCK_ACQUIRED : GuardResult.LOCK_DENIED);
     }
 
@@ -210,7 +198,7 @@ public class PolicyGuard {
             throw new IllegalArgumentException("null requestID for lock type " + targetType);
         }
         
-        return factory.getHostname() + ":" + targetType + ":" + requestID;
+        return targetType + ":" + requestID;
     }
     
     /**
@@ -224,12 +212,5 @@ public class PolicyGuard {
         public PolicyResourceLockManager getManager() {
             return PolicyResourceLockManager.getInstance();
         }
-        
-        /**
-         * @return the current host name
-         */
-        public String getHostname() {
-            return NetworkUtil.getHostname();
-        }
     }
 }

diff --git a/controlloop/common/guard/src/test/java/org/onap/policy/guard/PolicyGuardTest.java b/controlloop/common/guard/src/test/java/org/onap/policy/guard/PolicyGuardTest.java
index e247069..dd1ede4 100644 (file)
--- a/controlloop/common/guard/src/test/java/org/onap/policy/guard/PolicyGuardTest.java
+++ b/controlloop/common/guard/src/test/java/org/onap/policy/guard/PolicyGuardTest.java
@@ -43,7 +43,6 @@ import org.onap.policy.guard.impl.VMTargetLock;
 import org.onap.policy.guard.impl.VNFTargetLock;
 
 public class PolicyGuardTest {
-    private static final String HOSTNAME = "my.host";
     private static final String INSTANCENAME = "targetInstance";
     private static final int LOCK_SEC = 10;
     
@@ -117,7 +116,6 @@ public class PolicyGuardTest {
         
         factory = mock(Factory.class);
         when(factory.getManager()).thenReturn(mgr);
-        when(factory.getHostname()).thenReturn(HOSTNAME);
         
         uuid = UUID.randomUUID();
         dlcb = new DummyLockCallback();
@@ -277,42 +275,48 @@ public class PolicyGuardTest {
     }
 
     @Test
-    public void testManagerLockTarget() throws Exception {
+    public void testLockTargetTargetTypeStringUUIDLockCallbackInt() throws Exception {
         TargetType type = TargetType.VM;
 
         LockResult<GuardResult, TargetLock> result;
 
         // acquired
         result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC);
-        verify(mgr).lock(INSTANCENAME, HOSTNAME+":"+type+":"+uuid, LOCK_SEC);
+        verify(mgr).lock(INSTANCENAME, type+":"+uuid, LOCK_SEC);
         assertEquals(GuardResult.LOCK_ACQUIRED, result.getA());
         assertEquals(VMTargetLock.class, result.getB().getClass());
 
-        // diff host name - denied
-        when(factory.getHostname()).thenReturn(HOSTNAME+"x");
-        result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC+10);
-        verify(mgr).lock(INSTANCENAME, HOSTNAME+"x:"+type+":"+uuid, LOCK_SEC+10);
+        // diff owner - denied
+        UUID uuid2 = UUID.randomUUID();
+        result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid2, dlcb, LOCK_SEC+10);
+        verify(mgr).lock(INSTANCENAME, type+":"+uuid2, LOCK_SEC+10);
         assertEquals(GuardResult.LOCK_DENIED, result.getA());
         assertNull(result.getB());
     }
 
     @Test
-    public void testManagerLockTargetTargetLockInt() throws Exception {
+    public void testLockTargetTargetLockInt() throws Exception {
         TargetType type = TargetType.VM;
-        DummyTargetLock lock = new DummyTargetLock(type, uuid);
+
+        LockResult<GuardResult, TargetLock> result;
 
         // acquired
-        assertEquals(GuardResult.LOCK_ACQUIRED, PolicyGuard.lockTarget(lock, LOCK_SEC));
-        verify(mgr).lock(INSTANCENAME, HOSTNAME+":"+type+":"+uuid, LOCK_SEC);
+        result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC);
+        verify(mgr).lock(INSTANCENAME, type+":"+uuid, LOCK_SEC);
+        assertEquals(GuardResult.LOCK_ACQUIRED, result.getA());
+        assertEquals(VMTargetLock.class, result.getB().getClass());
         
-        // same host name - re-acquired
+        TargetLock lock = result.getB();
+        
+        // refresh - re-acquired
         assertEquals(GuardResult.LOCK_ACQUIRED, PolicyGuard.lockTarget(lock, LOCK_SEC+1));
-        verify(mgr).lock(INSTANCENAME, HOSTNAME+":"+type+":"+uuid, LOCK_SEC+1);
+        verify(mgr).refresh(INSTANCENAME, type+":"+uuid, LOCK_SEC+1);
+        
+        // unlock
+        PolicyGuard.unlockTarget(lock);
         
-        // diff host name - denied
-        when(factory.getHostname()).thenReturn(HOSTNAME+"_");
+        // refresh - denied, as we no longer own the lock
         assertEquals(GuardResult.LOCK_DENIED, PolicyGuard.lockTarget(lock, LOCK_SEC+2));
-        verify(mgr).lock(INSTANCENAME, HOSTNAME+"_:"+type+":"+uuid, LOCK_SEC+2);
     }
 
     @Test(expected = IllegalArgumentException.class)