Send pdp-update if PDP response doesn't match DB

Because multiple PAPs can be updating the DB, it's possible that a
pdp-update sent by a PAP does not reflect the latest deployment data
in the DB. To solve that problem, modified code to compare any response
received from a PDP with what's in the DB, potentially generating a new
pdp-update (and/or pdp-state-change).

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

8 files changed, 199 insertions, 17 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 087aa577..bf9f290a 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
@@ -39,6 +39,7 @@ import org.onap.policy.models.pdp.concepts.PdpGroup;
 import org.onap.policy.models.pdp.concepts.PdpGroupFilter;
 import org.onap.policy.models.pdp.concepts.PdpMessage;
 import org.onap.policy.models.pdp.concepts.PdpStateChange;
+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;
@@ -351,6 +352,14 @@ public class PdpModifyRequestMap {
     }
 
     /**
+     * Makes a handler for PDP responses.
+     * @return a response handler
+     */
+    protected PdpStatusMessageHandler makePdpResponseHandler() {
+        return new PdpStatusMessageHandler(params.getParams());
+    }
+
+    /**
      * Listener for singleton request events.
      */
     private class SingletonListener implements RequestListener {
@@ -397,8 +406,20 @@ public class PdpModifyRequestMap {
         }
 
         @Override
-        public void success(String responsePdpName) {
+        public void success(String responsePdpName, PdpStatus response) {
             requestCompleted(responsePdpName);
+
+            if (!(request instanceof UpdateReq)) {
+                // other response types may not include the list of policies
+                return;
+            }
+
+            /*
+             * Update PDP time stamps. Also send pdp-update and pdp-state-change, as
+             * necessary, if the response does not reflect what's in the DB.
+             */
+            var handler = makePdpResponseHandler();
+            handler.handlePdpStatus(response);
         }
 
         /**
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 897a0049..b430bb33 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
@@ -22,8 +22,10 @@
 
 package org.onap.policy.pap.main.comm;
 
+import java.sql.SQLIntegrityConstraintViolationException;
 import java.time.Instant;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
@@ -32,6 +34,7 @@ import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.eclipse.persistence.exceptions.EclipseLinkException;
 import org.onap.policy.models.base.PfModelException;
 import org.onap.policy.models.pdp.concepts.Pdp;
 import org.onap.policy.models.pdp.concepts.PdpGroup;
@@ -90,6 +93,10 @@ public class PdpStatusMessageHandler extends PdpMessageGenerator {
      * @param message the PdpStatus message
      */
     public void handlePdpStatus(final PdpStatus message) {
+        if (message.getPolicies() == null) {
+            message.setPolicies(Collections.emptyList());
+        }
+
         long diffms = System.currentTimeMillis() - message.getTimestampMs();
         if (diffms > params.getMaxMessageAgeMs()) {
             long diffsec = TimeUnit.SECONDS.convert(diffms, TimeUnit.MILLISECONDS);
@@ -107,9 +114,45 @@ public class PdpStatusMessageHandler extends PdpMessageGenerator {
             } catch (final PolicyPapException exp) {
                 LOGGER.error("Operation Failed", exp);
             } catch (final Exception exp) {
-                LOGGER.error("Failed connecting to database provider", exp);
+                if (isDuplicateKeyException(exp)) {
+                    /*
+                     * this is to be expected, if multiple PAPs are processing the same
+                     * heartbeat at a time, thus we log the exception at a trace level
+                     * instead of an error level.
+                     */
+                    LOGGER.info("Failed updating PDP information for {} - may have been added by another PAP",
+                                    message.getName());
+                    LOGGER.trace("Failed updating PDP information for {}", message.getName(), exp);
+                } else {
+                    LOGGER.error("Failed connecting to database provider", exp);
+                }
+            }
+        }
+    }
+
+    /**
+     * Determines if the exception indicates a duplicate key.
+     *
+     * @param thrown exception to check
+     * @return {@code true} if the exception occurred due to a duplicate key
+     */
+    protected static boolean isDuplicateKeyException(Throwable thrown) {
+        while (thrown != null) {
+            if (thrown instanceof SQLIntegrityConstraintViolationException) {
+                return true;
+            }
+
+            if (thrown instanceof EclipseLinkException) {
+                EclipseLinkException ele = (EclipseLinkException) thrown;
+                if (isDuplicateKeyException(ele.getInternalException())) {
+                    return true;
+                }
             }
+
+            thrown = thrown.getCause();
         }
+
+        return false;
     }
 
     private void handlePdpRegistration(final PdpStatus message, final PolicyModelsProvider databaseProvider)
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 31b2df02..ff161469 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
@@ -272,7 +272,7 @@ public abstract class RequestImpl implements Request {
             }
 
             logger.info("{} successful", getName());
-            listener.success(pdpName);
+            listener.success(pdpName, response);
         }
     }
 
diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/RequestListener.java b/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/RequestListener.java
index 13ebfe3c..8fa2b950 100644
--- a/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/RequestListener.java
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/RequestListener.java
@@ -20,6 +20,8 @@
 
 package org.onap.policy.pap.main.comm.msgdata;
 
+import org.onap.policy.models.pdp.concepts.PdpStatus;
+
 /**
  * Listener for request events.
  */
@@ -37,8 +39,9 @@ public interface RequestListener {
      * Indicates that a successful response was received from a PDP.
      *
      * @param pdpName name of the PDP from which the response was received
+     * @param response successful response
      */
-    public void success(String pdpName);
+    public void success(String pdpName, PdpStatus response);
 
     /**
      * Indicates that the retry count was exhausted.
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 78a301f6..ed73c3e1 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
@@ -234,7 +234,7 @@ public class PapActivator extends ServiceManagerContainer {
                     frequencyMs,
                     TimeUnit.MILLISECONDS);
             },
-            () -> pdpExpirationTimer.get().shutdownNow());
+            () -> pdpExpirationTimer.get().shutdown());
 
         addAction("PAP client executor",
             () ->
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 0e9be09f..fd5ff86e 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
@@ -101,6 +101,9 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
     @Mock
     private PolicyUndeployer undeployer;
 
+    @Mock
+    private PdpStatusMessageHandler responseHandler;
+
     private MyMap map;
     private PdpUpdate update;
     private PdpStateChange change;
@@ -149,9 +152,10 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         assertFalse(map.isEmpty());
 
         // indicate success
-        getListener(getSingletons(1).get(0)).success(PDP1);
+        getListener(getSingletons(1).get(0)).success(PDP1, response);
 
         assertTrue(map.isEmpty());
+        verify(responseHandler, never()).handlePdpStatus(response);
     }
 
     @Test
@@ -325,7 +329,9 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         map.addRequest(change);
 
         // indicate success
-        getListener(getSingletons(1).get(0)).success(PDP1);
+        getListener(getSingletons(1).get(0)).success(PDP1, response);
+
+        verify(responseHandler, never()).handlePdpStatus(response);
 
         /*
          * the above should have removed the requests so next time should allocate a new
@@ -344,7 +350,10 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
 
         // indicate success with the update
         when(requests.startNextRequest(updateReq)).thenReturn(true);
-        getListener(updateReq).success(PDP1);
+        getListener(updateReq).success(PDP1, response);
+
+        // should be called for the update
+        verify(responseHandler).handlePdpStatus(response);
 
         // should have started the next request
         verify(requests).startNextRequest(updateReq);
@@ -654,7 +663,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
      * @param count expected number of requests
      */
     private void invokeSuccessHandler(int count) {
-        getListener(getSingletons(count).get(0)).success(PDP1);
+        getListener(getSingletons(count).get(0)).success(PDP1, response);
     }
 
     /**
@@ -764,5 +773,10 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
             ++nalloc;
             return requests;
         }
+
+        @Override
+        protected PdpStatusMessageHandler makePdpResponseHandler() {
+            return responseHandler;
+        }
     }
 }
diff --git a/main/src/test/java/org/onap/policy/pap/main/comm/PdpStatusMessageHandlerTest.java b/main/src/test/java/org/onap/policy/pap/main/comm/PdpStatusMessageHandlerTest.java
new file mode 100644
index 00000000..5129bf5f
--- /dev/null
+++ b/main/src/test/java/org/onap/policy/pap/main/comm/PdpStatusMessageHandlerTest.java
@@ -0,0 +1,101 @@
+/*-
+ * ============LICENSE_START=======================================================
+ * ONAP
+ * ================================================================================
+ * Copyright (C) 2021 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 static org.assertj.core.api.Assertions.assertThat;
+
+import java.sql.SQLIntegrityConstraintViolationException;
+import org.eclipse.persistence.exceptions.EclipseLinkException;
+import org.junit.Test;
+
+public class PdpStatusMessageHandlerTest {
+
+    @Test
+    public void testIsDuplicateKeyException() {
+
+        // @formatter:off
+
+        // null exception
+        assertThat(PdpStatusMessageHandler.isDuplicateKeyException(null)).isFalse();
+
+        // plain exception
+        assertThat(PdpStatusMessageHandler.isDuplicateKeyException(
+                        new Exception()))
+            .isFalse();
+
+        // cause is also plain
+        assertThat(PdpStatusMessageHandler.isDuplicateKeyException(
+                        new Exception(
+                            new Exception())))
+            .isFalse();
+
+        // dup key
+        assertThat(PdpStatusMessageHandler.isDuplicateKeyException(
+                        new SQLIntegrityConstraintViolationException()))
+            .isTrue();
+
+        // cause is dup key
+        assertThat(PdpStatusMessageHandler.isDuplicateKeyException(
+                        new Exception(
+                            new SQLIntegrityConstraintViolationException())))
+            .isTrue();
+
+        // eclipselink exception, no internal exception
+        assertThat(PdpStatusMessageHandler.isDuplicateKeyException(
+                        new MyEclipseLinkException()))
+            .isFalse();
+
+        // eclipselink exception, cause is plain
+        assertThat(PdpStatusMessageHandler.isDuplicateKeyException(
+                        new MyEclipseLinkException(
+                            new Exception())))
+            .isFalse();
+
+        // eclipselink exception, cause is dup
+        assertThat(PdpStatusMessageHandler.isDuplicateKeyException(
+                        new MyEclipseLinkException(
+                            new SQLIntegrityConstraintViolationException())))
+            .isTrue();
+
+        // multiple cause both inside and outside of the eclipselink exception
+        assertThat(PdpStatusMessageHandler.isDuplicateKeyException(
+                        new Exception(
+                            new Exception(
+                                new MyEclipseLinkException(
+                                    new Exception(
+                                        new SQLIntegrityConstraintViolationException()))))))
+            .isTrue();
+
+        // @formatter:on
+    }
+
+    public static class MyEclipseLinkException extends EclipseLinkException {
+        private static final long serialVersionUID = 1L;
+
+        public MyEclipseLinkException() {
+            // do nothing
+        }
+
+        public MyEclipseLinkException(Exception exception) {
+            setInternalException(exception);
+        }
+    }
+}
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 abce7eb3..dd635627 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
@@ -292,7 +292,7 @@ public class RequestImplTest extends CommonRequestBase {
 
         invokeProcessResponse(response);
 
-        verify(listener).success(PDP1);
+        verify(listener).success(PDP1, response);
         verify(listener, never()).failure(any(), any());
         verify(timer).cancel();
     }
@@ -305,7 +305,7 @@ public class RequestImplTest extends CommonRequestBase {
 
         invokeProcessResponse(response);
 
-        verify(listener, never()).success(any());
+        verify(listener, never()).success(any(), any());
         verify(listener, never()).failure(any(), any());
     }
 
@@ -317,7 +317,7 @@ public class RequestImplTest extends CommonRequestBase {
 
         invokeProcessResponse(response);
 
-        verify(listener, never()).success(any());
+        verify(listener, never()).success(any(), any());
         verify(listener, never()).failure(any(), any());
         verify(timer, never()).cancel();
     }
@@ -330,7 +330,7 @@ public class RequestImplTest extends CommonRequestBase {
 
         invokeProcessResponse(response);
 
-        verify(listener, never()).success(any());
+        verify(listener, never()).success(any(), any());
         verify(listener).failure(DIFFERENT, "PDP name does not match");
         verify(timer).cancel();
     }
@@ -390,7 +390,7 @@ public class RequestImplTest extends CommonRequestBase {
 
         invokeProcessResponse(response);
 
-        verify(listener).success(PDP1);
+        verify(listener).success(PDP1, response);
         verify(listener, never()).failure(any(), any());
     }
 
@@ -402,7 +402,7 @@ public class RequestImplTest extends CommonRequestBase {
 
         invokeProcessResponse(response);
 
-        verify(listener, never()).success(any());
+        verify(listener, never()).success(any(), any());
         verify(listener).failure(null, "null PDP name");
     }
 
@@ -414,7 +414,7 @@ public class RequestImplTest extends CommonRequestBase {
 
         invokeProcessResponse(response);
 
-        verify(listener, never()).success(any());
+        verify(listener, never()).success(any(), any());
         verify(listener).failure(DIFFERENT, "PDP name does not match");
     }
 
@@ -427,7 +427,7 @@ public class RequestImplTest extends CommonRequestBase {
 
         invokeProcessResponse(response);
 
-        verify(listener).success(DIFFERENT);
+        verify(listener).success(DIFFERENT, response);
         verify(listener, never()).failure(any(), any());
     }