From ee2313141a40222a83d6bf39a15c3f9d4c3a239e Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Wed, 27 Jun 2018 10:12:45 -0400 Subject: Add time limit to locking facility Modified the locking facility to add a time limit and remove the callback parameter. This affected both the default facility as well as the distributed locking feature. It will also require a change to the rules for Closed Loop. Changed testUnlock() to try locking with a different owner. Default feature API should be OPER_UNHANDLED. Put a few things back so-as not to break the drools-applications build. They can be removed once drools-applications is updated. Fix newlines in API java. Change-Id: I3ed7835cac6a582493a9bc8f6d1d4f3e6cb6289e Issue-ID: POLICY-872 Signed-off-by: Jim Hahn --- .../drools/core/lock/LockRequestFutureTest.java | 395 --------------------- .../lock/PolicyResourceLockFeatureAPITest.java | 3 +- .../core/lock/PolicyResourceLockManagerTest.java | 105 +++--- .../drools/core/lock/SimpleLockManagerTest.java | 108 +++--- 4 files changed, 108 insertions(+), 503 deletions(-) delete mode 100644 policy-core/src/test/java/org/onap/policy/drools/core/lock/LockRequestFutureTest.java (limited to 'policy-core/src/test') diff --git a/policy-core/src/test/java/org/onap/policy/drools/core/lock/LockRequestFutureTest.java b/policy-core/src/test/java/org/onap/policy/drools/core/lock/LockRequestFutureTest.java deleted file mode 100644 index 7ac9e31c..00000000 --- a/policy-core/src/test/java/org/onap/policy/drools/core/lock/LockRequestFutureTest.java +++ /dev/null @@ -1,395 +0,0 @@ -/* - * ============LICENSE_START======================================================= - * ONAP - * ================================================================================ - * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved. - * ================================================================================ - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * ============LICENSE_END========================================================= - */ - -package org.onap.policy.drools.core.lock; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.anyBoolean; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.onap.policy.drools.core.lock.TestUtils.expectException; -import java.util.concurrent.CancellationException; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; -import org.junit.Before; -import org.junit.Test; -import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.Callback; - -public class LockRequestFutureTest { - - private static final int WAIT_SEC = 1; - private static final String RESOURCE = "my.resource"; - private static final String OWNER = "my.owner"; - private static final String EXPECTED_EXCEPTION = "expected exception"; - - private Callback callback; - private LockRequestFuture fut; - - @Before - public void setUp() { - callback = mock(Callback.class); - fut = new LockRequestFuture(RESOURCE, OWNER, callback); - } - - @Test - public void testLockRequestFutureStringStringBoolean_False() throws Exception { - fut = new LockRequestFuture(RESOURCE, OWNER, false); - - assertTrue(fut.isDone()); - assertEquals(RESOURCE, fut.getResourceId()); - assertEquals(OWNER, fut.getOwner()); - - assertFalse(fut.isLocked()); - assertFalse(fut.get(0, TimeUnit.SECONDS)); - } - - @Test - public void testLockRequestFutureStringStringBoolean_True() throws Exception { - fut = new LockRequestFuture(RESOURCE, OWNER, true); - - assertTrue(fut.isDone()); - assertEquals(RESOURCE, fut.getResourceId()); - assertEquals(OWNER, fut.getOwner()); - - assertTrue(fut.isLocked()); - assertTrue(fut.get(0, TimeUnit.SECONDS)); - } - - @Test - public void testLockRequestFutureStringStringBoolean_ArgEx() throws Exception { - - // null resource id - IllegalArgumentException ex = - expectException(IllegalArgumentException.class, () -> new LockRequestFuture(null, OWNER, true)); - assertEquals("null resourceId", ex.getMessage()); - - - // null owner - ex = expectException(IllegalArgumentException.class, () -> new LockRequestFuture(RESOURCE, null, true)); - assertEquals("null owner", ex.getMessage()); - } - - @Test - public void testLockRequestFutureStringStringCallback() throws Exception { - assertFalse(fut.isDone()); - assertEquals(RESOURCE, fut.getResourceId()); - assertEquals(OWNER, fut.getOwner()); - - fut.setLocked(true); - fut.invokeCallback(); - - // ensure it invoked the callback - verify(callback).set(true); - } - - @Test - public void testLockRequestFutureStringStringCallback_ArgEx() throws Exception { - - // null resource id - IllegalArgumentException ex = expectException(IllegalArgumentException.class, - () -> new LockRequestFuture(null, OWNER, callback)); - assertEquals("null resourceId", ex.getMessage()); - - - // null owner - ex = expectException(IllegalArgumentException.class, () -> new LockRequestFuture(RESOURCE, null, callback)); - assertEquals("null owner", ex.getMessage()); - - - // null callback is OK - new LockRequestFuture(RESOURCE, OWNER, null); - } - - @Test - public void testGetResourceId() { - assertEquals(RESOURCE, fut.getResourceId()); - } - - @Test - public void testGetOwner() { - assertEquals(OWNER, fut.getOwner()); - } - - @Test - public void testCancel() throws Exception { - // not cancelled yet - assertFalse(fut.isDone()); - - // cancel it - assertTrue(fut.cancel(false)); - assertTrue(fut.isDone()); - - // should not block now - expectException(CancellationException.class, () -> fut.get(0, TimeUnit.SECONDS)); - - } - - @Test - public void testCancel_AlreadyCancelled() throws Exception { - - fut.cancel(true); - - assertFalse(fut.cancel(true)); - assertTrue(fut.isDone()); - - expectException(CancellationException.class, () -> fut.get(0, TimeUnit.SECONDS)); - } - - @Test - public void testCancel_AlreadyAcquired() throws Exception { - - fut.setLocked(true); - - assertFalse(fut.cancel(true)); - assertTrue(fut.isDone()); - assertTrue(fut.get(0, TimeUnit.SECONDS)); - } - - @Test - public void testCancel_AlreadyDenied() throws Exception { - - fut.setLocked(false); - - assertFalse(fut.cancel(true)); - assertTrue(fut.isDone()); - assertFalse(fut.get(0, TimeUnit.SECONDS)); - } - - @Test - public void testSetLocked_True() throws Exception { - assertTrue(fut.setLocked(true)); - - assertTrue(fut.isDone()); - assertTrue(fut.get(0, TimeUnit.SECONDS)); - } - - @Test - public void testSetLocked_False() throws Exception { - assertTrue(fut.setLocked(false)); - - assertTrue(fut.isDone()); - assertFalse(fut.get(0, TimeUnit.SECONDS)); - } - - @Test - public void testSetLocked_AlreadyCancelled() { - - fut.cancel(true); - - assertFalse(fut.setLocked(true)); - assertFalse(fut.setLocked(false)); - - assertTrue(fut.isDone()); - expectException(CancellationException.class, () -> fut.get(0, TimeUnit.SECONDS)); - } - - @Test - public void testSetLocked_AlreadyAcquired() throws Exception { - fut.setLocked(true); - - assertTrue(fut.isDone()); - assertTrue(fut.get(0, TimeUnit.SECONDS)); - - assertFalse(fut.cancel(true)); - assertFalse(fut.setLocked(true)); - assertFalse(fut.setLocked(false)); - } - - @Test - public void testSetLocked_AlreadyDenied() throws Exception { - fut.setLocked(false); - - assertTrue(fut.isDone()); - assertFalse(fut.get(0, TimeUnit.SECONDS)); - - assertFalse(fut.cancel(true)); - assertFalse(fut.setLocked(true)); - assertFalse(fut.setLocked(false)); - } - - @Test - public void testIsCancelled() { - assertFalse(fut.isCancelled()); - - fut.cancel(false); - assertTrue(fut.isCancelled()); - } - - @Test - public void testIsCancelled_Acquired() { - fut.setLocked(true); - assertFalse(fut.isCancelled()); - } - - @Test - public void testIsCancelled_Denied() { - fut.setLocked(false); - assertFalse(fut.isCancelled()); - } - - @Test - public void testIsDone_Cancelled() { - fut.cancel(false); - assertTrue(fut.isDone()); - } - - @Test - public void testIsDone_Acquired() { - fut.setLocked(true); - assertTrue(fut.isDone()); - } - - @Test - public void testIsDone_Denied() { - fut.setLocked(false); - assertTrue(fut.isDone()); - } - - @Test - public void testIsDone_Waiting() { - assertFalse(fut.isDone()); - } - - @Test - public void testIsLocked_Cancelled() { - fut.cancel(false); - assertFalse(fut.isLocked()); - } - - @Test - public void testIsLocked_Acquired() { - fut.setLocked(true); - assertTrue(fut.isLocked()); - } - - @Test - public void testIsLocked_Denied() { - fut.setLocked(false); - assertFalse(fut.isLocked()); - } - - @Test - public void testIsLocked_Waiting() { - assertFalse(fut.isLocked()); - } - - @Test - public void testGet_Cancelled() throws Exception { - new Thread(() -> fut.cancel(false)).start(); - expectException(CancellationException.class, () -> fut.get()); - } - - @Test - public void testGet_Acquired() throws Exception { - new Thread(() -> fut.setLocked(true)).start(); - assertTrue(fut.get()); - } - - @Test - public void testGet_Denied() throws Exception { - new Thread(() -> fut.setLocked(false)).start(); - assertFalse(fut.get()); - } - - @Test - public void testGetLongTimeUnit() throws Exception { - expectException(TimeoutException.class, () -> fut.get(0, TimeUnit.SECONDS)); - - fut.setLocked(true); - assertTrue(fut.get(0, TimeUnit.SECONDS)); - } - - @Test - public void testGetLongTimeUnit_Timeout() throws Exception { - expectException(TimeoutException.class, () -> fut.get(0, TimeUnit.SECONDS)); - expectException(TimeoutException.class, () -> fut.get(2, TimeUnit.MILLISECONDS)); - } - - @Test - public void testGetLongTimeUnit_Cancelled() throws Exception { - new Thread(() -> fut.cancel(false)).start(); - expectException(CancellationException.class, () -> fut.get(WAIT_SEC, TimeUnit.SECONDS)); - } - - @Test - public void testGetLongTimeUnit_Acquired() throws Exception { - new Thread(() -> fut.setLocked(true)).start(); - assertTrue(fut.get(WAIT_SEC, TimeUnit.SECONDS)); - } - - @Test - public void testGetLongTimeUnit_Denied() throws Exception { - new Thread(() -> fut.setLocked(false)).start(); - assertFalse(fut.get(WAIT_SEC, TimeUnit.SECONDS)); - } - - @Test(expected = IllegalStateException.class) - public void testInvokeCallback() { - fut.setLocked(true); - fut.invokeCallback(); - - // re-invoke - should throw an exception - fut.invokeCallback(); - } - - @Test(expected = IllegalStateException.class) - public void testInvokeCallback_Cancelled() { - fut.cancel(false); - - // invoke after cancel - should throw an exception - fut.invokeCallback(); - } - - @Test - public void testInvokeCallback_Acquired() { - fut.setLocked(true); - fut.invokeCallback(); - - verify(callback).set(true); - verify(callback, never()).set(false); - } - - @Test - public void testInvokeCallback_Denied() { - fut.setLocked(false); - fut.invokeCallback(); - - verify(callback).set(false); - verify(callback, never()).set(true); - } - - @Test - public void testInvokeCallback_Ex() { - doThrow(new RuntimeException(EXPECTED_EXCEPTION)).when(callback).set(anyBoolean()); - - fut.setLocked(false); - fut.invokeCallback(); - } - - @Test - public void testMakeNullArgException() { - IllegalArgumentException ex = LockRequestFuture.makeNullArgException(EXPECTED_EXCEPTION); - assertEquals(EXPECTED_EXCEPTION, ex.getMessage()); - } -} 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 57adc847..0404877b 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 @@ -22,7 +22,6 @@ package org.onap.policy.drools.core.lock; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; import org.junit.Before; import org.junit.Test; import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.OperResult; @@ -46,7 +45,7 @@ public class PolicyResourceLockFeatureAPITest { @Test public void testBeforeLock() { - assertNull(api.beforeLock(RESOURCE_ID, OWNER, null)); + assertEquals(OperResult.OPER_UNHANDLED, api.beforeLock(RESOURCE_ID, OWNER, 0)); } @Test 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 26732e42..92e60268 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 @@ -24,8 +24,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -36,17 +36,17 @@ import static org.onap.policy.drools.core.lock.TestUtils.expectException; import java.util.Arrays; import java.util.LinkedList; import java.util.List; -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.drools.core.lock.PolicyResourceLockFeatureAPI.Callback; import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.OperResult; import org.onap.policy.drools.core.lock.PolicyResourceLockManager.Factory; public class PolicyResourceLockManagerTest { + private static final int MAX_AGE_SEC = 4 * 60; + private static final String NULL_RESOURCE_ID = "null resourceId"; private static final String NULL_OWNER = "null owner"; @@ -63,13 +63,10 @@ public class PolicyResourceLockManagerTest { */ private static Factory saveFactory; - private Callback callback1; private PolicyResourceLockFeatureAPI impl1; private PolicyResourceLockFeatureAPI impl2; private List implList; - private Future fut; - private PolicyResourceLockManager mgr; @BeforeClass @@ -84,7 +81,6 @@ public class PolicyResourceLockManagerTest { @Before public void setUp() { - callback1 = mock(Callback.class); impl1 = mock(PolicyResourceLockFeatureAPI.class); impl2 = mock(PolicyResourceLockFeatureAPI.class); @@ -111,7 +107,7 @@ public class PolicyResourceLockManagerTest { * @param impl */ private void initImplementer(PolicyResourceLockFeatureAPI impl) { - when(impl.beforeLock(anyString(), anyString(), any(Callback.class))).thenReturn(null); + when(impl.beforeLock(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); @@ -119,13 +115,10 @@ public class PolicyResourceLockManagerTest { @Test public void testLock() throws Exception { - fut = mgr.lock(RESOURCE_A, OWNER1, callback1); + assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); - assertTrue(fut.isDone()); - assertTrue(fut.get()); - - verify(impl1).beforeLock(RESOURCE_A, OWNER1, callback1); - verify(impl2).beforeLock(RESOURCE_A, OWNER1, callback1); + verify(impl1).beforeLock(RESOURCE_A, OWNER1, MAX_AGE_SEC); + verify(impl2).beforeLock(RESOURCE_A, OWNER1, MAX_AGE_SEC); verify(impl1).afterLock(RESOURCE_A, OWNER1, true); verify(impl2).afterLock(RESOURCE_A, OWNER1, true); @@ -135,43 +128,48 @@ public class PolicyResourceLockManagerTest { assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER2)); // null callback - not locked yet - fut = mgr.lock(RESOURCE_C, OWNER3, null); - assertTrue(fut.isDone()); - assertTrue(fut.get()); + assertTrue(mgr.lock(RESOURCE_C, OWNER3, MAX_AGE_SEC)); // null callback - already locked - fut = mgr.lock(RESOURCE_A, OWNER3, null); - assertTrue(fut.isDone()); - assertFalse(fut.get()); + assertFalse(mgr.lock(RESOURCE_A, OWNER3, MAX_AGE_SEC)); } @Test public void testLock_ArgEx() { IllegalArgumentException ex = - expectException(IllegalArgumentException.class, () -> mgr.lock(null, OWNER1, callback1)); + expectException(IllegalArgumentException.class, () -> mgr.lock(null, OWNER1, MAX_AGE_SEC)); assertEquals(NULL_RESOURCE_ID, ex.getMessage()); - ex = expectException(IllegalArgumentException.class, () -> mgr.lock(RESOURCE_A, null, callback1)); + ex = expectException(IllegalArgumentException.class, () -> mgr.lock(RESOURCE_A, null, MAX_AGE_SEC)); assertEquals(NULL_OWNER, ex.getMessage()); // this should not throw an exception - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); } @Test - public void testLock_BeforeIntercepted() { - fut = mock(LockRequestFuture.class); + public void testLock_Acquired_BeforeIntercepted() { + // have impl1 intercept + when(impl1.beforeLock(RESOURCE_A, OWNER1, MAX_AGE_SEC)).thenReturn(OperResult.OPER_ACCEPTED); - // NOT async - when(fut.isDone()).thenReturn(true); + assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + verify(impl1).beforeLock(RESOURCE_A, OWNER1, MAX_AGE_SEC); + verify(impl2, never()).beforeLock(anyString(), anyString(), anyInt()); + + verify(impl1, never()).afterLock(anyString(), anyString(), anyBoolean()); + verify(impl2, never()).afterLock(anyString(), anyString(), anyBoolean()); + } + + @Test + public void testLock_Denied_BeforeIntercepted() { // have impl1 intercept - when(impl1.beforeLock(RESOURCE_A, OWNER1, callback1)).thenReturn(fut); + when(impl1.beforeLock(RESOURCE_A, OWNER1, MAX_AGE_SEC)).thenReturn(OperResult.OPER_DENIED); - assertEquals(fut, mgr.lock(RESOURCE_A, OWNER1, callback1)); + assertFalse(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); - verify(impl1).beforeLock(RESOURCE_A, OWNER1, callback1); - verify(impl2, never()).beforeLock(anyString(), anyString(), any(Callback.class)); + verify(impl1).beforeLock(RESOURCE_A, OWNER1, MAX_AGE_SEC); + verify(impl2, never()).beforeLock(anyString(), anyString(), anyInt()); verify(impl1, never()).afterLock(anyString(), anyString(), anyBoolean()); verify(impl2, never()).afterLock(anyString(), anyString(), anyBoolean()); @@ -183,10 +181,7 @@ public class PolicyResourceLockManagerTest { // impl1 intercepts during afterLock() when(impl1.afterLock(RESOURCE_A, OWNER1, true)).thenReturn(true); - fut = mgr.lock(RESOURCE_A, OWNER1, callback1); - - assertTrue(fut.isDone()); - assertTrue(fut.get()); + assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); // impl1 sees it, but impl2 does not verify(impl1).afterLock(RESOURCE_A, OWNER1, true); @@ -195,10 +190,7 @@ public class PolicyResourceLockManagerTest { @Test public void testLock_Acquired() throws Exception { - fut = mgr.lock(RESOURCE_A, OWNER1, callback1); - - assertTrue(fut.isDone()); - assertTrue(fut.get()); + assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); verify(impl1).afterLock(RESOURCE_A, OWNER1, true); verify(impl2).afterLock(RESOURCE_A, OWNER1, true); @@ -207,16 +199,13 @@ public class PolicyResourceLockManagerTest { @Test public void testLock_Denied_AfterIntercepted() throws Exception { - mgr.lock(RESOURCE_A, OWNER1, callback1); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); // impl1 intercepts during afterLock() when(impl1.afterLock(RESOURCE_A, OWNER2, false)).thenReturn(true); // owner2 tries to lock - fut = mgr.lock(RESOURCE_A, OWNER2, null); - - assertTrue(fut.isDone()); - assertFalse(fut.get()); + assertFalse(mgr.lock(RESOURCE_A, OWNER2, MAX_AGE_SEC)); // impl1 sees it, but impl2 does not verify(impl1).afterLock(RESOURCE_A, OWNER2, false); @@ -226,10 +215,10 @@ public class PolicyResourceLockManagerTest { @Test public void testLock_Denied() { - mgr.lock(RESOURCE_A, OWNER1, callback1); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); // owner2 tries to lock - fut = mgr.lock(RESOURCE_A, OWNER2, null); + mgr.lock(RESOURCE_A, OWNER2, MAX_AGE_SEC); verify(impl1).afterLock(RESOURCE_A, OWNER2, false); verify(impl2).afterLock(RESOURCE_A, OWNER2, false); @@ -237,8 +226,8 @@ public class PolicyResourceLockManagerTest { @Test public void testUnlock() throws Exception { - mgr.lock(RESOURCE_A, OWNER1, null); - mgr.lock(RESOURCE_B, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); + mgr.lock(RESOURCE_B, OWNER1, MAX_AGE_SEC); assertTrue(mgr.unlock(RESOURCE_A, OWNER1)); @@ -261,7 +250,7 @@ public class PolicyResourceLockManagerTest { @Test public void testUnlock_BeforeInterceptedTrue() { - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); // have impl1 intercept when(impl1.beforeUnlock(RESOURCE_A, OWNER1)).thenReturn(OperResult.OPER_ACCEPTED); @@ -278,7 +267,7 @@ public class PolicyResourceLockManagerTest { @Test public void testUnlock_BeforeInterceptedFalse() { - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); // have impl1 intercept when(impl1.beforeUnlock(RESOURCE_A, OWNER1)).thenReturn(OperResult.OPER_DENIED); @@ -294,7 +283,7 @@ public class PolicyResourceLockManagerTest { @Test public void testUnlock_Unlocked() { - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); assertTrue(mgr.unlock(RESOURCE_A, OWNER1)); @@ -310,7 +299,7 @@ public class PolicyResourceLockManagerTest { // have impl1 intercept when(impl1.afterUnlock(RESOURCE_A, OWNER1, true)).thenReturn(true); - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); assertTrue(mgr.unlock(RESOURCE_A, OWNER1)); @@ -348,7 +337,7 @@ public class PolicyResourceLockManagerTest { @Test public void testIsLocked_True() { - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); assertTrue(mgr.isLocked(RESOURCE_A)); @@ -386,7 +375,7 @@ public class PolicyResourceLockManagerTest { public void testIsLocked_BeforeIntercepted_False() { // lock it so we can verify that impl1 overrides the superclass isLocker() - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); // have impl1 intercept when(impl1.beforeIsLocked(RESOURCE_A)).thenReturn(OperResult.OPER_DENIED); @@ -399,7 +388,7 @@ public class PolicyResourceLockManagerTest { @Test public void testIsLockedBy_True() { - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); assertTrue(mgr.isLockedBy(RESOURCE_A, OWNER1)); @@ -410,7 +399,7 @@ public class PolicyResourceLockManagerTest { @Test public void testIsLockedBy_False() { // different owner - mgr.lock(RESOURCE_A, OWNER2, null); + mgr.lock(RESOURCE_A, OWNER2, MAX_AGE_SEC); assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER1)); @@ -444,7 +433,7 @@ public class PolicyResourceLockManagerTest { public void testIsLockedBy_BeforeIntercepted_False() { // lock it so we can verify that impl1 overrides the superclass isLocker() - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); // have impl1 intercept when(impl1.beforeIsLockedBy(RESOURCE_A, OWNER1)).thenReturn(OperResult.OPER_DENIED); @@ -470,7 +459,7 @@ public class PolicyResourceLockManagerTest { // clear the implementer list implList.clear(); - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); assertTrue(mgr.isLocked(RESOURCE_A)); assertFalse(mgr.isLocked(RESOURCE_B)); 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 9aeba53c..14964e0e 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 @@ -26,10 +26,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.onap.policy.drools.core.lock.TestUtils.expectException; import java.util.LinkedList; -import java.util.concurrent.CancellationException; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import org.junit.AfterClass; @@ -43,6 +40,12 @@ import org.powermock.reflect.Whitebox; public class SimpleLockManagerTest { + // Note: this must be a multiple of four + private static final int MAX_AGE_SEC = 4 * 60; + private static final int MAX_AGE_MS = MAX_AGE_SEC * 1000; + + private static final String EXPECTED_EXCEPTION = "expected exception"; + private static final String NULL_RESOURCE_ID = "null resourceId"; private static final String NULL_OWNER = "null owner"; @@ -63,7 +66,6 @@ public class SimpleLockManagerTest { private static CurrentTime savedTime; private TestTime testTime; - private Future fut; private SimpleLockManager mgr; @BeforeClass @@ -91,10 +93,7 @@ public class SimpleLockManagerTest { @Test public void testLock() throws Exception { - fut = mgr.lock(RESOURCE_A, OWNER1, null); - - assertTrue(fut.isDone()); - assertTrue(fut.get()); + assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); assertTrue(mgr.isLocked(RESOURCE_A)); assertTrue(mgr.isLockedBy(RESOURCE_A, OWNER1)); @@ -102,49 +101,59 @@ public class SimpleLockManagerTest { assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER2)); // null callback - not locked yet - fut = mgr.lock(RESOURCE_C, OWNER3, null); - assertTrue(fut.isDone()); - assertTrue(fut.get()); + assertTrue(mgr.lock(RESOURCE_C, OWNER3, MAX_AGE_SEC)); // null callback - already locked - fut = mgr.lock(RESOURCE_A, OWNER3, null); - assertTrue(fut.isDone()); - assertFalse(fut.get()); + assertFalse(mgr.lock(RESOURCE_A, OWNER3, MAX_AGE_SEC)); } @Test - public void testLock_AlreadyLocked() throws Exception { - mgr.lock(RESOURCE_A, OWNER1, null); - - fut = mgr.lock(RESOURCE_A, OWNER2, null); - assertTrue(fut.isDone()); - assertFalse(fut.get()); + public void testLock_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.lock(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); + assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER1)); } - @Test(expected = IllegalStateException.class) - public void testLock_SameOwner() throws Exception { - mgr.lock(RESOURCE_A, OWNER1, null); + @Test + public void testLock_AlreadyLocked() throws Exception { + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); + + // same owner + assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); - // should throw an exception - mgr.lock(RESOURCE_A, OWNER1, null); + // different owner + assertFalse(mgr.lock(RESOURCE_A, OWNER2, MAX_AGE_SEC)); } @Test public void testLock_ArgEx() { IllegalArgumentException ex = - expectException(IllegalArgumentException.class, () -> mgr.lock(null, OWNER1, null)); + expectException(IllegalArgumentException.class, () -> mgr.lock(null, OWNER1, MAX_AGE_SEC)); assertEquals(NULL_RESOURCE_ID, ex.getMessage()); - ex = expectException(IllegalArgumentException.class, () -> mgr.lock(RESOURCE_A, null, null)); + ex = expectException(IllegalArgumentException.class, () -> mgr.lock(RESOURCE_A, null, MAX_AGE_SEC)); assertEquals(NULL_OWNER, ex.getMessage()); // this should not throw an exception - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); } @Test public void testUnlock() throws Exception { - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); // unlock it assertTrue(mgr.unlock(RESOURCE_A, OWNER1)); @@ -166,7 +175,7 @@ public class SimpleLockManagerTest { @Test public void testUnlock_DiffOwner() { - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); assertFalse(mgr.unlock(RESOURCE_A, OWNER2)); } @@ -174,8 +183,8 @@ public class SimpleLockManagerTest { public void testIsLocked() { assertFalse(mgr.isLocked(RESOURCE_A)); - mgr.lock(RESOURCE_A, OWNER1, null); - mgr.lock(RESOURCE_B, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); + mgr.lock(RESOURCE_B, OWNER1, MAX_AGE_SEC); assertTrue(mgr.isLocked(RESOURCE_A)); assertTrue(mgr.isLocked(RESOURCE_B)); @@ -204,7 +213,7 @@ public class SimpleLockManagerTest { public void testIsLockedBy() { assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER1)); - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); assertFalse(mgr.isLockedBy(RESOURCE_B, OWNER1)); @@ -230,7 +239,7 @@ public class SimpleLockManagerTest { @Test public void testIsLockedBy_NotLocked() { - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); // different resource, thus no lock assertFalse(mgr.isLockedBy(RESOURCE_B, OWNER1)); @@ -238,7 +247,7 @@ public class SimpleLockManagerTest { @Test public void testIsLockedBy_LockedButNotOwner() { - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); // different owner assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER2)); @@ -247,43 +256,49 @@ public class SimpleLockManagerTest { @Test public void testCleanUpLocks() throws Exception { // note: this assumes that MAX_AGE_MS is divisible by 4 - mgr.lock(RESOURCE_A, OWNER1, null); + mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); assertTrue(mgr.isLocked(RESOURCE_A)); testTime.sleep(10); - mgr.lock(RESOURCE_B, OWNER1, null); + mgr.lock(RESOURCE_B, OWNER1, MAX_AGE_SEC); assertTrue(mgr.isLocked(RESOURCE_A)); assertTrue(mgr.isLocked(RESOURCE_B)); - testTime.sleep(SimpleLockManager.MAX_AGE_MS/4); - mgr.lock(RESOURCE_C, OWNER1, null); + testTime.sleep(MAX_AGE_MS/4); + mgr.lock(RESOURCE_C, OWNER1, MAX_AGE_SEC); assertTrue(mgr.isLocked(RESOURCE_A)); assertTrue(mgr.isLocked(RESOURCE_B)); assertTrue(mgr.isLocked(RESOURCE_C)); - testTime.sleep(SimpleLockManager.MAX_AGE_MS/4); - mgr.lock(RESOURCE_D, OWNER1, null); + testTime.sleep(MAX_AGE_MS/4); + mgr.lock(RESOURCE_D, OWNER1, MAX_AGE_SEC); assertTrue(mgr.isLocked(RESOURCE_A)); assertTrue(mgr.isLocked(RESOURCE_B)); assertTrue(mgr.isLocked(RESOURCE_C)); assertTrue(mgr.isLocked(RESOURCE_D)); // sleep remainder of max age - first two should expire - testTime.sleep(SimpleLockManager.MAX_AGE_MS/2); + testTime.sleep(MAX_AGE_MS/2); assertFalse(mgr.isLocked(RESOURCE_A)); assertFalse(mgr.isLocked(RESOURCE_B)); assertTrue(mgr.isLocked(RESOURCE_C)); assertTrue(mgr.isLocked(RESOURCE_D)); // another quarter - next one should expire - testTime.sleep(SimpleLockManager.MAX_AGE_MS/4); + testTime.sleep(MAX_AGE_MS/4); assertFalse(mgr.isLocked(RESOURCE_C)); assertTrue(mgr.isLocked(RESOURCE_D)); // another quarter - last one should expire - testTime.sleep(SimpleLockManager.MAX_AGE_MS/4); + testTime.sleep(MAX_AGE_MS/4); assertFalse(mgr.isLocked(RESOURCE_D)); } + + @Test + public void testMakeNullArgException() { + IllegalArgumentException ex = SimpleLockManager.makeNullArgException(EXPECTED_EXCEPTION); + assertEquals(EXPECTED_EXCEPTION, ex.getMessage()); + } @Test public void testDataGetXxx() { @@ -390,16 +405,13 @@ public class SimpleLockManagerTest { try { // some locks will be acquired, some denied - mgr.lock(res, owner, null).get(); + mgr.lock(res, owner, MAX_AGE_SEC); // do some "work" stopper.await(1L, TimeUnit.MILLISECONDS); mgr.unlock(res, owner); - } catch (CancellationException | ExecutionException e) { - nfail.incrementAndGet(); - } catch (InterruptedException expected) { Thread.currentThread().interrupt(); break; -- cgit 1.2.3-korg