Issue #5086 Review and rework o.e.j.util.Scanner

jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java

@@ -41,18 +41,14 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-/**
- *
- */
 @ManagedObject("Abstract Provider for loading webapps")
 public abstract class ScanningAppProvider extends ContainerLifeCycle implements AppProvider
 {
     private static final Logger LOG = LoggerFactory.getLogger(ScanningAppProvider.class);
 
-    private Map<String, App> _appMap = new HashMap<String, App>();
-
+    private final Map<String, App> _appMap = new HashMap<>();
     private DeploymentManager _deploymentManager;
-    protected FilenameFilter _filenameFilter;
+    private FilenameFilter _filenameFilter;
     private final List<Resource> _monitored = new CopyOnWriteArrayList<>();
     private int _scanInterval = 10;
     private Scanner _scanner;

jetty-deploy/src/test/java/org/eclipse/jetty/deploy/providers/ScanningAppProviderRuntimeUpdatesTest.java

@@ -78,11 +78,6 @@ public void setupEnvironment() throws Exception
                 _providers++;
                 ((ScanningAppProvider)provider).addScannerListener(new Scanner.ScanCycleListener()
                 {
-                    @Override
-                    public void scanStarted(int cycle)
-                    {
-                    }
-
                     @Override
                     public void scanEnded(int cycle)
                     {

jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java

@@ -59,13 +59,11 @@ public class Scanner extends AbstractLifeCycle
      * When walking a directory, a depth of 1 ensures that
      * the directory's descendants are visited, not just the
      * directory itself (as a file).
-     * 
-     * @see Visitor#preVisitDirectory
      */
     public static final int DEFAULT_SCAN_DEPTH = 1;
     public static final int MAX_SCAN_DEPTH = Integer.MAX_VALUE;
     private static final Logger LOG = LoggerFactory.getLogger(Scanner.class);
-    private static int __scannerId = 0;
+    private static final AtomicInteger __scannerId = new AtomicInteger();
 
     private int _scanInterval;
     private final AtomicInteger _scanCount = new AtomicInteger(0);
@@ -76,15 +74,15 @@ public class Scanner extends AbstractLifeCycle
     private boolean _reportExisting = true;
     private boolean _reportDirs = true;
     private Scheduler.Task _task;
-    private ScheduledExecutorScheduler _scheduler;
+    private Scheduler _scheduler;
     private int _scanDepth = DEFAULT_SCAN_DEPTH;
 
-    public enum Status
+    private enum Status
     {
         ADDED, CHANGED, REMOVED, STABLE
     }
 
-    public enum Notification
+    enum Notification
     {
         ADDED, CHANGED, REMOVED
     }
@@ -127,12 +125,6 @@ public MetaData(long lastModified, long size)
             _size = size;
         }
 
-        @Override
-        public int hashCode()
-        {
-            return (int)_lastModified ^ (int)_size;
-        }
-
         public boolean isModified(MetaData m)
         {
             return m._lastModified != _lastModified || m._size != _size;
@@ -161,7 +153,7 @@ public void run()
      * A FileVisitor for walking a subtree of paths. The Scanner uses
      * this to examine the dirs and files it has been asked to scan.
      */
-    class Visitor implements FileVisitor<Path>
+    private class Visitor implements FileVisitor<Path>
     {
         Map<String, MetaData> scanInfoMap;
         IncludeExcludeSet<PathMatcher,Path> rootIncludesExcludes;
@@ -284,14 +276,15 @@ public interface BulkListener extends Listener
      */
     public interface ScanCycleListener extends Listener
     {
-        public void scanStarted(int cycle) throws Exception;
+        public default void scanStarted(int cycle) throws Exception
+        {
+        }
 
-        public void scanEnded(int cycle) throws Exception;
+        public default void scanEnded(int cycle) throws Exception
+        {
+        }
     }
 
-    /**
-     *
-     */
     public Scanner()
     {
     }
@@ -528,7 +521,7 @@ public void doStart() throws Exception
 
 
         //Create the scheduler and start it
-        _scheduler = new ScheduledExecutorScheduler("Scanner-" + __scannerId++, true, 1);
+        _scheduler = new ScheduledExecutorScheduler("Scanner-" + __scannerId.getAndIncrement(), true, 1);
         _scheduler.start();
 
         //schedule the scan
@@ -538,31 +531,23 @@ public void doStart() throws Exception
     private void schedule()
     {
         if (isRunning() && getScanInterval() > 0)
-        {
             _task = _scheduler.schedule(new ScanTask(), 1010L * getScanInterval(), TimeUnit.MILLISECONDS);
-        }
     }
 
     /**
      * Stop the scanning.
      */
     @Override
-    public void doStop()
+    public void doStop() throws Exception
     {
-        try
-        {
-            if (_scheduler != null)
-                _scheduler.stop();
-        }
-        catch (Exception e)
-        {
-            LOG.warn("Error stopping scheduler", e);
-        }
-
-        if (_task != null)
-            _task.cancel();
-        _scheduler = null;
+        Scheduler.Task task = _task;
         _task = null;
+        if (task != null)
+            task.cancel();
+        Scheduler scheduler = _scheduler;
+        _scheduler = null;
+        if (scheduler != null)
+            scheduler.stop();
     }
 
     /**
@@ -604,12 +589,7 @@ public void nudge()
     {
         if (!isRunning())
             throw new IllegalStateException("Scanner not running");
-
-        _scheduler.schedule(() ->
-        {
-            scan();
-
-        }, 0, TimeUnit.MILLISECONDS);  
+        scan(Callback.NOOP);
     }
 
     /**
@@ -620,12 +600,15 @@ public void nudge()
      */
     public void scan(Callback complete)
     {
-        Scheduler s = _scheduler;
+        Scheduler scheduler = _scheduler;
 
-        if (!isRunning() || s == null)
+        if (!isRunning() || scheduler == null)
+        {
             complete.failed(new IllegalStateException("Scanner not running"));
+            return;
+        }
 
-        s.schedule(() ->
+        scheduler.schedule(() ->
         {
             try
             {
@@ -636,11 +619,9 @@ public void scan(Callback complete)
             {
                 complete.failed(t);
             }
-
-        }, 0, TimeUnit.MILLISECONDS);   
+        }, 0, TimeUnit.MILLISECONDS);
     }
-
-
+
     /**
      * Perform a pass of the scanner and report changes
      */

jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java

@@ -119,7 +119,7 @@ public void fileRemoved(String filename)
     }
 
     @ManagedOperation(value = "Scan for changes in the SSL Keystore", impact = "ACTION")
-    public void scan()
+    public boolean scan()
     {
         if (LOG.isDebugEnabled())
             LOG.debug("scanning");
@@ -135,8 +135,7 @@ public void scan()
 
             _scanner.scan(callback);
             _scanner.scan(callback);
-            complete.await(10, TimeUnit.SECONDS);
-
+            return complete.await(10, TimeUnit.SECONDS);
         }
         catch (Exception e)
         {

jetty-util/src/test/java/org/eclipse/jetty/util/ScannerTest.java

@@ -73,31 +73,24 @@ public static void setUpBeforeClass() throws Exception
         _scanner.addListener(new Scanner.DiscreteListener()
         {
             @Override
-            public void fileRemoved(String filename) throws Exception
+            public void fileRemoved(String filename)
             {
                 _queue.add(new Event(filename, Notification.REMOVED));
             }
 
             @Override
-            public void fileChanged(String filename) throws Exception
+            public void fileChanged(String filename)
             {
                 _queue.add(new Event(filename, Notification.CHANGED));
             }
 
             @Override
-            public void fileAdded(String filename) throws Exception
+            public void fileAdded(String filename)
             {
                 _queue.add(new Event(filename, Notification.ADDED));
             }
         });
-        _scanner.addListener(new Scanner.BulkListener()
-        {
-            @Override
-            public void filesChanged(Set<String> filenames) throws Exception
-            {
-                _bulk.add(filenames);
-            }
-        });
+        _scanner.addListener((Scanner.BulkListener)filenames -> _bulk.add(filenames));
 
         _scanner.start();
         _scanner.scan();
@@ -127,7 +120,12 @@ public Event(String filename, Notification notification)
         @Override
         public boolean equals(Object obj)
         {
-            return ((Event)obj)._filename.equals(_filename) && ((Event)obj)._notification == _notification;
+            if (this == obj)
+                return true;
+            if (obj == null || getClass() != obj.getClass())
+                return false;
+            Event that = (Event)obj;
+            return _filename.equals(that._filename) && _notification == that._notification;
         }
 
         @Override
@@ -163,7 +161,7 @@ public void testDepth() throws Exception
         File y2 = new File(dir2, "yyy2.foo");
         FS.touch(y2);
 
-        BlockingQueue<Event> queue = new LinkedBlockingQueue<Event>();
+        BlockingQueue<Event> queue = new LinkedBlockingQueue<>();
         Scanner scanner = new Scanner();
         scanner.setScanInterval(0);
         scanner.setScanDepth(0);
@@ -173,19 +171,19 @@ public void testDepth() throws Exception
         scanner.addListener(new Scanner.DiscreteListener()
         {
             @Override
-            public void fileRemoved(String filename) throws Exception
+            public void fileRemoved(String filename)
             {
                 queue.add(new Event(filename, Notification.REMOVED));
             }
 
             @Override
-            public void fileChanged(String filename) throws Exception
+            public void fileChanged(String filename)
             {
                 queue.add(new Event(filename, Notification.CHANGED));
             }
 
             @Override
-            public void fileAdded(String filename) throws Exception
+            public void fileAdded(String filename)
             {
                 queue.add(new Event(filename, Notification.ADDED));
             }
@@ -243,7 +241,7 @@ public void testPatterns() throws Exception
         File y2 = new File(dir2, "yyy.txt");
         FS.touch(y2);
 
-        BlockingQueue<Event> queue = new LinkedBlockingQueue<Event>();
+        BlockingQueue<Event> queue = new LinkedBlockingQueue<>();
         //only scan the *.txt files for changes
         Scanner scanner = new Scanner();
         IncludeExcludeSet<PathMatcher, Path> pattern = scanner.addDirectory(root.toPath());
@@ -256,19 +254,19 @@ public void testPatterns() throws Exception
         scanner.addListener(new Scanner.DiscreteListener()
         {
             @Override
-            public void fileRemoved(String filename) throws Exception
+            public void fileRemoved(String filename)
             {
                 queue.add(new Event(filename, Notification.REMOVED));
             }
 
             @Override
-            public void fileChanged(String filename) throws Exception
+            public void fileChanged(String filename)
             {
                 queue.add(new Event(filename, Notification.CHANGED));
             }
 
             @Override
-            public void fileAdded(String filename) throws Exception
+            public void fileAdded(String filename)
             {
                 queue.add(new Event(filename, Notification.ADDED));
             }
@@ -350,7 +348,7 @@ public void testAddedChangeRemove() throws Exception
         // not stable after 1scan so nothing should not be seen yet.
         _scanner.scan();
         event = _queue.poll();
-        assertTrue(event == null);
+        assertNull(event);
 
         // Keep a2 unstable
         Thread.sleep(1100); // make sure time in seconds changes
@@ -468,7 +466,7 @@ public void testSizeChange() throws Exception
         }
     }
 
-    private void delete(String string) throws IOException
+    private void delete(String string)
     {
         File file = new File(_directory, string);
         if (file.exists())

tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java

@@ -126,7 +126,7 @@ public void testKeystoreHotReload() throws Exception
 
         // Switch to use newKeystore which has a later expiry date.
         useKeystore("newKeystore");
-        keystoreScanner.scan();
+        assertTrue(keystoreScanner.scan());
 
         // The scanner should have detected the updated keystore, expiry should be renewed.
         X509Certificate cert2 = getCertificateFromServer();