Fixing bad behavior in IOUtil.deletePaths (#1416)

* The method IOUtil.deletePaths(Iterable<Path>) had pathological.
  * A Path is an Iterable<Path> which iterates over the subpaths in the Path.
     Therefore, this version was binding instead of the varargs version that takes Path...
     Then the method would try to delete all of the subpaths in the path.
  * This is obviously bad behavior.  Fixed by adding an explicit check to see if it's called with a single Path.
  * Extracted a new method deletePath which is used for deleting a single path.

* Fixes ##1414

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);
     }
 
     /**

src/main/java/htsjdk/samtools/util/IOUtil.java

@@ -303,16 +303,33 @@ public static void deleteFiles(final Iterable<File> 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<Path>.  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<Path> 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<Path> 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);
         }
     }
 

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<String> fileNames) throws Exception {
         final File tmpDir = IOUtil.createTempDir("testDeletePath", "");
         final List<Path> 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<String> fileNames) throws Exception {
         final List<Path> paths = createJimfsFiles("testDeletePath", fileNames);
-        testDeletePath(paths);
+        testDeletePaths(paths);
     }
 
     @Test(dataProvider = "fileNamesForDelete")
@@ -340,7 +352,8 @@ public void testDeleteArrayPathJims(final List<String> fileNames) throws Excepti
         testDeletePathArray(paths);
     }
 
-    private static void testDeletePath(final List<Path> paths) {
+
+    private static void testDeletePaths(final List<Path> paths) {
         paths.forEach(p -> Assert.assertTrue(Files.exists(p)));
         IOUtil.deletePaths(paths);
         paths.forEach(p -> Assert.assertFalse(Files.exists(p)));