Skip to content

Commit 48eecb5

Browse files
committed
fix: prevent extra query and invalid size in Model::chunk()
1 parent a5fc034 commit 48eecb5

File tree

4 files changed

+66
-1
lines changed

4 files changed

+66
-1
lines changed

system/BaseModel.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,7 @@ abstract public function countAllResults(bool $reset = true, bool $test = false)
582582
* @return void
583583
*
584584
* @throws DataException
585+
* @throws InvalidArgumentException if $size is not a positive integer
585586
*/
586587
abstract public function chunk(int $size, Closure $userFunc);
587588

system/Model.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use CodeIgniter\Database\Exceptions\DataException;
2222
use CodeIgniter\Entity\Entity;
2323
use CodeIgniter\Exceptions\BadMethodCallException;
24+
use CodeIgniter\Exceptions\InvalidArgumentException;
2425
use CodeIgniter\Exceptions\ModelException;
2526
use CodeIgniter\Validation\ValidationInterface;
2627
use Config\Database;
@@ -533,10 +534,14 @@ public function countAllResults(bool $reset = true, bool $test = false)
533534
*/
534535
public function chunk(int $size, Closure $userFunc)
535536
{
537+
if ($size <= 0) {
538+
throw new InvalidArgumentException('chunk() requires a positive integer for the $size argument.');
539+
}
540+
536541
$total = $this->builder()->countAllResults(false);
537542
$offset = 0;
538543

539-
while ($offset <= $total) {
544+
while ($offset < $total) {
540545
$builder = clone $this->builder();
541546
$rows = $builder->get($size, $offset);
542547

tests/system/Models/MiscellaneousModelTest.php

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
namespace CodeIgniter\Models;
1515

1616
use CodeIgniter\Database\Exceptions\DataException;
17+
use CodeIgniter\Events\Events;
18+
use CodeIgniter\Exceptions\InvalidArgumentException;
1719
use CodeIgniter\I18n\Time;
1820
use PHPUnit\Framework\Attributes\Group;
1921
use Tests\Support\Models\EntityModel;
@@ -39,6 +41,62 @@ public function testChunk(): void
3941
$this->assertSame(4, $rowCount);
4042
}
4143

44+
public function testChunkThrowsOnZeroSize(): void
45+
{
46+
$this->expectException(InvalidArgumentException::class);
47+
$this->expectExceptionMessage('chunk() requires a positive integer for the $size argument.');
48+
49+
$this->createModel(UserModel::class)->chunk(0, static function ($row): void {});
50+
}
51+
52+
public function testChunkThrowsOnNegativeSize(): void
53+
{
54+
$this->expectException(InvalidArgumentException::class);
55+
$this->expectExceptionMessage('chunk() requires a positive integer for the $size argument.');
56+
57+
$this->createModel(UserModel::class)->chunk(-1, static function ($row): void {});
58+
}
59+
60+
public function testChunkEarlyExit(): void
61+
{
62+
$rowCount = 0;
63+
64+
$this->createModel(UserModel::class)->chunk(2, static function ($row) use (&$rowCount): bool {
65+
$rowCount++;
66+
67+
return false;
68+
});
69+
70+
$this->assertSame(1, $rowCount);
71+
}
72+
73+
public function testChunkDoesNotRunExtraQuery(): void
74+
{
75+
$queryCount = 0;
76+
$listener = static function () use (&$queryCount): void {
77+
$queryCount++;
78+
};
79+
80+
Events::on('DBQuery', $listener);
81+
$this->createModel(UserModel::class)->chunk(4, static function ($row): void {});
82+
Events::removeListener('DBQuery', $listener);
83+
84+
$this->assertSame(2, $queryCount);
85+
}
86+
87+
public function testChunkEmptyTable(): void
88+
{
89+
$this->db->table('user')->truncate();
90+
91+
$rowCount = 0;
92+
93+
$this->createModel(UserModel::class)->chunk(2, static function ($row) use (&$rowCount): void {
94+
$rowCount++;
95+
});
96+
97+
$this->assertSame(0, $rowCount);
98+
}
99+
42100
public function testCanCreateAndSaveEntityClasses(): void
43101
{
44102
$entity = $this->createModel(EntityModel::class)->where('name', 'Developer')->first();

user_guide_src/source/changelogs/v4.7.1.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ Bugs Fixed
4141
- **ContentSecurityPolicy:** Fixed a bug where custom CSP tags were not removed from generated HTML when CSP was disabled. The method now ensures that all custom CSP tags are removed from the generated HTML.
4242
- **ContentSecurityPolicy:** Fixed a bug where ``generateNonces()`` produces corrupted JSON responses by replacing CSP nonce placeholders with unescaped double quotes. The method now automatically JSON-escapes nonce attributes when the response Content-Type is JSON.
4343
- **Model:** Fixed a bug where ``BaseModel::updateBatch()`` threw an exception when ``updateOnlyChanged`` was ``true`` and the index field value did not change.
44+
- **Model:** Fixed a bug where ``Model::chunk()`` ran an unnecessary extra database query at the end of iteration. ``chunk()`` now also throws ``InvalidArgumentException`` when called with a non-positive chunk size.
4445
- **Session:** Fixed a bug in ``MemcachedHandler`` where the constructor incorrectly threw an exception when ``savePath`` was not empty.
4546
- **Toolbar:** Fixed a bug where the standalone toolbar page loaded from ``?debugbar_time=...`` was not interactive.
4647

0 commit comments

Comments
 (0)