From 43f8999d3f94e23231bc3cbb45089d8541c4f2cf Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Tue, 5 Nov 2019 11:51:04 -0500 Subject: Simplify request handling in PAP PAP handles outgoing requests using a complicated priority queue. Simplified it significantly. Change-Id: I9f49dfebd7bf323c5e81bc8ca3459913fa95c43d Issue-ID: POLICY-2155 Signed-off-by: Jim Hahn --- .../policy/pap/main/comm/PdpModifyRequestMap.java | 13 +- .../org/onap/policy/pap/main/comm/PdpRequests.java | 188 ++++---------------- .../onap/policy/pap/main/comm/msgdata/Request.java | 39 +--- .../policy/pap/main/comm/msgdata/RequestImpl.java | 69 +++----- .../pap/main/comm/msgdata/StateChangeReq.java | 27 ++- .../policy/pap/main/comm/msgdata/UpdateReq.java | 24 ++- .../policy/pap/main/comm/CommonRequestBase.java | 2 - .../pap/main/comm/PdpModifyRequestMapTest.java | 67 +++++-- .../onap/policy/pap/main/comm/PdpRequestsTest.java | 196 +++++---------------- .../pap/main/comm/msgdata/RequestImplTest.java | 180 +++---------------- .../pap/main/comm/msgdata/StateChangeReqTest.java | 33 ++-- .../pap/main/comm/msgdata/UpdateReqTest.java | 51 +++--- 12 files changed, 285 insertions(+), 604 deletions(-) (limited to 'main/src') 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 f058da32..6f763204 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 @@ -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); + } } } 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 4e1e9233..92ed596f 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 @@ -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 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 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 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 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; } } 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 62aea789..dcc35cde 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 @@ -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 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 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 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); } 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 a110ef44..dc91338e 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 @@ -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 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 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 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 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 tok = token; - token = null; - - return tok; } /** diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/StateChangeReq.java b/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/StateChangeReq.java index 40acd3ad..de777c89 100644 --- a/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/StateChangeReq.java +++ b/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/StateChangeReq.java @@ -20,8 +20,10 @@ 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; } } 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 6b04e726..23743bc0 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 @@ -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 List alwaysList(List list) { return (list != null ? list : Collections.emptyList()); } - - @Override - public int getPriority() { - return 1; - } } diff --git a/main/src/test/java/org/onap/policy/pap/main/comm/CommonRequestBase.java b/main/src/test/java/org/onap/policy/pap/main/comm/CommonRequestBase.java index f10abdda..a9cd7d1f 100644 --- a/main/src/test/java/org/onap/policy/pap/main/comm/CommonRequestBase.java +++ b/main/src/test/java/org/onap/policy/pap/main/comm/CommonRequestBase.java @@ -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; 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 d0bc200f..e90566eb 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 @@ -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 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 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 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 ccb13fea..fb29c193 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 @@ -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 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 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 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 e47f8792..4ebd532b 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 @@ -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 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 token = queue.poll(); - assertNotNull(token); - token.replaceItem(null); - - QueueToken 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 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 token = new QueueToken<>(new PdpStateChange()); - - req.startPublishing(token); - assertNull(queue.poll()); - assertSame(msg, token.get()); - } - - @Test - public void testReplaceToken_SameToken() { - req.startPublishing(); - - QueueToken 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 token2 = new QueueToken<>(new PdpStateChange()); - req.startPublishing(token2); - - QueueToken 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 token = queue.poll(); @@ -329,35 +217,13 @@ public class RequestImplTest extends CommonRequestBase { assertSame(msg, token2.get()); } - @Test - public void testStopPublishingBoolean_FalsePublishing() { - req.startPublishing(); - - QueueToken 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 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 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 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; } } } diff --git a/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/StateChangeReqTest.java b/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/StateChangeReqTest.java index 35531145..e16cd899 100644 --- a/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/StateChangeReqTest.java +++ b/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/StateChangeReqTest.java @@ -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()); } } 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 80ed0fb1..4aa8075d 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 @@ -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") -- cgit 1.2.3-korg