Small patches to custom InflaterFactory support left over from code review

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

@@ -50,7 +50,14 @@
     private boolean checkCrcs = false;
 
     /**
-     * Create BlockGunziper using the provided inflaterFactory
+     * Create a BlockGunzipper using the default inflaterFactory
+     */
+    BlockGunzipper() {
+        inflater = defaultInflaterFactory.makeInflater(true); // GZIP mode
+    }
+
+    /**
+     * Create a BlockGunzipper using the provided inflaterFactory
      * @param inflaterFactory
      */
     BlockGunzipper(InflaterFactory inflaterFactory) {

src/test/java/htsjdk/samtools/SamReaderFactoryTest.java

@@ -67,7 +67,6 @@ public Inflater makeInflater(final boolean gzipCompatible) {
         final File input = new File(TEST_DATA_DIR, inputFile);
         try (final SamReader reader = SamReaderFactory.makeDefault().inflaterFactory(myInflaterFactory).open(input)) {
             for (final SAMRecord ignored : reader) { }
-            reader.close();
         }
         Assert.assertNotEquals(inflateCalls[0], 0, "Not using Inflater from InflateFactory on file : " + inputFile);
     }

src/test/java/htsjdk/samtools/util/BlockCompressedInputStreamTest.java

@@ -93,80 +93,85 @@ public void available_should_return_number_of_bytes_left_in_current_block() thro
 		}
 	}
 
-    private static class MyInflater extends Inflater {
-        MyInflater(boolean gzipCompatible) {
+    private static class CountingInflater extends Inflater {
+        // Must be static unfortunately, since there's no way to reach down into an inflater instance given a stream
+        static int inflateCalls = 0;
+
+        CountingInflater(boolean gzipCompatible) {
             super(gzipCompatible);
         }
         @Override
         public int inflate(byte[] b, int off, int len) throws java.util.zip.DataFormatException {
             inflateCalls++;
             return super.inflate(b, off, len);
         }
-        static int inflateCalls;
     }
 
-    @DataProvider(name = "customInflaterInput")
-    public Object[][] customInflateInput() throws IOException {
-        final File tempFile = File.createTempFile("testCustomInflater.", ".bam");
-        tempFile.deleteOnExit();
-        System.out.println("Creating file " + tempFile);
+    private static class CountingInflaterFactory extends InflaterFactory {
+        @Override
+        public Inflater makeInflater( boolean gzipCompatible ) {
+            return new CountingInflater(gzipCompatible);
+        }
+    }
+
+    @FunctionalInterface
+    private interface CheckedExceptionInputStreamSupplier {
+        InputStream get() throws IOException;
+    }
 
+    private List<String> writeTempBlockCompressedFileForInflaterTest( final File tempFile ) throws IOException {
         final List<String> linesWritten = new ArrayList<>();
-        final BlockCompressedOutputStream bcos = new BlockCompressedOutputStream(tempFile, 5);
-        String s = "Hi, Mom!\n";
-        bcos.write(s.getBytes()); //Call 1
-        linesWritten.add(s);
-        s = "Hi, Dad!\n";
-        bcos.write(s.getBytes()); //Call 2
-        linesWritten.add(s);
-        bcos.flush();
-        final StringBuilder sb = new StringBuilder(BlockCompressedStreamConstants.DEFAULT_UNCOMPRESSED_BLOCK_SIZE * 2);
-        s = "1234567890123456789012345678901234567890123456789012345678901234567890\n";
-        while (sb.length() <= BlockCompressedStreamConstants.DEFAULT_UNCOMPRESSED_BLOCK_SIZE) {
-            sb.append(s);
+        try ( final BlockCompressedOutputStream bcos = new BlockCompressedOutputStream(tempFile, 5) ) {
+            String s = "Hi, Mom!\n";
+            bcos.write(s.getBytes()); //Call 1
             linesWritten.add(s);
-        }
-        bcos.write(sb.toString().getBytes()); //Call 3
-        bcos.close();
-
-        final InflaterFactory myInflaterFactory = new InflaterFactory() {
-            public Inflater makeInflater(final boolean gzipCompatible) {
-                return new MyInflater(gzipCompatible);
+            s = "Hi, Dad!\n";
+            bcos.write(s.getBytes()); //Call 2
+            linesWritten.add(s);
+            bcos.flush();
+            final StringBuilder sb = new StringBuilder(BlockCompressedStreamConstants.DEFAULT_UNCOMPRESSED_BLOCK_SIZE * 2);
+            s = "1234567890123456789012345678901234567890123456789012345678901234567890\n";
+            while ( sb.length() <= BlockCompressedStreamConstants.DEFAULT_UNCOMPRESSED_BLOCK_SIZE ) {
+                sb.append(s);
+                linesWritten.add(s);
             }
-        };
-
-        // test catching null InflaterFactory
-        try {
-            BlockGunzipper.setDefaultInflaterFactory(null);
-            Assert.fail("Did not catch 'null' InflaterFactory");
+            bcos.write(sb.toString().getBytes()); //Call 3
         }
-        catch (java.lang.IllegalArgumentException e) { /* expected */ }
+        return linesWritten;
+    }
+
+    @DataProvider(name = "customInflaterInput")
+    public Object[][] customInflateInput() throws IOException {
+        final File tempFile = File.createTempFile("testCustomInflater.", ".bam");
+        tempFile.deleteOnExit();
+        final List<String> linesWritten = writeTempBlockCompressedFileForInflaterTest(tempFile);
 
-        BlockGunzipper.setDefaultInflaterFactory(myInflaterFactory);
+        final InflaterFactory countingInflaterFactory = new CountingInflaterFactory();
+        BlockGunzipper.setDefaultInflaterFactory(countingInflaterFactory);
 
         return new Object[][]{
                 // use default InflaterFactory
-                {new BlockCompressedInputStream(new FileInputStream(tempFile), false), linesWritten, 4},
-                {new BlockCompressedInputStream(tempFile), linesWritten, 4},
-                {new AsyncBlockCompressedInputStream(tempFile), linesWritten, 4},
-                {new BlockCompressedInputStream(new URL("http://broadinstitute.github.io/picard/testdata/index_test.bam")), null, 21},
+                {(CheckedExceptionInputStreamSupplier) () -> new BlockCompressedInputStream(new FileInputStream(tempFile), false), linesWritten, 4},
+                {(CheckedExceptionInputStreamSupplier) () -> new BlockCompressedInputStream(tempFile), linesWritten, 4},
+                {(CheckedExceptionInputStreamSupplier) () -> new AsyncBlockCompressedInputStream(tempFile), linesWritten, 4},
+                {(CheckedExceptionInputStreamSupplier) () -> new BlockCompressedInputStream(new URL("http://broadinstitute.github.io/picard/testdata/index_test.bam")), null, 21},
                 // provide InflaterFactory
-                {new BlockCompressedInputStream(new FileInputStream(tempFile), false, myInflaterFactory), linesWritten, 4},
-                {new BlockCompressedInputStream(tempFile, myInflaterFactory), linesWritten, 4},
-                {new AsyncBlockCompressedInputStream(tempFile, myInflaterFactory), linesWritten, 4},
-                {new BlockCompressedInputStream(new URL("http://broadinstitute.github.io/picard/testdata/index_test.bam"), myInflaterFactory), null, 21}
+                {(CheckedExceptionInputStreamSupplier) () -> new BlockCompressedInputStream(new FileInputStream(tempFile), false, countingInflaterFactory), linesWritten, 4},
+                {(CheckedExceptionInputStreamSupplier) () -> new BlockCompressedInputStream(tempFile, countingInflaterFactory), linesWritten, 4},
+                {(CheckedExceptionInputStreamSupplier) () -> new AsyncBlockCompressedInputStream(tempFile, countingInflaterFactory), linesWritten, 4},
+                {(CheckedExceptionInputStreamSupplier) () -> new BlockCompressedInputStream(new URL("http://broadinstitute.github.io/picard/testdata/index_test.bam"), countingInflaterFactory), null, 21}
         };
     }
 
-    @Test(dataProvider = "customInflaterInput")
-    public void testCustomInflater(final BlockCompressedInputStream bcis,
+    @Test(dataProvider = "customInflaterInput", singleThreaded = true)
+    public void testCustomInflater(final CheckedExceptionInputStreamSupplier bcisSupplier,
                                    final List<String> expectedOutput,
                                    final int expectedInflateCalls) throws Exception
     {
-        // clear inflate call counter in MyInflater
-        MyInflater.inflateCalls = 0;
+        // clear inflate call counter in CountingInflater
+        CountingInflater.inflateCalls = 0;
 
-        try (final BufferedReader reader = new BufferedReader(new InputStreamReader(bcis))) {
+        try (final BufferedReader reader = new BufferedReader(new InputStreamReader(bcisSupplier.get()))) {
             String line;
             for (int i = 0; (line = reader.readLine()) != null; ++i) {
                 // check expected output, if provided
@@ -175,9 +180,14 @@ public void testCustomInflater(final BlockCompressedInputStream bcis,
                 }
             }
         }
-        bcis.close();
 
         // verify custom inflater was used by checking number of inflate calls
-        Assert.assertEquals(MyInflater.inflateCalls, expectedInflateCalls, "inflate calls");
+        Assert.assertEquals(CountingInflater.inflateCalls, expectedInflateCalls, "inflate calls");
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void testSetNullInflaterFactory() {
+        // test catching null InflaterFactory
+        BlockGunzipper.setDefaultInflaterFactory(null);
     }
 }