Skip to content

Commit

Permalink
[GEOS-11533] Address PMD warnings from org.apache.commons.vfs2 use of…
Browse files Browse the repository at this point in the history 
… AutoClosable
  • Loading branch information
@jodygarnett
jodygarnett committed Sep 18, 2024
1 parent 64baeb1 commit c5fd49d
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 90 deletions.
4 changes: 2 additions & 2 deletions build/qa/pmd-ruleset.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ GeoTools ruleset. See https://pmd.github.io/latest/pmd_userdocs_understanding_ru
<properties>
<!-- When calling the store to close, PMD wants the full prefix before the call to -->
<!-- the method to match, so let's try to use common var names for store ... -->
<property name="closeTargets" value="releaseConnection,store.releaseConnection,closeQuietly,closeConnection,closeSafe,store.closeSafe,dataStore.closeSafe,getDataStore().closeSafe,close,closeResultSet,closeStmt,closeFinally,JDBCUtils.close"/>
<property name="allowedResourceTypes" value="java.io.ByteArrayOutputStream|java.io.ByteArrayInputStream|java.io.StringWriter|java.io.CharArrayWriter|java.util.stream.Stream|java.util.stream.IntStream|java.util.stream.LongStream|java.util.stream.DoubleStream|java.io.StringReader" />
<property name="closeTargets" value="releaseConnection,store.releaseConnection,closeQuietly,closeConnection,closeSafe,store.closeSafe,dataStore.closeSafe,getDataStore().closeSafe,close,closeResultSet,closeStmt,closeFinally,JDBCUtils.close,closeFileSystem"/>
<property name="allowedResourceTypes" value="java.io.ByteArrayOutputStream|java.io.ByteArrayInputStream|java.io.StringWriter|java.io.CharArrayWriter|java.util.stream.Stream|java.util.stream.IntStream|java.util.stream.LongStream|java.util.stream.DoubleStream|java.io.StringReader|org.apache.commons.vfs2.FileSystemManager" />
</properties>
</rule>
<rule ref="category/java/multithreading.xml/AvoidThreadGroup"/>
Expand Down
11 changes: 4 additions & 7 deletions src/extension/importer/core/src/main/java/org/geoserver/importer/Directory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.apache.commons.vfs2.AllFileSelector;
import org.apache.commons.vfs2.FileName;
import org.apache.commons.vfs2.FileObject;
import org.apache.commons.vfs2.FileSystemManager;
import org.apache.commons.vfs2.VFS;
import org.geoserver.importer.job.ProgressMonitor;
import org.geoserver.util.IOUtils;
Expand Down Expand Up @@ -386,19 +387,15 @@ public String toString() {
}

public void accept(FileObject fo) throws IOException {
FileSystemManager sharedManager = VFS.getManager();

FileName name = fo.getName();
String localName = name.getBaseName();
File dest = child(localName);
FileObject dfo = null;
try {
dfo = VFS.getManager().resolveFile(dest.getAbsolutePath());
try (FileObject dfo = sharedManager.resolveFile(dest.getAbsolutePath())) {
dfo.copyFrom(fo, new AllFileSelector());

unpack(dest);
} finally {
if (dfo != null) {
dfo.close();
}
}
}

Expand Down
44 changes: 24 additions & 20 deletions src/extension/importer/core/src/main/java/org/geoserver/importer/RemoteData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,32 +82,36 @@ public void setPassword(String password) {
this.password = password;
}

@SuppressWarnings("PMD.UseTryWithResources")
public ImportData resolve(Importer importer) throws IOException {
// prepare the target
// Prepare the target directory
Directory target = Directory.createNew(importer.getUploadRoot());

FileSystemManager manager = null;
FileObject fo = null;
try {
manager = VFS.getManager();

if (username != null) {
StaticUserAuthenticator auth =
new StaticUserAuthenticator(domain, username, password);
FileSystemOptions opts = new FileSystemOptions();
DefaultFileSystemConfigBuilder.getInstance().setUserAuthenticator(opts, auth);
fo = manager.resolveFile(location, opts);
} else {
fo = manager.resolveFile(location);
}
// shared file system manager - do not close
FileSystemManager sharedManager = VFS.getManager();

// try-with-resources not used, as finally block responsible to close
// both the file object and the associated file system.
FileObject fileObject;

target.accept(fo);
if (username != null) {
StaticUserAuthenticator auth = new StaticUserAuthenticator(domain, username, password);
FileSystemOptions opts = new FileSystemOptions();
DefaultFileSystemConfigBuilder.getInstance().setUserAuthenticator(opts, auth);
fileObject = sharedManager.resolveFile(location, opts);
} else {
fileObject = sharedManager.resolveFile(location);
}

try {
target.accept(fileObject);
} finally {
if (fo != null) {
FileSystem fs = fo.getFileSystem();
fo.close();
manager.closeFileSystem(fs);
if (fileObject != null) {
FileSystem fs = fileObject.getFileSystem();
fileObject.close();

// careful that we do not close shared file system, only remote file system
sharedManager.closeFileSystem(fs);
}
}

Expand Down
138 changes: 77 additions & 61 deletions src/extension/importer/core/src/main/java/org/geoserver/importer/VFSWorker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,60 +75,75 @@ public static String getExtension(String name) {
return extension;
}

/** */
/**
* List contents of archiveFile according to provided filter.
*
* @param archiveFile
* @param filter
*/
@SuppressWarnings({"PMD.UseTryWithResources", "PMD.ForLoopCanBeForeach"})
public List<String> listFiles(final File archiveFile, final FilenameFilter filter) {
FileSystemManager fsManager;
try {
fsManager = VFS.getManager();
String absolutePath = resolveArchiveURI(archiveFile);
FileObject resolvedFile = fsManager.resolveFile(absolutePath);

FileSelector fileSelector =
new FileSelector() {
/**
* @see
* org.apache.commons.vfs2.FileSelector#traverseDescendents(org.apache.commons.vfs2.FileSelectInfo)
*/
@Override
public boolean traverseDescendents(FileSelectInfo folderInfo)
throws Exception {
return true;
}

/**
* @see
* org.apache.commons.vfs2.FileSelector#includeFile(org.apache.commons.vfs2.FileSelectInfo)
*/
@Override
public boolean includeFile(FileSelectInfo fileInfo) throws Exception {
File folder = archiveFile.getParentFile();
String name = fileInfo.getFile().getName().getFriendlyURI();
return filter.accept(folder, name);
try (FileObject resolvedFile = fsManager.resolveFile(absolutePath)) {

FileSelector fileSelector =
new FileSelector() {
/**
* @see
* org.apache.commons.vfs2.FileSelector#traverseDescendents(org.apache.commons.vfs2.FileSelectInfo)
*/
@Override
public boolean traverseDescendents(FileSelectInfo folderInfo)
throws Exception {
return true;
}

/**
* @see
* org.apache.commons.vfs2.FileSelector#includeFile(org.apache.commons.vfs2.FileSelectInfo)
*/
@Override
public boolean includeFile(FileSelectInfo fileInfo) throws Exception {
File folder = archiveFile.getParentFile();
String name = fileInfo.getFile().getName().getFriendlyURI();
return filter.accept(folder, name);
}
};

FileObject fileSystem = null;
try {
if (fsManager.canCreateFileSystem(resolvedFile)) {
fileSystem = fsManager.createFileSystem(resolvedFile);
} else {
fileSystem = resolvedFile;
}
LOGGER.fine("Listing spatial data files archived in " + archiveFile.getName());
FileObject[] containedFiles = fileSystem.findFiles(fileSelector);
List<String> names = new ArrayList<>(containedFiles.length);
for (int i = 0; i < containedFiles.length; i++) {
try (FileObject childFileObject = containedFiles[i]) {
// path relative to its filesystem (ie, to the archive file)
String pathDecoded = childFileObject.getName().getPathDecoded();
names.add(pathDecoded);
}
};

FileObject fileSystem;
if (fsManager.canCreateFileSystem(resolvedFile)) {
fileSystem = fsManager.createFileSystem(resolvedFile);
} else {
fileSystem = resolvedFile;
}
LOGGER.fine("Listing spatial data files archived in " + archiveFile.getName());
FileObject[] containedFiles = fileSystem.findFiles(fileSelector);
List<String> names = new ArrayList<>(containedFiles.length);
for (FileObject fo : containedFiles) {
// path relative to its filesystem (ie, to the archive file)
String pathDecoded = fo.getName().getPathDecoded();
names.add(pathDecoded);
}
LOGGER.fine(
"Found "
+ names.size()
+ " spatial data files in "
+ archiveFile.getName()
+ ": "
+ names);
return names;
} finally {
if (fileSystem != null) {
fileSystem.close();
}
}
}
LOGGER.fine(
"Found "
+ names.size()
+ " spatial data files in "
+ archiveFile.getName()
+ ": "
+ names);
return names;
} catch (FileSystemException e) {
LOGGER.log(Level.SEVERE, "", e);
}
Expand Down Expand Up @@ -174,20 +189,12 @@ private String getaArchiveURLProtocol(final File file) {
* Extracts the archive file {@code archiveFile} to {@code targetFolder}; both shall previously
* exist.
*/
@SuppressWarnings("PMD.CloseResource")
public void extractTo(File archiveFile, File targetFolder) throws IOException {

FileSystemManager manager = VFS.getManager();
String sourceURI = resolveArchiveURI(archiveFile);
// String targetURI = resolveArchiveURI(targetFolder);
FileObject source = manager.resolveFile(sourceURI);
if (manager.canCreateFileSystem(source)) {
source = manager.createFileSystem(source);
}
FileObject target =
manager.createVirtualFileSystem(
manager.resolveFile(targetFolder.getAbsolutePath()));

FileSelector selector =
FileSelector allFiles =
new AllFileSelector() {
@Override
public boolean includeFile(FileSelectInfo fileInfo) {
Expand All @@ -196,10 +203,19 @@ public boolean includeFile(FileSelectInfo fileInfo) {
return true;
}
};
target.copyFrom(source, selector);
source.close();
target.close();
manager.closeFileSystem(source.getFileSystem());

try (FileObject source = manager.resolveFile(sourceURI);
FileObject target = manager.resolveFile(targetFolder.getAbsolutePath());
FileObject targetFileSystem = manager.createVirtualFileSystem(target)) {
if (manager.canCreateFileSystem(source)) {
try (FileObject sourceFileSystem = manager.createFileSystem(source)) {
targetFileSystem.copyFrom(sourceFileSystem, allFiles);
manager.closeFileSystem(sourceFileSystem.getFileSystem());
}
} else {
target.copyFrom(source, allFiles);
}
}
}

public Collection<File> listFilesInFolder(
Expand Down

0 comments on commit c5fd49d

Please sign in to comment.