Fix more sonars in drools-pdp 89/111889/3
authorJim Hahn <jrh3@att.com>
Mon, 31 Aug 2020 12:22:13 +0000 (08:22 -0400)
committerJim Hahn <jrh3@att.com>
Mon, 31 Aug 2020 14:15:07 +0000 (10:15 -0400)
Fixed more sonars in drools-pdp:
- remove commented code
- don't throw generic Exception
- unused field (made it protected instead of private)
- log conditionally
- cognitive complexity
- too many break/continue
- return empty list instead of null
- Random() is not secure

Fixed more eclipse warnings:
- parameterize generic types

Issue-ID: POLICY-2616-sonars3
Change-Id: Ia5ad769b2ea763568cfae3d81807926d89153b09
Signed-off-by: Jim Hahn <jrh3@att.com>
13 files changed:
api-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementFeatureApi.java
feature-active-standby-management/src/main/java/org/onap/policy/drools/activestandby/DroolsPdpsElectionHandler.java
feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java
feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java
feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/ServerPoolApi.java
feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/persistence/Persistence.java
feature-server-pool/src/test/java/org/onap/policy/drools/serverpool/AdapterImpl.java
feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/DroolsPdpIntegrityMonitor.java
feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/RepositoryAudit.java
feature-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementFeature.java
feature-state-management/src/test/java/org/onap/policy/drools/statemanagement/test/StateManagementTest.java
policy-management/src/main/java/org/onap/policy/drools/controller/IndexedDroolsControllerFactory.java
policy-management/src/main/java/org/onap/policy/drools/system/Main.java

index c267968..c4ba862 100644 (file)
@@ -23,7 +23,9 @@ package org.onap.policy.drools.statemanagement;
 import javax.validation.constraints.NotNull;
 import org.onap.policy.common.capabilities.Lockable;
 import org.onap.policy.common.im.AllSeemsWellException;
+import org.onap.policy.common.im.IntegrityMonitorException;
 import org.onap.policy.common.im.StateChangeNotifier;
+import org.onap.policy.common.im.StateManagementException;
 import org.onap.policy.common.utils.services.OrderedService;
 
 /**
@@ -105,18 +107,18 @@ public interface StateManagementFeatureApi extends OrderedService, Lockable {
      * will take a value of coldstandby.
      *
      * @param resourceName resource name
-     * @throws Exception exception
+     * @throws StateManagementException exception
      */
-    void disableFailed(String resourceName) throws Exception;
+    void disableFailed(String resourceName) throws StateManagementException;
 
     /**
      * This method moves the X.731 Operational State for this resource into a value of disabled and
      * the Availability Status to a value of failed. As a consequence the Standby Status value will
      * take a value of coldstandby.
      *
-     * @throws Exception exception
+     * @throws StateManagementException exception
      */
-    void disableFailed() throws Exception;
+    void disableFailed() throws StateManagementException;
 
     /**
      * This method moves the X.731 Standby Status for this resource from hotstandby to
@@ -124,18 +126,18 @@ public interface StateManagementFeatureApi extends OrderedService, Lockable {
      * value is null, it will move to providingservice assuming the Operational State is enabled and
      * Administrative State is unlocked.
      *
-     * @throws Exception exception
+     * @throws IntegrityMonitorException exception
      */
-    void promote() throws Exception;
+    void promote() throws IntegrityMonitorException;
 
     /**
      * This method moves the X.731 Standby Status for this resource from providingservice to
      * hotstandby. If the current value is null, it will move to hotstandby assuming the Operational
      * State is enabled and Administrative State is unlocked. Else, it will move to coldstandby
      *
-     * @throws Exception exception
+     * @throws StateManagementException exception
      */
-    void demote() throws Exception;
+    void demote() throws StateManagementException;
 
     /**
      * Returns the resourceName associated with this instance of the
index acb7cd5..4d40fd5 100644 (file)
@@ -587,10 +587,7 @@ public class DroolsPdpsElectionHandler implements ThreadRunningChecker {
                      * Only call promote if it is not already in the right state.  Don't worry about
                      * synching the lower level topic endpoint states.  That is done by the
                      * refreshStateAudit.
-                     * Note that we need to fetch the session list from 'mostRecentPrimary'
-                     * at this point -- soon, 'mostRecentPrimary' will be set to this host.
                      */
-                    //this.sessions = mostRecentPrimary.getSessions();
                     stateManagementFeature.promote();
                 }
             } catch (Exception e) {
index 6241a29..3284a6b 100644 (file)
@@ -783,7 +783,11 @@ public class Bucket {
              * current host.
              */
             UUID key = oldHost.uuid;
-            for (int count = new Random().nextInt(rb.testServers.size() - 1); count >= 0; count -= 1) {
+            /*
+             * Disabling sonar, because this Random() is not used for security purposes.
+             */
+            int randomStart = new Random().nextInt(rb.testServers.size() - 1);  // NOSONAR
+            for (int count = randomStart; count >= 0; count -= 1) {
                 key = rb.testServers.higherKey(key);
                 if (key == null) {
                     // wrap to the beginning of the list
index 80b5891..ffddf0a 100644 (file)
@@ -334,11 +334,8 @@ public class Server implements Comparable<Server> {
 
             // add any additional servlets
             for (ServerPoolApi feature : ServerPoolApi.impl.getList()) {
-                Collection<Class<?>> classes = feature.servletClasses();
-                if (classes != null) {
-                    for (Class<?> clazz : classes) {
-                        restServer.addServletClass(null, clazz.getName());
-                    }
+                for (Class<?> clazz : feature.servletClasses()) {
+                    restServer.addServletClass(null, clazz.getName());
                 }
             }
 
index d267ce8..0442912 100644 (file)
@@ -21,6 +21,7 @@
 package org.onap.policy.drools.serverpool;
 
 import java.util.Collection;
+import java.util.Collections;
 import org.onap.policy.common.utils.services.OrderedService;
 import org.onap.policy.common.utils.services.OrderedServiceImpl;
 
@@ -39,7 +40,7 @@ public interface ServerPoolApi extends OrderedService {
      * @return a Collection of classes implementing REST methods
      */
     public default Collection<Class<?>> servletClasses() {
-        return null;
+        return Collections.emptyList();
     }
 
     /**
index e8121f3..c1d7192 100644 (file)
@@ -31,6 +31,7 @@ import java.util.IdentityHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Properties;
 import java.util.Set;
 import java.util.UUID;
@@ -698,47 +699,53 @@ public class Persistence implements PolicySessionFeatureApi, ServerPoolApi {
         private List<CountDownLatch> restoreBucketDroolsSessions() {
             List<CountDownLatch> sessionLatches = new LinkedList<>();
             for (Map.Entry<String, ReceiverSessionBucketData> entry : sessionData.entrySet()) {
-                String sessionName = entry.getKey();
-                ReceiverSessionBucketData rsbd = entry.getValue();
+                restoreBucketDroolsSession(sessionLatches, entry);
+            }
+            return sessionLatches;
+        }
 
-                PolicySession policySession = detmPolicySession(sessionName);
-                if (policySession == null) {
-                    logger.error(RESTORE_BUCKET_ERROR
-                                 + "Can't find PolicySession{}", sessionName);
-                    continue;
-                }
+        private void restoreBucketDroolsSession(List<CountDownLatch> sessionLatches,
+                        Entry<String, ReceiverSessionBucketData> entry) {
 
-                final Map<?, ?> droolsObjects = deserializeMap(sessionName, rsbd, policySession);
-                if (droolsObjects == null) {
-                    continue;
-                }
+            String sessionName = entry.getKey();
+            ReceiverSessionBucketData rsbd = entry.getValue();
 
-                // if we reach this point, we have decoded the persistent data
+            PolicySession policySession = detmPolicySession(sessionName);
+            if (policySession == null) {
+                logger.error(RESTORE_BUCKET_ERROR
+                             + "Can't find PolicySession{}", sessionName);
+                return;
+            }
 
-                // signal when restore is complete
-                final CountDownLatch sessionLatch = new CountDownLatch(1);
+            final Map<?, ?> droolsObjects = deserializeMap(sessionName, rsbd, policySession);
+            if (droolsObjects == null) {
+                return;
+            }
 
-                // 'KieSession' object
-                final KieSession kieSession = policySession.getKieSession();
+            // if we reach this point, we have decoded the persistent data
 
-                // run the following within the Drools session thread
-                DroolsRunnable insertDroolsObjects = () -> {
-                    try {
-                        // insert all of the Drools objects into the session
-                        for (Object droolsObj : droolsObjects.keySet()) {
-                            kieSession.insert(droolsObj);
-                        }
-                    } finally {
-                        // signal completion
-                        sessionLatch.countDown();
+            // signal when restore is complete
+            final CountDownLatch sessionLatch = new CountDownLatch(1);
+
+            // 'KieSession' object
+            final KieSession kieSession = policySession.getKieSession();
+
+            // run the following within the Drools session thread
+            DroolsRunnable insertDroolsObjects = () -> {
+                try {
+                    // insert all of the Drools objects into the session
+                    for (Object droolsObj : droolsObjects.keySet()) {
+                        kieSession.insert(droolsObj);
                     }
-                };
-                kieSession.insert(insertDroolsObjects);
+                } finally {
+                    // signal completion
+                    sessionLatch.countDown();
+                }
+            };
+            kieSession.insert(insertDroolsObjects);
 
-                // add this to the set of 'CountDownLatch's we are waiting for
-                sessionLatches.add(sessionLatch);
-            }
-            return sessionLatches;
+            // add this to the set of 'CountDownLatch's we are waiting for
+            sessionLatches.add(sessionLatch);
         }
 
         private PolicySession detmPolicySession(String sessionName) {
index 044067a..865f4e9 100644 (file)
@@ -55,8 +55,12 @@ public class AdapterImpl extends Adapter {
      * Each 'AdapterImpl' instance has it's own class object, making it a
      * singleton. There is only a single 'Adapter' class object, and all
      * 'AdapterImpl' classes are derived from it.
+     *
+     * Sonar thinks this field isn't used.  However, it's value is actually
+     * retrieved via Whitebox, below.  Thus it is marked "protected" instead
+     * of "private" to avoid the sonar complaint.
      */
-    private static AdapterImpl adapter = null;
+    protected static AdapterImpl adapter = null;
 
     // this is the adapter index
     private int index;
@@ -347,7 +351,7 @@ public class AdapterImpl extends Adapter {
         boolean rval = false;
         ClassLoader myClassLoader = AdapterImpl.class.getClassLoader();
         for (Object o : objects) {
-            Class clazz = o.getClass();
+            Class<?> clazz = o.getClass();
             ClassLoader objClassLoader = clazz.getClassLoader();
 
             try {
index 2252a0f..08c8e3a 100644 (file)
@@ -355,7 +355,7 @@ public class DroolsPdpIntegrityMonitor extends IntegrityMonitor {
          * @param persistenceProperties Used for DB access
          * @throws Exception passed in by the audit
          */
-        abstract void invoke(Properties persistenceProperties) throws Exception;
+        abstract void invoke(Properties persistenceProperties) throws IntegrityMonitorException;
     }
 
     public static class IntegrityMonitorRestServer implements Startable {
index 8f33f92..438b6ec 100644 (file)
@@ -37,6 +37,7 @@ import java.util.TreeSet;
 import java.util.concurrent.TimeUnit;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import org.onap.policy.common.im.IntegrityMonitorException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -106,7 +107,7 @@ public class RepositoryAudit extends DroolsPdpIntegrityMonitor.AuditBase {
      * @param properties properties to be passed to the audit
      */
     @Override
-    public void invoke(Properties properties) throws IOException, InterruptedException {
+    public void invoke(Properties properties) throws IntegrityMonitorException {
         logger.debug("Running 'RepositoryAudit.invoke'");
 
         InvokeData data = new InvokeData();
@@ -121,25 +122,35 @@ public class RepositoryAudit extends DroolsPdpIntegrityMonitor.AuditBase {
             return;
         }
 
-        // Run audit for first nexus repository
-        logger.debug("Running read-only audit on first nexus repository: repository");
-        runAudit(data);
+        try {
+            // Run audit for first nexus repository
+            logger.debug("Running read-only audit on first nexus repository: repository");
+            runAudit(data);
 
-        // set of indices for supported nexus repos (ex: repository2 -> 2)
-        // TreeSet is used to maintain order so repos can be audited in numerical order
-        TreeSet<Integer> repoIndices = countAdditionalNexusRepos();
-        logger.debug("Additional nexus repositories: {}", repoIndices);
+            // set of indices for supported nexus repos (ex: repository2 -> 2)
+            // TreeSet is used to maintain order so repos can be audited in numerical
+            // order
+            TreeSet<Integer> repoIndices = countAdditionalNexusRepos();
+            logger.debug("Additional nexus repositories: {}", repoIndices);
 
-        // Run audit for remaining 'numNexusRepos' repositories
-        for (int index : repoIndices) {
-            logger.debug("Running read-only audit on nexus repository = repository{}", index);
+            // Run audit for remaining 'numNexusRepos' repositories
+            for (int index : repoIndices) {
+                logger.debug("Running read-only audit on nexus repository = repository{}", index);
 
-            data = new InvokeData(index);
-            data.initIsActive();
+                data = new InvokeData(index);
+                data.initIsActive();
 
-            if (data.isActive) {
-                runAudit(data);
+                if (data.isActive) {
+                    runAudit(data);
+                }
             }
+
+        } catch (IOException e) {
+            throw new IntegrityMonitorException(e);
+
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            throw new IntegrityMonitorException(e);
         }
     }
 
index 7523337..3dbb8d3 100644 (file)
@@ -23,8 +23,10 @@ package org.onap.policy.drools.statemanagement;
 import java.io.IOException;
 import java.util.Properties;
 import org.onap.policy.common.im.AllSeemsWellException;
+import org.onap.policy.common.im.IntegrityMonitorException;
 import org.onap.policy.common.im.StateChangeNotifier;
 import org.onap.policy.common.im.StateManagement;
+import org.onap.policy.common.im.StateManagementException;
 import org.onap.policy.drools.core.PolicySessionFeatureApi;
 import org.onap.policy.drools.features.PolicyEngineFeatureApi;
 import org.onap.policy.drools.utils.PropertyUtil;
@@ -146,7 +148,7 @@ public class StateManagementFeature implements StateManagementFeatureApi,
      * {@inheritDoc}.
      */
     @Override
-    public void disableFailed(String resourceName) throws Exception {
+    public void disableFailed(String resourceName) throws StateManagementException {
         stateManagement.disableFailed(resourceName);
 
     }
@@ -155,7 +157,7 @@ public class StateManagementFeature implements StateManagementFeatureApi,
      * {@inheritDoc}.
      */
     @Override
-    public void disableFailed() throws Exception {
+    public void disableFailed() throws StateManagementException {
         stateManagement.disableFailed();
     }
 
@@ -163,7 +165,7 @@ public class StateManagementFeature implements StateManagementFeatureApi,
      * {@inheritDoc}.
      */
     @Override
-    public void promote() throws Exception {
+    public void promote() throws IntegrityMonitorException {
         stateManagement.promote();
     }
 
@@ -171,7 +173,7 @@ public class StateManagementFeature implements StateManagementFeatureApi,
      * {@inheritDoc}.
      */
     @Override
-    public void demote() throws Exception {
+    public void demote() throws StateManagementException {
         stateManagement.demote();
     }
 
index 33bfaed..8d47e1d 100644 (file)
@@ -25,7 +25,6 @@ import static org.junit.Assert.assertTrue;
 
 import java.io.File;
 import java.io.FileInputStream;
-import java.io.IOException;
 import java.util.Properties;
 import javax.persistence.EntityManager;
 import javax.persistence.EntityManagerFactory;
@@ -37,6 +36,7 @@ import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.onap.policy.common.im.IntegrityMonitorException;
 import org.onap.policy.common.im.StateManagement;
 import org.onap.policy.drools.core.PolicySessionFeatureApi;
 import org.onap.policy.drools.statemanagement.DbAudit;
@@ -196,28 +196,18 @@ public class StateManagementTest {
             repositoryAudit.invoke(fsmProperties);
 
             //Should not throw an IOException in Linux Foundation env
-            assertTrue(true);
-        } catch (IOException e) {
+
+        } catch (IntegrityMonitorException e) {
             //Note: this catch is here because in a local environment mvn will not run in
             //in the temp directory
             logger.debug("testSubsytemTest RepositoryAudit IOException", e);
-        } catch (InterruptedException e) {
-            assertTrue(false);
-            logger.debug("testSubsytemTest RepositoryAudit InterruptedException", e);
         }
 
         /* ****************Db Audit Test. ************** */
         logger.debug("\n\ntestStateManagementOperation: DB Audit\n\n");
 
-        try {
-            DbAudit dbAudit = (DbAudit) DbAudit.getInstance();
-            dbAudit.invoke(fsmProperties);
-
-            assertTrue(true);
-        } catch (Exception e) {
-            assertTrue(false);
-            logger.debug("testSubsytemTest DbAudit exception", e);
-        }
+        DbAudit dbAudit = (DbAudit) DbAudit.getInstance();
+        dbAudit.invoke(fsmProperties);
 
         /* ************IntegrityMonitorRestManager Test. ************ */
         logger.debug("\n\ntestStateManagementOperation: IntegrityMonitorRestManager\n\n");
index e8234a4..d219668 100644 (file)
@@ -31,6 +31,7 @@ import org.onap.policy.common.endpoints.event.comm.Topic.CommInfrastructure;
 import org.onap.policy.common.endpoints.event.comm.TopicSink;
 import org.onap.policy.common.endpoints.event.comm.TopicSource;
 import org.onap.policy.common.endpoints.properties.PolicyEndPointProperties;
+import org.onap.policy.common.utils.services.FeatureApiUtils;
 import org.onap.policy.drools.controller.internal.MavenDroolsController;
 import org.onap.policy.drools.controller.internal.NullDroolsController;
 import org.onap.policy.drools.features.DroolsControllerFeatureApi;
@@ -152,24 +153,8 @@ class IndexedDroolsControllerFactory implements DroolsControllerFactory {
 
         /* new drools controller */
 
-        DroolsController controller = null;
-        for (DroolsControllerFeatureApi feature: getProviders()) {
-            try {
-                controller = feature.beforeInstance(properties,
-                        newGroupId, newArtifactId, newVersion,
+        DroolsController controller = applyBeforeInstance(properties, newGroupId, newArtifactId, newVersion,
                         decoderConfigurations, encoderConfigurations);
-                if (controller != null) {
-                    logger.info("feature {} ({}) beforeInstance() has intercepted drools controller {}:{}:{}",
-                            feature.getName(), feature.getSequenceNumber(),
-                            newGroupId, newArtifactId, newVersion);
-                    break;
-                }
-            } catch (RuntimeException r) {
-                logger.error("feature {} ({}) beforeInstance() of drools controller {}:{}:{} failed",
-                        feature.getName(), feature.getSequenceNumber(),
-                        newGroupId, newArtifactId, newVersion, r);
-            }
-        }
 
         if (controller == null) {
             controller = new MavenDroolsController(newGroupId, newArtifactId, newVersion, decoderConfigurations,
@@ -180,16 +165,38 @@ class IndexedDroolsControllerFactory implements DroolsControllerFactory {
             droolsControllers.put(controllerId, controller);
         }
 
+        final DroolsController controllerFinal = controller;
+
+        FeatureApiUtils.apply(getProviders(),
+            feature -> feature.afterInstance(controllerFinal, properties),
+            (feature, ex) -> logger.error("feature {} ({}) afterInstance() of drools controller {}:{}:{} failed",
+                            feature.getName(), feature.getSequenceNumber(),
+                            newGroupId, newArtifactId, newVersion, ex));
+
+        return controller;
+    }
+
+    private DroolsController applyBeforeInstance(Properties properties, String newGroupId, String newArtifactId,
+                    String newVersion, List<TopicCoderFilterConfiguration> decoderConfigurations,
+                    List<TopicCoderFilterConfiguration> encoderConfigurations) {
+        DroolsController controller = null;
         for (DroolsControllerFeatureApi feature: getProviders()) {
             try {
-                feature.afterInstance(controller, properties);
+                controller = feature.beforeInstance(properties,
+                        newGroupId, newArtifactId, newVersion,
+                        decoderConfigurations, encoderConfigurations);
+                if (controller != null) {
+                    logger.info("feature {} ({}) beforeInstance() has intercepted drools controller {}:{}:{}",
+                            feature.getName(), feature.getSequenceNumber(),
+                            newGroupId, newArtifactId, newVersion);
+                    break;
+                }
             } catch (RuntimeException r) {
-                logger.error("feature {} ({}) afterInstance() of drools controller {}:{}:{} failed",
+                logger.error("feature {} ({}) beforeInstance() of drools controller {}:{}:{} failed",
                         feature.getName(), feature.getSequenceNumber(),
                         newGroupId, newArtifactId, newVersion, r);
             }
         }
-
         return controller;
     }
 
index 9e6d043..be0cb82 100644 (file)
@@ -106,7 +106,9 @@ public class Main {
         /* 5. Open the engine for dynamic configuration */
         PolicyEngineConstants.getManager().open();
 
-        logger.info(String.format(MessageConstants.START_SUCCESS_MSG, MessageConstants.POLICY_DROOLS_PDP));
+        if (logger.isInfoEnabled()) {
+            logger.info(String.format(MessageConstants.START_SUCCESS_MSG, MessageConstants.POLICY_DROOLS_PDP));
+        }
     }
 
     private static void setSystemProperties() {