Improve ResultSetIterator to correctly follow Iterator specification

next() must throw NoSuchElementException

diff --git a/server/sonar-server/src/main/java/org/sonar/server/db/ResultSetIterator.java b/server/sonar-server/src/main/java/org/sonar/server/db/ResultSetIterator.java
index d4a73ecadbf9b6dd56b1be38b7876ed64a3cef8b..c3ab79b7d867a172a3584370d9983daec7bc626c 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/db/ResultSetIterator.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/db/ResultSetIterator.java
@@ -28,6 +28,7 @@ import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 /**
  * {@link java.util.Iterator} applied to {@link java.sql.ResultSet}
@@ -37,8 +38,8 @@ public abstract class ResultSetIterator<E> implements Iterator<E>, Closeable {
   private final ResultSet rs;
   private final PreparedStatement stmt;
 
-  private boolean didNext = false;
-  private boolean hasNext = false;
+  private volatile boolean didNext = false;
+  private volatile boolean hasNext = false;
 
   public ResultSetIterator(PreparedStatement stmt) throws SQLException {
     this.stmt = stmt;
@@ -54,7 +55,9 @@ public abstract class ResultSetIterator<E> implements Iterator<E>, Closeable {
   public boolean hasNext() {
     if (!didNext) {
       hasNext = doNextQuietly();
-      didNext = true;
+      if (hasNext) {
+        didNext = true;
+      }
     }
     return hasNext;
   }
@@ -62,14 +65,15 @@ public abstract class ResultSetIterator<E> implements Iterator<E>, Closeable {
   @Override
   @CheckForNull
   public E next() {
-    if (!didNext) {
-      doNextQuietly();
+    if (!hasNext()) {
+      throw new NoSuchElementException();
     }
-    didNext = false;
     try {
       return read(rs);
     } catch (SQLException e) {
       throw new IllegalStateException("Fail to read result set row", e);
+    } finally {
+      hasNext = doNextQuietly();
     }
   }
 

diff --git a/server/sonar-server/src/test/java/org/sonar/server/db/ResultSetIteratorTest.java b/server/sonar-server/src/test/java/org/sonar/server/db/ResultSetIteratorTest.java
index 7d76b0276d453b4d7cfb269a58211b6ac4eca643..bfc7a43c3ac16c071fb0e1c88fac9081a2df5119 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/db/ResultSetIteratorTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/db/ResultSetIteratorTest.java
@@ -32,6 +32,7 @@ import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.util.NoSuchElementException;
 
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
@@ -74,6 +75,13 @@ public class ResultSetIteratorTest {
     assertThat(iterator.next()).isEqualTo(30);
     assertThat(iterator.hasNext()).isFalse();
 
+    try {
+      iterator.next();
+      fail();
+    } catch (NoSuchElementException e) {
+      // ok
+    }
+
     iterator.close();
     // statement is closed by ResultSetIterator
     assertThat(stmt.isClosed()).isTrue();
@@ -115,6 +123,7 @@ public class ResultSetIteratorTest {
       iterator.remove();
       fail();
     } catch (UnsupportedOperationException ok) {
+      // ok
     }
 
     iterator.close();