Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 11 commits into from Dec 2, 2020
Merged

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

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4883332
Issue #5086 Review and rework o.e.j.util.Scanner
janbartel Dec 1, 2020
4304c78
Issue #5086 Improve ScannerTest
janbartel Dec 1, 2020
115a72d
Issue #5086 Fix checkstyle in test
janbartel Dec 1, 2020
90e48ba
Issue #5086 Fix javadoc
janbartel Dec 1, 2020
31cc298
Issue #5086 Changes after review
janbartel Dec 1, 2020
e39ec6c
Issue #5086 Fix error on addFile
janbartel Dec 2, 2020
c776eeb
Merge remote-tracking branch 'origin/jetty-10.0.x' into jetty-10.0.x-…
janbartel Dec 2, 2020
8fe8cb8
Issue #5086 Add blocking method for immediate scan.
janbartel Dec 2, 2020
97b3f26
Fixed scan(Callback) bug, returning early in case of failures.
sbordet Dec 2, 2020
fe0e076
Using capital case for static final field.
sbordet Dec 2, 2020
9c882ee
Improved KeyStoreScanner.scan().
sbordet Dec 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion jetty-ant/src/main/java/org/eclipse/jetty/ant/ServerProxyImpl.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -135,7 +136,7 @@ public WebAppScannerListener(AntWebAppContext awc)
}

@Override
public void filesChanged(List<String> changedFileNames)
public void filesChanged(Set<String> changedFileNames)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't like this. File names are unique by definition, so there is no risk of duplicates. I would not add additional randomness caused by Set. If I really don't care, I'd use Collection -- the Set is supposed to be immutable anyway, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand your point. If it's a List then by definition I can have duplicates in a List, but I can't in a Set. Plus, you can't rely on ordering as it depends on the filesystem in what order scanned directories are visited, so List is a furphy.

{
boolean isScanned = false;
try
Expand Down
23 changes: 3 additions & 20 deletions jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java
View file
Open in desktop
Expand Up @@ -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<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 boolean _recursive = false;
private int _scanInterval = 10;
private Scanner _scanner;

Expand Down Expand Up @@ -140,7 +135,6 @@ protected void doStart() throws Exception
_scanner = new Scanner();
_scanner.setScanDirs(files);
_scanner.setScanInterval(_scanInterval);
_scanner.setRecursive(_recursive);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scanner.setRecursive() was deprecated, but there is no Javadoc about why.
This is the only usage of ScanningAppProvider._recursive which then should also be deprecated and removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

_scanner.setFilenameFilter(_filenameFilter);
_scanner.setReportDirs(true);
_scanner.setScanDepth(1); //consider direct dir children of monitored dir
Expand Down Expand Up @@ -237,12 +231,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 @@ -295,11 +283,6 @@ public void setMonitoredDirectories(Collection<String> directories)
}
}

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

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

@Override
Expand Down
4 changes: 2 additions & 2 deletions ...c/test/java/org/eclipse/jetty/deploy/providers/ScanningAppProviderRuntimeUpdatesTest.java
View file
Open in desktop
Expand Up @@ -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();
}
Expand Down
4 changes: 2 additions & 2 deletions jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunMojo.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -191,7 +191,7 @@ protected void configureScanner()
}
scanner.addListener(new Scanner.BulkListener()
{
public void filesChanged(List<String> changes)
public void filesChanged(Set<String> changes)
{
try
{
Expand Down
4 changes: 2 additions & 2 deletions jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunWarMojo.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -197,7 +197,7 @@ public void configureScanner() throws MojoExecutionException
configureScanTargetPatterns(scanner);
scanner.addListener(new Scanner.BulkListener()
{
public void filesChanged(List<String> changes)
public void filesChanged(Set<String> changes)
{
try
{
Expand Down