diff options
author | Jim Hahn <jrh3@att.com> | 2020-08-31 08:22:13 -0400 |
---|---|---|
committer | Jim Hahn <jrh3@att.com> | 2020-08-31 10:15:07 -0400 |
commit | d5c365f60e492e37a361654974eba1719146bdc2 (patch) | |
tree | f717c2198c1aebe0cc03a5d3e9aec653d0d73807 | |
parent | 7dc71c3d0aedb322aff0cdaaa84340354c41b089 (diff) |
Fix more sonars in drools-pdp
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, 133 insertions, 109 deletions
diff --git a/api-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementFeatureApi.java b/api-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementFeatureApi.java index c267968c..c4ba8622 100644 --- a/api-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementFeatureApi.java +++ b/api-state-management/src/main/java/org/onap/policy/drools/statemanagement/StateManagementFeatureApi.java @@ -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 diff --git a/feature-active-standby-management/src/main/java/org/onap/policy/drools/activestandby/DroolsPdpsElectionHandler.java b/feature-active-standby-management/src/main/java/org/onap/policy/drools/activestandby/DroolsPdpsElectionHandler.java index acb7cd54..4d40fd5e 100644 --- a/feature-active-standby-management/src/main/java/org/onap/policy/drools/activestandby/DroolsPdpsElectionHandler.java +++ b/feature-active-standby-management/src/main/java/org/onap/policy/drools/activestandby/DroolsPdpsElectionHandler.java @@ -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) { diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java index 6241a297..3284a6b7 100644 --- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java +++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java @@ -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 diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java index 80b5891a..ffddf0a0 100644 --- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java +++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java @@ -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()); } } diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/ServerPoolApi.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/ServerPoolApi.java index d267ce82..0442912f 100644 --- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/ServerPoolApi.java +++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/ServerPoolApi.java @@ -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(); } /** diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/persistence/Persistence.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/persistence/Persistence.java index e8121f3a..c1d7192f 100644 --- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/persistence/Persistence.java +++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/persistence/Persistence.java @@ -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) { diff --git a/feature-server-pool/src/test/java/org/onap/policy/drools/serverpool/AdapterImpl.java b/feature-server-pool/src/test/java/org/onap/policy/drools/serverpool/AdapterImpl.java index 044067a3..865f4e90 100644 --- a/feature-server-pool/src/test/java/org/onap/policy/drools/serverpool/AdapterImpl.java +++ b/feature-server-pool/src/test/java/org/onap/policy/drools/serverpool/AdapterImpl.java @@ -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 { 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 2252a0f4..08c8e3a5 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 @@ -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 { 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 8f33f929..438b6ec8 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 @@ -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); } } 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 75233376..3dbb8d3e 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 @@ -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(); } 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 33bfaedc..8d47e1d6 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 @@ -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"); diff --git a/policy-management/src/main/java/org/onap/policy/drools/controller/IndexedDroolsControllerFactory.java b/policy-management/src/main/java/org/onap/policy/drools/controller/IndexedDroolsControllerFactory.java index e8234a46..d2196680 100644 --- a/policy-management/src/main/java/org/onap/policy/drools/controller/IndexedDroolsControllerFactory.java +++ b/policy-management/src/main/java/org/onap/policy/drools/controller/IndexedDroolsControllerFactory.java @@ -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; } diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/Main.java b/policy-management/src/main/java/org/onap/policy/drools/system/Main.java index 9e6d0432..be0cb820 100644 --- a/policy-management/src/main/java/org/onap/policy/drools/system/Main.java +++ b/policy-management/src/main/java/org/onap/policy/drools/system/Main.java @@ -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() { |