Skip to content

Commit

Permalink
Fixed scan(Callback) bug, returning early in case of failures.
Browse files Browse the repository at this point in the history 
More cleanups in code adding more privateness, getting rid of unnecessary exceptions, making fields final, etc.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
@sbordet
sbordet committed Dec 2, 2020
1 parent 8fe8cb8 commit 97b3f26
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 85 deletions.
8 changes: 2 additions & 6 deletions jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 0 additions & 5 deletions ...c/test/java/org/eclipse/jetty/deploy/providers/ScanningAppProviderRuntimeUpdatesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
77 changes: 29 additions & 48 deletions jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()
{
}
Expand Down Expand Up @@ -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
Expand All @@ -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();
}

/**
Expand Down Expand Up @@ -604,12 +589,7 @@ public void nudge()
{
if (!isRunning())
throw new IllegalStateException("Scanner not running");

_scheduler.schedule(() ->
{
scan();

}, 0, TimeUnit.MILLISECONDS);
scan(Callback.NOOP);
}

/**
Expand All @@ -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
{
Expand All @@ -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
*/
Expand Down
5 changes: 2 additions & 3 deletions jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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)
{
Expand Down
42 changes: 20 additions & 22 deletions jetty-util/src/test/java/org/eclipse/jetty/util/ScannerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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));
}
Expand Down Expand Up @@ -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());
Expand All @@ -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));
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 97b3f26

Please sign in to comment.