Sonar fixes for policy-core locks

Made a number of changes to the locking code in policy-core, to address
sonar issues.  This entaileed changing some of the Lock API methods to
return OperResult instead of Boolean.
Updated distributed locking with the new API return types.
Simplified Thread creation using functional methods.

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

10 files changed, 183 insertions, 199 deletions

diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java
index 7906dba7..5994beb6 100644
--- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java
+++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java
@@ -86,24 +86,24 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy
 	}
 
 	@Override
-	public Boolean beforeUnlock(String resourceId, String owner) {
+	public OperResult beforeUnlock(String resourceId, String owner) {
 		TargetLock tLock = new TargetLock(resourceId, uuid, owner, lockProps);
 		
-		return tLock.unlock();
+		return(tLock.unlock() ? OperResult.OPER_ACCEPTED : OperResult.OPER_DENIED);
 	}
 	
 	@Override
-	public Boolean beforeIsLockedBy(String resourceId, String owner) {
+	public OperResult beforeIsLockedBy(String resourceId, String owner) {
 		TargetLock tLock = new TargetLock(resourceId, uuid, owner, lockProps);
-		
-		return tLock.isActive();
+
+        return(tLock.isActive() ? OperResult.OPER_ACCEPTED : OperResult.OPER_DENIED);
 	}
 	
 	@Override
-	public Boolean beforeIsLocked(String resourceId) {
+	public OperResult beforeIsLocked(String resourceId) {
 		TargetLock tLock = new TargetLock(resourceId, uuid, "dummyOwner", lockProps);
-		
-		return tLock.isLocked();
+
+        return(tLock.isLocked() ? OperResult.OPER_ACCEPTED : OperResult.OPER_DENIED);
 	}
 	
 	@Override
diff --git a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java
index 2dc2ceb8..b01d9676 100644
--- a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java
+++ b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java
@@ -25,10 +25,11 @@ import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.onap.policy.distributed.locking.DistributedLockingFeature;
+import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.OperResult;
 import org.onap.policy.drools.persistence.SystemPersistence;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -115,7 +116,7 @@ public class TargetLockTest {
 		}
 		
 		// Grab reference to heartbeat object
-		Heartbeat heartbeat = distLockFeat.getHeartbeat();
+		Heartbeat heartbeat = DistributedLockingFeature.getHeartbeat();
 
 		// Pass heartbeat object countdown latch
 		try {
@@ -149,16 +150,16 @@ public class TargetLockTest {
 	public void testUnlock() throws InterruptedException, ExecutionException {
 		distLockFeat.beforeLock("resource1", "owner1", null);
 
-		assertTrue(distLockFeat.beforeUnlock("resource1", "owner1"));
+		assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeUnlock("resource1", "owner1"));
 		assertTrue(distLockFeat.beforeLock("resource1", "owner1", null).get());
 	}
 	
 	@Test
 	public void testIsActive() {
-		assertFalse(distLockFeat.beforeIsLockedBy("resource1", "owner1"));
+		assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
 		distLockFeat.beforeLock("resource1", "owner1", null);
-		assertTrue(distLockFeat.beforeIsLockedBy("resource1", "owner1"));
-		assertFalse(distLockFeat.beforeIsLockedBy("resource1", "owner2"));
+		assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
+		assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner2"));
 
 		// isActive on expiredLock
 		try (PreparedStatement updateStatement = conn
@@ -172,12 +173,12 @@ public class TargetLockTest {
 			throw new RuntimeException(e);
 		}
 
-		assertFalse(distLockFeat.beforeIsLockedBy("resource1", "owner1"));
+		assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
 
 		distLockFeat.beforeLock("resource1", "owner1", null);
 			//Unlock record, next isActive attempt should fail
 		distLockFeat.beforeUnlock("resource1", "owner1");
-		assertFalse(distLockFeat.beforeIsLockedBy("resource1", "owner1"));
+		assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
 		
 	}
 	
@@ -198,7 +199,7 @@ public class TargetLockTest {
 		}
 		
 			//Grab reference to heartbeat object
-		Heartbeat heartbeat = distLockFeat.getHeartbeat();
+		Heartbeat heartbeat = DistributedLockingFeature.getHeartbeat();
 		
 			//Pass heartbeat object countdown latch
 		try {
@@ -210,22 +211,22 @@ public class TargetLockTest {
 			//At this point the heartbeat object should hve
 			//refreshed the lock. assert that resource1 is
 			//locked
-		assertTrue(distLockFeat.beforeIsLocked("resource1"));
+		assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeIsLocked("resource1"));
 	}
 	
 	@Test
 	public void unlockBeforeLock() {
-		assertFalse(distLockFeat.beforeUnlock("resource1", "owner1"));
+		assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeUnlock("resource1", "owner1"));
 		distLockFeat.beforeLock("resource1", "owner1", null);
-		assertTrue(distLockFeat.beforeUnlock("resource1", "owner1"));
-		assertFalse(distLockFeat.beforeUnlock("resource1", "owner1"));
+		assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeUnlock("resource1", "owner1"));
+		assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeUnlock("resource1", "owner1"));
 	}
 	
 	@Test
 	public void testIsLocked() {
-		assertFalse(distLockFeat.beforeIsLocked("resource1"));
+		assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLocked("resource1"));
 		distLockFeat.beforeLock("resource1", "owner1", null);
-		assertTrue(distLockFeat.beforeIsLocked("resource1"));
+		assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeIsLocked("resource1"));
 	
 	}
 	
diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java
index 46d1ff2d..a2e9e62e 100644
--- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java
+++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java
@@ -183,7 +183,7 @@ public class LockRequestFuture implements Future<Boolean> {
      * @return {@code true} if the lock was acquired, {@code false} if it was denied
      */
     @Override
-    public Boolean get() throws CancellationException, InterruptedException {
+    public Boolean get() throws InterruptedException {
         waiter.await();
 
         switch (state.get()) {
@@ -202,7 +202,7 @@ public class LockRequestFuture implements Future<Boolean> {
      */
     @Override
     public Boolean get(long timeout, TimeUnit unit)
-                    throws CancellationException, InterruptedException, TimeoutException {
+                    throws InterruptedException, TimeoutException {
 
         if (!waiter.await(timeout, unit)) {
             throw new TimeoutException("lock request did not complete in time");
diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java
index 718ed5e9..9f42936d 100644
--- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java
+++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java
@@ -46,9 +46,9 @@ public interface PolicyResourceLockFeatureAPI extends OrderedService {
                     new OrderedServiceImpl<>(PolicyResourceLockFeatureAPI.class);
 
     /**
-     * Callback that an implementer invokes when a lock is acquired (or denied),
-     * asynchronously. The implementer invokes the method to indicate that the lock was
-     * acquired (or denied).
+     * Callback that an implementer invokes, asynchronously, when a lock is acquired (or
+     * denied). The implementer invokes the method to indicate that the lock was acquired
+     * (or denied).
      */
     @FunctionalInterface
     public static interface Callback {
@@ -62,6 +62,31 @@ public interface PolicyResourceLockFeatureAPI extends OrderedService {
     }
 
     /**
+     * Result of a requested operation.
+     */
+    public static enum OperResult {
+
+        /**
+         * The implementer accepted the request; no additional locking logic should be
+         * performed.
+         */
+        OPER_ACCEPTED,
+
+        /**
+         * The implementer denied the request; no additional locking logic should be
+         * performed.
+         */
+        OPER_DENIED,
+
+
+        /**
+         * The implementer did not handle the request; additional locking logic <i>should
+         * be<i> performed.
+         */
+        OPER_UNHANDLED
+    }
+
+    /**
      * This method is called before a lock is acquired on a resource. If a callback is
      * provided, and the implementer is unable to acquire the lock immediately, then the
      * implementer will invoke the callback once the lock is acquired. If the implementer
@@ -112,21 +137,11 @@ public interface PolicyResourceLockFeatureAPI extends OrderedService {
      * 
      * @param resourceId
      * @param owner
-     *        <dt>true</dt>
-     *        <dd>the implementer handled the request and found the resource to be locked
-     *        by the given owner; the resource was unlocked and no additional locking
-     *        logic should be performed</dd>
-     *        <dt>false</dt>
-     *        <dd>the implementer handled the request and found the resource was not
-     *        locked by given the owner; no additional locking logic should be
-     *        performed</dd>
-     *        <dt>null</dt>
-     *        <dd>the implementer did not handle the request; additional locking logic
-     *        <i>should be</i> performed
-     *        </dl>
+     * @return the result, where <b>OPER_DENIED</b> indicates that the lock is not
+     *         currently held by the given owner
      */
-    public default Boolean beforeUnlock(String resourceId, String owner) {
-        return null;
+    public default OperResult beforeUnlock(String resourceId, String owner) {
+        return OperResult.OPER_UNHANDLED;
     }
 
     /**
@@ -147,21 +162,11 @@ public interface PolicyResourceLockFeatureAPI extends OrderedService {
      * This method is called before a check is made to determine if a resource is locked.
      * 
      * @param resourceId
-     * @return
-     *         <dl>
-     *         <dt>true</dt>
-     *         <dd>the implementer handled the request and found the resource to be
-     *         locked; no additional locking logic should be performed</dd>
-     *         <dt>false</dt>
-     *         <dd>the implementer handled the request and found the resource was not
-     *         locked; no additional locking logic should be performed</dd>
-     *         <dt>null</dt>
-     *         <dd>the implementer did not handle the request; additional locking logic
-     *         <i>should be</i> performed
-     *         </dl>
+     * @return the result, where <b>OPER_ACCEPTED</b> indicates that the resource is
+     *         locked, while <b>OPER_DENIED</b> indicates that it is not
      */
-    public default Boolean beforeIsLocked(String resourceId) {
-        return null;
+    public default OperResult beforeIsLocked(String resourceId) {
+        return OperResult.OPER_UNHANDLED;
     }
 
     /**
@@ -170,21 +175,11 @@ public interface PolicyResourceLockFeatureAPI extends OrderedService {
      * 
      * @param resourceId
      * @param owner
-     * @return
-     *         <dl>
-     *         <dt>true</dt>
-     *         <dd>the implementer handled the request and found the resource to be locked
-     *         by the given owner; no additional locking logic should be performed</dd>
-     *         <dt>false</dt>
-     *         <dd>the implementer handled the request and found the resource was not
-     *         locked by given the owner; no additional locking logic should be
-     *         performed</dd>
-     *         <dt>null</dt>
-     *         <dd>the implementer did not handle the request; additional locking logic
-     *         <i>should be</i> performed
-     *         </dl>
+     * @return the result, where <b>OPER_ACCEPTED</b> indicates that the resource is
+     *         locked by the given owner, while <b>OPER_DENIED</b> indicates that it is
+     *         not
      */
-    public default Boolean beforeIsLockedBy(String resourceId, String owner) {
-        return null;
+    public default OperResult beforeIsLockedBy(String resourceId, String owner) {
+        return OperResult.OPER_UNHANDLED;
     }
 }
diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java
index d51f2b91..a9305e51 100644
--- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java
+++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java
@@ -26,7 +26,9 @@ import static org.onap.policy.drools.core.lock.LockRequestFuture.makeNullArgExce
 import java.util.List;
 import java.util.concurrent.Future;
 import java.util.function.Function;
+import java.util.function.Supplier;
 import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.Callback;
+import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.OperResult;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -40,7 +42,7 @@ public class PolicyResourceLockManager extends SimpleLockManager {
     /**
      * Used to access various objects.
      */
-    public static Factory factory = new Factory();
+    private static Factory factory = new Factory();
 
     /**
      * Used by junit tests.
@@ -105,17 +107,16 @@ public class PolicyResourceLockManager extends SimpleLockManager {
             throw makeNullArgException(MSG_NULL_OWNER);
         }
 
-        Boolean result = doIntercept(null, impl -> impl.beforeUnlock(resourceId, owner));
-        if (result != null) {
-            return result;
-        }
 
-        // implementer didn't do the work - use superclass
-        boolean unlocked = super.unlock(resourceId, owner);
+        return doBoolIntercept(impl -> impl.beforeUnlock(resourceId, owner), () -> {
+
+            // implementer didn't do the work - defer to the superclass
+            boolean unlocked = super.unlock(resourceId, owner);
 
-        doIntercept(false, impl -> impl.afterUnlock(resourceId, owner, unlocked));
+            doIntercept(false, impl -> impl.afterUnlock(resourceId, owner, unlocked));
 
-        return unlocked;
+            return unlocked;
+        });
     }
 
     /**
@@ -128,12 +129,12 @@ public class PolicyResourceLockManager extends SimpleLockManager {
             throw makeNullArgException(MSG_NULL_RESOURCE_ID);
         }
 
-        Boolean result = doIntercept(null, impl -> impl.beforeIsLocked(resourceId));
-        if (result != null) {
-            return result;
-        }
 
-        return super.isLocked(resourceId);
+        return doBoolIntercept(impl -> impl.beforeIsLocked(resourceId), () -> {
+
+            // implementer didn't do the work - defer to the superclass
+            return super.isLocked(resourceId);
+        });
     }
 
     /**
@@ -150,12 +151,31 @@ public class PolicyResourceLockManager extends SimpleLockManager {
             throw makeNullArgException(MSG_NULL_OWNER);
         }
 
-        Boolean result = doIntercept(null, impl -> impl.beforeIsLockedBy(resourceId, owner));
-        if (result != null) {
-            return result;
+        return doBoolIntercept(impl -> impl.beforeIsLockedBy(resourceId, owner), () -> {
+
+            // implementer didn't do the work - defer to the superclass
+            return super.isLockedBy(resourceId, owner);
+        });
+    }
+
+    /**
+     * Applies a function to each implementer of the lock feature. Returns as soon as one
+     * of them returns a result other than <b>OPER_UNHANDLED</b>. If they all return
+     * <b>OPER_UNHANDLED</b>, then it returns the result of applying the default function.
+     * 
+     * @param interceptFunc
+     * @param defaultFunc
+     * @return {@code true} if success, {@code false} otherwise
+     */
+    private boolean doBoolIntercept(Function<PolicyResourceLockFeatureAPI, OperResult> interceptFunc,
+                    Supplier<Boolean> defaultFunc) {
+
+        OperResult result = doIntercept(OperResult.OPER_UNHANDLED, interceptFunc);
+        if (result != OperResult.OPER_UNHANDLED) {
+            return (result == OperResult.OPER_ACCEPTED);
         }
 
-        return super.isLockedBy(resourceId, owner);
+        return defaultFunc.get();
     }
 
     /**
@@ -168,7 +188,7 @@ public class PolicyResourceLockManager extends SimpleLockManager {
      * @return first non-null value returned by an implementer, <i>continueValue<i/> if
      *         they all returned <i>continueValue<i/>
      */
-    public static <T> T doIntercept(T continueValue, Function<PolicyResourceLockFeatureAPI, T> func) {
+    private static <T> T doIntercept(T continueValue, Function<PolicyResourceLockFeatureAPI, T> func) {
 
         for (PolicyResourceLockFeatureAPI impl : factory.getImplementers()) {
             try {
@@ -189,6 +209,14 @@ public class PolicyResourceLockManager extends SimpleLockManager {
      * Initialization-on-demand holder idiom.
      */
     private static class Singleton {
+
+        /**
+         * Not invoked.
+         */
+        private Singleton() {
+            super();
+        }
+
         private static final PolicyResourceLockManager instance = new PolicyResourceLockManager();
     }
 
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
index 883778eb..2dd90f00 100644
--- 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
@@ -80,13 +80,13 @@ public class LockRequestFutureTest {
     public void testLockRequestFutureStringStringBoolean_ArgEx() throws Exception {
 
         // null resource id
-        IllegalArgumentException ex = expectException(IllegalArgumentException.class,
-                        xxx -> new LockRequestFuture(null, OWNER, true));
+        IllegalArgumentException ex =
+                        expectException(IllegalArgumentException.class, () -> new LockRequestFuture(null, OWNER, true));
         assertEquals("null resourceId", ex.getMessage());
 
 
         // null owner
-        ex = expectException(IllegalArgumentException.class, xxx -> new LockRequestFuture(RESOURCE, null, true));
+        ex = expectException(IllegalArgumentException.class, () -> new LockRequestFuture(RESOURCE, null, true));
         assertEquals("null owner", ex.getMessage());
     }
 
@@ -108,12 +108,12 @@ public class LockRequestFutureTest {
 
         // null resource id
         IllegalArgumentException ex = expectException(IllegalArgumentException.class,
-                        xxx -> new LockRequestFuture(null, OWNER, callback));
+                        () -> new LockRequestFuture(null, OWNER, callback));
         assertEquals("null resourceId", ex.getMessage());
 
 
         // null owner
-        ex = expectException(IllegalArgumentException.class, xxx -> new LockRequestFuture(RESOURCE, null, callback));
+        ex = expectException(IllegalArgumentException.class, () -> new LockRequestFuture(RESOURCE, null, callback));
         assertEquals("null owner", ex.getMessage());
 
 
@@ -141,7 +141,7 @@ public class LockRequestFutureTest {
         assertTrue(fut.isDone());
 
         // should not block now
-        expectException(CancellationException.class, xxx -> fut.get(0, TimeUnit.SECONDS));
+        expectException(CancellationException.class, () -> fut.get(0, TimeUnit.SECONDS));
 
     }
 
@@ -153,7 +153,7 @@ public class LockRequestFutureTest {
         assertFalse(fut.cancel(true));
         assertTrue(fut.isDone());
 
-        expectException(CancellationException.class, xxx -> fut.get(0, TimeUnit.SECONDS));
+        expectException(CancellationException.class, () -> fut.get(0, TimeUnit.SECONDS));
     }
 
     @Test
@@ -201,7 +201,7 @@ public class LockRequestFutureTest {
         assertFalse(fut.setLocked(false));
 
         assertTrue(fut.isDone());
-        expectException(CancellationException.class, xxx -> fut.get(0, TimeUnit.SECONDS));
+        expectException(CancellationException.class, () -> fut.get(0, TimeUnit.SECONDS));
     }
 
     @Test
@@ -296,43 +296,25 @@ public class LockRequestFutureTest {
 
     @Test
     public void testGet_Cancelled() throws Exception {
-        new Thread() {
-            @Override
-            public void run() {
-                fut.cancel(false);
-            }
-        }.start();
-
-        expectException(CancellationException.class, xxx -> fut.get());
+        new Thread(() -> fut.cancel(false)).start();
+        expectException(CancellationException.class, () -> fut.get());
     }
 
     @Test
     public void testGet_Acquired() throws Exception {
-        new Thread() {
-            @Override
-            public void run() {
-                fut.setLocked(true);
-            }
-        }.start();
-
+        new Thread(() -> fut.setLocked(true)).start();
         assertTrue(fut.get());
     }
 
     @Test
     public void testGet_Denied() throws Exception {
-        new Thread() {
-            @Override
-            public void run() {
-                fut.setLocked(false);
-            }
-        }.start();
-
+        new Thread(() -> fut.setLocked(false)).start();
         assertFalse(fut.get());
     }
 
     @Test
     public void testGetLongTimeUnit() throws Exception {
-        expectException(TimeoutException.class, xxx -> fut.get(0, TimeUnit.SECONDS));
+        expectException(TimeoutException.class, () -> fut.get(0, TimeUnit.SECONDS));
 
         fut.setLocked(true);
         assertTrue(fut.get(0, TimeUnit.SECONDS));
@@ -340,43 +322,25 @@ public class LockRequestFutureTest {
 
     @Test
     public void testGetLongTimeUnit_Timeout() throws Exception {
-        expectException(TimeoutException.class, xxx -> fut.get(0, TimeUnit.SECONDS));
-        expectException(TimeoutException.class, xxx -> fut.get(2, TimeUnit.MILLISECONDS));
+        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() {
-            @Override
-            public void run() {
-                fut.cancel(false);
-            }
-        }.start();
-
-        expectException(CancellationException.class, xxx -> fut.get(WAIT_SEC, TimeUnit.SECONDS));
+        new Thread(() -> fut.cancel(false)).start();
+        expectException(CancellationException.class, () -> fut.get(WAIT_SEC, TimeUnit.SECONDS));
     }
 
     @Test
     public void testGetLongTimeUnit_Acquired() throws Exception {
-        new Thread() {
-            @Override
-            public void run() {
-                fut.setLocked(true);
-            }
-        }.start();
-
+        new Thread(() -> fut.setLocked(true)).start();
         assertTrue(fut.get(WAIT_SEC, TimeUnit.SECONDS));
     }
 
     @Test
     public void testGetLongTimeUnit_Denied() throws Exception {
-        new Thread() {
-            @Override
-            public void run() {
-                fut.setLocked(false);
-            }
-        }.start();
-
+        new Thread(() -> fut.setLocked(false)).start();
         assertFalse(fut.get(WAIT_SEC, TimeUnit.SECONDS));
     }
 
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 5a826bb7..57adc847 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
@@ -20,10 +20,12 @@
 
 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;
 
 public class PolicyResourceLockFeatureAPITest {
 
@@ -55,7 +57,7 @@ public class PolicyResourceLockFeatureAPITest {
 
     @Test
     public void testBeforeUnlock() {
-        assertNull(api.beforeUnlock(RESOURCE_ID, OWNER));
+        assertEquals(OperResult.OPER_UNHANDLED, api.beforeUnlock(RESOURCE_ID, OWNER));
     }
 
     @Test
@@ -66,11 +68,11 @@ public class PolicyResourceLockFeatureAPITest {
 
     @Test
     public void testBeforeIsLocked() {
-        assertNull(api.beforeIsLocked(RESOURCE_ID));
+        assertEquals(OperResult.OPER_UNHANDLED, api.beforeIsLocked(RESOURCE_ID));
     }
 
     @Test
     public void testBeforeIsLockedBy() {
-        assertNull(api.beforeIsLockedBy(RESOURCE_ID, OWNER));
+        assertEquals(OperResult.OPER_UNHANDLED, api.beforeIsLockedBy(RESOURCE_ID, OWNER));
     }
 }
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 2f8b5f83..fe883acf 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
@@ -42,6 +42,7 @@ 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 {
@@ -111,9 +112,9 @@ public class PolicyResourceLockManagerTest {
      */
     private void initImplementer(PolicyResourceLockFeatureAPI impl) {
         when(impl.beforeLock(anyString(), anyString(), any(Callback.class))).thenReturn(null);
-        when(impl.beforeUnlock(anyString(), anyString())).thenReturn(null);
-        when(impl.beforeIsLocked(anyString())).thenReturn(null);
-        when(impl.beforeIsLockedBy(anyString(), anyString())).thenReturn(null);
+        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);
     }
 
     @Test
@@ -147,10 +148,10 @@ public class PolicyResourceLockManagerTest {
     @Test
     public void testLock_ArgEx() {
         IllegalArgumentException ex =
-                        expectException(IllegalArgumentException.class, xxx -> mgr.lock(null, OWNER1, callback1));
+                        expectException(IllegalArgumentException.class, () -> mgr.lock(null, OWNER1, callback1));
         assertEquals(NULL_RESOURCE_ID, ex.getMessage());
 
-        ex = expectException(IllegalArgumentException.class, xxx -> mgr.lock(RESOURCE_A, null, callback1));
+        ex = expectException(IllegalArgumentException.class, () -> mgr.lock(RESOURCE_A, null, callback1));
         assertEquals(NULL_OWNER, ex.getMessage());
 
         // this should not throw an exception
@@ -250,10 +251,10 @@ public class PolicyResourceLockManagerTest {
 
     @Test
     public void testUnlock_ArgEx() {
-        IllegalArgumentException ex = expectException(IllegalArgumentException.class, xxx -> mgr.unlock(null, OWNER1));
+        IllegalArgumentException ex = expectException(IllegalArgumentException.class, () -> mgr.unlock(null, OWNER1));
         assertEquals(NULL_RESOURCE_ID, ex.getMessage());
 
-        ex = expectException(IllegalArgumentException.class, xxx -> mgr.unlock(RESOURCE_A, null));
+        ex = expectException(IllegalArgumentException.class, () -> mgr.unlock(RESOURCE_A, null));
         assertEquals(NULL_OWNER, ex.getMessage());
     }
 
@@ -263,7 +264,7 @@ public class PolicyResourceLockManagerTest {
         mgr.lock(RESOURCE_A, OWNER1, null);
 
         // have impl1 intercept
-        when(impl1.beforeUnlock(RESOURCE_A, OWNER1)).thenReturn(true);
+        when(impl1.beforeUnlock(RESOURCE_A, OWNER1)).thenReturn(OperResult.OPER_ACCEPTED);
 
         assertTrue(mgr.unlock(RESOURCE_A, OWNER1));
 
@@ -280,7 +281,7 @@ public class PolicyResourceLockManagerTest {
         mgr.lock(RESOURCE_A, OWNER1, null);
 
         // have impl1 intercept
-        when(impl1.beforeUnlock(RESOURCE_A, OWNER1)).thenReturn(false);
+        when(impl1.beforeUnlock(RESOURCE_A, OWNER1)).thenReturn(OperResult.OPER_DENIED);
 
         assertFalse(mgr.unlock(RESOURCE_A, OWNER1));
 
@@ -365,7 +366,7 @@ public class PolicyResourceLockManagerTest {
 
     @Test
     public void testIsLocked_ArgEx() {
-        IllegalArgumentException ex = expectException(IllegalArgumentException.class, xxx -> mgr.isLocked(null));
+        IllegalArgumentException ex = expectException(IllegalArgumentException.class, () -> mgr.isLocked(null));
         assertEquals(NULL_RESOURCE_ID, ex.getMessage());
     }
 
@@ -373,7 +374,7 @@ public class PolicyResourceLockManagerTest {
     public void testIsLocked_BeforeIntercepted_True() {
 
         // have impl1 intercept
-        when(impl1.beforeIsLocked(RESOURCE_A)).thenReturn(true);
+        when(impl1.beforeIsLocked(RESOURCE_A)).thenReturn(OperResult.OPER_ACCEPTED);;
 
         assertTrue(mgr.isLocked(RESOURCE_A));
 
@@ -388,7 +389,7 @@ public class PolicyResourceLockManagerTest {
         mgr.lock(RESOURCE_A, OWNER1, null);
 
         // have impl1 intercept
-        when(impl1.beforeIsLocked(RESOURCE_A)).thenReturn(false);
+        when(impl1.beforeIsLocked(RESOURCE_A)).thenReturn(OperResult.OPER_DENIED);
 
         assertFalse(mgr.isLocked(RESOURCE_A));
 
@@ -420,10 +421,10 @@ public class PolicyResourceLockManagerTest {
     @Test
     public void testIsLockedBy_ArgEx() {
         IllegalArgumentException ex =
-                        expectException(IllegalArgumentException.class, xxx -> mgr.isLockedBy(null, OWNER1));
+                        expectException(IllegalArgumentException.class, () -> mgr.isLockedBy(null, OWNER1));
         assertEquals(NULL_RESOURCE_ID, ex.getMessage());
 
-        ex = expectException(IllegalArgumentException.class, xxx -> mgr.isLockedBy(RESOURCE_A, null));
+        ex = expectException(IllegalArgumentException.class, () -> mgr.isLockedBy(RESOURCE_A, null));
         assertEquals(NULL_OWNER, ex.getMessage());
     }
 
@@ -431,7 +432,7 @@ public class PolicyResourceLockManagerTest {
     public void testIsLockedBy_BeforeIntercepted_True() {
 
         // have impl1 intercept
-        when(impl1.beforeIsLockedBy(RESOURCE_A, OWNER1)).thenReturn(true);
+        when(impl1.beforeIsLockedBy(RESOURCE_A, OWNER1)).thenReturn(OperResult.OPER_ACCEPTED);;
 
         assertTrue(mgr.isLockedBy(RESOURCE_A, OWNER1));
 
@@ -446,7 +447,7 @@ public class PolicyResourceLockManagerTest {
         mgr.lock(RESOURCE_A, OWNER1, null);
 
         // have impl1 intercept
-        when(impl1.beforeIsLockedBy(RESOURCE_A, OWNER1)).thenReturn(false);
+        when(impl1.beforeIsLockedBy(RESOURCE_A, OWNER1)).thenReturn(OperResult.OPER_DENIED);
 
         assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER1));
 
@@ -479,7 +480,7 @@ public class PolicyResourceLockManagerTest {
 
     @Test
     public void testDoIntercept_Impl1() {
-        when(impl1.beforeIsLocked(RESOURCE_A)).thenReturn(true);
+        when(impl1.beforeIsLocked(RESOURCE_A)).thenReturn(OperResult.OPER_ACCEPTED);;
 
         assertTrue(mgr.isLocked(RESOURCE_A));
 
@@ -489,7 +490,7 @@ public class PolicyResourceLockManagerTest {
 
     @Test
     public void testDoIntercept_Impl2() {
-        when(impl2.beforeIsLocked(RESOURCE_A)).thenReturn(true);
+        when(impl2.beforeIsLocked(RESOURCE_A)).thenReturn(OperResult.OPER_ACCEPTED);;
 
         assertTrue(mgr.isLocked(RESOURCE_A));
 
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 6abc5bf9..833e1c7d 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
@@ -98,10 +98,10 @@ public class SimpleLockManagerTest {
     @Test
     public void testLock_ArgEx() {
         IllegalArgumentException ex =
-                        expectException(IllegalArgumentException.class, xxx -> mgr.lock(null, OWNER1, null));
+                        expectException(IllegalArgumentException.class, () -> mgr.lock(null, OWNER1, null));
         assertEquals(NULL_RESOURCE_ID, ex.getMessage());
 
-        ex = expectException(IllegalArgumentException.class, xxx -> mgr.lock(RESOURCE_A, null, null));
+        ex = expectException(IllegalArgumentException.class, () -> mgr.lock(RESOURCE_A, null, null));
         assertEquals(NULL_OWNER, ex.getMessage());
 
         // this should not throw an exception
@@ -118,10 +118,10 @@ public class SimpleLockManagerTest {
 
     @Test
     public void testUnlock_ArgEx() {
-        IllegalArgumentException ex = expectException(IllegalArgumentException.class, xxx -> mgr.unlock(null, OWNER1));
+        IllegalArgumentException ex = expectException(IllegalArgumentException.class, () -> mgr.unlock(null, OWNER1));
         assertEquals(NULL_RESOURCE_ID, ex.getMessage());
 
-        ex = expectException(IllegalArgumentException.class, xxx -> mgr.unlock(RESOURCE_A, null));
+        ex = expectException(IllegalArgumentException.class, () -> mgr.unlock(RESOURCE_A, null));
         assertEquals(NULL_OWNER, ex.getMessage());
     }
 
@@ -156,7 +156,7 @@ public class SimpleLockManagerTest {
 
     @Test
     public void testIsLocked_ArgEx() {
-        IllegalArgumentException ex = expectException(IllegalArgumentException.class, xxx -> mgr.isLocked(null));
+        IllegalArgumentException ex = expectException(IllegalArgumentException.class, () -> mgr.isLocked(null));
         assertEquals(NULL_RESOURCE_ID, ex.getMessage());
     }
 
@@ -181,10 +181,10 @@ public class SimpleLockManagerTest {
     @Test
     public void testIsLockedBy_ArgEx() {
         IllegalArgumentException ex =
-                        expectException(IllegalArgumentException.class, xxx -> mgr.isLockedBy(null, OWNER1));
+                        expectException(IllegalArgumentException.class, () -> mgr.isLockedBy(null, OWNER1));
         assertEquals(NULL_RESOURCE_ID, ex.getMessage());
 
-        ex = expectException(IllegalArgumentException.class, xxx -> mgr.isLockedBy(RESOURCE_A, null));
+        ex = expectException(IllegalArgumentException.class, () -> mgr.isLockedBy(RESOURCE_A, null));
         assertEquals(NULL_OWNER, ex.getMessage());
     }
 
@@ -221,34 +221,31 @@ public class SimpleLockManagerTest {
         for (int x = 0; x < nthreads; ++x) {
             String owner = "owner." + x;
 
-            Thread t = new Thread() {
-                @Override
-                public void run() {
+            Thread t = new Thread(() -> {
 
-                    for (int y = 0; y < nlocks; ++y) {
-                        String res = resources[y % resources.length];
+                for (int y = 0; y < nlocks; ++y) {
+                    String res = resources[y % resources.length];
 
-                        try {
-                            // some locks will be acquired, some denied
-                            mgr.lock(res, owner, null).get();
+                    try {
+                        // some locks will be acquired, some denied
+                        mgr.lock(res, owner, null).get();
 
-                            // do some "work"
-                            stopper.await(1L, TimeUnit.MILLISECONDS);
+                        // do some "work"
+                        stopper.await(1L, TimeUnit.MILLISECONDS);
 
-                            mgr.unlock(res, owner);
+                        mgr.unlock(res, owner);
 
-                        } catch (CancellationException | ExecutionException e) {
-                            nfail.incrementAndGet();
+                    } catch (CancellationException | ExecutionException e) {
+                        nfail.incrementAndGet();
 
-                        } catch (InterruptedException expected) {
-                            Thread.currentThread().interrupt();
-                            break;
-                        }
+                    } catch (InterruptedException expected) {
+                        Thread.currentThread().interrupt();
+                        break;
                     }
-
-                    completed.countDown();
                 }
-            };
+
+                completed.countDown();
+            });
 
             t.setDaemon(true);
             threads.add(t);
diff --git a/policy-core/src/test/java/org/onap/policy/drools/core/lock/TestUtils.java b/policy-core/src/test/java/org/onap/policy/drools/core/lock/TestUtils.java
index 2e353936..f843f6ab 100644
--- a/policy-core/src/test/java/org/onap/policy/drools/core/lock/TestUtils.java
+++ b/policy-core/src/test/java/org/onap/policy/drools/core/lock/TestUtils.java
@@ -31,7 +31,7 @@ public class TestUtils {
      */
     public static <T> T expectException(Class<T> clazz, VoidFunction func) {
         try {
-            func.apply(null);
+            func.apply();
             throw new AssertionError("missing exception");
 
         } catch (Exception e) {
@@ -50,10 +50,6 @@ public class TestUtils {
     @FunctionalInterface
     public static interface VoidFunction {
 
-        /**
-         * 
-         * @param arg always {@code null}
-         */
-        public void apply(Void arg) throws Exception;
+        public void apply() throws Exception;
     }
 }