Allow BCFCodec subclasses to provide custom version compatibility. (#…

…1352)

src/main/java/htsjdk/variant/bcf2/BCF2Codec.java

@@ -55,13 +55,13 @@
 /**
  * Decode BCF2 files
  */
-public final class BCF2Codec extends BinaryFeatureCodec<VariantContext> {
-    private final static int ALLOWED_MAJOR_VERSION = 2;
-    private final static int MIN_MINOR_VERSION = 1;
+public class BCF2Codec extends BinaryFeatureCodec<VariantContext> {
+    protected final static int ALLOWED_MAJOR_VERSION = 2;
+    protected final static int ALLOWED_MINOR_VERSION = 1;
+    public static final BCFVersion ALLOWED_BCF_VERSION = new BCFVersion(ALLOWED_MAJOR_VERSION, ALLOWED_MINOR_VERSION);
 
     /** sizeof a BCF header (+ min/max version). Used when trying to detect when a streams starts with a bcf header */
     public static final int SIZEOF_BCF_HEADER =  BCFVersion.MAGIC_HEADER_START.length + 2*Byte.BYTES;
-
 
     private BCFVersion bcfVersion = null;
 
@@ -144,20 +144,39 @@ public Class<VariantContext> getFeatureType() {
         return VariantContext.class;
     }
 
+    /**
+     * Validate the actual version against the supported version to determine compatibility. Throws a
+     * TribbleException if the actualVersion is not compatible with the supportedVersion. Subclasses can override
+     * this to provide a custom version compatibility policy, but allowing something other than the
+     * supported version is dangerous and should be done with great care.
+     *
+     * The default policy is to require an exact version match.
+     * @param supportedVersion the current BCF implementation version
+     * @param actualVersion the actual version
+     * @thows TribbleException if the version policy determines that {@code actualVersion} is not compatible
+     * with {@code supportedVersion}
+     */
+    protected void validateVersionCompatibility(final BCFVersion supportedVersion, final BCFVersion actualVersion) {
+        if ( actualVersion.getMajorVersion() != ALLOWED_MAJOR_VERSION ) {
+            error("BCF2Codec can only process BCF2 files, this file has major version " + bcfVersion.getMajorVersion());
+        }
+
+        // require the minor version to be an exact match and reject minor versions form the future
+        if ( actualVersion.getMinorVersion() != ALLOWED_MINOR_VERSION ) {
+            error("BCF2Codec can only process BCF2 files with minor version = " + ALLOWED_MINOR_VERSION + " but this file has minor version " + bcfVersion.getMinorVersion());
+        }
+    }
+
     @Override
     public FeatureCodecHeader readHeader( final PositionalBufferedStream inputStream ) {
         try {
             // note that this reads the magic as well, and so does double duty
             bcfVersion = BCFVersion.readBCFVersion(inputStream);
-            if ( bcfVersion == null )
+            if ( bcfVersion == null ) {
                 error("Input stream does not contain a BCF encoded file; BCF magic header info not found");
+            }
 
-            if ( bcfVersion.getMajorVersion() != ALLOWED_MAJOR_VERSION )
-                error("BCF2Codec can only process BCF2 files, this file has major version " + bcfVersion.getMajorVersion());
-            // require the minor version to be an exact match and reject minor versions form the future
-            if ( bcfVersion.getMinorVersion() != MIN_MINOR_VERSION )
-                error("BCF2Codec can only process BCF2 files with minor version = " + MIN_MINOR_VERSION + " but this file has minor version " + bcfVersion.getMinorVersion());
-
+            validateVersionCompatibility(BCF2Codec.ALLOWED_BCF_VERSION, bcfVersion);
             if ( GeneralUtils.DEBUG_MODE_ENABLED ) {
                 System.err.println("Parsing data stream with BCF version " + bcfVersion);
             }
@@ -479,7 +498,7 @@ protected BCF2GenotypeFieldDecoders.Decoder getGenotypeFieldDecoder(final String
         return gtFieldDecoders.getDecoder(field);
     }
 
-    private void error(final String message) throws RuntimeException {
+    protected void error(final String message) throws RuntimeException {
         throw new TribbleException(String.format("%s, at record %d with position %d:", message, recordNo, pos));
     }
 

src/main/java/htsjdk/variant/bcf2/BCFVersion.java

@@ -37,7 +37,7 @@
  * Date: 8/2/12
  * Time: 2:16 PM
  */
-public class BCFVersion {
+public final class BCFVersion {
     /**
      * BCF2 begins with the MAGIC info BCF_M_m where M is the major version (currently 2)
      * and m is the minor version, currently 1
@@ -87,6 +87,24 @@ public static BCFVersion readBCFVersion(final InputStream stream) throws IOExcep
             return null;
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+
+        BCFVersion that = (BCFVersion) o;
+
+        if (getMajorVersion() != that.getMajorVersion()) return false;
+        return getMinorVersion() == that.getMinorVersion();
+    }
+
+    @Override
+    public int hashCode() {
+        int result = getMajorVersion();
+        result = 31 * result + getMinorVersion();
+        return result;
+    }
+
     /**
      * Write out the BCF magic information indicating this is a BCF file with corresponding major and minor versions
      * @param out

src/test/java/htsjdk/variant/bcf2/BCF2VersionTest.java

@@ -0,0 +1,53 @@
+package htsjdk.variant.bcf2;
+
+import htsjdk.variant.VariantBaseTest;
+import org.testng.Assert;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+public class BCF2VersionTest extends VariantBaseTest {
+
+    @DataProvider(name = "bcfVersionEqualsHashData")
+    public Object[][] bcfVersionEqualsHashData() {
+        return new Object[][]{
+                {
+                        BCF2Codec.ALLOWED_BCF_VERSION,
+                        new BCFVersion(BCF2Codec.ALLOWED_MAJOR_VERSION, BCF2Codec.ALLOWED_MINOR_VERSION),
+                        true
+                },
+                {
+                        new BCFVersion(0, 0),
+                        new BCFVersion(0, 0),
+                        true
+                },
+                {
+                        BCF2Codec.ALLOWED_BCF_VERSION,
+                        new BCFVersion(0, 0),
+                        false
+                },
+                {
+                        BCF2Codec.ALLOWED_BCF_VERSION,
+                        new BCFVersion(0, BCF2Codec.ALLOWED_MAJOR_VERSION),
+                        false
+                },
+                {
+                        BCF2Codec.ALLOWED_BCF_VERSION,
+                        new BCFVersion(0, BCF2Codec.ALLOWED_MAJOR_VERSION),
+                        false
+                },
+        };
+    }
+
+    @Test(dataProvider = "bcfVersionEqualsHashData")
+    private final void testBCFVersionEquals(final BCFVersion v1, BCFVersion v2, boolean expected) {
+        Assert.assertEquals(expected, v1.equals(v2));
+        Assert.assertEquals(expected, v2.equals(v1));
+    }
+
+    @Test(dataProvider = "bcfVersionEqualsHashData")
+    private final void testBCFVersionHash(final BCFVersion v1, BCFVersion v2, boolean expected) {
+        // given the small space the test data is drawn from, assume not equals => different
+        // hash codes just for this test
+        Assert.assertEquals(expected,v1.hashCode() == v2.hashCode());
+    }
+}

src/test/java/htsjdk/variant/bcf2/BCFCodecTest.java

@@ -0,0 +1,47 @@
+package htsjdk.variant.bcf2;
+
+import htsjdk.tribble.FeatureCodecHeader;
+import htsjdk.tribble.TribbleException;
+import htsjdk.tribble.readers.PositionalBufferedStream;
+import htsjdk.variant.VariantBaseTest;
+import htsjdk.variant.vcf.VCFHeader;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+
+public class BCFCodecTest extends VariantBaseTest {
+    final String TEST_DATA_DIR = "src/test/resources/htsjdk/variant/";
+
+    // should reject bcf v2.2 on read, see issue https://github.com/samtools/htsjdk/issues/1323
+    @Test(expectedExceptions = TribbleException.class)
+    private void testRejectBCFVersion22() throws IOException {
+        BCF2Codec bcfCodec = new BCF2Codec();
+        try (final FileInputStream fis = new FileInputStream(new File(TEST_DATA_DIR, "BCFVersion22Uncompressed.bcf"));
+             final PositionalBufferedStream pbs = new PositionalBufferedStream(fis)) {
+            bcfCodec.readHeader(pbs);
+        }
+    }
+
+    @Test
+    private void testBCFCustomVersionCompatibility() throws IOException {
+        final BCF2Codec bcfCodec = new BCF2Codec() {
+            @Override
+            protected void validateVersionCompatibility(final BCFVersion supportedVersion, final BCFVersion actualVersion) {
+                return;
+            }
+        };
+
+        // the default BCF2Codec version compatibility policy is to reject BCF 2.2 input; but make sure we can
+        // provide a codec that implements a more tolerant custom policy that accepts
+        try (final FileInputStream fis = new FileInputStream(new File(TEST_DATA_DIR, "BCFVersion22Uncompressed.bcf"));
+             final PositionalBufferedStream pbs = new PositionalBufferedStream(fis)) {
+            final FeatureCodecHeader featureCodecHeader = (FeatureCodecHeader)  bcfCodec.readHeader(pbs);
+            final VCFHeader vcfHeader = (VCFHeader) featureCodecHeader.getHeaderValue();
+            Assert.assertNotEquals(vcfHeader.getMetaDataInInputOrder().size(), 0);
+        }
+    }
+}
+