Sonar fixes for policy-core locks 61/42561/3
authorJim Hahn <jrh3@att.com>
Thu, 12 Apr 2018 18:23:25 +0000 (14:23 -0400)
committerJim Hahn <jrh3@att.com>
Thu, 12 Apr 2018 18:43:20 +0000 (14:43 -0400)
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>
feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java
feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java
policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java
policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java
policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java
policy-core/src/test/java/org/onap/policy/drools/core/lock/LockRequestFutureTest.java
policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPITest.java
policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockManagerTest.java
policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java
policy-core/src/test/java/org/onap/policy/drools/core/lock/TestUtils.java

index 7906dba..5994beb 100644 (file)
@@ -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
index 2dc2ceb..b01d967 100644 (file)
@@ -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"));
        
        }
        
index 46d1ff2..a2e9e62 100644 (file)
@@ -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");
index 718ed5e..9f42936 100644 (file)
@@ -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 {
@@ -61,6 +61,31 @@ public interface PolicyResourceLockFeatureAPI extends OrderedService {
         public void set(boolean locked);
     }
 
+    /**
+     * 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
@@ -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;
     }
 }
index d51f2b9..a9305e5 100644 (file)
@@ -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();
     }
 
index 883778e..2dd90f0 100644 (file)
@@ -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));
     }
 
index 5a826bb..57adc84 100644 (file)
 
 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));
     }
 }
index 2f8b5f8..fe883ac 100644 (file)
@@ -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));
 
index 6abc5bf..833e1c7 100644 (file)
@@ -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);
index 2e35393..f843f6a 100644 (file)
@@ -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;
     }
 }