From ee362e5becd75618924cada049c722c1288fdbb6 Mon Sep 17 00:00:00 2001 From: Ron Levine Date: Mon, 3 Jul 2017 16:11:20 -0400 Subject: [PATCH] Make stream seek error message informative --- .../samtools/util/BlockCompressedInputStream.java | 17 +++++++++++++++-- .../util/BlockCompressedOutputStreamTest.java | 20 +++++++++++++++----- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/main/java/htsjdk/samtools/util/BlockCompressedInputStream.java b/src/main/java/htsjdk/samtools/util/BlockCompressedInputStream.java index e108d1bb3..622ca67ac 100755 --- a/src/main/java/htsjdk/samtools/util/BlockCompressedInputStream.java +++ b/src/main/java/htsjdk/samtools/util/BlockCompressedInputStream.java @@ -57,10 +57,12 @@ public final static String INCORRECT_HEADER_SIZE_MSG = "Incorrect header size for file: "; public final static String UNEXPECTED_BLOCK_LENGTH_MSG = "Unexpected compressed block length: "; public final static String PREMATURE_END_MSG = "Premature end of file: "; - public final static String CANNOT_SEEK_STREAM_MSG = "Cannot seek on stream based file "; + public final static String CANNOT_SEEK_STREAM_MSG = "Cannot seek a position for a non-file stream"; + public final static String CANNOT_SEEK_CLOSED_STREAM_MSG = "Cannot seek a position for a closed stream"; public final static String INVALID_FILE_PTR_MSG = "Invalid file pointer: "; private InputStream mStream = null; + private boolean mIsClosed = false; private SeekableStream mFile = null; private byte[] mFileBuffer = null; private DecompressedBlock mCurrentBlock = null; @@ -222,6 +224,9 @@ public void close() throws IOException { // Encourage garbage collection mFileBuffer = null; mCurrentBlock = null; + + // Mark as closed + mIsClosed = true; } /** @@ -344,12 +349,20 @@ public int read(final byte[] buffer, int offset, int length) throws IOException * Seek to the given position in the file. Note that pos is a special virtual file pointer, * not an actual byte offset. * - * @param pos virtual file pointer + * @param pos virtual file pointer position + * @throws IOException if stream is closed or not a file based stream */ public void seek(final long pos) throws IOException { + // Must be before the mFile == null check because mFile == null for closed files and streams + if (mIsClosed) { + throw new IOException(CANNOT_SEEK_CLOSED_STREAM_MSG); + } + + // Cannot seek on streams that are not file based if (mFile == null) { throw new IOException(CANNOT_SEEK_STREAM_MSG); } + // Decode virtual file pointer // Upper 48 bits is the byte offset into the compressed stream of a // block. diff --git a/src/test/java/htsjdk/samtools/util/BlockCompressedOutputStreamTest.java b/src/test/java/htsjdk/samtools/util/BlockCompressedOutputStreamTest.java index a678c8dca..35175cd1d 100644 --- a/src/test/java/htsjdk/samtools/util/BlockCompressedOutputStreamTest.java +++ b/src/test/java/htsjdk/samtools/util/BlockCompressedOutputStreamTest.java @@ -81,6 +81,7 @@ public void testBasic() throws Exception { Assert.assertEquals(bcis2.read(buffer), available, "Should read to end of block"); Assert.assertTrue(bcis2.endOfBlock(), "Should be at end of block"); bcis2.close(); + Assert.assertEquals(bcis2.read(buffer), -1, "Should be end of file"); } @DataProvider(name = "seekReadExceptionsData") @@ -89,24 +90,32 @@ public void testBasic() throws Exception { return new Object[][]{ {HTSJDK_TRIBBLE_RESOURCES + "vcfexample.vcf.truncated.gz", FileTruncatedException.class, BlockCompressedInputStream.PREMATURE_END_MSG + System.getProperty("user.dir") + "/" + - HTSJDK_TRIBBLE_RESOURCES + "vcfexample.vcf.truncated.gz", true, false, 0}, + HTSJDK_TRIBBLE_RESOURCES + "vcfexample.vcf.truncated.gz", true, false, false, 0}, {HTSJDK_TRIBBLE_RESOURCES + "vcfexample.vcf.truncated.hdr.gz", IOException.class, BlockCompressedInputStream.INCORRECT_HEADER_SIZE_MSG + System.getProperty("user.dir") + "/" + - HTSJDK_TRIBBLE_RESOURCES + "vcfexample.vcf.truncated.hdr.gz", true, false, 0}, + HTSJDK_TRIBBLE_RESOURCES + "vcfexample.vcf.truncated.hdr.gz", true, false, false, 0}, {HTSJDK_TRIBBLE_RESOURCES + "vcfexample.vcf.gz", IOException.class, - BlockCompressedInputStream.CANNOT_SEEK_STREAM_MSG, false, true, 0}, + BlockCompressedInputStream.CANNOT_SEEK_STREAM_MSG, false, true, false, 0}, + {HTSJDK_TRIBBLE_RESOURCES + "vcfexample.vcf.gz", IOException.class, + BlockCompressedInputStream.CANNOT_SEEK_CLOSED_STREAM_MSG, false, true, true, 0}, {HTSJDK_TRIBBLE_RESOURCES + "vcfexample.vcf.gz", IOException.class, BlockCompressedInputStream.INVALID_FILE_PTR_MSG + 1000 + " for " + System.getProperty("user.dir") + "/" + - HTSJDK_TRIBBLE_RESOURCES + "vcfexample.vcf.gz", true, true, 1000 } + HTSJDK_TRIBBLE_RESOURCES + "vcfexample.vcf.gz", true, true, false, 1000 } }; } @Test(dataProvider = "seekReadExceptionsData") - public void testSeekReadExceptions(final String filePath, final Class c, final String msg, final boolean isFile, final boolean isSeek, final int pos) throws Exception { + public void testSeekReadExceptions(final String filePath, final Class c, final String msg, final boolean isFile, final boolean isSeek, final boolean isClosed, + final int pos) throws Exception { final BlockCompressedInputStream bcis = isFile ? new BlockCompressedInputStream(new File(filePath)) : new BlockCompressedInputStream(new FileInputStream(filePath)); + + if ( isClosed ) { + bcis.close(); + } + boolean haveException = false; try { if ( isSeek ) { @@ -212,5 +221,6 @@ public Deflater makeDeflater(final int compressionLevel, final boolean gzipCompa } bcis.close(); Assert.assertEquals(deflateCalls[0], 3, "deflate calls"); + Assert.assertEquals(reader.readLine(), null); } }