diff --git a/src/main/java/htsjdk/samtools/util/DiskBackedQueue.java b/src/main/java/htsjdk/samtools/util/DiskBackedQueue.java index a3c7ff2801..b353fb132f 100644 --- a/src/main/java/htsjdk/samtools/util/DiskBackedQueue.java +++ b/src/main/java/htsjdk/samtools/util/DiskBackedQueue.java @@ -339,7 +339,7 @@ private E readFileRecord (final Path file) { private void closeIOResources() { CloserUtil.close(this.outputStream); CloserUtil.close(this.inputStream); - if (this.diskRecords != null) IOUtil.deletePaths(this.diskRecords); + if (this.diskRecords != null) IOUtil.deletePath(this.diskRecords); } /** diff --git a/src/main/java/htsjdk/samtools/util/IOUtil.java b/src/main/java/htsjdk/samtools/util/IOUtil.java index d32b7f9d40..f98c2a2b2c 100755 --- a/src/main/java/htsjdk/samtools/util/IOUtil.java +++ b/src/main/java/htsjdk/samtools/util/IOUtil.java @@ -303,16 +303,33 @@ public static void deleteFiles(final Iterable files) { } public static void deletePaths(final Path... paths) { - deletePaths(Arrays.asList(paths)); + for(Path path: paths){ + deletePath(path); + } } + /** + * Iterate through Paths and delete each one. + * Note: Path is itself an Iterable. This method special cases that and deletes the single Path rather than + * Iterating the Path for targets to delete. + * @param paths an iterable of Paths to delete + */ public static void deletePaths(final Iterable paths) { - for (final Path p : paths) { - try { - Files.delete(p); - } catch (IOException e) { - System.err.println("Could not delete file " + p); - } + //Path is itself an Iterable which causes very confusing behavior if we don't explicitly check here. + if( paths instanceof Path){ + deletePath((Path)paths); + } + paths.forEach(IOUtil::deletePath); + } + + /** + * Attempt to delete a single path and log an error if it is not deleted. + */ + public static void deletePath(Path path){ + try { + Files.delete(path); + } catch (IOException e) { + System.err.println("Could not delete file " + path); } } diff --git a/src/test/java/htsjdk/samtools/util/IOUtilTest.java b/src/test/java/htsjdk/samtools/util/IOUtilTest.java index 8bdfd0cca9..9b3b60ff34 100644 --- a/src/test/java/htsjdk/samtools/util/IOUtilTest.java +++ b/src/test/java/htsjdk/samtools/util/IOUtilTest.java @@ -35,7 +35,6 @@ import java.nio.file.Paths; import java.nio.file.spi.FileSystemProvider; -import htsjdk.samtools.BamFileIoUtils; import htsjdk.samtools.SAMException; import org.testng.Assert; import org.testng.annotations.AfterClass; @@ -44,15 +43,12 @@ import org.testng.annotations.Test; import java.lang.IllegalArgumentException; -import java.nio.file.Path; -import java.nio.file.Files; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.stream.Collectors; import java.util.Random; -import java.util.stream.Stream; import java.util.zip.GZIPOutputStream; @@ -318,13 +314,29 @@ public void testGetDefaultTmpDirPath() throws Exception { public void testDeletePathLocal(final List fileNames) throws Exception { final File tmpDir = IOUtil.createTempDir("testDeletePath", ""); final List paths = createLocalFiles(tmpDir, fileNames); - testDeletePath(paths); + testDeletePaths(paths); + } + + @Test + public void testDeleteSinglePath() throws Exception { + final Path toDelete = Files.createTempFile("file",".bad"); + Assert.assertTrue(Files.exists(toDelete)); + IOUtil.deletePath(toDelete); + Assert.assertFalse(Files.exists(toDelete)); + } + + @Test + public void testDeleteSingleWithDeletePaths() throws Exception { + final Path toDelete = Files.createTempFile("file",".bad"); + Assert.assertTrue(Files.exists(toDelete)); + IOUtil.deletePaths(toDelete); + Assert.assertFalse(Files.exists(toDelete)); } @Test(dataProvider = "fileNamesForDelete") public void testDeletePathJims(final List fileNames) throws Exception { final List paths = createJimfsFiles("testDeletePath", fileNames); - testDeletePath(paths); + testDeletePaths(paths); } @Test(dataProvider = "fileNamesForDelete") @@ -340,7 +352,8 @@ public void testDeleteArrayPathJims(final List fileNames) throws Excepti testDeletePathArray(paths); } - private static void testDeletePath(final List paths) { + + private static void testDeletePaths(final List paths) { paths.forEach(p -> Assert.assertTrue(Files.exists(p))); IOUtil.deletePaths(paths); paths.forEach(p -> Assert.assertFalse(Files.exists(p)));