From 4401819b97be13c244dfe206256b59d13c89f7a6 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Wed, 31 Jan 2018 17:49:08 -0500 Subject: [PATCH] Fix sonar issues in feature-state-management DroolsPDPIntegrityMonitor.java: Modified init() to throw just IntegrityMonitorException. Modified DroolsPDPIntegrityMonitor init() method to throw specific types of exceptions. StateManagementFeature.java: Sonar complained about needing to merge "if" statements, but chose to eliminate the "if(logger.isDebugEnabled())" instead - did this through-out the source file. Removed extra runtime exception from "throws" declaration. DbAudit.java: Fixed sonar issue regarding setting a static variable from within a non-static method. Removed logger.isDebugEnabled() tests where method calls are not involed. Simplified invoke() method complexity as reported by sonar. DroolsPDPIntegrityMonitor.java: Reduced init() complexity reported by sonar. Change-Id: Ib2722b21bbf3aad130af46c8790f40d8777e36be Issue-ID: POLICY-469 Signed-off-by: Jim Hahn --- .../policy/drools/statemanagement/DbAudit.java | 54 ++-- .../statemanagement/DroolsPDPIntegrityMonitor.java | 281 +++++++++++---------- .../statemanagement/StateManagementFeature.java | 46 ++-- .../statemanagement/test/StateManagementTest.java | 1 + 4 files changed, 198 insertions(+), 184 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 67991c7b..33f672c5 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 @@ -84,29 +84,11 @@ public class DbAudit extends DroolsPDPIntegrityMonitor.AuditBase @Override public void invoke(Properties properties) { - if(logger.isDebugEnabled()){ - logger.debug("Running 'DbAudit.invoke'"); - } - if(isJunit){ - createTableNeeded = false; - } - boolean isActive = true; - String dbAuditIsActive = StateManagementProperties.getProperty("db.audit.is.active"); - if(logger.isDebugEnabled()){ - logger.debug("DbAudit.invoke: dbAuditIsActive = {}", dbAuditIsActive); - } - - if (dbAuditIsActive != null) { - try { - isActive = Boolean.parseBoolean(dbAuditIsActive.trim()); - } catch (NumberFormatException e) { - logger.warn("DbAudit.invoke: Ignoring invalid property: db.audit.is.active = {}", dbAuditIsActive); - } - } + logger.debug("Running 'DbAudit.invoke'"); + boolean doCreate = createTableNeeded && !isJunit; - if(!isActive){ - - logger.info("DbAudit.invoke: exiting because isActive = {}", isActive); + if(!isActive()){ + logger.info("DbAudit.invoke: exiting because isActive = false"); return; } @@ -123,14 +105,12 @@ public class DbAudit extends DroolsPDPIntegrityMonitor.AuditBase // create connection to DB phase = "creating connection"; - if(logger.isDebugEnabled()){ - logger.debug("DbAudit: Creating connection to {}", url); - } + logger.debug("DbAudit: Creating connection to {}", url); try (Connection connection = DriverManager.getConnection(url, user, password)) { // create audit table, if needed - if (createTableNeeded) + if (doCreate) { phase = "create table"; createTable(connection); @@ -155,15 +135,33 @@ public class DbAudit extends DroolsPDPIntegrityMonitor.AuditBase } } + /** + * Determines if the DbAudit is active, based on properties. Defaults to + * {@code true}, if not found in the properties. + * @return {@code true} if DbAudit is active, {@code false} otherwise + */ + private boolean isActive() { + String dbAuditIsActive = StateManagementProperties.getProperty("db.audit.is.active"); + logger.debug("DbAudit.invoke: dbAuditIsActive = {}", dbAuditIsActive); + + if (dbAuditIsActive != null) { + try { + return Boolean.parseBoolean(dbAuditIsActive.trim()); + } catch (NumberFormatException e) { + logger.warn("DbAudit.invoke: Ignoring invalid property: db.audit.is.active = {}", dbAuditIsActive); + } + } + + return true; + } + /** * 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" 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 0a4eb513..382f01e2 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 @@ -20,6 +20,7 @@ package org.onap.policy.drools.statemanagement; +import java.io.IOException; import java.util.ArrayList; import java.util.Properties; import org.onap.policy.common.im.IntegrityMonitor; @@ -67,10 +68,10 @@ public class DroolsPDPIntegrityMonitor extends IntegrityMonitor super(resourceName, consolidatedProperties); } - private static void missingProperty(String prop) throws StateManagementPropertiesException{ + private static void missingProperty(String prop) throws IntegrityMonitorException{ String msg = "init: missing IntegrityMonitor property: ".concat(prop); logger.error(msg); - throw new StateManagementPropertiesException(msg); + throw new IntegrityMonitorException(msg); } private static void logPropertyValue(String prop, String val){ @@ -83,156 +84,182 @@ public class DroolsPDPIntegrityMonitor extends IntegrityMonitor /** * Static initialization -- create Drools Integrity Monitor, and * an HTTP server to handle REST 'test' requests + * @throws StateManagementPropertiesException + * @throws IntegrityMonitorException */ - public static DroolsPDPIntegrityMonitor init(String configDir) throws Exception + public static DroolsPDPIntegrityMonitor init(String configDir) throws IntegrityMonitorException { logger.info("init: Entering and invoking PropertyUtil.getProperties() on '{}'", configDir); // read in properties - Properties stateManagementProperties = - PropertyUtil.getProperties(configDir + "/" + PROPERTIES_NAME); - // fetch and verify definitions of some properties + Properties stateManagementProperties = getProperties(configDir); + + // fetch and verify definitions of some properties, adding defaults where + // appropriate // (the 'IntegrityMonitor' constructor does some additional verification) + + checkPropError(stateManagementProperties, StateManagementProperties.TEST_HOST); + checkPropError(stateManagementProperties, StateManagementProperties.TEST_PORT); + + addDefaultPropError(stateManagementProperties, + StateManagementProperties.TEST_SERVICES, + StateManagementProperties.TEST_SERVICES_DEFAULT); + + addDefaultPropError(stateManagementProperties, + StateManagementProperties.TEST_REST_CLASSES, + StateManagementProperties.TEST_REST_CLASSES_DEFAULT); + + addDefaultPropWarn(stateManagementProperties, + StateManagementProperties.TEST_MANAGED, + StateManagementProperties.TEST_MANAGED_DEFAULT); + + addDefaultPropWarn(stateManagementProperties, + StateManagementProperties.TEST_SWAGGER, + StateManagementProperties.TEST_SWAGGER_DEFAULT); + + checkPropError(stateManagementProperties, StateManagementProperties.RESOURCE_NAME); + checkPropError(stateManagementProperties, StateManagementProperties.FP_MONITOR_INTERVAL); + checkPropError(stateManagementProperties, StateManagementProperties.FAILED_COUNTER_THRESHOLD); + checkPropError(stateManagementProperties, StateManagementProperties.TEST_TRANS_INTERVAL); + checkPropError(stateManagementProperties, StateManagementProperties.WRITE_FPC_INTERVAL); + checkPropError(stateManagementProperties, StateManagementProperties.SITE_NAME); + checkPropError(stateManagementProperties, StateManagementProperties.NODE_TYPE); + checkPropError(stateManagementProperties, StateManagementProperties.DEPENDENCY_GROUPS); + checkPropError(stateManagementProperties, StateManagementProperties.DB_DRIVER); + checkPropError(stateManagementProperties, StateManagementProperties.DB_URL); + checkPropError(stateManagementProperties, StateManagementProperties.DB_USER); + checkPropError(stateManagementProperties, StateManagementProperties.DB_PWD); + String testHost = stateManagementProperties.getProperty(StateManagementProperties.TEST_HOST); String testPort = stateManagementProperties.getProperty(StateManagementProperties.TEST_PORT); - String testServices = stateManagementProperties.getProperty(StateManagementProperties.TEST_SERVICES); - String testRestClasses = stateManagementProperties.getProperty(StateManagementProperties.TEST_REST_CLASSES); - String testManaged = stateManagementProperties.getProperty(StateManagementProperties.TEST_MANAGED); - String testSwagger = stateManagementProperties.getProperty(StateManagementProperties.TEST_SWAGGER); String resourceName = stateManagementProperties.getProperty(StateManagementProperties.RESOURCE_NAME); - String fpMonitorInterval = stateManagementProperties.getProperty(StateManagementProperties.FP_MONITOR_INTERVAL); - String failedCounterThreshold = stateManagementProperties.getProperty(StateManagementProperties.FAILED_COUNTER_THRESHOLD); - String testTransInterval = stateManagementProperties.getProperty(StateManagementProperties.TEST_TRANS_INTERVAL); - String writeFpcInterval = stateManagementProperties.getProperty(StateManagementProperties.WRITE_FPC_INTERVAL); - String siteName = stateManagementProperties.getProperty(StateManagementProperties.SITE_NAME); - String nodeType = stateManagementProperties.getProperty(StateManagementProperties.NODE_TYPE); - String dependencyGroups = stateManagementProperties.getProperty(StateManagementProperties.DEPENDENCY_GROUPS); - String javaxPersistenceJdbcDriver = stateManagementProperties.getProperty(StateManagementProperties.DB_DRIVER); - String javaxPersistenceJdbcUrl = stateManagementProperties.getProperty(StateManagementProperties.DB_URL); - String javaxPersistenceJdbcUser = stateManagementProperties.getProperty(StateManagementProperties.DB_USER); - String javaxPersistenceJdbcPassword = stateManagementProperties.getProperty(StateManagementProperties.DB_PWD); - - if (testHost == null){ - missingProperty(StateManagementProperties.TEST_HOST); - } - if (testPort == null){ - missingProperty(StateManagementProperties.TEST_PORT); - } - if (testServices == null) { - testServices = StateManagementProperties.TEST_SERVICES_DEFAULT; - stateManagementProperties.put(StateManagementProperties.TEST_SERVICES, testServices); - } - if (testRestClasses == null) { - testRestClasses = StateManagementProperties.TEST_REST_CLASSES_DEFAULT; - stateManagementProperties.put(StateManagementProperties.TEST_REST_CLASSES, testRestClasses); - } - if (testManaged == null) { - testManaged = StateManagementProperties.TEST_MANAGED_DEFAULT; - stateManagementProperties.put(StateManagementProperties.TEST_MANAGED, testManaged); - } - if (testSwagger == null) { - testSwagger = StateManagementProperties.TEST_SWAGGER_DEFAULT; - stateManagementProperties.put(StateManagementProperties.TEST_SWAGGER, testSwagger); - } - if (!testServices.equals(StateManagementProperties.TEST_SERVICES_DEFAULT)){ - logger.error(INVALID_PROPERTY_VALUE, - StateManagementProperties.TEST_SERVICES, - StateManagementProperties.TEST_SERVICES_DEFAULT); - } - if (!testRestClasses.equals(StateManagementProperties.TEST_REST_CLASSES_DEFAULT)){ - logger.error(INVALID_PROPERTY_VALUE, - StateManagementProperties.TEST_REST_CLASSES, - StateManagementProperties.TEST_REST_CLASSES_DEFAULT); - } - if (!testManaged.equals(StateManagementProperties.TEST_MANAGED_DEFAULT)){ - logger.warn(INVALID_PROPERTY_VALUE, - StateManagementProperties.TEST_MANAGED, - StateManagementProperties.TEST_MANAGED_DEFAULT); - } - if (!testSwagger.equals(StateManagementProperties.TEST_SWAGGER_DEFAULT)){ - logger.warn(INVALID_PROPERTY_VALUE, - StateManagementProperties.TEST_SWAGGER, - StateManagementProperties.TEST_SWAGGER_DEFAULT); - } - if (resourceName == null){ - missingProperty(StateManagementProperties.RESOURCE_NAME); - } - if (fpMonitorInterval == null){ - missingProperty(StateManagementProperties.FP_MONITOR_INTERVAL); - } - if (failedCounterThreshold == null){ - missingProperty(StateManagementProperties.FAILED_COUNTER_THRESHOLD); - } - if (testTransInterval == null){ - missingProperty(StateManagementProperties.TEST_TRANS_INTERVAL); - } - if (writeFpcInterval == null){ - missingProperty(StateManagementProperties.WRITE_FPC_INTERVAL); - } - if (siteName == null){ - missingProperty(StateManagementProperties.SITE_NAME); - } - if (nodeType == null){ - missingProperty(StateManagementProperties.NODE_TYPE); - } - if (dependencyGroups == null){ - missingProperty(StateManagementProperties.DEPENDENCY_GROUPS); - } - if (javaxPersistenceJdbcDriver == null){ - missingProperty(StateManagementProperties.DB_DRIVER); - } - if (javaxPersistenceJdbcUrl == null){ - missingProperty(StateManagementProperties.DB_URL); - } - if (javaxPersistenceJdbcUser == null){ - missingProperty(StateManagementProperties.DB_USER); - } - if (javaxPersistenceJdbcPassword == null){ - missingProperty(StateManagementProperties.DB_PWD); - } - - //Log the values so we can diagnose any issues - logPropertyValue(StateManagementProperties.TEST_HOST,testHost); - logPropertyValue(StateManagementProperties.TEST_PORT,testPort); - logPropertyValue(StateManagementProperties.TEST_SERVICES,testServices); - logPropertyValue(StateManagementProperties.TEST_REST_CLASSES,testRestClasses); - logPropertyValue(StateManagementProperties.TEST_MANAGED,testManaged); - logPropertyValue(StateManagementProperties.TEST_SWAGGER,testSwagger); - logPropertyValue(StateManagementProperties.RESOURCE_NAME,resourceName); - logPropertyValue(StateManagementProperties.FP_MONITOR_INTERVAL,fpMonitorInterval); - logPropertyValue(StateManagementProperties.FAILED_COUNTER_THRESHOLD,failedCounterThreshold); - logPropertyValue(StateManagementProperties.TEST_TRANS_INTERVAL,testTransInterval); - logPropertyValue(StateManagementProperties.WRITE_FPC_INTERVAL,writeFpcInterval); - logPropertyValue(StateManagementProperties.SITE_NAME,siteName); - logPropertyValue(StateManagementProperties.NODE_TYPE,nodeType); - logPropertyValue(StateManagementProperties.DEPENDENCY_GROUPS,dependencyGroups); - logPropertyValue(StateManagementProperties.DB_DRIVER,javaxPersistenceJdbcDriver); - logPropertyValue(StateManagementProperties.DB_URL,javaxPersistenceJdbcUrl); - logPropertyValue(StateManagementProperties.DB_USER,javaxPersistenceJdbcUser); - logPropertyValue(StateManagementProperties.DB_PWD,javaxPersistenceJdbcPassword); - + subsystemTestProperties = stateManagementProperties; // Now that we've validated the properties, create Drools Integrity Monitor // with these properties. - im = new DroolsPDPIntegrityMonitor(resourceName, - stateManagementProperties); + im = makeMonitor(resourceName, stateManagementProperties); logger.info("init: New DroolsPDPIntegrityMonitor instantiated, resourceName = ", resourceName); // create http server + makeRestServer(testHost, testPort, stateManagementProperties); + logger.info("init: Exiting and returning DroolsPDPIntegrityMonitor"); + + return im; + } + + /** + * Makes an Integrity Monitor. + * @param resourceName unique name of this Integrity Monitor + * @param properties properties used to configure the Integrity Monitor + * @return + * @throws IntegrityMonitorException + */ + private static DroolsPDPIntegrityMonitor makeMonitor(String resourceName, Properties properties) + throws IntegrityMonitorException { + + try { + return new DroolsPDPIntegrityMonitor(resourceName, properties); + + } catch (Exception e) { + throw new IntegrityMonitorException(e); + } + } + + /** + * Makes a rest server for the Integrity Monitor. + * @param testHost host name + * @param testPort port + * @param properties properties used to configure the rest server + * @throws IntegrityMonitorException + */ + private static void makeRestServer(String testHost, String testPort, Properties properties) + throws IntegrityMonitorException { + try { logger.info("init: Starting HTTP server, addr= {}", testHost+":"+testPort); + IntegrityMonitorRestServer server = new IntegrityMonitorRestServer(); + server.init(properties); - server.init(stateManagementProperties); } catch (Exception e) { logger.error("init: Caught Exception attempting to start server on testPort= {} message:", testPort, e); - throw e; + throw new IntegrityMonitorException(e); } - logger.info("init: Exiting and returning DroolsPDPIntegrityMonitor"); - return im; + } + + /** + * Gets the properties from the property file. + * @param configDir directory containing the property file + * @return the properties + * @throws IntegrityMonitorException + */ + private static Properties getProperties(String configDir) throws IntegrityMonitorException { + try { + return PropertyUtil.getProperties(configDir + "/" + PROPERTIES_NAME); + + } catch (IOException e) { + throw new IntegrityMonitorException(e); + } + } + + /** + * Checks that a property is defined. + * @param props set of properties + * @param name name of the property to check + * @throws IntegrityMonitorException + */ + private static void checkPropError(Properties props, String name) throws IntegrityMonitorException { + String val = props.getProperty(name); + if(val == null) { + missingProperty(name); + } + + logPropertyValue(name, val); + } + + /** + * Checks a property's value to verify that it matches the expected value. + * If the property is not defined, then it is added to the property set, + * with the expected value. Logs an error if the property is defined, + * but does not have the expected value. + * @param props set of properties + * @param name name of the property to check + * @param expected expected/default value + */ + private static void addDefaultPropError(Properties props, String name, String expected) { + String val = props.getProperty(name); + if(val == null) { + props.setProperty(name, expected); + + } else if( ! val.equals(expected)) { + logger.error(INVALID_PROPERTY_VALUE, name, expected); + } + + logPropertyValue(name, val); + } + + /** + * Checks a property's value to verify that it matches the expected value. + * If the property is not defined, then it is added to the property set, + * with the expected value. Logs a warning if the property is defined, + * but does not have the expected value. + * @param props set of properties + * @param name name of the property to check + * @param expected expected/default value + */ + private static void addDefaultPropWarn(Properties props, String name, String dflt) { + String val = props.getProperty(name); + if(val == null) { + props.setProperty(name, dflt); + + } else if( ! val.equals(dflt)) { + logger.warn(INVALID_PROPERTY_VALUE, name, dflt); + } + + logPropertyValue(name, val); } /** 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 a34d4f98..3cc27907 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 @@ -58,18 +58,14 @@ public class StateManagementFeature implements StateManagementFeatureAPI, /**************************/ public StateManagementFeature(){ - if(logger.isDebugEnabled()){ - logger.debug("StateManagementFeature() constructor"); - } + logger.debug("StateManagementFeature() constructor"); } @Override public void globalInit(String[] args, String configDir) { // Initialization code associated with 'PolicyContainer' - if(logger.isDebugEnabled()){ - logger.debug("StateManagementFeature.globalInit({}) entry", configDir); - } + logger.debug("StateManagementFeature.globalInit({}) entry", configDir); try { @@ -77,9 +73,7 @@ public class StateManagementFeature implements StateManagementFeatureAPI, } catch (Exception e) { - if(logger.isDebugEnabled()){ - logger.debug("DroolsPDPIntegrityMonitor initialization exception: ", e); - } + logger.debug("DroolsPDPIntegrityMonitor initialization exception: ", e); logger.error("DroolsPDPIntegrityMonitor.init()", e); } @@ -89,20 +83,16 @@ public class StateManagementFeature implements StateManagementFeatureAPI, try { droolsPdpIntegrityMonitor = DroolsPDPIntegrityMonitor.getInstance(); stateManagement = droolsPdpIntegrityMonitor.getStateManager(); - if(logger.isDebugEnabled()){ - logger.debug("StateManagementFeature.globalInit(): " - + "stateManagement.getAdminState(): {}", stateManagement.getAdminState()); - } + + logger.debug("StateManagementFeature.globalInit(): " + + "stateManagement.getAdminState(): {}", stateManagement.getAdminState()); + if(stateManagement == null){ - if(logger.isDebugEnabled()){ - logger.debug("StateManagementFeature.globalInit(): stateManagement is NULL!"); - } + logger.debug("StateManagementFeature.globalInit(): stateManagement is NULL!"); } } catch (Exception e1) { - if(logger.isDebugEnabled()){ - logger.debug("StateManagementFeature.globalInit(): DroolsPDPIntegrityMonitor" - + " initialization failed with exception:", e1); - } + logger.debug("StateManagementFeature.globalInit(): DroolsPDPIntegrityMonitor" + + " initialization failed with exception:", e1); logger.error("DroolsPDPIntegrityMonitor.init(): StateManagementFeature startup failed " + "to get DroolsPDPIntegrityMonitor instance:", e1); } @@ -113,15 +103,13 @@ public class StateManagementFeature implements StateManagementFeatureAPI, */ @Override public void addObserver(Observer stateChangeObserver) { - if(logger.isDebugEnabled()){ - logger.debug("StateManagementFeature.addObserver() entry\n" - + "StateManagementFeature.addObserver(): " - + "stateManagement.getAdminState(): {}", stateManagement.getAdminState()); - } + logger.debug("StateManagementFeature.addObserver() entry\n" + + "StateManagementFeature.addObserver(): " + + "stateManagement.getAdminState(): {}", stateManagement.getAdminState()); + stateManagement.addObserver(stateChangeObserver); - if(logger.isDebugEnabled()){ - logger.debug("StateManagementFeature.addObserver() exit"); - } + + logger.debug("StateManagementFeature.addObserver() exit"); } /** @@ -267,7 +255,7 @@ public class StateManagementFeature implements StateManagementFeatureAPI, @Override public void allSeemsWell(String key, Boolean asw, String msg) - throws IllegalArgumentException, AllSeemsWellException { + throws AllSeemsWellException { droolsPdpIntegrityMonitor.allSeemsWell(key, asw, msg); 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 85e0ed85..b364ef83 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 @@ -185,6 +185,7 @@ public class StateManagementTest { /**************Repository Audit Test**************/ logger.debug("\n\ntestStateManagementOperation: Repository Audit\n\n"); try{ + StateManagementProperties.initProperties(fsmProperties); RepositoryAudit repositoryAudit = (RepositoryAudit) RepositoryAudit.getInstance(); repositoryAudit.invoke(fsmProperties); -- 2.16.6