Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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

* Use File.list and File.walk within a try with resource

The API contract of File.list and File.walk requires them to be closed after use.

* Fix from review

Left out filter

* Fix from review

Factored out deleteFile with better debug

* Fix from review

Can delete files whilst walking

* Fix from review

Restored sweepFile
fixed minor code suggestions
This commit is contained in:
Greg Wilkins 2020-11-24 18:12:42 +01:00 committed by GitHub
parent d0a7c881c2
commit 9f82ca0a80
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 133 additions and 123 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java
View File

@ -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<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())
@ -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<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)
{
@ -237,23 +227,28 @@ 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;
if (p != null)
{
try
{
long expiry = getExpiryFromFilename(p.getFileName().toString());
//files with 0 expiry never expire
if (expiry > 0 && ((now - expiry) >= (5 * TimeUnit.SECONDS.toMillis(_gracePeriodSec))))
{
Files.deleteIfExists(p);
if (LOG.isDebugEnabled())
LOG.debug("Sweep deleted {}", p.getFileName());
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)
@ -262,6 +257,7 @@ public class FileSessionDataStore extends AbstractSessionDataStore
LOG.warn(e);
}
}
}
@Override
public SessionData doLoad(String id) throws Exception
@ -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,20 +365,16 @@ 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()))
// 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, regardless of which
//context they are for
try
{
// 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);
@ -431,6 +427,7 @@ public class FileSessionDataStore extends AbstractSessionDataStore
me.ifExceptionThrow();
}
}
}
@Override
@ManagedAttribute(value = "are sessions serialized by this store", readonly = true)
@ -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();

3
jetty-server/src/test/java/org/eclipse/jetty/server/CustomResourcesMonitorTest.java
View File

@ -146,9 +146,8 @@ public class CustomResourcesMonitorTest
@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())
{

5
jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
View File

@ -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<Path> s = Files.list(path))
{
return Files.list(path).count();
return s.count();
}
catch (IOException e)
{

7
jetty-util/src/test/java/org/eclipse/jetty/util/RolloverFileOutputStreamTest.java
View File

@ -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<Path> s = Files.list(path))
{
return s.map(p -> p.getFileName().toString()).toArray(String[]::new);
}
}
}

6
jetty-util/src/test/java/org/eclipse/jetty/util/resource/JarResourceTest.java
View File

@ -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<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

6
jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java
View File

@ -365,10 +365,14 @@ public class WebAppContextTest
WebAppClassLoader webAppClassLoader = (WebAppClassLoader)contextClassLoader;
Path extLibsDir = MavenTestingUtils.getTestResourcePathDir("ext");
extLibsDir = extLibsDir.toAbsolutePath();
List<Path> expectedPaths = Files.list(extLibsDir)
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())
{

30
tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/FileTestHelper.java
View File

@ -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,17 +49,21 @@ public class FileTestHelper
public static File getFile(WorkDir workDir, String sessionId) throws IOException
{
Optional<Path> sessionPath = Files.list(workDir.getPath())
try (Stream<Path> s = Files.list(workDir.getPath()))
{
return s
.filter((path) -> path.getFileName().toString().contains(sessionId))
.findFirst();
if (sessionPath.isPresent())
return sessionPath.get().toFile();
return null;
.findFirst()
.map(Path::toFile)
.orElse(null);
}
}
public static void assertSessionExists(WorkDir workDir, String sessionId, boolean exists) throws IOException
{
Optional<Path> sessionPath = Files.list(workDir.getPath())
try (Stream<Path> s = Files.list(workDir.getPath()))
{
Optional<Path> sessionPath = s
.filter((path) -> path.getFileName().toString().contains(sessionId))
.findFirst();
if (exists)
@ -67,6 +71,7 @@ public class FileTestHelper
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<Path> 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<Path> 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);
}
}