Merge "PropertyUtil: remove sleep when running junit test"

3 files changed, 79 insertions, 29 deletions

diff --git a/common-logging/pom.xml b/common-logging/pom.xml
index 82849b93..747e4b2a 100644
--- a/common-logging/pom.xml
+++ b/common-logging/pom.xml
@@ -2,7 +2,7 @@
   ============LICENSE_START=======================================================
   ONAP Policy Engine - Common Modules
   ================================================================================
-  Copyright (C) 2017 AT&T Intellectual Property. All rights reserved.
+  Copyright (C) 2017-2018 AT&T Intellectual Property. All rights reserved.
   ================================================================================
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -31,6 +31,10 @@
   <artifactId>ONAP-Logging</artifactId>
   <description>ONAP Logging Framework</description>
   <packaging>jar</packaging>
+  
+  <properties>
+	<powermock.version>1.6.6</powermock.version>
+  </properties>
 
   <dependencies>
 	<dependency>
@@ -54,6 +58,12 @@
 		<artifactId>eelf-core</artifactId>
 		<version>1.0.1-oss</version>
 	</dependency>
+	<dependency>
+		<groupId>org.powermock</groupId>
+		<artifactId>powermock-api-mockito</artifactId>
+		<version>${powermock.version}</version>
+		<scope>test</scope>
+	</dependency>
   </dependencies>
  
 	<build>
diff --git a/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/PropertyUtil.java b/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/PropertyUtil.java
index 54e06411..566362c9 100644
--- a/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/PropertyUtil.java
+++ b/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/PropertyUtil.java
@@ -36,9 +36,14 @@ import java.util.TimerTask;
  * notifications of future changes.
  */
 public class PropertyUtil {
-
-    // timer thread used for polling for property file changes
-    private static Timer timer = null;
+    
+    protected static class LazyHolder {
+        /**
+         * Timer thread.  Will not be allocated by the JVM until it is first referenced.
+         * This may be overridden by junit tests.
+         */
+        private static Timer timer = new Timer("PropertyUtil-Timer", true);
+    }
 
     // this table maps canonical file into a 'ListenerRegistration' instance
     private static HashMap<File, ListenerRegistration> registrations = new HashMap<>();
@@ -138,17 +143,6 @@ public class PropertyUtil {
             // add to static table, so this instance can be shared
             registrations.put(file, this);
 
-            if (timer == null) {
-                // still need to create a timer thread
-                synchronized (PropertyUtil.class) {
-                    // an additional check is added inside the 'synchronized' block,
-                    // just in case someone beat us to it
-                    if (timer == null) {
-                        timer = new Timer("PropertyUtil-Timer", true);
-                    }
-                }
-            }
-
             // create and schedule the timer task, so this is periodically polled
             timerTask = new TimerTask() {
                 @Override
@@ -160,7 +154,7 @@ public class PropertyUtil {
                     }
                 }
             };
-            timer.schedule(timerTask, 10000L, 10000L);
+            LazyHolder.timer.schedule(timerTask, 10000L, 10000L);
         }
 
         /**
diff --git a/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/PropertyUtilTest.java b/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/PropertyUtilTest.java
index d97df4ce..b4fcfc18 100644
--- a/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/PropertyUtilTest.java
+++ b/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/PropertyUtilTest.java
@@ -2,7 +2,7 @@
  * ============LICENSE_START=======================================================
  * ONAP-Logging
  * ================================================================================
- * Copyright (C) 2018 Ericsson. All rights reserved.
+ * Copyright (C) 2018 Ericsson, AT&T. All rights reserved.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,29 +21,70 @@
 package org.onap.policy.common.logging.flexlogger;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
-
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.Properties;
 import java.util.Set;
-
+import java.util.Timer;
+import java.util.TimerTask;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 import org.junit.After;
+import org.junit.AfterClass;
 import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.onap.policy.common.logging.flexlogger.PropertyUtil.Listener;
+import org.powermock.reflect.Whitebox;
 
 public class PropertyUtilTest {
 
+    /**
+     * 
+     */
+    private static final String TIMER_FIELD = "timer";
     private static final File FILE = new File("target/test.properties");
-    private TestListener testListener = new TestListener();
+    private static Timer saveTimer;
+    
+    private TimerTask task;
+    private Timer timer;
+    private TestListener testListener;
+    
+    @BeforeClass
+    public static void setUpBeforeClass() {
+        saveTimer = Whitebox.getInternalState(PropertyUtil.LazyHolder.class, TIMER_FIELD);
+        
+    }
+    
+    @AfterClass
+    public static void tearDownAfterClass() {
+        Whitebox.setInternalState(PropertyUtil.LazyHolder.class, TIMER_FIELD, saveTimer);
+        
+    }
 
     /**
      * Perform test case set up.
      */
     @Before
     public void setUp() throws IOException {
+        task = null;
+        timer = mock(Timer.class);
+        Whitebox.setInternalState(PropertyUtil.LazyHolder.class, TIMER_FIELD, timer);
+        
+        doAnswer(args -> {
+            task = args.getArgumentAt(0, TimerTask.class);
+            return null;
+        }).when(timer).schedule(any(TimerTask.class), anyLong(), anyLong());
+        
+        testListener = new TestListener();
+        
         FileOutputStream fileOutputStream = new FileOutputStream(FILE);
         Properties properties = new Properties();
         properties.put("testProperty", "testValue");
@@ -56,6 +97,11 @@ public class PropertyUtilTest {
         PropertyUtil.stopListening(FILE, testListener);
         FILE.delete();
     }
+    
+    @Test
+    public void testTimer() {
+        assertNotNull(saveTimer);
+    }
 
     @Test
     public void testGetProperties() throws IOException {
@@ -78,6 +124,8 @@ public class PropertyUtilTest {
         newProperties.put("testProperty", "testValueNew");
         newProperties.store(fileOutputStream, "");
 
+        // fire task and verify that it notifies the listener
+        task.run();
         assertTrue(testListener.isPropertiesChangedInvoked());
 
     }
@@ -104,23 +152,21 @@ public class PropertyUtilTest {
         assertEquals("testValueNew", readProperties.getProperty("testProperty"));
     }
 
+    /**
+     * The {@link #propertiesChanged(Properties, Set)} method is invoked via a background
+     * thread, thus we have to use a latch to wait for it to be invoked.
+     */
     private class TestListener implements Listener {
 
-        boolean propertiesChangedInvoked = false;
+        private CountDownLatch latch = new CountDownLatch(1);
 
         @Override
         public void propertiesChanged(Properties properties, Set<String> changedKeys) {
-            propertiesChangedInvoked = true;
+            latch.countDown();
         }
 
         public boolean isPropertiesChangedInvoked() throws InterruptedException {
-            for (int i = 0; i < 20; i++) {
-                if (propertiesChangedInvoked) {
-                    return true;
-                }
-                Thread.sleep(1000);
-            }
-            return false;
+            return latch.await(5, TimeUnit.SECONDS);
         }
     }