Cram implementation reports validation errors at container granularity

src/main/java/htsjdk/samtools/CRAMIterator.java

@@ -37,7 +37,6 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
-import java.util.List;
 
 import htsjdk.samtools.cram.CRAMException;
 
@@ -87,7 +86,7 @@ public CRAMIterator(final InputStream inputStream, final CRAMReferenceSource ref
         this.containerIterator = containerIterator;
 
         firstContainerOffset = this.countingInputStream.getCount();
-        records = new ArrayList<SAMRecord>(10000);
+        records = new ArrayList<SAMRecord>(CRAMContainerStreamWriter.DEFAULT_RECORDS_PER_SLICE);
         normalizer = new CramNormalizer(cramHeader.getSamFileHeader(),
                 referenceSource);
         parser = new ContainerParser(cramHeader.getSamFileHeader());
@@ -106,7 +105,7 @@ public CRAMIterator(final SeekableStream seekableStream, final CRAMReferenceSour
         this.containerIterator = containerIterator;
 
         firstContainerOffset = containerIterator.getFirstContainerOffset();
-        records = new ArrayList<SAMRecord>(10000);
+        records = new ArrayList<SAMRecord>(CRAMContainerStreamWriter.DEFAULT_RECORDS_PER_SLICE);
         normalizer = new CramNormalizer(cramHeader.getSamFileHeader(),
                 referenceSource);
         parser = new ContainerParser(cramHeader.getSamFileHeader());
@@ -131,6 +130,7 @@ void nextContainer() throws IOException, IllegalArgumentException,
                 nextRecord = null;
                 return;
             }
+
             container = containerIterator.next();
             if (container.isEOF()) {
                 records.clear();
@@ -146,10 +146,7 @@ void nextContainer() throws IOException, IllegalArgumentException,
             }
         }
 
-        if (records == null)
-            records = new ArrayList<SAMRecord>(container.nofRecords);
-        else
-            records.clear();
+        records.clear();
         if (cramRecords == null)
             cramRecords = new ArrayList<CramCompressionRecord>(container.nofRecords);
         else
@@ -175,15 +172,17 @@ void nextContainer() throws IOException, IllegalArgumentException,
 
         for (int i = 0; i < container.slices.length; i++) {
             final Slice slice = container.slices[i];
+
             if (slice.sequenceId < 0)
                 continue;
+
             if (!slice.validateRefMD5(refs)) {
                 final String msg = String.format(
                         "Reference sequence MD5 mismatch for slice: sequence id %d, start %d, span %d, expected MD5 %s",
-                            slice.sequenceId,
-                            slice.alignmentStart,
-                            slice.alignmentSpan,
-                            String.format("%032x", new BigInteger(1, slice.refMD5)));
+                        slice.sequenceId,
+                        slice.alignmentStart,
+                        slice.alignmentSpan,
+                        String.format("%032x", new BigInteger(1, slice.refMD5)));
                 throw new CRAMException(msg);
             }
         }
@@ -204,12 +203,6 @@ void nextContainer() throws IOException, IllegalArgumentException,
 
             samRecord.setValidationStringency(validationStringency);
 
-            if (validationStringency != ValidationStringency.SILENT) {
-                final List<SAMValidationError> validationErrors = samRecord.isValid();
-                SAMUtils.processValidationErrors(validationErrors,
-                        samRecordIndex, validationStringency);
-            }
-
             if (mReader != null) {
                 final long chunkStart = (container.offset << 16) | cramRecord.sliceIndex;
                 final long chunkEnd = ((container.offset << 16) | cramRecord.sliceIndex) + 1;
@@ -218,7 +211,6 @@ void nextContainer() throws IOException, IllegalArgumentException,
             }
 
             records.add(samRecord);
-            samRecordIndex++;
         }
         cramRecords.clear();
         iterator = records.iterator();
@@ -271,7 +263,12 @@ public boolean hasNext() {
 
     @Override
     public SAMRecord next() {
-        return iterator.next();
+        SAMRecord samRecord = iterator.next();
+        if (validationStringency != ValidationStringency.SILENT) {
+            SAMUtils.processValidationErrors(samRecord.isValid(), samRecordIndex++, validationStringency);
+        }
+
+        return samRecord;
     }
 
     @Override
@@ -286,8 +283,7 @@ public void close() {
         try {
             if (countingInputStream != null)
                 countingInputStream.close();
-        } catch (final IOException e) {
-        }
+        } catch (final IOException e) { }
     }
 
     @Override
@@ -306,5 +302,4 @@ public void setFileSource(final SamReader mReader) {
     public SAMFileHeader getSAMFileHeader() {
         return cramHeader.getSamFileHeader();
     }
-
 }

src/test/java/htsjdk/samtools/CRAMIteratorTest.java

@@ -0,0 +1,56 @@
+package htsjdk.samtools;
+
+import htsjdk.HtsjdkTest;
+import htsjdk.samtools.cram.ref.ReferenceSource;
+import htsjdk.samtools.seekablestream.SeekableStream;
+import org.junit.Assert;
+import org.testng.annotations.Test;
+
+import java.io.File;
+
+
+/**
+ * This test serve for verifying CRAMIterator records validation using strict validation strategy
+ *
+ * @author [email protected], EPAM Systems, Inc.
+ **/
+
+public class CRAMIteratorTest extends HtsjdkTest {
+
+    @Test(description = "This test checks that records validation is deferred until they are retrieved")
+    public void notThrowOnOpeningContainerWithInvalidRecords() {
+        final File refFile = new File("src/test/resources/htsjdk/samtools/cram/ce.fa");
+        final File cramFileWithInvalidRecs = new File("src/test/resources/htsjdk/samtools/cram/ce#containsInvalidRecords.3.0.cram");
+        final ReferenceSource source = new ReferenceSource(refFile);
+        final SAMRecordIterator cramIteratorOverInvalidRecords =
+                getCramFileIterator(cramFileWithInvalidRecs, source, ValidationStringency.STRICT);
+
+        Assert.assertTrue(cramIteratorOverInvalidRecords.hasNext());
+        cramIteratorOverInvalidRecords.close();
+    }
+
+    @Test(expectedExceptions = SAMException.class)
+    public void throwOnRecordValidationFailure() {
+        final File refFile = new File("src/test/resources/htsjdk/samtools/cram/ce.fa");
+        final File cramFileWithInvalidRecs = new File("src/test/resources/htsjdk/samtools/cram/ce#containsInvalidRecords.3.0.cram");
+        final ReferenceSource source = new ReferenceSource(refFile);
+        final SAMRecordIterator cramIteratorOverInvalidRecords =
+                getCramFileIterator(cramFileWithInvalidRecs, source, ValidationStringency.STRICT);
+        try{
+            while (cramIteratorOverInvalidRecords.hasNext()) {
+                cramIteratorOverInvalidRecords.next();
+            }
+        } finally {
+            cramIteratorOverInvalidRecords.close();
+        }
+    }
+
+    private SAMRecordIterator getCramFileIterator(File cramFile,
+                                                  ReferenceSource source,
+                                                  ValidationStringency valStringency) {
+
+        final CRAMFileReader cramFileReader = new CRAMFileReader(cramFile, (SeekableStream) null, source);
+        cramFileReader.setValidationStringency(valStringency);
+        return cramFileReader.getIterator();
+    }
+}