Merge "Undeploy policies when deploy fails"

14 files changed, 653 insertions, 13 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 49ded962..cb9d51b8 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
@@ -21,10 +21,13 @@
 package org.onap.policy.pap.main.comm;
 
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import lombok.Setter;
 import org.onap.policy.models.base.PfModelException;
 import org.onap.policy.models.pdp.concepts.Pdp;
 import org.onap.policy.models.pdp.concepts.PdpGroup;
@@ -35,6 +38,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.ToscaPolicyIdentifier;
 import org.onap.policy.pap.main.PolicyModelsProviderFactoryWrapper;
 import org.onap.policy.pap.main.comm.msgdata.Request;
 import org.onap.policy.pap.main.comm.msgdata.RequestListener;
@@ -79,6 +83,17 @@ public class PdpModifyRequestMap {
      */
     private final PolicyNotifier policyNotifier;
 
+    /**
+     * Used to undeploy policies from the system, when they cannot be deployed to a PDP.
+     *
+     * <p/>
+     * Note: there's a "catch-22" here. The request map needs an undeployer, but the
+     * undeployer needs the request map. Consequently, the request map is created first,
+     * then the undeployer, and finally, this field is set.
+     */
+    @Setter
+    private PolicyUndeployer policyUndeployer;
+
 
     /**
      * Constructs the object.
@@ -335,7 +350,34 @@ public class PdpModifyRequestMap {
 
         @Override
         public void failure(String responsePdpName, String reason) {
-            requestCompleted(responsePdpName);
+            Collection<ToscaPolicyIdentifier> undeployPolicies = requestCompleted(responsePdpName);
+            if (undeployPolicies.isEmpty()) {
+                // nothing to undeploy
+                return;
+            }
+
+            /*
+             * Undeploy the extra policies. Note: this will likely cause a new message to
+             * be assigned to the request, thus we must re-start it after making the
+             * change.
+             */
+            PdpMessage oldmsg = request.getMessage();
+
+            try {
+                logger.warn("undeploy policies from {}:{} that failed to deploy: {}", oldmsg.getPdpGroup(),
+                                oldmsg.getPdpSubgroup(), undeployPolicies);
+                policyUndeployer.undeploy(oldmsg.getPdpGroup(), oldmsg.getPdpSubgroup(), undeployPolicies);
+            } catch (PfModelException | RuntimeException e) {
+                logger.error("cannot undeploy policies {}", undeployPolicies, e);
+            }
+
+            if (request.getMessage() == oldmsg) {
+                // message is unchanged - start the next request
+                startNextRequest(request);
+            } else {
+                // message changed - restart the request
+                request.startPublishing();
+            }
         }
 
         @Override
@@ -347,17 +389,31 @@ public class PdpModifyRequestMap {
          * Handles a request completion, starting the next request, if there is one.
          *
          * @param responsePdpName name of the PDP provided in the response
+         * @return a list of policies to be undeployed
          */
-        private void requestCompleted(String responsePdpName) {
-            if (pdpName.equals(responsePdpName)) {
-                if (pdp2requests.get(pdpName) == requests) {
-                    startNextRequest(request);
-
-                } else {
-                    logger.info("discard old requests for {}", responsePdpName);
-                    requests.stopPublishing();
-                }
+        private Collection<ToscaPolicyIdentifier> requestCompleted(String responsePdpName) {
+            if (!pdpName.equals(responsePdpName)) {
+                return Collections.emptyList();
+            }
+
+            if (pdp2requests.get(pdpName) != requests) {
+                logger.info("discard old requests for {}", responsePdpName);
+                requests.stopPublishing();
+                return Collections.emptyList();
             }
+
+            if (!requests.isFirstInQueue(request)) {
+                logger.error("request is not first in the queue {}", request.getMessage());
+                return Collections.emptyList();
+            }
+
+            Collection<ToscaPolicyIdentifier> undeployPolicies = request.getUndeployPolicies();
+            if (undeployPolicies.isEmpty()) {
+                // nothing to undeploy - just start the next request
+                startNextRequest(request);
+            }
+
+            return undeployPolicies;
         }
 
         @Override
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 92ed596f..6a539a46 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
@@ -108,6 +108,17 @@ public class PdpRequests {
     }
 
     /**
+     * Determines if a request is the first request in the queue.
+     *
+     * @param request request of interest
+     * @return {@code true} if the request is the first in the queue, {@code false}
+     *         otherwise
+     */
+    public boolean isFirstInQueue(Request request) {
+        return (requests.peek() == request);
+    }
+
+    /**
      * Starts publishing the next request in the queue.
      *
      * @param request the request that just completed
diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/PolicyUndeployer.java b/main/src/main/java/org/onap/policy/pap/main/comm/PolicyUndeployer.java
new file mode 100644
index 00000000..ecf34893
--- /dev/null
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/PolicyUndeployer.java
@@ -0,0 +1,39 @@
+/*-
+ * ============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.Collection;
+import org.onap.policy.models.base.PfModelException;
+import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifier;
+
+@FunctionalInterface
+public interface PolicyUndeployer {
+    /**
+     * Undeploys a list of policies.
+     *
+     * @param group name of the group from which to undeploy the policies
+     * @param subgroup name of the subgroup from which to undeploy the policies
+     * @param policies policies to be undeployed
+     * @throws PfModelException if an error occurs
+     */
+    public void undeploy(String group, String subgroup, Collection<ToscaPolicyIdentifier> policies)
+                    throws PfModelException;
+}
diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/Request.java b/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/Request.java
index dcc35cde..7edb9041 100644
--- a/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/Request.java
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/Request.java
@@ -20,8 +20,10 @@
 
 package org.onap.policy.pap.main.comm.msgdata;
 
+import java.util.Collection;
 import org.onap.policy.models.pdp.concepts.PdpMessage;
 import org.onap.policy.models.pdp.concepts.PdpStatus;
+import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifier;
 import org.onap.policy.pap.main.notification.PolicyNotifier;
 
 /**
@@ -94,4 +96,11 @@ public interface Request {
      * @return an error message, if a fatal error has occurred, {@code null} otherwise
      */
     public String checkResponse(PdpStatus response);
+
+    /**
+     * If a request fails, this gets a list of the policies that should be undeployed.
+     *
+     * @return a list of policies to be undeployed
+     */
+    public Collection<ToscaPolicyIdentifier> getUndeployPolicies();
 }
diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/RequestImpl.java b/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/RequestImpl.java
index dc91338e..03a32557 100644
--- a/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/RequestImpl.java
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/RequestImpl.java
@@ -20,6 +20,8 @@
 
 package org.onap.policy.pap.main.comm.msgdata;
 
+import java.util.Collection;
+import java.util.Collections;
 import lombok.AccessLevel;
 import lombok.Getter;
 import lombok.NonNull;
@@ -28,6 +30,7 @@ import org.onap.policy.common.endpoints.event.comm.Topic.CommInfrastructure;
 import org.onap.policy.common.utils.services.ServiceManager;
 import org.onap.policy.models.pdp.concepts.PdpMessage;
 import org.onap.policy.models.pdp.concepts.PdpStatus;
+import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifier;
 import org.onap.policy.pap.main.comm.QueueToken;
 import org.onap.policy.pap.main.comm.TimerManager;
 import org.onap.policy.pap.main.notification.PolicyNotifier;
@@ -308,4 +311,12 @@ public abstract class RequestImpl implements Request {
 
         return null;
     }
+
+    /**
+     * Just returns an empty list.
+     */
+    @Override
+    public Collection<ToscaPolicyIdentifier> getUndeployPolicies() {
+        return Collections.emptyList();
+    }
 }
diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/UpdateReq.java b/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/UpdateReq.java
index 23743bc0..69a4e3aa 100644
--- a/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/UpdateReq.java
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/UpdateReq.java
@@ -20,11 +20,13 @@
 
 package org.onap.policy.pap.main.comm.msgdata;
 
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
+import lombok.Getter;
 import org.apache.commons.lang3.StringUtils;
 import org.onap.policy.models.pdp.concepts.PdpMessage;
 import org.onap.policy.models.pdp.concepts.PdpStatus;
@@ -40,6 +42,12 @@ import org.onap.policy.pap.main.parameters.RequestParams;
 public class UpdateReq extends RequestImpl {
 
     /**
+     * Policies to be undeployed if the request fails.
+     */
+    @Getter
+    private Collection<ToscaPolicyIdentifier> undeployPolicies = Collections.emptyList();
+
+    /**
      * Constructs the object, and validates the parameters.
      *
      * @param params configuration parameters
@@ -59,6 +67,9 @@ public class UpdateReq extends RequestImpl {
 
     @Override
     public String checkResponse(PdpStatus response) {
+        // reset the list
+        undeployPolicies = Collections.emptyList();
+
         String reason = super.checkResponse(response);
         if (reason != null) {
             // response isn't for this PDP - don't generate notifications
@@ -78,12 +89,21 @@ public class UpdateReq extends RequestImpl {
             return "subgroup does not match";
         }
 
+        if (message.getPdpSubgroup() == null) {
+            return null;
+        }
+
         // see if the policies match
 
         Set<ToscaPolicyIdentifier> expectedSet = new HashSet<>(alwaysList(message.getPolicies()).stream()
                         .map(ToscaPolicy::getIdentifier).collect(Collectors.toSet()));
 
         if (!actualSet.equals(expectedSet)) {
+            // need to undeploy the policies that are expected, but missing from the
+            // response
+            undeployPolicies = expectedSet;
+            undeployPolicies.removeAll(actualSet);
+
             return "policies do not match";
         }
 
diff --git a/main/src/main/java/org/onap/policy/pap/main/rest/depundep/PolicyUndeployerImpl.java b/main/src/main/java/org/onap/policy/pap/main/rest/depundep/PolicyUndeployerImpl.java
new file mode 100644
index 00000000..f224b131
--- /dev/null
+++ b/main/src/main/java/org/onap/policy/pap/main/rest/depundep/PolicyUndeployerImpl.java
@@ -0,0 +1,121 @@
+/*
+ * ============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.rest.depundep;
+
+import java.util.Collection;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.onap.policy.models.base.PfModelException;
+import org.onap.policy.models.pdp.concepts.Pdp;
+import org.onap.policy.models.pdp.concepts.PdpGroup;
+import org.onap.policy.models.pdp.concepts.PdpSubGroup;
+import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy;
+import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifier;
+import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifierOptVersion;
+import org.onap.policy.pap.main.comm.PolicyUndeployer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Implementation of policy undeployer.
+ */
+public class PolicyUndeployerImpl extends ProviderBase implements PolicyUndeployer {
+    private static final Logger logger = LoggerFactory.getLogger(PolicyUndeployerImpl.class);
+
+
+    /**
+     * Constructs the object.
+     */
+    public PolicyUndeployerImpl() {
+        super();
+    }
+
+    @Override
+    public void undeploy(String group, String subgroup, Collection<ToscaPolicyIdentifier> policies)
+                    throws PfModelException {
+
+        process(new Info(group, subgroup, policies), this::undeployPolicies);
+    }
+
+    /**
+     * Undeploys policies from all PDPs within a subgroup.
+     *
+     * @param data session data
+     * @param policyInfo information about the policies to be undeployed
+     * @throws PfModelException if an error occurs
+     */
+    private void undeployPolicies(SessionData data, Info policyInfo) throws PfModelException {
+        // get group and subgroup
+        PdpGroup group = data.getGroup(policyInfo.group);
+        if (group == null) {
+            return;
+        }
+
+        Optional<PdpSubGroup> optsub = group.getPdpSubgroups().stream()
+                        .filter(subgroup -> subgroup.getPdpType().equals(policyInfo.subgroup)).findAny();
+        if (!optsub.isPresent()) {
+            logger.warn("subgroup {} {} not found", policyInfo.group, policyInfo.subgroup);
+            return;
+        }
+
+        PdpSubGroup subgroup = optsub.get();
+
+        // remove the policies
+        boolean updated = false;
+        Set<String> pdps = subgroup.getPdpInstances().stream().map(Pdp::getInstanceId).collect(Collectors.toSet());
+
+        for (ToscaPolicyIdentifier ident : policyInfo.policies) {
+            if (!subgroup.getPolicies().remove(ident)) {
+                continue;
+            }
+
+            logger.info("remove policy {} {} from subgroup {} {} count={}", ident.getName(), ident.getVersion(),
+                            group.getName(), subgroup.getPdpType(), subgroup.getPolicies().size());
+
+            updated = true;
+            data.trackUndeploy(ident, pdps);
+        }
+
+        // push the updates
+        if (updated) {
+            makeUpdates(data, group, subgroup);
+            data.update(group);
+        }
+    }
+
+    @Override
+    protected Updater makeUpdater(SessionData data, ToscaPolicy policy, ToscaPolicyIdentifierOptVersion desiredPolicy) {
+        throw new UnsupportedOperationException("makeUpdater should not be invoked");
+    }
+
+    private static class Info {
+        private String group;
+        private String subgroup;
+        private Collection<ToscaPolicyIdentifier> policies;
+
+        public Info(String group, String subgroup, Collection<ToscaPolicyIdentifier> policies) {
+            this.group = group;
+            this.subgroup = subgroup;
+            this.policies = policies;
+        }
+    }
+}
diff --git a/main/src/main/java/org/onap/policy/pap/main/startstop/PapActivator.java b/main/src/main/java/org/onap/policy/pap/main/startstop/PapActivator.java
index 1fc9784e..8af66362 100644
--- a/main/src/main/java/org/onap/policy/pap/main/startstop/PapActivator.java
+++ b/main/src/main/java/org/onap/policy/pap/main/startstop/PapActivator.java
@@ -57,6 +57,7 @@ import org.onap.policy.pap.main.rest.PolicyStatusControllerV1;
 import org.onap.policy.pap.main.rest.StatisticsRestControllerV1;
 import org.onap.policy.pap.main.rest.depundep.PdpGroupDeleteControllerV1;
 import org.onap.policy.pap.main.rest.depundep.PdpGroupDeployControllerV1;
+import org.onap.policy.pap.main.rest.depundep.PolicyUndeployerImpl;
 
 /**
  * This class activates Policy Administration (PAP) as a complete service together with all its controllers, listeners &
@@ -218,6 +219,9 @@ public class PapActivator extends ServiceManagerContainer {
                                     .setStateChangeTimers(pdpStChgTimers.get())
                                     .setUpdateTimers(pdpUpdTimers.get())));
                 Registry.register(PapConstants.REG_PDP_MODIFY_MAP, requestMap.get());
+
+                // now that it's registered, we can attach a "policy undeploy" provider
+                requestMap.get().setPolicyUndeployer(new PolicyUndeployerImpl());
             },
             () -> Registry.unregister(PapConstants.REG_PDP_MODIFY_MAP));
 
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 74f8b392..edc70103 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
@@ -28,6 +28,8 @@ import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -35,6 +37,7 @@ import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.stream.Collectors;
@@ -54,6 +57,8 @@ import org.onap.policy.models.pdp.concepts.PdpStatus;
 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.tosca.authorative.concepts.ToscaPolicy;
+import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifier;
 import org.onap.policy.pap.main.comm.msgdata.Request;
 import org.onap.policy.pap.main.comm.msgdata.RequestListener;
 import org.onap.policy.pap.main.parameters.PdpModifyRequestMapParams;
@@ -75,9 +80,18 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     @Captor
     private ArgumentCaptor<List<PdpGroup>> updateCaptor;
 
+    /**
+     * Used to capture input to undeployer.undeploy().
+     */
+    @Captor
+    private ArgumentCaptor<Collection<ToscaPolicyIdentifier>> undeployCaptor;
+
     @Mock
     private PdpRequests requests;
 
+    @Mock
+    private PolicyUndeployer undeployer;
+
     private MyMap map;
     private PdpUpdate update;
     private PdpStateChange change;
@@ -100,6 +114,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         change = makeStateChange(PDP1, MY_STATE);
 
         when(requests.getPdpName()).thenReturn(PDP1);
+        when(requests.isFirstInQueue(any())).thenReturn(true);
 
         response.setName(MY_NAME);
         response.setState(MY_STATE);
@@ -108,6 +123,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         response.setPolicies(Collections.emptyList());
 
         map = new MyMap(mapParams);
+        map.setPolicyUndeployer(undeployer);
     }
 
     @Test
@@ -434,7 +450,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     public void testRemoveFromGroup_DaoEx() throws Exception {
         map.addRequest(change);
 
-        when(dao.getFilteredPdpGroups(any())).thenThrow(new PfModelException(Status.BAD_REQUEST, "expected exception"));
+        when(dao.getFilteredPdpGroups(any())).thenThrow(makeException());
 
         invokeLastRetryHandler(1);
 
@@ -446,6 +462,10 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         assertEquals(2, map.nalloc);
     }
 
+    protected PfModelException makeException() {
+        return new PfModelException(Status.BAD_REQUEST, "expected exception");
+    }
+
     @Test
     public void testRemoveFromGroup_NoGroups() throws Exception {
         map.addRequest(change);
@@ -510,6 +530,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         // invoke the method
         invokeFailureHandler(1);
 
+        verify(undeployer, never()).undeploy(any(), any(), any());
         verify(requests, never()).stopPublishing();
 
         // requests should have been removed from the map so this should allocate another
@@ -517,6 +538,83 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         assertEquals(2, map.nalloc);
     }
 
+    /**
+     * Tests Listener.failure() when something has to be undeployed.
+     */
+    @Test
+    public void testSingletonListenerFailureUndeploy() throws Exception {
+
+        ToscaPolicyIdentifier ident = new ToscaPolicyIdentifier("undeployed", "2.3.4");
+        ToscaPolicy policy = mock(ToscaPolicy.class);
+        when(policy.getIdentifier()).thenReturn(ident);
+
+        // add some policies to the update
+        update.setPolicies(Arrays.asList(policy));
+
+        map.addRequest(update);
+
+        /*
+         * Reconfigure the request when undeploy() is called. Also arrange for undeploy()
+         * to throw an exception.
+         */
+        Request req = getSingletons(1).get(0);
+
+        doAnswer(ans -> {
+            PdpUpdate update2 = new PdpUpdate(update);
+            update2.setPolicies(Collections.emptyList());
+            assertTrue(req.reconfigure(update2));
+            throw makeException();
+        }).when(undeployer).undeploy(any(), any(), any());
+
+        // indicate that all policies failed (because response has no policies)
+        response.setName(PDP1);
+        req.setNotifier(notifier);
+        req.checkResponse(response);
+
+        // invoke the method
+        invokeFailureHandler(1);
+
+        verify(undeployer).undeploy(eq(MY_GROUP), eq(MY_SUBGROUP), undeployCaptor.capture());
+        assertEquals(Arrays.asList(ident).toString(), undeployCaptor.getValue().toString());
+
+        // no effect on the map
+        map.addRequest(update);
+        assertEquals(1, map.nalloc);
+    }
+
+    /**
+     * Tests Listener.failure() when something has to be undeployed, but the message
+     * remains unchanged.
+     */
+    @Test
+    public void testSingletonListenerFailureUndeployMessageUnchanged() throws Exception {
+
+        ToscaPolicyIdentifier ident = new ToscaPolicyIdentifier("msg-unchanged", "8.7.6");
+        ToscaPolicy policy = mock(ToscaPolicy.class);
+        when(policy.getIdentifier()).thenReturn(ident);
+
+        // add some policies to the update
+        update.setPolicies(Arrays.asList(policy));
+
+        map.addRequest(update);
+
+        // indicate that all policies failed (because response has no policies)
+        response.setName(PDP1);
+        Request req = getSingletons(1).get(0);
+        req.setNotifier(notifier);
+        req.checkResponse(response);
+
+        // invoke the method
+        invokeFailureHandler(1);
+
+        verify(undeployer).undeploy(eq(MY_GROUP), eq(MY_SUBGROUP), undeployCaptor.capture());
+        assertEquals(Arrays.asList(ident).toString(), undeployCaptor.getValue().toString());
+
+        // requests should have been removed from the map so this should allocate another
+        map.addRequest(update);
+        assertEquals(2, map.nalloc);
+    }
+
     @Test
     public void testSingletonListenerSuccess() throws Exception {
         map.addRequest(change);
@@ -589,6 +687,23 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     }
 
     @Test
+    public void testRequestCompleted_NotFirstInQueue() throws Exception {
+        map.addRequest(change);
+
+        when(requests.isFirstInQueue(any())).thenReturn(false);
+
+        // invoke the method
+        invokeSuccessHandler(1);
+
+        // should not have called this
+        verify(requests, never()).stopPublishing();
+
+        // no effect on the map
+        map.addRequest(update);
+        assertEquals(1, map.nalloc);
+    }
+
+    @Test
     public void testSingletonListenerRetryCountExhausted() throws Exception {
         map.addRequest(change);
 
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 fb29c193..8c257f0b 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
@@ -181,6 +181,19 @@ public class PdpRequestsTest extends CommonRequestBase {
     }
 
     @Test
+    public void testIsFirstInQueue() {
+        // test with empty queue
+        assertFalse(data.isFirstInQueue(update));
+
+        data.addSingleton(update);
+        assertTrue(data.isFirstInQueue(update));
+
+        data.addSingleton(change);
+        assertTrue(data.isFirstInQueue(update));
+        assertFalse(data.isFirstInQueue(change));
+    }
+
+    @Test
     public void testGetPdpName() {
         assertEquals(PDP1, data.getPdpName());
     }
diff --git a/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/RequestImplTest.java b/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/RequestImplTest.java
index 4ebd532b..4b6c2d9a 100644
--- a/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/RequestImplTest.java
+++ b/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/RequestImplTest.java
@@ -430,6 +430,11 @@ public class RequestImplTest extends CommonRequestBase {
         assertSame(reqParams, req.getParams());
     }
 
+    @Test
+    public void testGetUndeployPolicies() {
+        assertTrue(req.getUndeployPolicies().isEmpty());
+    }
+
     private class MyRequest extends RequestImpl {
 
         public MyRequest(RequestParams params, String name, PdpMessage message) {
diff --git a/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/UpdateReqTest.java b/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/UpdateReqTest.java
index 4aa8075d..bbaf6571 100644
--- a/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/UpdateReqTest.java
+++ b/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/UpdateReqTest.java
@@ -31,7 +31,9 @@ import static org.mockito.Mockito.verify;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.stream.Collectors;
 import org.junit.Before;
 import org.junit.Test;
@@ -79,12 +81,14 @@ public class UpdateReqTest extends CommonRequestBase {
     @Test
     public void testCheckResponse() {
         assertNull(data.checkResponse(response));
+        assertTrue(data.getUndeployPolicies().isEmpty());
         verifyResponse();
 
         // both policy lists null
         update.setPolicies(null);
         response.setPolicies(null);
         assertNull(data.checkResponse(response));
+        assertTrue(data.getUndeployPolicies().isEmpty());
     }
 
     @Test
@@ -92,6 +96,7 @@ public class UpdateReqTest extends CommonRequestBase {
         response.setName(null);
 
         assertEquals("null PDP name", data.checkResponse(response));
+        assertTrue(data.getUndeployPolicies().isEmpty());
         verifyNoResponse();
     }
 
@@ -100,6 +105,7 @@ public class UpdateReqTest extends CommonRequestBase {
         update.setName(null);
 
         assertEquals(null, data.checkResponse(response));
+        assertTrue(data.getUndeployPolicies().isEmpty());
         verifyResponse();
     }
 
@@ -108,6 +114,7 @@ public class UpdateReqTest extends CommonRequestBase {
         response.setPdpGroup(DIFFERENT);
 
         assertEquals("group does not match", data.checkResponse(response));
+        assertTrue(data.getUndeployPolicies().isEmpty());
         verifyResponse();
     }
 
@@ -116,6 +123,20 @@ public class UpdateReqTest extends CommonRequestBase {
         response.setPdpSubgroup(DIFFERENT);
 
         assertEquals("subgroup does not match", data.checkResponse(response));
+        assertTrue(data.getUndeployPolicies().isEmpty());
+        verifyResponse();
+    }
+
+    @Test
+    public void testCheckResponse_NullSubGroup() {
+        update.setPdpSubgroup(null);
+        response.setPdpSubgroup(null);
+
+        // different policy list - should have no impact
+        response.setPolicies(Collections.emptyList());
+
+        assertEquals(null, data.checkResponse(response));
+        assertTrue(data.getUndeployPolicies().isEmpty());
         verifyResponse();
     }
 
@@ -128,6 +149,10 @@ public class UpdateReqTest extends CommonRequestBase {
 
         assertEquals("policies do not match", data.checkResponse(response));
         verifyResponse();
+
+        // the first policy from the original update is all that should be undeployed
+        assertEquals(Collections.singleton(update.getPolicies().get(0).getIdentifier()).toString(),
+                        data.getUndeployPolicies().toString());
     }
 
     @Test
@@ -135,6 +160,7 @@ public class UpdateReqTest extends CommonRequestBase {
         update.setPolicies(null);
 
         assertEquals("policies do not match", data.checkResponse(response));
+        assertTrue(data.getUndeployPolicies().isEmpty());
         verifyResponse();
     }
 
@@ -144,6 +170,10 @@ public class UpdateReqTest extends CommonRequestBase {
 
         assertEquals("policies do not match", data.checkResponse(response));
         verifyResponse();
+
+        // all policies in the update should be undeployed
+        assertEquals(update.getPolicies().stream().map(ToscaPolicy::getIdentifier).collect(Collectors.toList())
+                        .toString(), new TreeSet<>(data.getUndeployPolicies()).toString());
     }
 
     @Test
diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPolicyUndeployerImpl.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPolicyUndeployerImpl.java
new file mode 100644
index 00000000..5ccb7714
--- /dev/null
+++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPolicyUndeployerImpl.java
@@ -0,0 +1,200 @@
+/*-
+ * ============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.rest.depundep;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Set;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.onap.policy.common.utils.services.Registry;
+import org.onap.policy.models.base.PfModelException;
+import org.onap.policy.models.pdp.concepts.Pdp;
+import org.onap.policy.models.pdp.concepts.PdpGroup;
+import org.onap.policy.models.pdp.concepts.PdpSubGroup;
+import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifier;
+
+public class TestPolicyUndeployerImpl extends ProviderSuper {
+    private static final String MY_GROUP = "my-group";
+    private static final String MY_SUBGROUP = "my-subgroup";
+    private static final String MY_SUBGROUP0 = "my-subgroup-0";
+    private static final String PDP1 = "my-pdp-a";
+
+    @Mock
+    private SessionData session;
+
+    @Captor
+    private ArgumentCaptor<Set<String>> pdpCaptor;
+
+    private ToscaPolicyIdentifier ident1;
+    private ToscaPolicyIdentifier ident2;
+    private ToscaPolicyIdentifier ident3;
+    private ToscaPolicyIdentifier ident4;
+    private PdpGroup group;
+    private PdpSubGroup subgroup;
+    private MyProvider prov;
+
+
+    @AfterClass
+    public static void tearDownAfterClass() {
+        Registry.newRegistry();
+    }
+
+    /**
+     * Configures mocks and objects.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Before
+    public void setUp() throws Exception {
+
+        super.setUp();
+
+        ident1 = new ToscaPolicyIdentifier("ident-a", "2.3.1");
+        ident2 = new ToscaPolicyIdentifier("ident-b", "2.3.2");
+        ident3 = new ToscaPolicyIdentifier("ident-c", "2.3.3");
+        ident4 = new ToscaPolicyIdentifier("ident-d", "2.3.4");
+
+        group = new PdpGroup();
+        group.setName(MY_GROUP);
+
+        subgroup = new PdpSubGroup();
+        subgroup.setPdpType(MY_SUBGROUP);
+
+        Pdp pdp1 = new Pdp();
+        pdp1.setInstanceId(PDP1);
+
+        subgroup.setPdpInstances(Arrays.asList(pdp1));
+
+        // this subgroup should never be touched
+        PdpSubGroup subgroup0 = new PdpSubGroup();
+        subgroup0.setPdpType(MY_SUBGROUP0);
+        subgroup0.setPolicies(Collections.unmodifiableList(Arrays.asList(ident1, ident2, ident3, ident4)));
+        subgroup.setPdpInstances(Arrays.asList(pdp1));
+
+        group.setPdpSubgroups(Arrays.asList(subgroup0, subgroup));
+
+        when(session.getGroup(MY_GROUP)).thenReturn(group);
+        when(session.getPolicy(any())).thenReturn(policy1);
+
+        prov = new MyProvider();
+    }
+
+    @Test
+    public void testUndeployPolicies() throws PfModelException {
+        subgroup.setPolicies(new LinkedList<>(Arrays.asList(ident1, ident2, ident3, ident4)));
+
+        prov.undeploy(MY_GROUP, MY_SUBGROUP, Arrays.asList(ident1, ident2));
+
+        // group should have been updated
+        verify(session).update(group);
+
+        // subgroup should only have remaining policies
+        assertEquals(Arrays.asList(ident3, ident4).toString(), subgroup.getPolicies().toString());
+
+        // should have generated PDP-UPDATE for the PDP
+        verify(session).addUpdate(any());
+    }
+
+    /**
+     * Tests undeployPolicies() when the policies do not exist in the subgroup.
+     */
+    @Test
+    public void testUndeployPoliciesUnchanged() throws PfModelException {
+        List<ToscaPolicyIdentifier> origlist = Arrays.asList(ident3, ident4);
+        subgroup.setPolicies(new LinkedList<>(origlist));
+
+        prov.undeploy(MY_GROUP, MY_SUBGROUP, Arrays.asList(ident1, ident2));
+
+        // group NOT should have been updated
+        verify(session, never()).update(group);
+
+        // subgroup's policies should be unchanged
+        assertEquals(origlist.toString(), subgroup.getPolicies().toString());
+
+        // should NOT have generated PDP-UPDATE for the PDP
+        verify(session, never()).addUpdate(any());
+    }
+
+    /**
+     * Tests undeployPolicies() when the group is not found.
+     */
+    @Test
+    public void testUndeployPoliciesGroupNotFound() throws PfModelException {
+        // force exception to be thrown if the list is changed
+        subgroup.setPolicies(Collections.unmodifiableList(Arrays.asList(ident1, ident2, ident3, ident4)));
+
+        when(session.getGroup(any())).thenReturn(null);
+
+        prov.undeploy(MY_GROUP, MY_SUBGROUP, Arrays.asList(ident1, ident2));
+
+        // group should have been updated
+        verify(session, never()).update(group);
+
+        // should have generated PDP-UPDATE for the PDP
+        verify(session, never()).addUpdate(any());
+    }
+
+    /**
+     * Tests undeployPolicies() when the subgroup is not found.
+     */
+    @Test
+    public void testUndeployPoliciesSubGroupNotFound() throws PfModelException {
+        // force exception to be thrown if the list is changed
+        subgroup.setPolicies(Collections.unmodifiableList(Arrays.asList(ident1, ident2, ident3, ident4)));
+
+        subgroup.setPdpType(MY_SUBGROUP + "X");
+
+        prov.undeploy(MY_GROUP, MY_SUBGROUP, Arrays.asList(ident1, ident2));
+
+        // group should have been updated
+        verify(session, never()).update(group);
+
+        // should have generated PDP-UPDATE for the PDP
+        verify(session, never()).addUpdate(any());
+    }
+
+    @Test(expected = UnsupportedOperationException.class)
+    public void testMakeUpdater() {
+        prov.makeUpdater(null, null, null);
+    }
+
+
+    private class MyProvider extends PolicyUndeployerImpl {
+
+        @Override
+        protected <T> void process(T request, BiConsumerWithEx<SessionData, T> processor) throws PfModelException {
+            processor.accept(session, request);
+        }
+    }
+}
diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/e2e/PdpGroupStateChangeTest.java b/main/src/test/java/org/onap/policy/pap/main/rest/e2e/PdpGroupStateChangeTest.java
index 6fdd516b..8cfc800c 100644
--- a/main/src/test/java/org/onap/policy/pap/main/rest/e2e/PdpGroupStateChangeTest.java
+++ b/main/src/test/java/org/onap/policy/pap/main/rest/e2e/PdpGroupStateChangeTest.java
@@ -24,6 +24,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 
 import java.util.Collections;
+import java.util.List;
 import javax.ws.rs.client.Entity;
 import javax.ws.rs.client.Invocation;
 import javax.ws.rs.core.Response;
@@ -33,6 +34,7 @@ import org.junit.Test;
 import org.onap.policy.models.pap.concepts.PdpGroupStateChangeResponse;
 import org.onap.policy.models.pdp.concepts.PdpStatus;
 import org.onap.policy.models.pdp.enums.PdpState;
+import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifier;
 
 public class PdpGroupStateChangeTest extends End2EndBase {
     private static final String PDP1 = "pdpAA_1";
@@ -70,13 +72,17 @@ public class PdpGroupStateChangeTest extends End2EndBase {
     public void testMakePassive() throws Exception {
         addGroups("stateChangeGroupDeactivate.json");
 
+        ToscaPolicyIdentifier policy =
+                        new ToscaPolicyIdentifier("onap.restart.tca", "1.0.0");
+        List<ToscaPolicyIdentifier> policies = Collections.singletonList(policy);
+
         PdpStatus status11 = new PdpStatus();
         status11.setName(PDP1);
         status11.setState(PdpState.ACTIVE);
         status11.setPdpGroup(DEACT_GROUP);
         status11.setPdpType(SUBGROUP1);
         status11.setPdpSubgroup(SUBGROUP1);
-        status11.setPolicies(Collections.emptyList());
+        status11.setPolicies(policies);
 
         PdpStatus status12 = makeCopy(status11);
         status12.setState(PdpState.PASSIVE);
@@ -87,7 +93,7 @@ public class PdpGroupStateChangeTest extends End2EndBase {
         status21.setPdpGroup(DEACT_GROUP);
         status21.setPdpType(SUBGROUP1);
         status21.setPdpSubgroup(SUBGROUP1);
-        status21.setPolicies(Collections.emptyList());
+        status21.setPolicies(policies);
 
         PdpStatus status22 = makeCopy(status21);
         status22.setState(PdpState.PASSIVE);