Added more tests to enhance reliability of Cursor implementation

src/main/java/org/apache/ibatis/cursor/Cursor.java

@@ -39,7 +39,7 @@ public interface Cursor<T> extends Closeable, Iterable<T> {
 
     /**
      * Get the current item index. The first item has the index 0.
-     * @return -1 if the cursor is not open and has not been consumed. The index of the current item retrieved.
+     * @return -1 if the first cursor item has not been retrieved. The index of the current item retrieved.
      */
     int getCurrentIndex();
 }

src/main/java/org/apache/ibatis/cursor/CursorException.java

@@ -0,0 +1,36 @@
+/**
+ *    Copyright 2009-2015 the original author or authors.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+package org.apache.ibatis.cursor;
+
+import org.apache.ibatis.exceptions.PersistenceException;
+
+/**
+ * @author Guillaume Darmont / guillaume@dropinocean.com
+ */
+public class CursorException extends PersistenceException {
+
+    public CursorException(String message) {
+        super(message);
+    }
+
+    public CursorException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+    public CursorException(Throwable cause) {
+        super(cause);
+    }
+}

src/main/java/org/apache/ibatis/cursor/defaults/DefaultCursor.java

@@ -16,6 +16,7 @@
 package org.apache.ibatis.cursor.defaults;
 
 import org.apache.ibatis.cursor.Cursor;
+import org.apache.ibatis.cursor.CursorException;
 import org.apache.ibatis.executor.resultset.DefaultResultSetHandler;
 import org.apache.ibatis.executor.resultset.ResultSetWrapper;
 import org.apache.ibatis.mapping.ResultMap;
@@ -30,6 +31,9 @@
 import java.util.NoSuchElementException;
 
 /**
+ * This is the default implementation of a MyBatis Cursor.
+ * This implementation is not thread safe.
+ *
  * @author Guillaume Darmont / guillaume@dropinocean.com
  */
 public class DefaultCursor<T> implements Cursor<T> {
@@ -41,13 +45,31 @@ public class DefaultCursor<T> implements Cursor<T> {
     private final RowBounds rowBounds;
     private final ObjectWrapperResultHandler<T> objectWrapperResultHandler = new ObjectWrapperResultHandler<T>();
 
-    private int currentIndex = -1;
+    private CursorIterator cursorIterator = new CursorIterator();
+    private boolean iteratorRetrieved = false;
 
-    private boolean iteratorAlreadyOpened = false;
+    private CursorStatus status = CursorStatus.CREATED;
+    private int indexWithRowBound = -1;
 
-    private boolean opened = false;
+    private enum CursorStatus {
 
-    private boolean resultSetConsumed = false;
+        /**
+         * A freshly created cursor, database ResultSet consuming has not started
+         */
+        CREATED,
+        /**
+         * A cursor currently in use, database ResultSet consuming has started
+         */
+        OPEN,
+        /**
+         * A closed cursor, not fully consumed
+         */
+        CLOSED,
+        /**
+         * A fully consumed cursor, a consumed cursor is always closed
+         */
+        CONSUMED
+    }
 
     public DefaultCursor(DefaultResultSetHandler resultSetHandler, ResultMap resultMap, ResultSetWrapper rsw, RowBounds rowBounds) {
         this.resultSetHandler = resultSetHandler;
@@ -58,26 +80,34 @@ public DefaultCursor(DefaultResultSetHandler resultSetHandler, ResultMap resultM
 
     @Override
     public boolean isOpen() {
-        return opened;
+        return status == CursorStatus.OPEN;
     }
 
     @Override
     public boolean isConsumed() {
-        return resultSetConsumed;
+        return status == CursorStatus.CONSUMED;
     }
 
     @Override
     public int getCurrentIndex() {
-        return currentIndex;
+        return rowBounds.getOffset() + cursorIterator.iteratorIndex;
     }
 
     @Override
     public Iterator<T> iterator() {
-        return new CursorIterator();
+        if (iteratorRetrieved) {
+            throw new IllegalStateException("Cannot open more than one iterator on a Cursor");
+        }
+        iteratorRetrieved = true;
+        return cursorIterator;
     }
 
     @Override
     public void close() {
+        if (isClosed()) {
+            return;
+        }
+
         ResultSet rs = rsw.getResultSet();
         try {
             if (rs != null) {
@@ -88,56 +118,60 @@ public void close() {
                     statement.close();
                 }
             }
-            opened = false;
+            status = CursorStatus.CLOSED;
         } catch (SQLException e) {
             // ignore
         }
     }
 
     protected T fetchNextUsingRowBound() {
         T result = fetchNextObjectFromDatabase();
-        while (currentIndex < rowBounds.getOffset()) {
+        while (result != null && indexWithRowBound < rowBounds.getOffset()) {
             result = fetchNextObjectFromDatabase();
         }
         return result;
     }
 
     protected T fetchNextObjectFromDatabase() {
-        if (resultSetConsumed) {
+        if (isClosed()) {
             return null;
         }
 
         try {
-            opened = true;
+            status = CursorStatus.OPEN;
             resultSetHandler.handleRowValues(rsw, resultMap, objectWrapperResultHandler, RowBounds.DEFAULT, null);
         } catch (SQLException e) {
             throw new RuntimeException(e);
         }
 
         T next = objectWrapperResultHandler.result;
         if (next != null) {
-            currentIndex++;
+            indexWithRowBound++;
         }
         // No more object or limit reached
         if (next == null || (getReadItemsCount() == rowBounds.getOffset() + rowBounds.getLimit())) {
             close();
-            resultSetConsumed = true;
+            status = CursorStatus.CONSUMED;
         }
         objectWrapperResultHandler.result = null;
 
         return next;
     }
 
+    private boolean isClosed() {
+        return status == CursorStatus.CLOSED || status == CursorStatus.CONSUMED;
+    }
+
     private int getReadItemsCount() {
-        return currentIndex + 1;
+        return indexWithRowBound + 1;
     }
 
-    private static class ObjectWrapperResultHandler<E> implements ResultHandler {
+    private static class ObjectWrapperResultHandler<E> implements ResultHandler<E> {
 
         private E result;
 
         @Override
-        public void handleResult(ResultContext context) {
+        public void handleResult(ResultContext<? extends E> context) {
             this.result = (E) context.getResultObject();
             context.stop();
         }
@@ -146,19 +180,21 @@ public void handleResult(ResultContext context) {
     private class CursorIterator implements Iterator<T> {
 
         /**
-         * Holder for the next objet to be returned
+         * Holder for the next object to be returned
          */
         T object;
 
-        public CursorIterator() {
-            if (iteratorAlreadyOpened) {
-                throw new IllegalStateException("Cannot open more than one iterator on a Cursor");
-            }
-            iteratorAlreadyOpened = true;
-        }
+        /**
+         * Index of objects returned using next(), and as such, visible to users.
+         */
+        int iteratorIndex = -1;
 
         @Override
         public boolean hasNext() {
+            if (isClosed()) {
+                return false;
+            }
+
             if (object == null) {
                 object = fetchNextUsingRowBound();
             }
@@ -167,6 +203,10 @@ public boolean hasNext() {
 
         @Override
         public T next() {
+            if (isClosed()) {
+                throw new CursorException("Cursor is closed");
+            }
+
             // Fill next with object fetched from hasNext()
             T next = object;
 
@@ -176,14 +216,15 @@ public T next() {
 
             if (next != null) {
                 object = null;
+                iteratorIndex++;
                 return next;
             }
             throw new NoSuchElementException();
         }
 
         @Override
         public void remove() {
-            throw new UnsupportedOperationException("Cannot currently remove element from Cursor");
+            throw new UnsupportedOperationException("Cannot remove element from Cursor");
         }
     }
 }