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

Use File.list and File.walk within a try with resource #5718

Merged
merged 5 commits into from Nov 24, 2020
Merged

Use File.list and File.walk within a try with resource #5718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7627b42
Use File.list and File.walk within a try with resource
gregw Nov 24, 2020
dd196e7
Fix from review
gregw Nov 24, 2020
5a63e60
Fix from review
gregw Nov 24, 2020
a83462d
Fix from review
gregw Nov 24, 2020
154c679
Fix from review
gregw Nov 24, 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
179 changes: 88 additions & 91 deletions jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java
View file
Open in desktop
Expand Up @@ -35,6 +35,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;

import org.eclipse.jetty.util.ClassLoadingObjectInputStream;
import org.eclipse.jetty.util.MultiException;
Expand Down Expand Up @@ -153,7 +154,7 @@ public boolean deleteFile(String filename) throws Exception
public Set<String> doGetExpired(final Set<String> candidates)
{
final long now = System.currentTimeMillis();
HashSet<String> expired = new HashSet<String>();
HashSet<String> expired = new HashSet<>();

//iterate over the files and work out which have expired
for (String filename : _sessionFileMap.values())
Expand Down Expand Up @@ -206,23 +207,12 @@ public void sweepDisk()
long now = System.currentTimeMillis();
if (LOG.isDebugEnabled())
LOG.debug("Sweeping {} for old session files", _storeDir);
try
try (Stream<Path> stream = Files.walk(_storeDir.toPath(), 1, FileVisitOption.FOLLOW_LINKS))
{
Files.walk(_storeDir.toPath(), 1, FileVisitOption.FOLLOW_LINKS)
stream
.filter(p -> !Files.isDirectory(p)).filter(p -> !isOurContextSessionFilename(p.getFileName().toString()))
.filter(p -> isSessionFilename(p.getFileName().toString()))
.forEach(p ->
{

try
{
sweepFile(now, p);
}
catch (Exception e)
{
LOG.warn(e);
}
});
.forEach(p -> sweepFile(now, p));
}
catch (Exception e)
{
Expand All @@ -237,30 +227,36 @@ public void sweepDisk()
*
* @param now the time now in msec
* @param p the file to check
* @throws Exception indicating error in sweep
*/
public void sweepFile(long now, Path p)
throws Exception
{
if (p == null)
return;

try
if (p != null)
{
long expiry = getExpiryFromFilename(p.getFileName().toString());
//files with 0 expiry never expire
if (expiry > 0 && ((now - expiry) >= (5 * TimeUnit.SECONDS.toMillis(_gracePeriodSec))))
try
{
long expiry = getExpiryFromFilename(p.getFileName().toString());
//files with 0 expiry never expire
if (expiry > 0 && ((now - expiry) >= (5 * TimeUnit.SECONDS.toMillis(_gracePeriodSec))))
{
try
{
if (!Files.deleteIfExists(p))
LOG.warn("Could not delete {}", p.getFileName());
else if (LOG.isDebugEnabled())
LOG.debug("Deleted {}", p.getFileName());
}
catch (IOException e)
{
LOG.warn("Could not delete {}", p.getFileName(), e);
}
}
}
catch (NumberFormatException e)
{
Files.deleteIfExists(p);
if (LOG.isDebugEnabled())
LOG.debug("Sweep deleted {}", p.getFileName());
LOG.warn("Not valid session filename {}", p.getFileName());
LOG.warn(e);
}
}
catch (NumberFormatException e)
{
LOG.warn("Not valid session filename {}", p.getFileName());
LOG.warn(e);
}
}

@Override
Expand Down Expand Up @@ -311,7 +307,7 @@ public SessionData doLoad(String id) throws Exception
@Override
public void doStore(String id, SessionData data, long lastSaveTime) throws Exception
{
File file = null;
File file;
if (_storeDir != null)
{
delete(id);
Expand All @@ -328,8 +324,9 @@ public void doStore(String id, SessionData data, long lastSaveTime) throws Excep
}
catch (Exception e)
{
if (file != null)
file.delete(); // No point keeping the file if we didn't save the whole session
// No point keeping the file if we didn't save the whole session
if (!file.delete())
e.addSuppressed(new IOException("Could not delete " + file));
throw new UnwriteableSessionDataException(id, _context, e);
}
}
Expand All @@ -353,7 +350,10 @@ public void initializeStore()
throw new IllegalStateException("No file store specified");

if (!_storeDir.exists())
_storeDir.mkdirs();
{
if (!_storeDir.mkdirs())
throw new IllegalStateException("Could not create " + _storeDir);
}
else
{
if (!(_storeDir.isDirectory() && _storeDir.canWrite() && _storeDir.canRead()))
Expand All @@ -365,70 +365,67 @@ public void initializeStore()
MultiException me = new MultiException();
long now = System.currentTimeMillis();

Files.walk(_storeDir.toPath(), 1, FileVisitOption.FOLLOW_LINKS)
.filter(p -> !Files.isDirectory(p)).filter(p -> isSessionFilename(p.getFileName().toString()))
.forEach(p ->
{
//first get rid of all ancient files, regardless of which
//context they are for
try
// Build session file map by walking directory
try (Stream<Path> stream = Files.walk(_storeDir.toPath(), 1, FileVisitOption.FOLLOW_LINKS))
{
stream
.filter(p -> !Files.isDirectory(p))
.filter(p -> isSessionFilename(p.getFileName().toString()))
.forEach(p ->
{
// first get rid of all ancient files
sweepFile(now, p);
}
catch (Exception x)
{
me.add(x);
}

String filename = p.getFileName().toString();
String context = getContextFromFilename(filename);
//now process it if it wasn't deleted, and it is for our context
if (Files.exists(p) && _contextString.equals(context))
{
//the session is for our context, populate the map with it
String sessionIdWithContext = getIdWithContextFromFilename(filename);
if (sessionIdWithContext != null)
String filename = p.getFileName().toString();
String context = getContextFromFilename(filename);
//now process it if it wasn't deleted, and it is for our context
if (Files.exists(p) && _contextString.equals(context))
{
//handle multiple session files existing for the same session: remove all
//but the file with the most recent expiry time
String existing = _sessionFileMap.putIfAbsent(sessionIdWithContext, filename);
if (existing != null)
//the session is for our context, populate the map with it
String sessionIdWithContext = getIdWithContextFromFilename(filename);
if (sessionIdWithContext != null)
{
//if there was a prior filename, work out which has the most
//recent modify time
try
//handle multiple session files existing for the same session: remove all
//but the file with the most recent expiry time
String existing = _sessionFileMap.putIfAbsent(sessionIdWithContext, filename);
if (existing != null)
{
long existingExpiry = getExpiryFromFilename(existing);
long thisExpiry = getExpiryFromFilename(filename);

if (thisExpiry > existingExpiry)
//if there was a prior filename, work out which has the most
//recent modify time
try
{
//replace with more recent file
Path existingPath = _storeDir.toPath().resolve(existing);
//update the file we're keeping
_sessionFileMap.put(sessionIdWithContext, filename);
//delete the old file
Files.delete(existingPath);
if (LOG.isDebugEnabled())
LOG.debug("Replaced {} with {}", existing, filename);
long existingExpiry = getExpiryFromFilename(existing);
long thisExpiry = getExpiryFromFilename(filename);

if (thisExpiry > existingExpiry)
{
//replace with more recent file
Path existingPath = _storeDir.toPath().resolve(existing);
//update the file we're keeping
_sessionFileMap.put(sessionIdWithContext, filename);
//delete the old file
Files.delete(existingPath);
janbartel marked this conversation as resolved.
Show resolved Hide resolved
if (LOG.isDebugEnabled())
LOG.debug("Replaced {} with {}", existing, filename);
}
else
{
//we found an older file, delete it
Files.delete(p);
if (LOG.isDebugEnabled())
LOG.debug("Deleted expired session file {}", filename);
}
}
else
catch (IOException e)
{
//we found an older file, delete it
Files.delete(p);
if (LOG.isDebugEnabled())
LOG.debug("Deleted expired session file {}", filename);
me.add(e);
}
}
catch (IOException e)
{
me.add(e);
}
}
}
}
});
me.ifExceptionThrow();
});
me.ifExceptionThrow();
}
}
}

Expand Down Expand Up @@ -511,11 +508,11 @@ protected String getIdFromFilename(String filename)

protected long getExpiryFromFilename(String filename)
{
if (StringUtil.isBlank(filename) || filename.indexOf("_") < 0)
if (StringUtil.isBlank(filename) || !filename.contains("_"))
throw new IllegalStateException("Invalid or missing filename");

String s = filename.substring(0, filename.indexOf('_'));
return (s == null ? 0 : Long.parseLong(s));
return Long.parseLong(s);
}

protected String getContextFromFilename(String filename)
Expand Down Expand Up @@ -592,11 +589,11 @@ protected boolean isOurContextSessionFilename(String filename)
protected SessionData load(InputStream is, String expectedId)
throws Exception
{
String id = null; //the actual id from inside the file
String id; //the actual id from inside the file

try
{
SessionData data = null;
SessionData data;
DataInputStream di = new DataInputStream(is);

id = di.readUTF();
Expand Down
3 changes: 1 addition & 2 deletions jetty-server/src/test/java/org/eclipse/jetty/server/CustomResourcesMonitorTest.java
View file
Open in desktop
Expand Up @@ -146,9 +146,8 @@ public FileOnDirectoryMonitor(Path pathToMonitor)
@Override
public boolean isLowOnResources()
{
try
try (Stream<Path> paths = Files.list(_pathToMonitor))
{
Stream<Path> paths = Files.list(_pathToMonitor);
List<Path> content = paths.collect(Collectors.toList());
if (!content.isEmpty())
{
Expand Down
5 changes: 3 additions & 2 deletions jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
View file
Open in desktop
Expand Up @@ -36,6 +36,7 @@
import java.util.Locale;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Stream;
import javax.servlet.DispatcherType;
import javax.servlet.MultipartConfigElement;
import javax.servlet.ServletException;
Expand Down Expand Up @@ -1837,9 +1838,9 @@ public void testGetterSafeFromNullPointerException()

private static long getFileCount(Path path)
{
try
try (Stream<Path> s = Files.list(path))
{
return Files.list(path).count();
return s.count();
}
catch (IOException e)
{
Expand Down
7 changes: 5 additions & 2 deletions jetty-util/src/test/java/org/eclipse/jetty/util/RolloverFileOutputStreamTest.java
View file
Open in desktop
Expand Up @@ -31,7 +31,7 @@
import java.util.Arrays;
import java.util.TimeZone;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
Expand Down Expand Up @@ -379,6 +379,9 @@ private String readPath(Path path) throws IOException

private String[] ls(Path path) throws IOException
{
return Files.list(path).map(p -> p.getFileName().toString()).collect(Collectors.toList()).toArray(new String[0]);
try (Stream<Path> s = Files.list(path))
{
return s.map(p -> p.getFileName().toString()).toArray(String[]::new);
}
}
}
6 changes: 5 additions & 1 deletion jetty-util/src/test/java/org/eclipse/jetty/util/resource/JarResourceTest.java
View file
Open in desktop
Expand Up @@ -30,6 +30,7 @@
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.zip.ZipFile;

import org.eclipse.jetty.toolchain.test.FS;
Expand Down Expand Up @@ -264,7 +265,10 @@ public void testJarFileResourceListDirWithSpace() throws Exception

private List<Path> listFiles(Path dir) throws IOException
{
return Files.list(dir).collect(Collectors.toList());
try (Stream<Path> s = Files.list(dir))
{
return s.collect(Collectors.toList());
}
}

private List<Path> listFiles(Path dir, DirectoryStream.Filter<? super Path> filter) throws IOException
Expand Down
12 changes: 8 additions & 4 deletions jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java
View file
Open in desktop
Expand Up @@ -365,10 +365,14 @@ public void testExtraClasspathGlob(String description, String extraClasspathGlob
WebAppClassLoader webAppClassLoader = (WebAppClassLoader)contextClassLoader;
Path extLibsDir = MavenTestingUtils.getTestResourcePathDir("ext");
extLibsDir = extLibsDir.toAbsolutePath();
List<Path> expectedPaths = Files.list(extLibsDir)
.filter(Files::isRegularFile)
.filter((path) -> path.toString().endsWith(".jar"))
.collect(Collectors.toList());
List<Path> expectedPaths;
try (Stream<Path> s = Files.list(extLibsDir))
{
expectedPaths = s
.filter(Files::isRegularFile)
.filter((path) -> path.toString().endsWith(".jar"))
.collect(Collectors.toList());
}
List<Path> actualPaths = new ArrayList<>();
for (URL url : webAppClassLoader.getURLs())
{
Expand Down