diff --git a/src/Illuminate/Database/Eloquent/Builder.php b/src/Illuminate/Database/Eloquent/Builder.php index d1fb30f6418c..d4bfa84a7704 100755 --- a/src/Illuminate/Database/Eloquent/Builder.php +++ b/src/Illuminate/Database/Eloquent/Builder.php @@ -394,6 +394,8 @@ public function cursor() */ public function chunk($count, callable $callback) { + $this->enforceOrderBy(); + $page = 1; do { @@ -428,6 +430,8 @@ public function chunk($count, callable $callback) */ public function chunkById($count, callable $callback, $column = 'id') { + $this->enforceOrderBy(); + $lastId = 0; do { @@ -458,10 +462,6 @@ public function chunkById($count, callable $callback, $column = 'id') */ public function each(callable $callback, $count = 1000) { - if (empty($this->query->orders) && empty($this->query->unionOrders)) { - $this->orderBy($this->model->getQualifiedKeyName(), 'asc'); - } - return $this->chunk($count, function ($results) use ($callback) { foreach ($results as $key => $value) { if ($callback($value, $key) === false) { @@ -471,6 +471,18 @@ public function each(callable $callback, $count = 1000) }); } + /** + * Add a generic orderBy clause if the query doesn't already have one. + * + * @return void + */ + protected function enforceOrderBy() + { + if (empty($this->query->orders) && empty($this->query->unionOrders)) { + $this->orderBy($this->model->getQualifiedKeyName(), 'asc'); + } + } + /** * Get an array with the values of a given column. * diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index 25ba419c73bd..96db7be4ab67 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -1807,6 +1807,8 @@ public function cursor() */ public function chunk($count, callable $callback) { + $this->enforceOrderBy(); + $page = 1; do { @@ -1842,6 +1844,8 @@ public function chunk($count, callable $callback) */ public function chunkById($count, callable $callback, $column = 'id', $alias = null) { + $this->enforceOrderBy(); + $alias = $alias ?: $column; $lastId = 0; @@ -1871,15 +1875,9 @@ public function chunkById($count, callable $callback, $column = 'id', $alias = n * @param callable $callback * @param int $count * @return bool - * - * @throws \RuntimeException */ public function each(callable $callback, $count = 1000) { - if (empty($this->orders) && empty($this->unionOrders)) { - throw new RuntimeException('You must specify an orderBy clause when using the "each" function.'); - } - return $this->chunk($count, function ($results) use ($callback) { foreach ($results as $key => $value) { if ($callback($value, $key) === false) { @@ -1889,6 +1887,20 @@ public function each(callable $callback, $count = 1000) }); } + /** + * Throw an exception if the query doesn't have an orderBy clause. + * + * @return void + * + * @throws \RuntimeException + */ + protected function enforceOrderBy() + { + if (empty($this->orders) && empty($this->unionOrders)) { + throw new RuntimeException('You must specify an orderBy clause when using this function.'); + } + } + /** * Get an array with the values of a given column. * diff --git a/tests/Database/DatabaseEloquentBuilderTest.php b/tests/Database/DatabaseEloquentBuilderTest.php index bd6722fb9c6e..cd33a2278800 100755 --- a/tests/Database/DatabaseEloquentBuilderTest.php +++ b/tests/Database/DatabaseEloquentBuilderTest.php @@ -157,6 +157,8 @@ public function testValueMethodWithModelNotFound() public function testChunkWithLastChunkComplete() { $builder = m::mock('Illuminate\Database\Eloquent\Builder[forPage,get]', [$this->getMockQueryBuilder()]); + $builder->getQuery()->orders[] = ['column' => 'foobar', 'direction' => 'asc']; + $chunk1 = new Collection(['foo1', 'foo2']); $chunk2 = new Collection(['foo3', 'foo4']); $chunk3 = new Collection([]); @@ -178,6 +180,8 @@ public function testChunkWithLastChunkComplete() public function testChunkWithLastChunkPartial() { $builder = m::mock('Illuminate\Database\Eloquent\Builder[forPage,get]', [$this->getMockQueryBuilder()]); + $builder->getQuery()->orders[] = ['column' => 'foobar', 'direction' => 'asc']; + $chunk1 = new Collection(['foo1', 'foo2']); $chunk2 = new Collection(['foo3']); $builder->shouldReceive('forPage')->once()->with(1, 2)->andReturnSelf(); @@ -196,6 +200,8 @@ public function testChunkWithLastChunkPartial() public function testChunkCanBeStoppedByReturningFalse() { $builder = m::mock('Illuminate\Database\Eloquent\Builder[forPage,get]', [$this->getMockQueryBuilder()]); + $builder->getQuery()->orders[] = ['column' => 'foobar', 'direction' => 'asc']; + $chunk1 = new Collection(['foo1', 'foo2']); $chunk2 = new Collection(['foo3']); $builder->shouldReceive('forPage')->once()->with(1, 2)->andReturnSelf(); @@ -216,6 +222,8 @@ public function testChunkCanBeStoppedByReturningFalse() public function testChunkPaginatesUsingIdWithLastChunkComplete() { $builder = m::mock('Illuminate\Database\Eloquent\Builder[forPageAfterId,get]', [$this->getMockQueryBuilder()]); + $builder->getQuery()->orders[] = ['column' => 'foobar', 'direction' => 'asc']; + $chunk1 = new Collection([(object) ['someIdField' => 1], (object) ['someIdField' => 2]]); $chunk2 = new Collection([(object) ['someIdField' => 10], (object) ['someIdField' => 11]]); $chunk3 = new Collection([]); @@ -237,6 +245,8 @@ public function testChunkPaginatesUsingIdWithLastChunkComplete() public function testChunkPaginatesUsingIdWithLastChunkPartial() { $builder = m::mock('Illuminate\Database\Eloquent\Builder[forPageAfterId,get]', [$this->getMockQueryBuilder()]); + $builder->getQuery()->orders[] = ['column' => 'foobar', 'direction' => 'asc']; + $chunk1 = new Collection([(object) ['someIdField' => 1], (object) ['someIdField' => 2]]); $chunk2 = new Collection([(object) ['someIdField' => 10]]); $builder->shouldReceive('forPageAfterId')->once()->with(2, 0, 'someIdField')->andReturnSelf(); diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index fff1d775f035..184fc991e19e 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -1743,6 +1743,8 @@ public function testTableValuedFunctionAsTableInSqlServer() public function testChunkWithLastChunkComplete() { $builder = $this->getMockQueryBuilder(); + $builder->orders[] = ['column' => 'foobar', 'direction' => 'asc']; + $chunk1 = collect(['foo1', 'foo2']); $chunk2 = collect(['foo3', 'foo4']); $chunk3 = collect([]); @@ -1764,6 +1766,8 @@ public function testChunkWithLastChunkComplete() public function testChunkWithLastChunkPartial() { $builder = $this->getMockQueryBuilder(); + $builder->orders[] = ['column' => 'foobar', 'direction' => 'asc']; + $chunk1 = collect(['foo1', 'foo2']); $chunk2 = collect(['foo3']); $builder->shouldReceive('forPage')->once()->with(1, 2)->andReturnSelf(); @@ -1782,6 +1786,8 @@ public function testChunkWithLastChunkPartial() public function testChunkCanBeStoppedByReturningFalse() { $builder = $this->getMockQueryBuilder(); + $builder->orders[] = ['column' => 'foobar', 'direction' => 'asc']; + $chunk1 = collect(['foo1', 'foo2']); $chunk2 = collect(['foo3']); $builder->shouldReceive('forPage')->once()->with(1, 2)->andReturnSelf(); @@ -1802,6 +1808,8 @@ public function testChunkCanBeStoppedByReturningFalse() public function testChunkPaginatesUsingIdWithLastChunkComplete() { $builder = $this->getMockQueryBuilder(); + $builder->orders[] = ['column' => 'foobar', 'direction' => 'asc']; + $chunk1 = collect([(object) ['someIdField' => 1], (object) ['someIdField' => 2]]); $chunk2 = collect([(object) ['someIdField' => 10], (object) ['someIdField' => 11]]); $chunk3 = collect([]); @@ -1823,6 +1831,8 @@ public function testChunkPaginatesUsingIdWithLastChunkComplete() public function testChunkPaginatesUsingIdWithLastChunkPartial() { $builder = $this->getMockQueryBuilder(); + $builder->orders[] = ['column' => 'foobar', 'direction' => 'asc']; + $chunk1 = collect([(object) ['someIdField' => 1], (object) ['someIdField' => 2]]); $chunk2 = collect([(object) ['someIdField' => 10]]); $builder->shouldReceive('forPageAfterId')->once()->with(2, 0, 'someIdField')->andReturnSelf(); @@ -1841,6 +1851,8 @@ public function testChunkPaginatesUsingIdWithLastChunkPartial() public function testChunkPaginatesUsingIdWithAlias() { $builder = $this->getMockQueryBuilder(); + $builder->orders[] = ['column' => 'foobar', 'direction' => 'asc']; + $chunk1 = collect([(object) ['table_id' => 1], (object) ['table_id' => 10]]); $chunk2 = collect([]); $builder->shouldReceive('forPageAfterId')->once()->with(2, 0, 'table.id')->andReturnSelf();