Add junit coverage to drools-pdp

Added coverage to distributed locking.
Added coverage to test-transaction - still more to do.
Added coverage to healthcheck - still more to do.
Also fixed "code smell" in pooling.
Also fixed "code smell" in session persistence.
Fixed typo in comment.
Removed unneeded setUp() method from test.
Fixed new checkstyle errors.
Fixed another new checkstyle error.

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

4 files changed, 223 insertions, 13 deletions

diff --git a/feature-distributed-locking/pom.xml b/feature-distributed-locking/pom.xml
index a7eeed6d..0c61e34c 100644
--- a/feature-distributed-locking/pom.xml
+++ b/feature-distributed-locking/pom.xml
@@ -119,6 +119,11 @@
             <scope>test</scope>
         </dependency>
         <dependency>
+            <groupId>org.powermock</groupId>
+            <artifactId>powermock-api-mockito</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
             <groupId>com.h2database</groupId>
             <artifactId>h2</artifactId>
             <scope>test</scope>
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 86b5a66c..ddfe6a5a 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
@@ -134,7 +134,7 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy
      * @return a new, pooled data source
      * @throws Exception exception
      */
-    private BasicDataSource makeDataSource() throws Exception {
+    protected BasicDataSource makeDataSource() throws Exception {
         Properties props = new Properties();
         props.put("driverClassName", lockProps.getDbDriver());
         props.put("url", lockProps.getDbUrl());
diff --git a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockingFeatureTest.java b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockingFeatureTest.java
new file mode 100644
index 00000000..90e8bfc0
--- /dev/null
+++ b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockingFeatureTest.java
@@ -0,0 +1,107 @@
+/*
+ * ============LICENSE_START=======================================================
+ * ONAP
+ * ================================================================================
+ * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved.
+ * ================================================================================
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ============LICENSE_END=========================================================
+ */
+
+package org.onap.policy.distributed.locking;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.sql.SQLException;
+import org.apache.commons.dbcp2.BasicDataSource;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.onap.policy.common.utils.properties.exception.PropertyException;
+import org.onap.policy.drools.persistence.SystemPersistence;
+
+/**
+ * Partially tests DistributedLockingFeature; most of the methods are tested via
+ * {@link TargetLockTest}.
+ */
+public class DistributedLockingFeatureTest {
+    private static final String EXPECTED = "expected exception";
+
+    private BasicDataSource dataSrc;
+
+    @BeforeClass
+    public static void setUpBeforeClass() throws Exception {
+        SystemPersistence.manager.setConfigurationDir("src/test/resources");
+    }
+
+    @Before
+    public void setUp() throws Exception {
+        dataSrc = mock(BasicDataSource.class);
+    }
+
+    @Test
+    public void testGetSequenceNumber() {
+        assertEquals(1000, new DistributedLockingFeature().getSequenceNumber());
+    }
+
+    @Test(expected = DistributedLockingFeatureException.class)
+    public void testAfterStart_PropEx() {
+        new DistributedLockingFeatureImpl(new PropertyException("prop", "val")).afterStart(null);
+    }
+
+    @Test(expected = DistributedLockingFeatureException.class)
+    public void testAfterStart_InterruptEx() {
+        new DistributedLockingFeatureImpl(new InterruptedException(EXPECTED)).afterStart(null);
+    }
+
+    @Test(expected = DistributedLockingFeatureException.class)
+    public void testAfterStart_OtherEx() {
+        new DistributedLockingFeatureImpl(new RuntimeException(EXPECTED)).afterStart(null);
+    }
+
+    @Test
+    public void testCleanLockTable() throws Exception {
+        when(dataSrc.getConnection()).thenThrow(new SQLException(EXPECTED));
+
+        new DistributedLockingFeatureImpl().afterStart(null);
+    }
+
+    /**
+     * Feature that overrides {@link #makeDataSource()}.
+     */
+    private class DistributedLockingFeatureImpl extends DistributedLockingFeature {
+        /**
+         * Exception to throw when {@link #makeDataSource()} is invoked.
+         */
+        private final Exception makeEx;
+
+        public DistributedLockingFeatureImpl() {
+            makeEx = null;
+        }
+
+        public DistributedLockingFeatureImpl(Exception ex) {
+            this.makeEx = ex;
+        }
+
+        @Override
+        protected BasicDataSource makeDataSource() throws Exception {
+            if (makeEx != null) {
+                throw makeEx;
+            }
+
+            return dataSrc;
+        }
+    }
+}
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 49f1026d..6fa1febb 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
@@ -21,12 +21,19 @@
 package org.onap.policy.distributed.locking;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
+import java.util.UUID;
 import java.util.concurrent.ExecutionException;
+import org.apache.commons.dbcp2.BasicDataSource;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
@@ -43,6 +50,10 @@ public class TargetLockTest {
             "jdbc:h2:mem:pooling;INIT=CREATE SCHEMA IF NOT EXISTS pooling\\;SET SCHEMA pooling";
     private static final String DB_USER = "user";
     private static final String DB_PASSWORD = "password";
+    private static final String EXPECTED = "expected exception";
+    private static final String MY_RESOURCE = "my-resource-id";
+    private static final String MY_OWNER = "my-owner";
+    private static final UUID MY_UUID = UUID.randomUUID();
     private static Connection conn = null;
     private static DistributedLockingFeature distLockFeat;
 
@@ -139,7 +150,64 @@ public class TargetLockTest {
     }
 
     @Test
-    public void testUpdateLock() throws InterruptedException, ExecutionException {
+    public void testSecondGrab_UpdateOk() throws Exception {
+        PreparedStatement grabLockInsert = mock(PreparedStatement.class);
+        when(grabLockInsert.executeUpdate()).thenThrow(new SQLException(EXPECTED));
+
+        PreparedStatement secondGrabUpdate = mock(PreparedStatement.class);
+        when(secondGrabUpdate.executeUpdate()).thenReturn(1);
+
+        Connection connMock = mock(Connection.class);
+        when(connMock.prepareStatement(anyString())).thenReturn(grabLockInsert, secondGrabUpdate);
+
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenReturn(connMock);
+
+        assertTrue(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).lock(MAX_AGE_SEC));
+    }
+
+    @Test
+    public void testSecondGrab_UpdateFail_InsertOk() throws Exception {
+        PreparedStatement grabLockInsert = mock(PreparedStatement.class);
+        when(grabLockInsert.executeUpdate()).thenThrow(new SQLException(EXPECTED));
+
+        PreparedStatement secondGrabUpdate = mock(PreparedStatement.class);
+        when(secondGrabUpdate.executeUpdate()).thenReturn(0);
+
+        PreparedStatement secondGrabInsert = mock(PreparedStatement.class);
+        when(secondGrabInsert.executeUpdate()).thenReturn(1);
+
+        Connection connMock = mock(Connection.class);
+        when(connMock.prepareStatement(anyString())).thenReturn(grabLockInsert, secondGrabUpdate, secondGrabInsert);
+
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenReturn(connMock);
+
+        assertTrue(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).lock(MAX_AGE_SEC));
+    }
+
+    @Test
+    public void testSecondGrab_UpdateFail_InsertFail() throws Exception {
+        PreparedStatement grabLockInsert = mock(PreparedStatement.class);
+        when(grabLockInsert.executeUpdate()).thenThrow(new SQLException(EXPECTED));
+
+        PreparedStatement secondGrabUpdate = mock(PreparedStatement.class);
+        when(secondGrabUpdate.executeUpdate()).thenReturn(0);
+
+        PreparedStatement secondGrabInsert = mock(PreparedStatement.class);
+        when(secondGrabInsert.executeUpdate()).thenReturn(0);
+
+        Connection connMock = mock(Connection.class);
+        when(connMock.prepareStatement(anyString())).thenReturn(grabLockInsert, secondGrabUpdate, secondGrabInsert);
+
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenReturn(connMock);
+
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).lock(MAX_AGE_SEC));
+    }
+
+    @Test
+    public void testUpdateLock() throws Exception {
         // not locked yet - refresh should fail
         assertEquals(
                 OperResult.OPER_DENIED, distLockFeat.beforeRefresh("resource1", "owner1", MAX_AGE_SEC));
@@ -160,10 +228,6 @@ public class TargetLockTest {
                   "UPDATE pooling.locks SET expirationTime = timestampadd(second, -1, now()) WHERE resourceId = ?"); ) {
             updateStatement.setString(1, "resource1");
             updateStatement.executeUpdate();
-
-        } catch (SQLException e) {
-            logger.error("Error in TargetLockTest.testGrabLockSuccess()", e);
-            throw new RuntimeException(e);
         }
 
         // refresh should fail now
@@ -171,19 +235,29 @@ public class TargetLockTest {
                 OperResult.OPER_DENIED, distLockFeat.beforeRefresh("resource1", "owner1", MAX_AGE_SEC));
 
         assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
+
+        // test exception case
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenThrow(new SQLException(EXPECTED));
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).refresh(MAX_AGE_SEC));
     }
 
     @Test
-    public void testUnlock() throws InterruptedException, ExecutionException {
+    public void testUnlock() throws Exception {
         distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
 
         assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeUnlock("resource1", "owner1"));
         assertEquals(
                 OperResult.OPER_ACCEPTED, distLockFeat.beforeLock("resource1", "owner2", MAX_AGE_SEC));
+
+        // test exception case
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenThrow(new SQLException(EXPECTED));
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).unlock());
     }
 
     @Test
-    public void testIsActive() {
+    public void testIsActive() throws Exception {
         assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
         distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
         assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
@@ -195,10 +269,6 @@ public class TargetLockTest {
                   "UPDATE pooling.locks SET expirationTime = timestampadd(second, -5, now()) WHERE resourceId = ?"); ) {
             updateStatement.setString(1, "resource1");
             updateStatement.executeUpdate();
-
-        } catch (SQLException e) {
-            logger.error("Error in TargetLockTest.testIsActive()", e);
-            throw new RuntimeException(e);
         }
 
         assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
@@ -207,6 +277,20 @@ public class TargetLockTest {
         // Unlock record, next isActive attempt should fail
         distLockFeat.beforeUnlock("resource1", "owner1");
         assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
+
+        // test exception case for outer "try"
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenThrow(new SQLException(EXPECTED));
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).isActive());
+
+        // test exception case for inner "try"
+        PreparedStatement stmt = mock(PreparedStatement.class);
+        when(stmt.executeQuery()).thenThrow(new SQLException(EXPECTED));
+        Connection connMock = mock(Connection.class);
+        when(connMock.prepareStatement(anyString())).thenReturn(stmt);
+        dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenReturn(connMock);
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).isActive());
     }
 
     @Test
@@ -218,10 +302,24 @@ public class TargetLockTest {
     }
 
     @Test
-    public void testIsLocked() {
+    public void testIsLocked() throws Exception {
         assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLocked("resource1"));
         distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
         assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeIsLocked("resource1"));
+
+        // test exception case for outer "try"
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenThrow(new SQLException(EXPECTED));
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).isLocked());
+
+        // test exception case for inner "try"
+        PreparedStatement stmt = mock(PreparedStatement.class);
+        when(stmt.executeQuery()).thenThrow(new SQLException(EXPECTED));
+        Connection connMock = mock(Connection.class);
+        when(connMock.prepareStatement(anyString())).thenReturn(stmt);
+        dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenReturn(connMock);
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).isLocked());
     }
 
     private static void getDbConnection() {