Merged bug fix from Stable into Unstable

-- Resolved conflict

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

public/java/src/org/broadinstitute/sting/commandline/IntervalBinding.java

@@ -98,8 +98,8 @@ public final class IntervalBinding<T extends Feature> {
                     intervals.add(toolkit.getGenomeLocParser().createGenomeLoc(feature));
                     line = lineReader.readLine();
                 }
-            } catch (IOException e) {
+            } catch (Exception e) {
-                throw new UserException("Problem reading the interval file " + featureIntervals.getSource() + "; " + e.getMessage());
+                throw new UserException.MalformedFile(featureIntervals.getSource(), "Problem reading the interval file", e);
             }
 
         } else {

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

@@ -31,6 +31,7 @@ import net.sf.samtools.GATKChunk;
 import net.sf.samtools.util.CloseableIterator;
 import org.broadinstitute.sting.utils.GenomeLoc;
 import org.broadinstitute.sting.utils.exceptions.ReviewedStingException;
+import org.broadinstitute.sting.utils.exceptions.StingException;
 import org.broadinstitute.sting.utils.exceptions.UserException;
 
 import java.io.File;
@@ -193,7 +194,28 @@ public class BAMSchedule implements CloseableIterator<BAMScheduleEntry> {
             scheduleFileChannel.close();
         }
         catch(IOException ex) {
-            throw new ReviewedStingException("Unable to close schedule file.");
+            throw makeIOFailureException(true, "Unable to close schedule file.", ex);
+        }
+    }
+
+    /**
+     * Convenience routine for creating UserExceptions
+     * @param wasWriting
+     * @param message
+     * @param e
+     * @return
+     */
+    private final StingException makeIOFailureException(final boolean wasWriting, final String message, final Exception e) {
+        if ( wasWriting ) {
+            if ( e == null )
+                return new UserException.CouldNotCreateOutputFile(scheduleFile, message);
+            else
+                return new UserException.CouldNotCreateOutputFile(scheduleFile, message, e);
+        } else {
+            if ( e == null )
+                return new UserException.CouldNotReadInputFile(scheduleFile, message);
+            else
+                return new UserException.CouldNotReadInputFile(scheduleFile, message, e);
         }
     }
 
@@ -297,7 +319,7 @@ public class BAMSchedule implements CloseableIterator<BAMScheduleEntry> {
             return scheduleFileChannel.read(buffer);
         }
         catch(IOException ex) {
-            throw new ReviewedStingException("Unable to read data from BAM schedule file..",ex);
+            throw makeIOFailureException(false, "Unable to read data from BAM schedule file.", ex);
         }
     }
 
@@ -305,10 +327,10 @@ public class BAMSchedule implements CloseableIterator<BAMScheduleEntry> {
         try {
             scheduleFileChannel.write(buffer);
             if(buffer.remaining() > 0)
-                throw new ReviewedStingException("Unable to write entire buffer to file.");
+                throw makeIOFailureException(true, "Unable to write entire buffer to file.", null);
         }
         catch(IOException ex) {
-            throw new ReviewedStingException("Unable to write data to BAM schedule file.",ex);
+            throw makeIOFailureException(true, "Unable to write data to BAM schedule file.", ex);
         }
     }
 
@@ -321,7 +343,7 @@ public class BAMSchedule implements CloseableIterator<BAMScheduleEntry> {
             return scheduleFileChannel.position();
         }
         catch(IOException ex) {
-            throw new ReviewedStingException("Unable to retrieve position of BAM schedule file.",ex);
+            throw makeIOFailureException(false, "Unable to retrieve position of BAM schedule file.", ex);
         }
     }
 
@@ -334,7 +356,7 @@ public class BAMSchedule implements CloseableIterator<BAMScheduleEntry> {
             scheduleFileChannel.position(position);
         }
         catch(IOException ex) {
-            throw new ReviewedStingException("Unable to position BAM schedule file.",ex);
+            throw makeIOFailureException(false, "Unable to position BAM schedule file.",ex);
         }
     }
 

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

@@ -579,6 +579,8 @@ public class SAMDataSource {
             else {
                 iterator = readers.getReader(id).iterator(shard.getFileSpans().get(id));
             }
+
+            iterator = new MalformedBAMErrorReformatingIterator(id.samFile, iterator);
             if(shard.getGenomeLocs().size() > 0)
                 iterator = new IntervalOverlapFilteringIterator(iterator,shard.getGenomeLocs());
             iteratorMap.put(readers.getReader(id), iterator);
@@ -934,6 +936,8 @@ public class SAMDataSource {
                     throw new UserException.CouldNotReadInputFile(readerID.samFile, e);
                 else
                     throw e;
+            } catch ( SAMFormatException e ) {
+                throw new UserException.MalformedBAM(readerID.samFile, e.getMessage());
             }
             reader.setSAMRecordFactory(factory);
             reader.enableFileSource(true);

public/java/src/org/broadinstitute/sting/gatk/iterators/MalformedBAMErrorReformatingIterator.java

@@ -0,0 +1,45 @@
+package org.broadinstitute.sting.gatk.iterators;
+
+import net.sf.samtools.SAMFormatException;
+import net.sf.samtools.SAMRecord;
+import net.sf.samtools.util.CloseableIterator;
+import org.broadinstitute.sting.utils.exceptions.UserException;
+
+import java.io.File;
+import java.util.Iterator;
+
+/**
+ * Traps BAM formatting errors in underlying iterator and rethrows meaningful GATK UserExceptions
+ */
+public class MalformedBAMErrorReformatingIterator implements CloseableIterator<SAMRecord> {
+    File source;
+    CloseableIterator<SAMRecord> it;
+
+    public MalformedBAMErrorReformatingIterator(final File source, final CloseableIterator<SAMRecord> it) {
+        this.it = it;
+        this.source = source;
+    }
+
+    public boolean hasNext() {
+        try {
+            return this.it.hasNext();
+        } catch ( SAMFormatException e ) {
+            throw new UserException.MalformedBAM(source, e.getMessage());
+        }
+    }
+
+    public SAMRecord next() {
+        try {
+            return it.next();
+        } catch ( SAMFormatException e ) {
+            throw new UserException.MalformedBAM(source, e.getMessage());
+        }
+    }
+
+    public void remove() {
+        throw new UnsupportedOperationException("Can not remove records from a SAM file via an iterator!");
+    }
+
+    public void close() { it.close(); }
+    public Iterator<SAMRecord> iterator() { return this; }
+}

public/java/src/org/broadinstitute/sting/gatk/walkers/annotator/NBaseCount.java

@@ -27,8 +27,9 @@ public class NBaseCount extends InfoFieldAnnotation {
         int countNBaseSolid = 0;
         int countRegularBaseSolid = 0;
 
-        for( final Map.Entry<String, AlignmentContext> sample : stratifiedContexts.entrySet() ) {
+        for( final AlignmentContext context : stratifiedContexts.values() ) {
-            for( final PileupElement p : sample.getValue().getBasePileup()) {
+            if ( context.hasBasePileup() ) { // must be called as getBasePileup may throw error when pileup has no bases
+                for( final PileupElement p : context.getBasePileup()) {
                     if( p.getRead().getReadGroup().getPlatform().toUpperCase().contains("SOLID") ) {
                         if( BaseUtils.isNBase( p.getBase() ) ) {
                             countNBaseSolid++;
@@ -38,6 +39,7 @@ public class NBaseCount extends InfoFieldAnnotation {
                     }
                 }
             }
+        }
         final Map<String, Object> map = new HashMap<String, Object>();
         map.put(getKeyNames().get(0), String.format("%.4f", (double)countNBaseSolid / (double)(countNBaseSolid + countRegularBaseSolid + 1)));
         return map;

public/java/src/org/broadinstitute/sting/utils/codecs/vcf/VCFHeaderLineTranslator.java

@@ -31,8 +31,6 @@ interface VCFLineParser {
  * a class that handles the to and from disk for VCF 4 lines
  */
 class VCF4Parser implements VCFLineParser {
-    Set<String> bracketed = new HashSet<String>();
-
     /**
      * parse a VCF4 line
      * @param valueLine the line

public/java/src/org/broadinstitute/sting/utils/exceptions/UserException.java

@@ -140,6 +140,10 @@ public class UserException extends ReviewedStingException {
             super(String.format("Couldn't write file %s because %s with exception %s", file.getAbsolutePath(), message, e.getMessage()));
         }
 
+        public CouldNotCreateOutputFile(File file, String message) {
+            super(String.format("Couldn't write file %s because %s", file.getAbsolutePath(), message));
+        }
+
         public CouldNotCreateOutputFile(String filename, String message, Exception e) {
             super(String.format("Couldn't write file %s because %s with exception %s", filename, message, e.getMessage()));
         }

public/java/src/org/broadinstitute/sting/utils/variantcontext/VariantContextBuilder.java

@@ -29,6 +29,7 @@ import org.broad.tribble.Feature;
 import org.broad.tribble.TribbleException;
 import org.broad.tribble.util.ParsingUtils;
 import org.broadinstitute.sting.utils.codecs.vcf.VCFConstants;
+import org.broadinstitute.sting.utils.exceptions.ReviewedStingException;
 
 import java.util.*;
 
@@ -102,9 +103,10 @@ public class VariantContextBuilder {
      * Returns a new builder based on parent -- the new VC will have all fields initialized
      * to their corresponding values in parent.  This is the best way to create a derived VariantContext
      *
-     * @param parent
+     * @param parent  Cannot be null
      */
     public VariantContextBuilder(VariantContext parent) {
+        if ( parent == null ) throw new ReviewedStingException("BUG: VariantContext parent argument cannot be null in VariantContextBuilder");
         this.alleles = parent.alleles;
         this.attributes = parent.getAttributes();
         this.attributesCanBeModified = false;