populateSlicesAndIndexingParameters -> distributeIndexingParametersTo…

…Slices

- called on Container write now as well

src/main/java/htsjdk/samtools/cram/structure/Container.java

@@ -34,7 +34,7 @@ public class Container {
     /**
      * Byte size of the content excluding header.
      */
-    public int containerByteSize;
+    public int containerByteSize = 0;
 
     // minimum alignment start of the reads in this Container
     // uses a 1-based coordinate system
@@ -81,7 +81,7 @@ public Slice[] getSlices() {
      * @param slices the Slices belonging to this container
      * @param byteOffset the byte location in the stream where this Container begins
      */
-    private void setSlices(final Slice[] slices, final long byteOffset) {
+    void setSlices(final Slice[] slices, final long byteOffset) {
         this.slices = slices;
         setByteOffsetInSlices(byteOffset);
     }
@@ -195,26 +195,39 @@ else if (sliceRefContexts.size() > 1) {
     }
 
     /**
-     * Assign this Container's slices, and populate those slices'
-     * indexing parameters from this Container
-     * @param slicesToPopulate the slices to populate
-     * @param containerByteOffset the Container's byte offset from the start of the stream
+     * Populate the indexing parameters of this Container's slices
+     *
+     * Requires: valid landmarks and containerByteSize
+     *
+     * @throws CRAMException when the Container is in an invalid state
      */
-    void populateSlicesAndIndexingParameters(final ArrayList<Slice> slicesToPopulate, final long containerByteOffset) {
-
-        if (slicesToPopulate.isEmpty()) {
+    void distributeIndexingParametersToSlices() {
+        if (slices.length == 0) {
             return;
         }
 
-        final int lastSliceIndex = slicesToPopulate.size() - 1;
+        if (landmarks == null) {
+            throw new CRAMException("Cannot set Slice indexing parameters if this Container does not have landmarks");
+        }
+
+        if (landmarks.length != slices.length) {
+            final String format = "This Container's landmark and slice counts do not match: %d landmarks and %d slices";
+            throw new CRAMException(String.format(format, landmarks.length, slices.length));
+        }
+
+        if (containerByteSize == 0) {
+            throw new CRAMException("Cannot set Slice indexing parameters if this Container's byte size is unknown");
+        }
+
+        final int lastSliceIndex = slices.length - 1;
         for (int i = 0; i < lastSliceIndex; i++) {
-            final Slice slice = slicesToPopulate.get(i);
+            final Slice slice = slices[i];
             slice.index = i;
             slice.byteOffsetFromContainer = landmarks[i];
             slice.byteSize = landmarks[i + 1] - slice.byteOffsetFromContainer;
         }
 
-        final Slice lastSlice = slicesToPopulate.get(lastSliceIndex);
+        final Slice lastSlice = slices[lastSliceIndex];
         lastSlice.index = lastSliceIndex;
         lastSlice.byteOffsetFromContainer = landmarks[lastSliceIndex];
 
@@ -224,8 +237,6 @@ void populateSlicesAndIndexingParameters(final ArrayList<Slice> slicesToPopulate
         final int containerHeaderSize = landmarks[0];
         final int containerTotalByteSize = containerHeaderSize + containerByteSize;
         lastSlice.byteSize = containerTotalByteSize - lastSlice.byteOffsetFromContainer;
-
-        setSlices(slicesToPopulate.toArray(new Slice[0]), containerByteOffset);
     }
 
     /**

src/main/java/htsjdk/samtools/cram/structure/ContainerIO.java

@@ -105,7 +105,8 @@ private static Container readContainerInternal(final int major,
             slices.add(SliceIO.read(major, inputStream));
         }
 
-        container.populateSlicesAndIndexingParameters(slices, containerByteOffset);
+        container.setSlices(slices.toArray(new Slice[0]), containerByteOffset);
+        container.distributeIndexingParametersToSlices();
 
         log.debug("READ CONTAINER: " + container.toString());
 
@@ -158,9 +159,11 @@ public static int writeContainer(final Version version, final Container containe
             container.blockCount += slice.external.size();
         }
         container.landmarks = landmarks.stream().mapToInt(Integer::intValue).toArray();
-
         container.containerByteSize = byteArrayOutputStream.size();
 
+        // Slices require the Container's landmarks and containerByteSize before indexing
+        container.distributeIndexingParametersToSlices();
+
         int length = ContainerHeaderIO.writeContainerHeader(version.major, container, outputStream);
         try {
             outputStream.write(byteArrayOutputStream.toByteArray(), 0, byteArrayOutputStream.size());

src/test/java/htsjdk/samtools/cram/structure/ContainerTest.java

@@ -162,12 +162,12 @@ public void getSpansTest(final List<CramCompressionRecord> records,
     // show that we can populate all of the slice indexing fields from the
     // values in the container's header
 
-    // this is part of the deserialization process, and supports index creation
+    // this is part of the serialization/deserialization process, and supports index creation
 
     // single slice
 
     @Test
-    public static void populateSlicesAndIndexingParametersOneSlice() {
+    public static void distributeIndexingParametersToSlicesOneSlice() {
         // this container starts 100,000 bytes into the CRAM stream
         final int containerStreamByteOffset = 100000;
 
@@ -186,7 +186,7 @@ public static void populateSlicesAndIndexingParametersOneSlice() {
     // two slices
 
     @Test
-    public static void populateSlicesAndIndexingParametersTwoSlices() {
+    public static void distributeIndexingParametersToSlicesTwoSlices() {
         // this container starts 200,000 bytes into the CRAM stream
         final int containerStreamByteOffset = 200000;
 
@@ -205,6 +205,42 @@ public static void populateSlicesAndIndexingParametersTwoSlices() {
         assertSliceIndexingParams(container.getSlices()[1], 1, containerStreamByteOffset, slice1size, containerHeaderSize + slice0size);
     }
 
+    @DataProvider(name = "containerDistributeNegative")
+    private Object[][] containerDistributeNegative() {
+        final ReferenceContext refContext = new ReferenceContext(0);
+
+        final Container nullLandmarks = new Container(refContext);
+        nullLandmarks.containerByteSize = 6789;
+        nullLandmarks.landmarks = null;
+        nullLandmarks.setSlices(new Slice[] { new Slice(refContext) }, 999);
+
+        final Container tooManyLandmarks = new Container(refContext);
+        tooManyLandmarks.containerByteSize = 111;
+        tooManyLandmarks.landmarks = new int[]{ 1, 2, 3, 4, 5 };
+        tooManyLandmarks.setSlices(new Slice[] { new Slice(refContext) }, 12345);
+
+        final Container tooManySlices = new Container(refContext);
+        tooManySlices.containerByteSize = 675345389;
+        tooManySlices.landmarks = new int[]{ 1 };
+        tooManySlices.setSlices(new Slice[] { new Slice(refContext), new Slice(refContext) }, 12345);
+
+        final Container noByteSize = new Container(refContext);
+        noByteSize.landmarks = new int[]{ 1, 2 };
+        noByteSize.setSlices(new Slice[] { new Slice(refContext), new Slice(refContext) }, 12345);
+
+        return new Object[][] {
+                { nullLandmarks },
+                { tooManyLandmarks },
+                { tooManySlices },
+                { noByteSize },
+        };
+    }
+
+    @Test(expectedExceptions = CRAMException.class, dataProvider = "containerDistributeNegative")
+    public static void distributeIndexingParametersToSlicesNegative(final Container container) {
+        container.distributeIndexingParametersToSlices();
+    }
+
     private static Container createOneSliceContainer(final int containerStreamByteOffset,
                                                      final int containerHeaderSize,
                                                      final int slice0size) {
@@ -219,7 +255,8 @@ private static Container createOneSliceContainer(final int containerStreamByteOf
         final ArrayList<Slice> slices = new ArrayList<Slice>() {{
             add(new Slice(refContext));
         }};
-        container.populateSlicesAndIndexingParameters(slices, containerStreamByteOffset);
+        container.setSlices(slices.toArray(new Slice[0]), containerStreamByteOffset);
+        container.distributeIndexingParametersToSlices();
         return container;
     }
 
@@ -242,7 +279,8 @@ private static Container createTwoSliceContainer(final int containerStreamByteOf
             add(new Slice(refContext));
             add(new Slice(refContext));
         }};
-        container.populateSlicesAndIndexingParameters(slices, containerStreamByteOffset);
+        container.setSlices(slices.toArray(new Slice[0]), containerStreamByteOffset);
+        container.distributeIndexingParametersToSlices();
         return container;
     }