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>

2 files changed, 24 insertions, 23 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"));
 	
 	}