IndexingVariantContextWriter cleanup (#706)

* extract PositionalOutputStream
  * extract setIndexSequenceDictionary to IndexCreator with no-op default and TribbleIndexCreator implementation

src/main/java/htsjdk/samtools/util/PositionalOutputStream.java

@@ -0,0 +1,65 @@
+/*
+* Copyright (c) 2012 The Broad Institute
+*
+* Permission is hereby granted, free of charge, to any person
+* obtaining a copy of this software and associated documentation
+* files (the "Software"), to deal in the Software without
+* restriction, including without limitation the rights to use,
+* copy, modify, merge, publish, distribute, sublicense, and/or sell
+* copies of the Software, and to permit persons to whom the
+* Software is furnished to do so, subject to the following
+* conditions:
+*
+* The above copyright notice and this permission notice shall be
+* included in all copies or substantial portions of the Software.
+*
+* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
+* THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+*/
+
+package htsjdk.samtools.util;
+
+import java.io.IOException;
+import java.io.OutputStream;
+
+/**
+ * Wraps output stream in a manner which keeps track of the position within the file and allowing writes
+ * at arbitrary points
+ */
+public final class PositionalOutputStream extends OutputStream implements LocationAware
+{
+    private final OutputStream out;
+    private long position = 0;
+
+    public PositionalOutputStream(final OutputStream out) {
+        this.out = out;
+    }
+
+    public final void write(final byte[] bytes) throws IOException {
+        write(bytes, 0, bytes.length);
+    }
+
+    public final void write(final byte[] bytes, final int startIndex, final int numBytes) throws IOException {
+        position += numBytes;
+        out.write(bytes, startIndex, numBytes);
+    }
+
+    public final void write(final int c)  throws IOException {
+        position++;
+        out.write(c);
+    }
+
+    public final long getPosition() { return position; }
+
+    @Override
+    public void close() throws IOException {
+        super.close();
+        out.close();
+    }
+}

src/main/java/htsjdk/tribble/index/IndexCreator.java

@@ -23,6 +23,7 @@
  */
 package htsjdk.tribble.index;
 
+import htsjdk.samtools.SAMSequenceDictionary;
 import htsjdk.tribble.Feature;
 
 /**
@@ -45,6 +46,12 @@
      * @return an index object
      */
     public Index finalizeIndex(long finalFilePosition);
+
+    /**
+     * Set the sequence dictionary for the index. Default implementation does nothing.
+     * @param dict the dictionary to add to the index.
+     */
+    public default void setIndexSequenceDictionary(final SAMSequenceDictionary dict) { }
 }
 
 

src/main/java/htsjdk/tribble/index/TribbleIndexCreator.java

@@ -23,15 +23,31 @@
  */
 package htsjdk.tribble.index;
 
+import htsjdk.samtools.SAMSequenceDictionary;
+import htsjdk.samtools.SAMSequenceRecord;
+
 import java.util.LinkedHashMap;
 
 /**
  * Base class for Tribble-specific index creators.
  */
 public abstract class TribbleIndexCreator implements IndexCreator {
+    // a constant we use for marking sequence dictionary entries in the Tribble index property list
+    private static final String SEQUENCE_DICTIONARY_PROPERTY_PREDICATE = "DICT:";
+
     protected LinkedHashMap<String, String> properties = new LinkedHashMap<String, String>();
 
     public void addProperty(final String key, final String value) {
         properties.put(key, value);
     }
+
+    /** Set the sequence dictionary entries for the index property list. */
+    @Override
+    public void setIndexSequenceDictionary(final SAMSequenceDictionary dict) {
+        for (final SAMSequenceRecord seq : dict.getSequences()) {
+            final String contig = SEQUENCE_DICTIONARY_PROPERTY_PREDICATE + seq.getSequenceName();
+            final String length = String.valueOf(seq.getSequenceLength());
+            addProperty(contig,length);
+        }
+    }
 }

src/main/java/htsjdk/variant/variantcontext/writer/IndexingVariantContextWriter.java

@@ -26,14 +26,13 @@
 package htsjdk.variant.variantcontext.writer;
 
 import htsjdk.samtools.SAMSequenceDictionary;
-import htsjdk.samtools.SAMSequenceRecord;
 import htsjdk.samtools.util.LocationAware;
 import htsjdk.samtools.util.RuntimeIOException;
 import htsjdk.tribble.index.DynamicIndexCreator;
 import htsjdk.tribble.index.Index;
 import htsjdk.tribble.index.IndexCreator;
 import htsjdk.tribble.index.IndexFactory;
-import htsjdk.tribble.index.TribbleIndexCreator;
+import htsjdk.samtools.util.PositionalOutputStream;
 import htsjdk.variant.variantcontext.VariantContext;
 import htsjdk.variant.vcf.VCFHeader;
 
@@ -139,9 +138,7 @@ public void close() {
 
             // close the index stream (keep it separate to help debugging efforts)
             if (indexer != null) {
-                if (indexer instanceof TribbleIndexCreator) {
-                    setIndexSequenceDictionary((TribbleIndexCreator)indexer, refDict);
-                }
+                indexer.setIndexSequenceDictionary(refDict);
                 final Index index = indexer.finalizeIndex(locationSource.getPosition());
                 index.writeBasedOnFeatureFile(location);
             }
@@ -180,51 +177,4 @@ public void add(final VariantContext vc) {
     protected static final String writerName(final File location, final OutputStream stream) {
         return location == null ? stream.toString() : location.getAbsolutePath();
     }
-
-    // a constant we use for marking sequence dictionary entries in the Tribble index property list
-    private static final String SequenceDictionaryPropertyPredicate = "DICT:";
-
-    private static void setIndexSequenceDictionary(final TribbleIndexCreator indexCreator, final SAMSequenceDictionary dict) {
-        for (final SAMSequenceRecord seq : dict.getSequences()) {
-            final String contig = SequenceDictionaryPropertyPredicate + seq.getSequenceName();
-            final String length = String.valueOf(seq.getSequenceLength());
-            indexCreator.addProperty(contig,length);
-        }
-    }
-}
-
-/**
- * Wraps output stream in a manner which keeps track of the position within the file and allowing writes
- * at arbitrary points
- */
-final class PositionalOutputStream extends OutputStream implements LocationAware
-{
-    private final OutputStream out;
-    private long position = 0;
-
-    public PositionalOutputStream(final OutputStream out) {
-        this.out = out;
-    }
-
-    public final void write(final byte[] bytes) throws IOException {
-        write(bytes, 0, bytes.length);
-    }
-
-    public final void write(final byte[] bytes, final int startIndex, final int numBytes) throws IOException {
-        position += numBytes;
-        out.write(bytes, startIndex, numBytes);
-    }
-
-    public final void write(final int c)  throws IOException {
-        position++;
-        out.write(c);
-    }
-
-    public final long getPosition() { return position; }
-
-    @Override
-    public void close() throws IOException {
-        super.close();
-        out.close();
-    }
 }

src/test/java/htsjdk/samtools/util/PositionalOutputStreamTest.java

@@ -0,0 +1,62 @@
+/*
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2015 Daniel Gómez-Sánchez
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package htsjdk.samtools.util;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import java.io.IOException;
+import java.io.OutputStream;
+
+/**
+ * @author Daniel Gomez-Sanchez (magicDGS)
+ */
+public class PositionalOutputStreamTest {
+
+    @Test
+    public void basicPositionTest() throws Exception {
+        // wrapped null output stream to check
+        final PositionalOutputStream wrapped = new PositionalOutputStream(new OutputStream() {
+            @Override
+            public void write(int b) throws IOException {}
+        });
+        int position = 0;
+        // check that we start at position 0
+        Assert.assertEquals(wrapped.getPosition(), position);
+        // check that write one int just add one
+        wrapped.write(100);
+        Assert.assertEquals(wrapped.getPosition(), ++position);
+        // check that write a byte array adds its length
+        final byte[] bytes = new byte[]{1, 3, 5, 7};
+        wrapped.write(bytes);
+        position += bytes.length;
+        Assert.assertEquals(wrapped.getPosition(), position);
+        // check that write just some bytes from an array adds its length
+        wrapped.write(bytes, 2, 2);
+        position += 2;
+        Assert.assertEquals(wrapped.getPosition(), position);
+    }
+
+}