From 64dfda604f7200cf61f7576bcb57f4cc69fb7c0c Mon Sep 17 00:00:00 2001
From: Simon Brandhof <simon.brandhof@sonarsource.com>
Date: Mon, 15 Dec 2014 09:35:27 +0100
Subject: [PATCH] Improve ResultSetIterator to correctly follow Iterator
 specification

next() must throw NoSuchElementException
---
 .../org/sonar/server/db/ResultSetIterator.java   | 16 ++++++++++------
 .../sonar/server/db/ResultSetIteratorTest.java   |  9 +++++++++
 2 files changed, 19 insertions(+), 6 deletions(-)

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 d4a73ecadbf..c3ab79b7d86 100644
--- 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 7d76b0276d4..bfc7a43c3ac 100644
--- 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();
-- 
2.39.5