Merged bug fix from Stable into Unstable

public/java/src/org/broadinstitute/sting/gatk/executive/WindowMaker.java

@@ -10,6 +10,7 @@ import org.broadinstitute.sting.gatk.iterators.LocusIteratorByState;
 import org.broadinstitute.sting.gatk.iterators.StingSAMIterator;
 import org.broadinstitute.sting.utils.GenomeLoc;
 import org.broadinstitute.sting.utils.GenomeLocParser;
+import org.broadinstitute.sting.utils.exceptions.ReviewedStingException;
 
 import java.util.Iterator;
 import java.util.List;
@@ -37,7 +38,7 @@ public class WindowMaker implements Iterable<WindowMaker.WindowMakerIterator>, I
     /**
      * The data source for reads.  Will probably come directly from the BAM file.
      */
-    private final Iterator<AlignmentContext> sourceIterator;
+    private final PeekableIterator<AlignmentContext> sourceIterator;
 
     /**
      * Stores the sequence of intervals that the windowmaker should be tracking.
@@ -69,7 +70,7 @@ public class WindowMaker implements Iterable<WindowMaker.WindowMakerIterator>, I
         this.sourceInfo = shard.getReadProperties();
         this.readIterator = iterator;
 
-        this.sourceIterator = new LocusIteratorByState(iterator,sourceInfo,genomeLocParser,sampleData);
+        this.sourceIterator = new PeekableIterator<AlignmentContext>(new LocusIteratorByState(iterator,sourceInfo,genomeLocParser,sampleData));
         this.intervalIterator = intervals.size()>0 ? new PeekableIterator<GenomeLoc>(intervals.iterator()) : null;
     }
 
@@ -100,11 +101,6 @@ public class WindowMaker implements Iterable<WindowMaker.WindowMakerIterator>, I
          */
         private final GenomeLoc locus;
 
-        /**
-         * Signal not to advance the iterator because we're currently sitting at the next element.
-         */
-        private boolean atNextElement = false;
-
         public WindowMakerIterator(GenomeLoc locus) {
             this.locus = locus;
             advance();
@@ -124,58 +120,45 @@ public class WindowMaker implements Iterable<WindowMaker.WindowMakerIterator>, I
 
         public boolean hasNext() {
             advance();
-            return atNextElement;
+            return currentAlignmentContext != null;
         }
 
         public AlignmentContext next() {
-            advance();
+            if(!hasNext()) throw new NoSuchElementException("WindowMakerIterator is out of elements for this interval.");
-            if(!atNextElement) throw new NoSuchElementException("WindowMakerIterator is out of elements for this interval.");
 
-            // Prepare object state for no next element.
+            // Consume this alignment context.
             AlignmentContext toReturn = currentAlignmentContext;
             currentAlignmentContext = null;
-            atNextElement = false;
 
             // Return the current element.
             return toReturn;
         }
 
         private void advance() {
-            // No shard boundaries specified.  If currentAlignmentContext has been consumed, grab the next one.
-            if(locus == null) {
-                if(!atNextElement && sourceIterator.hasNext()) {
-                    currentAlignmentContext = sourceIterator.next();
-                    atNextElement = true;
-                }
-                return;
-            }
-
-            // Can't possibly find another element.  Skip out early.
-            if(currentAlignmentContext == null && !sourceIterator.hasNext())
-                return;
-
             // Need to find the next element that is not past shard boundaries.  If we travel past the edge of
             // shard boundaries, stop and let the next interval pick it up.
-            while(sourceIterator.hasNext()) {
+            while(currentAlignmentContext == null && sourceIterator.hasNext()) {
-                // Seed the current alignment context first time through the loop.
+                // Advance the iterator and try again.
-                if(currentAlignmentContext == null)
+                AlignmentContext candidateAlignmentContext = sourceIterator.peek();
-                    currentAlignmentContext = sourceIterator.next();
 
-                // Found a match.
+                if(locus == null) {
-                if(locus.containsP(currentAlignmentContext.getLocation())) {
+                    // No filter present.  Return everything that LocusIteratorByState provides us.
-                    atNextElement = true;
+                    currentAlignmentContext = sourceIterator.next();
+                }
+                else if(locus.isPast(candidateAlignmentContext.getLocation()))
+                    // Found a locus before the current window; claim this alignment context and throw it away.
+                    sourceIterator.next();
+                else if(locus.containsP(candidateAlignmentContext.getLocation())) {
+                    // Found a locus within the current window; claim this alignment context and call it the next entry.
+                    currentAlignmentContext = sourceIterator.next();
+                }
+                else if(locus.isBefore(candidateAlignmentContext.getLocation())) {
+                    // Whoops.  Skipped passed the end of the region.  Iteration for this window is complete.  Do
+                    // not claim this alignment context in case it is part of the next shard.
                     break;
                 }
-                // Whoops.  Skipped passed the end of the region.  Iteration for this window is complete.
+                else
-                if(locus.isBefore(currentAlignmentContext.getLocation()))
+                    throw new ReviewedStingException("BUG: filtering locus does not contain, is not before, and is not past the given alignment context");
-                    break;
-
-                // No more elements to examine.  Iteration is complete.
-                if(!sourceIterator.hasNext())
-                    break;
-
-                // Advance the iterator and try again.
-                currentAlignmentContext = sourceIterator.next();
             }
         }
     }

public/java/test/org/broadinstitute/sting/gatk/datasources/providers/LocusViewTemplate.java

@@ -118,6 +118,20 @@ public abstract class LocusViewTemplate extends BaseTest {
         testReadsInContext(view, shard.getGenomeLocs(), Collections.singletonList(read));
     }
 
+    @Test
+    public void readAndLocusOverlapAtLastBase() {
+        SAMRecord read = buildSAMRecord("chr1", 1, 5);
+        SAMRecordIterator iterator = new SAMRecordIterator(read);
+
+        Shard shard = new MockLocusShard(genomeLocParser,Collections.singletonList(genomeLocParser.createGenomeLoc("chr1", 5, 5)));
+        WindowMaker windowMaker = new WindowMaker(shard,genomeLocParser,iterator,shard.getGenomeLocs(),new SampleDataSource());
+        WindowMaker.WindowMakerIterator window = windowMaker.next();
+        LocusShardDataProvider dataProvider = new LocusShardDataProvider(shard, window.getSourceInfo(), genomeLocParser, window.getLocus(), window, null, null);
+        LocusView view = createView(dataProvider);
+
+        testReadsInContext(view, shard.getGenomeLocs(), Collections.singletonList(read));
+    }
+
     @Test
     public void readOverlappingStartTest() {
         SAMRecord read = buildSAMRecord("chr1", 1, 10);