Skip to content

Commit

Permalink
Issue #5086 Changes after review
Browse files Browse the repository at this point in the history
Signed-off-by: Jan Bartel <janb@webtide.com>
  • Loading branch information
janbartel committed Dec 1, 2020
1 parent 90e48ba commit 31cc298
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ public abstract class ScanningAppProvider extends ContainerLifeCycle implements
private DeploymentManager _deploymentManager;
protected FilenameFilter _filenameFilter;
private final List<Resource> _monitored = new CopyOnWriteArrayList<>();
private boolean _recursive = false;
private int _scanInterval = 10;
private Scanner _scanner;

Expand Down Expand Up @@ -236,12 +235,6 @@ public int getScanInterval()
return _scanInterval;
}

@ManagedAttribute("recursive scanning supported")
public boolean isRecursive()
{
return _recursive;
}

@Override
public void setDeploymentManager(DeploymentManager deploymentManager)
{
Expand Down Expand Up @@ -294,11 +287,6 @@ public void setMonitoredDirectories(Collection<String> directories)
}
}

protected void setRecursive(boolean recursive)
{
_recursive = recursive;
}

public void setScanInterval(int scanInterval)
{
_scanInterval = scanInterval;
Expand All @@ -311,7 +299,7 @@ public void scan()
getMonitoredResources().stream().map((r) -> r.getURI().toASCIIString())
.collect(Collectors.joining(", ", "[", "]"))
);
_scanner.scan();
_scanner.nudge();
}

@Override
Expand Down
117 changes: 61 additions & 56 deletions jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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<Listener> _listeners = new CopyOnWriteArrayList<>();
private Map<String, MetaData> _prevScan;
private FilenameFilter _filter;
private final Map<Path, IncludeExcludeSet<PathMatcher, Path>> _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
Expand Down Expand Up @@ -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;
Expand All @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -259,7 +261,6 @@ public interface Listener

/**
* Notification of exact file changes in the last scan.
*
*/
public interface DiscreteListener extends Listener
{
Expand All @@ -272,7 +273,6 @@ public interface DiscreteListener extends Listener

/**
* Notification of files that changed in the last scan.
*
*/
public interface BulkListener extends Listener
{
Expand Down Expand Up @@ -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<File> dirs)
Expand Down Expand Up @@ -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={}",
Expand All @@ -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);
}
}

Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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);
Expand All @@ -620,7 +625,7 @@ public void scan()
/**
* Scan all of the given paths.
*/
public Map<String, MetaData> scanFiles()
private Map<String, MetaData> scanFiles()
{
Map<String, MetaData> currentScan = new HashMap<>();
for (Path p : _scannables.keySet())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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")
Expand Down
23 changes: 16 additions & 7 deletions jetty-util/src/test/java/org/eclipse/jetty/util/ScannerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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<Event> 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"));
}
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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");

Expand All @@ -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());
Expand Down

0 comments on commit 31cc298

Please sign in to comment.