Browse Source

Fix first() failing on unbuffered queries.

Re-add first() to ResultSet. Because of how `LimitIterator` and
unbuffered queries interact, valid() was being called multiple times,
advancing the iterator position beyond the result set data.

By having a specific first() on ResultSet we can ensure that the
statement is closed after the first record is read. This of course makes
ResultSet forget any additional length they may have.

Refs #5720
Mark Story 11 years ago
parent
commit
affd04b8a4
3 changed files with 40 additions and 10 deletions
  1. 20 7
      src/ORM/ResultSet.php
  2. 1 1
      tests/TestCase/ORM/QueryRegressionTest.php
  3. 19 2
      tests/TestCase/ORM/QueryTest.php

+ 20 - 7
src/ORM/ResultSet.php

@@ -226,24 +226,37 @@ class ResultSet implements ResultSetInterface
             }
         }
 
-        if (!$valid) {
-            if ($this->_statement !== null) {
-                $this->_statement->closeCursor();
-            }
-            return false;
-        }
-
         $this->_current = $this->_fetchResult();
         $valid = $this->_current !== false;
 
         if ($valid && $this->_useBuffering) {
             $this->_results[$this->_index] = $this->_current;
         }
+        if (!$valid && $this->_statement !== null) {
+            $this->_statement->closeCursor();
+        }
 
         return $valid;
     }
 
     /**
+     * Get the first record from a result set.
+     *
+     * This method will also close the underlying statement cursor.
+     *
+     * @return array|object
+     */
+    public function first()
+    {
+        foreach ($this as $result) {
+            if ($this->_statement) {
+                $this->_statement->closeCursor();
+            }
+            return $result;
+        }
+    }
+
+    /**
      * Serializes a resultset.
      *
      * Part of Serializable interface.

+ 1 - 1
tests/TestCase/ORM/QueryRegressionTest.php

@@ -598,7 +598,7 @@ class QueryRegressionTest extends TestCase
         $results = TableRegistry::get('Articles')->find()->all();
         $this->assertEquals(3, $results->count());
         $this->assertNotNull($results->first());
-        $this->assertCount(3, $results->toArray());
+        $this->assertCount(1, $results->toArray());
     }
 
     /**

+ 19 - 2
tests/TestCase/ORM/QueryTest.php

@@ -997,9 +997,9 @@ class QueryTest extends TestCase
             '\Database\StatementInterface',
             ['fetch', 'closeCursor', 'rowCount']
         );
-        $statement->expects($this->exactly(2))
+        $statement->expects($this->exactly(3))
             ->method('fetch')
-            ->will($this->onConsecutiveCalls(['a' => 1], ['a' => 2]));
+            ->will($this->onConsecutiveCalls(['a' => 1], ['a' => 2], false));
 
         $statement->expects($this->once())
             ->method('rowCount')
@@ -1098,6 +1098,23 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Tests that first can be called on an unbuffered query
+     *
+     * @return void
+     */
+    public function testFirstUnbuffered()
+    {
+        $table = TableRegistry::get('Articles');
+        $query = new Query($this->connection, $table);
+        $query->select(['id']);
+
+        $first = $query->hydrate(false)
+            ->bufferResults(false)->first();
+
+        $this->assertEquals(['id' => 1], $first);
+    }
+
+    /**
      * Testing hydrating a result set into Entity objects
      *
      * @return void