Simplify request handling in PAP 87/97987/2
authorJim Hahn <jrh3@att.com>
Tue, 5 Nov 2019 16:51:04 +0000 (11:51 -0500)
committerJim Hahn <jrh3@att.com>
Tue, 5 Nov 2019 21:38:50 +0000 (16:38 -0500)
PAP handles outgoing requests using a complicated priority queue.
Simplified it significantly.

Change-Id: I9f49dfebd7bf323c5e81bc8ca3459913fa95c43d
Issue-ID: POLICY-2155
Signed-off-by: Jim Hahn <jrh3@att.com>
12 files changed:
main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java
main/src/main/java/org/onap/policy/pap/main/comm/PdpRequests.java
main/src/main/java/org/onap/policy/pap/main/comm/msgdata/Request.java
main/src/main/java/org/onap/policy/pap/main/comm/msgdata/RequestImpl.java
main/src/main/java/org/onap/policy/pap/main/comm/msgdata/StateChangeReq.java
main/src/main/java/org/onap/policy/pap/main/comm/msgdata/UpdateReq.java
main/src/test/java/org/onap/policy/pap/main/comm/CommonRequestBase.java
main/src/test/java/org/onap/policy/pap/main/comm/PdpModifyRequestMapTest.java
main/src/test/java/org/onap/policy/pap/main/comm/PdpRequestsTest.java
main/src/test/java/org/onap/policy/pap/main/comm/msgdata/RequestImplTest.java
main/src/test/java/org/onap/policy/pap/main/comm/msgdata/StateChangeReqTest.java
main/src/test/java/org/onap/policy/pap/main/comm/msgdata/UpdateReqTest.java

index f058da3..6f76320 100644 (file)
@@ -130,11 +130,22 @@ public class PdpModifyRequestMap {
         if (update == null) {
             addRequest(stateChange);
 
-        } else {
+        } else if (stateChange == null) {
+            addRequest(update);
+
+        } else if (stateChange.getState() == PdpState.ACTIVE) {
+            // publish update before activating
             synchronized (modifyLock) {
                 addRequest(update);
                 addRequest(stateChange);
             }
+
+        } else {
+            // deactivate before publishing update
+            synchronized (modifyLock) {
+                addRequest(stateChange);
+                addRequest(update);
+            }
         }
     }
 
index 4e1e923..92ed596 100644 (file)
@@ -20,6 +20,8 @@
 
 package org.onap.policy.pap.main.comm;
 
+import java.util.ArrayDeque;
+import java.util.Queue;
 import lombok.Getter;
 import org.onap.policy.models.pdp.concepts.PdpMessage;
 import org.onap.policy.pap.main.comm.msgdata.Request;
@@ -34,11 +36,6 @@ import org.slf4j.LoggerFactory;
 public class PdpRequests {
     private static final Logger logger = LoggerFactory.getLogger(PdpRequests.class);
 
-    /**
-     * The maximum request priority + 1.
-     */
-    private static final int MAX_PRIORITY = 2;
-
     /**
      * Name of the PDP with which the requests are associated.
      */
@@ -52,14 +49,11 @@ public class PdpRequests {
     private final PolicyNotifier notifier;
 
     /**
-     * Index of request currently being published.
-     */
-    private int curIndex = 0;
-
-    /**
-     * Singleton requests. Items may be {@code null}.
+     * Queue of requests to be published. The first item in the queue is currently being
+     * published. Currently, there will be at most three messages in the queue: PASSIVE,
+     * ACTIVE, and UPDATE.
      */
-    private Request[] singletons = new Request[MAX_PRIORITY];
+    private final Queue<Request> requests = new ArrayDeque<>(3);
 
 
     /**
@@ -85,110 +79,34 @@ public class PdpRequests {
             throw new IllegalArgumentException("unexpected broadcast for " + pdpName);
         }
 
-        if (checkExisting(request)) {
-            // have an existing request that's similar - discard this request
-            return;
-        }
-
-        // no existing request of this type
-
-        int priority = request.getPriority();
-        singletons[priority] = request;
-
-        // stop publishing anything of a lower priority
-        final QueueToken<PdpMessage> token = stopPublishingLowerPriority(priority);
-
-        // start publishing if nothing of higher priority
-        if (higherPrioritySingleton(priority)) {
-            logger.info("{} not publishing due to priority higher than {}", pdpName, priority);
-            return;
+        // try to reconfigure an existing request with the new message
+        PdpMessage newMessage = request.getMessage();
+        for (Request req : requests) {
+            if (req.reconfigure(newMessage)) {
+                return;
+            }
         }
 
-        curIndex = priority;
-        request.startPublishing(token);
-    }
-
-    /**
-     * Checks for an existing request.
-     *
-     * @param request the request of interest
-     * @return {@code true} if a similar request already exists, {@code false} otherwise
-     */
-    private boolean checkExisting(Request request) {
-
-        return checkExistingSingleton(request);
-    }
-
-    /**
-     * Checks for an existing singleton request.
-     *
-     * @param request the request of interest
-     * @return {@code true} if a similar singleton request already exists, {@code false}
-     *         otherwise
-     */
-    private boolean checkExistingSingleton(Request request) {
-
-        Request exsingle = singletons[request.getPriority()];
+        // couldn't reconfigure an existing request - must add the new one
 
-        if (exsingle == null) {
-            return false;
-        }
+        requests.add(request);
 
-        if (exsingle.isSameContent(request)) {
-            // unchanged from existing request
-            logger.info("{} message content unchanged for {}", pdpName, exsingle.getClass().getSimpleName());
-            return true;
+        if (requests.peek() == request) {
+            // this is the first request in the queue - publish it
+            request.startPublishing();
         }
-
-        // reconfigure the existing request
-        PdpMessage message = request.getMessage();
-        exsingle.reconfigure(message, null);
-
-        // still have a singleton in the queue for this request
-        return true;
     }
 
     /**
      * Stops all publishing and removes this PDP from any broadcast messages.
      */
     public void stopPublishing() {
-        // stop singletons
-        for (int x = 0; x < MAX_PRIORITY; ++x) {
-            Request single = singletons[x];
-
-            if (single != null) {
-                singletons[x] = null;
-                single.stopPublishing();
-            }
+        Request request = requests.peek();
+        if (request != null) {
+            request.stopPublishing();
         }
     }
 
-    /**
-     * Stops publishing requests of a lower priority than the specified priority.
-     *
-     * @param priority priority of interest
-     * @return the token that was being used to publish a lower priority request
-     */
-    private QueueToken<PdpMessage> stopPublishingLowerPriority(int priority) {
-
-        // stop singletons
-        for (int x = 0; x < priority; ++x) {
-            Request single = singletons[x];
-
-            if (single != null) {
-                logger.info("{} stop publishing priority {}", pdpName, single.getPriority());
-
-                QueueToken<PdpMessage> token = single.stopPublishing(false);
-                if (token != null) {
-                    // found one that was publishing
-                    return token;
-                }
-            }
-        }
-
-        return null;
-    }
-
     /**
      * Starts publishing the next request in the queue.
      *
@@ -197,64 +115,24 @@ public class PdpRequests {
      *         requests for this PDP have been processed
      */
     public boolean startNextRequest(Request request) {
-        if (!zapRequest(curIndex, request)) {
-            // not at curIndex - look for it in other indices
-            for (int x = 0; x < MAX_PRIORITY; ++x) {
-                if (zapRequest(x, request)) {
-                    break;
-                }
-            }
-        }
-
-        // find/start the highest priority request
-        for (curIndex = MAX_PRIORITY - 1; curIndex >= 0; --curIndex) {
-
-            if (singletons[curIndex] != null) {
-                logger.info("{} start publishing priority {}", pdpName, curIndex);
-
-                singletons[curIndex].startPublishing();
-                return true;
-            }
+        if (request != requests.peek()) {
+            // not the request we're looking for
+            return !requests.isEmpty();
         }
 
-        logger.info("{} has no more requests", pdpName);
-        curIndex = 0;
+        // remove the completed request
+        requests.remove();
 
-        return false;
-    }
-
-    /**
-     * Zaps request pointers, if the request appears at the given index.
-     *
-     * @param index index to examine
-     * @param request request of interest
-     * @return {@code true} if a request pointer was zapped, {@code false} if the request
-     *         did not appear at the given index
-     */
-    private boolean zapRequest(int index, Request request) {
-        if (singletons[index] == request) {
-            singletons[index] = null;
-            return true;
+        // start publishing next request, but don't remove it from the queue
+        Request nextRequest = requests.peek();
+        if (nextRequest == null) {
+            logger.info("{} has no more requests", pdpName);
+            return false;
         }
 
-        return false;
-    }
+        logger.info("{} start publishing next request", pdpName);
 
-    /**
-     * Determines if any singleton request, with a higher priority, is associated with the
-     * PDP.
-     *
-     * @param priority priority of interest
-     *
-     * @return {@code true} if the PDP has a singleton, {@code false} otherwise
-     */
-    private boolean higherPrioritySingleton(int priority) {
-        for (int x = priority + 1; x < MAX_PRIORITY; ++x) {
-            if (singletons[x] != null) {
-                return true;
-            }
-        }
-
-        return false;
+        nextRequest.startPublishing();
+        return true;
     }
 }
index 62aea78..dcc35cd 100644 (file)
@@ -22,7 +22,6 @@ package org.onap.policy.pap.main.comm.msgdata;
 
 import org.onap.policy.models.pdp.concepts.PdpMessage;
 import org.onap.policy.models.pdp.concepts.PdpStatus;
-import org.onap.policy.pap.main.comm.QueueToken;
 import org.onap.policy.pap.main.notification.PolicyNotifier;
 
 /**
@@ -31,14 +30,6 @@ import org.onap.policy.pap.main.notification.PolicyNotifier;
  */
 public interface Request {
 
-    /**
-     * Gets the request priority. Higher priority requests are published before lower
-     * priority requests.
-     *
-     * @return the request priority
-     */
-    public int getPriority();
-
     /**
      * Gets the name with which this data is associated, used for logging purposes. This
      * may be changed when this is reconfigured.
@@ -81,38 +72,20 @@ public interface Request {
      */
     public void startPublishing();
 
-    /**
-     * Starts the publishing process.
-     *
-     * @param token2 token that can be used when publishing, or {@code null} to allocate a
-     *        new token
-     */
-    public void startPublishing(QueueToken<PdpMessage> token2);
-
     /**
      * Unregisters the listener, cancels the timer, and removes the message from the
      * queue.
      */
     public void stopPublishing();
 
-    /**
-     * Unregisters the listener and cancels the timer.
-     *
-     * @param removeFromQueue {@code true} if the message should be removed from the
-     *        queue, {@code false} otherwise
-     * @return the token that was being used to publish the message, or {@code null} if
-     *         the request was not being published
-     */
-    public QueueToken<PdpMessage> stopPublishing(boolean removeFromQueue);
-
     /**
      * Reconfigures the fields based on the {@link #message} type. Suspends publishing,
      * updates the configuration, and then resumes publishing.
      *
      * @param newMessage the new message
-     * @param token2 token to use when publishing, or {@code null} to allocate a new token
+     * @return {@code true} if reconfiguration was successful, {@code false} otherwise
      */
-    public void reconfigure(PdpMessage newMessage, QueueToken<PdpMessage> token2);
+    public boolean reconfigure(PdpMessage newMessage);
 
     /**
      * Checks the response to ensure it is as expected.
@@ -121,12 +94,4 @@ public interface Request {
      * @return an error message, if a fatal error has occurred, {@code null} otherwise
      */
     public String checkResponse(PdpStatus response);
-
-    /**
-     * Determines if this request has the same content as another request.
-     *
-     * @param other request against which to compare
-     * @return {@code true} if the requests have the same content, {@code false} otherwise
-     */
-    public boolean isSameContent(Request other);
 }
index a110ef4..dc91338 100644 (file)
@@ -122,13 +122,19 @@ public abstract class RequestImpl implements Request {
                         .addAction("enqueue",
                             this::enqueue,
                             () -> {
-                                // do not remove from the queue - token may be re-used
+                                // do not remove from the queue - token may be re-used if re-started
                             });
         // @formatter:on
     }
 
-    @Override
-    public void reconfigure(PdpMessage newMessage, QueueToken<PdpMessage> token2) {
+    /**
+     * Reconfigures the current request with a new message. If it's currently publishing a
+     * message, then it stops publishing, replaces the message, and then starts publishing
+     * the new message.
+     *
+     * @param newMessage the new message
+     */
+    protected void reconfigure2(PdpMessage newMessage) {
         if (newMessage.getClass() != message.getClass()) {
             throw new IllegalArgumentException("expecting " + message.getClass().getSimpleName() + " instead of "
                             + newMessage.getClass().getSimpleName());
@@ -136,13 +142,15 @@ public abstract class RequestImpl implements Request {
 
         logger.info("reconfiguring {} with new message", getName());
 
-        if (svcmgr.isAlive()) {
-            token = stopPublishing(false);
-            message = newMessage;
-            startPublishing(token2);
+        synchronized (params.getModifyLock()) {
+            if (svcmgr.isAlive()) {
+                stopPublishing(false);
+                message = newMessage;
+                startPublishing();
 
-        } else {
-            message = newMessage;
+            } else {
+                message = newMessage;
+            }
         }
     }
 
@@ -153,18 +161,11 @@ public abstract class RequestImpl implements Request {
 
     @Override
     public void startPublishing() {
-        startPublishing(null);
-    }
-
-    @Override
-    public void startPublishing(QueueToken<PdpMessage> token2) {
         if (listener == null) {
             throw new IllegalStateException("listener has not been set");
         }
 
         synchronized (params.getModifyLock()) {
-            replaceToken(token2);
-
             if (svcmgr.isAlive()) {
                 logger.info("{} is already publishing", getName());
 
@@ -175,42 +176,22 @@ public abstract class RequestImpl implements Request {
         }
     }
 
-    /**
-     * Replaces the current token with a new token.
-     * @param newToken the new token
-     */
-    private void replaceToken(QueueToken<PdpMessage> newToken) {
-        if (newToken != null) {
-            if (token == null) {
-                token = newToken;
-
-            } else if (token != newToken) {
-                // already have a token - discard the new token
-                newToken.replaceItem(null);
-            }
-        }
-    }
-
     @Override
     public void stopPublishing() {
         stopPublishing(true);
     }
 
-    @Override
-    public QueueToken<PdpMessage> stopPublishing(boolean removeFromQueue) {
-        if (svcmgr.isAlive()) {
-            svcmgr.stop();
+    private void stopPublishing(boolean removeFromQueue) {
+        synchronized (params.getModifyLock()) {
+            if (svcmgr.isAlive()) {
+                svcmgr.stop();
 
-            if (removeFromQueue) {
-                token.replaceItem(null);
-                token = null;
+                if (removeFromQueue) {
+                    token.replaceItem(null);
+                    token = null;
+                }
             }
         }
-
-        QueueToken<PdpMessage> tok = token;
-        token = null;
-
-        return tok;
     }
 
     /**
index 40acd3a..de777c8 100644 (file)
 
 package org.onap.policy.pap.main.comm.msgdata;
 
+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.enums.PdpState;
 import org.onap.policy.pap.main.parameters.RequestParams;
 
 /**
@@ -63,17 +65,26 @@ public class StateChangeReq extends RequestImpl {
     }
 
     @Override
-    public boolean isSameContent(Request other) {
-        if (!(other instanceof StateChangeReq)) {
+    public boolean reconfigure(PdpMessage newMessage) {
+        if (!(newMessage instanceof PdpStateChange)) {
+            // not a state change - no change to this request
             return false;
         }
 
-        PdpStateChange message = (PdpStateChange) other.getMessage();
-        return (getMessage().getState() == message.getState());
-    }
+        PdpStateChange newState = (PdpStateChange) newMessage;
+        PdpStateChange current = getMessage();
 
-    @Override
-    public int getPriority() {
-        return 0;
+        if (current.getState() == newState.getState()) {
+            // content hasn't changed - nothing more to do
+            return true;
+        }
+
+        if (newState.getState() == PdpState.ACTIVE) {
+            // can't replace a non-active state with an active state
+            return false;
+        }
+
+        reconfigure2(newMessage);
+        return true;
     }
 }
index 6b04e72..23743bc 100644 (file)
@@ -26,6 +26,7 @@ import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
 import org.apache.commons.lang3.StringUtils;
+import org.onap.policy.models.pdp.concepts.PdpMessage;
 import org.onap.policy.models.pdp.concepts.PdpStatus;
 import org.onap.policy.models.pdp.concepts.PdpUpdate;
 import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy;
@@ -90,13 +91,25 @@ public class UpdateReq extends RequestImpl {
     }
 
     @Override
-    public boolean isSameContent(Request other) {
-        if (!(other instanceof UpdateReq)) {
+    public boolean reconfigure(PdpMessage newMessage) {
+        if (!(newMessage instanceof PdpUpdate)) {
+            // not an update - no change to this request
             return false;
         }
 
+        PdpUpdate update = (PdpUpdate) newMessage;
+
+        if (isSameContent(update)) {
+            // content hasn't changed - nothing more to do
+            return true;
+        }
+
+        reconfigure2(newMessage);
+        return true;
+    }
+
+    protected final boolean isSameContent(PdpUpdate second) {
         PdpUpdate first = getMessage();
-        PdpUpdate second = (PdpUpdate) other.getMessage();
 
         if (!StringUtils.equals(first.getPdpGroup(), second.getPdpGroup())) {
             return false;
@@ -122,9 +135,4 @@ public class UpdateReq extends RequestImpl {
     private <T> List<T> alwaysList(List<T> list) {
         return (list != null ? list : Collections.emptyList());
     }
-
-    @Override
-    public int getPriority() {
-        return 1;
-    }
 }
index f10abdd..a9cd7d1 100644 (file)
@@ -188,7 +188,6 @@ public class CommonRequestBase {
         UpdateReq req = mock(UpdateReq.class);
 
         when(req.getName()).thenReturn(MY_REQ_NAME);
-        when(req.getPriority()).thenReturn(1);
         when(req.getMessage()).thenReturn(makeUpdate(pdpName, group, subgroup));
 
         return req;
@@ -224,7 +223,6 @@ public class CommonRequestBase {
         StateChangeReq req = mock(StateChangeReq.class);
 
         when(req.getName()).thenReturn(MY_REQ_NAME);
-        when(req.getPriority()).thenReturn(0);
         when(req.getMessage()).thenReturn(makeStateChange(pdpName, state));
 
         return req;
index d0bc200..e90566e 100644 (file)
@@ -22,6 +22,7 @@ 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.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
@@ -117,6 +118,19 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         assertSame(daoFactory, Whitebox.getInternalState(map, "daoFactory"));
     }
 
+    @Test
+    public void testIsEmpty() {
+        assertTrue(map.isEmpty());
+
+        map.addRequest(change);
+        assertFalse(map.isEmpty());
+
+        // indicate success
+        getListener(getSingletons(1).get(0)).success(PDP1);
+
+        assertTrue(map.isEmpty());
+    }
+
     @Test
     public void testStopPublishing() {
         // try with non-existent PDP
@@ -156,8 +170,13 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         assertEquals("pdp_1 PdpUpdate", req.getName());
     }
 
+    /**
+     * Tests addRequest() when two requests are provided and the second is an "activate"
+     * message.
+     */
     @Test
-    public void testAddRequestPdpUpdatePdpStateChange_BothProvided() {
+    public void testAddRequestPdpUpdatePdpStateChange_BothProvided_Active() {
+        change.setState(PdpState.ACTIVE);
         map.addRequest(update, change);
 
         // should have only allocated one request structure
@@ -166,6 +185,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         // both requests should have been added
         List<Request> values = getSingletons(2);
 
+        // update should appear first
         Request req = values.remove(0);
         assertSame(update, req.getMessage());
         assertEquals("pdp_1 PdpUpdate", req.getName());
@@ -175,6 +195,31 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         assertEquals("pdp_1 PdpStateChange", req.getName());
     }
 
+    /**
+     * Tests addRequest() when two requests are provided and the second is "deactivate"
+     * message.
+     */
+    @Test
+    public void testAddRequestPdpUpdatePdpStateChange_BothProvided_Passive() {
+        change.setState(PdpState.PASSIVE);
+        map.addRequest(update, change);
+
+        // should have only allocated one request structure
+        assertEquals(1, map.nalloc);
+
+        // both requests should have been added
+        List<Request> values = getSingletons(2);
+
+        // state-change should appear first
+        Request req = values.remove(0);
+        assertSame(change, req.getMessage());
+        assertEquals("pdp_1 PdpStateChange", req.getName());
+
+        req = values.remove(0);
+        assertSame(update, req.getMessage());
+        assertEquals("pdp_1 PdpUpdate", req.getName());
+    }
+
     @Test
     public void testAddRequestPdpUpdatePdpStateChange() {
         // null should be ok
@@ -308,9 +353,18 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         // should have generated a notification
         verify(notifier).removePdp(PDP1);
 
-        // should have published a new update
+        // should have published a state-change
         PdpMessage msg2 = getSingletons(3).get(1).getMessage();
         assertNotNull(msg2);
+        assertTrue(msg2 instanceof PdpStateChange);
+
+        change = (PdpStateChange) msg2;
+        assertEquals(PDP1, change.getName());
+        assertEquals(PdpState.PASSIVE, change.getState());
+
+        // should have published a new update
+        msg2 = getSingletons(3).get(2).getMessage();
+        assertNotNull(msg2);
         assertTrue(msg2 instanceof PdpUpdate);
 
         // update should have null group & subgroup
@@ -318,15 +372,6 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
         assertEquals(PDP1, update.getName());
         assertNull(update.getPdpGroup());
         assertNull(update.getPdpSubgroup());
-
-        // should have published a state-change
-        msg2 = getSingletons(3).get(2).getMessage();
-        assertNotNull(msg2);
-        assertTrue(msg2 instanceof PdpStateChange);
-
-        change = (PdpStateChange) msg2;
-        assertEquals(PDP1, change.getName());
-        assertEquals(PdpState.PASSIVE, change.getState());
     }
 
     @Test
index ccb13fe..fb29c19 100644 (file)
@@ -23,8 +23,8 @@ 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.assertSame;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -32,7 +32,6 @@ import static org.mockito.Mockito.when;
 
 import org.junit.Before;
 import org.junit.Test;
-import org.onap.policy.models.pdp.concepts.PdpMessage;
 import org.onap.policy.pap.main.comm.msgdata.StateChangeReq;
 import org.onap.policy.pap.main.comm.msgdata.UpdateReq;
 
@@ -56,6 +55,7 @@ public class PdpRequestsTest extends CommonRequestBase {
     @Test
     public void testPdpRequests_testGetLastGroupName() {
         assertEquals(PDP1, data.getPdpName());
+        assertSame(notifier, data.getNotifier());
     }
 
     @Test
@@ -63,7 +63,7 @@ public class PdpRequestsTest extends CommonRequestBase {
         data.addSingleton(update);
 
         verify(update).setNotifier(notifier);
-        verify(update).startPublishing(any());
+        verify(update).startPublishing();
     }
 
     @Test
@@ -75,35 +75,34 @@ public class PdpRequestsTest extends CommonRequestBase {
         data.addSingleton(req2);
 
         // should not publish duplicate
-        verify(req2, never()).startPublishing(any());
+        verify(req2, never()).startPublishing();
+
+        // should not re-publish original
+        verify(update, times(1)).startPublishing();
     }
 
     @Test
-    public void testAddSingleton_LowerPriority() {
+    public void testAddSingleton_AnotherRequest() {
         data.addSingleton(update);
 
-        // add lower priority request
+        // add another request
         data.addSingleton(change);
 
-        // should not publish lower priority request
-        verify(change, never()).startPublishing(any());
-    }
+        // add duplicate requests
+        StateChangeReq change2 = makeStateChangeReq(PDP1, MY_STATE);
+        when(change.reconfigure(change2.getMessage())).thenReturn(true);
+        data.addSingleton(change2);
 
-    @Test
-    public void testAddSingleton_HigherPriority() {
-        data.addSingleton(change);
-
-        QueueToken<PdpMessage> token = new QueueToken<>(change.getMessage());
-        when(change.stopPublishing(false)).thenReturn(token);
+        UpdateReq update2 = makeUpdateReq(PDP1, MY_GROUP, MY_SUBGROUP);
+        when(update.reconfigure(update2.getMessage())).thenReturn(true);
+        data.addSingleton(update2);
 
-        // add higher priority request
-        data.addSingleton(update);
-
-        // should stop publishing lower priority request
-        verify(change).stopPublishing(false);
+        // should still only be publishing the first request
+        verify(update).startPublishing();
 
-        // should start publishing higher priority request
-        verify(update).startPublishing(token);
+        verify(change, never()).startPublishing();
+        verify(change2, never()).startPublishing();
+        verify(update2, never()).startPublishing();
     }
 
     @Test
@@ -113,111 +112,54 @@ public class PdpRequestsTest extends CommonRequestBase {
                         .withMessage("unexpected broadcast for pdp_1");
     }
 
-    @Test
-    public void testCheckExistingSingleton_DoesNotExist() {
-        data.addSingleton(update);
-        verify(update).startPublishing(any());
-    }
-
-    @Test
-    public void testCheckExistingSingleton_SameContent() {
-        data.addSingleton(update);
-
-        // add duplicate update
-        UpdateReq req2 = makeUpdateReq(PDP1, MY_GROUP, MY_SUBGROUP);
-        when(update.isSameContent(req2)).thenReturn(true);
-        data.addSingleton(req2);
-
-        // should not publish duplicate
-        verify(req2, never()).startPublishing(any());
-    }
-
-    @Test
-    public void testCheckExistingSingleton_DifferentContent() {
-        data.addSingleton(update);
-
-        // add different update
-        UpdateReq req2 = makeUpdateReq(PDP1, MY_GROUP, MY_SUBGROUP);
-        when(req2.isSameContent(update)).thenReturn(false);
-        data.addSingleton(req2);
-
-        // should not publish duplicate
-        verify(req2, never()).startPublishing(any());
-
-        // should have re-configured the original
-        verify(update).reconfigure(req2.getMessage(), null);
-
-        // should not have started publishing again
-        verify(update).startPublishing(any());
-    }
-
     @Test
     public void testStopPublishing() {
+        // nothing in the queue - nothing should happen
+        data.stopPublishing();
+
         data.addSingleton(update);
         data.addSingleton(change);
 
         data.stopPublishing();
 
         verify(update).stopPublishing();
-        verify(change).stopPublishing();
+        verify(change, never()).stopPublishing();
 
         // repeat, but with only one request in the queue
         data.addSingleton(update);
         data.stopPublishing();
         verify(update, times(2)).stopPublishing();
-
-        // should not have been invoked again
-        verify(change).stopPublishing();
-    }
-
-    @Test
-    public void testStopPublishingLowerPriority() {
-        data.addSingleton(change);
-
-        QueueToken<PdpMessage> token = new QueueToken<>(change.getMessage());
-        when(change.stopPublishing(false)).thenReturn(token);
-
-        // add higher priority request
-        data.addSingleton(update);
-
-        // should stop publishing lower priority request
-        verify(change).stopPublishing(false);
-
-        // should start publishing higher priority request, with the old token
-        verify(update).startPublishing(token);
-    }
-
-    @Test
-    public void testStopPublishingLowerPriority_NothingPublishing() {
-        data.addSingleton(change);
-
-        // change will return a null token when stopPublishing(false) is called
-
-        data.addSingleton(update);
-
-        // should stop publishing lower priority request
-        verify(change).stopPublishing(false);
-
-        // should start publishing higher priority request
-        verify(update).startPublishing(null);
+        verify(change, never()).stopPublishing();
     }
 
     @Test
     public void testStartNextRequest_NothingToStart() {
         assertFalse(data.startNextRequest(update));
+
+        // should not have published it
+        verify(update, never()).startPublishing();
     }
 
+    /**
+     * Tests addSingleton() when only one request is in the queue.
+     */
     @Test
-    public void testStartNextRequest_ZapCurrent() {
+    public void testStartNextRequest_OneRequest() {
         data.addSingleton(update);
         assertFalse(data.startNextRequest(update));
 
         // invoke again
         assertFalse(data.startNextRequest(update));
+
+        // should have only been published once
+        verify(update, times(1)).startPublishing();
     }
 
+    /**
+     * Tests addSingleton() when more than one request is in the queue.
+     */
     @Test
-    public void testStartNextRequest_ZapOther() {
+    public void testStartNextRequest_MultipleRequests() {
         data.addSingleton(update);
         data.addSingleton(change);
 
@@ -227,61 +169,15 @@ public class PdpRequestsTest extends CommonRequestBase {
         // invoke again
         assertTrue(data.startNextRequest(change));
 
-        // nothing more once update completes
-        assertFalse(data.startNextRequest(update));
-
-        assertFalse(data.startNextRequest(change));
-    }
-
-    @Test
-    public void testStartNextRequest_StartOther() {
-        data.addSingleton(update);
-        data.addSingleton(change);
-
-        assertTrue(data.startNextRequest(change));
-
-        // should have published update twice, with and without a token
-        verify(update).startPublishing(any());
-        verify(update).startPublishing();
-    }
-
-    @Test
-    public void testStartNextRequest_NoOther() {
-        data.addSingleton(update);
-
-        // nothing else to start
-        assertFalse(data.startNextRequest(update));
-
-        verify(update).startPublishing(any());
-        verify(update, never()).startPublishing();
-    }
-
-    @Test
-    public void testHigherPrioritySingleton_True() {
-        data.addSingleton(update);
-        data.addSingleton(change);
-
-        verify(update).startPublishing(any());
-
-        verify(update, never()).startPublishing();
+        // should not have published yet
         verify(change, never()).startPublishing();
-        verify(change, never()).startPublishing(any());
-    }
-
-    @Test
-    public void testHigherPrioritySingleton_FalseWithUpdate() {
-        data.addSingleton(update);
-
-        verify(update).startPublishing(any());
-        verify(update, never()).startPublishing();
-    }
 
-    @Test
-    public void testHigherPrioritySingleton_FalseWithStateChange() {
-        data.addSingleton(change);
+        // should publish "change" once update completes
+        assertTrue(data.startNextRequest(update));
+        verify(change).startPublishing();
 
-        verify(change).startPublishing(any());
-        verify(change, never()).startPublishing();
+        // nothing more in the queue once it completes
+        assertFalse(data.startNextRequest(change));
     }
 
     @Test
index e47f879..4ebd532 100644 (file)
@@ -46,7 +46,6 @@ import org.onap.policy.pap.main.comm.QueueToken;
 import org.onap.policy.pap.main.parameters.RequestParams;
 
 public class RequestImplTest extends CommonRequestBase {
-    private static final int MY_PRIORITY = 10;
 
     private MyRequest req;
     private PdpStatus response;
@@ -86,28 +85,28 @@ public class RequestImplTest extends CommonRequestBase {
     }
 
     @Test
-    public void testReconfigure_WrongMsgClass() {
-        assertThatIllegalArgumentException().isThrownBy(() -> req.reconfigure(new PdpUpdate(), null))
+    public void testReconfigure2_WrongMsgClass() {
+        assertThatIllegalArgumentException().isThrownBy(() -> req.reconfigure(new PdpUpdate()))
                         .withMessage("expecting PdpStateChange instead of PdpUpdate");
     }
 
     @Test
-    public void testReconfigure_NotPublishing() {
+    public void testReconfigure2_NotPublishing() {
 
         // replace the message with a new message
-        req.reconfigure(new PdpStateChange(), null);
+        req.reconfigure(new PdpStateChange());
 
         // nothing should have been placed in the queue
         assertNull(queue.poll());
     }
 
     @Test
-    public void testRequestImpl_testReconfigure_Publishing() {
+    public void testRequestImpl_testReconfigure2_Publishing() {
         req.startPublishing();
 
         // replace the message with a new message
         PdpStateChange msg2 = new PdpStateChange();
-        req.reconfigure(msg2, null);
+        req.reconfigure(msg2);
 
         // should have cancelled the first timer
         verify(timer).cancel();
@@ -128,45 +127,6 @@ public class RequestImplTest extends CommonRequestBase {
         verify(publisher).enqueue(any());
     }
 
-    @Test
-    public void testReconfigure_PublishingNullToken() {
-        req.startPublishing();
-
-        // replace the message with a new message
-        PdpStateChange msg2 = new PdpStateChange();
-        req.reconfigure(msg2, null);
-
-        // should have cancelled the first timer
-        verify(timer).cancel();
-
-        // should only be one token in the queue
-        QueueToken<PdpMessage> token = queue.poll();
-        assertNotNull(token);
-        assertSame(msg2, token.get());
-    }
-
-    @Test
-    public void testReconfigure_PublishingNewToken() {
-        req.startPublishing();
-
-        // null out the original token so it isn't reused
-        QueueToken<PdpMessage> token = queue.poll();
-        assertNotNull(token);
-        token.replaceItem(null);
-
-        QueueToken<PdpMessage> token2 = new QueueToken<>(new PdpStateChange());
-
-        // replace the message with a new message
-        PdpStateChange msg2 = new PdpStateChange();
-        req.reconfigure(msg2, token2);
-
-        // new token should have the new message
-        token = queue.poll();
-        assertSame(msg2, token.get());
-
-        assertNull(queue.poll());
-    }
-
     @Test
     public void testIsPublishing() {
         assertFalse(req.isPublishing());
@@ -178,32 +138,6 @@ public class RequestImplTest extends CommonRequestBase {
         assertFalse(req.isPublishing());
     }
 
-    @Test
-    public void testStartPublishingQueueToken() {
-        req.startPublishing(null);
-
-        assertTrue(req.isPublishing());
-
-        verify(dispatcher).register(eq(msg.getRequestId()), any());
-        verify(timers).register(eq(msg.getRequestId()), any());
-        verify(publisher).enqueue(any());
-
-        QueueToken<PdpMessage> token = queue.poll();
-        assertNotNull(token);
-        assertSame(msg, token.get());
-
-
-        // invoking start() again has no effect - invocation counts remain the same
-        req.startPublishing(null);
-        verify(dispatcher, times(1)).register(any(), any());
-        verify(timers, times(1)).register(any(), any());
-        verify(publisher, times(1)).enqueue(any());
-        assertNull(queue.poll());
-
-        // should NOT have cancelled the timer
-        verify(timer, never()).cancel();
-    }
-
     @Test
     public void testStartPublishingQueueToken_NoListener() {
         req.setListener(null);
@@ -234,53 +168,6 @@ public class RequestImplTest extends CommonRequestBase {
         assertNull(queue.poll());
     }
 
-    @Test
-    public void testReplaceToken_NullNewToken() {
-        req.startPublishing(null);
-        assertSame(msg, queue.poll().get());
-    }
-
-    @Test
-    public void testReplaceToken_NullOldToken() {
-        QueueToken<PdpMessage> token = new QueueToken<>(new PdpStateChange());
-
-        req.startPublishing(token);
-        assertNull(queue.poll());
-        assertSame(msg, token.get());
-    }
-
-    @Test
-    public void testReplaceToken_SameToken() {
-        req.startPublishing();
-
-        QueueToken<PdpMessage> token = queue.poll();
-        req.startPublishing(token);
-
-        // nothing else should have been enqueued
-        assertNull(queue.poll());
-
-        assertSame(msg, token.get());
-    }
-
-    @Test
-    public void testReplaceToken_DifferentToken() {
-        req.startPublishing();
-
-        QueueToken<PdpMessage> token2 = new QueueToken<>(new PdpStateChange());
-        req.startPublishing(token2);
-
-        QueueToken<PdpMessage> token = queue.poll();
-
-        // old token should still have the message
-        assertSame(msg, token.get());
-
-        // should not have added new token to the queue
-        assertNull(queue.poll());
-
-        // new token should have been nulled out
-        assertNull(token2.get());
-    }
-
     @Test
     public void testStopPublishing() {
         // not publishing yet
@@ -304,14 +191,15 @@ public class RequestImplTest extends CommonRequestBase {
 
     @Test
     public void testStopPublishingBoolean_NotPublishing() {
-        assertNull(req.stopPublishing(false));
+        // should not throw an exception
+        req.stopPublishing();
     }
 
     @Test
     public void testStopPublishingBoolean_TruePublishing() {
         req.startPublishing();
 
-        assertNull(req.stopPublishing(true));
+        req.stopPublishing();
 
         // should be nulled out
         QueueToken<PdpMessage> token = queue.poll();
@@ -329,35 +217,13 @@ public class RequestImplTest extends CommonRequestBase {
         assertSame(msg, token2.get());
     }
 
-    @Test
-    public void testStopPublishingBoolean_FalsePublishing() {
-        req.startPublishing();
-
-        QueueToken<PdpMessage> token = req.stopPublishing(false);
-        assertNotNull(token);
-        assertSame(token, queue.poll());
-
-        // should not be nulled out
-        assertSame(msg, token.get());
-
-        verify(dispatcher).unregister(eq(msg.getRequestId()));
-        verify(timer).cancel();
-
-        // if start publishing again - should use a new token
-        req.startPublishing();
-        QueueToken<PdpMessage> token2 = queue.poll();
-        assertNotNull(token2);
-        assertTrue(token2 != token);
-        assertSame(msg, token2.get());
-    }
-
     @Test
     public void testEnqueue() {
         req.startPublishing();
 
         // replace the message with a new message
         PdpStateChange msg2 = new PdpStateChange();
-        req.reconfigure(msg2, null);
+        req.reconfigure(msg2);
 
         // should still only be one token in the queue
         QueueToken<PdpMessage> token = queue.poll();
@@ -370,7 +236,7 @@ public class RequestImplTest extends CommonRequestBase {
 
         // enqueue a new message
         PdpStateChange msg3 = new PdpStateChange();
-        req.reconfigure(msg3, null);
+        req.reconfigure(msg3);
         req.startPublishing();
 
         // a new token should have been placed in the queue
@@ -379,6 +245,18 @@ public class RequestImplTest extends CommonRequestBase {
         assertNull(queue.poll());
         assertNotNull(token2);
         assertSame(msg3, token2.get());
+
+        // zap the token, indicating it's been published
+        token2.replaceItem(null);
+        PdpStateChange msg4 = new PdpStateChange();
+        req.reconfigure(msg4);
+
+        // a new token should have been placed in the queue
+        QueueToken<PdpMessage> token3 = queue.poll();
+        assertTrue(token2 != token3);
+        assertNull(queue.poll());
+        assertNotNull(token3);
+        assertSame(msg4, token3.get());
     }
 
     @Test
@@ -543,7 +421,7 @@ public class RequestImplTest extends CommonRequestBase {
         assertSame(msg, req.getMessage());
 
         PdpStateChange msg2 = new PdpStateChange();
-        req.reconfigure(msg2, null);
+        req.reconfigure(msg2);
         assertSame(msg2, req.getMessage());
     }
 
@@ -559,13 +437,9 @@ public class RequestImplTest extends CommonRequestBase {
         }
 
         @Override
-        public int getPriority() {
-            return MY_PRIORITY;
-        }
-
-        @Override
-        public boolean isSameContent(Request other) {
-            return false;
+        public boolean reconfigure(PdpMessage newMessage) {
+            reconfigure2(newMessage);
+            return true;
         }
     }
 }
index 3553114..e16cd89 100644 (file)
@@ -31,6 +31,7 @@ import org.junit.Test;
 import org.onap.policy.models.pdp.concepts.PdpStateChange;
 import org.onap.policy.models.pdp.concepts.PdpStatus;
 import org.onap.policy.models.pdp.concepts.PdpUpdate;
+import org.onap.policy.models.pdp.enums.PdpState;
 import org.onap.policy.pap.main.comm.CommonRequestBase;
 
 public class StateChangeReqTest extends CommonRequestBase {
@@ -41,6 +42,7 @@ public class StateChangeReqTest extends CommonRequestBase {
 
     /**
      * Sets up.
+     *
      * @throws Exception if an error occurs
      */
     @Before
@@ -92,22 +94,25 @@ public class StateChangeReqTest extends CommonRequestBase {
     }
 
     @Test
-    public void isSameContent() {
-        PdpStateChange msg2 = new PdpStateChange();
-        msg2.setName("world");
-        msg2.setState(MY_STATE);
-        assertTrue(data.isSameContent(new StateChangeReq(reqParams, MY_REQ_NAME, msg2)));
+    public void testReconfigure() {
+        // different message type should fail and leave message unchanged
+        assertFalse(data.reconfigure(new PdpUpdate()));
+        assertSame(msg, data.getMessage());
 
-        // different state
-        msg2.setState(DIFF_STATE);
-        assertFalse(data.isSameContent(new StateChangeReq(reqParams, MY_REQ_NAME, msg2)));
+        // same state - should succeed, but leave message unchanged
+        PdpStateChange msg2 = new PdpStateChange();
+        msg2.setState(msg.getState());
+        assertTrue(data.reconfigure(msg2));
+        assertSame(msg, data.getMessage());
 
-        // different request type
-        assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, new PdpUpdate())));
-    }
+        // change old state to active - should fail and leave message unchanged
+        msg2.setState(PdpState.ACTIVE);
+        assertFalse(data.reconfigure(msg2));
+        assertSame(msg, data.getMessage());
 
-    @Test
-    public void testGetPriority() {
-        assertTrue(data.getPriority() < new UpdateReq(reqParams, MY_REQ_NAME, new PdpUpdate()).getPriority());
+        // different, inactive state - should succeed and install NEW message
+        msg2.setState(PdpState.PASSIVE);
+        assertTrue(data.reconfigure(msg2));
+        assertSame(msg2, data.getMessage());
     }
 }
index 80ed0fb..4aa8075 100644 (file)
@@ -146,19 +146,33 @@ public class UpdateReqTest extends CommonRequestBase {
         verifyResponse();
     }
 
+    @Test
+    public void testReconfigure() {
+        // different message type should fail and leave message unchanged
+        assertFalse(data.reconfigure(new PdpStateChange()));
+        assertSame(update, data.getMessage());
+
+        // same content - should succeed, but leave message unchanged
+        PdpUpdate msg2 = new PdpUpdate(update);
+        assertTrue(data.reconfigure(msg2));
+        assertSame(update, data.getMessage());
+
+        // different content - should succeed and install NEW message
+        msg2.setPdpGroup(DIFFERENT);
+        assertTrue(data.reconfigure(msg2));
+        assertSame(msg2, data.getMessage());
+    }
+
     @Test
     public void isSameContent() {
         PdpUpdate msg2 = new PdpUpdate(update);
         msg2.setName("world");
-        assertTrue(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
-
-        // different request type
-        assertFalse(data.isSameContent(new StateChangeReq(reqParams, MY_REQ_NAME, new PdpStateChange())));
+        assertTrue(data.isSameContent(msg2));
 
         // both policy lists null
         update.setPolicies(null);
         msg2.setPolicies(null);
-        assertTrue(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+        assertTrue(data.isSameContent(msg2));
     }
 
     @Test
@@ -166,7 +180,7 @@ public class UpdateReqTest extends CommonRequestBase {
         PdpUpdate msg2 = new PdpUpdate(update);
         msg2.setPdpGroup(null);
         update.setPdpGroup(null);
-        assertTrue(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+        assertTrue(data.isSameContent(msg2));
     }
 
     @Test
@@ -174,33 +188,33 @@ public class UpdateReqTest extends CommonRequestBase {
         PdpUpdate msg2 = new PdpUpdate(update);
         msg2.setPdpSubgroup(null);
         update.setPdpSubgroup(null);
-        assertTrue(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+        assertTrue(data.isSameContent(msg2));
     }
 
     @Test
     public void isSameContent_DiffGroup() {
         PdpUpdate msg2 = new PdpUpdate(update);
         msg2.setPdpGroup(null);
-        assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+        assertFalse(data.isSameContent(msg2));
 
         msg2.setPdpGroup(DIFFERENT);
-        assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+        assertFalse(data.isSameContent(msg2));
 
         update.setPdpGroup(null);
-        assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+        assertFalse(data.isSameContent(msg2));
     }
 
     @Test
     public void isSameContent_DiffSubGroup() {
         PdpUpdate msg2 = new PdpUpdate(update);
         msg2.setPdpSubgroup(null);
-        assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+        assertFalse(data.isSameContent(msg2));
 
         msg2.setPdpSubgroup(DIFFERENT);
-        assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+        assertFalse(data.isSameContent(msg2));
 
         update.setPdpSubgroup(null);
-        assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+        assertFalse(data.isSameContent(msg2));
     }
 
     @Test
@@ -211,7 +225,7 @@ public class UpdateReqTest extends CommonRequestBase {
         policies.set(0, makePolicy(DIFFERENT, "10.0.0"));
         msg2.setPolicies(policies);
 
-        assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+        assertFalse(data.isSameContent(msg2));
     }
 
     @Test
@@ -219,7 +233,7 @@ public class UpdateReqTest extends CommonRequestBase {
         PdpUpdate msg2 = new PdpUpdate(update);
         msg2.setPolicies(null);
 
-        assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+        assertFalse(data.isSameContent(msg2));
     }
 
     @Test
@@ -228,12 +242,7 @@ public class UpdateReqTest extends CommonRequestBase {
 
         update.setPolicies(null);
 
-        assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
-    }
-
-    @Test
-    public void testGetPriority() {
-        assertTrue(data.getPriority() > new StateChangeReq(reqParams, MY_REQ_NAME, new PdpStateChange()).getPriority());
+        assertFalse(data.isSameContent(msg2));
     }
 
     @SuppressWarnings("unchecked")