This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-compress.git
The following commit(s) were added to refs/heads/master by this push: new 62c773cd Use try-with-resources 62c773cd is described below commit 62c773cdaec41092fb3988e007850f2221233832 Author: Gary Gregory <garydgreg...@gmail.com> AuthorDate: Wed Nov 1 08:28:24 2023 -0400 Use try-with-resources - Better error messages - Better local variable names - Inline single use local variable --- .../apache/commons/compress/AbstractTestCase.java | 203 +++++++++------------ 1 file changed, 83 insertions(+), 120 deletions(-) diff --git a/src/test/java/org/apache/commons/compress/AbstractTestCase.java b/src/test/java/org/apache/commons/compress/AbstractTestCase.java index be5055ac..06c1f155 100644 --- a/src/test/java/org/apache/commons/compress/AbstractTestCase.java +++ b/src/test/java/org/apache/commons/compress/AbstractTestCase.java @@ -28,10 +28,11 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -40,14 +41,16 @@ import org.apache.commons.compress.archivers.ArchiveEntry; import org.apache.commons.compress.archivers.ArchiveInputStream; import org.apache.commons.compress.archivers.ArchiveOutputStream; import org.apache.commons.compress.archivers.ArchiveStreamFactory; +import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.SystemUtils; +import org.apache.commons.lang3.ThreadUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; public abstract class AbstractTestCase { protected interface StreamWrapper<I extends InputStream> { - I wrap(InputStream in) throws Exception; + I wrap(InputStream inputStream) throws Exception; } public static File getFile(final String path) throws IOException { @@ -56,9 +59,8 @@ public abstract class AbstractTestCase { throw new FileNotFoundException("couldn't find " + path); } try { - final URI uri = url.toURI(); - return new File(uri); - } catch (final java.net.URISyntaxException ex) { + return new File(url.toURI()); + } catch (final URISyntaxException ex) { throw new IOException(ex); } } @@ -67,31 +69,31 @@ public abstract class AbstractTestCase { return getFile(path).toPath(); } - public static File mkdir(final String name) throws IOException { - return Files.createTempDirectory(name).toFile(); + public static File mkdir(final String prefix) throws IOException { + return Files.createTempDirectory(prefix).toFile(); } public static InputStream newInputStream(final String path) throws IOException { return Files.newInputStream(getPath(path)); } - public static void rmdir(final File f) { - final String[] s = f.list(); - if (s != null) { - for (final String element : s) { - final File file = new File(f, element); + public static void rmdir(final File directory) { + final String[] fileList = directory.list(); + if (fileList != null) { + for (final String element : fileList) { + final File file = new File(directory, element); if (file.isDirectory()) { rmdir(file); } - final boolean ok = tryHardToDelete(file); - if (!ok && file.exists()) { - System.out.println("Failed to delete " + element + " in " + f.getPath()); + final boolean deleted = tryHardToDelete(file); + if (!deleted && file.exists()) { + System.err.println("Failed to delete " + element + " in " + directory.getPath()); } } } - tryHardToDelete(f); // safer to delete and check - if (f.exists()) { - fail("Failed to delete " + f.getPath()); + tryHardToDelete(directory); // safer to delete and check + if (directory.exists()) { + fail("Failed to delete " + directory.getPath()); } } @@ -104,17 +106,13 @@ public abstract class AbstractTestCase { * * @return whether deletion was successful */ - public static boolean tryHardToDelete(final File f) { - if (f != null && f.exists() && !f.delete()) { + public static boolean tryHardToDelete(final File file) { + if (file != null && file.exists() && !file.delete()) { if (SystemUtils.IS_OS_WINDOWS) { System.gc(); } - try { - Thread.sleep(10); - } catch (final InterruptedException ex) { - // Ignore Exception - } - return f.delete(); + ThreadUtils.sleepQuietly(Duration.ofSeconds(10)); + return file.delete(); } return true; } @@ -128,8 +126,8 @@ public abstract class AbstractTestCase { * * @return whether deletion was successful */ - public static boolean tryHardToDelete(final Path f) { - return tryHardToDelete(f != null ? f.toFile() : null); + public static boolean tryHardToDelete(final Path path) { + return tryHardToDelete(path != null ? path.toFile() : null); } protected File dir; @@ -145,71 +143,71 @@ public abstract class AbstractTestCase { /** * Add an entry to the archive, and keep track of the names in archiveList. * - * @param out + * @param outputStream * @param fileName - * @param infile + * @param inputFile * @throws IOException * @throws FileNotFoundException */ - private void addArchiveEntry(final ArchiveOutputStream out, final String fileName, final File infile) + private void addArchiveEntry(final ArchiveOutputStream outputStream, final String fileName, final File inputFile) throws IOException, FileNotFoundException { - final ArchiveEntry entry = out.createArchiveEntry(infile, fileName); - out.putArchiveEntry(entry); - Files.copy(infile.toPath(), out); - out.closeArchiveEntry(); + final ArchiveEntry entry = outputStream.createArchiveEntry(inputFile, fileName); + outputStream.putArchiveEntry(entry); + Files.copy(inputFile.toPath(), outputStream); + outputStream.closeArchiveEntry(); archiveList.add(fileName); } /** * Checks that an archive input stream can be read, and that the file data matches file sizes. * - * @param in + * @param inputStream * @param expected list of expected entries or {@code null} if no check of names desired * @throws Exception */ - protected void checkArchiveContent(final ArchiveInputStream in, final List<String> expected) + protected void checkArchiveContent(final ArchiveInputStream inputStream, final List<String> expected) throws Exception { - checkArchiveContent(in, expected, true); + checkArchiveContent(inputStream, expected, true); } /** * Checks that an archive input stream can be read, and that the file data matches file sizes. * - * @param in + * @param inputStream * @param expected list of expected entries or {@code null} if no check of names desired * @param cleanUp Cleans up resources if true * @return returns the created result file if cleanUp = false, or null otherwise * @throws Exception */ - protected File checkArchiveContent(final ArchiveInputStream in, final List<String> expected, final boolean cleanUp) + protected File checkArchiveContent(final ArchiveInputStream inputStream, final List<String> expected, final boolean cleanUp) throws Exception { final File result = mkdir("dir-result"); result.deleteOnExit(); try { ArchiveEntry entry; - while ((entry = in.getNextEntry()) != null) { - final File outfile = new File(result.getCanonicalPath() + "/result/" + entry.getName()); - long copied = 0; + while ((entry = inputStream.getNextEntry()) != null) { + final File outputFile = new File(result.getCanonicalPath() + "/result/" + entry.getName()); + long bytesCopied = 0; if (entry.isDirectory()) { - outfile.mkdirs(); + outputFile.mkdirs(); } else { - outfile.getParentFile().mkdirs(); - copied = Files.copy(in, outfile.toPath()); + outputFile.getParentFile().mkdirs(); + bytesCopied = Files.copy(inputStream, outputFile.toPath()); } final long size = entry.getSize(); if (size != ArchiveEntry.SIZE_UNKNOWN) { - assertEquals(size, copied, "Entry.size should equal bytes read."); + assertEquals(size, bytesCopied, "Entry.size should equal bytes read."); } - if (!outfile.exists()) { - fail("extraction failed: " + entry.getName()); + if (!outputFile.exists()) { + fail("Extraction failed: " + entry.getName()); } if (expected != null && !expected.remove(getExpectedString(entry))) { - fail("unexpected entry: " + getExpectedString(entry)); + fail("Unexpected entry: " + getExpectedString(entry)); } } - in.close(); + inputStream.close(); if (expected != null && !expected.isEmpty()) { fail(expected.size() + " missing entries: " + Arrays.toString(expected.toArray())); } @@ -235,20 +233,14 @@ public abstract class AbstractTestCase { */ protected void checkArchiveContent(final File archive, final List<String> expected) throws Exception { - try (InputStream is = Files.newInputStream(archive.toPath()); - final ArchiveInputStream in = factory.createArchiveInputStream(new BufferedInputStream(is))) { - this.checkArchiveContent(in, expected); + try (InputStream inputStream = Files.newInputStream(archive.toPath()); + ArchiveInputStream archiveInputStream = factory.createArchiveInputStream(new BufferedInputStream(inputStream))) { + checkArchiveContent(archiveInputStream, expected); } } - protected void closeQuietly(final Closeable closeable){ - if (closeable != null) { - try { - closeable.close(); - } catch (final IOException ignored) { - // ignored - } - } + protected void closeQuietly(final Closeable closeable) { + IOUtils.closeQuietly(closeable); } /** @@ -276,16 +268,11 @@ public abstract class AbstractTestCase { * in case something goes wrong */ protected Path createArchive(final String archiveName) throws Exception { - ArchiveOutputStream out = null; - OutputStream stream = null; - try { - archive = Files.createTempFile("test", "." + archiveName); - archive.toFile().deleteOnExit(); - archiveList = new ArrayList<>(); - - stream = Files.newOutputStream(archive); - out = factory.createArchiveOutputStream(archiveName, stream); - + archive = Files.createTempFile("test", "." + archiveName); + archive.toFile().deleteOnExit(); + archiveList = new ArrayList<>(); + try (OutputStream outputStream = Files.newOutputStream(archive); + ArchiveOutputStream archiveOutputStream = factory.createArchiveOutputStream(archiveName, outputStream);) { final File file1 = getFile("test1.xml"); final File file2 = getFile("test2.xml"); final File file3 = getFile("test3.xml"); @@ -293,24 +280,18 @@ public abstract class AbstractTestCase { final File file5 = getFile("test.txt"); final File file6 = getFile("test with spaces.txt"); - addArchiveEntry(out, "testdata/test1.xml", file1); - addArchiveEntry(out, "testdata/test2.xml", file2); - addArchiveEntry(out, "test/test3.xml", file3); - addArchiveEntry(out, "bla/test4.xml", file4); - addArchiveEntry(out, "bla/test5.xml", file4); - addArchiveEntry(out, "bla/blubber/test6.xml", file4); - addArchiveEntry(out, "test.txt", file5); - addArchiveEntry(out, "something/bla", file6); - addArchiveEntry(out, "test with spaces.txt", file6); - - out.finish(); + addArchiveEntry(archiveOutputStream, "testdata/test1.xml", file1); + addArchiveEntry(archiveOutputStream, "testdata/test2.xml", file2); + addArchiveEntry(archiveOutputStream, "test/test3.xml", file3); + addArchiveEntry(archiveOutputStream, "bla/test4.xml", file4); + addArchiveEntry(archiveOutputStream, "bla/test5.xml", file4); + addArchiveEntry(archiveOutputStream, "bla/blubber/test6.xml", file4); + addArchiveEntry(archiveOutputStream, "test.txt", file5); + addArchiveEntry(archiveOutputStream, "something/bla", file6); + addArchiveEntry(archiveOutputStream, "test with spaces.txt", file6); + + archiveOutputStream.finish(); return archive; - } finally { - if (out != null) { - out.close(); - } else if (stream != null) { - stream.close(); - } } } @@ -321,21 +302,12 @@ public abstract class AbstractTestCase { * @throws Exception */ protected Path createEmptyArchive(final String archiveName) throws Exception { - ArchiveOutputStream out = null; - OutputStream stream = null; archiveList = new ArrayList<>(); - try { - archive = Files.createTempFile("empty", "." + archiveName); - archive.toFile().deleteOnExit(); - stream = Files.newOutputStream(archive); - out = factory.createArchiveOutputStream(archiveName, stream); - out.finish(); - } finally { - if (out != null) { - out.close(); - } else if (stream != null) { - stream.close(); - } + archive = Files.createTempFile("empty", "." + archiveName); + archive.toFile().deleteOnExit(); + try (OutputStream outputStream = Files.newOutputStream(archive); + ArchiveOutputStream archiveOutputStream = factory.createArchiveOutputStream(archiveName, outputStream)) { + archiveOutputStream.finish(); } return archive; } @@ -348,23 +320,14 @@ public abstract class AbstractTestCase { * @throws Exception */ protected Path createSingleEntryArchive(final String archiveName) throws Exception { - ArchiveOutputStream out = null; - OutputStream stream = null; archiveList = new ArrayList<>(); - try { - archive = Files.createTempFile("empty", "." + archiveName); - archive.toFile().deleteOnExit(); - stream = Files.newOutputStream(archive); - out = factory.createArchiveOutputStream(archiveName, stream); + archive = Files.createTempFile("empty", "." + archiveName); + archive.toFile().deleteOnExit(); + try (OutputStream outputStream = Files.newOutputStream(archive); + ArchiveOutputStream archiveOutputStream = factory.createArchiveOutputStream(archiveName, outputStream)) { // Use short file name so does not cause problems for ar - addArchiveEntry(out, "test1.xml", getFile("test1.xml")); - out.finish(); - } finally { - if (out != null) { - out.close(); - } else if (stream != null) { - stream.close(); - } + addArchiveEntry(archiveOutputStream, "test1.xml", getFile("test1.xml")); + archiveOutputStream.finish(); } return archive; } @@ -384,8 +347,8 @@ public abstract class AbstractTestCase { final File tmpDir = createTempDir(); final File tmpFile = File.createTempFile("testfile", "", tmpDir); tmpFile.deleteOnExit(); - try (OutputStream fos = Files.newOutputStream(tmpFile.toPath())) { - fos.write(new byte[] { 'f', 'o', 'o' }); + try (OutputStream outputStream = Files.newOutputStream(tmpFile.toPath())) { + outputStream.write(new byte[] { 'f', 'o', 'o' }); return new File[] { tmpDir, tmpFile }; } }