diff --git a/jetty-ant/src/main/java/org/eclipse/jetty/ant/ServerProxyImpl.java b/jetty-ant/src/main/java/org/eclipse/jetty/ant/ServerProxyImpl.java index e71da97c6576..f532858f2b2f 100644 --- a/jetty-ant/src/main/java/org/eclipse/jetty/ant/ServerProxyImpl.java +++ b/jetty-ant/src/main/java/org/eclipse/jetty/ant/ServerProxyImpl.java @@ -24,6 +24,7 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.Set; import org.eclipse.jetty.ant.types.Connector; import org.eclipse.jetty.ant.types.ContextHandlers; @@ -135,7 +136,7 @@ public WebAppScannerListener(AntWebAppContext awc) } @Override - public void filesChanged(List changedFileNames) + public void filesChanged(Set changedFileNames) { boolean isScanned = false; try 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 3bffee94bc80..918ee18dba87 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 @@ -41,20 +41,15 @@ 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 _appMap = new HashMap(); - + private final Map _appMap = new HashMap<>(); private DeploymentManager _deploymentManager; - protected FilenameFilter _filenameFilter; + private FilenameFilter _filenameFilter; private final List _monitored = new CopyOnWriteArrayList<>(); - private boolean _recursive = false; private int _scanInterval = 10; private Scanner _scanner; @@ -140,7 +135,6 @@ protected void doStart() throws Exception _scanner = new Scanner(); _scanner.setScanDirs(files); _scanner.setScanInterval(_scanInterval); - _scanner.setRecursive(_recursive); _scanner.setFilenameFilter(_filenameFilter); _scanner.setReportDirs(true); _scanner.setScanDepth(1); //consider direct dir children of monitored dir @@ -237,12 +231,6 @@ public int getScanInterval() return _scanInterval; } - @ManagedAttribute("recursive scanning supported") - public boolean isRecursive() - { - return _recursive; - } - @Override public void setDeploymentManager(DeploymentManager deploymentManager) { @@ -295,11 +283,6 @@ public void setMonitoredDirectories(Collection directories) } } - protected void setRecursive(boolean recursive) - { - _recursive = recursive; - } - public void setScanInterval(int scanInterval) { _scanInterval = scanInterval; @@ -312,7 +295,7 @@ public void scan() getMonitoredResources().stream().map((r) -> r.getURI().toASCIIString()) .collect(Collectors.joining(", ", "[", "]")) ); - _scanner.scan(); + _scanner.nudge(); } @Override diff --git a/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/providers/ScanningAppProviderRuntimeUpdatesTest.java b/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/providers/ScanningAppProviderRuntimeUpdatesTest.java index 5a8c294fcc3b..19df28d42eca 100644 --- a/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/providers/ScanningAppProviderRuntimeUpdatesTest.java +++ b/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/providers/ScanningAppProviderRuntimeUpdatesTest.java @@ -76,10 +76,10 @@ public void setupEnvironment() throws Exception if (provider instanceof ScanningAppProvider) { _providers++; - ((ScanningAppProvider)provider).addScannerListener(new Scanner.ScanListener() + ((ScanningAppProvider)provider).addScannerListener(new Scanner.ScanCycleListener() { @Override - public void scan() + public void scanEnded(int cycle) { _scans.incrementAndGet(); } diff --git a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunMojo.java b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunMojo.java index 531a57d30001..4afc8f604ab4 100644 --- a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunMojo.java +++ b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunMojo.java @@ -22,7 +22,7 @@ import java.nio.file.Path; import java.nio.file.PathMatcher; import java.util.Date; -import java.util.List; +import java.util.Set; import org.apache.maven.artifact.Artifact; import org.apache.maven.plugin.MojoExecutionException; @@ -191,7 +191,7 @@ protected void configureScanner() } scanner.addListener(new Scanner.BulkListener() { - public void filesChanged(List changes) + public void filesChanged(Set changes) { try { diff --git a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunWarMojo.java b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunWarMojo.java index 740ac9783158..35445b241d80 100644 --- a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunWarMojo.java +++ b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunWarMojo.java @@ -22,7 +22,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Date; -import java.util.List; +import java.util.Set; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugins.annotations.Execute; @@ -197,7 +197,7 @@ public void configureScanner() throws MojoExecutionException configureScanTargetPatterns(scanner); scanner.addListener(new Scanner.BulkListener() { - public void filesChanged(List changes) + public void filesChanged(Set changes) { try { 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 7dddfa712b34..caba9bf7f54a 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 @@ -28,22 +28,22 @@ import java.nio.file.Path; import java.nio.file.PathMatcher; import java.nio.file.attribute.BasicFileAttributes; -import java.util.ArrayList; import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Map.Entry; 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.AutoLock; +import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; +import org.eclipse.jetty.util.thread.Scheduler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -59,35 +59,33 @@ 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 SCANNER_IDS = new AtomicInteger(); - private final AutoLock _lock = new AutoLock(); private int _scanInterval; - private int _scanCount = 0; - private final List _listeners = new ArrayList<>(); - private final Map _prevScan = new HashMap<>(); - private final Map _currentScan = new HashMap<>(); + private final AtomicInteger _scanCount = new AtomicInteger(0); + private final List _listeners = new CopyOnWriteArrayList<>(); + private Map _prevScan; private FilenameFilter _filter; - private final Map> _scannables = new HashMap<>(); - private volatile boolean _running = false; + 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 Scheduler _scheduler; private int _scanDepth = DEFAULT_SCAN_DEPTH; - public enum Notification + private enum Status + { + ADDED, CHANGED, REMOVED, STABLE + } + + enum Notification { ADDED, CHANGED, REMOVED } - - private final Map _notifications = new HashMap<>(); /** * PathMatcherSet @@ -110,42 +108,42 @@ public boolean test(Path p) } /** - * TimeNSize + * MetaData * - * Metadata about a file: Last modified time and file size. + * Metadata about a file: Last modified time, file size and + * last file status (ADDED, CHANGED, DELETED, STABLE) */ - static class TimeNSize + private static class MetaData { final long _lastModified; final long _size; + Status _status; - public TimeNSize(long lastModified, long size) + public MetaData(long lastModified, long size) { _lastModified = lastModified; _size = size; } - @Override - public int hashCode() + public boolean isModified(MetaData m) { - return (int)_lastModified ^ (int)_size; + return m._lastModified != _lastModified || m._size != _size; } @Override - public boolean equals(Object o) + public String toString() { - if (o instanceof TimeNSize) - { - TimeNSize tns = (TimeNSize)o; - return tns._lastModified == _lastModified && tns._size == _size; - } - return false; + return "[lm=" + _lastModified + ",sz=" + _size + ",s=" + _status + "]"; } - + } + + private class ScanTask implements Runnable + { @Override - public String toString() + public void run() { - return "[lm=" + _lastModified + ",s=" + _size + "]"; + scan(); + schedule(); } } @@ -155,13 +153,13 @@ public String toString() * 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 + private class Visitor implements FileVisitor { - Map scanInfoMap; + Map scanInfoMap; IncludeExcludeSet rootIncludesExcludes; Path root; - public Visitor(Path root, IncludeExcludeSet rootIncludesExcludes, Map scanInfoMap) + public Visitor(Path root, IncludeExcludeSet rootIncludesExcludes, Map scanInfoMap) { this.root = root; this.rootIncludesExcludes = rootIncludesExcludes; @@ -183,9 +181,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 { @@ -195,7 +191,7 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) th if (accepted) { - scanInfoMap.put(f.getCanonicalPath(), new TimeNSize(f.lastModified(), f.isDirectory() ? 0 : f.length())); + scanInfoMap.put(f.getCanonicalPath(), new MetaData(f.lastModified(), f.isDirectory() ? 0 : f.length())); if (LOG.isDebugEnabled()) LOG.debug("scan accepted dir {} mod={}", f, f.lastModified()); } } @@ -217,9 +213,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; @@ -227,7 +221,7 @@ else if (_filter == null || _filter.accept(f.getParentFile(), f.getName())) if (accepted) { - scanInfoMap.put(f.getCanonicalPath(), new TimeNSize(f.lastModified(), f.isDirectory() ? 0 : f.length())); + scanInfoMap.put(f.getCanonicalPath(), new MetaData(f.lastModified(), f.isDirectory() ? 0 : f.length())); if (LOG.isDebugEnabled()) LOG.debug("scan accepted {} mod={}", f, f.lastModified()); } @@ -257,11 +251,9 @@ public interface Listener { } - public interface ScanListener extends Listener - { - public void scan(); - } - + /** + * Notification of exact file changes in the last scan. + */ public interface DiscreteListener extends Listener { public void fileChanged(String filename) throws Exception; @@ -271,9 +263,12 @@ public interface DiscreteListener extends Listener public void fileRemoved(String filename) throws Exception; } + /** + * Notification of files that changed in the last scan. + */ public interface BulkListener extends Listener { - public void filesChanged(List filenames) throws Exception; + public void filesChanged(Set filenames) throws Exception; } /** @@ -281,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() { } @@ -300,10 +296,7 @@ public Scanner() */ public int getScanInterval() { - try (AutoLock l = _lock.lock()) - { - return _scanInterval; - } + return _scanInterval; } /** @@ -313,61 +306,51 @@ public int getScanInterval() */ public void setScanInterval(int scanInterval) { - try (AutoLock l = _lock.lock()) - { - _scanInterval = scanInterval; - schedule(); - } + if (isRunning()) + throw new IllegalStateException("Scanner started"); + + _scanInterval = scanInterval; } public void setScanDirs(List dirs) { + if (isRunning()) + throw new IllegalStateException("Scanner started"); + _scannables.clear(); if (dirs == null) return; - - for (File f:dirs) + for (File f :dirs) { - addScanDir(f); - } - } - - @Deprecated - public void addScanDir(File dir) - { - if (dir == null) - return; - try (AutoLock l = _lock.lock()) - { - if (dir.isDirectory()) - addDirectory(dir.toPath()); + if (f.isDirectory()) + addDirectory(f.toPath()); else - addFile(dir.toPath()); - } - catch (Exception e) - { - LOG.warn("Unable to add: {}", dir, e); + addFile(f.toPath()); } } - + /** * Add a file to be scanned. The file must not be null, and must exist. * * @param p the Path of the file to scan. - * @throws IOException */ - public void addFile(Path p) throws IOException + public void addFile(Path p) { + if (isRunning()) + throw new IllegalStateException("Scanner started"); + if (p == null) throw new IllegalStateException("Null path"); - File f = p.toFile(); - if (!f.exists() || f.isDirectory()) - throw new IllegalStateException("Not file or doesn't exist: " + f.getCanonicalPath()); - - try (AutoLock l = _lock.lock()) + if (!Files.exists(p) || Files.isDirectory(p)) + throw new IllegalStateException("Not file or doesn't exist: " + p); + try + { + _scannables.putIfAbsent(p.toRealPath(), new IncludeExcludeSet<>(PathMatcherSet.class)); + } + catch (IOException e) { - _scannables.put(p, null); + throw new IllegalStateException(e); } } @@ -376,61 +359,59 @@ public void addFile(Path p) throws IOException * * @param p the directory to scan. * @return an IncludeExcludeSet to which the caller can add PathMatcher patterns to match - * @throws IOException */ - public IncludeExcludeSet addDirectory(Path p) throws IOException + public IncludeExcludeSet addDirectory(Path p) { + if (isRunning()) + throw new IllegalStateException("Scanner started"); + if (p == null) throw new IllegalStateException("Null path"); - File f = p.toFile(); - if (!f.exists() || !f.isDirectory()) - throw new IllegalStateException("Not directory or doesn't exist: " + f.getCanonicalPath()); + if (!Files.exists(p) || !Files.isDirectory(p)) + throw new IllegalStateException("Not directory or doesn't exist: " + p); - try (AutoLock l = _lock.lock()) + try { - IncludeExcludeSet includesExcludes = _scannables.get(p); - if (includesExcludes == null) - { - includesExcludes = new IncludeExcludeSet<>(PathMatcherSet.class); - _scannables.put(p.toRealPath(), includesExcludes); - } + IncludeExcludeSet includesExcludes = new IncludeExcludeSet<>(PathMatcherSet.class); + IncludeExcludeSet prev = _scannables.putIfAbsent(p.toRealPath(), includesExcludes); + if (prev != null) + includesExcludes = prev; return includesExcludes; } - } - - @Deprecated - public List getScanDirs() - { - ArrayList files = new ArrayList<>(); - for (Path p : _scannables.keySet()) - files.add(p.toFile()); - return Collections.unmodifiableList(files); + catch (IOException e) + { + throw new IllegalStateException(e); + } } - public Set getScannables() - { - return _scannables.keySet(); - } /** - * @param recursive True if scanning is recursive - * @see #setScanDepth(int) + * Apply a filter to files found in the scan directory. + * Only files matching the filter will be reported as added/changed/removed. + * + * @param filter the filename filter to use */ @Deprecated - public void setRecursive(boolean recursive) + public void setFilenameFilter(FilenameFilter filter) { - _scanDepth = recursive ? Integer.MAX_VALUE : 1; + _filter = filter; } /** - * @return True if scanning is recursive - * @see #getScanDepth() + * Get any filter applied to files in the scan dir. + * + * @return the filename filter */ @Deprecated - public boolean getRecursive() + public FilenameFilter getFilenameFilter() + { + return _filter; + } + + public Set getScannables() { - return _scanDepth > 1; + return Collections.unmodifiableSet(_scannables.keySet()); } /** @@ -450,32 +431,12 @@ public int getScanDepth() */ public void setScanDepth(int scanDepth) { + if (isRunning()) + throw new IllegalStateException("Scanner started"); + _scanDepth = scanDepth; } - /** - * Apply a filter to files found in the scan directory. - * Only files matching the filter will be reported as added/changed/removed. - * - * @param filter the filename filter to use - */ - @Deprecated - public void setFilenameFilter(FilenameFilter filter) - { - _filter = filter; - } - - /** - * Get any filter applied to files in the scan dir. - * - * @return the filename filter - */ - @Deprecated - public FilenameFilter getFilenameFilter() - { - return _filter; - } - /** * Whether or not an initial scan will report all files as being * added. @@ -485,6 +446,8 @@ public FilenameFilter getFilenameFilter() */ public void setReportExistingFilesOnStartup(boolean reportExisting) { + if (isRunning()) + throw new IllegalStateException("Scanner started"); _reportExisting = reportExisting; } @@ -500,6 +463,8 @@ public boolean getReportExistingFilesOnStartup() */ public void setReportDirs(boolean dirs) { + if (isRunning()) + throw new IllegalStateException("Scanner started"); _reportDirs = dirs; } @@ -517,10 +482,7 @@ public void addListener(Listener listener) { if (listener == null) return; - try (AutoLock l = _lock.lock()) - { - _listeners.add(listener); - } + _listeners.add(listener); } /** @@ -532,99 +494,62 @@ public void removeListener(Listener listener) { if (listener == null) return; - try (AutoLock l = _lock.lock()) - { - _listeners.remove(listener); - } + _listeners.remove(listener); } /** * Start the scanning action. */ @Override - public void doStart() + public void doStart() throws Exception { - try (AutoLock l = _lock.lock()) - { - if (_running) - return; + if (LOG.isDebugEnabled()) + LOG.debug("Scanner start: rprtExists={}, depth={}, rprtDirs={}, interval={}, filter={}, scannables={}", + _reportExisting, _scanDepth, _reportDirs, _scanInterval, _filter, _scannables); - _running = true; - if (LOG.isDebugEnabled()) - LOG.debug("Scanner start: rprtExists={}, depth={}, rprtDirs={}, interval={}, filter={}, scannables={}", - _reportExisting, _scanDepth, _reportDirs, _scanInterval, _filter, _scannables); - - if (_reportExisting) - { - // if files exist at startup, report them - scan(); - scan(); // scan twice so files reported as stable - } - else - { - //just register the list of existing files and only report changes - scanFiles(); - _prevScan.putAll(_currentScan); - } - schedule(); + if (_reportExisting) + { + // if files exist at startup, report them + scan(); + scan(); // scan twice so files reported as stable } - } - - public TimerTask newTimerTask() - { - return new TimerTask() + else { - @Override - public void run() - { - scan(); - } - }; - } - - public Timer newTimer() - { - return new Timer("Scanner-" + __scannerId++, true); + //just register the list of existing files and only report changes + _prevScan = scanFiles(); + } + + + //Create the scheduler and start it + _scheduler = new ScheduledExecutorScheduler("Scanner-" + SCANNER_IDS.getAndIncrement(), true, 1); + _scheduler.start(); + + //schedule the scan + schedule(); } - public void schedule() + private void schedule() { - if (_running) - { - if (_timer != null) - _timer.cancel(); - if (_task != null) - _task.cancel(); - if (getScanInterval() > 0) - { - _timer = newTimer(); - _task = newTimerTask(); - _timer.schedule(_task, 1010L * getScanInterval(), 1010L * getScanInterval()); - } - } + 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 (AutoLock l = _lock.lock()) - { - if (_running) - { - _running = false; - if (_timer != null) - _timer.cancel(); - if (_task != null) - _task.cancel(); - _task = null; - _timer = null; - } - } + Scheduler.Task task = _task; + _task = null; + if (task != null) + task.cancel(); + Scheduler scheduler = _scheduler; + _scheduler = null; + if (scheduler != null) + scheduler.stop(); } - + /** * Clear the list of scannables. The scanner must first * be in the stopped state. @@ -633,13 +558,12 @@ public void reset() { if (!isStopped()) throw new IllegalStateException("Not stopped"); - + //clear the scannables _scannables.clear(); - + //clear the previous scans - _currentScan.clear(); - _prevScan.clear(); + _prevScan = null; } /** @@ -657,150 +581,162 @@ public boolean exists(String path) } /** - * Perform a pass of the scanner and report changes + * 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 scan() + public void nudge() { - try (AutoLock l = _lock.lock()) + if (!isRunning()) + throw new IllegalStateException("Scanner not running"); + scan(Callback.NOOP); + } + + /** + * Get the scanner to perform a scan cycle as soon as possible + * and call the Callback when the scan is finished or failed. + * + * @param complete called when the scan cycle finishes or fails. + */ + public void scan(Callback complete) + { + Scheduler scheduler = _scheduler; + + if (!isRunning() || scheduler == null) { - reportScanStart(++_scanCount); - scanFiles(); - reportDifferences(_currentScan, _prevScan); - _prevScan.clear(); - _prevScan.putAll(_currentScan); - reportScanEnd(_scanCount); + complete.failed(new IllegalStateException("Scanner not running")); + return; + } - for (Listener listener : _listeners) + scheduler.schedule(() -> + { + try { - try - { - if (listener instanceof ScanListener) - ((ScanListener)listener).scan(); - } - catch (Throwable e) - { - LOG.warn("Unable to scan", e); - } + scan(); + complete.succeeded(); } - } + catch (Throwable t) + { + complete.failed(t); + } + }, 0, TimeUnit.MILLISECONDS); + } + + /** + * Perform a pass of the scanner and report changes + */ + void scan() + { + int cycle = _scanCount.incrementAndGet(); + reportScanStart(cycle); + Map currentScan = scanFiles(); + reportDifferences(currentScan, _prevScan == null ? Collections.emptyMap() : Collections.unmodifiableMap(_prevScan)); + _prevScan = currentScan; + reportScanEnd(cycle); } /** * Scan all of the given paths. */ - public void scanFiles() + private Map scanFiles() { - try (AutoLock l = _lock.lock()) + Map currentScan = new HashMap<>(); + for (Path p : _scannables.keySet()) { - _currentScan.clear(); - for (Path p : _scannables.keySet()) + try { - try - { - Files.walkFileTree(p, EnumSet.allOf(FileVisitOption.class),_scanDepth, new Visitor(p, _scannables.get(p), _currentScan)); - } - catch (IOException e) - { - LOG.warn("Error scanning files.", e); - } + Files.walkFileTree(p, EnumSet.allOf(FileVisitOption.class),_scanDepth, new Visitor(p, _scannables.get(p), currentScan)); + } + catch (IOException e) + { + LOG.warn("Error scanning files.", e); } } + return currentScan; } /** * Report the adds/changes/removes to the registered listeners + * + * Only report an add or change once a file has stablilized in size. * * @param currentScan the info from the most recent pass * @param oldScan info from the previous pass */ - private void reportDifferences(Map currentScan, Map oldScan) + private void reportDifferences(Map currentScan, Map oldScan) { - try (AutoLock l = _lock.lock()) + Map changes = new HashMap<>(); + + //Handle deleted files + Set oldScanKeys = new HashSet<>(oldScan.keySet()); + oldScanKeys.removeAll(currentScan.keySet()); + for (String file : oldScanKeys) + { + changes.put(file, Notification.REMOVED); + } + + // Handle new and changed files + for (String file : currentScan.keySet()) { - // scan the differences and add what was found to the map of notifications: - Set oldScanKeys = new HashSet<>(oldScan.keySet()); + MetaData current = currentScan.get(file); + MetaData previous = oldScan.get(file); - // Look for new and changed files - for (Entry entry : currentScan.entrySet()) + if (previous == null) { - String file = entry.getKey(); - if (!oldScanKeys.contains(file)) - { - Notification old = _notifications.put(file, Notification.ADDED); - if (old != null) - { - switch (old) - { - case REMOVED: - case CHANGED: - _notifications.put(file, Notification.CHANGED); - break; - default: - break; - } - } - } - else if (!oldScan.get(file).equals(currentScan.get(file))) - { - Notification old = _notifications.put(file, Notification.CHANGED); - if (old == Notification.ADDED) - _notifications.put(file, Notification.ADDED); - } + //New file - don't immediately + //notify this, wait until the size has + //settled down then notify the add. + current._status = Status.ADDED; } - - // Look for deleted files - for (String file : oldScan.keySet()) + else if (current.isModified(previous)) { - if (!currentScan.containsKey(file)) - { - Notification old = _notifications.put(file, Notification.REMOVED); - if (old == Notification.ADDED) - _notifications.remove(file); - } + //Changed file - handle case where file + //that was added on previous scan has since + //been modified. We need to retain status + //as added, so we send the ADDED event once + //the file has settled down. + if (previous._status == Status.ADDED) + current._status = Status.ADDED; + else + current._status = Status.CHANGED; + } + else + { + //Unchanged file: if it was previously + //ADDED, we can now send the ADDED event. + if (previous._status == Status.ADDED) + changes.put(file, Notification.ADDED); + else if (previous._status == Status.CHANGED) + changes.put(file, Notification.CHANGED); + + current._status = Status.STABLE; } + } - if (LOG.isDebugEnabled()) - LOG.debug("scanned {}: {}", _scannables.keySet(), _notifications); + if (LOG.isDebugEnabled()) + LOG.debug("scanned {}", _scannables.keySet()); - // Process notifications - // Only process notifications that are for stable files (ie same in old and current scan). - List bulkChanges = new ArrayList<>(); - for (Iterator> iter = _notifications.entrySet().iterator(); iter.hasNext(); ) + //Call the DiscreteListeners + for (Map.Entry entry : changes.entrySet()) + { + switch (entry.getValue()) { - - Entry entry = iter.next(); - String file = entry.getKey(); - // Is the file stable? - if (oldScan.containsKey(file)) - { - if (!oldScan.get(file).equals(currentScan.get(file))) - continue; - } - else if (currentScan.containsKey(file)) - continue; - - // File is stable so notify - Notification notification = entry.getValue(); - iter.remove(); - bulkChanges.add(file); - switch (notification) - { - case ADDED: - reportAddition(file); - break; - case CHANGED: - reportChange(file); - break; - case REMOVED: - reportRemoval(file); - break; - default: - break; - } + case ADDED: + reportAddition(entry.getKey()); + break; + case CHANGED: + reportChange(entry.getKey()); + break; + case REMOVED: + reportRemoval(entry.getKey()); + break; + default: + LOG.warn("Unknown file change: {}", entry.getValue()); + break; } - if (!bulkChanges.isEmpty()) - reportBulkChanges(bulkChanges); } + //Call the BulkListeners + reportBulkChanges(changes.keySet()); } private void warn(Object listener, String filename, Throwable th) @@ -857,6 +793,9 @@ private void reportRemoval(String filename) */ private void reportChange(String filename) { + if (filename == null) + return; + for (Listener l : _listeners) { try @@ -876,8 +815,11 @@ private void reportChange(String filename) * * @param filenames names of all files added/changed/removed */ - private void reportBulkChanges(List filenames) + private void reportBulkChanges(Set filenames) { + if (filenames == null || filenames.isEmpty()) + return; + for (Listener l : _listeners) { try @@ -894,6 +836,8 @@ private void reportBulkChanges(List filenames) /** * Call ScanCycleListeners with start of scan + * + * @param cycle scan count */ private void reportScanStart(int cycle) { @@ -902,9 +846,7 @@ private void reportScanStart(int cycle) try { if (listener instanceof ScanCycleListener) - { ((ScanCycleListener)listener).scanStarted(cycle); - } } catch (Exception e) { @@ -915,6 +857,8 @@ private void reportScanStart(int cycle) /** * Call ScanCycleListeners with end of scan. + * + * @param cycle scan count */ private void reportScanEnd(int cycle) { @@ -923,9 +867,8 @@ private void reportScanEnd(int cycle) try { if (listener instanceof ScanCycleListener) - { ((ScanCycleListener)listener).scanEnded(cycle); - } + } catch (Exception e) { 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 20ebfd1270c7..a4676120df71 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,9 +20,11 @@ import java.io.File; import java.io.IOException; -import java.util.Collections; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; import java.util.function.Consumer; +import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Scanner; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedOperation; @@ -77,7 +79,7 @@ public KeyStoreScanner(SslContextFactory sslContextFactory) throw new IllegalArgumentException("error obtaining keystore dir"); _scanner = new Scanner(); - _scanner.setScanDirs(Collections.singletonList(parentFile)); + _scanner.addDirectory(parentFile.toPath()); _scanner.setScanInterval(1); _scanner.setReportDirs(false); _scanner.setReportExistingFilesOnStartup(false); @@ -117,13 +119,23 @@ public void fileRemoved(String filename) } @ManagedOperation(value = "Scan for changes in the SSL Keystore", impact = "ACTION") - public void scan() + public boolean scan(long waitMillis) { if (LOG.isDebugEnabled()) LOG.debug("scanning"); - _scanner.scan(); - _scanner.scan(); + CompletableFuture cf = new CompletableFuture<>(); + try + { + // Perform 2 scans to be sure that the scan is stable. + _scanner.scan(Callback.from(() -> + _scanner.scan(Callback.from(() -> cf.complete(true), cf::completeExceptionally)), cf::completeExceptionally)); + return cf.get(waitMillis, TimeUnit.MILLISECONDS); + } + catch (Exception e) + { + throw new RuntimeException(e); + } } @ManagedOperation(value = "Reload the SSL Keystore", impact = "ACTION") @@ -135,7 +147,8 @@ public void reload() try { sslContextFactory.reload(scf -> - {}); + { + }); } catch (Throwable t) { 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 62a37f47587a..8e38c6aa8c8a 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 @@ -24,7 +24,10 @@ import java.io.OutputStream; import java.nio.file.Path; 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; import java.util.concurrent.TimeUnit; @@ -36,12 +39,12 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.DisabledIfSystemProperty; import org.junit.jupiter.api.condition.DisabledOnOs; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.condition.OS.WINDOWS; @@ -49,8 +52,8 @@ public class ScannerTest { static File _directory; static Scanner _scanner; - static BlockingQueue _queue = new LinkedBlockingQueue(); - static BlockingQueue> _bulk = new LinkedBlockingQueue>(); + static BlockingQueue _queue = new LinkedBlockingQueue<>(); + static BlockingQueue> _bulk = new LinkedBlockingQueue<>(); @BeforeAll public static void setUpBeforeClass() throws Exception @@ -63,38 +66,31 @@ public static void setUpBeforeClass() throws Exception _directory = testDir.toPath().toRealPath().toFile(); _scanner = new Scanner(); - _scanner.addScanDir(_directory); + _scanner.addDirectory(_directory.toPath()); _scanner.setScanInterval(0); _scanner.setReportDirs(false); _scanner.setReportExistingFilesOnStartup(false); _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(List filenames) throws Exception - { - _bulk.add(filenames); - } - }); + _scanner.addListener((Scanner.BulkListener)filenames -> _bulk.add(filenames)); _scanner.start(); _scanner.scan(); @@ -120,6 +116,29 @@ public Event(String filename, Notification notification) _filename = filename; _notification = notification; } + + @Override + public boolean equals(Object obj) + { + 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 + public int hashCode() + { + return Objects.hash(_filename, _notification); + } + + @Override + public String toString() + { + return ("File: " + _filename + ":" + _notification); + } } @Test @@ -142,7 +161,7 @@ public void testDepth() throws Exception File y2 = new File(dir2, "yyy2.foo"); FS.touch(y2); - BlockingQueue queue = new LinkedBlockingQueue(); + BlockingQueue queue = new LinkedBlockingQueue<>(); Scanner scanner = new Scanner(); scanner.setScanInterval(0); scanner.setScanDepth(0); @@ -152,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)); } @@ -222,7 +241,7 @@ public void testPatterns() throws Exception File y2 = new File(dir2, "yyy.txt"); FS.touch(y2); - BlockingQueue queue = new LinkedBlockingQueue(); + BlockingQueue queue = new LinkedBlockingQueue<>(); //only scan the *.txt files for changes Scanner scanner = new Scanner(); IncludeExcludeSet pattern = scanner.addDirectory(root.toPath()); @@ -235,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)); } @@ -264,8 +283,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")); } @@ -273,7 +294,6 @@ public void fileAdded(String filename) throws Exception @Test @DisabledOnOs(WINDOWS) // TODO: needs review - @DisabledIfSystemProperty(named = "env", matches = "ci") // TODO: SLOW, needs review public void testAddedChangeRemove() throws Exception { touch("a0"); @@ -282,7 +302,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); @@ -296,31 +316,31 @@ public void testAddedChangeRemove() throws Exception // not stable after 1 scan so should not be seen yet. _scanner.scan(); event = _queue.poll(); - assertTrue(event == null); + assertNull(event); - // Keep a2 unstable and remove a3 before it stabalized + // Keep a2 unstable and remove a3 before it stabilized Thread.sleep(1100); // make sure time in seconds changes touch("a2"); delete("a3"); - // only a1 is stable so it should be seen. + // only a1 is stable so it should be seen, a3 is deleted _scanner.scan(); - event = _queue.poll(); - assertTrue(event != null); - assertEquals(_directory + "/a1", event._filename); - assertEquals(Notification.ADDED, event._notification); + List actualEvents = new ArrayList<>(); + _queue.drainTo(actualEvents); + assertEquals(2, actualEvents.size()); + Event a1 = new Event(_directory + "/a1", Notification.ADDED); + 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(); - assertTrue(event != null); + assertNotNull(event); assertEquals(_directory + "/a2", event._filename); assertEquals(Notification.ADDED, event._notification); assertTrue(_queue.isEmpty()); - // We never see a3 as it was deleted before it stabalised - // touch a1 and a2 Thread.sleep(1100); // make sure time in seconds changes touch("a1"); @@ -328,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 @@ -337,7 +357,7 @@ public void testAddedChangeRemove() throws Exception // only a1 is stable so it should be seen. _scanner.scan(); event = _queue.poll(); - assertTrue(event != null); + assertNotNull(event); assertEquals(_directory + "/a1", event._filename); assertEquals(Notification.CHANGED, event._notification); assertTrue(_queue.isEmpty()); @@ -345,7 +365,7 @@ public void testAddedChangeRemove() throws Exception // Now a2 is stable _scanner.scan(); event = _queue.poll(); - assertTrue(event != null); + assertNotNull(event); assertEquals(_directory + "/a2", event._filename); assertEquals(Notification.CHANGED, event._notification); assertTrue(_queue.isEmpty()); @@ -353,28 +373,32 @@ public void testAddedChangeRemove() throws Exception // delete a1 and a2 delete("a1"); delete("a2"); - // not stable after 1scan so nothing should not be seen yet. + + //Immediate notification of deletes. _scanner.scan(); - event = _queue.poll(); - assertTrue(event == null); + a1 = new Event(_directory + "/a1", Notification.REMOVED); + Event a2 = new Event(_directory + "/a2", Notification.REMOVED); + actualEvents = new ArrayList<>(); + _queue.drainTo(actualEvents); + assertEquals(2, actualEvents.size()); + assertThat(actualEvents, Matchers.containsInAnyOrder(a1, a2)); + assertTrue(_queue.isEmpty()); - // readd a2 + // recreate a2 touch("a2"); - // only a1 is stable so it should be seen. + // a2 not stable yet, shouldn't be seen _scanner.scan(); event = _queue.poll(); - assertTrue(event != null); - assertEquals(_directory + "/a1", event._filename); - assertEquals(Notification.REMOVED, event._notification); + assertNull(event); assertTrue(_queue.isEmpty()); - // Now a2 is stable and is a changed file rather than a remove + //Now a2 is reported as ADDED. _scanner.scan(); event = _queue.poll(); - assertTrue(event != null); + assertNotNull(event); assertEquals(_directory + "/a2", event._filename); - assertEquals(Notification.CHANGED, event._notification); + assertEquals(Notification.ADDED, event._notification); assertTrue(_queue.isEmpty()); } @@ -386,9 +410,9 @@ public void testSizeChange() throws Exception _scanner.scan(); _scanner.scan(); - // takes 2s to notice tsc0 and check that it is stable. This syncs us with the scan + // takes 2 scans to notice tsc0 and check that it is stable. Event event = _queue.poll(); - assertTrue(event != null); + assertNotNull(event); assertEquals(_directory + "/tsc0", event._filename); assertEquals(Notification.ADDED, event._notification); @@ -404,7 +428,7 @@ public void testSizeChange() throws Exception // Not stable yet so no notification. _scanner.scan(); event = _queue.poll(); - assertTrue(event == null); + assertNull(event); // Modify size only out.write('x'); @@ -414,12 +438,12 @@ public void testSizeChange() throws Exception // Still not stable yet so no notification. _scanner.scan(); event = _queue.poll(); - assertTrue(event == null); + assertNull(event); // now stable so finally see the ADDED _scanner.scan(); event = _queue.poll(); - assertTrue(event != null); + assertNotNull(event); assertEquals(_directory + "/st", event._filename); assertEquals(Notification.ADDED, event._notification); @@ -431,18 +455,18 @@ public void testSizeChange() throws Exception // Still not stable yet so no notification. _scanner.scan(); event = _queue.poll(); - assertTrue(event == null); + assertNull(event); // now stable so finally see the ADDED _scanner.scan(); event = _queue.poll(); - assertTrue(event != null); + assertNotNull(event); assertEquals(_directory + "/st", event._filename); assertEquals(Notification.CHANGED, event._notification); } } - private void delete(String string) throws IOException + private void delete(String string) { File file = new File(_directory, string); if (file.exists()) diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java index 57b9390c9c98..35fb52af9505 100644 --- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java +++ b/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(5000)); // The scanner should have detected the updated keystore, expiry should be renewed. X509Certificate cert2 = getCertificateFromServer(); @@ -146,7 +146,7 @@ public void testReloadWithBadKeystore() throws Exception try (StacklessLogging ignored = new StacklessLogging(KeyStoreScanner.class)) { useKeystore("badKeystore"); - keystoreScanner.scan(); + keystoreScanner.scan(5000); } // The good keystore is removed, now the bad keystore now causes an exception. @@ -167,7 +167,7 @@ public void testKeystoreRemoval() throws Exception { Path keystorePath = keystoreDir.resolve("keystore"); assertTrue(Files.deleteIfExists(keystorePath)); - keystoreScanner.scan(); + keystoreScanner.scan(5000); } // The good keystore is removed, having no keystore causes an exception. @@ -175,7 +175,7 @@ public void testKeystoreRemoval() throws Exception // Switch to use keystore2 which has a later expiry date. useKeystore("newKeystore"); - keystoreScanner.scan(); + keystoreScanner.scan(5000); X509Certificate cert2 = getCertificateFromServer(); assertThat(getExpiryYear(cert2), is(2020)); } @@ -200,7 +200,7 @@ public void testReloadChangingSymbolicLink() throws Exception // Change the symlink to point to the newKeystore file location which has a later expiry date. Files.delete(keystorePath); Files.createSymbolicLink(keystorePath, useKeystore("newKeystore")); - keystoreScanner.scan(); + keystoreScanner.scan(5000); // The scanner should have detected the updated keystore, expiry should be renewed. X509Certificate cert2 = getCertificateFromServer(); @@ -232,7 +232,7 @@ public void testReloadChangingTargetOfSymbolicLink() throws Exception // Change the target file of the symlink to the newKeystore which has a later expiry date. Files.copy(newKeystoreSrc, target, StandardCopyOption.REPLACE_EXISTING); System.err.println("### Triggering scan"); - keystoreScanner.scan(); + keystoreScanner.scan(5000); // The scanner should have detected the updated keystore, expiry should be renewed. X509Certificate cert2 = getCertificateFromServer();