From d1313757cc0b0b0b0a073057bdc5f6f554960ef7 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Tue, 23 Jul 2019 15:00:59 -0400 Subject: Fixed xacml-pdp registration Apparently, TimerTasks may not be cancelled and then re-scheduled. Modified the code to use a scheduled thread pool instead. Change-Id: I2e26a5a37636f570f362481823a0274fe558e2e9 Issue-ID: POLICY-1939 Signed-off-by: Jim Hahn --- .../pdpx/main/comm/XacmlPdpHearbeatPublisher.java | 34 ++++++++------- .../main/comm/XacmlPdpHearbeatPublisherTest.java | 50 ++++++++++++---------- 2 files changed, 46 insertions(+), 38 deletions(-) diff --git a/main/src/main/java/org/onap/policy/pdpx/main/comm/XacmlPdpHearbeatPublisher.java b/main/src/main/java/org/onap/policy/pdpx/main/comm/XacmlPdpHearbeatPublisher.java index c160c1da..3177c096 100644 --- a/main/src/main/java/org/onap/policy/pdpx/main/comm/XacmlPdpHearbeatPublisher.java +++ b/main/src/main/java/org/onap/policy/pdpx/main/comm/XacmlPdpHearbeatPublisher.java @@ -20,8 +20,10 @@ package org.onap.policy.pdpx.main.comm; -import java.util.Timer; -import java.util.TimerTask; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; import lombok.Getter; import org.onap.policy.common.endpoints.event.comm.client.TopicSinkClient; import org.onap.policy.models.pdp.concepts.PdpStatus; @@ -29,7 +31,7 @@ import org.onap.policy.pdpx.main.XacmlState; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class XacmlPdpHearbeatPublisher extends TimerTask { +public class XacmlPdpHearbeatPublisher implements Runnable { public static final int DEFAULT_INTERVAL_MS = 60000; private static final Logger LOGGER = LoggerFactory.getLogger(XacmlPdpHearbeatPublisher.class); @@ -47,7 +49,9 @@ public class XacmlPdpHearbeatPublisher extends TimerTask { @Getter private long intervalMs = DEFAULT_INTERVAL_MS; - private Timer timer = null; + private ScheduledExecutorService timerThread; + + private ScheduledFuture timer; /** @@ -73,9 +77,9 @@ public class XacmlPdpHearbeatPublisher extends TimerTask { * Method to terminate the heart beat. */ public synchronized void terminate() { - if (timer != null) { - timer.cancel(); - timer.purge(); + if (timerThread != null) { + timerThread.shutdownNow(); + timerThread = null; timer = null; } } @@ -90,9 +94,9 @@ public class XacmlPdpHearbeatPublisher extends TimerTask { if (intervalMs != null && intervalMs > 0 && intervalMs != this.intervalMs) { this.intervalMs = intervalMs; - if (timer != null) { - terminate(); - start(); + if (timerThread != null) { + timer.cancel(false); + timer = timerThread.scheduleWithFixedDelay(this, 0, this.intervalMs, TimeUnit.MILLISECONDS); } } } @@ -101,15 +105,15 @@ public class XacmlPdpHearbeatPublisher extends TimerTask { * Starts the timer. */ public synchronized void start() { - if (timer == null) { - timer = makeTimer(); - timer.scheduleAtFixedRate(this, 0, this.intervalMs); + if (timerThread == null) { + timerThread = makeTimerThread(); + timer = timerThread.scheduleWithFixedDelay(this, 0, this.intervalMs, TimeUnit.MILLISECONDS); } } // these may be overridden by junit tests - protected Timer makeTimer() { - return new Timer(true); + protected ScheduledExecutorService makeTimerThread() { + return Executors.newScheduledThreadPool(1); } } diff --git a/main/src/test/java/org/onap/policy/pdpx/main/comm/XacmlPdpHearbeatPublisherTest.java b/main/src/test/java/org/onap/policy/pdpx/main/comm/XacmlPdpHearbeatPublisherTest.java index 34bb0fa3..a1f50771 100644 --- a/main/src/test/java/org/onap/policy/pdpx/main/comm/XacmlPdpHearbeatPublisherTest.java +++ b/main/src/test/java/org/onap/policy/pdpx/main/comm/XacmlPdpHearbeatPublisherTest.java @@ -23,6 +23,8 @@ package org.onap.policy.pdpx.main.comm; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertSame; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyLong; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -32,7 +34,9 @@ import static org.mockito.Mockito.when; import java.util.Arrays; import java.util.LinkedList; import java.util.Queue; -import java.util.Timer; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -54,15 +58,18 @@ public class XacmlPdpHearbeatPublisherTest { private XacmlState state; @Mock - private Timer timer1; + private ScheduledExecutorService executor; @Mock - private Timer timer2; + private ScheduledFuture timer1; + + @Mock + private ScheduledFuture timer2; @Mock private PdpStatus status; - private Queue timers; + private Queue> timers; private XacmlPdpHearbeatPublisher publisher; @@ -78,6 +85,8 @@ public class XacmlPdpHearbeatPublisherTest { timers = new LinkedList<>(Arrays.asList(timer1, timer2)); + when(executor.scheduleWithFixedDelay(any(), anyLong(), anyLong(), any())).thenAnswer(args -> timers.remove()); + publisher = new MyPublisher(client, state); } @@ -94,16 +103,11 @@ public class XacmlPdpHearbeatPublisherTest { // not yet started publisher.terminate(); - verify(timer1, never()).cancel(); - verify(timer2, never()).cancel(); - // now start it and then try again publisher.start(); publisher.terminate(); - verify(timer1).cancel(); - // timer2 should still be in the queue assertSame(timer2, timers.peek()); @@ -111,8 +115,6 @@ public class XacmlPdpHearbeatPublisherTest { // repeat - nothing more should happen publisher.terminate(); - verify(timer1, times(1)).cancel(); - // timer2 should still be in the queue assertSame(timer2, timers.peek()); } @@ -127,47 +129,49 @@ public class XacmlPdpHearbeatPublisherTest { // now start it publisher.start(); - verify(timer1).scheduleAtFixedRate(publisher, 0, INTERVAL1); + verify(executor).scheduleWithFixedDelay(publisher, 0, INTERVAL1, TimeUnit.MILLISECONDS); // null interval - no changes publisher.restart(null); - verify(timer1, times(1)).scheduleAtFixedRate(any(), anyLong(), anyLong()); + verify(executor, times(1)).scheduleWithFixedDelay(any(), anyInt(), anyLong(), any()); assertSame(timer2, timers.peek()); // same interval - no changes publisher.restart(INTERVAL1); - verify(timer1, times(1)).scheduleAtFixedRate(any(), anyLong(), anyLong()); + verify(executor, times(1)).scheduleWithFixedDelay(any(), anyInt(), anyLong(), any()); assertSame(timer2, timers.peek()); // invalid interval - no changes publisher.restart(INTERVAL_INVALID); - verify(timer1, times(1)).scheduleAtFixedRate(any(), anyLong(), anyLong()); + verify(executor, times(1)).scheduleWithFixedDelay(any(), anyInt(), anyLong(), any()); assertSame(timer2, timers.peek()); // new interval - old timer should be cancelled and new started publisher.restart(INTERVAL2); - verify(timer1).cancel(); - verify(timer2).scheduleAtFixedRate(publisher, 0, INTERVAL2); + verify(timer1).cancel(anyBoolean()); + verify(executor).scheduleWithFixedDelay(publisher, 0, INTERVAL2, TimeUnit.MILLISECONDS); } @Test public void testStart() { publisher.start(); - verify(timer1).scheduleAtFixedRate(publisher, 0, XacmlPdpHearbeatPublisher.DEFAULT_INTERVAL_MS); + verify(executor).scheduleWithFixedDelay(publisher, 0, XacmlPdpHearbeatPublisher.DEFAULT_INTERVAL_MS, + TimeUnit.MILLISECONDS); // repeat - nothing more should happen publisher.start(); - verify(timer1, times(1)).scheduleAtFixedRate(any(), anyLong(), anyLong()); - verify(timer1, never()).cancel(); + verify(executor, times(1)).scheduleWithFixedDelay(any(), anyInt(), anyLong(), any()); + verify(timer1, never()).cancel(anyBoolean()); } @Test - public void testMakeTimer() { + public void testMakeTimerThread() { // create a plain listener to test the "real" makeTimer() method publisher = new XacmlPdpHearbeatPublisher(client, state); publisher.start(); + publisher.restart(100L); publisher.terminate(); } @@ -178,8 +182,8 @@ public class XacmlPdpHearbeatPublisherTest { } @Override - protected Timer makeTimer() { - return timers.remove(); + protected ScheduledExecutorService makeTimerThread() { + return executor; } } } -- cgit 1.2.3-korg