From 08e514c9b116ee427d9320ce9d00443e674c474c Mon Sep 17 00:00:00 2001
From: vlakoff <vlakoff@gmail.com>
Date: Sun, 30 Oct 2016 11:00:32 +0100
Subject: [PATCH] Avoid extraneous query in chunkById() methods as well

---
 src/Illuminate/Database/Eloquent/Builder.php  | 15 ++++++++-----
 src/Illuminate/Database/Query/Builder.php     | 17 ++++++++------
 .../Database/DatabaseEloquentBuilderTest.php  | 21 +++++++++++++++---
 tests/Database/DatabaseQueryBuilderTest.php   | 22 ++++++++++++++++---
 4 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/src/Illuminate/Database/Eloquent/Builder.php b/src/Illuminate/Database/Eloquent/Builder.php
index 3dff643cb20d..a14805656508 100755
--- a/src/Illuminate/Database/Eloquent/Builder.php
+++ b/src/Illuminate/Database/Eloquent/Builder.php
@@ -404,19 +404,22 @@ public function chunk($count, callable $callback)
      */
     public function chunkById($count, callable $callback, $column = 'id')
     {
-        $lastId = null;
+        $lastId = 0;
 
-        $results = $this->forPageAfterId($count, 0, $column)->get();
+        do {
+            $results = $this->forPageAfterId($count, $lastId, $column)->get();
+            $countResults = $results->count();
+
+            if ($countResults == 0) {
+                break;
+            }
 
-        while (! $results->isEmpty()) {
             if (call_user_func($callback, $results) === false) {
                 return false;
             }
 
             $lastId = $results->last()->{$column};
-
-            $results = $this->forPageAfterId($count, $lastId, $column)->get();
-        }
+        } while ($countResults == $count);
 
         return true;
     }
diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php
index a8b04928c80b..7e44ce9d1b11 100755
--- a/src/Illuminate/Database/Query/Builder.php
+++ b/src/Illuminate/Database/Query/Builder.php
@@ -1841,21 +1841,24 @@ public function chunk($count, callable $callback)
      */
     public function chunkById($count, callable $callback, $column = 'id', $alias = null)
     {
-        $lastId = null;
-
         $alias = $alias ?: $column;
 
-        $results = $this->forPageAfterId($count, 0, $column)->get();
+        $lastId = 0;
+
+        do {
+            $results = $this->forPageAfterId($count, $lastId, $column)->get();
+            $countResults = $results->count();
+
+            if ($countResults == 0) {
+                break;
+            }
 
-        while (! $results->isEmpty()) {
             if (call_user_func($callback, $results) === false) {
                 return false;
             }
 
             $lastId = $results->last()->{$alias};
-
-            $results = $this->forPageAfterId($count, $lastId, $column)->get();
-        }
+        } while ($countResults == $count);
 
         return true;
     }
diff --git a/tests/Database/DatabaseEloquentBuilderTest.php b/tests/Database/DatabaseEloquentBuilderTest.php
index f92c3a31fef7..188c89d4fbab 100755
--- a/tests/Database/DatabaseEloquentBuilderTest.php
+++ b/tests/Database/DatabaseEloquentBuilderTest.php
@@ -213,16 +213,16 @@ public function testChunkCanBeStoppedByReturningFalse()
         });
     }
 
-    public function testChunkPaginatesUsingId()
+    public function testChunkPaginatesUsingIdWithLastChunkComplete()
     {
         $builder = m::mock('Illuminate\Database\Eloquent\Builder[forPageAfterId,get]', [$this->getMockQueryBuilder()]);
         $builder->shouldReceive('forPageAfterId')->once()->with(2, 0, 'someIdField')->andReturn($builder);
         $builder->shouldReceive('forPageAfterId')->once()->with(2, 2, 'someIdField')->andReturn($builder);
-        $builder->shouldReceive('forPageAfterId')->once()->with(2, 10, 'someIdField')->andReturn($builder);
+        $builder->shouldReceive('forPageAfterId')->once()->with(2, 11, 'someIdField')->andReturn($builder);
 
         $builder->shouldReceive('get')->times(3)->andReturn(
             new Collection([(object) ['someIdField' => 1], (object) ['someIdField' => 2]]),
-            new Collection([(object) ['someIdField' => 10]]),
+            new Collection([(object) ['someIdField' => 10], (object) ['someIdField' => 11]]),
             new Collection([])
         );
 
@@ -230,6 +230,21 @@ public function testChunkPaginatesUsingId()
         }, 'someIdField');
     }
 
+    public function testChunkPaginatesUsingIdWithLastChunkPartial()
+    {
+        $builder = m::mock('Illuminate\Database\Eloquent\Builder[forPageAfterId,get]', [$this->getMockQueryBuilder()]);
+        $builder->shouldReceive('forPageAfterId')->once()->with(2, 0, 'someIdField')->andReturn($builder);
+        $builder->shouldReceive('forPageAfterId')->once()->with(2, 2, 'someIdField')->andReturn($builder);
+
+        $builder->shouldReceive('get')->times(2)->andReturn(
+            new Collection([(object) ['someIdField' => 1], (object) ['someIdField' => 2]]),
+            new Collection([(object) ['someIdField' => 10]])
+        );
+
+        $builder->chunkById(2, function ($results) {
+        }, 'someIdField');
+    }
+
     public function testPluckReturnsTheMutatedAttributesOfAModel()
     {
         $builder = $this->getBuilder();
diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php
index 82bb4aca20f9..11a3bac4da5d 100755
--- a/tests/Database/DatabaseQueryBuilderTest.php
+++ b/tests/Database/DatabaseQueryBuilderTest.php
@@ -1734,17 +1734,17 @@ public function testTableValuedFunctionAsTableInSqlServer()
         $this->assertEquals('select * from [users](1,2)', $builder->toSql());
     }
 
-    public function testChunkPaginatesUsingId()
+    public function testChunkPaginatesUsingIdWithLastChunkComplete()
     {
         $builder = $this->getMockQueryBuilder();
 
         $builder->shouldReceive('forPageAfterId')->once()->with(2, 0, 'someIdField')->andReturn($builder);
         $builder->shouldReceive('forPageAfterId')->once()->with(2, 2, 'someIdField')->andReturn($builder);
-        $builder->shouldReceive('forPageAfterId')->once()->with(2, 10, 'someIdField')->andReturn($builder);
+        $builder->shouldReceive('forPageAfterId')->once()->with(2, 11, 'someIdField')->andReturn($builder);
 
         $builder->shouldReceive('get')->times(3)->andReturn(
             collect([(object) ['someIdField' => 1], (object) ['someIdField' => 2]]),
-            collect([(object) ['someIdField' => 10]]),
+            collect([(object) ['someIdField' => 10], (object) ['someIdField' => 11]]),
             collect([])
         );
 
@@ -1752,6 +1752,22 @@ public function testChunkPaginatesUsingId()
         }, 'someIdField');
     }
 
+    public function testChunkPaginatesUsingIdWithLastChunkPartial()
+    {
+        $builder = $this->getMockQueryBuilder();
+
+        $builder->shouldReceive('forPageAfterId')->once()->with(2, 0, 'someIdField')->andReturn($builder);
+        $builder->shouldReceive('forPageAfterId')->once()->with(2, 2, 'someIdField')->andReturn($builder);
+
+        $builder->shouldReceive('get')->times(2)->andReturn(
+            collect([(object) ['someIdField' => 1], (object) ['someIdField' => 2]]),
+            collect([(object) ['someIdField' => 10]])
+        );
+
+        $builder->chunkById(2, function ($results) {
+        }, 'someIdField');
+    }
+
     public function testChunkPaginatesUsingIdWithAlias()
     {
         $builder = $this->getMockQueryBuilder();