Bugfix: defaultBaseQualities actually works now

-- It was being applied in the wrong order (after the first call to the underlying MalformedReadFilter) so if your first read was malformed you'd blow up there instead of being fixed properly.  Added integration tests to ensure this continues to work.
-- [delivers #49538319]

public/java/src/org/broadinstitute/sting/gatk/arguments/GATKArgumentCollection.java

@@ -187,6 +187,12 @@ public class GATKArgumentCollection {
     @Argument(fullName = "allow_potentially_misencoded_quality_scores", shortName="allowPotentiallyMisencodedQuals", doc="Do not fail when encountering base qualities that are too high and that seemingly indicate a problem with the base quality encoding of the BAM file", required = false)
     public boolean ALLOW_POTENTIALLY_MISENCODED_QUALS = false;
 
+    @Argument(fullName="useOriginalQualities", shortName = "OQ", doc = "If set, use the original base quality scores from the OQ tag when present instead of the standard scores", required=false)
+    public Boolean useOriginalBaseQualities = false;
+
+    @Argument(fullName="defaultBaseQualities", shortName = "DBQ", doc = "If reads are missing some or all base quality scores, this value will be used for all base quality scores", required=false)
+    public byte defaultBaseQualities = -1;
+
     // --------------------------------------------------------------------------------------------------------------
     //
     // performance log arguments
@@ -201,9 +207,6 @@ public class GATKArgumentCollection {
     @Argument(fullName = "performanceLog", shortName="PF", doc="If provided, a GATK runtime performance log will be written to this file", required = false)
     public File performanceLog = null;
 
-    @Argument(fullName="useOriginalQualities", shortName = "OQ", doc = "If set, use the original base quality scores from the OQ tag when present instead of the standard scores", required=false)
-    public Boolean useOriginalBaseQualities = false;
-
     // --------------------------------------------------------------------------------------------------------------
     //
     // BQSR arguments
@@ -267,9 +270,6 @@ public class GATKArgumentCollection {
     //
     // --------------------------------------------------------------------------------------------------------------
 
-    @Argument(fullName="defaultBaseQualities", shortName = "DBQ", doc = "If reads are missing some or all base quality scores, this value will be used for all base quality scores", required=false)
-    public byte defaultBaseQualities = -1;
-
     @Argument(fullName = "validation_strictness", shortName = "S", doc = "How strict should we be with validation", required = false)
     public SAMFileReader.ValidationStringency strictnessLevel = SAMFileReader.ValidationStringency.SILENT;
 

public/java/src/org/broadinstitute/sting/gatk/datasources/reads/SAMDataSource.java

@@ -630,6 +630,10 @@ public class SAMDataSource {
         // *     (otherwise we will process something that we may end up throwing away)                   * //
         // ************************************************************************************************ //
 
+        if (useOriginalBaseQualities || defaultBaseQualities >= 0)
+            // only wrap if we are replacing the original qualities or using a default base quality
+            wrappedIterator = new ReadFormattingIterator(wrappedIterator, useOriginalBaseQualities, defaultBaseQualities);
+
         // Filters:
         wrappedIterator = StingSAMIteratorAdapter.adapt(new CountingFilteringIterator(readMetrics,wrappedIterator,supplementalFilters));
 
@@ -654,10 +658,6 @@ public class SAMDataSource {
         if (!noValidationOfReadOrder && enableVerification)
             wrappedIterator = new VerifyingSamIterator(wrappedIterator);
 
-        if (useOriginalBaseQualities || defaultBaseQualities >= 0)
-            // only wrap if we are replacing the original qualities or using a default base quality
-            wrappedIterator = new ReadFormattingIterator(wrappedIterator, useOriginalBaseQualities, defaultBaseQualities);
-
         // set up read transformers
         for ( final ReadTransformer readTransformer : readTransformers ) {
             if ( readTransformer.enabled() && readTransformer.getApplicationTime() == ReadTransformer.ApplicationTime.ON_INPUT )

public/java/src/org/broadinstitute/sting/gatk/filters/MalformedReadFilter.java

@@ -234,7 +234,10 @@ public class MalformedReadFilter extends ReadFilter {
         else if (filterMismatchingBaseAndQuals)
             result = false;
         else
-            throw new UserException.MalformedBAM(read, String.format("BAM file has a read with mismatching number of bases and base qualities. Offender: %s [%d bases] [%d quals]", read.getReadName(), read.getReadLength(), read.getBaseQualities().length));
+            throw new UserException.MalformedBAM(read,
+                    String.format("BAM file has a read with mismatching number of bases and base qualities. Offender: %s [%d bases] [%d quals].%s",
+                            read.getReadName(), read.getReadLength(), read.getBaseQualities().length,
+                            read.getBaseQualities().length == 0 ? " You can use --defaultBaseQualities to assign a default base quality for all reads, but this can be dangerous in you don't know what you are doing." : ""));
 
         return result;
     }

public/java/test/org/broadinstitute/sting/gatk/EngineFeaturesIntegrationTest.java

@@ -227,4 +227,32 @@ public class EngineFeaturesIntegrationTest extends WalkerTest {
             nLines++;
         Assert.assertTrue(nLines > 0);
     }
+
+    // --------------------------------------------------------------------------------
+    //
+    // Test that defaultBaseQualities actually works
+    //
+    // --------------------------------------------------------------------------------
+
+    public WalkerTestSpec testDefaultBaseQualities(final Integer value, final String md5) {
+        return new WalkerTestSpec("-T PrintReads -R " + b37KGReference + " -I " + privateTestDir + "/baseQualitiesToFix.bam -o %s"
+                + (value != null ? " --defaultBaseQualities " + value : ""),
+                1, Arrays.asList(md5));
+    }
+
+    @Test()
+    public void testDefaultBaseQualities20() {
+        executeTest("testDefaultBaseQualities20", testDefaultBaseQualities(20, "7d254a9d0ec59c66ee3e137f56f4c78f"));
+    }
+
+    @Test()
+    public void testDefaultBaseQualities30() {
+        executeTest("testDefaultBaseQualities30", testDefaultBaseQualities(30, "0f50def6cbbbd8ccd4739e2b3998e503"));
+    }
+
+    @Test(expectedExceptions = Exception.class)
+    public void testDefaultBaseQualitiesNoneProvided() {
+        executeTest("testDefaultBaseQualitiesNoneProvided", testDefaultBaseQualities(null, ""));
+    }
+
 }