fix CRAMIterator when next() is called without hasNext()

* fixing a bug in CRAMIterator which previously threw if next() was called without first calling hasNext()
* adding CRAMTestUtils and extracting a method to it from CramEdgeCasesTest

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

@@ -34,10 +34,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.math.BigInteger;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
 
 import htsjdk.samtools.cram.CRAMException;
 
@@ -259,9 +256,7 @@ public boolean hasNext() {
         if (!iterator.hasNext()) {
             try {
                 nextContainer();
-            } catch (IOException e) {
-                throw new SAMException(e);
-            } catch (IllegalAccessException e) {
+            } catch (IOException | IllegalAccessException e) {
                 throw new SAMException(e);
             }
         }
@@ -271,7 +266,11 @@ public boolean hasNext() {
 
     @Override
     public SAMRecord next() {
-        return iterator.next();
+        if(hasNext()) {
+            return iterator.next();
+        } else {
+            throw new NoSuchElementException();
+        }
     }
 
     @Override

src/test/java/htsjdk/samtools/CRAMEdgeCasesTest.java

@@ -3,19 +3,16 @@
 import htsjdk.HtsjdkTest;
 import htsjdk.samtools.cram.CRAMException;
 import htsjdk.samtools.cram.ref.ReferenceSource;
-import htsjdk.samtools.reference.InMemoryReferenceSequenceFile;
-import htsjdk.samtools.seekablestream.SeekableStream;
 import htsjdk.samtools.util.Log;
 import org.testng.Assert;
 import org.testng.annotations.BeforeTest;
 import org.testng.annotations.Test;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Iterator;
 
 /**
@@ -79,22 +76,9 @@ public void testNullsAndBeyondRef() throws IOException {
         testSingleRecord("AAA".getBytes(), "!!!".getBytes(), "A".getBytes());
     }
 
-    private void testRecords(Collection<SAMRecord> records, byte[] ref) throws IOException {
-        ByteArrayOutputStream baos = new ByteArrayOutputStream();
-        InMemoryReferenceSequenceFile refFile = new InMemoryReferenceSequenceFile();
-        refFile.add("chr1", ref);
-        ReferenceSource source = new ReferenceSource(refFile);
-        final SAMFileHeader header = records.iterator().next().getHeader();
-        CRAMFileWriter cramFileWriter = new CRAMFileWriter(baos, source, header, "whatever");
-
-        Iterator<SAMRecord> it = records.iterator();
-        while (it.hasNext()) {
-            SAMRecord record = it.next();
-            cramFileWriter.addAlignment(record);
-        }
-        cramFileWriter.close();
-
-        CRAMFileReader cramFileReader = new CRAMFileReader(new ByteArrayInputStream(baos.toByteArray()), (SeekableStream) null, source, ValidationStringency.SILENT);
+    private static void testRecords(Collection<SAMRecord> records, byte[] ref) throws IOException {
+        CRAMFileReader cramFileReader = CRAMTestUtils.writeAndReadFromInMemoryCram(records, ref);
+        Iterator<SAMRecord> it;
         final SAMRecordIterator iterator = cramFileReader.getIterator();
         Assert.assertTrue(iterator.hasNext());
 
@@ -113,16 +97,8 @@ private void testRecords(Collection<SAMRecord> records, byte[] ref) throws IOExc
         Assert.assertFalse(iterator.hasNext());
     }
 
-    private void testSingleRecord(SAMRecord record, byte[] ref) throws IOException {
-        ByteArrayOutputStream baos = new ByteArrayOutputStream();
-        InMemoryReferenceSequenceFile refFile = new InMemoryReferenceSequenceFile();
-        refFile.add("chr1", ref);
-        ReferenceSource source = new ReferenceSource(refFile);
-        CRAMFileWriter cramFileWriter = new CRAMFileWriter(baos, source, record.getHeader(), "whatever");
-        cramFileWriter.addAlignment(record);
-        cramFileWriter.close();
-
-        CRAMFileReader cramFileReader = new CRAMFileReader(new ByteArrayInputStream(baos.toByteArray()), (SeekableStream) null, source, ValidationStringency.SILENT);
+    private static void testSingleRecord(SAMRecord record, byte[] ref) throws IOException {
+        CRAMFileReader cramFileReader = CRAMTestUtils.writeAndReadFromInMemoryCram(Collections.singletonList(record), ref);
         final SAMRecordIterator iterator = cramFileReader.getIterator();
         Assert.assertTrue(iterator.hasNext());
         SAMRecord s2 = iterator.next();

src/test/java/htsjdk/samtools/CRAMFileReaderTest.java

@@ -37,6 +37,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.NoSuchElementException;
 
 /**
  * Additional tests for CRAMFileReader are in CRAMFileIndexTest
@@ -229,4 +230,14 @@ public void testCRAMReader7_WithoutCRAMIndex() throws IOException {
         CRAMFileReader reader = new CRAMFileReader(CRAM_WITHOUT_CRAI, indexFile, REFERENCE, ValidationStringency.STRICT);
         reader.getIndex();
     }
+
+    @Test
+    public void testCramIteratorWithoutCallingHasNextFirst() throws IOException {
+        final SAMRecordSetBuilder builder = new SAMRecordSetBuilder(false, SAMFileHeader.SortOrder.unsorted);
+        builder.addFrag("1", 0, 2, false);
+        final CRAMFileReader reader = CRAMTestUtils.writeAndReadFromInMemoryCram(builder);
+        final SAMRecordIterator iterator = reader.getIterator();
+        Assert.assertNotNull(iterator.next());
+        Assert.assertThrows(NoSuchElementException.class, iterator::next);
+    }
 }

src/test/java/htsjdk/samtools/CRAMTestUtils.java

@@ -0,0 +1,64 @@
+package htsjdk.samtools;
+
+import htsjdk.samtools.cram.ref.CRAMReferenceSource;
+import htsjdk.samtools.cram.ref.ReferenceSource;
+import htsjdk.samtools.reference.InMemoryReferenceSequenceFile;
+import htsjdk.samtools.seekablestream.SeekableStream;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Iterator;
+
+public final class CRAMTestUtils {
+
+    private CRAMTestUtils(){};
+
+    /**
+     * Write a collection of SAMRecords into an in memory Cram file and then open a reader over it
+     * @param records a Collection of SAMRecords
+     * @param ref a set of bases to use as the single reference contig named "chr1"
+     * @return a CRAMFileReader reading from an in memory buffer that has had the records written into it
+     */
+    public static CRAMFileReader writeAndReadFromInMemoryCram(Collection<SAMRecord> records, byte[] ref) throws IOException {
+        InMemoryReferenceSequenceFile refFile = new InMemoryReferenceSequenceFile();
+        refFile.add("chr1", ref);
+        ReferenceSource source = new ReferenceSource(refFile);
+        final SAMFileHeader header = records.iterator().next().getHeader();
+        return writeAndReadFromInMemoryCram(records, source, header);
+    }
+
+    /**
+     * Write a collection of SAMRecords into an in memory Cram file and then open a reader over it
+     * @param records a SAMRecordSetBuilder which has been initialized with records
+     * @return a CRAMFileReader reading from an in memory buffer that has had the records written into it, uses a fake reference with all A's
+     */
+    public static CRAMFileReader writeAndReadFromInMemoryCram(SAMRecordSetBuilder records) throws IOException {
+        return writeAndReadFromInMemoryCram(records.getRecords(), getFakeReferenceSource(), records.getHeader());
+    }
+
+    private static CRAMFileReader writeAndReadFromInMemoryCram(Collection<SAMRecord> records, CRAMReferenceSource source, SAMFileHeader header) throws IOException {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        CRAMFileWriter cramFileWriter = new CRAMFileWriter(baos, source, header, "whatever");
+
+        for (SAMRecord record : records) {
+            cramFileWriter.addAlignment(record);
+        }
+        cramFileWriter.close();
+
+        return new CRAMFileReader(new ByteArrayInputStream(baos.toByteArray()), (SeekableStream) null, source, ValidationStringency.SILENT);
+    }
+
+    /**
+     * return a CRAMReferenceSource that returns all A's for any sequence queried
+     */
+    public static CRAMReferenceSource getFakeReferenceSource(){
+        return (sequenceRecord, tryNameVariants) -> {
+            byte[] bases = new byte[sequenceRecord.getSequenceLength()];
+            Arrays.fill(bases, (byte)'A');
+            return bases;
+        };
+    }
+}