From 818ca89dcea7a89f8f7a989f6c98bb75d36b3a51 Mon Sep 17 00:00:00 2001 From: michalsn Date: Fri, 23 May 2025 14:43:29 +0200 Subject: [PATCH] fix: use native PHP truthiness for condition evaluation in when()/whenNot() --- system/Traits/ConditionalTrait.php | 4 +- tests/system/Database/Builder/WhenTest.php | 58 +++++++++++++++++++ user_guide_src/source/changelogs/v4.6.2.rst | 1 + .../source/database/query_builder.rst | 7 ++- 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/system/Traits/ConditionalTrait.php b/system/Traits/ConditionalTrait.php index a778e9375a36..8f9d4a91f3a8 100644 --- a/system/Traits/ConditionalTrait.php +++ b/system/Traits/ConditionalTrait.php @@ -29,7 +29,7 @@ trait ConditionalTrait */ public function when($condition, callable $callback, ?callable $defaultCallback = null): self { - if ($condition !== '' && $condition !== false && $condition !== null) { + if ((bool) $condition) { $callback($this, $condition); } elseif ($defaultCallback !== null) { $defaultCallback($this); @@ -52,7 +52,7 @@ public function when($condition, callable $callback, ?callable $defaultCallback */ public function whenNot($condition, callable $callback, ?callable $defaultCallback = null): self { - if ($condition === '' || $condition === null || $condition === false || $condition === '0') { + if (! (bool) $condition) { $callback($this, $condition); } elseif ($defaultCallback !== null) { $defaultCallback($this); diff --git a/tests/system/Database/Builder/WhenTest.php b/tests/system/Database/Builder/WhenTest.php index a7fede7879d3..87f9b3ff233d 100644 --- a/tests/system/Database/Builder/WhenTest.php +++ b/tests/system/Database/Builder/WhenTest.php @@ -15,7 +15,9 @@ use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\Mock\MockConnection; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; +use stdClass; /** * @internal @@ -101,6 +103,23 @@ public function testWhenPassesParemeters(): void $this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->getCompiledSelect())); } + #[DataProvider('provideConditionValues')] + public function testWhenRunsDefaultCallbackBasedOnCondition(mixed $condition, bool $expectDefault): void + { + $builder = $this->db->table('jobs'); + + $builder = $builder->when($condition, static function ($query): void { + $query->select('id'); + }, static function ($query): void { + $query->select('name'); + }); + + $expected = $expectDefault ? 'name' : 'id'; + $expectedSQL = 'SELECT "' . $expected . '" FROM "jobs"'; + + $this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->getCompiledSelect())); + } + public function testWhenNotFalse(): void { $builder = $this->db->table('jobs'); @@ -166,4 +185,43 @@ public function testWhenNotPassesParemeters(): void $expectedSQL = 'SELECT * FROM "jobs" WHERE "name" = \'0\''; $this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->getCompiledSelect())); } + + #[DataProvider('provideConditionValues')] + public function testWhenNotRunsDefaultCallbackBasedOnCondition(mixed $condition, bool $expectDefault): void + { + $builder = $this->db->table('jobs'); + + $builder = $builder->whenNot($condition, static function ($query): void { + $query->select('id'); + }, static function ($query): void { + $query->select('name'); + }); + + $expected = $expectDefault ? 'id' : 'name'; + $expectedSQL = 'SELECT "' . $expected . '" FROM "jobs"'; + + $this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->getCompiledSelect())); + } + + /** + * @return array + */ + public static function provideConditionValues(): array + { + return [ + 'false' => [false, true], // [condition, expectedDefaultCallbackRuns] + 'int 0' => [0, true], + 'float 0.0' => [0.0, true], + 'empty string' => ['', true], + 'string 0' => ['0', true], + 'empty array' => [[], true], + 'null' => [null, true], + 'true' => [true, false], + 'int 1' => [1, false], + 'float 1.1' => [1.1, false], + 'non-empty string' => ['foo', false], + 'non-empty array' => [[1], false], + 'object' => [new stdClass(), false], + ]; + } } diff --git a/user_guide_src/source/changelogs/v4.6.2.rst b/user_guide_src/source/changelogs/v4.6.2.rst index 089fb01e1514..8be89ae72b32 100644 --- a/user_guide_src/source/changelogs/v4.6.2.rst +++ b/user_guide_src/source/changelogs/v4.6.2.rst @@ -35,6 +35,7 @@ Deprecations Bugs Fixed ********** +- **Database:** Fixed a bug where ``when()`` and ``whenNot()`` in ``ConditionalTrait`` incorrectly evaluated certain falsy values (such as ``[]``, ``0``, ``0.0``, and ``'0'``) as truthy, causing callbacks to be executed unexpectedly. These methods now cast the condition to a boolean using ``(bool)`` to ensure consistent behavior with PHP's native truthiness. - **Security:** Fixed a bug where the ``sanitize_filename()`` function from the Security helper would throw an error when used in CLI requests. See the repo's diff --git a/user_guide_src/source/database/query_builder.rst b/user_guide_src/source/database/query_builder.rst index 31515768d530..e901869fb5f7 100644 --- a/user_guide_src/source/database/query_builder.rst +++ b/user_guide_src/source/database/query_builder.rst @@ -1272,9 +1272,10 @@ $builder->when() .. versionadded:: 4.3.0 This allows modifying the query based on a condition without breaking out of the -query builder chain. The first parameter is the condition, and it should evaluate -to a boolean. The second parameter is a callable that will be ran -when the condition is true. +query builder chain. The first parameter is the condition, and it is evaluated +using PHP's native boolean logic - meaning that values like ``false``, ``null``, +``0``, ``'0'``, ``0.0``, empty string ``''`` and empty array ``[]`` will be considered false. +The second parameter is a callable that will be ran when the condition is true. For example, you might only want to apply a given WHERE statement based on the value sent within an HTTP request: