clean up iterators / iterators bugs / style

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/branches/lucene4547@1440234 13f79535-47bb-0310-9956-ffa450edef68

lucene/core/src/java/org/apache/lucene/codecs/DocValuesConsumer.java

@@ -406,6 +406,9 @@ public abstract class DocValuesConsumer implements Closeable {
 
                          @Override
                          public BytesRef next() {
+                           if (!hasNext()) {
+                             throw new NoSuchElementException();
+                           }
                            return merger.mergedTerms.get(ordUpto++);
                          }
                        };

lucene/core/src/java/org/apache/lucene/index/BinaryDocValuesWriter.java

@@ -21,6 +21,7 @@ import static org.apache.lucene.util.ByteBlockPool.BYTE_BLOCK_SIZE;
 
 import java.io.IOException;
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.apache.lucene.codecs.DocValuesConsumer;
 import org.apache.lucene.store.RAMFile;
@@ -99,22 +100,32 @@ class BinaryDocValuesWriter extends DocValuesWriter {
   @Override
   public void flush(SegmentWriteState state, DocValuesConsumer dvConsumer) throws IOException {
     final int maxDoc = state.segmentInfo.getDocCount();
-    final int size = addedValues;
-
     dvConsumer.addBinaryField(fieldInfo,
                               new Iterable<BytesRef>() {
-
                                 @Override
                                 public Iterator<BytesRef> iterator() {
-                                   return new Iterator<BytesRef>() {
+                                   return new BytesIterator(maxDoc);                                 
-                                     RAMInputStream bytesReader;
+                                }
-                                     AppendingLongBuffer.Iterator iter = lengths.iterator();
+                              });
-                                     BytesRef value = new BytesRef();
+  }
+
+  @Override
+  public void abort() {
+  }
+  
+  // iterates over the values we have in ram
+  private class BytesIterator implements Iterator<BytesRef> {
+    final BytesRef value = new BytesRef();
+    final AppendingLongBuffer.Iterator lengthsIterator = lengths.iterator();
+    final int size = lengths.size();
+    final int maxDoc;
+    final RAMInputStream bytesReader;
     int upto;
     
-                                     {
+    BytesIterator(int maxDoc) {
+      this.maxDoc = maxDoc;
       try {
-                                         bytesReader = new RAMInputStream("bogus", bytes);
+        bytesReader = new RAMInputStream("BinaryDocValuesWriter", bytes);
       } catch (IOException e) {
         throw new RuntimeException(e);
       }
@@ -125,15 +136,13 @@ class BinaryDocValuesWriter extends DocValuesWriter {
       return upto < maxDoc;
     }
 
-                                     @Override
-                                     public void remove() {
-                                       throw new UnsupportedOperationException();
-                                     }
-
     @Override
     public BytesRef next() {
+      if (!hasNext()) {
+        throw new NoSuchElementException();
+      }
       if (upto < size) {
-                                         int length = (int) iter.next();
+        int length = (int) lengthsIterator.next();
         value.grow(length);
         try {
           bytesReader.readBytes(value.bytes, 0, length);
@@ -147,12 +156,10 @@ class BinaryDocValuesWriter extends DocValuesWriter {
       upto++;
       return value;
     }
-                                   };
-                                 }
-                               });
-  }
 
     @Override
-  public void abort() {
+    public void remove() {
+      throw new UnsupportedOperationException();
+    }
   }
 }

lucene/core/src/java/org/apache/lucene/index/NumericDocValuesWriter.java

@@ -19,6 +19,7 @@ package org.apache.lucene.index;
 
 import java.io.IOException;
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.apache.lucene.codecs.DocValuesConsumer;
 import org.apache.lucene.util.Counter;
@@ -75,27 +76,40 @@ class NumericDocValuesWriter extends DocValuesWriter {
 
     dvConsumer.addNumericField(fieldInfo,
                                new Iterable<Number>() {
-
                                  @Override
                                  public Iterator<Number> iterator() {
-                                   return new Iterator<Number>() {
+                                   return new NumericIterator(maxDoc);
+                                 }
+                               });
+  }
+
+  @Override
+  public void abort() {
+  }
+  
+  // iterates over the values we have in ram
+  private class NumericIterator implements Iterator<Number> {
+    final AppendingLongBuffer.Iterator iter = pending.iterator();
+    final int size = pending.size();
+    final int maxDoc;
     int upto;
-                                     AppendingLongBuffer.Iterator iter = pending.iterator();
+    
+    NumericIterator(int maxDoc) {
+      this.maxDoc = maxDoc;
+    }
     
     @Override
     public boolean hasNext() {
       return upto < maxDoc;
     }
 
-                                     @Override
-                                     public void remove() {
-                                       throw new UnsupportedOperationException();
-                                     }
-
     @Override
     public Number next() {
+      if (!hasNext()) {
+        throw new NoSuchElementException();
+      }
       long value;
-                                       if (upto < pending.size()) {
+      if (upto < size) {
         value = iter.next();
       } else {
         value = 0;
@@ -104,12 +118,10 @@ class NumericDocValuesWriter extends DocValuesWriter {
       // TODO: make reusable Number
       return value;
     }
-                                   };
-                                 }
-                               });
-  }
 
     @Override
-  public void abort() {
+    public void remove() {
+      throw new UnsupportedOperationException();
+    }
   }
 }

lucene/core/src/java/org/apache/lucene/index/SortedDocValuesWriter.java

@@ -21,6 +21,7 @@ import static org.apache.lucene.util.ByteBlockPool.BYTE_BLOCK_SIZE;
 
 import java.io.IOException;
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.apache.lucene.codecs.DocValuesConsumer;
 import org.apache.lucene.util.ArrayUtil;
@@ -110,11 +111,12 @@ class SortedDocValuesWriter extends DocValuesWriter {
         emptyOrd = ord;
       }
     } else {
-      emptyOrd = -1;
+      emptyOrd = -1; // nocommit: HUH? how can this possibly work?
     }
 
     final int valueCount = hash.size();
 
+    // nocommit: account for both sortedValues and ordMap as-we-go...
     final int[] sortedValues = hash.sort(BytesRef.getUTF8SortedAsUnicodeComparator());
     final int sortedValueRamUsage = RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + RamUsageEstimator.NUM_BYTES_INT*valueCount;
     final int[] ordMap = new int[valueCount];
@@ -131,27 +133,7 @@ class SortedDocValuesWriter extends DocValuesWriter {
                               new Iterable<BytesRef>() {
                                 @Override
                                 public Iterator<BytesRef> iterator() {
-                                  return new Iterator<BytesRef>() {
+                                  return new ValuesIterator(sortedValues, valueCount);
-                                    int ordUpto;
-                                    BytesRef scratch = new BytesRef();
-
-                                    @Override
-                                    public boolean hasNext() {
-                                      return ordUpto < valueCount;
-                                    }
-
-                                    @Override
-                                    public void remove() {
-                                      throw new UnsupportedOperationException();
-                                    }
-
-                                    @Override
-                                    public BytesRef next() {
-                                      hash.get(sortedValues[ordUpto], scratch);
-                                      ordUpto++;
-                                      return scratch;
-                                    }
-                                  };
                                 }
                               },
 
@@ -159,23 +141,75 @@ class SortedDocValuesWriter extends DocValuesWriter {
                               new Iterable<Number>() {
                                 @Override
                                 public Iterator<Number> iterator() {
-                                  return new Iterator<Number>() {
+                                  return new OrdsIterator(ordMap, bufferedDocCount, maxDoc, emptyOrd);
+                                }
+                              });
+  }
+
+  @Override
+  public void abort() {
+  }
+  
+  // iterates over the unique values we have in ram
+  private class ValuesIterator implements Iterator<BytesRef> {
+    final int sortedValues[];
+    final BytesRef scratch = new BytesRef();
+    final int valueCount;
+    int ordUpto;
+    
+    ValuesIterator(int sortedValues[], int valueCount) {
+      this.sortedValues = sortedValues;
+      this.valueCount = valueCount;
+    }
+
+    @Override
+    public boolean hasNext() {
+      return ordUpto < valueCount;
+    }
+
+    @Override
+    public BytesRef next() {
+      if (!hasNext()) {
+        throw new NoSuchElementException();
+      }
+      hash.get(sortedValues[ordUpto], scratch);
+      ordUpto++;
+      return scratch;
+    }
+
+    @Override
+    public void remove() {
+      throw new UnsupportedOperationException();
+    }
+  }
+  
+  // iterates over the ords for each doc we have in ram
+  private class OrdsIterator implements Iterator<Number> {
+    final int ordMap[];
+    final int size;
+    final int maxDoc;
+    final int emptyOrd; // nocommit
     int docUpto;
     
+    OrdsIterator(int ordMap[], int size, int maxDoc, int emptyOrd) {
+      this.ordMap = ordMap;
+      this.size = size;
+      this.maxDoc = maxDoc;
+      this.emptyOrd = emptyOrd;
+    }
+    
     @Override
     public boolean hasNext() {
       return docUpto < maxDoc;
     }
 
-                                    @Override
-                                    public void remove() {
-                                      throw new UnsupportedOperationException();
-                                    }
-
     @Override
     public Number next() {
+      if (!hasNext()) {
+        throw new NoSuchElementException();
+      }
       int ord;
-                                      if (docUpto < bufferedDocCount) {
+      if (docUpto < size) {
         ord = pending[docUpto];
       } else {
         ord = emptyOrd;
@@ -184,12 +218,10 @@ class SortedDocValuesWriter extends DocValuesWriter {
       // TODO: make reusable Number
       return ordMap[ord];
     }
-                                  };
-                                }
-                              });
-  }
 
     @Override
-  public void abort() {
+    public void remove() {
+      throw new UnsupportedOperationException();
+    }
   }
 }

lucene/test-framework/src/java/org/apache/lucene/codecs/asserting/AssertingDocValuesFormat.java

@@ -18,6 +18,8 @@ package org.apache.lucene.codecs.asserting;
  */
 
 import java.io.IOException;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.apache.lucene.codecs.DocValuesConsumer;
 import org.apache.lucene.codecs.DocValuesFormat;
@@ -75,6 +77,7 @@ public class AssertingDocValuesFormat extends DocValuesFormat {
         count++;
       }
       assert count == maxDoc;
+      checkIterator(values.iterator(), maxDoc);
       in.addNumericField(field, values);
     }
     
@@ -87,6 +90,7 @@ public class AssertingDocValuesFormat extends DocValuesFormat {
         count++;
       }
       assert count == maxDoc;
+      checkIterator(values.iterator(), maxDoc);
       in.addBinaryField(field, values);
     }
     
@@ -118,9 +122,33 @@ public class AssertingDocValuesFormat extends DocValuesFormat {
       
       assert count == maxDoc;
       assert seenOrds.cardinality() == valueCount;
+      checkIterator(values.iterator(), valueCount);
+      checkIterator(docToOrd.iterator(), maxDoc);
       in.addSortedField(field, values, docToOrd);
     }
     
+    private <T> void checkIterator(Iterator<T> iterator, int expectedSize) {
+      for (int i = 0; i < expectedSize; i++) {
+        boolean hasNext = iterator.hasNext();
+        assert hasNext;
+        T v = iterator.next();
+        assert v != null;
+        try {
+          iterator.remove();
+          throw new AssertionError("broken iterator (supports remove): " + iterator);
+        } catch (UnsupportedOperationException expected) {
+          // ok
+        }
+      }
+      assert !iterator.hasNext();
+      try {
+        iterator.next();
+        throw new AssertionError("broken iterator (allows next() when hasNext==false) " + iterator);
+      } catch (NoSuchElementException expected) {
+        // ok
+      }
+    }
+    
     @Override
     public void close() throws IOException {
       in.close();