From 15014b8ca386a8bfd5c26435f45de94ca06e95e8 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Mon, 6 Apr 2020 15:33:23 -0400 Subject: Address sonar issues in drools-pdp Addressed the following sonar issues: - add "final" to public static fields - commented code; some were bogus - just updated the comments so sonar is happy - use "{}" instead of string concatenation - junit should assert something - when using logger, invoke compute-intensive tasks conditionally - use superclass name instead of subclass name to access static fields - don't always return the same value - remove "transient" from fields of classes that aren't Serializable - don't nest try/catch blocks - use appropriate class name in Logger.getLogger() - use Predicate instead of Function - remove unused parameters from private methods - either log or throw - remove duplicate methods - use remove() TLS instead of set(null) - null check is implicit in instanceof check - do something with return value - don't use volatile - don't return "null" list; used Optional instead - add no-arg constructor to non-Serializable superclass - add callSuper=true for EqualsAndHashCode - don't declare "throws XXX" where XXX is a subclass of RuntimeException - remove serialVersionUID field if the class isn't Serializable Also addressed some eclipse warnings: - unused fields - suppress generic typic cast warnings Issue-ID: POLICY-2305 Change-Id: I906d5bf71c1f86531423e23b3667a585cdba45e1 Signed-off-by: Jim Hahn --- .../policy/drools/lifecycle/LifecycleFeature.java | 4 ++-- .../lifecycle/PolicyTypeDroolsController.java | 24 +++++++++++++--------- .../PolicyTypeNativeArtifactController.java | 2 +- .../PolicyTypeNativeDroolsController.java | 19 ++++++++--------- .../LifecycleStateActivePoliciesTest.java | 3 ++- .../drools/lifecycle/LifecycleStateActiveTest.java | 2 +- .../lifecycle/PolicyTypeDroolsControllerTest.java | 3 +-- 7 files changed, 30 insertions(+), 27 deletions(-) (limited to 'feature-lifecycle') diff --git a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/LifecycleFeature.java b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/LifecycleFeature.java index 12828e02..003740c1 100644 --- a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/LifecycleFeature.java +++ b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/LifecycleFeature.java @@ -71,7 +71,7 @@ public class LifecycleFeature @Override public boolean beforeShutdown(PolicyEngine engine) { - return fsmShutdown(engine); + return fsmShutdown(); } @Override @@ -114,7 +114,7 @@ public class LifecycleFeature return false; } - private boolean fsmShutdown(PolicyEngine engine) { + private boolean fsmShutdown() { fsm.shutdown(); return false; } diff --git a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsController.java b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsController.java index 8dfbf2f3..71e40726 100644 --- a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsController.java +++ b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsController.java @@ -24,7 +24,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Function; +import java.util.function.Predicate; import lombok.Getter; import lombok.NonNull; import org.apache.commons.lang3.StringUtils; @@ -50,7 +50,7 @@ public class PolicyTypeDroolsController implements PolicyTypeController { protected static final ToscaPolicyTypeIdentifier compliantType = new ToscaPolicyTypeIdentifier("onap.policies.controlloop.operational.common.Drools", "1.0.0"); - private static final Logger logger = LoggerFactory.getLogger(PolicyTypeController.class); + private static final Logger logger = LoggerFactory.getLogger(PolicyTypeDroolsController.class); @Getter protected final Map controllers = new ConcurrentHashMap<>(); @@ -60,7 +60,7 @@ public class PolicyTypeDroolsController implements PolicyTypeController { @GsonJsonIgnore @JsonIgnore - protected final transient LifecycleFsm fsm; + protected final LifecycleFsm fsm; /** * Creates a Policy Type Drools Controller. @@ -120,17 +120,12 @@ public class PolicyTypeDroolsController implements PolicyTypeController { return List.of(this.controllers.get(controllerName)); } - private boolean perform(ToscaPolicy policy, Function operation) { + private boolean perform(ToscaPolicy policy, Predicate operation) { try { List selected = selectControllers(policy); boolean success = true; for (PolicyController controller : selected) { - try { - success = operation.apply(controller) && success; - } catch (RuntimeException r) { - logger.warn("invalid offer to controller: {}", controller); - success = false; - } + success = modifyController(operation, controller) && success; } return success && !selected.isEmpty(); } catch (CoderException e) { @@ -139,6 +134,15 @@ public class PolicyTypeDroolsController implements PolicyTypeController { } } + private boolean modifyController(Predicate operation, PolicyController controller) { + try { + return operation.test(controller); + } catch (RuntimeException r) { + logger.warn("invalid offer to controller: {}", controller); + return false; + } + } + private List selectControllers(ToscaPolicy policy) throws CoderException { List selected; if (legacyType.equals(policyType)) { diff --git a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeArtifactController.java b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeArtifactController.java index 113bb885..810c2ecf 100644 --- a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeArtifactController.java +++ b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeArtifactController.java @@ -44,7 +44,7 @@ public class PolicyTypeNativeArtifactController implements PolicyTypeController @GsonJsonIgnore @JsonIgnore - protected final transient LifecycleFsm fsm; + protected final LifecycleFsm fsm; public PolicyTypeNativeArtifactController(LifecycleFsm fsm, ToscaPolicyTypeIdentifier policyType) { this.policyType = policyType; diff --git a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeDroolsController.java b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeDroolsController.java index e74b2895..9a11955a 100644 --- a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeDroolsController.java +++ b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeDroolsController.java @@ -55,7 +55,7 @@ public class PolicyTypeNativeDroolsController implements PolicyTypeController { @GsonJsonIgnore @JsonIgnore - protected final transient LifecycleFsm fsm; + protected final LifecycleFsm fsm; public PolicyTypeNativeDroolsController(LifecycleFsm fsm, ToscaPolicyTypeIdentifier policyType) { this.policyType = policyType; @@ -72,9 +72,10 @@ public class PolicyTypeNativeDroolsController implements PolicyTypeController { ControllerProperties controllerConfig = controllerPolicy.getProperties(); + configControllerName(controllerConfig, controllerProps); + boolean success = - configControllerName(controllerConfig, controllerProps) - && configControllerSources(controllerConfig, controllerProps) + configControllerSources(controllerConfig, controllerProps) && configControllerSinks(controllerConfig, controllerProps) && configControllerCustom(controllerConfig, controllerProps); @@ -145,10 +146,9 @@ public class PolicyTypeNativeDroolsController implements PolicyTypeController { return nativePolicy; } - private boolean configControllerName(ControllerProperties controllerConfig, Properties controllerProps) { + private void configControllerName(ControllerProperties controllerConfig, Properties controllerProps) { controllerProps .setProperty(DroolsPropertyConstants.PROPERTY_CONTROLLER_NAME, controllerConfig.getControllerName()); - return true; } private boolean configControllerSources(ControllerProperties controllerConfig, Properties controllerProps) { @@ -241,11 +241,10 @@ public class PolicyTypeNativeDroolsController implements PolicyTypeController { private boolean configControllerCustom(ControllerProperties controllerConfig, Properties controllerProps) { Map configCustom = controllerConfig.getCustomConfig(); - if (configCustom == null || configCustom.isEmpty()) { - return true; + if (configCustom != null && !configCustom.isEmpty()) { + controllerProps.putAll(configCustom); } - controllerProps.putAll(configCustom); return true; } @@ -255,7 +254,7 @@ public class PolicyTypeNativeDroolsController implements PolicyTypeController { private List sourceTopics(List sourceTopics) { if (sourceTopics == null) { - return Collections.EMPTY_LIST; + return Collections.emptyList(); } return sourceTopics.stream() @@ -265,7 +264,7 @@ public class PolicyTypeNativeDroolsController implements PolicyTypeController { private List sinkTopics(List sinkTopics) { if (sinkTopics == null) { - return Collections.EMPTY_LIST; + return Collections.emptyList(); } return sinkTopics.stream() diff --git a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActivePoliciesTest.java b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActivePoliciesTest.java index 0e5937ff..fdcaac50 100644 --- a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActivePoliciesTest.java +++ b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActivePoliciesTest.java @@ -125,6 +125,7 @@ public class LifecycleStateActivePoliciesTest extends LifecycleStateRunningTest ToscaPolicy policyNativeArtifact = getPolicyFromFile(EXAMPLE_NATIVE_ARTIFACT_POLICY_JSON, EXAMPLE_NATIVE_DROOLS_ARTIFACT_POLICY_NAME); + @SuppressWarnings("unchecked") Map controllerMap = (Map) policyNativeArtifact.getProperties().get("controller"); controllerMap.put("name", "xyz987"); @@ -239,7 +240,7 @@ public class LifecycleStateActivePoliciesTest extends LifecycleStateRunningTest assertEquals(policyNativeController, fsm.getPoliciesMap().get(policyNativeController.getIdentifier())); assertEquals(policyNativeFooController, fsm.getPoliciesMap().get(policyNativeFooController.getIdentifier())); - update.setPolicies(Collections.EMPTY_LIST); + update.setPolicies(Collections.emptyList()); assertTrue(fsm.update(update)); assertThatIllegalArgumentException().isThrownBy(() -> controllerSupport.getController().getDrools()); assertNull(fsm.getPoliciesMap().get(policyNativeController.getIdentifier())); diff --git a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActiveTest.java b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActiveTest.java index f370d0db..54f7d68f 100644 --- a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActiveTest.java +++ b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActiveTest.java @@ -155,7 +155,7 @@ public class LifecycleStateActiveTest extends LifecycleStateRunningTest { fsm.source.offer(new StandardCoder().encode(change)); assertEquals(PdpState.ACTIVE, fsm.state()); - assertEquals(LifecycleFsm.DEFAULT_PDP_GROUP, fsm.getGroup());; + assertEquals(LifecycleFsm.DEFAULT_PDP_GROUP, fsm.getGroup()); assertNotEquals("b", fsm.getSubgroup()); change.setName(fsm.getName()); diff --git a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsControllerTest.java b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsControllerTest.java index a56e250b..bb4b5638 100644 --- a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsControllerTest.java +++ b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsControllerTest.java @@ -45,7 +45,6 @@ public class PolicyTypeDroolsControllerTest extends LifecycleStateRunningTest { "policies/vCPE.policy.operational.input.tosca.json"; private ToscaPolicy policy; - private OperationalPolicy operationalPolicy; private PolicyTypeDroolsController controller; /** @@ -55,7 +54,7 @@ public class PolicyTypeDroolsControllerTest extends LifecycleStateRunningTest { public void init() throws CoderException { fsm = makeFsmWithPseudoTime(); policy = getExamplesPolicy(VCPE_OPERATIONAL_DROOLS_POLICY_JSON, OP_POLICY_NAME_VCPE); - operationalPolicy = fsm.getDomainMaker().convertTo(policy, OperationalPolicy.class); + fsm.getDomainMaker().convertTo(policy, OperationalPolicy.class); controller = new PolicyTypeDroolsController( fsm, PolicyTypeDroolsController.compliantType, controllerSupport.getController()); -- cgit 1.2.3-korg