Merge "Modify PolicyGuard to use new locking API"

2 files changed, 256 insertions, 83 deletions

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 47faa88c2..66a25ea63 100644
--- 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
@@ -20,11 +20,11 @@
 
 package org.onap.policy.guard;
 
-import java.util.HashMap;
-import java.util.Map;
 import java.util.UUID;
-
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import org.onap.policy.controlloop.policy.TargetType;
+import org.onap.policy.drools.core.lock.PolicyResourceLockManager;
 import org.onap.policy.guard.impl.PNFTargetLock;
 import org.onap.policy.guard.impl.VMTargetLock;
 import org.onap.policy.guard.impl.VNFTargetLock;
@@ -36,8 +36,12 @@ public class PolicyGuard {
         // Cannot instantiate this static class
     }
 
-    private static Map<String, TargetLock> activeLocks = new HashMap<>();
     private static final Logger logger = LoggerFactory.getLogger(PolicyGuard.class);
+    
+    /**
+     * Factory to access various objects.  Can be changed for junit tests.
+     */
+    private static Factory factory = new Factory();
 
     public static class LockResult<A, B> {
         private A parameterA;
@@ -60,6 +64,21 @@ public class PolicyGuard {
             return parameterB;
         }
     }
+    
+    /**
+     * @return the factory used to access various objects
+     */
+    protected static Factory getFactory() {
+        return factory;
+    }
+    
+    /**
+     * Sets the factory to be used by junit tests.
+     * @param factory
+     */
+    protected static void setFactory(Factory factory) {
+        PolicyGuard.factory = factory;
+    }
 
     /**
      * Lock a target.
@@ -69,50 +88,75 @@ public class PolicyGuard {
      * @param requestID the request Id
      * @param callback the LockCallback
      * @return the LockResult
+     * @throws IllegalArgumentException if an argument is null
      */
     public static LockResult<GuardResult, TargetLock> lockTarget(TargetType targetType, String targetInstance,
             UUID requestID, LockCallback callback) {
+        
+        String owner = makeOwner(targetType, requestID);
+        
+        GuardResult guardResult = managerLockTarget(targetInstance, owner);
+        if(guardResult != GuardResult.LOCK_ACQUIRED) {
+            return LockResult.createLockResult(guardResult, null);
+        }
+        
+        TargetLock lock = null;
+        switch (targetType) {
+            case PNF:
+                //
+                // Create the Lock object
+                //
+                lock = new PNFTargetLock(targetType, targetInstance, requestID, callback);
+                break;
+            case VM:
+                //
+                // Create the Lock object
+                //
+                lock = new VMTargetLock(targetType, targetInstance, requestID, callback);
+                break;
+            case VNF:
+                //
+                // Create the Lock object
+                //
+                lock = new VNFTargetLock(targetType, targetInstance, requestID, callback);
+                break;
+
+            default:
+                logger.error("invalid target type {} for lock on {}", targetType, targetInstance);
+                factory.getManager().unlock(targetInstance, owner);
+                return LockResult.createLockResult(GuardResult.LOCK_EXCEPTION, null);
+        }
+        
+        //
+        // Return result
+        //
+        logger.debug("Locked {}", lock);
+        return LockResult.createLockResult(GuardResult.LOCK_ACQUIRED, lock);
+    }
 
-        synchronized (activeLocks) {
-            //
-            // Is there a lock on this instance already?
-            //
-            if (activeLocks.containsKey(targetInstance)) {
-                return LockResult.createLockResult(GuardResult.LOCK_DENIED, null);
-            }
-            TargetLock lock = null;
-            switch (targetType) {
-                case PNF:
-                    //
-                    // Create the Lock object
-                    //
-                    lock = new PNFTargetLock(targetType, targetInstance, requestID, callback);
-                    break;
-                case VM:
-                    //
-                    // Create the Lock object
-                    //
-                    lock = new VMTargetLock(targetType, targetInstance, requestID, callback);
-                    break;
-                case VNF:
-                    //
-                    // Create the Lock object
-                    //
-                    lock = new VNFTargetLock(targetType, targetInstance, requestID, callback);
-                    break;
-
-                default:
-                    return LockResult.createLockResult(GuardResult.LOCK_EXCEPTION, null);
-            }
-            //
-            // Keep track of it
-            //
-            activeLocks.put(targetInstance, lock);
-            //
-            // Return result
-            //
-            logger.debug("Locking {}", lock);
-            return LockResult.createLockResult(GuardResult.LOCK_ACQUIRED, lock);
+    /**
+     * Asks the manager to lock the given target.
+     * @param targetInstance
+     * @param owner
+     * @return the result: acquired, denied, or exception
+     */
+    private static GuardResult managerLockTarget(String targetInstance, String owner) {
+        try {
+            Future<Boolean> result = factory.getManager().lock(targetInstance, owner, null);
+            return(result.get() ? GuardResult.LOCK_ACQUIRED : GuardResult.LOCK_DENIED);
+            
+        } catch(IllegalStateException e) {
+            logger.warn("{} attempted to re-lock {}", owner, targetInstance);
+            return GuardResult.LOCK_DENIED;
+            
+        } catch (InterruptedException e) {
+            logger.error("exception getting lock for {}", targetInstance, e);
+            Thread.currentThread().interrupt();
+            return GuardResult.LOCK_EXCEPTION;
+            
+        } catch (ExecutionException e) {
+            logger.error("exception getting lock for {}", targetInstance, e);
+            return GuardResult.LOCK_EXCEPTION;
         }
     }
 
@@ -122,15 +166,18 @@ public class PolicyGuard {
      * @param lock the target lock to unlock
      * @return <code>true</code> if the target is successfully unlocked, <code>false</code>
      *         otherwise
+     * @throws IllegalArgumentException if an argument is null
      */
     public static boolean unlockTarget(TargetLock lock) {
-        synchronized (activeLocks) {
-            if (activeLocks.containsKey(lock.getTargetInstance())) {
-                logger.debug("Unlocking {}", lock);
-                return (activeLocks.remove(lock.getTargetInstance()) != null);
-            }
-            return false;
+        String owner = makeOwner(lock.getTargetType(), lock.getRequestID());
+        boolean result = factory.getManager().unlock(lock.getTargetInstance(), owner);
+        
+        if(result) {
+            logger.debug("Unlocked {}", lock);
+            return true;            
         }
+        
+        return false;
     }
 
     /**
@@ -140,14 +187,42 @@ public class PolicyGuard {
      * @param targetInstance the target instance
      * @param requestID the request Id
      * @return <code>true</code> if the target is locked, <code>false</code> otherwise
+     * @throws IllegalArgumentException if an argument is null
      */
     public static boolean isLocked(TargetType targetType, String targetInstance, UUID requestID) {
-        synchronized (activeLocks) {
-            if (activeLocks.containsKey(targetInstance)) {
-                TargetLock lock = activeLocks.get(targetInstance);
-                return (lock.getTargetType().equals(targetType) && lock.getRequestID().equals(requestID));
-            }
-            return false;
+        String owner = makeOwner(targetType, requestID);
+        return factory.getManager().isLockedBy(targetInstance, owner);
+    }
+
+    /**
+     * Combines the target type and request ID to yield a single, "owner" string.
+     * @param targetType
+     * @param requestID
+     * @return the "owner" of a resource
+     * @throws IllegalArgumentException if either argument is null
+     */
+    private static String makeOwner(TargetType targetType, UUID requestID) {
+        if(targetType == null) {
+            throw new IllegalArgumentException("null targetType for lock request id " + requestID);
+        }
+
+        if(requestID == null) {
+            throw new IllegalArgumentException("null requestID for lock type " + targetType);
+        }
+        
+        return targetType.toString() + ":" + requestID.toString();
+    }
+    
+    /**
+     * Factory to access various objects.
+     */
+    public static class Factory {
+
+        /**
+         * @return the lock manager to be used
+         */
+        public PolicyResourceLockManager getManager() {
+            return PolicyResourceLockManager.getInstance();
         }
     }
 }
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 9c85845b5..b073fe5a9 100644
--- 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
@@ -25,11 +25,20 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
-
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 import java.util.UUID;
-
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.onap.policy.controlloop.policy.TargetType;
+import org.onap.policy.drools.core.lock.PolicyResourceLockManager;
+import org.onap.policy.guard.PolicyGuard.Factory;
 import org.onap.policy.guard.PolicyGuard.LockResult;
 import org.onap.policy.guard.impl.PNFTargetLock;
 import org.onap.policy.guard.impl.VMTargetLock;
@@ -37,6 +46,13 @@ import org.onap.policy.guard.impl.VNFTargetLock;
 
 public class PolicyGuardTest {
     private static final String INSTANCENAME = "targetInstance";
+    
+    private static Factory saveFactory;
+    
+    private Factory factory;
+    private PolicyResourceLockManager mgr;
+    private UUID uuid;
+    private DummyLockCallback dlcb;
 
     private class DummyLockCallback implements LockCallback {
         @Override
@@ -51,6 +67,14 @@ public class PolicyGuardTest {
     }
 
     private class DummyTargetLock implements TargetLock {
+        private TargetType type;
+        private UUID reqid;
+        
+        public DummyTargetLock(TargetType type, UUID reqid) {
+            this.type = type;
+            this.reqid = reqid;
+        }
+        
         @Override
         public UUID getLockID() {
             return null;
@@ -58,7 +82,7 @@ public class PolicyGuardTest {
 
         @Override
         public TargetType getTargetType() {
-            return null;
+            return type;
         }
 
         @Override
@@ -68,18 +92,45 @@ public class PolicyGuardTest {
 
         @Override
         public UUID getRequestID() {
-            return null;
+            return reqid;
         }
     }
+    
+    @BeforeClass
+    public static void setUpBeforeClass() {
+        saveFactory = PolicyGuard.getFactory();
+    }
+    
+    @AfterClass
+    public static void tearDownAfterClass() {
+        PolicyGuard.setFactory(saveFactory);
+    }
+    
+    @Before
+    public void setUp() {
+        mgr = new PolicyResourceLockManager() {
+            // only way to access the constructor            
+        };
+        
+        factory = new Factory() {
+            @Override
+            public PolicyResourceLockManager getManager() {
+                return mgr;
+            }
+        };
+        
+        uuid = UUID.randomUUID();
+        dlcb = new DummyLockCallback();
+
+        PolicyGuard.setFactory(factory);
+    }
 
     @Test
     public void testLockVm() {
-        UUID uuid = UUID.randomUUID();
         TargetType type = TargetType.VM;
 
         // Test isLocked before and after lock added
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
-        DummyLockCallback dlcb = new DummyLockCallback();
         LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb);
         assertTrue(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
 
@@ -94,18 +145,16 @@ public class PolicyGuardTest {
         assertEquals(dlcb, vtl.getCallback());
 
         // Test isLocked after lock removed
-        PolicyGuard.unlockTarget(new DummyTargetLock());
+        PolicyGuard.unlockTarget(new DummyTargetLock(type, uuid));
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
     }
 
     @Test
     public void testLockPnf() {
-        UUID uuid = UUID.randomUUID();
         TargetType type = TargetType.PNF;
 
         // Test isLocked before and after lock added
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
-        DummyLockCallback dlcb = new DummyLockCallback();
         LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb);
         assertTrue(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
 
@@ -120,19 +169,17 @@ public class PolicyGuardTest {
         assertEquals(dlcb, ptl.getCallback());
 
         // Test isLocked after lock removed
-        PolicyGuard.unlockTarget(new DummyTargetLock());
+        PolicyGuard.unlockTarget(new DummyTargetLock(type, uuid));
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
     }
 
 
     @Test
     public void testLockVnf() {
-        UUID uuid = UUID.randomUUID();
         TargetType type = TargetType.VNF;
 
         // Test isLocked before and after lock added
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
-        DummyLockCallback dlcb = new DummyLockCallback();
         LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb);
         assertTrue(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
 
@@ -147,18 +194,16 @@ public class PolicyGuardTest {
         assertEquals(dlcb, vtl.getCallback());
 
         // Test isLocked after lock removed
-        PolicyGuard.unlockTarget(new DummyTargetLock());
+        PolicyGuard.unlockTarget(new DummyTargetLock(type, uuid));
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
     }
 
     @Test
     public void testLockVfc() {
-        UUID uuid = UUID.randomUUID();
         TargetType type = TargetType.VFC;
 
         // Test isLocked before and after lock added
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
-        DummyLockCallback dlcb = new DummyLockCallback();
         LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb);
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
 
@@ -166,18 +211,16 @@ public class PolicyGuardTest {
         assertNull(result.getB());
 
         // Test isLocked after lock removed
-        PolicyGuard.unlockTarget(new DummyTargetLock());
+        PolicyGuard.unlockTarget(new DummyTargetLock(type, uuid));
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
     }
 
     @Test
     public void testUnLockNotLocked() {
-        UUID uuid = UUID.randomUUID();
         TargetType type = TargetType.VM;
 
         // Test isLocked before and after lock added
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
-        DummyLockCallback dlcb = new DummyLockCallback();
         LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb);
         assertTrue(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
 
@@ -191,22 +234,20 @@ public class PolicyGuardTest {
         assertNull(result.getB());
 
         // Test isLocked after lock removed
-        PolicyGuard.unlockTarget(new DummyTargetLock());
+        PolicyGuard.unlockTarget(new DummyTargetLock(type, uuid));
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
 
         // Test unlock after lock removed
-        PolicyGuard.unlockTarget(new DummyTargetLock());
+        PolicyGuard.unlockTarget(new DummyTargetLock(type, uuid));
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
     }
 
     @Test
     public void testLockAlreadyLocked() {
-        UUID uuid = UUID.randomUUID();
         TargetType type = TargetType.VM;
 
         // Test isLocked before and after lock added
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
-        DummyLockCallback dlcb = new DummyLockCallback();
         LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb);
         assertTrue(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
 
@@ -220,21 +261,78 @@ public class PolicyGuardTest {
         assertNull(result.getB());
 
         // Test isLocked after lock removed
-        PolicyGuard.unlockTarget(new DummyTargetLock());
+        PolicyGuard.unlockTarget(new DummyTargetLock(type, uuid));
         assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid));
     }
 
     @Test
     public void testInnards() {
+        TargetType type = TargetType.VM;
 
-        DummyLockCallback dlcb = new DummyLockCallback();
         assertFalse(dlcb.isActive());
         assertFalse(dlcb.releaseLock());
 
-        DummyTargetLock dtl = new DummyTargetLock();
+        DummyTargetLock dtl = new DummyTargetLock(type, uuid);
         assertNull(dtl.getLockID());
-        assertNull(dtl.getRequestID());
+        assertEquals(uuid, dtl.getRequestID());
         assertEquals(INSTANCENAME, dtl.getTargetInstance());
-        assertNull(dtl.getTargetType());
+        assertEquals(type, dtl.getTargetType());
+    }
+
+    @Test
+    public void testManagerLockTarget() throws Exception {
+        TargetType type = TargetType.VM;
+        
+        @SuppressWarnings("unchecked")
+        Future<Boolean> fut = mock(Future.class);
+        
+        mgr = mock(PolicyResourceLockManager.class);
+        when(mgr.lock(any(), any(), any())).thenReturn(fut);
+
+        LockResult<GuardResult, TargetLock> result;
+
+        // acquired
+        when(fut.get()).thenReturn(true);
+        result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb);
+        assertEquals(GuardResult.LOCK_ACQUIRED, result.getA());
+        assertEquals(VMTargetLock.class, result.getB().getClass());
+
+        // denied
+        when(fut.get()).thenReturn(false);
+        result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb);
+        assertEquals(GuardResult.LOCK_DENIED, result.getA());
+        assertNull(result.getB());
+
+        // illegal state exception
+        doThrow(new IllegalStateException("state expected")).when(fut).get();
+        result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb);
+        assertEquals(GuardResult.LOCK_DENIED, result.getA());
+        assertNull(result.getB());
+        assertFalse(Thread.currentThread().isInterrupted());
+
+        // interrupted exception
+        doThrow(new InterruptedException("interrupt expected")).when(fut).get();
+        result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb);
+        assertEquals(GuardResult.LOCK_EXCEPTION, result.getA());
+        assertNull(result.getB());
+        // verify interrupted & reset interrupt status
+        assertTrue(Thread.interrupted());
+
+        // exec exception
+        doThrow(new ExecutionException(new IllegalArgumentException("exec expected"))).when(fut).get();
+        result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb);
+        assertEquals(GuardResult.LOCK_EXCEPTION, result.getA());
+        assertNull(result.getB());
+        assertFalse(Thread.currentThread().isInterrupted());
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testMakeOwner_NullTargetType() {
+        PolicyGuard.isLocked(null, INSTANCENAME, uuid);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testMakeOwner_NullReqId() {
+        PolicyGuard.isLocked(TargetType.PNF, INSTANCENAME, null);
     }
 }