fix: prevent extra query and invalid size in Model::chunk() (#9961)

Author: michalsn

system/BaseModel.php

@@ -582,6 +582,7 @@ abstract public function countAllResults(bool $reset = true, bool $test = false)
      * @return void
      *
      * @throws DataException
+     * @throws InvalidArgumentException if $size is not a positive integer
      */
     abstract public function chunk(int $size, Closure $userFunc);
 

system/Model.php

@@ -21,6 +21,7 @@
 use CodeIgniter\Database\Exceptions\DataException;
 use CodeIgniter\Entity\Entity;
 use CodeIgniter\Exceptions\BadMethodCallException;
+use CodeIgniter\Exceptions\InvalidArgumentException;
 use CodeIgniter\Exceptions\ModelException;
 use CodeIgniter\Validation\ValidationInterface;
 use Config\Database;
@@ -533,10 +534,14 @@ public function countAllResults(bool $reset = true, bool $test = false)
      */
     public function chunk(int $size, Closure $userFunc)
     {
+        if ($size <= 0) {
+            throw new InvalidArgumentException('chunk() requires a positive integer for the $size argument.');
+        }
+
         $total  = $this->builder()->countAllResults(false);
         $offset = 0;
 
-        while ($offset <= $total) {
+        while ($offset < $total) {
             $builder = clone $this->builder();
             $rows    = $builder->get($size, $offset);
 

tests/system/Models/MiscellaneousModelTest.php

@@ -14,6 +14,8 @@
 namespace CodeIgniter\Models;
 
 use CodeIgniter\Database\Exceptions\DataException;
+use CodeIgniter\Events\Events;
+use CodeIgniter\Exceptions\InvalidArgumentException;
 use CodeIgniter\I18n\Time;
 use PHPUnit\Framework\Attributes\Group;
 use Tests\Support\Models\EntityModel;
@@ -39,6 +41,62 @@ public function testChunk(): void
         $this->assertSame(4, $rowCount);
     }
 
+    public function testChunkThrowsOnZeroSize(): void
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('chunk() requires a positive integer for the $size argument.');
+
+        $this->createModel(UserModel::class)->chunk(0, static function ($row): void {});
+    }
+
+    public function testChunkThrowsOnNegativeSize(): void
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('chunk() requires a positive integer for the $size argument.');
+
+        $this->createModel(UserModel::class)->chunk(-1, static function ($row): void {});
+    }
+
+    public function testChunkEarlyExit(): void
+    {
+        $rowCount = 0;
+
+        $this->createModel(UserModel::class)->chunk(2, static function ($row) use (&$rowCount): bool {
+            $rowCount++;
+
+            return false;
+        });
+
+        $this->assertSame(1, $rowCount);
+    }
+
+    public function testChunkDoesNotRunExtraQuery(): void
+    {
+        $queryCount = 0;
+        $listener   = static function () use (&$queryCount): void {
+            $queryCount++;
+        };
+
+        Events::on('DBQuery', $listener);
+        $this->createModel(UserModel::class)->chunk(4, static function ($row): void {});
+        Events::removeListener('DBQuery', $listener);
+
+        $this->assertSame(2, $queryCount);
+    }
+
+    public function testChunkEmptyTable(): void
+    {
+        $this->db->table('user')->truncate();
+
+        $rowCount = 0;
+
+        $this->createModel(UserModel::class)->chunk(2, static function ($row) use (&$rowCount): void {
+            $rowCount++;
+        });
+
+        $this->assertSame(0, $rowCount);
+    }
+
     public function testCanCreateAndSaveEntityClasses(): void
     {
         $entity = $this->createModel(EntityModel::class)->where('name', 'Developer')->first();

user_guide_src/source/changelogs/v4.7.1.rst

@@ -43,6 +43,7 @@ Bugs Fixed
 - **Database:** Fixed a bug where ``BaseConnection::callFunction()`` could double-prefix already-prefixed function names.
 - **Database:** Fixed a bug where ``BasePreparedQuery::prepare()`` could mis-handle SQL containing colon syntax by over-broad named-placeholder replacement. It now preserves PostgreSQL cast syntax like ``::timestamp``.
 - **Model:** Fixed a bug where ``BaseModel::updateBatch()`` threw an exception when ``updateOnlyChanged`` was ``true`` and the index field value did not change.
+- **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.
 - **Session:** Fixed a bug in ``MemcachedHandler`` where the constructor incorrectly threw an exception when ``savePath`` was not empty.
 - **Toolbar:** Fixed a bug where the standalone toolbar page loaded from ``?debugbar_time=...`` was not interactive.
 - **Toolbar:** Fixed a bug in the Routes panel where only the first route parameter was converted to an input field on hover.