summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJim Hahn <jrh3@att.com>2021-06-23 16:27:20 -0400
committerJim Hahn <jrh3@att.com>2021-06-25 15:23:39 -0400
commitf76d2083c9440bac3ac866b30b52a3abedb9292c (patch)
treeae9819860b4c257222567bdca5aabb96d84b62c0
parent1a4b8b99c9903847640363b36beceffcfe44e1ac (diff)
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>
-rw-r--r--main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java23
-rw-r--r--main/src/main/java/org/onap/policy/pap/main/comm/PdpStatusMessageHandler.java45
-rw-r--r--main/src/main/java/org/onap/policy/pap/main/comm/msgdata/RequestImpl.java2
-rw-r--r--main/src/main/java/org/onap/policy/pap/main/comm/msgdata/RequestListener.java5
-rw-r--r--main/src/main/java/org/onap/policy/pap/main/startstop/PapActivator.java2
-rw-r--r--main/src/test/java/org/onap/policy/pap/main/comm/PdpModifyRequestMapTest.java22
-rw-r--r--main/src/test/java/org/onap/policy/pap/main/comm/PdpStatusMessageHandlerTest.java101
-rw-r--r--main/src/test/java/org/onap/policy/pap/main/comm/msgdata/RequestImplTest.java16
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());
}