From a1c8c6703a339b9c7aaffa85b9f62fc03117d80a Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Fri, 27 Apr 2018 09:54:17 -0400 Subject: Modify PolicyGuard to use new locking API Modified PolicyGuard to use the locking API from policy-core instead of its own internal map. Initialized PolicyGuard.factory. Fixed bug in eventmanager junit test uncovered due to changes to PolicyGuard. Change-Id: I853ee5f146f3bde16b3f6e65bc188ca7c0cc4f73 Issue-ID: POLICY-577 Signed-off-by: Jim Hahn --- .../eventmanager/ControlLoopEventManagerTest.java | 2 +- .../java/org/onap/policy/guard/PolicyGuard.java | 187 +++++++++++++++------ .../org/onap/policy/guard/PolicyGuardTest.java | 152 ++++++++++++++--- 3 files changed, 257 insertions(+), 84 deletions(-) (limited to 'controlloop/common') diff --git a/controlloop/common/eventmanager/src/test/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManagerTest.java b/controlloop/common/eventmanager/src/test/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManagerTest.java index 5bd361af3..182791d22 100644 --- a/controlloop/common/eventmanager/src/test/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManagerTest.java +++ b/controlloop/common/eventmanager/src/test/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManagerTest.java @@ -717,7 +717,7 @@ public class ControlLoopEventManagerTest { event.setClosedLoopAlarmStart(Instant.now()); event.setClosedLoopEventStatus(ControlLoopEventStatus.ONSET); event.setAai(new HashMap<>()); - event.getAai().put("generic-vnf.vnf-name", "onsetOne"); + event.getAai().put("generic-vnf.vnf-id", "onsetOne"); ControlLoopEventManager manager = makeManager(event); try { 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 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 { 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 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 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 true if the target is successfully unlocked, false * 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 true if the target is locked, false 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 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 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 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 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 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 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 fut = mock(Future.class); + + mgr = mock(PolicyResourceLockManager.class); + when(mgr.lock(any(), any(), any())).thenReturn(fut); + + LockResult 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); } } -- cgit 1.2.3-korg