Command-line read filters are now applied before Walker default filters

-- This allows us to use -rf ReassignMappingQuality to reassign mapping qualities to 60 *before* the BQSR filters them out with MappingQualityUnassignedFilter.
-- delivers #50222251

public/java/src/org/broadinstitute/sting/gatk/GenomeAnalysisEngine.java

@@ -344,11 +344,18 @@ public class GenomeAnalysisEngine {
      * @return A collection of available filters.
      */
     public Collection<ReadFilter> createFilters() {
-        final List<ReadFilter> filters = WalkerManager.getReadFilters(walker,this.getFilterManager());
+        final List<ReadFilter> filters = new LinkedList<>();
+
+        // First add the user requested filters
         if (this.getArguments().readGroupBlackList != null && this.getArguments().readGroupBlackList.size() > 0)
             filters.add(new ReadGroupBlackListFilter(this.getArguments().readGroupBlackList));
         for(final String filterName: this.getArguments().readFilters)
             filters.add(this.getFilterManager().createByName(filterName));
+
+        // now add the walker default filters.  This ordering is critical important if
+        // users need to apply filters that fix up reads that would be removed by default walker filters
+        filters.addAll(WalkerManager.getReadFilters(walker,this.getFilterManager()));
+
         return Collections.unmodifiableList(filters);
     }
 

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

@@ -26,12 +26,20 @@
 package org.broadinstitute.sting.gatk;
 
 import org.broadinstitute.sting.WalkerTest;
+import org.broadinstitute.sting.commandline.Output;
+import org.broadinstitute.sting.gatk.contexts.ReferenceContext;
+import org.broadinstitute.sting.gatk.filters.MappingQualityUnavailableFilter;
+import org.broadinstitute.sting.gatk.refdata.RefMetaDataTracker;
+import org.broadinstitute.sting.gatk.walkers.ReadFilters;
+import org.broadinstitute.sting.gatk.walkers.ReadWalker;
 import org.broadinstitute.sting.gatk.walkers.qc.ErrorThrowing;
 import org.broadinstitute.sting.utils.exceptions.ReviewedStingException;
 import org.broadinstitute.sting.utils.exceptions.UserException;
+import org.broadinstitute.sting.utils.sam.GATKSAMRecord;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
+import java.io.PrintStream;
 import java.util.Arrays;
 
 /**
@@ -126,4 +134,44 @@ public class EngineFeaturesIntegrationTest extends WalkerTest {
             executeTest(cfg.toString(), spec);
         }
     }
+
+    // --------------------------------------------------------------------------------
+    //
+    // Test that read filters are being applied in the order we expect
+    //
+    // --------------------------------------------------------------------------------
+
+    @ReadFilters({MappingQualityUnavailableFilter.class})
+    public static class DummyReadWalkerWithMapqUnavailableFilter extends ReadWalker<Integer, Integer> {
+        @Output
+        PrintStream out;
+
+        @Override
+        public Integer map(ReferenceContext ref, GATKSAMRecord read, RefMetaDataTracker metaDataTracker) {
+            return 1;
+        }
+
+        @Override
+        public Integer reduceInit() {
+            return 0;
+        }
+
+        @Override
+        public Integer reduce(Integer value, Integer sum) {
+            return value + sum;
+        }
+
+        @Override
+        public void onTraversalDone(Integer result) {
+            out.println(result);
+        }
+    }
+
+    @Test(enabled = true)
+    public void testUserReadFilterAppliedBeforeWalker() {
+        WalkerTestSpec spec = new WalkerTestSpec("-R " + b37KGReference + " -I " + privateTestDir + "allMAPQ255.bam"
+                + " -T DummyReadWalkerWithMapqUnavailableFilter -o %s -L MT -rf ReassignMappingQuality",
+                1, Arrays.asList("ecf27a776cdfc771defab1c5d19de9ab"));
+        executeTest("testUserReadFilterAppliedBeforeWalker", spec);
+    }
 }