Merge "Refactor duplicate code from lock managers"

3 files changed, 695 insertions, 96 deletions

diff --git a/policy-management/src/test/java/org/onap/policy/drools/system/internal/FeatureLockImplTest.java b/policy-management/src/test/java/org/onap/policy/drools/system/internal/FeatureLockImplTest.java
new file mode 100644
index 00000000..258ee0c5
--- /dev/null
+++ b/policy-management/src/test/java/org/onap/policy/drools/system/internal/FeatureLockImplTest.java
@@ -0,0 +1,415 @@
+/*
+ * ============LICENSE_START=======================================================
+ * ONAP
+ * ================================================================================
+ * Copyright (C) 2019 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.system.internal;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.util.concurrent.ScheduledExecutorService;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.onap.policy.drools.core.lock.LockCallback;
+import org.onap.policy.drools.core.lock.LockState;
+import org.onap.policy.drools.system.PolicyEngineConstants;
+import org.powermock.reflect.Whitebox;
+
+public class FeatureLockImplTest {
+    private static final String POLICY_ENGINE_EXECUTOR_FIELD = "executorService";
+    private static final String OWNER_KEY = "my key";
+    private static final String RESOURCE = "my resource";
+    private static final int HOLD_SEC = 100;
+    private static final int HOLD_SEC2 = 120;
+
+    private static ScheduledExecutorService saveExec;
+
+    @Mock
+    private ScheduledExecutorService exsvc;
+
+    @Mock
+    private LockCallback callback;
+
+
+    /**
+     * Saves static fields and configures the location of the property files.
+     */
+    @BeforeClass
+    public static void setUpBeforeClass() {
+        saveExec = Whitebox.getInternalState(PolicyEngineConstants.getManager(), POLICY_ENGINE_EXECUTOR_FIELD);
+    }
+
+    /**
+     * Restores static fields.
+     */
+    @AfterClass
+    public static void tearDownAfterClass() {
+        Whitebox.setInternalState(PolicyEngineConstants.getManager(), POLICY_ENGINE_EXECUTOR_FIELD, saveExec);
+    }
+
+    /**
+     * Initializes the mocks and creates a feature that uses {@link #exsvc} to execute
+     * tasks.
+     */
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+
+        Whitebox.setInternalState(PolicyEngineConstants.getManager(), POLICY_ENGINE_EXECUTOR_FIELD, exsvc);
+    }
+
+    @Test
+    public void testNoArgs() {
+        MyLock lock = new MyLock();
+        assertNull(lock.getResourceId());
+        assertNull(lock.getOwnerKey());
+        assertNull(lock.getCallback());
+        assertEquals(0, lock.getHoldSec());
+    }
+
+    @Test
+    public void testFeatureLockImpl() {
+        MyLock lock = new MyLock(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+        assertTrue(lock.isWaiting());
+        assertEquals(RESOURCE, lock.getResourceId());
+        assertEquals(OWNER_KEY, lock.getOwnerKey());
+        assertSame(callback, lock.getCallback());
+        assertEquals(HOLD_SEC, lock.getHoldSec());
+    }
+
+    @Test
+    public void testSerializable() throws Exception {
+        MyLock lock = new MyLock(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+        lock = roundTrip(lock);
+
+        assertTrue(lock.isWaiting());
+
+        assertEquals(RESOURCE, lock.getResourceId());
+        assertEquals(OWNER_KEY, lock.getOwnerKey());
+        assertNull(lock.getCallback());
+        assertEquals(HOLD_SEC, lock.getHoldSec());
+    }
+
+    /**
+     * Tests grant(), when using the foreground thread.
+     */
+    @Test
+    public void testGrantForeground() {
+        MyLock lock = new MyLock(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+        lock.grant(true);
+
+        assertTrue(lock.isActive());
+        assertEquals(1, lock.nupdates);
+
+        verify(exsvc, never()).execute(any());
+
+        verify(callback).lockAvailable(any());
+        verify(callback, never()).lockUnavailable(any());
+    }
+
+    /**
+     * Tests grant(), when using the background thread.
+     */
+    @Test
+    public void testGrantBackground() {
+        MyLock lock = new MyLock(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+        lock.grant(false);
+
+        assertTrue(lock.isActive());
+        assertEquals(1, lock.nupdates);
+
+        invokeCallback(1);
+        verify(callback).lockAvailable(any());
+        verify(callback, never()).lockUnavailable(any());
+    }
+
+    /**
+     * Tests grant() when the lock is already unavailable.
+     */
+    @Test
+    public void testGrantUnavailable() {
+        MyLock lock = new MyLock(LockState.UNAVAILABLE, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+        lock.setState(LockState.UNAVAILABLE);
+        lock.grant(true);
+
+        assertTrue(lock.isUnavailable());
+        assertEquals(0, lock.nupdates);
+
+        verify(exsvc, never()).execute(any());
+    }
+
+    /**
+     * Tests deny(), when using the foreground thread.
+     */
+    @Test
+    public void testDenyForeground() {
+        MyLock lock = new MyLock(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+        lock.deny("my reason", true);
+
+        assertTrue(lock.isUnavailable());
+
+        verify(exsvc, never()).execute(any());
+
+        verify(callback, never()).lockAvailable(any());
+        verify(callback).lockUnavailable(any());
+    }
+
+    /**
+     * Tests deny(), when using the background thread.
+     */
+    @Test
+    public void testDenyBackground() {
+        MyLock lock = new MyLock(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+        lock.deny("my reason", false);
+
+        assertTrue(lock.isUnavailable());
+
+        invokeCallback(1);
+        verify(callback, never()).lockAvailable(any());
+        verify(callback).lockUnavailable(any());
+    }
+
+    @Test
+    public void testFreeAllowed() {
+        MyLock lock = new MyLock(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+        assertTrue(lock.freeAllowed());
+    }
+
+    /**
+     * Tests freeAllowed() when the lock is unavailable.
+     */
+    @Test
+    public void testFreeAllowedUnavailable() {
+        MyLock lock = new MyLock(LockState.UNAVAILABLE, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+        assertFalse(lock.freeAllowed());
+        assertTrue(lock.isUnavailable());
+    }
+
+    /**
+     * Tests that free() works on a serialized lock with a new feature.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Test
+    public void testFreeAllowedSerialized() throws Exception {
+        MyLock lock = new MyLock(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+
+        lock = roundTrip(lock);
+        assertTrue(lock.freeAllowed());
+    }
+
+    /**
+     * Tests free() on a serialized lock without a feature.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Test
+    public void testFreeAllowedNoFeature() throws Exception {
+        MyLock lock = new MyLockNoFeature(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+
+        lock = roundTrip(lock);
+        assertFalse(lock.freeAllowed());
+        assertTrue(lock.isUnavailable());
+    }
+
+    @Test
+    public void testExtendAllowed() {
+        MyLock lock = new MyLock(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+
+        LockCallback scallback = mock(LockCallback.class);
+        assertTrue(lock.extendAllowed(HOLD_SEC2, scallback));
+        assertTrue(lock.isWaiting());
+        assertEquals(HOLD_SEC2, lock.getHoldSec());
+        assertSame(scallback, lock.getCallback());
+
+        verify(exsvc, never()).execute(any());
+
+        // invalid arguments
+
+        // @formatter:off
+        assertThatIllegalArgumentException().isThrownBy(
+            () -> new MyLock(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback)
+                            .extendAllowed(-1, callback))
+            .withMessageContaining("holdSec");
+        // @formatter:on
+    }
+
+    /**
+     * Tests extendAllowed() when the lock is unavailable.
+     */
+    @Test
+    public void testExtendAllowedUnavailable() {
+        MyLock lock = new MyLock(LockState.UNAVAILABLE, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+
+        LockCallback scallback = mock(LockCallback.class);
+        assertFalse(lock.extendAllowed(HOLD_SEC2, scallback));
+        assertTrue(lock.isUnavailable());
+        assertEquals(HOLD_SEC2, lock.getHoldSec());
+        assertSame(scallback, lock.getCallback());
+
+        verify(scallback, never()).lockAvailable(lock);
+        verify(scallback).lockUnavailable(lock);
+    }
+
+    /**
+     * Tests that extendAllowed() works on a serialized lock with a new feature.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Test
+    public void testExtendAllowedSerialized() throws Exception {
+        MyLock lock = new MyLock(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+
+        lock = roundTrip(lock);
+
+        LockCallback scallback = mock(LockCallback.class);
+        assertTrue(lock.extendAllowed(HOLD_SEC2, scallback));
+        assertTrue(lock.isWaiting());
+        assertEquals(HOLD_SEC2, lock.getHoldSec());
+        assertSame(scallback, lock.getCallback());
+
+        verify(exsvc, never()).execute(any());
+    }
+
+    /**
+     * Tests extendAllowed() on a serialized lock without a feature.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Test
+    public void testExtendAllowedNoFeature() throws Exception {
+        MyLock lock = new MyLockNoFeature(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback);
+
+        lock = roundTrip(lock);
+
+        LockCallback scallback = mock(LockCallback.class);
+        assertFalse(lock.extendAllowed(HOLD_SEC2, scallback));
+        assertTrue(lock.isUnavailable());
+        assertEquals(HOLD_SEC2, lock.getHoldSec());
+        assertSame(scallback, lock.getCallback());
+
+        verify(scallback, never()).lockAvailable(lock);
+        verify(scallback).lockUnavailable(lock);
+    }
+
+    @Test
+    public void testToString() {
+        String text = new MyLock(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback).toString();
+        assertNotNull(text);
+        assertThat(text).contains("LockImpl");
+    }
+
+    private MyLock roundTrip(MyLock lock) throws Exception {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        try (ObjectOutputStream oos = new ObjectOutputStream(baos)) {
+            oos.writeObject(lock);
+        }
+
+        ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
+        try (ObjectInputStream ois = new ObjectInputStream(bais)) {
+            return (MyLock) ois.readObject();
+        }
+    }
+
+    /**
+     * Invokes the last call-back in the work queue.
+     *
+     * @param nexpected number of call-backs expected in the work queue
+     */
+    private void invokeCallback(int nexpected) {
+        ArgumentCaptor<Runnable> captor = ArgumentCaptor.forClass(Runnable.class);
+        verify(exsvc, times(nexpected)).execute(captor.capture());
+
+        if (nexpected > 0) {
+            captor.getAllValues().get(nexpected - 1).run();
+        }
+    }
+
+    public static class MyLock extends FeatureLockImpl {
+        private static final long serialVersionUID = 1L;
+        private int nupdates = 0;
+
+        public MyLock() {
+            super();
+        }
+
+        public MyLock(LockState state, String resourceId, String ownerKey, int holdSec, LockCallback callback) {
+            super(state, resourceId, ownerKey, holdSec, callback);
+        }
+
+        @Override
+        protected void updateGrant() {
+            super.updateGrant();
+            ++nupdates;
+        }
+
+        @Override
+        public boolean free() {
+            return false;
+        }
+
+        @Override
+        public void extend(int holdSec, LockCallback callback) {
+            // do nothing
+        }
+
+        @Override
+        protected boolean addToFeature() {
+            return true;
+        }
+    }
+
+    public static class MyLockNoFeature extends MyLock {
+        private static final long serialVersionUID = 1L;
+
+        public MyLockNoFeature() {
+            super();
+        }
+
+        public MyLockNoFeature(LockState state, String resourceId, String ownerKey, int holdSec,
+                        LockCallback callback) {
+            super(state, resourceId, ownerKey, holdSec, callback);
+        }
+
+        @Override
+        protected boolean addToFeature() {
+            return false;
+        }
+    }
+}
diff --git a/policy-management/src/test/java/org/onap/policy/drools/system/internal/LockManagerTest.java b/policy-management/src/test/java/org/onap/policy/drools/system/internal/LockManagerTest.java
new file mode 100644
index 00000000..1cda079d
--- /dev/null
+++ b/policy-management/src/test/java/org/onap/policy/drools/system/internal/LockManagerTest.java
@@ -0,0 +1,259 @@
+/*
+ * ============LICENSE_START=======================================================
+ * ONAP
+ * ================================================================================
+ * Copyright (C) 2019 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.system.internal;
+
+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.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.concurrent.ScheduledExecutorService;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.onap.policy.drools.core.lock.AlwaysFailLock;
+import org.onap.policy.drools.core.lock.Lock;
+import org.onap.policy.drools.core.lock.LockCallback;
+import org.onap.policy.drools.core.lock.LockState;
+
+public class LockManagerTest {
+    private static final String OWNER_KEY = "my key";
+    private static final String RESOURCE = "my resource";
+    private static final String RESOURCE2 = "my resource #2";
+    private static final int HOLD_SEC = 100;
+
+    @Mock
+    private LockCallback callback;
+
+    @Mock
+    private ScheduledExecutorService exsvc;
+
+    private MyManager mgr;
+
+    /**
+     * Resets fields and creates {@link #mgr}.
+     */
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+
+        doAnswer(args -> {
+            args.getArgumentAt(0, Runnable.class).run();
+            return null;
+        }).when(exsvc).execute(any());
+
+        mgr = new MyManager();
+    }
+
+    @After
+    public void tearDown() {
+
+    }
+
+    @Test
+    public void testIsAlive() {
+        assertFalse(mgr.isAlive());
+        assertFalse(mgr.isLocked());
+
+        mgr.start();
+        assertTrue(mgr.isAlive());
+        assertFalse(mgr.isLocked());
+
+        mgr.stop();
+        assertFalse(mgr.isAlive());
+    }
+
+    @Test
+    public void testStart() {
+        assertTrue(mgr.start());
+        assertTrue(mgr.isAlive());
+
+        assertFalse(mgr.start());
+        assertTrue(mgr.isAlive());
+
+        mgr.stop();
+        assertTrue(mgr.start());
+        assertTrue(mgr.isAlive());
+    }
+
+    @Test
+    public void testStop() {
+        assertFalse(mgr.stop());
+
+        mgr.start();
+        assertTrue(mgr.stop());
+        assertFalse(mgr.isAlive());
+    }
+
+    @Test
+    public void testShutdown() {
+        mgr.start();
+        mgr.shutdown();
+        assertFalse(mgr.isAlive());
+
+        mgr.shutdown();
+        assertFalse(mgr.isAlive());
+    }
+
+    @Test
+    public void testIsLocked() {
+        assertFalse(mgr.isLocked());
+        assertFalse(mgr.isAlive());
+
+        mgr.lock();
+        assertTrue(mgr.isLocked());
+        assertFalse(mgr.isAlive());
+
+        mgr.unlock();
+        assertFalse(mgr.isLocked());
+    }
+
+    @Test
+    public void testLock() {
+        assertTrue(mgr.lock());
+        assertTrue(mgr.isLocked());
+
+        assertFalse(mgr.lock());
+        assertTrue(mgr.isLocked());
+
+        mgr.unlock();
+        assertTrue(mgr.lock());
+        assertTrue(mgr.isLocked());
+    }
+
+    @Test
+    public void testUnlock() {
+        assertFalse(mgr.unlock());
+
+        mgr.lock();
+        assertTrue(mgr.unlock());
+        assertFalse(mgr.isLocked());
+    }
+
+    @Test
+    public void testCreateLock() {
+        Lock lock = mgr.createLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
+        assertTrue(lock.isActive());
+        verify(callback).lockAvailable(lock);
+        verify(callback, never()).lockUnavailable(lock);
+
+        // should not be able to lock it again
+        LockCallback callback2 = mock(LockCallback.class);
+        Lock lock2 = mgr.createLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback2, false);
+        assertTrue(lock2.isUnavailable());
+        verify(callback2, never()).lockAvailable(lock2);
+        verify(callback2).lockUnavailable(lock2);
+
+        // should be able to lock another resource
+        LockCallback callback3 = mock(LockCallback.class);
+        Lock lock3 = mgr.createLock(RESOURCE2, OWNER_KEY, HOLD_SEC, callback3, false);
+        assertTrue(lock3.isActive());
+        verify(callback3).lockAvailable(lock3);
+        verify(callback3, never()).lockUnavailable(lock3);
+    }
+
+    /**
+     * Tests createLock() when the feature instance has changed.
+     */
+    @Test
+    public void testCreateLockInstanceChanged() {
+        mgr = spy(mgr);
+        when(mgr.hasInstanceChanged()).thenReturn(true);
+
+        Lock lock = mgr.createLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
+        assertTrue(lock instanceof AlwaysFailLock);
+        assertTrue(lock.isUnavailable());
+
+        verify(callback, never()).lockAvailable(lock);
+        verify(callback).lockUnavailable(lock);
+    }
+
+    @Test
+    public void testGetResource2lock() {
+        assertNotNull(mgr.getResource2lock());
+    }
+
+    private class MyManager extends LockManager<MyLock> {
+
+        @Override
+        protected boolean hasInstanceChanged() {
+            return false;
+        }
+
+        @Override
+        protected void finishLock(MyLock lock) {
+            lock.grant(true);
+        }
+
+        @Override
+        protected MyLock makeLock(LockState waiting, String resourceId, String ownerKey, int holdSec,
+                        LockCallback callback) {
+            return new MyLock(waiting, resourceId, ownerKey, holdSec, callback);
+        }
+
+    }
+
+    private class MyLock extends FeatureLockImpl {
+        private static final long serialVersionUID = 1L;
+
+        public MyLock(LockState waiting, String resourceId, String ownerKey, int holdSec, LockCallback callback) {
+            super(waiting, resourceId, ownerKey, holdSec, callback);
+        }
+
+        @Override
+        public boolean free() {
+            return false;
+        }
+
+        @Override
+        public void extend(int holdSec, LockCallback callback) {
+            // do nothing
+        }
+
+        @Override
+        protected boolean addToFeature() {
+            return false;
+        }
+
+        @Override
+        public void notifyAvailable() {
+            getCallback().lockAvailable(this);
+        }
+
+        @Override
+        public void notifyUnavailable() {
+            getCallback().lockUnavailable(this);
+        }
+
+        @Override
+        protected ScheduledExecutorService getThreadPool() {
+            return exsvc;
+        }
+    }
+}
diff --git a/policy-management/src/test/java/org/onap/policy/drools/system/internal/SimpleLockManagerTest.java b/policy-management/src/test/java/org/onap/policy/drools/system/internal/SimpleLockManagerTest.java
index 66406898..79e20673 100644
--- a/policy-management/src/test/java/org/onap/policy/drools/system/internal/SimpleLockManagerTest.java
+++ b/policy-management/src/test/java/org/onap/policy/drools/system/internal/SimpleLockManagerTest.java
@@ -63,7 +63,6 @@ import org.onap.policy.common.utils.time.TestTime;
 import org.onap.policy.drools.core.lock.Lock;
 import org.onap.policy.drools.core.lock.LockCallback;
 import org.onap.policy.drools.core.lock.LockState;
-import org.onap.policy.drools.system.PolicyEngine;
 import org.onap.policy.drools.system.PolicyEngineConstants;
 import org.onap.policy.drools.system.internal.SimpleLockManager.SimpleLock;
 import org.powermock.reflect.Whitebox;
@@ -95,9 +94,6 @@ public class SimpleLockManagerTest {
     private ScheduledExecutorService exsvc;
 
     @Mock
-    private PolicyEngine engine;
-
-    @Mock
     private ScheduledFuture<?> future;
 
     @Mock
@@ -113,7 +109,6 @@ public class SimpleLockManagerTest {
         saveExec = Whitebox.getInternalState(PolicyEngineConstants.getManager(), POLICY_ENGINE_EXECUTOR_FIELD);
 
         realExec = Executors.newScheduledThreadPool(3);
-        Whitebox.setInternalState(PolicyEngineConstants.getManager(), POLICY_ENGINE_EXECUTOR_FIELD, realExec);
     }
 
     /**
@@ -141,7 +136,7 @@ public class SimpleLockManagerTest {
 
         Whitebox.setInternalState(SimpleLockManager.class, TIME_FIELD, testTime);
 
-        when(engine.getExecutorService()).thenReturn(exsvc);
+        Whitebox.setInternalState(PolicyEngineConstants.getManager(), POLICY_ENGINE_EXECUTOR_FIELD, exsvc);
 
         feature = new MyLockingFeature();
         feature.start();
@@ -156,19 +151,14 @@ public class SimpleLockManagerTest {
         Properties props = new Properties();
         props.setProperty(SimpleLockProperties.EXPIRE_CHECK_SEC, "abc");
 
-        assertThatThrownBy(() -> new MyLockingFeature(engine, props)).isInstanceOf(SimpleLockManagerException.class);
+        assertThatThrownBy(() -> new MyLockingFeature(props)).isInstanceOf(SimpleLockManagerException.class);
     }
 
     @Test
-    public void testIsAlive() {
+    public void testStart() {
         assertTrue(feature.isAlive());
+        verify(exsvc).scheduleWithFixedDelay(any(), anyLong(), anyLong(), any());
 
-        feature.stop();
-        assertFalse(feature.isAlive());
-    }
-
-    @Test
-    public void testStart() {
         assertFalse(feature.start());
 
         feature.stop();
@@ -178,6 +168,7 @@ public class SimpleLockManagerTest {
     @Test
     public void testStop() {
         assertTrue(feature.stop());
+        assertFalse(feature.isAlive());
         verify(future).cancel(true);
 
         assertFalse(feature.stop());
@@ -194,21 +185,12 @@ public class SimpleLockManagerTest {
     }
 
     @Test
-    public void testLockApi() {
-        assertFalse(feature.isLocked());
-        assertTrue(feature.lock());
-        assertTrue(feature.unlock());
-    }
-
-    @Test
     public void testCreateLock() {
         // this lock should be granted immediately
         SimpleLock lock = getLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
         assertTrue(lock.isActive());
         assertEquals(testTime.getMillis() + HOLD_MS, lock.getHoldUntilMs());
 
-        invokeCallback(1);
-
         verify(callback).lockAvailable(lock);
         verify(callback, never()).lockUnavailable(lock);
 
@@ -218,8 +200,6 @@ public class SimpleLockManagerTest {
         assertFalse(lock2.isActive());
         assertTrue(lock2.isUnavailable());
 
-        invokeCallback(2);
-
         verify(callback, never()).lockAvailable(lock2);
         verify(callback).lockUnavailable(lock2);
 
@@ -231,13 +211,12 @@ public class SimpleLockManagerTest {
         // should work with "true" value also
         Lock lock3 = feature.createLock(RESOURCE2, OWNER_KEY, HOLD_SEC, callback, true);
         assertTrue(lock3.isActive());
-        invokeCallback(3);
         verify(callback).lockAvailable(lock3);
         verify(callback, never()).lockUnavailable(lock3);
     }
 
     /**
-     * Tests lock() when the feature is not the latest instance.
+     * Tests createLock() when the feature is not the latest instance.
      */
     @Test
     public void testCreateLockNotLatestInstance() {
@@ -275,7 +254,7 @@ public class SimpleLockManagerTest {
 
         // run the callbacks
         captor = ArgumentCaptor.forClass(Runnable.class);
-        verify(exsvc, times(5)).execute(captor.capture());
+        verify(exsvc, times(2)).execute(captor.capture());
         captor.getAllValues().forEach(Runnable::run);
         verify(callback).lockUnavailable(lock);
         verify(callback).lockUnavailable(lock2);
@@ -295,7 +274,7 @@ public class SimpleLockManagerTest {
 
         // run the callback
         captor = ArgumentCaptor.forClass(Runnable.class);
-        verify(exsvc, times(9)).execute(captor.capture());
+        verify(exsvc, times(3)).execute(captor.capture());
         captor.getValue().run();
         verify(callback).lockUnavailable(lock3);
     }
@@ -328,7 +307,6 @@ public class SimpleLockManagerTest {
         feature.start();
 
         feature.createLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
-        invokeCallback(1);
 
         ArgumentCaptor<Runnable> captor = ArgumentCaptor.forClass(Runnable.class);
         verify(exsvc).scheduleWithFixedDelay(captor.capture(), anyLong(), anyLong(), any());
@@ -339,14 +317,12 @@ public class SimpleLockManagerTest {
 
         // lock should now be gone and we should be able to get another
         feature.createLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
-        invokeCallback(2);
 
         // should have succeeded twice
         verify(callback, times(2)).lockAvailable(any());
 
         // lock should not be available now
         feature.createLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
-        invokeCallback(3);
         verify(callback).lockUnavailable(any());
     }
 
@@ -389,7 +365,6 @@ public class SimpleLockManagerTest {
         feature.start();
 
         feature.createLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
-        invokeCallback(1);
 
         ArgumentCaptor<Runnable> captor = ArgumentCaptor.forClass(Runnable.class);
         verify(exsvc).scheduleWithFixedDelay(captor.capture(), anyLong(), anyLong(), any());
@@ -400,14 +375,13 @@ public class SimpleLockManagerTest {
 
         // lock should not be available now
         feature.createLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
-        invokeCallback(3);
         verify(callback).lockUnavailable(any());
     }
 
     @Test
     public void testGetThreadPool() {
         // use a real feature
-        feature = new SimpleLockManager(engine, new Properties());
+        feature = new SimpleLockManager(null, new Properties());
 
         // load properties
         feature.start();
@@ -459,45 +433,28 @@ public class SimpleLockManagerTest {
     @Test
     public void testSimpleLockExpired() {
         SimpleLock lock = getLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
-        lock.grant();
+        lock.grant(true);
 
         assertFalse(lock.expired(testTime.getMillis()));
         assertFalse(lock.expired(testTime.getMillis() + HOLD_MS - 1));
         assertTrue(lock.expired(testTime.getMillis() + HOLD_MS));
     }
 
-    /**
-     * Tests grant() when the lock is already unavailable.
-     */
-    @Test
-    public void testSimpleLockGrantUnavailable() {
-        SimpleLock lock = getLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
-        lock.setState(LockState.UNAVAILABLE);
-        lock.grant();
-
-        assertTrue(lock.isUnavailable());
-        verify(callback, never()).lockAvailable(any());
-        verify(callback, never()).lockUnavailable(any());
-    }
-
     @Test
     public void testSimpleLockFree() {
         final SimpleLock lock = getLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
 
         // lock2 should be denied
         SimpleLock lock2 = getLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
-        invokeCallback(2);
         verify(callback, never()).lockAvailable(lock2);
         verify(callback).lockUnavailable(lock2);
 
         // lock2 was denied, so nothing new should happen when freed
         assertFalse(lock2.free());
-        invokeCallback(2);
 
         // force lock2 to be active - still nothing should happen
         Whitebox.setInternalState(lock2, "state", LockState.ACTIVE);
         assertFalse(lock2.free());
-        invokeCallback(2);
 
         // now free the first lock
         assertTrue(lock.free());
@@ -506,6 +463,9 @@ public class SimpleLockManagerTest {
         // should be able to get the lock now
         SimpleLock lock3 = getLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
         assertTrue(lock3.isActive());
+
+        verify(callback).lockAvailable(lock3);
+        verify(callback, never()).lockUnavailable(lock3);
     }
 
     /**
@@ -525,41 +485,22 @@ public class SimpleLockManagerTest {
         assertTrue(lock.isUnavailable());
     }
 
-    /**
-     * Tests free() on a serialized lock without a feature.
-     *
-     * @throws Exception if an error occurs
-     */
-    @Test
-    public void testSimpleLockFreeNoFeature() throws Exception {
-        SimpleLock lock = getLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
-
-        SimpleLockManager.setLatestInstance(null);
-
-        lock = roundTrip(lock);
-        assertFalse(lock.free());
-        assertTrue(lock.isUnavailable());
-    }
-
     @Test
     public void testSimpleLockExtend() {
         final SimpleLock lock = getLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
 
         // lock2 should be denied
         SimpleLock lock2 = getLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false);
-        invokeCallback(2);
         verify(callback, never()).lockAvailable(lock2);
         verify(callback).lockUnavailable(lock2);
 
         // lock2 will still be denied
         lock2.extend(HOLD_SEC, callback);
-        invokeCallback(3);
         verify(callback, times(2)).lockUnavailable(lock2);
 
         // force lock2 to be active - should still be denied
         Whitebox.setInternalState(lock2, "state", LockState.ACTIVE);
         lock2.extend(HOLD_SEC, callback);
-        invokeCallback(4);
         verify(callback, times(3)).lockUnavailable(lock2);
 
         assertThatIllegalArgumentException().isThrownBy(() -> lock.extend(-1, callback))
@@ -569,8 +510,7 @@ public class SimpleLockManagerTest {
         lock.extend(HOLD_SEC2, callback);
         assertEquals(HOLD_SEC2, lock.getHoldSec());
         assertEquals(testTime.getMillis() + HOLD_MS2, lock.getHoldUntilMs());
-        invokeCallback(5);
-        verify(callback).lockAvailable(lock);
+        verify(callback, times(2)).lockAvailable(lock);
         verify(callback, never()).lockUnavailable(lock);
     }
 
@@ -592,13 +532,12 @@ public class SimpleLockManagerTest {
         lock.extend(HOLD_SEC, scallback);
         assertTrue(lock.isActive());
 
-        invokeCallback(1);
         verify(scallback).lockAvailable(lock);
         verify(scallback, never()).lockUnavailable(lock);
     }
 
     /**
-     * Tests extend() on a serialized lock without a feature.
+     * Tests that extend() fails when there is no feature.
      *
      * @throws Exception if an error occurs
      */
@@ -614,7 +553,6 @@ public class SimpleLockManagerTest {
         lock.extend(HOLD_SEC, scallback);
         assertTrue(lock.isUnavailable());
 
-        invokeCallback(1);
         verify(scallback, never()).lockAvailable(lock);
         verify(scallback).lockUnavailable(lock);
     }
@@ -635,7 +573,8 @@ public class SimpleLockManagerTest {
     @Test
     public void testMultiThreaded() throws InterruptedException {
         Whitebox.setInternalState(SimpleLockManager.class, TIME_FIELD, testTime);
-        feature = new SimpleLockManager(PolicyEngineConstants.getManager(), new Properties());
+        Whitebox.setInternalState(PolicyEngineConstants.getManager(), POLICY_ENGINE_EXECUTOR_FIELD, realExec);
+        feature = new SimpleLockManager(null, new Properties());
         feature.start();
 
         List<MyThread> threads = new ArrayList<>(MAX_THREADS);
@@ -677,33 +616,19 @@ public class SimpleLockManagerTest {
     }
 
     /**
-     * Invokes the last call-back in the work queue.
-     *
-     * @param nexpected number of call-backs expected in the work queue
-     */
-    private void invokeCallback(int nexpected) {
-        ArgumentCaptor<Runnable> captor = ArgumentCaptor.forClass(Runnable.class);
-        verify(exsvc, times(nexpected)).execute(captor.capture());
-
-        if (nexpected > 0) {
-            captor.getAllValues().get(nexpected - 1).run();
-        }
-    }
-
-    /**
      * Feature that uses <i>exsvc</i> to execute requests.
      */
     private class MyLockingFeature extends SimpleLockManager {
 
         public MyLockingFeature() {
-            this(engine, new Properties());
+            this(new Properties());
         }
 
-        public MyLockingFeature(PolicyEngine engine, Properties props) {
-            super(engine, props);
+        public MyLockingFeature(Properties props) {
+            super(null, props);
 
             exsvc = mock(ScheduledExecutorService.class);
-            when(engine.getExecutorService()).thenReturn(exsvc);
+            Whitebox.setInternalState(PolicyEngineConstants.getManager(), POLICY_ENGINE_EXECUTOR_FIELD, exsvc);
 
             when(exsvc.scheduleWithFixedDelay(any(), anyLong(), anyLong(), any())).thenAnswer(answer -> {
                 return future;