From 31cc29898758197f7a4c7643a4b353cc2ca2cbee Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Tue, 1 Dec 2020 22:05:33 +0100 Subject: [PATCH] Issue #5086 Changes after review Signed-off-by: Jan Bartel --- .../deploy/providers/ScanningAppProvider.java | 14 +-- .../java/org/eclipse/jetty/util/Scanner.java | 117 +++++++++--------- .../jetty/util/ssl/KeyStoreScanner.java | 5 +- .../org/eclipse/jetty/util/ScannerTest.java | 23 ++-- 4 files changed, 80 insertions(+), 79 deletions(-) diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java index 548015d5498f..b1153d809bce 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java @@ -54,7 +54,6 @@ public abstract class ScanningAppProvider extends ContainerLifeCycle implements private DeploymentManager _deploymentManager; protected FilenameFilter _filenameFilter; private final List _monitored = new CopyOnWriteArrayList<>(); - private boolean _recursive = false; private int _scanInterval = 10; private Scanner _scanner; @@ -236,12 +235,6 @@ public int getScanInterval() return _scanInterval; } - @ManagedAttribute("recursive scanning supported") - public boolean isRecursive() - { - return _recursive; - } - @Override public void setDeploymentManager(DeploymentManager deploymentManager) { @@ -294,11 +287,6 @@ public void setMonitoredDirectories(Collection directories) } } - protected void setRecursive(boolean recursive) - { - _recursive = recursive; - } - public void setScanInterval(int scanInterval) { _scanInterval = scanInterval; @@ -311,7 +299,7 @@ public void scan() getMonitoredResources().stream().map((r) -> r.getURI().toASCIIString()) .collect(Collectors.joining(", ", "[", "]")) ); - _scanner.scan(); + _scanner.nudge(); } @Override diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java index e2b6dfa8e89f..c322ecc1589c 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java @@ -35,14 +35,15 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.Timer; -import java.util.TimerTask; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; import org.eclipse.jetty.util.component.AbstractLifeCycle; +import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; +import org.eclipse.jetty.util.thread.Scheduler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -67,15 +68,15 @@ public class Scanner extends AbstractLifeCycle private static int __scannerId = 0; private int _scanInterval; - private AtomicInteger _scanCount = new AtomicInteger(0); + private final AtomicInteger _scanCount = new AtomicInteger(0); private final List _listeners = new CopyOnWriteArrayList<>(); private Map _prevScan; private FilenameFilter _filter; private final Map> _scannables = new ConcurrentHashMap<>(); private boolean _reportExisting = true; private boolean _reportDirs = true; - private Timer _timer; - private TimerTask _task; + private Scheduler.Task _task; + private ScheduledExecutorScheduler _scheduler; private int _scanDepth = DEFAULT_SCAN_DEPTH; public enum Status @@ -114,7 +115,7 @@ public boolean test(Path p) * Metadata about a file: Last modified time, file size and * last file status (ADDED, CHANGED, DELETED, STABLE) */ - static class MetaData + private static class MetaData { final long _lastModified; final long _size; @@ -125,11 +126,6 @@ public MetaData(long lastModified, long size) _lastModified = lastModified; _size = size; } - - public void setStatus(Status status) - { - _status = status; - } @Override public int hashCode() @@ -148,6 +144,16 @@ public String toString() return "[lm=" + _lastModified + ",sz=" + _size + ",s=" + _status + "]"; } } + + private class ScanTask implements Runnable + { + @Override + public void run() + { + scan(); + schedule(); + } + } /** * Visitor @@ -183,9 +189,7 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) th if (rootIncludesExcludes != null && !rootIncludesExcludes.isEmpty()) { //accepted if not explicitly excluded and either is explicitly included or there are no explicit inclusions - Boolean result = rootIncludesExcludes.test(dir); - if (Boolean.TRUE == result) - accepted = true; + accepted = rootIncludesExcludes.test(dir); } else { @@ -217,9 +221,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO if (rootIncludesExcludes != null && !rootIncludesExcludes.isEmpty()) { //accepted if not explicitly excluded and either is explicitly included or there are no explicit inclusions - Boolean result = rootIncludesExcludes.test(file); - if (Boolean.TRUE == result) - accepted = true; + accepted = rootIncludesExcludes.test(file); } else if (_filter == null || _filter.accept(f.getParentFile(), f.getName())) accepted = true; @@ -259,7 +261,6 @@ public interface Listener /** * Notification of exact file changes in the last scan. - * */ public interface DiscreteListener extends Listener { @@ -272,7 +273,6 @@ public interface DiscreteListener extends Listener /** * Notification of files that changed in the last scan. - * */ public interface BulkListener extends Listener { @@ -312,12 +312,11 @@ public int getScanInterval() * @param scanInterval pause between scans in seconds, or 0 for no scan after the initial scan. */ public void setScanInterval(int scanInterval) - { + { if (isRunning()) throw new IllegalStateException("Scanner started"); _scanInterval = scanInterval; - schedule(); } public void setScanDirs(List dirs) @@ -506,7 +505,7 @@ public void removeListener(Listener listener) * Start the scanning action. */ @Override - public void doStart() + public void doStart() throws Exception { if (LOG.isDebugEnabled()) LOG.debug("Scanner start: rprtExists={}, depth={}, rprtDirs={}, interval={}, filter={}, scannables={}", @@ -523,40 +522,21 @@ public void doStart() //just register the list of existing files and only report changes _prevScan = scanFiles(); } + + + //Create the scheduler and start it + _scheduler = new ScheduledExecutorScheduler("Scanner-" + __scannerId++, true, 1); + _scheduler.start(); + + //schedule the scan schedule(); } - public TimerTask newTimerTask() - { - return new TimerTask() - { - @Override - public void run() - { - scan(); - } - }; - } - - public Timer newTimer() - { - return new Timer("Scanner-" + __scannerId++, true); - } - - public void schedule() + private void schedule() { - if (isRunning()) + if (isRunning() && getScanInterval() > 0) { - if (_timer != null) - _timer.cancel(); - if (_task != null) - _task.cancel(); - if (getScanInterval() > 0) - { - _timer = newTimer(); - _task = newTimerTask(); - _timer.schedule(_task, 1010L * getScanInterval(), 1010L * getScanInterval()); - } + _task = _scheduler.schedule(new ScanTask(), 1010L * getScanInterval(), TimeUnit.MILLISECONDS); } } @@ -566,12 +546,20 @@ public void schedule() @Override public void doStop() { - if (_timer != null) - _timer.cancel(); + try + { + if (_scheduler != null) + _scheduler.stop(); + } + catch (Exception e) + { + LOG.warn("Error stopping scheduler", e); + } + if (_task != null) _task.cancel(); + _scheduler = null; _task = null; - _timer = null; } /** @@ -604,10 +592,27 @@ public boolean exists(String path) return false; } + /** + * Hint to the scanner to perform a scan cycle as soon as possible. + * NOTE that the scan is not guaranteed to have happened by the + * time this method returns. + */ + public void nudge() + { + if (!isRunning()) + throw new IllegalStateException("Scanner not running"); + + _scheduler.schedule(() -> + { + scan(); + + }, 0, TimeUnit.MILLISECONDS); + } + /** * Perform a pass of the scanner and report changes */ - public void scan() + void scan() { int cycle = _scanCount.incrementAndGet(); reportScanStart(cycle); @@ -620,7 +625,7 @@ public void scan() /** * Scan all of the given paths. */ - public Map scanFiles() + private Map scanFiles() { Map currentScan = new HashMap<>(); for (Path p : _scannables.keySet()) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java index 225c09a91305..097c4da8f207 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java @@ -20,7 +20,6 @@ import java.io.File; import java.io.IOException; -import java.util.Collections; import java.util.function.Consumer; import org.eclipse.jetty.util.Scanner; @@ -122,8 +121,8 @@ public void scan() if (LOG.isDebugEnabled()) LOG.debug("scanning"); - _scanner.scan(); - _scanner.scan(); + _scanner.nudge(); + _scanner.nudge(); } @ManagedOperation(value = "Reload the SSL Keystore", impact = "ACTION") diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/ScannerTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/ScannerTest.java index 09b5b0261d69..93476a328bb3 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/ScannerTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/ScannerTest.java @@ -26,6 +26,7 @@ import java.nio.file.PathMatcher; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; @@ -129,6 +130,12 @@ public boolean equals(Object obj) return ((Event)obj)._filename.equals(_filename) && ((Event)obj)._notification == _notification; } + @Override + public int hashCode() + { + return Objects.hash(_filename, _notification); + } + @Override public String toString() { @@ -278,8 +285,10 @@ public void fileAdded(String filename) throws Exception scanner.scan(); scanner.scan(); //2 scans for file to be considered settled - assertThat(queue.size(), Matchers.equalTo(2)); - for (Event e : queue) + List results = new ArrayList<>(); + queue.drainTo(results); + assertThat(results.size(), Matchers.equalTo(2)); + for (Event e : results) { assertTrue(e._filename.endsWith("ttt.txt") || e._filename.endsWith("xxx.txt")); } @@ -295,7 +304,7 @@ public void testAddedChangeRemove() throws Exception _scanner.scan(); _scanner.scan(); - Event event = _queue.poll(); + Event event = _queue.poll(5, TimeUnit.SECONDS); assertNotNull(event, "Event should not be null"); assertEquals(_directory + "/a0", event._filename); assertEquals(Notification.ADDED, event._notification); @@ -325,7 +334,7 @@ public void testAddedChangeRemove() throws Exception Event a3 = new Event(_directory + "/a3", Notification.REMOVED); assertThat(actualEvents, Matchers.containsInAnyOrder(a1, a3)); assertTrue(_queue.isEmpty()); - + // Now a2 is stable _scanner.scan(); event = _queue.poll(); @@ -366,7 +375,7 @@ public void testAddedChangeRemove() throws Exception // delete a1 and a2 delete("a1"); delete("a2"); - + //Immediate notification of deletes. _scanner.scan(); a1 = new Event(_directory + "/a1", Notification.REMOVED); @@ -376,7 +385,7 @@ public void testAddedChangeRemove() throws Exception assertEquals(2, actualEvents.size()); assertThat(actualEvents, Matchers.containsInAnyOrder(a1, a2)); assertTrue(_queue.isEmpty()); - + // recreate a2 touch("a2"); @@ -389,7 +398,7 @@ public void testAddedChangeRemove() throws Exception //Now a2 is reported as ADDED. _scanner.scan(); event = _queue.poll(); - assertTrue(event != null); + assertNotNull(event); assertEquals(_directory + "/a2", event._filename); assertEquals(Notification.ADDED, event._notification); assertTrue(_queue.isEmpty());