diff --git a/src/Illuminate/Database/Eloquent/Builder.php b/src/Illuminate/Database/Eloquent/Builder.php index cf269b446d5d..a14805656508 100755 --- a/src/Illuminate/Database/Eloquent/Builder.php +++ b/src/Illuminate/Database/Eloquent/Builder.php @@ -371,9 +371,16 @@ public function cursor() */ public function chunk($count, callable $callback) { - $results = $this->forPage($page = 1, $count)->get(); + $page = 1; + + do { + $results = $this->forPage($page, $count)->get(); + $countResults = $results->count(); + + if ($countResults == 0) { + break; + } - while (! $results->isEmpty()) { // On each chunk result set, we will pass them to the callback and then let the // developer take care of everything within the callback, which allows us to // keep the memory low for spinning through large result sets for working. @@ -382,9 +389,7 @@ public function chunk($count, callable $callback) } $page++; - - $results = $this->forPage($page, $count)->get(); - } + } while ($countResults == $count); return true; } @@ -399,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 ba20e42c68b8..7e44ce9d1b11 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -1807,9 +1807,16 @@ public function cursor() */ public function chunk($count, callable $callback) { - $results = $this->forPage($page = 1, $count)->get(); + $page = 1; + + do { + $results = $this->forPage($page, $count)->get(); + $countResults = $results->count(); + + if ($countResults == 0) { + break; + } - while (! $results->isEmpty()) { // On each chunk result set, we will pass them to the callback and then let the // developer take care of everything within the callback, which allows us to // keep the memory low for spinning through large result sets for working. @@ -1818,9 +1825,7 @@ public function chunk($count, callable $callback) } $page++; - - $results = $this->forPage($page, $count)->get(); - } + } while ($countResults == $count); return true; } @@ -1836,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 0bded7cd104c..188c89d4fbab 100755 --- a/tests/Database/DatabaseEloquentBuilderTest.php +++ b/tests/Database/DatabaseEloquentBuilderTest.php @@ -154,57 +154,75 @@ public function testValueMethodWithModelNotFound() $this->assertNull($builder->value('name')); } - public function testChunkExecuteCallbackOverPaginatedRequest() + public function testChunkWithLastChunkComplete() { $builder = m::mock('Illuminate\Database\Eloquent\Builder[forPage,get]', [$this->getMockQueryBuilder()]); + $chunk1 = new Collection(['foo1', 'foo2']); + $chunk2 = new Collection(['foo3', 'foo4']); + $chunk3 = new Collection([]); $builder->shouldReceive('forPage')->once()->with(1, 2)->andReturn($builder); $builder->shouldReceive('forPage')->once()->with(2, 2)->andReturn($builder); $builder->shouldReceive('forPage')->once()->with(3, 2)->andReturn($builder); - $builder->shouldReceive('get')->times(3)->andReturn(new Collection(['foo1', 'foo2']), new Collection(['foo3']), new Collection([])); + $builder->shouldReceive('get')->times(3)->andReturn($chunk1, $chunk2, $chunk3); $callbackExecutionAssertor = m::mock('StdClass'); - $callbackExecutionAssertor->shouldReceive('doSomething')->with('foo1')->once(); - $callbackExecutionAssertor->shouldReceive('doSomething')->with('foo2')->once(); - $callbackExecutionAssertor->shouldReceive('doSomething')->with('foo3')->once(); + $callbackExecutionAssertor->shouldReceive('doSomething')->with($chunk1)->once(); + $callbackExecutionAssertor->shouldReceive('doSomething')->with($chunk2)->once(); + $callbackExecutionAssertor->shouldReceive('doSomething')->with($chunk3)->never(); $builder->chunk(2, function ($results) use ($callbackExecutionAssertor) { - foreach ($results as $result) { - $callbackExecutionAssertor->doSomething($result); - } + $callbackExecutionAssertor->doSomething($results); + }); + } + + public function testChunkWithLastChunkPartial() + { + $builder = m::mock('Illuminate\Database\Eloquent\Builder[forPage,get]', [$this->getMockQueryBuilder()]); + $chunk1 = new Collection(['foo1', 'foo2']); + $chunk2 = new Collection(['foo3']); + $builder->shouldReceive('forPage')->once()->with(1, 2)->andReturn($builder); + $builder->shouldReceive('forPage')->once()->with(2, 2)->andReturn($builder); + $builder->shouldReceive('get')->times(2)->andReturn($chunk1, $chunk2); + + $callbackExecutionAssertor = m::mock('StdClass'); + $callbackExecutionAssertor->shouldReceive('doSomething')->with($chunk1)->once(); + $callbackExecutionAssertor->shouldReceive('doSomething')->with($chunk2)->once(); + + $builder->chunk(2, function ($results) use ($callbackExecutionAssertor) { + $callbackExecutionAssertor->doSomething($results); }); } public function testChunkCanBeStoppedByReturningFalse() { $builder = m::mock('Illuminate\Database\Eloquent\Builder[forPage,get]', [$this->getMockQueryBuilder()]); + $chunk1 = new Collection(['foo1', 'foo2']); + $chunk2 = new Collection(['foo3']); $builder->shouldReceive('forPage')->once()->with(1, 2)->andReturn($builder); $builder->shouldReceive('forPage')->never()->with(2, 2); - $builder->shouldReceive('get')->times(1)->andReturn(new Collection(['foo1', 'foo2'])); + $builder->shouldReceive('get')->times(1)->andReturn($chunk1); $callbackExecutionAssertor = m::mock('StdClass'); - $callbackExecutionAssertor->shouldReceive('doSomething')->with('foo1')->once(); - $callbackExecutionAssertor->shouldReceive('doSomething')->with('foo2')->once(); - $callbackExecutionAssertor->shouldReceive('doSomething')->with('foo3')->never(); + $callbackExecutionAssertor->shouldReceive('doSomething')->with($chunk1)->once(); + $callbackExecutionAssertor->shouldReceive('doSomething')->with($chunk2)->never(); $builder->chunk(2, function ($results) use ($callbackExecutionAssertor) { - foreach ($results as $result) { - $callbackExecutionAssertor->doSomething($result); - } + $callbackExecutionAssertor->doSomething($results); return false; }); } - 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([]) ); @@ -212,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();