From 9c0e904fbe9d1f373d6c4208db65345862679ffb Mon Sep 17 00:00:00 2001 From: michalsn Date: Sat, 4 Jul 2020 19:32:50 +0200 Subject: [PATCH 1/3] Fix results in Model class --- system/Model.php | 14 ++-- tests/system/Database/Live/ModelTest.php | 84 +++++++++++++++++++++++- 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/system/Model.php b/system/Model.php index dda2b6f400cb..7afcd2d01193 100644 --- a/system/Model.php +++ b/system/Model.php @@ -555,8 +555,12 @@ public function save($data): bool else { $response = $this->insert($data, false); - // call insert directly if you want the ID or the record object - if ($response !== false) + + if ($response instanceof BaseResult) + { + $response = $response->resultID; + } + elseif ($response !== false) { $response = true; } @@ -658,7 +662,7 @@ public function getInsertID(): int * @param array|object $data * @param boolean $returnID Whether insert ID should be returned or not. * - * @return integer|string|boolean + * @return BaseResult|integer|string|false * @throws \ReflectionException */ public function insert($data = null, bool $returnID = true) @@ -734,7 +738,7 @@ public function insert($data = null, bool $returnID = true) ->insert(); // If insertion succeeded then save the insert ID - if ($result) + if ($result->resultID) { $this->insertID = $this->db->insertID(); } @@ -912,7 +916,7 @@ public function updateBatch(array $set = null, string $index = null, int $batchS * @param integer|string|array|null $id The rows primary key(s) * @param boolean $purge Allows overriding the soft deletes setting. * - * @return mixed + * @return BaseResult|boolean * @throws \CodeIgniter\Database\Exceptions\DatabaseException */ public function delete($id = null, bool $purge = false) diff --git a/tests/system/Database/Live/ModelTest.php b/tests/system/Database/Live/ModelTest.php index e81858875f08..54e270e667c4 100644 --- a/tests/system/Database/Live/ModelTest.php +++ b/tests/system/Database/Live/ModelTest.php @@ -470,13 +470,30 @@ public function testDeleteBasics() $this->seeInDatabase('job', ['name' => 'Developer']); - $model->delete(1); + $result = $model->delete(1); + $this->assertTrue($result->resultID); $this->dontSeeInDatabase('job', ['name' => 'Developer']); } //-------------------------------------------------------------------- + public function testDeleteFail() + { + $this->setPrivateProperty($this->db, 'DBDebug', false); + + $model = new JobModel(); + + $this->seeInDatabase('job', ['name' => 'Developer']); + + $result = $model->where('name123', 'Developer')->delete(); + $this->assertFalse($result->resultID); + + $this->seeInDatabase('job', ['name' => 'Developer']); + } + + //-------------------------------------------------------------------- + public function testDeleteStringPrimaryKey() { $model = new StringifyPkeyModel(); @@ -496,13 +513,30 @@ public function testDeleteWithSoftDeletes() $this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NULL' => null]); - $model->delete(1); + $result = $model->delete(1); + $this->assertTrue($result); $this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NOT NULL' => null]); } //-------------------------------------------------------------------- + public function testDeleteWithSoftDeleteFail() + { + $this->setPrivateProperty($this->db, 'DBDebug', false); + + $model = new UserModel(); + + $this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NULL' => null]); + + $result = $model->where('name123', 'Derek Jones')->delete(); + $this->assertFalse($result); + + $this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NULL' => null]); + } + + //-------------------------------------------------------------------- + public function testDeleteWithSoftDeletesPurge() { $model = new UserModel(); @@ -1456,6 +1490,52 @@ public function testInsertID() //-------------------------------------------------------------------- + public function testInsertResult() + { + $model = new JobModel(); + + $data = [ + 'name' => 'Apprentice', + 'description' => 'That thing you do.', + ]; + + $result = $model->protect(false) + ->insert($data, false); + + $this->assertTrue($result->resultID); + + $lastInsertId = $model->getInsertID(); + + $this->seeInDatabase('job', ['id' => $lastInsertId]); + } + + //-------------------------------------------------------------------- + + public function testInsertResultFail() + { + $this->setPrivateProperty($this->db, 'DBDebug', false); + + $model = new JobModel(); + + $data = [ + 'name123' => 'Apprentice', + 'description' => 'That thing you do.', + ]; + + $result = $model->protect(false) + ->insert($data, false); + + $this->assertFalse($result->resultID); + + $lastInsertId = $model->getInsertID(); + + $this->assertEquals(0, $lastInsertId); + + $this->dontSeeInDatabase('job', ['id' => $lastInsertId]); + } + + //-------------------------------------------------------------------- + public function testSetTable() { $model = new JobModel(); From a050c89a662eb9649b2e154472eab9eb652b7c0a Mon Sep 17 00:00:00 2001 From: michalsn Date: Sun, 5 Jul 2020 07:39:42 +0200 Subject: [PATCH 2/3] Fix resultID check --- system/Database/BaseBuilder.php | 2 +- system/Model.php | 2 +- tests/system/Database/Live/ModelTest.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/system/Database/BaseBuilder.php b/system/Database/BaseBuilder.php index ff3b16bf3c37..03988b6d9cd3 100644 --- a/system/Database/BaseBuilder.php +++ b/system/Database/BaseBuilder.php @@ -2454,7 +2454,7 @@ public function update(array $set = null, $where = null, int $limit = null): boo $result = $this->db->query($sql, $this->binds, false); - if ($result->resultID) + if ($result->resultID !== false) { // Clear our binds so we don't eat up memory $this->binds = []; diff --git a/system/Model.php b/system/Model.php index af8263256aec..f59276451fc9 100644 --- a/system/Model.php +++ b/system/Model.php @@ -558,7 +558,7 @@ public function save($data): bool if ($response instanceof BaseResult) { - $response = $response->resultID; + $response = $response->resultID !== false; } elseif ($response !== false) { diff --git a/tests/system/Database/Live/ModelTest.php b/tests/system/Database/Live/ModelTest.php index 5ed60d330914..6dd99106dbca 100644 --- a/tests/system/Database/Live/ModelTest.php +++ b/tests/system/Database/Live/ModelTest.php @@ -471,7 +471,7 @@ public function testDeleteBasics() $this->seeInDatabase('job', ['name' => 'Developer']); $result = $model->delete(1); - $this->assertTrue($result->resultID); + $this->assertTrue($result->resultID !== false); $this->dontSeeInDatabase('job', ['name' => 'Developer']); } @@ -1529,7 +1529,7 @@ public function testInsertResult() $result = $model->protect(false) ->insert($data, false); - $this->assertTrue($result->resultID); + $this->assertTrue($result->resultID !== false); $lastInsertId = $model->getInsertID(); From 5c5aabdfb84dd8bd22621d305328056d9d6fe9c8 Mon Sep 17 00:00:00 2001 From: michalsn Date: Sun, 5 Jul 2020 10:00:27 +0200 Subject: [PATCH 3/3] More tests for save() method --- system/Model.php | 1 + tests/system/Database/Live/ModelTest.php | 47 +++++++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/system/Model.php b/system/Model.php index f59276451fc9..eb47315850ba 100644 --- a/system/Model.php +++ b/system/Model.php @@ -42,6 +42,7 @@ use Closure; use CodeIgniter\Database\BaseBuilder; use CodeIgniter\Database\BaseConnection; +use CodeIgniter\Database\BaseResult; use CodeIgniter\Database\ConnectionInterface; use CodeIgniter\Database\Exceptions\DatabaseException; use CodeIgniter\Database\Exceptions\DataException; diff --git a/tests/system/Database/Live/ModelTest.php b/tests/system/Database/Live/ModelTest.php index 6dd99106dbca..9a295568a349 100644 --- a/tests/system/Database/Live/ModelTest.php +++ b/tests/system/Database/Live/ModelTest.php @@ -409,7 +409,28 @@ public function testSaveNewRecordArray() //-------------------------------------------------------------------- - public function testSaveUpdateRecordObject() + public function testSaveNewRecordArrayFail() + { + $this->setPrivateProperty($this->db, 'DBDebug', false); + + $model = new JobModel(); + + $data = [ + 'name123' => 'Apprentice', + 'description' => 'That thing you do.', + ]; + + $result = $model->protect(false) + ->save($data); + + $this->assertFalse($result); + + $this->dontSeeInDatabase('job', ['name' => 'Apprentice']); + } + + //-------------------------------------------------------------------- + + public function testSaveUpdateRecordArray() { $model = new JobModel(); @@ -428,7 +449,29 @@ public function testSaveUpdateRecordObject() //-------------------------------------------------------------------- - public function testSaveUpdateRecordArray() + public function testSaveUpdateRecordArrayFail() + { + $this->setPrivateProperty($this->db, 'DBDebug', false); + + $model = new JobModel(); + + $data = [ + 'id' => 1, + 'name123' => 'Apprentice', + 'description' => 'That thing you do.', + ]; + + $result = $model->protect(false) + ->save($data); + + $this->assertFalse($result); + + $this->dontSeeInDatabase('job', ['name' => 'Apprentice']); + } + + //-------------------------------------------------------------------- + + public function testSaveUpdateRecordObject() { $model = new JobModel();