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>

3 files changed, 90 insertions, 67 deletions

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();
     }