diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java index bcb8eef69cc..68284a51891 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java @@ -35,6 +35,7 @@ import java.util.Map; 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; @@ -153,7 +154,7 @@ public class FileSessionDataStore extends AbstractSessionDataStore public Set doGetExpired(final Set candidates) { final long now = System.currentTimeMillis(); - HashSet expired = new HashSet(); + HashSet expired = new HashSet<>(); //iterate over the files and work out which have expired for (String filename : _sessionFileMap.values()) @@ -206,23 +207,12 @@ public class FileSessionDataStore extends AbstractSessionDataStore long now = System.currentTimeMillis(); if (LOG.isDebugEnabled()) LOG.debug("Sweeping {} for old session files", _storeDir); - try + try (Stream 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) { @@ -237,29 +227,35 @@ public class FileSessionDataStore extends AbstractSessionDataStore * * @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 { - Files.deleteIfExists(p); - if (LOG.isDebugEnabled()) - LOG.debug("Sweep deleted {}", p.getFileName()); + 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) + { + 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); } } @@ -311,7 +307,7 @@ public class FileSessionDataStore extends AbstractSessionDataStore @Override public void doStore(String id, SessionData data, long lastSaveTime) throws Exception { - File file = null; + File file; if (_storeDir != null) { delete(id); @@ -328,8 +324,9 @@ public class FileSessionDataStore extends AbstractSessionDataStore } 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); } } @@ -353,7 +350,10 @@ public class FileSessionDataStore extends AbstractSessionDataStore 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())) @@ -365,70 +365,67 @@ public class FileSessionDataStore extends AbstractSessionDataStore 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 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 there was a prior filename, work out which has the most + //recent modify time + try + { + 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); - if (LOG.isDebugEnabled()) - LOG.debug("Replaced {} with {}", existing, 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); + 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(); + } } } @@ -511,11 +508,11 @@ public class FileSessionDataStore extends AbstractSessionDataStore 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) @@ -592,11 +589,11 @@ public class FileSessionDataStore extends AbstractSessionDataStore 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(); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/CustomResourcesMonitorTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/CustomResourcesMonitorTest.java index c3926e6761e..7ff1f00add5 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/CustomResourcesMonitorTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/CustomResourcesMonitorTest.java @@ -146,9 +146,8 @@ public class CustomResourcesMonitorTest @Override public boolean isLowOnResources() { - try + try (Stream paths = Files.list(_pathToMonitor)) { - Stream paths = Files.list(_pathToMonitor); List content = paths.collect(Collectors.toList()); if (!content.isEmpty()) { diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 20486ca84b7..f18c5aa6d80 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -36,6 +36,7 @@ import java.util.List; 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; @@ -1837,9 +1838,9 @@ public class RequestTest private static long getFileCount(Path path) { - try + try (Stream s = Files.list(path)) { - return Files.list(path).count(); + return s.count(); } catch (IOException e) { diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/RolloverFileOutputStreamTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/RolloverFileOutputStreamTest.java index 75c494fa37a..ab311d2a947 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/RolloverFileOutputStreamTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/RolloverFileOutputStreamTest.java @@ -31,7 +31,7 @@ import java.time.temporal.TemporalAccessor; 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; @@ -379,6 +379,9 @@ public class RolloverFileOutputStreamTest 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 s = Files.list(path)) + { + return s.map(p -> p.getFileName().toString()).toArray(String[]::new); + } } } diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/JarResourceTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/JarResourceTest.java index db944868ea5..cc4a7ed917f 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/JarResourceTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/JarResourceTest.java @@ -30,6 +30,7 @@ import java.util.HashSet; 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; @@ -264,7 +265,10 @@ public class JarResourceTest private List listFiles(Path dir) throws IOException { - return Files.list(dir).collect(Collectors.toList()); + try (Stream s = Files.list(dir)) + { + return s.collect(Collectors.toList()); + } } private List listFiles(Path dir, DirectoryStream.Filter filter) throws IOException diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java index 93f16756412..123955c2027 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java @@ -365,10 +365,14 @@ public class WebAppContextTest WebAppClassLoader webAppClassLoader = (WebAppClassLoader)contextClassLoader; Path extLibsDir = MavenTestingUtils.getTestResourcePathDir("ext"); extLibsDir = extLibsDir.toAbsolutePath(); - List expectedPaths = Files.list(extLibsDir) - .filter(Files::isRegularFile) - .filter((path) -> path.toString().endsWith(".jar")) - .collect(Collectors.toList()); + List expectedPaths; + try (Stream s = Files.list(extLibsDir)) + { + expectedPaths = s + .filter(Files::isRegularFile) + .filter((path) -> path.toString().endsWith(".jar")) + .collect(Collectors.toList()); + } List actualPaths = new ArrayList<>(); for (URL url : webAppClassLoader.getURLs()) { diff --git a/tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/FileTestHelper.java b/tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/FileTestHelper.java index a46322b5968..15f9d753830 100644 --- a/tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/FileTestHelper.java +++ b/tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/FileTestHelper.java @@ -27,10 +27,10 @@ import java.io.ObjectOutputStream; import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Path; -import java.util.List; import java.util.Map; import java.util.Optional; 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; @@ -49,23 +49,28 @@ public class FileTestHelper public static File getFile(WorkDir workDir, String sessionId) throws IOException { - Optional sessionPath = Files.list(workDir.getPath()) - .filter((path) -> path.getFileName().toString().contains(sessionId)) - .findFirst(); - if (sessionPath.isPresent()) - return sessionPath.get().toFile(); - return null; + try (Stream s = Files.list(workDir.getPath())) + { + return s + .filter((path) -> path.getFileName().toString().contains(sessionId)) + .findFirst() + .map(Path::toFile) + .orElse(null); + } } public static void assertSessionExists(WorkDir workDir, String sessionId, boolean exists) throws IOException { - Optional sessionPath = Files.list(workDir.getPath()) - .filter((path) -> path.getFileName().toString().contains(sessionId)) - .findFirst(); - if (exists) - assertTrue(sessionPath.isPresent()); - else - assertFalse(sessionPath.isPresent()); + try (Stream s = Files.list(workDir.getPath())) + { + Optional sessionPath = s + .filter((path) -> path.getFileName().toString().contains(sessionId)) + .findFirst(); + if (exists) + assertTrue(sessionPath.isPresent()); + else + assertFalse(sessionPath.isPresent()); + } } public static void assertFileExists(WorkDir workDir, String filename, boolean exists) @@ -170,14 +175,11 @@ public class FileTestHelper public static void deleteFile(WorkDir workDir, String sessionId) throws IOException { // Collect - List matches = Files.list(workDir.getPath()) - .filter((path) -> path.getFileName().toString().contains(sessionId)) - .collect(Collectors.toList()); - - // Delete outside of lambda - for (Path path : matches) + try (Stream s = Files.list(workDir.getPath())) { - FS.deleteFile(path); + s.filter((path) -> path.getFileName().toString().contains(sessionId)) + .collect(Collectors.toList()) // Delete outside of list stream + .forEach(FS::deleteFile); } }