Modified state mgmt to fix some sonar issues

Reordered modifiers.
Reordered variables, methods, and constructors.
Removed useless parentheses.
Removed unneeded "catch" clauses.
Extracted nested try blocks into their own method.
Replaced a string with a constant.
Removed extra thrown exceptions when they are unnecessary (i.e., they're
subclasses of RuntimeException, or the method is already declared to
throw an Exception).
Replaced a large anonymous class with a named, nested class.
Separated variable declarations onto individual lines.
Changed "String args[]" to "String[] args".
Replaced if-then-else by single return statement.
Invoked super() inside empty, default constructor.
Removed Thread.sleep() calls from junit test per comments on 11/29.

Commented out Thread.sleep() in junit tests, as they don't appear to
be necessary.  If that turns out to be untrue, then CountdownLatch.await()
can be used instead.

Sonar complained about useless assignments to "phase", but those did
not appear to be useless.
Did not remove commented-out lines, as they may be needed when debugging.

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

6 files changed, 198 insertions, 159 deletions

diff --git a/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/DbAudit.java b/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/DbAudit.java
index b2830e8c..67991c7b 100644
--- a/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/DbAudit.java
+++ b/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/DbAudit.java
@@ -24,6 +24,7 @@ import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
+import java.sql.SQLException;
 import java.util.Properties;
 import java.util.UUID;
 
@@ -38,19 +39,27 @@ public class DbAudit extends DroolsPDPIntegrityMonitor.AuditBase
 	// get an instance of logger 
   private static Logger  logger = LoggerFactory.getLogger(DbAudit.class);	
   // single global instance of this audit object
-  final static private DbAudit instance = new DbAudit();
+  private static final DbAudit instance = new DbAudit();
 
   // This indicates if 'CREATE TABLE IF NOT EXISTS Audit ...' should be
   // invoked -- doing this avoids the need to create the table in advance.
-  static private boolean createTableNeeded = true;
+  private static boolean createTableNeeded = true;
   
-  synchronized private static void setCreateTableNeeded(boolean b) {
-		DbAudit.createTableNeeded = b;
+  private static boolean isJunit = false;
+
+  /**
+   * Constructor - set the name to 'Database'
+   */
+  private DbAudit()
+  {
+	super("Database");
   }
   
-  static private boolean isJunit = false;
+  private static synchronized void setCreateTableNeeded(boolean b) {
+		DbAudit.createTableNeeded = b;
+  }
   
-  synchronized public static void setIsJunit(boolean b) {
+  public static synchronized void setIsJunit(boolean b) {
 		DbAudit.isJunit = b;
   }
   
@@ -64,15 +73,7 @@ public class DbAudit extends DroolsPDPIntegrityMonitor.AuditBase
    */
   public static DroolsPDPIntegrityMonitor.AuditBase getInstance()
   {
-	return(instance);
-  }
-
-  /**
-   * Constructor - set the name to 'Database'
-   */
-  private DbAudit()
-  {
-	super("Database");
+	return instance;
   }
 
   /**
@@ -132,64 +133,19 @@ public class DbAudit extends DroolsPDPIntegrityMonitor.AuditBase
 		if (createTableNeeded)
 		  {
 			phase = "create table";
-			if(logger.isDebugEnabled()){
-				logger.info("DbAudit: Creating 'Audit' table, if needed");
-			}
-			try (PreparedStatement statement = connection.prepareStatement
-			  ("CREATE TABLE IF NOT EXISTS Audit (\n"
-			   + " name varchar(64) DEFAULT NULL,\n"
-			   + " UNIQUE KEY name (name)\n"
-			   + ") DEFAULT CHARSET=latin1;")) {
-				statement.execute();
-				DbAudit.setCreateTableNeeded(false);
-			} catch (Exception e) {
-				throw e;
-			}
+			createTable(connection);
 		  }
 
 		// insert an entry into the table
 		phase = "insert entry";
 		String key = UUID.randomUUID().toString();
-		try (PreparedStatement statement = connection.prepareStatement
-		  ("INSERT INTO Audit (name) VALUES (?)")) {
-			statement.setString(1, key);
-			statement.executeUpdate();
-		} catch (Exception e) {
-			throw e;
-		}
+		insertEntry(connection, key);
 		
-		// fetch the entry from the table
 		phase = "fetch entry";
-		try (PreparedStatement statement = connection.prepareStatement
-		  ("SELECT name FROM Audit WHERE name = ?")) {
-			statement.setString(1, key);
-			try (ResultSet rs = statement.executeQuery()) {
-				if (rs.first())
-				  {
-					// found entry
-					if(logger.isDebugEnabled()){
-						logger.debug("DbAudit: Found key {}", rs.getString(1));
-					}
-				  }
-				else
-				  {
-					logger.error
-					  ("DbAudit: can't find newly-created entry with key {}", key);
-					setResponse("Can't find newly-created entry");
-				  }
-			} catch (Exception e) {
-				throw e;
-			}
-		}
-		// delete entries from table
+		findEntry(connection, key);
+		
 		phase = "delete entry";
-		try (PreparedStatement statement = connection.prepareStatement
-		  ("DELETE FROM Audit WHERE name = ?")) {
-			statement.setString(1, key);
-			statement.executeUpdate();
-		} catch (Exception e) {
-			throw e;
-		}
+		deleteEntry(connection, key);
 	}
 	catch (Exception e)
 	  {
@@ -199,4 +155,90 @@ public class DbAudit extends DroolsPDPIntegrityMonitor.AuditBase
 	  }
   }
 
+  /**
+   * Creates the table.
+   * @param connection
+   * @throws SQLException
+   */
+  private void createTable(Connection connection) throws SQLException {
+	if(logger.isDebugEnabled()){
+		logger.info("DbAudit: Creating 'Audit' table, if needed");
+	}
+	try (PreparedStatement statement = connection.prepareStatement
+	  ("CREATE TABLE IF NOT EXISTS Audit (\n"
+	   + " name varchar(64) DEFAULT NULL,\n"
+	   + " UNIQUE KEY name (name)\n"
+	   + ") DEFAULT CHARSET=latin1;")) {
+		statement.execute();
+		DbAudit.setCreateTableNeeded(false);
+	}
+  }
+
+  /**
+   * Inserts an entry.
+   * @param connection
+   * @param key
+   * @throws SQLException
+   */
+  private void insertEntry(Connection connection, String key) throws SQLException {
+	try (PreparedStatement statement = connection.prepareStatement
+	  ("INSERT INTO Audit (name) VALUES (?)")) {
+		statement.setString(1, key);
+		statement.executeUpdate();
+	}
+  }
+
+  /**
+   * Finds an entry.
+   * @param connection
+   * @param key
+   * @throws SQLException
+   */
+  private void findEntry(Connection connection, String key) throws SQLException {
+	try (PreparedStatement statement = connection.prepareStatement
+	  ("SELECT name FROM Audit WHERE name = ?")) {
+		statement.setString(1, key);
+		getEntry(statement, key);
+	}
+  }
+
+  /**
+   * Executes the query to determine if the entry exists.  Sets the response
+   * if it fails.
+   * @param statement
+   * @param key
+   * @throws SQLException
+   */
+  private void getEntry(PreparedStatement statement, String key) throws SQLException {
+	try (ResultSet rs = statement.executeQuery()) {
+		if (rs.first())
+		  {
+			// found entry
+			if(logger.isDebugEnabled()){
+				logger.debug("DbAudit: Found key {}", rs.getString(1));
+			}
+		  }
+		else
+		  {
+			logger.error
+			  ("DbAudit: can't find newly-created entry with key {}", key);
+			setResponse("Can't find newly-created entry");
+		  }
+	}
+  }
+
+  /**
+   * Deletes an entry.
+   * @param connection
+   * @param key
+   * @throws SQLException
+   */
+  private void deleteEntry(Connection connection, String key) throws SQLException {
+	try (PreparedStatement statement = connection.prepareStatement
+	  ("DELETE FROM Audit WHERE name = ?")) {
+		statement.setString(1, key);
+		statement.executeUpdate();
+	}
+  }
+
 }
diff --git a/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/DroolsPDPIntegrityMonitor.java b/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/DroolsPDPIntegrityMonitor.java
index 915a3225..f599e3dc 100644
--- a/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/DroolsPDPIntegrityMonitor.java
+++ b/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/DroolsPDPIntegrityMonitor.java
@@ -40,27 +40,43 @@ import org.slf4j.LoggerFactory;
 public class DroolsPDPIntegrityMonitor extends IntegrityMonitor
 {
 	
-	// get an instance of logger 
+	private static final String INVALID_PROPERTY_VALUE = "init: property {} does not have the expected value of {}";
+
+// get an instance of logger 
   private static final Logger  logger = LoggerFactory.getLogger(DroolsPDPIntegrityMonitor.class);	
 
   // static global instance
-  static private DroolsPDPIntegrityMonitor im = null;
+  private static DroolsPDPIntegrityMonitor im = null;
 
   // list of audits to run
-  static private AuditBase[] audits =
+  private static AuditBase[] audits =
 	new AuditBase[]{DbAudit.getInstance(), RepositoryAudit.getInstance()};
   
-  static private Properties subsystemTestProperties = null;
+  private static Properties subsystemTestProperties = null;
+
+  private static final String PROPERTIES_NAME = "feature-state-management.properties";
 
-  static private final String PROPERTIES_NAME = "feature-state-management.properties";
+  /**
+   * Constructor - pass arguments to superclass, but remember properties
+   * @param resourceName unique name of this Integrity Monitor
+   * @param url the JMX URL of the MBean server
+   * @param properties properties used locally, as well as by
+   *	'IntegrityMonitor'
+   * @throws Exception (passed from superclass)
+   */
+	private DroolsPDPIntegrityMonitor(String resourceName,
+			Properties consolidatedProperties
+			) throws Exception {
+	super(resourceName, consolidatedProperties);
+  }
   
-  static private void missingProperty(String prop) throws StateManagementPropertiesException{
+  private static void missingProperty(String prop) throws StateManagementPropertiesException{
 		String msg = "init: missing IntegrityMonitor property: ".concat(prop);
 		logger.error(msg);
 		throw new StateManagementPropertiesException(msg);
   }
   
-  static private void logPropertyValue(String prop, String val){
+  private static void logPropertyValue(String prop, String val){
 	  if(logger.isInfoEnabled()){
 		  String msg = "\n\n    init: property: " + prop + " = " + val + "\n";
 		  logger.info(msg);
@@ -70,7 +86,7 @@ public class DroolsPDPIntegrityMonitor extends IntegrityMonitor
    * Static initialization -- create Drools Integrity Monitor, and
    * an HTTP server to handle REST 'test' requests
    */
-  static public DroolsPDPIntegrityMonitor init(String configDir) throws Exception
+  public static DroolsPDPIntegrityMonitor init(String configDir) throws Exception
   {
 	  	  
 	logger.info("init: Entering and invoking PropertyUtil.getProperties() on '{}'", configDir);
@@ -110,22 +126,22 @@ public class DroolsPDPIntegrityMonitor extends IntegrityMonitor
 		missingProperty(StateManagementProperties.TEST_PORT);
 	}
 	if (!testServices.equals(StateManagementProperties.TEST_SERVICES_DEFAULT)){
-		logger.error("init: property {} does not have the expected value of {}",
+		logger.error(INVALID_PROPERTY_VALUE,
 				StateManagementProperties.TEST_SERVICES,
 				StateManagementProperties.TEST_SERVICES_DEFAULT);
 	}
 	if (!testRestClasses.equals(StateManagementProperties.TEST_REST_CLASSES_DEFAULT)){
-		logger.error("init: property {} does not have the expected value of {}",
+		logger.error(INVALID_PROPERTY_VALUE,
 				StateManagementProperties.TEST_REST_CLASSES,
 				StateManagementProperties.TEST_REST_CLASSES_DEFAULT);
 	}
 	if (!testManaged.equals(StateManagementProperties.TEST_MANAGED_DEFAULT)){
-		logger.warn("init: property {} does not have the expected value of {}",
+		logger.warn(INVALID_PROPERTY_VALUE,
 				StateManagementProperties.TEST_MANAGED,
 				StateManagementProperties.TEST_MANAGED_DEFAULT);
 	}
 	if (!testSwagger.equals(StateManagementProperties.TEST_SWAGGER_DEFAULT)){
-		logger.warn("init: property {} does not have the expected value of {}",
+		logger.warn(INVALID_PROPERTY_VALUE,
 				StateManagementProperties.TEST_SWAGGER,
 				StateManagementProperties.TEST_SWAGGER_DEFAULT);
 	}
@@ -211,20 +227,6 @@ public class DroolsPDPIntegrityMonitor extends IntegrityMonitor
   }
 
   /**
-   * Constructor - pass arguments to superclass, but remember properties
-   * @param resourceName unique name of this Integrity Monitor
-   * @param url the JMX URL of the MBean server
-   * @param properties properties used locally, as well as by
-   *	'IntegrityMonitor'
-   * @throws Exception (passed from superclass)
-   */
-	private DroolsPDPIntegrityMonitor(String resourceName,
-			Properties consolidatedProperties
-			) throws Exception {
-	super(resourceName, consolidatedProperties);
-  }
-
-  /**
    * Run tests (audits) unique to Drools PDP VM (Database + Repository)
    */
   @Override
@@ -284,7 +286,7 @@ public class DroolsPDPIntegrityMonitor extends IntegrityMonitor
   /**
    * This is the base class for audits invoked in 'subsystemTest'
    */
-  static public abstract class AuditBase
+  public abstract static class AuditBase
   {
 	// name of the audit
 	protected String name;
@@ -345,18 +347,14 @@ public class DroolsPDPIntegrityMonitor extends IntegrityMonitor
   		}
   		
   		@Override
-		public boolean start() throws IllegalStateException {
+		public boolean start() {
 			try {
 				ArrayList<HttpServletServer> servers = HttpServletServer.factory.build(integrityMonitorRestServerProperties);
 				
 				if (!servers.isEmpty()) {
 					server = servers.get(0);
 					
-					try {
-						server.waitedStart(5);
-					} catch (Exception e) {
-						logger.error("Exception waiting for servers to start: ", e);
-					}
+					waitServerStart();
 				}
 			} catch (Exception e) {
 				logger.error("Exception building servers", e);
@@ -366,8 +364,16 @@ public class DroolsPDPIntegrityMonitor extends IntegrityMonitor
 			return true;
 		}
 
+		private void waitServerStart() {
+			try {
+				server.waitedStart(5);
+			} catch (Exception e) {
+				logger.error("Exception waiting for servers to start: ", e);
+			}
+		}
+
   		@Override
-		public boolean stop() throws IllegalStateException {
+		public boolean stop() {
 			try {
 				server.stop();
 			} catch (Exception e) {
@@ -378,7 +384,7 @@ public class DroolsPDPIntegrityMonitor extends IntegrityMonitor
 		}
 
 		@Override
-		public void shutdown() throws IllegalStateException {
+		public void shutdown() {
 			this.stop();
 		}
 		
diff --git a/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/RepositoryAudit.java b/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/RepositoryAudit.java
index b36c1657..1e2a3a05 100644
--- a/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/RepositoryAudit.java
+++ b/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/RepositoryAudit.java
@@ -46,22 +46,22 @@ public class RepositoryAudit extends DroolsPDPIntegrityMonitor.AuditBase
   // get an instance of logger
   private static Logger  logger = LoggerFactory.getLogger(RepositoryAudit.class);		
   // single global instance of this audit object
-  static private RepositoryAudit instance = new RepositoryAudit();
+  private static RepositoryAudit instance = new RepositoryAudit();
 
   /**
-   * @return the single 'RepositoryAudit' instance
+   * Constructor - set the name to 'Repository'
    */
-  public static DroolsPDPIntegrityMonitor.AuditBase getInstance()
+  private RepositoryAudit()
   {
-	return instance;
+	super("Repository");
   }
 
   /**
-   * Constructor - set the name to 'Repository'
+   * @return the single 'RepositoryAudit' instance
    */
-  private RepositoryAudit()
+  public static DroolsPDPIntegrityMonitor.AuditBase getInstance()
   {
-	super("Repository");
+	return instance;
   }
 
   /**
@@ -433,34 +433,7 @@ public class RepositoryAudit extends DroolsPDPIntegrityMonitor.AuditBase
 	/*
 	 * 7) Remove the temporary directory
 	 */
-	Files.walkFileTree
-	  (dir,
-	   new SimpleFileVisitor<Path>()
-	   {
-		 @Override
-		 public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
-		   {
-			 // logger.info("RepositoryAudit: Delete " + file);
-			 file.toFile().delete();
-			 return FileVisitResult.CONTINUE;
-		   }
-
-		 @Override
-		 public FileVisitResult postVisitDirectory(Path file, IOException e)
-		   throws IOException
-		   {
-			 if (e == null)
-			   {
-				 // logger.info("RepositoryAudit: Delete " + file);
-				 file.toFile().delete();
-				 return FileVisitResult.CONTINUE;
-			   }
-			 else
-			   {
-				 throw e;
-			   }
-		   }
-	   });
+	Files.walkFileTree(dir, new RecursivelyDeleteDirectory());
   }
 
   /**
@@ -502,7 +475,37 @@ public class RepositoryAudit extends DroolsPDPIntegrityMonitor.AuditBase
 	return -1;
   }
 
-  /* ============================================================ */
+  /**
+   * This class is used to recursively delete a directory and all of its
+   * contents.
+   */
+  private final class RecursivelyDeleteDirectory extends SimpleFileVisitor<Path> {
+	@Override
+	 public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
+	   {
+		 // logger.info("RepositoryAudit: Delete " + file);
+		 file.toFile().delete();
+		 return FileVisitResult.CONTINUE;
+	   }
+
+	@Override
+	 public FileVisitResult postVisitDirectory(Path file, IOException e)
+	   throws IOException
+	   {
+		 if (e == null)
+		   {
+			 // logger.info("RepositoryAudit: Delete " + file);
+			 file.toFile().delete();
+			 return FileVisitResult.CONTINUE;
+		   }
+		 else
+		   {
+			 throw e;
+		   }
+	   }
+  }
+
+/* ============================================================ */
 
   /**
    * An instance of this class exists for each artifact that we are trying
@@ -510,7 +513,10 @@ public class RepositoryAudit extends DroolsPDPIntegrityMonitor.AuditBase
    */
   static class Artifact
   {
-	String groupId, artifactId, version, type;
+	String groupId;
+	String artifactId;
+	String version;
+	String type;
 
 	/**
 	 * Constructor - populate the 'Artifact' instance
diff --git a/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementFeature.java b/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementFeature.java
index 0143c58b..66d806be 100644
--- a/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementFeature.java
+++ b/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementFeature.java
@@ -24,7 +24,6 @@ import java.io.IOException;
 import java.util.Observer;
 import java.util.Properties;
 
-import org.onap.policy.common.im.StandbyStatusException;
 import org.onap.policy.common.im.StateManagement;
 import org.onap.policy.drools.core.PolicySessionFeatureAPI;
 import org.onap.policy.drools.features.PolicyEngineFeatureAPI;
@@ -64,7 +63,7 @@ public class StateManagementFeature implements StateManagementFeatureAPI,
 	}
 	
 	@Override
-	public void globalInit(String args[], String configDir)
+	public void globalInit(String[] args, String configDir)
 	{
 		// Initialization code associated with 'PolicyContainer'
 		if(logger.isDebugEnabled()){
@@ -185,7 +184,7 @@ public class StateManagementFeature implements StateManagementFeatureAPI,
 	 * {@inheritDoc}
 	 */
 	@Override
-	public void promote() throws StandbyStatusException, Exception {
+	public void promote() throws Exception {
 		stateManagement.promote();		
 	}
 
@@ -241,12 +240,7 @@ public class StateManagementFeature implements StateManagementFeatureAPI,
 	 */
 	@Override
 	public boolean isLocked(){
-		String admin = stateManagement.getAdminState();
-		if(admin.equals(StateManagement.LOCKED)){
-			return true;
-		}else{
-			return false;
-		}
+		return StateManagement.LOCKED.equals(stateManagement.getAdminState());
 	}
 	
 	@Override
diff --git a/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementPropertiesException.java b/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementPropertiesException.java
index 51145cd6..59802ebd 100644
--- a/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementPropertiesException.java
+++ b/feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementPropertiesException.java
@@ -22,8 +22,11 @@ package org.onap.policy.drools.statemanagement;
 
 public class StateManagementPropertiesException extends Exception{
 	private static final long serialVersionUID = 1L;
+	
 	public StateManagementPropertiesException() {
+		super();
 	}
+	
 	public StateManagementPropertiesException(String message) {
 		super(message);
 	}
diff --git a/feature-state-management/src/test/java/org/onap/policy/drools/statemanagement/test/StateManagementTest.java b/feature-state-management/src/test/java/org/onap/policy/drools/statemanagement/test/StateManagementTest.java
index 6f2a0e25..85e0ed85 100644
--- a/feature-state-management/src/test/java/org/onap/policy/drools/statemanagement/test/StateManagementTest.java
+++ b/feature-state-management/src/test/java/org/onap/policy/drools/statemanagement/test/StateManagementTest.java
@@ -53,12 +53,6 @@ public class StateManagementTest {
 	// get an instance of logger 
 	private static Logger  logger = LoggerFactory.getLogger(StateManagementTest.class);	
 	
-	/*
-	 * Sleep after each test to allow interrupt (shutdown) recovery.
-	 */
-	 
-	private long interruptRecoveryTime = 1500L;
-	
 	StateManagementFeatureAPI stateManagementFeature;
 	
 	/*
@@ -134,8 +128,6 @@ public class StateManagementTest {
 			logger.debug(msg);
 		}
 		
-		Thread.sleep(interruptRecoveryTime);
-		
 		String admin = stateManagementFeature.getAdminState();
 		String oper = stateManagementFeature.getOpState();
 		String avail = stateManagementFeature.getAvailStatus();
@@ -155,9 +147,7 @@ public class StateManagementTest {
 			logger.error(e.getMessage());
 			assertTrue(e.getMessage(), false);
 		}
-		
-		Thread.sleep(interruptRecoveryTime);
-		
+				
 		admin = stateManagementFeature.getAdminState();
 		oper = stateManagementFeature.getOpState();
 		avail = stateManagementFeature.getAvailStatus();
@@ -179,8 +169,6 @@ public class StateManagementTest {
 			logger.debug(e.getMessage());
 		}
 		
-		Thread.sleep(interruptRecoveryTime);
-		
 		admin = stateManagementFeature.getAdminState();
 		oper = stateManagementFeature.getOpState();
 		avail = stateManagementFeature.getAvailStatus();