Remove PDP from all active groups

The code to disable a PDP used the group name, extracted from a
prior PdpUpdate request, to identify the group from which the PDP
should be removed.  This is not sufficient, as the requests tracker
may have never seen an update request.  Therefore, the code was
modified to simply remove the PDP from all active groups.  Also
made removeFromGroups(pdpName) public so that it can be used by
other PAP code.

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

4 files changed, 36 insertions, 85 deletions

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 6a743a31..bc1f175e 100644
--- 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
@@ -20,7 +20,7 @@
 
 package org.onap.policy.pap.main.comm;
 
-import java.util.Collections;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -236,16 +236,13 @@ public class PdpModifyRequestMap {
 
         requests.stopPublishing();
 
-        // don't do anything if we don't have a group
-        String name = requests.getLastGroupName();
-        if (name == null) {
-            logger.warn("no group with which to disable {}", requests.getPdpName());
-            return;
+        // remove the PDP from all groups
+        try {
+            removeFromGroups(requests.getPdpName());
+        } catch (PfModelException e) {
+            logger.info("unable to remove PDP {} from subgroup", requests.getPdpName(), e);
         }
 
-        // remove the PDP from the group
-        removeFromGroup(requests.getPdpName(), name);
-
         // send the state change
         PdpStateChange change = new PdpStateChange();
         change.setName(requests.getPdpName());
@@ -254,38 +251,50 @@ public class PdpModifyRequestMap {
     }
 
     /**
-     * Removes a PDP from its group.
+     * Removes a PDP from all active groups.
      *
      * @param pdpName name of the PDP to be removed
-     * @param groupName name of the group from which it should be removed
+     * @throws PfModelException if an error occurs
      */
-    private void removeFromGroup(String pdpName, String groupName) {
+    public void removeFromGroups(String pdpName) throws PfModelException {
 
         try (PolicyModelsProvider dao = daoFactory.create()) {
 
-            PdpGroupFilter filter = PdpGroupFilter.builder().name(groupName).groupState(PdpState.ACTIVE)
-                            .version(PdpGroupFilter.LATEST_VERSION).build();
-
+            PdpGroupFilter filter = PdpGroupFilter.builder().groupState(PdpState.ACTIVE).build();
             List<PdpGroup> groups = dao.getFilteredPdpGroups(filter);
-            if (groups.isEmpty()) {
-                return;
-            }
+            List<PdpGroup> updates = new ArrayList<>(1);
 
-            PdpGroup group = groups.get(0);
-
-            for (PdpSubGroup subgrp : group.getPdpSubgroups()) {
-                if (removeFromSubgroup(pdpName, group, subgrp)) {
-                    dao.updatePdpGroups(Collections.singletonList(group));
-                    return;
+            for (PdpGroup group : groups) {
+                if (removeFromGroup(pdpName, group)) {
+                    updates.add(group);
                 }
             }
 
-        } catch (PfModelException e) {
-            logger.info("unable to remove PDP {} from subgroup", pdpName, e);
+            if (!updates.isEmpty()) {
+                dao.updatePdpGroups(updates);
+            }
         }
     }
 
     /**
+     * Removes a PDP from a group.
+     *
+     * @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
+     */
+    private boolean removeFromGroup(String pdpName, PdpGroup group) {
+        for (PdpSubGroup subgrp : group.getPdpSubgroups()) {
+            if (removeFromSubgroup(pdpName, group, subgrp)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
      * Removes a PDP from a subgroup.
      *
      * @param pdpName name of the PDP to be removed
@@ -293,9 +302,8 @@ public class PdpModifyRequestMap {
      * @param subgrp subgroup from which to attempt to remove the PDP
      * @return {@code true} if the PDP was removed, {@code false} if the PDP was not in
      *         the group
-     * @throws PfModelException if a DB error occurs
      */
-    private boolean removeFromSubgroup(String pdpName, PdpGroup group, PdpSubGroup subgrp) throws PfModelException {
+    private boolean removeFromSubgroup(String pdpName, PdpGroup group, PdpSubGroup subgrp) {
 
         Iterator<Pdp> iter = subgrp.getPdpInstances().iterator();
 
diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/PdpRequests.java b/main/src/main/java/org/onap/policy/pap/main/comm/PdpRequests.java
index 9fbf36d4..5863b2cb 100644
--- a/main/src/main/java/org/onap/policy/pap/main/comm/PdpRequests.java
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/PdpRequests.java
@@ -22,7 +22,6 @@ package org.onap.policy.pap.main.comm;
 
 import lombok.Getter;
 import org.onap.policy.models.pdp.concepts.PdpMessage;
-import org.onap.policy.models.pdp.concepts.PdpUpdate;
 import org.onap.policy.pap.main.comm.msgdata.Request;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -55,12 +54,6 @@ public class PdpRequests {
      */
     private Request[] singletons = new Request[MAX_PRIORITY];
 
-    /**
-     * Last group name to which the associated PDP was assigned.
-     */
-    @Getter
-    private String lastGroupName;
-
 
     /**
      * Constructs the object.
@@ -72,18 +65,6 @@ public class PdpRequests {
     }
 
     /**
-     * Records the group information from the request.
-     *
-     * @param request the request from which to extract the group information
-     */
-    private void recordGroup(Request request) {
-        PdpMessage message = request.getMessage();
-        if (message instanceof PdpUpdate) {
-            lastGroupName = message.getPdpGroup();
-        }
-    }
-
-    /**
      * Adds a singleton request.
      *
      * @param request the request to be added
@@ -94,8 +75,6 @@ public class PdpRequests {
             throw new IllegalArgumentException("unexpected broadcast for " + pdpName);
         }
 
-        recordGroup(request);
-
         if (checkExisting(request)) {
             // have an existing request that's similar - discard this request
             return;
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 199ebcf1..a92ff95b 100644
--- 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
@@ -293,8 +293,6 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     public void testDisablePdp() {
         map.addRequest(update);
 
-        when(requests.getLastGroupName()).thenReturn(MY_GROUP);
-
         // indicate failure
         invokeFailureHandler(1);
 
@@ -316,8 +314,6 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         map.addRequest(change);
         map.stopPublishing(PDP1);
 
-        when(requests.getLastGroupName()).thenReturn(MY_GROUP);
-
         invokeFailureHandler(1);
 
         // should not have stopped publishing a second time
@@ -338,8 +334,6 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     public void testRemoveFromGroup() throws Exception {
         map.addRequest(change);
 
-        when(requests.getLastGroupName()).thenReturn(MY_GROUP);
-
         PdpGroup group = makeGroup(MY_GROUP, MY_VERSION);
         group.setPdpSubgroups(Arrays.asList(makeSubGroup(MY_SUBGROUP + "a", PDP1 + "a"),
                         makeSubGroup(MY_SUBGROUP, PDP1), makeSubGroup(MY_SUBGROUP + "c", PDP1 + "c")));
@@ -364,8 +358,6 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     public void testRemoveFromGroup_DaoEx() throws Exception {
         map.addRequest(change);
 
-        when(requests.getLastGroupName()).thenReturn(MY_GROUP);
-
         when(dao.getFilteredPdpGroups(any())).thenThrow(new PfModelException(Status.BAD_REQUEST, "expected exception"));
 
         invokeFailureHandler(1);
@@ -382,8 +374,6 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     public void testRemoveFromGroup_NoGroups() throws Exception {
         map.addRequest(change);
 
-        when(requests.getLastGroupName()).thenReturn(MY_GROUP);
-
         invokeFailureHandler(1);
 
         verify(dao, never()).updatePdpGroups(any());
@@ -393,8 +383,6 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     public void testRemoveFromGroup_NoMatchingSubgroup() throws Exception {
         map.addRequest(change);
 
-        when(requests.getLastGroupName()).thenReturn(MY_GROUP);
-
         PdpGroup group = makeGroup(MY_GROUP, MY_VERSION);
         group.setPdpSubgroups(Arrays.asList(makeSubGroup(MY_SUBGROUP, DIFFERENT)));
 
@@ -409,8 +397,6 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     public void testRemoveFromSubgroup() throws Exception {
         map.addRequest(change);
 
-        when(requests.getLastGroupName()).thenReturn(MY_GROUP);
-
         PdpGroup group = makeGroup(MY_GROUP, MY_VERSION);
         group.setPdpSubgroups(Arrays.asList(makeSubGroup(MY_SUBGROUP, PDP1, PDP1 + "x", PDP1 + "y")));
 
diff --git a/main/src/test/java/org/onap/policy/pap/main/comm/PdpRequestsTest.java b/main/src/test/java/org/onap/policy/pap/main/comm/PdpRequestsTest.java
index e219c1d5..1bf73225 100644
--- a/main/src/test/java/org/onap/policy/pap/main/comm/PdpRequestsTest.java
+++ b/main/src/test/java/org/onap/policy/pap/main/comm/PdpRequestsTest.java
@@ -23,7 +23,6 @@ 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.assertFalse;
-import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.never;
@@ -60,27 +59,6 @@ public class PdpRequestsTest extends CommonRequestBase {
     }
 
     @Test
-    public void testRecordGroup_testGetLatestGroupXxx() {
-        assertNull(data.getLastGroupName());
-
-        data.addSingleton(update);
-        assertEquals(MY_GROUP, data.getLastGroupName());
-
-        UpdateReq req = makeUpdateReq(PDP1, MY_GROUP, MY_SUBGROUP);
-        req.getMessage().setPdpGroup(DIFFERENT);
-        data.addSingleton(req);
-        assertEquals(DIFFERENT, data.getLastGroupName());
-
-        // doesn't record info from other message types
-        StateChangeReq req2 = change;
-        req2.getMessage().setPdpGroup(MY_GROUP);
-        data.addSingleton(req2);
-
-        // should be unchanged
-        assertEquals(DIFFERENT, data.getLastGroupName());
-    }
-
-    @Test
     public void testAddSingleton() {
         data.addSingleton(update);