Fix sonar issues in PAP

Replace string concatenation with "{}" place-holders in logging
statements.
Refactored PdpStatusMessageHandler and PdpGroupStateChangeProvider
to extract common/duplicate code block into PdpMessageGenerator.
Refactored selectPdpGroupsForRegistration() to reduce "cyclomatic complexity";
extracted selectPdpSubGroupsForRegistration from it.

Issue-ID: POLICY-1791
Signed-off-by: jrh3 <jrh3@att.com>
Change-Id: Ia0ebc1cf41cd2fa288130b4a58614a4983f47f8b
Signed-off-by: jrh3 <jrh3@att.com>

4 files changed, 151 insertions, 145 deletions

diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/PdpMessageGenerator.java b/main/src/main/java/org/onap/policy/pap/main/comm/PdpMessageGenerator.java
new file mode 100644
index 00000000..e5246eb1
--- /dev/null
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/PdpMessageGenerator.java
@@ -0,0 +1,129 @@
+/*-
+ * ============LICENSE_START=======================================================
+ * ONAP PAP
+ * ================================================================================
+ * Copyright (C) 2019 AT&T Intellectual Property. All rights reserved.
+ * ================================================================================
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ============LICENSE_END=========================================================
+ */
+
+package org.onap.policy.pap.main.comm;
+
+import java.util.LinkedList;
+import java.util.List;
+import org.onap.policy.common.parameters.ParameterService;
+import org.onap.policy.common.utils.services.Registry;
+import org.onap.policy.models.base.PfModelException;
+import org.onap.policy.models.pdp.concepts.PdpStateChange;
+import org.onap.policy.models.pdp.concepts.PdpSubGroup;
+import org.onap.policy.models.pdp.concepts.PdpUpdate;
+import org.onap.policy.models.pdp.enums.PdpState;
+import org.onap.policy.models.provider.PolicyModelsProvider;
+import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy;
+import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifier;
+import org.onap.policy.pap.main.PapConstants;
+import org.onap.policy.pap.main.PolicyModelsProviderFactoryWrapper;
+import org.onap.policy.pap.main.parameters.PapParameterGroup;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * PDP message generator used to generate PDP-UPDATE and PDP-STATE-CHANGE messages.
+ */
+public class PdpMessageGenerator {
+    private static final Logger LOGGER = LoggerFactory.getLogger(PdpMessageGenerator.class);
+
+    private static final String PAP_GROUP_PARAMS_NAME = "PapGroup";
+
+    /**
+     * Lock used when updating PDPs.
+     */
+    protected final Object updateLock;
+
+    /**
+     * Used to send UPDATE and STATE-CHANGE requests to the PDPs.
+     */
+    protected final PdpModifyRequestMap requestMap;
+
+    /**
+     * Factory for PAP DAO.
+     */
+    protected final PolicyModelsProviderFactoryWrapper modelProviderWrapper;
+
+    /**
+     * Heart beat interval, in milliseconds, to pass to PDPs, or {@code null}.
+     */
+    private final Long heartBeatMs;
+
+
+    /**
+     * Constructs the object.
+     *
+     * @param includeHeartBeat if the heart beat interval is to be included in any
+     *        PDP-UPDATE messages
+     */
+    public PdpMessageGenerator(boolean includeHeartBeat) {
+        modelProviderWrapper = Registry.get(PapConstants.REG_PAP_DAO_FACTORY, PolicyModelsProviderFactoryWrapper.class);
+        updateLock = Registry.get(PapConstants.REG_PDP_MODIFY_LOCK, Object.class);
+        requestMap = Registry.get(PapConstants.REG_PDP_MODIFY_MAP, PdpModifyRequestMap.class);
+
+        if (includeHeartBeat) {
+            PapParameterGroup params = ParameterService.get(PAP_GROUP_PARAMS_NAME);
+            heartBeatMs = params.getPdpParameters().getHeartBeatMs();
+
+        } else {
+            heartBeatMs = null;
+        }
+    }
+
+    protected PdpUpdate createPdpUpdateMessage(final String pdpGroupName, final PdpSubGroup subGroup,
+                    final String pdpInstanceId, final PolicyModelsProvider databaseProvider) throws PfModelException {
+
+        final PdpUpdate update = new PdpUpdate();
+        update.setName(pdpInstanceId);
+        update.setPdpGroup(pdpGroupName);
+        update.setPdpSubgroup(subGroup.getPdpType());
+        update.setPolicies(getToscaPolicies(subGroup, databaseProvider));
+        update.setPdpHeartbeatIntervalMs(heartBeatMs);
+
+        LOGGER.debug("Created PdpUpdate message - {}", update);
+        return update;
+    }
+
+    private List<ToscaPolicy> getToscaPolicies(final PdpSubGroup subGroup, final PolicyModelsProvider databaseProvider)
+                    throws PfModelException {
+
+        final List<ToscaPolicy> policies = new LinkedList<>();
+        for (final ToscaPolicyIdentifier policyIdentifier : subGroup.getPolicies()) {
+            policies.addAll(databaseProvider.getPolicyList(policyIdentifier.getName(), policyIdentifier.getVersion()));
+        }
+
+        LOGGER.debug("Created ToscaPolicy list - {}", policies);
+        return policies;
+    }
+
+    protected PdpStateChange createPdpStateChangeMessage(final String pdpGroupName, final PdpSubGroup subGroup,
+                    final String pdpInstanceId, final PdpState pdpState) {
+
+        final PdpStateChange stateChange = new PdpStateChange();
+        stateChange.setName(pdpInstanceId);
+        stateChange.setPdpGroup(pdpGroupName);
+        stateChange.setPdpSubgroup(subGroup.getPdpType());
+        stateChange.setState(pdpState == null ? PdpState.ACTIVE : pdpState);
+
+        LOGGER.debug("Created PdpStateChange message - {}", stateChange);
+        return stateChange;
+    }
+}
diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/PdpStatusMessageHandler.java b/main/src/main/java/org/onap/policy/pap/main/comm/PdpStatusMessageHandler.java
index f5184c93..6998140f 100644
--- a/main/src/main/java/org/onap/policy/pap/main/comm/PdpStatusMessageHandler.java
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/PdpStatusMessageHandler.java
@@ -21,14 +21,11 @@
 
 package org.onap.policy.pap.main.comm;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.TreeMap;
-
 import org.apache.commons.lang3.builder.EqualsBuilder;
-import org.onap.policy.common.parameters.ParameterService;
 import org.onap.policy.common.utils.services.Registry;
 import org.onap.policy.models.base.PfModelException;
 import org.onap.policy.models.pdp.concepts.Pdp;
@@ -40,12 +37,8 @@ import org.onap.policy.models.pdp.concepts.PdpSubGroup;
 import org.onap.policy.models.pdp.concepts.PdpUpdate;
 import org.onap.policy.models.pdp.enums.PdpState;
 import org.onap.policy.models.provider.PolicyModelsProvider;
-import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy;
-import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifier;
 import org.onap.policy.pap.main.PapConstants;
-import org.onap.policy.pap.main.PolicyModelsProviderFactoryWrapper;
 import org.onap.policy.pap.main.PolicyPapException;
-import org.onap.policy.pap.main.parameters.PapParameterGroup;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -55,41 +48,14 @@ import org.slf4j.LoggerFactory;
  *
  * @author Ram Krishna Verma (ram.krishna.verma@est.tech)
  */
-public class PdpStatusMessageHandler {
+public class PdpStatusMessageHandler extends PdpMessageGenerator {
     private static final Logger LOGGER = LoggerFactory.getLogger(PdpStatusMessageHandler.class);
 
-    private static final String PAP_GROUP_PARAMS_NAME = "PapGroup";
-
-    /**
-     * Lock used when updating PDPs.
-     */
-    private final Object updateLock;
-
-    /**
-     * Used to send UPDATE and STATE-CHANGE requests to the PDPs.
-     */
-    private final PdpModifyRequestMap requestMap;
-
-    /**
-     * Factory for PAP DAO.
-     */
-    private final PolicyModelsProviderFactoryWrapper modelProviderWrapper;
-
-    /**
-     * Heart beat interval, in milliseconds, to pass to PDPs.
-     */
-    private final long heartBeatMs;
-
     /**
      * Constructs the object.
      */
     public PdpStatusMessageHandler() {
-        modelProviderWrapper = Registry.get(PapConstants.REG_PAP_DAO_FACTORY, PolicyModelsProviderFactoryWrapper.class);
-        updateLock = Registry.get(PapConstants.REG_PDP_MODIFY_LOCK, Object.class);
-        requestMap = Registry.get(PapConstants.REG_PDP_MODIFY_MAP, PdpModifyRequestMap.class);
-
-        PapParameterGroup params = ParameterService.get(PAP_GROUP_PARAMS_NAME);
-        heartBeatMs = params.getPdpParameters().getHeartBeatMs();
+        super(true);
     }
 
     /**
@@ -154,15 +120,21 @@ public class PdpStatusMessageHandler {
             final Map<Integer, PdpGroup> selectedPdpGroups, final List<PdpGroup> pdpGroups) {
         PdpGroup emptyPdpGroup = null;
         for (final PdpGroup pdpGroup : pdpGroups) {
-            for (final PdpSubGroup pdpSubGroup : pdpGroup.getPdpSubgroups()) {
-                if (message.getSupportedPolicyTypes().containsAll(pdpSubGroup.getSupportedPolicyTypes())) {
-                    if (pdpSubGroup.getCurrentInstanceCount() == 0) {
-                        emptyPdpGroup = pdpGroup;
-                    } else {
-                        selectedPdpGroups.put(
-                                pdpSubGroup.getDesiredInstanceCount() - pdpSubGroup.getCurrentInstanceCount(),
-                                pdpGroup);
-                    }
+            emptyPdpGroup = selectPdpSubGroupsForRegistration(message, selectedPdpGroups, pdpGroup, emptyPdpGroup);
+        }
+        return emptyPdpGroup;
+    }
+
+    private PdpGroup selectPdpSubGroupsForRegistration(final PdpStatus message,
+                    final Map<Integer, PdpGroup> selectedPdpGroups, final PdpGroup pdpGroup, PdpGroup emptyPdpGroup) {
+        for (final PdpSubGroup pdpSubGroup : pdpGroup.getPdpSubgroups()) {
+            if (message.getSupportedPolicyTypes().containsAll(pdpSubGroup.getSupportedPolicyTypes())) {
+                if (pdpSubGroup.getCurrentInstanceCount() == 0) {
+                    emptyPdpGroup = pdpGroup;
+                } else {
+                    selectedPdpGroups.put(
+                            pdpSubGroup.getDesiredInstanceCount() - pdpSubGroup.getCurrentInstanceCount(),
+                            pdpGroup);
                 }
             }
         }
@@ -308,40 +280,4 @@ public class PdpStatusMessageHandler {
         LOGGER.debug("Sent PdpUpdate message - {}", pdpUpdatemessage);
         LOGGER.debug("Sent PdpStateChange message - {}", pdpStateChangeMessage);
     }
-
-    private PdpUpdate createPdpUpdateMessage(final String pdpGroupName, final PdpSubGroup subGroup,
-            final String pdpInstanceId, final PolicyModelsProvider databaseProvider) throws PfModelException {
-
-        final PdpUpdate update = new PdpUpdate();
-        update.setName(pdpInstanceId);
-        update.setPdpGroup(pdpGroupName);
-        update.setPdpSubgroup(subGroup.getPdpType());
-        update.setPolicies(getToscaPolicies(subGroup, databaseProvider));
-        update.setPdpHeartbeatIntervalMs(heartBeatMs);
-
-        LOGGER.debug("Created PdpUpdate message - {}", update);
-        return update;
-    }
-
-    private List<ToscaPolicy> getToscaPolicies(final PdpSubGroup subGroup, final PolicyModelsProvider databaseProvider)
-            throws PfModelException {
-        final List<ToscaPolicy> policies = new ArrayList<>();
-        for (final ToscaPolicyIdentifier policyIdentifier : subGroup.getPolicies()) {
-            policies.addAll(databaseProvider.getPolicyList(policyIdentifier.getName(), policyIdentifier.getVersion()));
-        }
-        LOGGER.debug("Created ToscaPolicy list - {}", policies);
-        return policies;
-    }
-
-    private PdpStateChange createPdpStateChangeMessage(final String pdpGroupName, final PdpSubGroup subGroup,
-            final String pdpInstanceId, final PdpState pdpState) {
-
-        final PdpStateChange stateChange = new PdpStateChange();
-        stateChange.setName(pdpInstanceId);
-        stateChange.setPdpGroup(pdpGroupName);
-        stateChange.setPdpSubgroup(subGroup.getPdpType());
-        stateChange.setState(pdpState == null ? PdpState.ACTIVE : pdpState);
-        LOGGER.debug("Created PdpStateChange message - {}", stateChange);
-        return stateChange;
-    }
 }
diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/TimerManager.java b/main/src/main/java/org/onap/policy/pap/main/comm/TimerManager.java
index f19d7db8..99677fb8 100644
--- a/main/src/main/java/org/onap/policy/pap/main/comm/TimerManager.java
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/TimerManager.java
@@ -285,7 +285,7 @@ public class TimerManager implements Runnable {
                     return false;
                 }
 
-                logger.debug("{} timer " + cancelMsg + " {}", TimerManager.this.name, this);
+                logger.debug("{} timer {} {}", TimerManager.this.name, cancelMsg, this);
 
                 return true;
             }
diff --git a/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupStateChangeProvider.java b/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupStateChangeProvider.java
index 59bbef6c..97a214c7 100644
--- a/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupStateChangeProvider.java
+++ b/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupStateChangeProvider.java
@@ -1,6 +1,7 @@
 /*-
  * ============LICENSE_START=======================================================
  *  Copyright (C) 2019 Nordix Foundation.
+ *  Modifications Copyright (C) 2019 AT&T Intellectual Property.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,13 +21,9 @@
 
 package org.onap.policy.pap.main.rest;
 
-import java.util.ArrayList;
 import java.util.List;
-
 import javax.ws.rs.core.Response;
-
 import org.apache.commons.lang3.tuple.Pair;
-import org.onap.policy.common.utils.services.Registry;
 import org.onap.policy.models.base.PfModelException;
 import org.onap.policy.models.pap.concepts.PdpGroupStateChangeResponse;
 import org.onap.policy.models.pdp.concepts.Pdp;
@@ -37,11 +34,7 @@ import org.onap.policy.models.pdp.concepts.PdpSubGroup;
 import org.onap.policy.models.pdp.concepts.PdpUpdate;
 import org.onap.policy.models.pdp.enums.PdpState;
 import org.onap.policy.models.provider.PolicyModelsProvider;
-import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy;
-import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifier;
-import org.onap.policy.pap.main.PapConstants;
-import org.onap.policy.pap.main.PolicyModelsProviderFactoryWrapper;
-import org.onap.policy.pap.main.comm.PdpModifyRequestMap;
+import org.onap.policy.pap.main.comm.PdpMessageGenerator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -50,32 +43,15 @@ import org.slf4j.LoggerFactory;
  *
  * @author Ram Krishna Verma (ram.krishna.verma@est.tech)
  */
-public class PdpGroupStateChangeProvider {
+public class PdpGroupStateChangeProvider extends PdpMessageGenerator {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(PdpGroupStateChangeProvider.class);
 
     /**
-     * Lock used when updating PDPs.
-     */
-    private final Object updateLock;
-
-    /**
-     * Used to send UPDATE and STATE-CHANGE requests to the PDPs.
-     */
-    private final PdpModifyRequestMap requestMap;
-
-    /**
-     * Factory for PAP DAO.
-     */
-    PolicyModelsProviderFactoryWrapper modelProviderWrapper;
-
-    /**
      * Constructs the object.
      */
     public PdpGroupStateChangeProvider() {
-        modelProviderWrapper = Registry.get(PapConstants.REG_PAP_DAO_FACTORY, PolicyModelsProviderFactoryWrapper.class);
-        updateLock = Registry.get(PapConstants.REG_PDP_MODIFY_LOCK, Object.class);
-        requestMap = Registry.get(PapConstants.REG_PDP_MODIFY_MAP, PdpModifyRequestMap.class);
+        super(false);
     }
 
     /**
@@ -166,39 +142,4 @@ public class PdpGroupStateChangeProvider {
             }
         }
     }
-
-    private PdpUpdate createPdpUpdateMessage(final String pdpGroupName, final PdpSubGroup subGroup,
-            final String pdpInstanceId, final PolicyModelsProvider databaseProvider) throws PfModelException {
-
-        final PdpUpdate update = new PdpUpdate();
-        update.setName(pdpInstanceId);
-        update.setPdpGroup(pdpGroupName);
-        update.setPdpSubgroup(subGroup.getPdpType());
-        update.setPolicies(getToscaPolicies(subGroup, databaseProvider));
-
-        LOGGER.debug("Created PdpUpdate message - {}", update);
-        return update;
-    }
-
-    private List<ToscaPolicy> getToscaPolicies(final PdpSubGroup subGroup, final PolicyModelsProvider databaseProvider)
-            throws PfModelException {
-        final List<ToscaPolicy> policies = new ArrayList<>();
-        for (final ToscaPolicyIdentifier policyIdentifier : subGroup.getPolicies()) {
-            policies.addAll(databaseProvider.getPolicyList(policyIdentifier.getName(), policyIdentifier.getVersion()));
-        }
-        LOGGER.debug("Created ToscaPolicy list - {}", policies);
-        return policies;
-    }
-
-    private PdpStateChange createPdpStateChangeMessage(final String pdpGroupName, final PdpSubGroup subGroup,
-            final String pdpInstanceId, final PdpState pdpState) {
-
-        final PdpStateChange stateChange = new PdpStateChange();
-        stateChange.setName(pdpInstanceId);
-        stateChange.setPdpGroup(pdpGroupName);
-        stateChange.setPdpSubgroup(subGroup.getPdpType());
-        stateChange.setState(pdpState);
-        LOGGER.debug("Created PdpStateChange message - {}", stateChange);
-        return stateChange;
-    }
 }