Distributed-locking feature sonar cleanup

Code cleanup and some refactoring to resolve sonar issues.

Issue-ID: POLICY-728
Change-Id: I19051dcc8e1cec293d5d7104e63239fc2b89071e
Signed-off-by: Magnusen, Drew (dm741q) <dm741q@att.com>

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 3d19c87..7906dba 100644 (file)
--- 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
@@ -79,7 +79,7 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy
        @Override
        public Future<Boolean> beforeLock(String resourceId, String owner, Callback callback) {
                
-               TargetLock tLock = new TargetLock(resourceId, this.uuid, owner, lockProps);
+               TargetLock tLock = new TargetLock(resourceId, uuid, owner, lockProps);
                
                return new LockRequestFuture(resourceId, owner, tLock.lock());
                                
@@ -87,21 +87,21 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy
 
        @Override
        public Boolean beforeUnlock(String resourceId, String owner) {
-               TargetLock tLock = new TargetLock(resourceId, this.uuid, owner, lockProps);
+               TargetLock tLock = new TargetLock(resourceId, uuid, owner, lockProps);
                
                return tLock.unlock();
        }
        
        @Override
        public Boolean beforeIsLockedBy(String resourceId, String owner) {
-               TargetLock tLock = new TargetLock(resourceId, this.uuid, owner, lockProps);
+               TargetLock tLock = new TargetLock(resourceId, uuid, owner, lockProps);
                
                return tLock.isActive();
        }
        
        @Override
        public Boolean beforeIsLocked(String resourceId) {
-               TargetLock tLock = new TargetLock(resourceId, this.uuid, "dummyOwner", lockProps);
+               TargetLock tLock = new TargetLock(resourceId, uuid, "dummyOwner", lockProps);
                
                return tLock.isLocked();
        }
@@ -119,7 +119,7 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy
                long heartbeatInterval = this.lockProps.getHeartBeatIntervalProperty();
                
                cleanLockTable();
-               heartbeat = new Heartbeat(this.uuid, lockProps);
+               heartbeat = new Heartbeat(uuid, lockProps);
                
                this.scheduledExecutorService = Executors.newScheduledThreadPool(1);
                this.scheduledExecutorService.scheduleAtFixedRate(heartbeat, heartbeatInterval, heartbeatInterval, TimeUnit.MILLISECONDS);
@@ -148,7 +148,7 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy
                PreparedStatement statement = conn.prepareStatement("DELETE FROM pooling.locks WHERE host = ? OR expirationTime < ?");
                ){
                        
-                               statement.setString(1, this.uuid.toString());
+                               statement.setString(1, uuid.toString());
                                statement.setLong(2, System.currentTimeMillis());
                                statement.executeUpdate();
                        

diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingProperties.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingProperties.java
index 139bfb7..97ba3b1 100644 (file)
--- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingProperties.java
+++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingProperties.java
@@ -23,14 +23,10 @@ import java.util.Properties;
 
 import org.onap.policy.common.utils.properties.PropertyConfiguration;
 import org.onap.policy.common.utils.properties.exception.PropertyException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 
 public class DistributedLockingProperties extends PropertyConfiguration{
        
-       private static final Logger  logger = LoggerFactory.getLogger(DistributedLockingProperties.class);
-       
        /**
      * Feature properties all begin with this prefix.
      */

diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java
index ceaa849..4f09dc2 100644 (file)
--- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java
+++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java
@@ -88,19 +88,21 @@ public class TargetLock {
         */
        private boolean grabLock() {
 
+               // try to insert a record into the table(thereby grabbing the lock)
                try (Connection conn = DriverManager.getConnection(lockProps.getDbUrl(), lockProps.getDbUser(),
                                lockProps.getDbPwd());
 
-                               // try to insert a record into the table(thereby grabbing the lock)
-                               PreparedStatement statement = conn
-                                               .prepareStatement("INSERT INTO pooling.locks (resourceId, host, owner, expirationTime) values (?, ?, ?, ?)");) {
+                               PreparedStatement statement = conn.prepareStatement(
+                                               "INSERT INTO pooling.locks (resourceId, host, owner, expirationTime) values (?, ?, ?, ?)")) {
+                       
                        statement.setString(1, this.resourceId);
                        statement.setString(2, this.uuid.toString());
                        statement.setString(3, this.owner);
                        statement.setLong(4, System.currentTimeMillis() + lockProps.getAgingProperty());
-
                        statement.executeUpdate();
-               }  catch (SQLException e) {
+               }
+
+               catch (SQLException e) {
                        logger.error("error in TargetLock.grabLock()", e);
                        return secondGrab();
                }
@@ -117,9 +119,11 @@ public class TargetLock {
                try (Connection conn = DriverManager.getConnection(lockProps.getDbUrl(), lockProps.getDbUser(),
                                lockProps.getDbPwd());
 
-                               PreparedStatement updateStatement = conn.prepareStatement("UPDATE pooling.locks SET host = ?, owner = ?, expirationTime = ? WHERE expirationTime <= ? AND resourceId = ?");
-                               
-                               PreparedStatement insertStatement = conn.prepareStatement("INSERT INTO pooling.locks (resourceId, host, owner, expirationTime) values (?, ?, ?, ?)");) {
+                               PreparedStatement updateStatement = conn.prepareStatement(
+                                               "UPDATE pooling.locks SET host = ?, owner = ?, expirationTime = ? WHERE expirationTime <= ? AND resourceId = ?");
+
+                               PreparedStatement insertStatement = conn.prepareStatement(
+                                               "INSERT INTO pooling.locks (resourceId, host, owner, expirationTime) values (?, ?, ?, ?)");) {
 
                        updateStatement.setString(1, this.uuid.toString());
                        updateStatement.setString(2, this.owner);
@@ -132,6 +136,7 @@ public class TargetLock {
                        if (updateStatement.executeUpdate() == 1) {
                                return true;
                        }
+
                        // If our update does not return 1 row, the lock either has not expired
                        // or it was removed. Try one last grab
                        else {
@@ -140,7 +145,7 @@ public class TargetLock {
                                insertStatement.setString(3, this.owner);
                                insertStatement.setLong(4, System.currentTimeMillis() + lockProps.getAgingProperty());
 
-                                       // If our insert returns 1 we successfully grabbed the lock
+                               // If our insert returns 1 we successfully grabbed the lock
                                return (insertStatement.executeUpdate() == 1);
                        }
 
@@ -159,13 +164,13 @@ public class TargetLock {
                try (Connection conn = DriverManager.getConnection(lockProps.getDbUrl(), lockProps.getDbUser(),
                                lockProps.getDbPwd());
 
-                               PreparedStatement deleteStatement = conn
-                                               .prepareStatement("DELETE FROM pooling.locks WHERE resourceId = ? AND owner = ? AND host = ?");) {
+                               PreparedStatement deleteStatement = conn.prepareStatement(
+                                               "DELETE FROM pooling.locks WHERE resourceId = ? AND owner = ? AND host = ?")) {
 
                        deleteStatement.setString(1, this.resourceId);
                        deleteStatement.setString(2, this.owner);
                        deleteStatement.setString(3, this.uuid.toString());
-                       
+
                        return (deleteStatement.executeUpdate() == 1);
 
                } catch (SQLException e) {
@@ -174,34 +179,35 @@ public class TargetLock {
                }
 
        }
-       
+
        /**
         * Is the lock active
         */
        public boolean isActive() {
-
                try (Connection conn = DriverManager.getConnection(lockProps.getDbUrl(), lockProps.getDbUser(),
                                lockProps.getDbPwd());
 
-                               PreparedStatement selectStatement = conn
-                                               .prepareStatement("SELECT * FROM pooling.locks WHERE resourceId = ? AND host = ? AND owner= ? AND expirationTime >= ?");) {
-                       {
-                               selectStatement.setString(1, this.resourceId);
-                               selectStatement.setString(2, this.uuid.toString());
-                               selectStatement.setString(3, this.owner);
-                               selectStatement.setLong(4, System.currentTimeMillis());
+                               PreparedStatement selectStatement = conn.prepareStatement(
+                                               "SELECT * FROM pooling.locks WHERE resourceId = ? AND host = ? AND owner= ? AND expirationTime >= ?")) {
 
-                               ResultSet result = selectStatement.executeQuery();
+                       selectStatement.setString(1, this.resourceId);
+                       selectStatement.setString(2, this.uuid.toString());
+                       selectStatement.setString(3, this.owner);
+                       selectStatement.setLong(4, System.currentTimeMillis());
+                       try (ResultSet result = selectStatement.executeQuery()) {
 
                                // This will return true if the
                                // query returned at least one row
                                return result.first();
-
                        }
-               } catch (SQLException e) {
+
+               }
+
+               catch (SQLException e) {
                        logger.error("error in TargetLock.isActive()", e);
                        return false;
                }
+
        }
 
        /**
@@ -211,20 +217,20 @@ public class TargetLock {
 
                try (Connection conn = DriverManager.getConnection(lockProps.getDbUrl(), lockProps.getDbUser(),
                                lockProps.getDbPwd());
-
+                       
                                PreparedStatement selectStatement = conn
-                                               .prepareStatement("SELECT * FROM pooling.locks WHERE resourceId = ? AND expirationTime >= ?");) {
-                       {
-                               selectStatement.setString(1, this.resourceId);
-                               selectStatement.setLong(2, System.currentTimeMillis());
-                               ResultSet result = selectStatement.executeQuery();
+                                               .prepareStatement("SELECT * FROM pooling.locks WHERE resourceId = ? AND expirationTime >= ?")) {
 
+                       selectStatement.setString(1, this.resourceId);
+                       selectStatement.setLong(2, System.currentTimeMillis());
+                       try (ResultSet result = selectStatement.executeQuery()) {
                                // This will return true if the
                                // query returned at least one row
                                return result.first();
-
                        }
-               } catch (SQLException e) {
+               }
+
+               catch (SQLException e) {
                        logger.error("error in TargetLock.isActive()", e);
                        return false;
                }