PAP should discard responses for old requests

This is a more robust solution to the race condition previously
identified with back-to-back deployment requests.  The old fix has
been rolled back and replaced with this fix.

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

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 53d3fbb..eaacee9 100644 (file)
--- 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
@@ -80,12 +80,9 @@ public class PdpRequests {
         }
 
         // try to reconfigure an existing request with the new message
-        //
-        // don't reconfigure the first request
         PdpMessage newMessage = request.getMessage();
-        int count = 0;
         for (Request req : requests) {
-            if (count++ > 0 && req.reconfigure(newMessage)) {
+            if (req.reconfigure(newMessage)) {
                 return;
             }
         }

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 03a3255..d1ac3d1 100644 (file)
--- 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
@@ -2,7 +2,7 @@
  * ============LICENSE_START=======================================================
  * ONAP PAP
  * ================================================================================
- * Copyright (C) 2019 AT&T Intellectual Property. All rights reserved.
+ * Copyright (C) 2019-2020 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.
@@ -250,6 +250,16 @@ public abstract class RequestImpl implements Request {
                 return;
             }
 
+            /*
+             * Note: don't have to verify that getResponse() != null, as this code won't
+             * even be reached if that's the case.
+             */
+            if (!message.getRequestId().equals(response.getResponse().getResponseTo())) {
+                logger.info("{} ignore old response via {} {}: {}", getName(), infra, topic,
+                                response.getResponse().getResponseTo());
+                return;
+            }
+
             svcmgr.stop();
 
             String reason = checkResponse(response);

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 efd380e..414c1db 100644 (file)
--- 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
@@ -2,7 +2,7 @@
  * ============LICENSE_START=======================================================
  * ONAP PAP
  * ================================================================================
- * Copyright (C) 2019 AT&T Intellectual Property. All rights reserved.
+ * Copyright (C) 2019-2020 AT&T Intellectual Property. All rights reserved.
  * Modifications Copyright (C) 2020 Nordix Foundation.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -40,6 +40,7 @@ import org.assertj.core.api.Assertions;
 import org.junit.Before;
 import org.junit.Test;
 import org.onap.policy.models.pdp.concepts.PdpMessage;
+import org.onap.policy.models.pdp.concepts.PdpResponseDetails;
 import org.onap.policy.models.pdp.concepts.PdpStateChange;
 import org.onap.policy.models.pdp.concepts.PdpStatus;
 import org.onap.policy.models.pdp.concepts.PdpUpdate;
@@ -66,6 +67,8 @@ public class RequestImplTest extends CommonRequestBase {
         msg = new PdpStateChange();
 
         response.setName(PDP1);
+        response.setResponse(new PdpResponseDetails());
+        response.getResponse().setResponseTo(msg.getRequestId());
         msg.setName(PDP1);
 
         req = new MyRequest(reqParams, MY_REQ_NAME, msg);
@@ -305,6 +308,19 @@ public class RequestImplTest extends CommonRequestBase {
         verify(listener, never()).failure(any(), any());
     }
 
+    @Test
+    public void testProcessResponse_WrongRequest() {
+        req.startPublishing();
+
+        response.getResponse().setResponseTo(DIFFERENT);
+
+        invokeProcessResponse(response);
+
+        verify(listener, never()).success(any());
+        verify(listener, never()).failure(any(), any());
+        verify(timer, never()).cancel();
+    }
+
     @Test
     public void testProcessResponse_ResponseFailed() {
         req.startPublishing();