From 96e7914f232e0366668be861bcb82ea670308f2a Mon Sep 17 00:00:00 2001
From: Sam Minnee <sam@silverstripe.com>
Date: Fri, 28 Jun 2019 10:57:51 +1200
Subject: [PATCH 1/2] FIX: Fix MySQLQuery::seek() and Query::rewind() to fix
 repeated iteration API: Query::seek() and Query::rewind() no longer return a
 value.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Although breaking an API inside a patch release may seem odd, this in
fact is correcting a long-standing bug in our implementation of
Iterator::rewind(), so I think it’s appropriate.

https://github.com/silverstripe/silverstripe-framework/issues/9097
---
 src/ORM/Connect/MySQLQuery.php     |  5 ++++-
 src/ORM/Connect/MySQLStatement.php |  8 ++++++--
 src/ORM/Connect/Query.php          |  5 ++---
 tests/php/ORM/DatabaseTest.php     | 21 +++++++++++++++++++++
 4 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/src/ORM/Connect/MySQLQuery.php b/src/ORM/Connect/MySQLQuery.php
index 22f11a3eb..f1dbfefd2 100644
--- a/src/ORM/Connect/MySQLQuery.php
+++ b/src/ORM/Connect/MySQLQuery.php
@@ -48,8 +48,11 @@ class MySQLQuery extends Query
     public function seek($row)
     {
         if (is_object($this->handle)) {
+            // Fix for https://github.com/silverstripe/silverstripe-framework/issues/9097 without breaking the seek() API
             $this->handle->data_seek($row);
-            return $this->nextRecord();
+            $result = $this->nextRecord();
+            $this->handle->data_seek($row);
+            return $result;
         }
         return null;
     }
diff --git a/src/ORM/Connect/MySQLStatement.php b/src/ORM/Connect/MySQLStatement.php
index 9c7fdce26..f7e191133 100644
--- a/src/ORM/Connect/MySQLStatement.php
+++ b/src/ORM/Connect/MySQLStatement.php
@@ -105,8 +105,12 @@ class MySQLStatement extends Query
     public function seek($row)
     {
         $this->rowNum = $row - 1;
+
+        // Fix for https://github.com/silverstripe/silverstripe-framework/issues/9097 without breaking the seek() API
         $this->statement->data_seek($row);
-        return $this->next();
+        $result = $this->next();
+        $this->statement->data_seek($row);
+        return $result;
     }
 
     public function numRecords()
@@ -135,6 +139,6 @@ class MySQLStatement extends Query
 
     public function rewind()
     {
-        return $this->seek(0);
+        $this->seek(0);
     }
 }
diff --git a/src/ORM/Connect/Query.php b/src/ORM/Connect/Query.php
index 1f994dd7f..b1c3d4247 100644
--- a/src/ORM/Connect/Query.php
+++ b/src/ORM/Connect/Query.php
@@ -168,16 +168,15 @@ abstract class Query implements Iterator
      * Iterator function implementation. Rewind the iterator to the first item and return it.
      * Makes use of {@link seek()} and {@link numRecords()}, takes care of the plumbing.
      *
-     * @return array
+     * @return void
      */
     public function rewind()
     {
         if ($this->queryHasBegun && $this->numRecords() > 0) {
             $this->queryHasBegun = false;
             $this->currentRecord = null;
-            return $this->seek(0);
+            $this->seek(0);
         }
-        return null;
     }
 
     /**
diff --git a/tests/php/ORM/DatabaseTest.php b/tests/php/ORM/DatabaseTest.php
index babcdb402..607515613 100644
--- a/tests/php/ORM/DatabaseTest.php
+++ b/tests/php/ORM/DatabaseTest.php
@@ -272,4 +272,25 @@ class DatabaseTest extends SapphireTest
         $result = DB::query('SELECT TRUE')->first();
         $this->assertInternalType('int', reset($result));
     }
+
+    /**
+     * Test that repeated iteration of a query returns all records.
+     * See https://github.com/silverstripe/silverstripe-framework/issues/9097
+     */
+    public function testRepeatedIteration()
+    {
+        $inputData = ['one', 'two', 'three', 'four'];
+
+        foreach ($inputData as $i => $text) {
+            $x = new MyObject();
+            $x->MyField = $text;
+            $x->MyInt = $i;
+            $x->write();
+        }
+
+        $query = DB::query('SELECT "MyInt", "MyField" FROM "DatabaseTest_MyObject" ORDER BY "MyInt"');
+
+        $this->assertEquals($inputData, $query->map());
+        $this->assertEquals($inputData, $query->map());
+    }
 }

From 404366909ebaa74a4f47dbc792492c361b701522 Mon Sep 17 00:00:00 2001
From: Sam Minnee <sam@silverstripe.com>
Date: Mon, 1 Jul 2019 10:18:33 +1200
Subject: [PATCH 2/2] FIX: Fix MysqlStatement::rewind()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Its implementation is more naive than Query’s and leads to unnecessary
seek()ing. This causes issues with the previous commit.
---
 src/ORM/Connect/MySQLStatement.php | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/src/ORM/Connect/MySQLStatement.php b/src/ORM/Connect/MySQLStatement.php
index f7e191133..ca2cdbd32 100644
--- a/src/ORM/Connect/MySQLStatement.php
+++ b/src/ORM/Connect/MySQLStatement.php
@@ -136,9 +136,4 @@ class MySQLStatement extends Query
         }
         return $row;
     }
-
-    public function rewind()
-    {
-        $this->seek(0);
-    }
 }