Update PDP with null group/subgroup when disabled

Modified the code so that, when a PDP is unable to fulfill a request,
an UPDATE is sent to the PDP with group=null and subgroup=null.
Also made updates to eliminate group version due to changes in
policy/models.

Change-Id: I9400ef5f8c365f492113a5b592b9ee6b7218756e
Issue-ID: POLICY-1542
Signed-off-by: Jim Hahn <jrh3@att.com>

diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java b/main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java
index bc1f175..c65ebb3 100644 (file)
--- a/main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java
@@ -237,8 +237,9 @@ public class PdpModifyRequestMap {
         requests.stopPublishing();
 
         // remove the PDP from all groups
+        boolean removed = false;
         try {
-            removeFromGroups(requests.getPdpName());
+            removed = removeFromGroups(requests.getPdpName());
         } catch (PfModelException e) {
             logger.info("unable to remove PDP {} from subgroup", requests.getPdpName(), e);
         }
@@ -247,16 +248,28 @@ public class PdpModifyRequestMap {
         PdpStateChange change = new PdpStateChange();
         change.setName(requests.getPdpName());
         change.setState(PdpState.PASSIVE);
-        addRequest(change);
+
+        if (removed) {
+            // send an update, too
+            PdpUpdate update = new PdpUpdate();
+            update.setName(requests.getPdpName());
+
+            addRequest(update, change);
+
+        } else {
+            addRequest(change);
+        }
     }
 
     /**
      * Removes a PDP from all active groups.
      *
      * @param pdpName name of the PDP to be removed
+     * @return {@code true} if the PDP was removed from a group, {@code false} if it was
+     *         not assigned to a group
      * @throws PfModelException if an error occurs
      */
-    public void removeFromGroups(String pdpName) throws PfModelException {
+    public boolean removeFromGroups(String pdpName) throws PfModelException {
 
         try (PolicyModelsProvider dao = daoFactory.create()) {
 
@@ -270,8 +283,12 @@ public class PdpModifyRequestMap {
                 }
             }
 
-            if (!updates.isEmpty()) {
+            if (updates.isEmpty()) {
+                return false;
+
+            } else {
                 dao.updatePdpGroups(updates);
+                return true;
             }
         }
     }
@@ -281,8 +298,8 @@ public class PdpModifyRequestMap {
      *
      * @param pdpName name of the PDP to be removed
      * @param group group from which it should be removed
-     * @return {@code true} if the PDP was removed from the, {@code false} if it was not
-     *         assigned to the group
+     * @return {@code true} if the PDP was removed from the group, {@code false} if it was
+     *         not assigned to the group
      */
     private boolean removeFromGroup(String pdpName, PdpGroup group) {
         for (PdpSubGroup subgrp : group.getPdpSubgroups()) {
@@ -311,8 +328,7 @@ public class PdpModifyRequestMap {
             Pdp instance = iter.next();
 
             if (pdpName.equals(instance.getInstanceId())) {
-                logger.info("removed {} from group={} version={} subgroup={}", pdpName, group.getName(),
-                                group.getVersion(), subgrp.getPdpType());
+                logger.info("removed {} from group={} subgroup={}", pdpName, group.getName(), subgrp.getPdpType());
                 iter.remove();
                 subgrp.setCurrentInstanceCount(subgrp.getPdpInstances().size());
                 return true;

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 f02c881..b3bc140 100644 (file)
--- 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
@@ -113,7 +113,7 @@ public class PdpStatusMessageHandler {
         Optional<PdpSubGroup> subGroup = null;
         final PdpGroupFilter filter = PdpGroupFilter.builder().pdpType(message.getPdpType())
                 .policyTypeList(message.getSupportedPolicyTypes()).matchPolicyTypesExactly(true)
-                .groupState(PdpState.ACTIVE).version(PdpGroupFilter.LATEST_VERSION).build();
+                .groupState(PdpState.ACTIVE).build();
         final List<PdpGroup> pdpGroups = databaseProvider.getFilteredPdpGroups(filter);
         for (final PdpGroup pdpGroup : pdpGroups) {
             subGroup = findPdpSubGroup(message, pdpGroup);
@@ -142,7 +142,7 @@ public class PdpStatusMessageHandler {
 
         pdpSubGroup.setCurrentInstanceCount(pdpSubGroup.getCurrentInstanceCount() + 1);
 
-        databaseProvider.updatePdpSubGroup(pdpGroup.getName(), pdpGroup.getVersion(), pdpSubGroup);
+        databaseProvider.updatePdpSubGroup(pdpGroup.getName(), pdpSubGroup);
 
         LOGGER.debug("Updated PdpSubGroup in DB - {} belonging to PdpGroup - {}", pdpSubGroup, pdpGroup);
     }
@@ -214,7 +214,7 @@ public class PdpStatusMessageHandler {
             final PolicyModelsProvider databaseProvider) throws PfModelException {
         pdpSubGroup.getPdpInstances().remove(pdpInstance);
         pdpSubGroup.setCurrentInstanceCount(pdpSubGroup.getCurrentInstanceCount() - 1);
-        databaseProvider.updatePdpSubGroup(pdpGroup.getName(), pdpGroup.getVersion(), pdpSubGroup);
+        databaseProvider.updatePdpSubGroup(pdpGroup.getName(), pdpSubGroup);
 
         LOGGER.debug("Deleted PdpInstance - {} belonging to PdpSubGroup - {} and PdpGroup - {}", pdpInstance,
                 pdpSubGroup, pdpGroup);
@@ -233,7 +233,7 @@ public class PdpStatusMessageHandler {
     private void updatePdpHealthStatus(final PdpStatus message, final PdpSubGroup pdpSubgroup, final Pdp pdpInstance,
             final PdpGroup pdpGroup, final PolicyModelsProvider databaseProvider) throws PfModelException {
         pdpInstance.setHealthy(message.getHealthy());
-        databaseProvider.updatePdp(pdpGroup.getName(), pdpGroup.getVersion(), pdpSubgroup.getPdpType(), pdpInstance);
+        databaseProvider.updatePdp(pdpGroup.getName(), pdpSubgroup.getPdpType(), pdpInstance);
 
         LOGGER.debug("Updated Pdp in DB - {}", pdpInstance);
     }

diff --git a/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupQueryProvider.java b/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupQueryProvider.java
index 1012fbd..209f6a8 100644 (file)
--- a/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupQueryProvider.java
+++ b/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupQueryProvider.java
@@ -53,7 +53,7 @@ public class PdpGroupQueryProvider {
         final PolicyModelsProviderFactoryWrapper modelProviderWrapper =
                 Registry.get(PapConstants.REG_PAP_DAO_FACTORY, PolicyModelsProviderFactoryWrapper.class);
         try (PolicyModelsProvider databaseProvider = modelProviderWrapper.create()) {
-            pdpGroups.setGroups(databaseProvider.getPdpGroups(null, null));
+            pdpGroups.setGroups(databaseProvider.getPdpGroups(null));
         }
         LOGGER.debug("PdpGroup Query Response - {}", pdpGroups);
         return Pair.of(Response.Status.OK, pdpGroups);

diff --git a/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupStateChangeControllerV1.java b/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupStateChangeControllerV1.java
index 5c53bdc..c9369a2 100644 (file)
--- a/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupStateChangeControllerV1.java
+++ b/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupStateChangeControllerV1.java
@@ -58,13 +58,12 @@ public class PdpGroupStateChangeControllerV1 extends PapRestControllerV1 {
      *
      * @param requestId request ID used in ONAP logging
      * @param groupName name of the PDP group to be deleted
-     * @param version version of the PDP group
      * @param state state of the PDP group
      * @return a response
      */
     // @formatter:off
     @PUT
-    @Path("pdps/groups/{name}/versions/{version}")
+    @Path("pdps/groups/{name}")
     @ApiOperation(value = "Change state of a PDP Group",
         notes = "Changes state of PDP Group, returning optional error details",
         response = PdpGroupStateChangeResponse.class,
@@ -90,11 +89,10 @@ public class PdpGroupStateChangeControllerV1 extends PapRestControllerV1 {
     public Response changeGroupState(
             @HeaderParam(REQUEST_ID_NAME) @ApiParam(REQUEST_ID_PARAM_DESCRIPTION) final UUID requestId,
             @ApiParam(value = "PDP Group Name", required = true) @PathParam("name") final String groupName,
-            @ApiParam(value = "PDP Group Version", required = true) @PathParam("version") final String version,
             @ApiParam(value = "PDP Group State", required = true) @QueryParam("state") final PdpState state) {
 
         try {
-            final Pair<Status, PdpGroupStateChangeResponse> pair = provider.changeGroupState(groupName, version, state);
+            final Pair<Status, PdpGroupStateChangeResponse> pair = provider.changeGroupState(groupName, state);
             return addLoggingHeaders(addVersionControlHeaders(Response.status(pair.getLeft())), requestId)
                     .entity(pair.getRight()).build();
         } catch (final PfModelException exp) {

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 0ca5f76..59bbef6 100644 (file)
--- 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
@@ -82,20 +82,19 @@ public class PdpGroupStateChangeProvider {
      * Changes state of a PDP group.
      *
      * @param groupName name of the PDP group
-     * @param groupVersion version of the PDP group
      * @param pdpGroupState state of the PDP group
      * @return a pair containing the status and the response
      * @throws PfModelException in case of errors
      */
     public Pair<Response.Status, PdpGroupStateChangeResponse> changeGroupState(final String groupName,
-            final String groupVersion, final PdpState pdpGroupState) throws PfModelException {
+            final PdpState pdpGroupState) throws PfModelException {
         synchronized (updateLock) {
             switch (pdpGroupState) {
                 case ACTIVE:
-                    handleActiveState(groupName, groupVersion);
+                    handleActiveState(groupName);
                     break;
                 case PASSIVE:
-                    handlePassiveState(groupName, groupVersion);
+                    handlePassiveState(groupName);
                     break;
                 default:
                     throw new PfModelException(Response.Status.BAD_REQUEST,
@@ -105,16 +104,15 @@ public class PdpGroupStateChangeProvider {
         }
     }
 
-    private void handleActiveState(final String groupName, final String groupVersion) throws PfModelException {
+    private void handleActiveState(final String groupName) throws PfModelException {
         try (PolicyModelsProvider databaseProvider = modelProviderWrapper.create()) {
             final PdpGroupFilter filter = PdpGroupFilter.builder().name(groupName).groupState(PdpState.ACTIVE).build();
             final List<PdpGroup> activePdpGroups = databaseProvider.getFilteredPdpGroups(filter);
-            final List<PdpGroup> pdpGroups = databaseProvider.getPdpGroups(groupName, groupVersion);
+            final List<PdpGroup> pdpGroups = databaseProvider.getPdpGroups(groupName);
             if (activePdpGroups.isEmpty() && !pdpGroups.isEmpty()) {
                 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())) {
+            } else if (!pdpGroups.isEmpty() && !activePdpGroups.isEmpty()) {
                 updatePdpGroupAndPdp(databaseProvider, pdpGroups, PdpState.ACTIVE);
                 updatePdpGroup(databaseProvider, activePdpGroups, PdpState.PASSIVE);
                 sendPdpMessage(pdpGroups.get(0), PdpState.ACTIVE, databaseProvider);
@@ -122,9 +120,9 @@ public class PdpGroupStateChangeProvider {
         }
     }
 
-    private void handlePassiveState(final String groupName, final String groupVersion) throws PfModelException {
+    private void handlePassiveState(final String groupName) throws PfModelException {
         try (PolicyModelsProvider databaseProvider = modelProviderWrapper.create()) {
-            final List<PdpGroup> pdpGroups = databaseProvider.getPdpGroups(groupName, groupVersion);
+            final List<PdpGroup> pdpGroups = databaseProvider.getPdpGroups(groupName);
             if (!pdpGroups.isEmpty() && !PdpState.PASSIVE.equals(pdpGroups.get(0).getPdpGroupState())) {
                 updatePdpGroupAndPdp(databaseProvider, pdpGroups, PdpState.PASSIVE);
                 sendPdpMessage(pdpGroups.get(0), PdpState.PASSIVE, databaseProvider);

diff --git a/main/src/main/java/org/onap/policy/pap/main/startstop/PapDatabaseInitializer.java b/main/src/main/java/org/onap/policy/pap/main/startstop/PapDatabaseInitializer.java
index 5d72b6d..7c1e2e8 100644 (file)
--- a/main/src/main/java/org/onap/policy/pap/main/startstop/PapDatabaseInitializer.java
+++ b/main/src/main/java/org/onap/policy/pap/main/startstop/PapDatabaseInitializer.java
@@ -69,7 +69,7 @@ public class PapDatabaseInitializer {
             final String originalJson = ResourceUtils.getResourceAsString("PapDb.json");
             final PdpGroups pdpGroupsToCreate = standardCoder.decode(originalJson, PdpGroups.class);
             final List<PdpGroup> pdpGroupsFromDb = databaseProvider.getPdpGroups(
-                    pdpGroupsToCreate.getGroups().get(0).getName(), pdpGroupsToCreate.getGroups().get(0).getVersion());
+                    pdpGroupsToCreate.getGroups().get(0).getName());
             if (pdpGroupsFromDb.isEmpty()) {
                 databaseProvider.createPdpGroups(pdpGroupsToCreate.getGroups());
                 LOGGER.debug("Created initial pdpGroup in DB - {}", pdpGroupsToCreate);

diff --git a/main/src/test/java/org/onap/policy/pap/main/comm/PdpModifyRequestMapTest.java b/main/src/test/java/org/onap/policy/pap/main/comm/PdpModifyRequestMapTest.java
index a92ff95..92f5c5f 100644 (file)
--- a/main/src/test/java/org/onap/policy/pap/main/comm/PdpModifyRequestMapTest.java
+++ b/main/src/test/java/org/onap/policy/pap/main/comm/PdpModifyRequestMapTest.java
@@ -23,6 +23,7 @@ package org.onap.policy.pap.main.comm;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
@@ -60,7 +61,6 @@ import org.powermock.reflect.Whitebox;
 
 public class PdpModifyRequestMapTest extends CommonRequestBase {
     private static final String MY_REASON = "my reason";
-    private static final String MY_VERSION = "1.2.3";
 
     /**
      * Used to capture input to dao.createPdpGroups().
@@ -290,7 +290,44 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     }
 
     @Test
-    public void testDisablePdp() {
+    public void testDisablePdp() throws Exception {
+        map.addRequest(update);
+
+        // put the PDP in a group
+        PdpGroup group = makeGroup(MY_GROUP);
+        group.setPdpSubgroups(Arrays.asList(makeSubGroup(MY_SUBGROUP, PDP1)));
+
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group));
+
+        // indicate failure
+        invokeFailureHandler(1);
+
+        // should have stopped publishing
+        verify(requests).stopPublishing();
+
+        // should have published a new update
+        PdpMessage msg2 = getSingletons(3).get(1).getMessage();
+        assertNotNull(msg2);
+        assertTrue(msg2 instanceof PdpUpdate);
+
+        // update should have null group & subgroup
+        update = (PdpUpdate) msg2;
+        assertEquals(PDP1, update.getName());
+        assertNull(update.getPdpGroup());
+        assertNull(update.getPdpSubgroup());
+
+        // should have published a state-change
+        msg2 = getSingletons(3).get(2).getMessage();
+        assertNotNull(msg2);
+        assertTrue(msg2 instanceof PdpStateChange);
+
+        change = (PdpStateChange) msg2;
+        assertEquals(PDP1, change.getName());
+        assertEquals(PdpState.PASSIVE, change.getState());
+    }
+
+    @Test
+    public void testDisablePdp_NotInGroup() {
         map.addRequest(update);
 
         // indicate failure
@@ -334,7 +371,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     public void testRemoveFromGroup() throws Exception {
         map.addRequest(change);
 
-        PdpGroup group = makeGroup(MY_GROUP, MY_VERSION);
+        PdpGroup group = makeGroup(MY_GROUP);
         group.setPdpSubgroups(Arrays.asList(makeSubGroup(MY_SUBGROUP + "a", PDP1 + "a"),
                         makeSubGroup(MY_SUBGROUP, PDP1), makeSubGroup(MY_SUBGROUP + "c", PDP1 + "c")));
 
@@ -383,7 +420,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     public void testRemoveFromGroup_NoMatchingSubgroup() throws Exception {
         map.addRequest(change);
 
-        PdpGroup group = makeGroup(MY_GROUP, MY_VERSION);
+        PdpGroup group = makeGroup(MY_GROUP);
         group.setPdpSubgroups(Arrays.asList(makeSubGroup(MY_SUBGROUP, DIFFERENT)));
 
         when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group));
@@ -397,7 +434,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     public void testRemoveFromSubgroup() throws Exception {
         map.addRequest(change);
 
-        PdpGroup group = makeGroup(MY_GROUP, MY_VERSION);
+        PdpGroup group = makeGroup(MY_GROUP);
         group.setPdpSubgroups(Arrays.asList(makeSubGroup(MY_SUBGROUP, PDP1, PDP1 + "x", PDP1 + "y")));
 
         when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group));
@@ -556,11 +593,10 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         return Whitebox.getInternalState(request, "listener");
     }
 
-    private PdpGroup makeGroup(String name, String version) {
+    private PdpGroup makeGroup(String name) {
         PdpGroup group = new PdpGroup();
 
         group.setName(name);
-        group.setVersion(version);
 
         return group;
     }

diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/TestPdpGroupStateChangeControllerV1.java b/main/src/test/java/org/onap/policy/pap/main/rest/TestPdpGroupStateChangeControllerV1.java
index d824b8b..3220475 100644 (file)
--- a/main/src/test/java/org/onap/policy/pap/main/rest/TestPdpGroupStateChangeControllerV1.java
+++ b/main/src/test/java/org/onap/policy/pap/main/rest/TestPdpGroupStateChangeControllerV1.java
@@ -41,12 +41,12 @@ public class TestPdpGroupStateChangeControllerV1 extends CommonPapRestServer {
 
     @Test
     public void testSwagger() throws Exception {
-        super.testSwagger(GROUP_ENDPOINT + "/{name}/versions/{version}");
+        super.testSwagger(GROUP_ENDPOINT + "/{name}");
     }
 
     @Test
     public void testchangeGroupState() throws Exception {
-        final String uri = GROUP_ENDPOINT + "/my-name/versions/1.2.3?state=ACTIVE";
+        final String uri = GROUP_ENDPOINT + "/my-name?state=ACTIVE";
 
         final Invocation.Builder invocationBuilder = sendRequest(uri);
         Response rawresp = invocationBuilder.put(Entity.json(""));

diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployControllerV1.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployControllerV1.java
index 73a4f0e..6ebad5f 100644 (file)
--- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployControllerV1.java
+++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployControllerV1.java
@@ -93,7 +93,6 @@ public class TestPdpGroupDeployControllerV1 extends CommonPapRestServer {
         PdpGroup group = new PdpGroup();
         group.setName("drools-group");
         group.setDescription("my description");
-        group.setVersion("my-version");
         group.setPdpSubgroups(Arrays.asList(subgrp));
 
         return Entity.entity(group, MediaType.APPLICATION_JSON);