From 2ec6e002dd0574ea23855aa584c45dd687c7d2bd Mon Sep 17 00:00:00 2001 From: michalsn Date: Sun, 4 Jul 2021 10:20:59 +0200 Subject: [PATCH 1/3] Fix bug with handling boolean values in set() method in BaseBuilder and Model class --- system/Database/BaseBuilder.php | 4 +- system/Database/SQLSRV/Forge.php | 4 + system/Database/SQLite3/Forge.php | 4 + system/Model.php | 8 +- .../20160428212500_Create_test_tables.php | 1 + .../_support/Database/Seeds/CITestSeeder.php | 4 +- tests/system/Database/Builder/UpdateTest.php | 74 +++++++++++++++++++ tests/system/Database/Live/UpdateTest.php | 19 +++++ .../source/installation/upgrade_420.rst | 12 +++ 9 files changed, 123 insertions(+), 7 deletions(-) create mode 100644 user_guide_src/source/installation/upgrade_420.rst diff --git a/system/Database/BaseBuilder.php b/system/Database/BaseBuilder.php index 71bb38a03d1d..67eae76d05fa 100644 --- a/system/Database/BaseBuilder.php +++ b/system/Database/BaseBuilder.php @@ -1673,12 +1673,12 @@ protected function _limit(string $sql, bool $offsetIgnore = false): string * Allows key/value pairs to be set for insert(), update() or replace(). * * @param string|array|object $key Field name, or an array of field/value pairs - * @param string $value Field value, if $key is a single field + * @param mixed $value Field value, if $key is a single field * @param bool $escape Whether to escape values and identifiers * * @return $this */ - public function set($key, ?string $value = '', bool $escape = null) + public function set($key, $value = '', bool $escape = null) { $key = $this->objectToArray($key); diff --git a/system/Database/SQLSRV/Forge.php b/system/Database/SQLSRV/Forge.php index 185bb1663c63..28545d567427 100755 --- a/system/Database/SQLSRV/Forge.php +++ b/system/Database/SQLSRV/Forge.php @@ -401,6 +401,10 @@ protected function _attributeType(array &$attributes) $attributes['TYPE'] = 'DATETIME'; break; + case 'BOOLEAN': + $attributes['TYPE'] = 'BIT'; + break; + default: break; } diff --git a/system/Database/SQLite3/Forge.php b/system/Database/SQLite3/Forge.php index 3edc293ed481..598483f192b0 100644 --- a/system/Database/SQLite3/Forge.php +++ b/system/Database/SQLite3/Forge.php @@ -233,6 +233,10 @@ protected function _attributeType(array &$attributes) $attributes['TYPE'] = 'TEXT'; break; + case 'BOOLEAN': + $attributes['TYPE'] = 'INT'; + break; + default: break; } diff --git a/system/Model.php b/system/Model.php index 1994373351b2..6845a3f63698 100644 --- a/system/Model.php +++ b/system/Model.php @@ -579,13 +579,13 @@ public function builder(?string $table = null) * data here. This allows it to be used with any of the other * builder methods and still get validated data, like replace. * - * @param mixed $key Field name, or an array of field/value pairs - * @param string|null $value Field value, if $key is a single field - * @param bool|null $escape Whether to escape values and identifiers + * @param mixed $key Field name, or an array of field/value pairs + * @param mixed $value Field value, if $key is a single field + * @param bool|null $escape Whether to escape values and identifiers * * @return $this */ - public function set($key, ?string $value = '', ?bool $escape = null) + public function set($key, $value = '', ?bool $escape = null) { $data = is_array($key) ? $key : [$key => $value]; diff --git a/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php b/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php index 629550cbfbdd..5ef2a009bafa 100644 --- a/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php +++ b/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php @@ -61,6 +61,7 @@ public function up() 'type_double' => ['type' => 'DOUBLE', 'null' => true], 'type_decimal' => ['type' => 'DECIMAL', 'constraint' => '18,4', 'null' => true], 'type_blob' => ['type' => 'BLOB', 'null' => true], + 'type_boolean' => ['type' => 'BOOLEAN', 'null' => true], ]; if ($this->db->DBDriver === 'Postgre') { diff --git a/tests/_support/Database/Seeds/CITestSeeder.php b/tests/_support/Database/Seeds/CITestSeeder.php index e633543414f7..e7cd626efdda 100644 --- a/tests/_support/Database/Seeds/CITestSeeder.php +++ b/tests/_support/Database/Seeds/CITestSeeder.php @@ -97,6 +97,7 @@ public function run() 'type_datetime' => '2020-06-18T05:12:24.000+02:00', 'type_timestamp' => '2019-07-18T21:53:21.000+02:00', 'type_bigint' => 2342342, + 'type_boolean' => 1, ], ], ]; @@ -110,7 +111,8 @@ public function run() } if ($this->db->DBDriver === 'Postgre') { - $data['type_test'][0]['type_time'] = '15:22:00'; + $data['type_test'][0]['type_time'] = '15:22:00'; + $data['type_test'][0]['type_boolean'] = true; unset( $data['type_test'][0]['type_enum'], $data['type_test'][0]['type_set'], diff --git a/tests/system/Database/Builder/UpdateTest.php b/tests/system/Database/Builder/UpdateTest.php index ea2fdf8412c2..e1a527ec6f0f 100644 --- a/tests/system/Database/Builder/UpdateTest.php +++ b/tests/system/Database/Builder/UpdateTest.php @@ -85,6 +85,80 @@ public function testUpdateWithSet() $this->assertSame($expectedBinds, $builder->getBinds()); } + public function testUpdateWithSetAsInt() + { + $builder = new BaseBuilder('jobs', $this->db); + + $builder->testMode()->set('age', 22)->where('id', 1)->update(null, null, null); + + $expectedSQL = 'UPDATE "jobs" SET "age" = 22 WHERE "id" = 1'; + $expectedBinds = [ + 'age' => [ + 22, + true, + ], + 'id' => [ + 1, + true, + ], + ]; + + $this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->getCompiledUpdate())); + $this->assertSame($expectedBinds, $builder->getBinds()); + } + + public function testUpdateWithSetAsBoolean() + { + $builder = new BaseBuilder('jobs', $this->db); + + $builder->testMode()->set('manager', true)->where('id', 1)->update(null, null, null); + + $expectedSQL = 'UPDATE "jobs" SET "manager" = 1 WHERE "id" = 1'; + $expectedBinds = [ + 'manager' => [ + true, + true, + ], + 'id' => [ + 1, + true, + ], + ]; + + $this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->getCompiledUpdate())); + $this->assertSame($expectedBinds, $builder->getBinds()); + } + + public function testUpdateWithSetAsArray() + { + $builder = new BaseBuilder('jobs', $this->db); + + $builder->testMode()->set(['name' => 'Programmer', 'age' => 22, 'manager' => true])->where('id', 1)->update(null, null, null); + + $expectedSQL = 'UPDATE "jobs" SET "name" = \'Programmer\', "age" = 22, "manager" = 1 WHERE "id" = 1'; + $expectedBinds = [ + 'name' => [ + 'Programmer', + true, + ], + 'age' => [ + 22, + true, + ], + 'manager' => [ + true, + true, + ], + 'id' => [ + 1, + true, + ], + ]; + + $this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->getCompiledUpdate())); + $this->assertSame($expectedBinds, $builder->getBinds()); + } + public function testUpdateThrowsExceptionWithNoData() { $builder = new BaseBuilder('jobs', $this->db); diff --git a/tests/system/Database/Live/UpdateTest.php b/tests/system/Database/Live/UpdateTest.php index f7c94d96dea9..334d6f6c5d59 100644 --- a/tests/system/Database/Live/UpdateTest.php +++ b/tests/system/Database/Live/UpdateTest.php @@ -230,4 +230,23 @@ public function testSetWithoutEscape() 'description' => 'Developer', ]); } + + public function testSetWithBoolean() + { + $this->db->table('type_test') + ->set('type_boolean', false) + ->update(); + + $this->seeInDatabase('type_test', [ + 'type_boolean' => false, + ]); + + $this->db->table('type_test') + ->set('type_boolean', true) + ->update(); + + $this->seeInDatabase('type_test', [ + 'type_boolean' => true, + ]); + } } diff --git a/user_guide_src/source/installation/upgrade_420.rst b/user_guide_src/source/installation/upgrade_420.rst new file mode 100644 index 000000000000..a306b4978347 --- /dev/null +++ b/user_guide_src/source/installation/upgrade_420.rst @@ -0,0 +1,12 @@ +############################# +Upgrading from 4.1.3 to 4.2.0 +############################# + +**Changes for set() method in BaseBuilder and Model class** + +The casting for the ``$value`` parameter has been removed to fix a bug where passing parameters as array and string +to the ``set()`` method were handled differently. If you extended the ``BaseBuilder`` class or ``Model`` class yourself +and modified the ``set()`` method, then you need to change its definition from +``public function set($key, ?string $value = '', ?bool $escape = null)`` to +``public function set($key, $value = '', ?bool $escape = null)``. + From 02ba1d61f81340c9a755c02efc803546c7ef0eac Mon Sep 17 00:00:00 2001 From: michalsn Date: Sun, 4 Jul 2021 10:31:25 +0200 Subject: [PATCH 2/3] Update method definition in the user guide --- user_guide_src/source/database/query_builder.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user_guide_src/source/database/query_builder.rst b/user_guide_src/source/database/query_builder.rst index add336c51081..411abf002f34 100755 --- a/user_guide_src/source/database/query_builder.rst +++ b/user_guide_src/source/database/query_builder.rst @@ -1727,7 +1727,7 @@ Class Reference .. php:method:: set($key[, $value = ''[, $escape = null]]) :param mixed $key: Field name, or an array of field/value pairs - :param string $value: Field value, if $key is a single field + :param mixed $value: Field value, if $key is a single field :param bool $escape: Whether to escape values and identifiers :returns: ``BaseBuilder`` instance (method chaining) :rtype: ``BaseBuilder`` From 4cdaca646f23e0a2e703166d1d6efed9f17c1a9b Mon Sep 17 00:00:00 2001 From: michalsn Date: Fri, 23 Jul 2021 09:53:19 +0200 Subject: [PATCH 3/3] add to toctree --- user_guide_src/source/changelogs/index.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/user_guide_src/source/changelogs/index.rst b/user_guide_src/source/changelogs/index.rst index 038d259e4189..a997a57f0c3f 100644 --- a/user_guide_src/source/changelogs/index.rst +++ b/user_guide_src/source/changelogs/index.rst @@ -12,6 +12,7 @@ See all the changes. .. toctree:: :titlesonly: + v4.2.0 v4.1.4 v4.1.3 v4.1.2