From 0fe7bd5eb21ca5a8daef531ade9156bb5c2e0207 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Wed, 29 Nov 2017 17:01:15 -0500 Subject: 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 --- .../policy/drools/statemanagement/DbAudit.java | 172 +++++++++++++-------- .../statemanagement/DroolsPDPIntegrityMonitor.java | 76 ++++----- .../drools/statemanagement/RepositoryAudit.java | 80 +++++----- .../statemanagement/StateManagementFeature.java | 12 +- .../StateManagementPropertiesException.java | 3 + .../statemanagement/test/StateManagementTest.java | 14 +- 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); } @@ -210,20 +226,6 @@ public class DroolsPDPIntegrityMonitor extends IntegrityMonitor return im; } - /** - * 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) */ @@ -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 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() - { - @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 { + @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(); -- cgit 1.2.3-korg