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 <jrh3@att.com>

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 c160c1d..3177c09 100644 (file)
--- 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 34bb0fa..a1f5077 100644 (file)
--- 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<Timer> timers;
+    private Queue<ScheduledFuture<?>> 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;
         }
     }
 }