Fix review comments & add updateLock

1) Fixing review comments from previous review.
2) Adding updateLock to registration handler & state change provider.

Change-Id: I9f6e0de1f58190e490b28965c84a3a0c7aa90486
Issue-ID: POLICY-1443
Signed-off-by: ramverma <ram.krishna.verma@est.tech>

3 files changed, 74 insertions, 39 deletions

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 3c871352..f02c8813 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
@@ -30,7 +30,6 @@ import org.onap.policy.models.pdp.concepts.Pdp;
 import org.onap.policy.models.pdp.concepts.PdpGroup;
 import org.onap.policy.models.pdp.concepts.PdpGroupFilter;
 import org.onap.policy.models.pdp.concepts.PdpStateChange;
-import org.onap.policy.models.pdp.concepts.PdpStatistics;
 import org.onap.policy.models.pdp.concepts.PdpStatus;
 import org.onap.policy.models.pdp.concepts.PdpSubGroup;
 import org.onap.policy.models.pdp.concepts.PdpUpdate;
@@ -55,23 +54,47 @@ public class PdpStatusMessageHandler {
     private static final Logger LOGGER = LoggerFactory.getLogger(PdpStatusMessageHandler.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 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);
+    }
+
+    /**
      * Handles the PdpStatus message coming from various PDP's.
      *
      * @param message the PdpStatus message
      */
     public void handlePdpStatus(final PdpStatus message) {
-        final PolicyModelsProviderFactoryWrapper modelProviderWrapper =
-                Registry.get(PapConstants.REG_PAP_DAO_FACTORY, PolicyModelsProviderFactoryWrapper.class);
-        try (PolicyModelsProvider databaseProvider = modelProviderWrapper.create()) {
-            if (message.getPdpGroup() == null && message.getPdpSubgroup() == null) {
-                handlePdpRegistration(message, databaseProvider);
-            } else {
-                handlePdpHeartbeat(message, databaseProvider);
+        synchronized (updateLock) {
+            try (PolicyModelsProvider databaseProvider = modelProviderWrapper.create()) {
+                if (message.getPdpGroup() == null && message.getPdpSubgroup() == null) {
+                    handlePdpRegistration(message, databaseProvider);
+                } else {
+                    handlePdpHeartbeat(message, databaseProvider);
+                }
+            } catch (final PolicyPapException exp) {
+                LOGGER.error("Operation Failed", exp);
+            } catch (final Exception exp) {
+                LOGGER.error("Failed connecting to database provider", exp);
             }
-        } catch (final PolicyPapException exp) {
-            LOGGER.error("Operation Failed", exp);
-        } catch (final Exception exp) {
-            LOGGER.error("Failed connecting to database provider", exp);
         }
     }
 
@@ -89,7 +112,8 @@ public class PdpStatusMessageHandler {
         boolean pdpGroupFound = false;
         Optional<PdpSubGroup> subGroup = null;
         final PdpGroupFilter filter = PdpGroupFilter.builder().pdpType(message.getPdpType())
-                .policyTypeList(message.getSupportedPolicyTypes()).matchPolicyTypesExactly(true).build();
+                .policyTypeList(message.getSupportedPolicyTypes()).matchPolicyTypesExactly(true)
+                .groupState(PdpState.ACTIVE).version(PdpGroupFilter.LATEST_VERSION).build();
         final List<PdpGroup> pdpGroups = databaseProvider.getFilteredPdpGroups(filter);
         for (final PdpGroup pdpGroup : pdpGroups) {
             subGroup = findPdpSubGroup(message, pdpGroup);
@@ -130,7 +154,7 @@ public class PdpStatusMessageHandler {
         Optional<Pdp> pdpInstance = null;
 
         final PdpGroupFilter filter =
-                PdpGroupFilter.builder().name(message.getPdpGroup()).version(PdpGroupFilter.LATEST_VERSION).build();
+                PdpGroupFilter.builder().name(message.getPdpGroup()).groupState(PdpState.ACTIVE).build();
         final List<PdpGroup> pdpGroups = databaseProvider.getFilteredPdpGroups(filter);
         if (!pdpGroups.isEmpty()) {
             final PdpGroup pdpGroup = pdpGroups.get(0);
@@ -175,7 +199,7 @@ public class PdpStatusMessageHandler {
     private void processPdpDetails(final PdpStatus message, final PdpSubGroup pdpSubGroup, final Pdp pdpInstance,
             final PdpGroup pdpGroup, final PolicyModelsProvider databaseProvider) throws PfModelException {
         if (PdpState.TERMINATED.equals(message.getState())) {
-            processPdpTermination(message, pdpSubGroup, pdpInstance, pdpGroup, databaseProvider);
+            processPdpTermination(pdpSubGroup, pdpInstance, pdpGroup, databaseProvider);
         } else if (validatePdpDetails(message, pdpGroup, pdpSubGroup, pdpInstance)) {
             LOGGER.debug("PdpInstance details are correct. Saving current state in DB - {}", pdpInstance);
             updatePdpHealthStatus(message, pdpSubGroup, pdpInstance, pdpGroup, databaseProvider);
@@ -186,8 +210,8 @@ public class PdpStatusMessageHandler {
         }
     }
 
-    private void processPdpTermination(final PdpStatus message, final PdpSubGroup pdpSubGroup, final Pdp pdpInstance,
-            final PdpGroup pdpGroup, final PolicyModelsProvider databaseProvider) throws PfModelException {
+    private void processPdpTermination(final PdpSubGroup pdpSubGroup, final Pdp pdpInstance, final PdpGroup pdpGroup,
+            final PolicyModelsProvider databaseProvider) throws PfModelException {
         pdpSubGroup.getPdpInstances().remove(pdpInstance);
         pdpSubGroup.setCurrentInstanceCount(pdpSubGroup.getCurrentInstanceCount() - 1);
         databaseProvider.updatePdpSubGroup(pdpGroup.getName(), pdpGroup.getVersion(), pdpSubGroup);
@@ -220,7 +244,6 @@ public class PdpStatusMessageHandler {
                 createPdpUpdateMessage(pdpGroupName, subGroup, pdpInstanceId, databaseProvider);
         final PdpStateChange pdpStateChangeMessage =
                 createPdpStateChangeMessage(pdpGroupName, subGroup, pdpInstanceId, pdpState);
-        final PdpModifyRequestMap requestMap = Registry.get(PapConstants.REG_PDP_MODIFY_MAP, PdpModifyRequestMap.class);
         requestMap.addRequest(pdpUpdatemessage, pdpStateChangeMessage);
         LOGGER.debug("Sent PdpUpdate message - {}", pdpUpdatemessage);
         LOGGER.debug("Sent PdpStateChange message - {}", pdpStateChangeMessage);
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 f8032ee3..0ca5f76d 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
@@ -54,6 +54,19 @@ public class PdpGroupStateChangeProvider {
 
     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;
 
     /**
@@ -61,6 +74,8 @@ public class PdpGroupStateChangeProvider {
      */
     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);
     }
 
     /**
@@ -74,19 +89,20 @@ public class PdpGroupStateChangeProvider {
      */
     public Pair<Response.Status, PdpGroupStateChangeResponse> changeGroupState(final String groupName,
             final String groupVersion, final PdpState pdpGroupState) throws PfModelException {
-
-        switch (pdpGroupState) {
-            case ACTIVE:
-                handleActiveState(groupName, groupVersion);
-                break;
-            case PASSIVE:
-                handlePassiveState(groupName, groupVersion);
-                break;
-            default:
-                throw new PfModelException(Response.Status.BAD_REQUEST,
-                        "Only ACTIVE or PASSIVE state changes are allowed");
+        synchronized (updateLock) {
+            switch (pdpGroupState) {
+                case ACTIVE:
+                    handleActiveState(groupName, groupVersion);
+                    break;
+                case PASSIVE:
+                    handlePassiveState(groupName, groupVersion);
+                    break;
+                default:
+                    throw new PfModelException(Response.Status.BAD_REQUEST,
+                            "Only ACTIVE or PASSIVE state changes are allowed");
+            }
+            return Pair.of(Response.Status.OK, new PdpGroupStateChangeResponse());
         }
-        return Pair.of(Response.Status.OK, new PdpGroupStateChangeResponse());
     }
 
     private void handleActiveState(final String groupName, final String groupVersion) throws PfModelException {
@@ -95,13 +111,13 @@ public class PdpGroupStateChangeProvider {
             final List<PdpGroup> activePdpGroups = databaseProvider.getFilteredPdpGroups(filter);
             final List<PdpGroup> pdpGroups = databaseProvider.getPdpGroups(groupName, groupVersion);
             if (activePdpGroups.isEmpty() && !pdpGroups.isEmpty()) {
-                sendPdpMessage(pdpGroups.get(0), PdpState.ACTIVE, databaseProvider);
                 updatePdpGroupAndPdp(databaseProvider, pdpGroups, PdpState.ACTIVE);
+                sendPdpMessage(pdpGroups.get(0), PdpState.ACTIVE, databaseProvider);
             } else if (!pdpGroups.isEmpty() && !activePdpGroups.isEmpty()
                     && !pdpGroups.get(0).getVersion().equals(activePdpGroups.get(0).getVersion())) {
-                sendPdpMessage(pdpGroups.get(0), PdpState.ACTIVE, databaseProvider);
                 updatePdpGroupAndPdp(databaseProvider, pdpGroups, PdpState.ACTIVE);
                 updatePdpGroup(databaseProvider, activePdpGroups, PdpState.PASSIVE);
+                sendPdpMessage(pdpGroups.get(0), PdpState.ACTIVE, databaseProvider);
             }
         }
     }
@@ -110,8 +126,8 @@ public class PdpGroupStateChangeProvider {
         try (PolicyModelsProvider databaseProvider = modelProviderWrapper.create()) {
             final List<PdpGroup> pdpGroups = databaseProvider.getPdpGroups(groupName, groupVersion);
             if (!pdpGroups.isEmpty() && !PdpState.PASSIVE.equals(pdpGroups.get(0).getPdpGroupState())) {
-                sendPdpMessage(pdpGroups.get(0), PdpState.PASSIVE, databaseProvider);
                 updatePdpGroupAndPdp(databaseProvider, pdpGroups, PdpState.PASSIVE);
+                sendPdpMessage(pdpGroups.get(0), PdpState.PASSIVE, databaseProvider);
             }
         }
     }
@@ -146,8 +162,6 @@ public class PdpGroupStateChangeProvider {
                         createPdpUpdateMessage(pdpGroup.getName(), subGroup, pdp.getInstanceId(), databaseProvider);
                 final PdpStateChange pdpStateChangeMessage =
                         createPdpStateChangeMessage(pdpGroup.getName(), subGroup, pdp.getInstanceId(), pdpState);
-                final PdpModifyRequestMap requestMap =
-                        Registry.get(PapConstants.REG_PDP_MODIFY_MAP, PdpModifyRequestMap.class);
                 requestMap.addRequest(pdpUpdatemessage, pdpStateChangeMessage);
                 LOGGER.debug("Sent PdpUpdate message - {}", pdpUpdatemessage);
                 LOGGER.debug("Sent PdpStateChange message - {}", pdpStateChangeMessage);
diff --git a/main/src/main/java/org/onap/policy/pap/main/rest/depundep/SessionData.java b/main/src/main/java/org/onap/policy/pap/main/rest/depundep/SessionData.java
index 0c83a1c2..5194e985 100644
--- a/main/src/main/java/org/onap/policy/pap/main/rest/depundep/SessionData.java
+++ b/main/src/main/java/org/onap/policy/pap/main/rest/depundep/SessionData.java
@@ -22,6 +22,7 @@ package org.onap.policy.pap.main.rest.depundep;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -226,10 +227,7 @@ public class SessionData {
             return data.stream().map(GroupData::getCurrentGroup).collect(Collectors.toList());
         }
 
-        final List<ToscaPolicyTypeIdentifier> policyTypeList = new ArrayList<>(1);
-        policyTypeList.add(type);
-
-        PdpGroupFilter filter = PdpGroupFilter.builder().policyTypeList(policyTypeList).matchPolicyTypesExactly(true)
+        PdpGroupFilter filter = PdpGroupFilter.builder().policyTypeList(Collections.singletonList(type))
                 .groupState(PdpState.ACTIVE).build();
 
         List<PdpGroup> groups = dao.getFilteredPdpGroups(filter);