diff --git a/src/main/java/htsjdk/samtools/CRAMIterator.java b/src/main/java/htsjdk/samtools/CRAMIterator.java index aba0462cc2..b2a61d3c8e 100644 --- a/src/main/java/htsjdk/samtools/CRAMIterator.java +++ b/src/main/java/htsjdk/samtools/CRAMIterator.java @@ -41,7 +41,6 @@ public class CRAMIterator implements SAMRecordIterator { private final CountingInputStream countingInputStream; private final CramHeader cramHeader; private final ArrayList records; - private SAMRecord nextRecord = null; private final CramNormalizer normalizer; private byte[] refs; private int prevSeqId = SAMRecord.NO_ALIGNMENT_REFERENCE_INDEX; @@ -131,20 +130,17 @@ void nextContainer() throws IllegalArgumentException, CRAMException { if (containerIterator != null) { if (!containerIterator.hasNext()) { records.clear(); - nextRecord = null; return; } container = containerIterator.next(); if (container.isEOF()) { records.clear(); - nextRecord = null; return; } } else { container = ContainerIO.readContainer(cramHeader.getVersion(), countingInputStream); if (container.isEOF()) { records.clear(); - nextRecord = null; return; } } @@ -209,10 +205,9 @@ void nextContainer() throws IllegalArgumentException, CRAMException { if (mReader != null) { final long chunkStart = (container.offset << 16) | cramRecord.sliceIndex; final long chunkEnd = ((container.offset << 16) | cramRecord.sliceIndex) + 1; - nextRecord.setFileSource(new SAMFileSource(mReader, - new BAMFileSpan(new Chunk(chunkStart, chunkEnd)))); + samRecord.setFileSource(new SAMFileSource(mReader, new BAMFileSpan(new Chunk(chunkStart, chunkEnd)))); } - + records.add(samRecord); } cramRecords.clear(); diff --git a/src/main/java/htsjdk/samtools/cram/structure/CramHeader.java b/src/main/java/htsjdk/samtools/cram/structure/CramHeader.java index b45b099c95..373e5168b6 100644 --- a/src/main/java/htsjdk/samtools/cram/structure/CramHeader.java +++ b/src/main/java/htsjdk/samtools/cram/structure/CramHeader.java @@ -70,22 +70,6 @@ public void setID(final String stringID) { System.arraycopy(stringID.getBytes(), 0, this.id, 0, Math.min(this.id.length, stringID.length())); } - /** - * Copy the CRAM header into a new {@link CramHeader} object. - * @return a complete copy of the header - */ - @SuppressWarnings("CloneDoesntCallSuperClone") - @Override - public CramHeader clone() { - final CramHeader clone = new CramHeader(); - clone.version = version; - System.arraycopy(id, 0, clone.id, 0, id.length); - clone.samFileHeader = getSamFileHeader().clone(); - - return clone; - } - - /** * Get the {@link SAMFileHeader} object associated with this CRAM file header. * @return the SAM file header diff --git a/src/main/java/htsjdk/samtools/cram/structure/Slice.java b/src/main/java/htsjdk/samtools/cram/structure/Slice.java index d442845249..ada9d4f767 100644 --- a/src/main/java/htsjdk/samtools/cram/structure/Slice.java +++ b/src/main/java/htsjdk/samtools/cram/structure/Slice.java @@ -29,7 +29,6 @@ import htsjdk.samtools.cram.io.DefaultBitInputStream; import htsjdk.samtools.cram.io.DefaultBitOutputStream; import htsjdk.samtools.cram.structure.block.Block; -import htsjdk.samtools.cram.structure.block.ExternalBlock; import htsjdk.samtools.util.Log; import htsjdk.samtools.util.RuntimeIOException; import htsjdk.samtools.util.SequenceUtil; @@ -449,7 +448,7 @@ else if (slice.isMappedSingleRef()) { final ExternalCompressor compressor = header.externalCompressors.get(contentId); final byte[] rawContent = externalBlockMap.get(contentId).toByteArray(); - final ExternalBlock externalBlock = new ExternalBlock(compressor.getMethod(), contentId, + final Block externalBlock = Block.createExternalBlock(compressor.getMethod(), contentId, compressor.compress(rawContent), rawContent.length); slice.external.put(contentId, externalBlock); diff --git a/src/main/java/htsjdk/samtools/cram/structure/block/Block.java b/src/main/java/htsjdk/samtools/cram/structure/block/Block.java index db755ef18b..afd0aa1704 100644 --- a/src/main/java/htsjdk/samtools/cram/structure/block/Block.java +++ b/src/main/java/htsjdk/samtools/cram/structure/block/Block.java @@ -51,6 +51,11 @@ public class Block { */ private final BlockContentType contentType; + /** + * The External Block Content ID, or NO_CONTENT_ID for non-External block + */ + private final int contentId; + /** * The content stored in this block, in compressed form (if applicable) */ @@ -66,15 +71,18 @@ public class Block { * * @param compressionMethod the block compression method. Can be RAW, if uncompressed * @param contentType whether this is a header or data block, and which kind + * @param contentId the External Block Content ID, or NO_CONTENT_ID for non-External block * @param compressedContent the compressed form of the data to be stored in this block * @param uncompressedLength the length of the content stored in this block when uncompressed */ protected Block(final BlockCompressionMethod compressionMethod, final BlockContentType contentType, + final int contentId, final byte[] compressedContent, final int uncompressedLength) { this.compressionMethod = compressionMethod; this.contentType = contentType; + this.contentId = contentId; this.compressedContent = compressedContent; this.uncompressedLength = uncompressedLength; @@ -83,21 +91,24 @@ protected Block(final BlockCompressionMethod compressionMethod, // throw new CRAMException("Valid Content ID required for external blocks."); // } - if (contentType != BlockContentType.EXTERNAL && getContentId() != Block.NO_CONTENT_ID) { + if (contentType != BlockContentType.EXTERNAL && contentId != Block.NO_CONTENT_ID) { throw new CRAMException("Cannot set a Content ID for non-external blocks."); } } /** - * Protected constructor of a generic Block, to be called by static factory methods and subclasses. - * Creates RAW blocks only. + * Create a new non-external block with the given uncompressed content. + * The block will have RAW (no) compression. * * @param contentType whether this is a header or data block, and which kind * @param rawContent the raw data to be stored in this block */ - protected Block(final BlockContentType contentType, - final byte[] rawContent) { - this(BlockCompressionMethod.RAW, contentType, rawContent, rawContent.length); + private static Block createRawNonExternalBlock(final BlockContentType contentType, final byte[] rawContent) { + if (contentType == BlockContentType.EXTERNAL) { + throw new CRAMException("Code error: cannot use the non-external factory method for EXTERNAL blocks."); + } + + return new Block(BlockCompressionMethod.RAW, contentType, NO_CONTENT_ID, rawContent, rawContent.length); } /** @@ -108,7 +119,7 @@ protected Block(final BlockContentType contentType, * @return a new {@link Block} object */ public static Block createRawFileHeaderBlock(final byte[] rawContent) { - return new Block(BlockContentType.FILE_HEADER, rawContent); + return createRawNonExternalBlock(BlockContentType.FILE_HEADER, rawContent); } /** @@ -119,7 +130,7 @@ public static Block createRawFileHeaderBlock(final byte[] rawContent) { * @return a new {@link Block} object */ public static Block createRawCompressionHeaderBlock(final byte[] rawContent) { - return new Block(BlockContentType.COMPRESSION_HEADER, rawContent); + return createRawNonExternalBlock(BlockContentType.COMPRESSION_HEADER, rawContent); } /** @@ -130,7 +141,7 @@ public static Block createRawCompressionHeaderBlock(final byte[] rawContent) { * @return a new {@link Block} object */ public static Block createRawSliceHeaderBlock(final byte[] rawContent) { - return new Block(BlockContentType.MAPPED_SLICE, rawContent); + return createRawNonExternalBlock(BlockContentType.MAPPED_SLICE, rawContent); } /** @@ -141,7 +152,24 @@ public static Block createRawSliceHeaderBlock(final byte[] rawContent) { * @return a new {@link Block} object */ public static Block createRawCoreDataBlock(final byte[] rawContent) { - return new Block(BlockContentType.CORE, rawContent); + return createRawNonExternalBlock(BlockContentType.CORE, rawContent); + } + + /** + * Create a new external data block with the given compression method, uncompressed content, and content ID. + * The block will have EXTERNAL content type. + * + * @param compressionMethod the compression method used in this block + * @param contentId the external identifier for the block + * @param compressedContent the content of this block, in compressed mode + * @param uncompressedLength the length of the content stored in this block when uncompressed + */ + public static Block createExternalBlock(final BlockCompressionMethod compressionMethod, + final int contentId, + final byte[] compressedContent, + final int uncompressedLength) { + return new Block(compressionMethod, BlockContentType.EXTERNAL, + contentId, compressedContent, uncompressedLength); } public final BlockCompressionMethod getCompressionMethod() { @@ -157,14 +185,12 @@ public final BlockContentType getContentType() { } /** - * Return the External Content ID for this block. - * - * Only ExternalBlocks have a meaningful Content ID, so that class overrides this method. + * Return the External Content ID for this block. Only ExternalBlocks have a meaningful Content ID. * * @return the External Content ID, or NO_CONTENT_ID */ public int getContentId() { - return NO_CONTENT_ID; + return contentId; } /** @@ -232,11 +258,7 @@ public static Block read(final int major, InputStream inputStream) { } } - if (contentType == BlockContentType.EXTERNAL) { - return new ExternalBlock(compressionMethod, contentId, compressedContent, uncompressedSize); - } else { - return new Block(compressionMethod, contentType, compressedContent, uncompressedSize); - } + return new Block(compressionMethod, contentType, contentId, compressedContent, uncompressedSize); } catch (final IOException e) { throw new RuntimeIOException(e); diff --git a/src/main/java/htsjdk/samtools/cram/structure/block/ExternalBlock.java b/src/main/java/htsjdk/samtools/cram/structure/block/ExternalBlock.java deleted file mode 100644 index 48a9344e37..0000000000 --- a/src/main/java/htsjdk/samtools/cram/structure/block/ExternalBlock.java +++ /dev/null @@ -1,33 +0,0 @@ -package htsjdk.samtools.cram.structure.block; - -import htsjdk.samtools.cram.CRAMException; -import htsjdk.samtools.cram.compression.ExternalCompressor; - -/** - * A Block used by Slices to store data externally - */ -public class ExternalBlock extends Block { - private final int contentId; - - /** - * Create a new external data block with the given compression method, uncompressed content, and content ID. - * The block will have EXTERNAL content type. - * - * @param compressionMethod the compression method used in this block - * @param contentId the external identifier for the block - * @param compressedContent the content of this block, in compressed mode - * @param uncompressedLength the length of the content stored in this block when uncompressed - */ - public ExternalBlock(final BlockCompressionMethod compressionMethod, - final int contentId, - final byte[] compressedContent, - final int uncompressedLength) { - super(compressionMethod, BlockContentType.EXTERNAL, compressedContent, uncompressedLength); - this.contentId = contentId; - } - - @Override - public final int getContentId() { - return contentId; - } -} diff --git a/src/test/java/htsjdk/samtools/cram/structure/BlockTest.java b/src/test/java/htsjdk/samtools/cram/structure/block/BlockTest.java similarity index 76% rename from src/test/java/htsjdk/samtools/cram/structure/BlockTest.java rename to src/test/java/htsjdk/samtools/cram/structure/block/BlockTest.java index 9659e627f0..9c390a6c6d 100644 --- a/src/test/java/htsjdk/samtools/cram/structure/BlockTest.java +++ b/src/test/java/htsjdk/samtools/cram/structure/block/BlockTest.java @@ -1,10 +1,10 @@ -package htsjdk.samtools.cram.structure; +package htsjdk.samtools.cram.structure.block; import htsjdk.HtsjdkTest; +import htsjdk.samtools.cram.CRAMException; import htsjdk.samtools.cram.common.CramVersions; import htsjdk.samtools.cram.common.Version; import htsjdk.samtools.cram.compression.ExternalCompressor; -import htsjdk.samtools.cram.structure.block.*; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -102,7 +102,7 @@ public void testExternalBlockRoundTrips() throws IOException { final byte[] uncompressedData = "A TEST STRING WITH REDUNDANCY AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA".getBytes(); final byte[] compressedData = compressor.compress(uncompressedData); - final Block extBlock = new ExternalBlock(compressor.getMethod(), contentID, compressedData, uncompressedData.length); + final Block extBlock = Block.createExternalBlock(compressor.getMethod(), contentID, compressedData, uncompressedData.length); final Block rtBlock2 = roundTrip(extBlock, CramVersions.CRAM_v2_1); contentCheck(rtBlock2, uncompressedData, compressedData); @@ -110,4 +110,31 @@ public void testExternalBlockRoundTrips() throws IOException { final Block rtBlock3 = roundTrip(extBlock, CramVersions.CRAM_v3); contentCheck(rtBlock3, uncompressedData, compressedData); } + + @DataProvider(name = "nonExternalTypes") + private Object[][] nonExternalTypes() { + return new Object[][] { + {BlockContentType.COMPRESSION_HEADER}, + {BlockContentType.CORE}, + {BlockContentType.MAPPED_SLICE}, + {BlockContentType.FILE_HEADER}, + {BlockContentType.RESERVED}, + }; + } + + // show that we can build all non-external Block types with NO_CONTENT_ID + // but we can't if they have other values + + @Test(dataProvider = "nonExternalTypes") + public void nonExternalContentId(final BlockContentType contentType) { + new Block(BlockCompressionMethod.RAW, contentType, Block.NO_CONTENT_ID, new byte[0], 0); + } + + // show that non-external content types cannot have valid contentIds + + @Test(dataProvider = "nonExternalTypes", expectedExceptions = CRAMException.class) + public void nonExternalContentIdNegative(final BlockContentType contentType) { + final int VALID_CONTENT_ID = 1; + new Block(BlockCompressionMethod.RAW, contentType, VALID_CONTENT_ID, new byte[0], 0); + } }