Skip to content

Commit 90211f1

Browse files
VestkariannaCopilot
authored
Drop commons-io dependency in favor of JDK equivalents (#7562)
* Replace commons-io FilenameUtils with JDK equivalents Step 1 of removing the commons-io dependency. Adds private static helpers in BatchExporter for getExtension/removeExtension semantics (last-dot-in-final-segment), and uses java.io.File.getName() with a null guard in SpellsKnownTab. * Replace commons-io FileUtils.writeLines with Files.writeString Step 2 of removing commons-io. The behavior is preserved: UTF-8 encoding, System.lineSeparator() between lines, and a trailing separator after the last line — matching FileUtils.writeLines. * Add unit tests for PCGIOHandler.write party-file format Covers the structural contract of the party file: two lines, version prefix, comma-separated character paths, trailing line separator, UTF-8 encoding. Verified to pass against both the old FileUtils.writeLines implementation and the new Files.writeString one. * Refactor test method visibility in PCGIOHandlerWriteTest Signed-off-by: Vest <Vest@users.noreply.github.com> * Replace commons-io FileUtils.listFiles with Files.walk in DataTest Step 3 of removing commons-io. Adds a small private helper that mirrors FileUtils.listFiles(File, String[], boolean) semantics: recursive, regular files only, case-sensitive extension match. * Drop commons-io TeeOutputStream from BatchExporter PDF export The original code wrapped a ByteArrayOutputStream in a TeeOutputStream so the export simultaneously fed an in-memory buffer (consumed by FOP) and an optional debug temp file. Since the bytes are fully materialised in memory before FOP runs, the tee was unnecessary: write the temp file sequentially after the export completes instead. This removes the last commons-io dependency in this file and lets us delete the internal TeeOutputStream helper. * Report FOP errors from exportPartyToPDF exportCharacterToPDF inspects task.getErrorMessages() after running FOP and returns false (with a logged message) when FOP reports problems. exportPartyToPDF was missing the same check, so a party PDF that FOP failed to render would silently return true and leave the user thinking the export succeeded. Mirror the character path: log and return false when FOP produces a non-blank error message. * Rewrite removeTemporaryFiles to use listFiles for what it is The previous implementation called File.list(FilenameFilter) and used the filter lambda for its side effect — deleting matching files inside the predicate and always returning false so list() would return an empty array the caller then ignored. That misuse hid three real problems: - tf.delete() returned false silently; failures to delete were invisible. - The defensive try/catch around the filter caught exceptions a FilenameFilter is not allowed to throw — pure cargo cult. - File.list returns null when the directory does not exist; the accidental side-effect form happened to tolerate that, but only by coincidence. Use listFiles to actually list the temp files matching the prefix, guard against the null result, then iterate and delete with a logged warning when delete fails. Drop the manual File.separator concatenation in favour of SettingsHandler.getTempPath() (already a File). * Use StandardCharsets.UTF_8 consistently in BatchExporter exportCharacterToNonPDF already used StandardCharsets.UTF_8 but four sibling methods (exportPartyToNonPDF, both exportParty overloads, and the private exportCharacter overload) still passed the literal string "UTF-8" to OutputStreamWriter. The two forms behave identically but the string form forces callers to deal with UnsupportedEncodingException and lets a typo go undetected until runtime. Switch all four sites to the constant so the file is internally consistent and the encoding is checked at compile time. * Use Files.delete for diagnosable temp-file cleanup failures removeTemporaryFiles previously used File.delete, which returns a bare boolean on failure with no indication of why the OS refused the operation — permission denied, file locked by another process, filesystem read-only, path no longer present, etc. The log entry could only say "Could not delete X", which is unactionable. Switch to Files.delete(Path), which throws an IOException carrying the underlying cause (NoSuchFileException, AccessDeniedException, or a generic IOException with the platform message). Log the exception along with the path so the operator has something to act on. Sonar S4042 flags exactly this pattern. * Use unnamed pattern for ignored ExportException in catch block exportCharacterToNonPDF catches ExportException only to short-circuit to a false return; the comment "Error will already be reported to the log" makes the intent explicit. Naming the variable "e" suggested it might be used and forced a reader to scan the catch body to confirm it was not. Use the unnamed pattern (_) introduced in JEP 456 / Java 21 to declare the variable as deliberately unused. The compiler now enforces the intent and the discard is visible at the declaration site. Sonar S7467 flags exactly this case. * Convert Path to File directly in getXMLTemplate The original code converted the Path to a URI and then constructed a File from that URI. The intermediate URI step served no purpose: it introduced URI parsing and a second File constructor invocation while producing an equivalent File. Use Path.toFile() instead, which is the direct API for this conversion. * Guard generateOutputFilename against dotless filenames generateOutputFilename built the output name as charname.substring(0, charname.lastIndexOf('.')) + '.' + extension If charname has no dot, lastIndexOf returns -1 and substring(0, -1) throws StringIndexOutOfBoundsException. Today this cannot happen in practice because the only caller (exportCharacter / exportParty) gates on PCGFile.isPCGenCharacterFile / isPCGenPartyFile, which require .pcg or .pcp suffixes. But the guard is implicit and the crash is a latent landmine if validation ever changes upstream. Reuse removeFileExtension, which already handles the dotless case by returning the input unchanged. Behaviour is identical for every name that reaches this method today and safe for any name that might in the future. * Replace commons-io filters in ExportUtilities with JDK Predicate getValidFiles built its template filter as a chain of commons-io IOFileFilter wrappers: FileFilterUtils.asFileFilter(sheetFilter) + prefixFileFilter + and() + filterList. The SheetFilter enum already implements java.io.FilenameFilter, and the prefix check is a one-line String.startsWith — there is nothing here that needs the IOFileFilter abstraction. Combine the two checks into a single Predicate<File> lambda and run it through Stream.filter().sorted().collect(toList()) so the result matches the prior FileFilterUtils.filterList + Collections.sort semantics. This drops both commons-io.filefilter imports from the file and removes one more user of the dependency we are stripping. * Replace commons-io filters in PrintPreviewDialog with Files.walk SheetLoader located printable templates with FileUtils.listFiles plus a FileFilterUtils.and() chain of pdfFilter / suffixFilter / sheetFilter and a TrueFileFilter for directory descent. To do this it also implemented FilenameFilter purely so its accept() could be wrapped via FileFilterUtils.asFileFilter(this) — the class was serving as both worker and filter without need. Replace the listing with Files.walk(...) filtered by Files::isRegularFile and a single Predicate<File> that combines all three checks (parent dir name == "pdf", name does not end with .fo, name starts with the character template prefix). Drop the FilenameFilter implementation and the @NotNull import that came with it; the directory walk is recursive by default so no separate dir filter is needed. Failure mode is now explicit: an IOException from Files.walk is logged with the offending directory rather than swallowed inside commons-io. Removes the only remaining commons-io.* usage in this file. * Drop commons-io as a direct dependency With ExportUtilities, PrintPreviewDialog, BatchExporter and the rest of code/src/java migrated off org.apache.commons.io there are no remaining direct call sites — verified by grep across all source sets and by compiling main, test, itest and slowtest with the declarations removed. Drop the implementation declaration from build.gradle, the forceMerge entry under the jlink block, and the corresponding requires org.apache.commons.io from module-info.java. Commons-io may still be pulled transitively by FOP / batik on the runtime classpath, but our own module no longer reads it. * Refactor BatchExporter.removeTemporaryFiles to NIO and drop commons-io Rewrite the cleanup loop using java.nio.file: Files.list returns a Stream<Path>, so the temp-file scan, filter, and delete collapse into a try-with-resources block with two small helpers (isTemporaryExportFile, deleteQuietly). Behavioural improvements: - An unreadable temp directory is now logged, not silently treated as empty (the legacy listFiles returned null for both "not a directory" and "I/O error"). - A missing temp directory is handled explicitly via Files.isDirectory before listing, instead of relying on listFiles' implicit null. - Stream is closed deterministically by try-with-resources (matters on Windows file locking). This was the last in-tree usage of commons-io, so drop the dependency from build.gradle and the requires directive from module-info.java. * Fix XTSE0710 xsl:use-attribute-sets for equipment.title across all game modes Replace xsl:use-attribute-sets="equipment.title" with an equivalent xsl:call-template name="attrib" call in fantasy_common.xsl (fantasy, 5e, starfinder, pathfinder_2, 4e, sagaborn) and killshot_common.xsl. Saxon-HE rejects references to undeclared attribute sets at compile time (XTSE0710); the attrib template is the correct runtime skinning mechanism. Add XsltPdfTemplateCompilationTest to guard against future regressions: compiles each fixed file directly under Saxon and fails on any XTSE0710/XTSE0720 error, ignoring expected fragment-level errors. * Fix URIFactoryTest failing on Windows (#7527) * Create separate action * get the tag you created earlier * upload zip artifacts for all platforms * upload zip artifacts for all platforms * Fix URIFactoryTest failing on Windows Use File.toURI() instead of manually constructing a URI string with new URI("file:" + path). On Windows, System.getProperty("user.dir") returns backslash-separated paths (e.g. D:\a\pcgen\pcgen) which are illegal characters in a URI, causing URISyntaxException. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Signed-off-by: Vest <Vest@users.noreply.github.com> Co-authored-by: Martijn Verburg <martijnverburg@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6b487ef commit 90211f1

18 files changed

Lines changed: 413 additions & 120 deletions

File tree

build.gradle

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,6 @@ dependencies {
220220

221221
implementation fileTree(dir: 'mods/lib', include: ['javafx.*.jar'])
222222

223-
implementation 'commons-io:commons-io:2.22.0'
224223
implementation 'org.springframework:spring-web:7.0.7'
225224
implementation 'org.springframework:spring-beans:7.0.7'
226225
implementation 'org.springframework:spring-core:7.0.7'
@@ -313,7 +312,7 @@ jlink {
313312
options = ['--strip-debug', '--no-header-files', '--no-man-pages']
314313
addExtraDependencies('javafx')
315314
forceMerge('jep', 'fop', 'Saxon-HE',
316-
'commons-lang3', 'commons-io', 'spring',
315+
'commons-lang3', 'spring',
317316
'freemarker', 'jdom2', 'argparse4j', 'xmlunit', 'controlsfx',
318317
'annotations', 'spotbugs', 'xmlresolver')
319318

code/src/java/module-info.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
requires pcgen.formula;
2424

2525
requires org.apache.commons.lang3;
26-
requires org.apache.commons.io;
2726
requires freemarker;
2827
requires org.jdom2;
2928
requires net.sourceforge.argparse4j;

code/src/java/pcgen/gui2/dialog/PrintPreviewDialog.java

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,18 @@
3434
import java.beans.PropertyChangeEvent;
3535
import java.beans.PropertyChangeListener;
3636
import java.io.File;
37-
import java.io.FilenameFilter;
37+
import java.io.IOException;
3838
import java.io.PipedInputStream;
3939
import java.io.PipedOutputStream;
4040
import java.net.URI;
41+
import java.nio.file.Files;
42+
import java.nio.file.Path;
4143
import java.text.NumberFormat;
42-
import java.util.Collection;
44+
import java.util.List;
4345
import java.util.concurrent.ExecutionException;
46+
import java.util.function.Predicate;
47+
import java.util.stream.Collectors;
48+
import java.util.stream.Stream;
4449

4550
import javax.swing.BorderFactory;
4651
import javax.swing.Box;
@@ -69,15 +74,9 @@
6974
import pcgen.util.Logging;
7075
import pcgen.util.fop.FopTask;
7176

72-
import org.apache.commons.io.FileUtils;
73-
import org.apache.commons.io.filefilter.FileFilterUtils;
74-
import org.apache.commons.io.filefilter.IOFileFilter;
75-
import org.apache.commons.io.filefilter.SuffixFileFilter;
76-
import org.apache.commons.io.filefilter.TrueFileFilter;
7777
import org.apache.fop.apps.FOUserAgent;
7878
import org.apache.fop.render.awt.AWTRenderer;
7979
import org.apache.fop.render.awt.viewer.PreviewPanel;
80-
import org.jetbrains.annotations.NotNull;
8180

8281
/**
8382
* Dialog to allow the preview of character export.
@@ -421,28 +420,30 @@ private static ComboBoxModel<String> createPagesModel(int pages)
421420
return new DefaultComboBoxModel<>(pageNumbers);
422421
}
423422

424-
private class SheetLoader extends SwingWorker<Object[], Object> implements FilenameFilter
423+
private class SheetLoader extends SwingWorker<Object[], Object>
425424
{
426425

427-
@Override
428-
public boolean accept(@NotNull File dir, @NotNull String name)
429-
{
430-
return dir.getName().equalsIgnoreCase("pdf");
431-
}
432-
433426
@Override
434427
protected Object[] doInBackground()
435428
{
436-
IOFileFilter pdfFilter = FileFilterUtils.asFileFilter(this);
437-
IOFileFilter suffixFilter = FileFilterUtils.notFileFilter(new SuffixFileFilter(".fo"));
438-
IOFileFilter sheetFilter = FileFilterUtils.prefixFileFilter(Constants.CHARACTER_TEMPLATE_PREFIX);
439-
IOFileFilter fileFilter = FileFilterUtils.and(pdfFilter, suffixFilter, sheetFilter);
429+
Predicate<File> filter = f -> f.getParentFile().getName().equalsIgnoreCase("pdf")
430+
&& !f.getName().endsWith(".fo")
431+
&& f.getName().startsWith(Constants.CHARACTER_TEMPLATE_PREFIX);
440432

441-
IOFileFilter dirFilter = TrueFileFilter.INSTANCE;
442433
File dir = new File(ConfigurationSettings.getOutputSheetsDir());
443-
Collection<File> files = FileUtils.listFiles(dir, fileFilter, dirFilter);
444-
URI osPath = new File(ConfigurationSettings.getOutputSheetsDir()).toURI();
445-
return files.stream().map(v -> osPath.relativize(v.toURI())).toArray();
434+
URI osPath = dir.toURI();
435+
try (Stream<Path> walk = Files.walk(dir.toPath()))
436+
{
437+
List<File> files = walk.filter(Files::isRegularFile)
438+
.map(Path::toFile)
439+
.filter(filter)
440+
.collect(Collectors.toList());
441+
return files.stream().map(v -> osPath.relativize(v.toURI())).toArray();
442+
} catch (IOException ex)
443+
{
444+
Logging.errorPrint("could not walk output sheets directory " + dir, ex);
445+
return new Object[0];
446+
}
446447
}
447448

448449
@Override

code/src/java/pcgen/gui2/tabs/spells/SpellsKnownTab.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import javax.swing.JScrollPane;
3232
import javax.swing.JTextField;
3333

34-
import org.apache.commons.io.FilenameUtils;
3534
import pcgen.core.PCClass;
3635
import pcgen.facade.core.CharacterFacade;
3736
import pcgen.facade.core.SpellFacade;
@@ -154,7 +153,8 @@ public SortableTableModel getModel()
154153
hbox.add(spellSheetButton);
155154
hbox.add(Box.createHorizontalStrut(3));
156155

157-
String text = FilenameUtils.getName(PCGenSettings.getSelectedSpellSheet());
156+
String spellSheetPath = PCGenSettings.getSelectedSpellSheet();
157+
String text = spellSheetPath == null ? null : new File(spellSheetPath).getName();
158158
spellSheetField.setEditable(false);
159159
spellSheetField.setText(text);
160160
spellSheetField.setToolTipText(text);

code/src/java/pcgen/io/ExportUtilities.java

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.Collection;
3030
import java.util.Collections;
3131
import java.util.List;
32+
import java.util.function.Predicate;
3233
import java.util.stream.Collectors;
3334

3435
import freemarker.template.Configuration;
@@ -43,8 +44,6 @@
4344
import freemarker.template.ObjectWrapper;
4445
import freemarker.template.Version;
4546
import javafx.stage.FileChooser;
46-
import org.apache.commons.io.filefilter.FileFilterUtils;
47-
import org.apache.commons.io.filefilter.IOFileFilter;
4847
import org.apache.commons.lang3.StringUtils;
4948
import org.apache.commons.lang3.SystemUtils;
5049

@@ -140,10 +139,8 @@ public static File templateToAbsoluteTemplate(String sheetFilterPath, URI relati
140139

141140
public static URI[] getValidFiles(Collection<File> myAllTemplates, SheetFilter sheetFilter, boolean doPartyExport)
142141
{
143-
IOFileFilter prefixFilter;
144142
String outputSheetsDir;
145143
String outputSheetDirectory = SettingsHandler.getGameAsProperty().get().getOutputSheetDirectory();
146-
IOFileFilter ioFilter = FileFilterUtils.asFileFilter(sheetFilter);
147144
if (outputSheetDirectory == null)
148145
{
149146
outputSheetsDir = ConfigurationSettings.getOutputSheetsDir() + '/' + sheetFilter.getPath();
@@ -154,17 +151,14 @@ public static URI[] getValidFiles(Collection<File> myAllTemplates, SheetFilter s
154151
+ sheetFilter.getPath();
155152
}
156153

157-
if (doPartyExport)
158-
{
159-
prefixFilter = FileFilterUtils.prefixFileFilter(Constants.PARTY_TEMPLATE_PREFIX);
160-
} else
161-
{
162-
prefixFilter = FileFilterUtils.prefixFileFilter(Constants.CHARACTER_TEMPLATE_PREFIX);
163-
}
164-
IOFileFilter filter = FileFilterUtils.and(prefixFilter, ioFilter);
154+
String prefix = doPartyExport ? Constants.PARTY_TEMPLATE_PREFIX : Constants.CHARACTER_TEMPLATE_PREFIX;
155+
Predicate<File> filter = f -> f.getName().startsWith(prefix)
156+
&& sheetFilter.accept(f.getParentFile(), f.getName());
165157

166-
List<File> files = FileFilterUtils.filterList(filter, myAllTemplates);
167-
Collections.sort(files);
158+
List<File> files = myAllTemplates.stream()
159+
.filter(filter)
160+
.sorted()
161+
.collect(Collectors.toList());
168162
URI osPath = new File(outputSheetsDir).toURI();
169163
URI[] uriList = new URI[files.size()];
170164
Arrays.setAll(uriList, i -> osPath.relativize(files.get(i).toURI()));

code/src/java/pcgen/io/PCGIOHandler.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.io.Writer;
3535
import java.math.BigDecimal;
3636
import java.nio.charset.StandardCharsets;
37+
import java.nio.file.Files;
3738
import java.util.ArrayList;
3839
import java.util.Arrays;
3940
import java.util.HashMap;
@@ -60,7 +61,6 @@
6061
import pcgen.util.FileHelper;
6162
import pcgen.util.Logging;
6263

63-
import org.apache.commons.io.FileUtils;
6464
import org.apache.commons.lang3.StringUtils;
6565
import org.jetbrains.annotations.Nullable;
6666

@@ -520,7 +520,8 @@ public static void write(File partyFile, List<File> characterFiles)
520520
String filesLine = StringUtils.join(files, ',');
521521
try
522522
{
523-
FileUtils.writeLines(partyFile, "UTF-8", Arrays.asList(versionLine, filesLine));
523+
String eol = System.lineSeparator();
524+
Files.writeString(partyFile.toPath(), versionLine + eol + filesLine + eol, StandardCharsets.UTF_8);
524525
} catch (IOException ex)
525526
{
526527
Logging.errorPrint("Could not save the party file: " + partyFile.getAbsolutePath(), ex);

0 commit comments

Comments
 (0)