From c967bf7387a7586a13a2ae7d4f0e258e3f524a38 Mon Sep 17 00:00:00 2001 From: ramverma Date: Tue, 9 Apr 2019 21:39:55 +0000 Subject: 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 --- .../pap/main/comm/PdpStatusMessageHandler.java | 59 +++++++++++++++------- .../pap/main/rest/PdpGroupStateChangeProvider.java | 48 +++++++++++------- .../policy/pap/main/rest/depundep/SessionData.java | 6 +-- 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; @@ -54,24 +53,48 @@ 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 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 pdpGroups = databaseProvider.getFilteredPdpGroups(filter); for (final PdpGroup pdpGroup : pdpGroups) { subGroup = findPdpSubGroup(message, pdpGroup); @@ -130,7 +154,7 @@ public class PdpStatusMessageHandler { Optional 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 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 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 activePdpGroups = databaseProvider.getFilteredPdpGroups(filter); final List 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 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 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 groups = dao.getFilteredPdpGroups(filter); -- cgit 1.2.3-korg