From 6b23949b3b8164e584b9aafd852f992bd33e06fe Mon Sep 17 00:00:00 2001 From: michalsn Date: Wed, 11 Oct 2023 09:34:30 +0200 Subject: [PATCH 1/3] fix: add a primary key to an existing table --- system/Database/Forge.php | 8 +- tests/system/Database/Live/ForgeTest.php | 81 +++++++++++++++++++++ user_guide_src/source/changelogs/v4.4.2.rst | 2 + 3 files changed, 86 insertions(+), 5 deletions(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index f83a9acf94a6..bebf5c63aa7a 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -1084,12 +1084,10 @@ public function processIndexes(string $table): bool if (! empty($this->keys)) { $sqls = $this->_processIndexes($this->db->DBPrefix . $table, true); + } - $pk = $this->_processPrimaryKeys($table, true); - - if ($pk !== '') { - $sqls[] = $pk; - } + if (! empty($this->primaryKeys)) { + $sqls[] = $this->_processPrimaryKeys($table, true); } $this->foreignKeys = $fk; diff --git a/tests/system/Database/Live/ForgeTest.php b/tests/system/Database/Live/ForgeTest.php index e968ce9f5030..626660ee7246 100644 --- a/tests/system/Database/Live/ForgeTest.php +++ b/tests/system/Database/Live/ForgeTest.php @@ -1657,6 +1657,87 @@ public function testProcessIndexes(): void $this->forge->dropTable('user2', true); } + public function testProcessIndexesWithKeyOnly(): void + { + // make sure tables don't exist + $this->forge->dropTable('actions', true); + + $this->createActionsTable(); + $this->forge->addKey('name', false, false, 'db_actions_name'); + + // create indexes + $this->forge->processIndexes('actions'); + + // get a list of all indexes + $allIndexes = $this->db->getIndexData('actions'); + + // check that db_actions_name key exists + $indexes = array_filter( + $allIndexes, + static fn ($index) => ($index->name === 'db_actions_name') + && ($index->fields === [0 => 'name']) + ); + $this->assertCount(1, $indexes); + + // drop tables to avoid any future conflicts + $this->forge->dropTable('actions', true); + } + + public function testProcessIndexesWithPrimaryKeyOnly(): void + { + // make sure tables don't exist + $this->forge->dropTable('actions', true); + + $this->createActionsTable(); + $this->forge->addPrimaryKey('id'); + + // create indexes + $this->forge->processIndexes('actions'); + + // get a list of all indexes + $allIndexes = $this->db->getIndexData('actions'); + + // check that the primary key exists + $indexes = array_filter( + $allIndexes, + static fn ($index) => $index->type === 'PRIMARY' + ); + $this->assertCount(1, $indexes); + + // drop tables to avoid any future conflicts + $this->forge->dropTable('actions', true); + } + + public function testProcessIndexesWithForeignKeyOnly(): void + { + // make sure tables don't exist + $this->forge->dropTable('actions', true); + $this->forge->dropTable('user2', true); + + $this->createUser2TableWithKeys(); + $this->populateUser2Table(); + $this->createActionsTable(); + + // SQLite does not support custom foreign key name + if ($this->db->DBDriver === 'SQLite3') { + $this->forge->addForeignKey('userid', 'user', 'id'); + $this->forge->addForeignKey('userid2', 'user2', 'id'); + } else { + $this->forge->addForeignKey('userid', 'user', 'id', '', '', 'db_actions_userid_foreign'); + $this->forge->addForeignKey('userid2', 'user2', 'id', '', '', 'db_actions_userid2_foreign'); + } + + // create indexes + $this->forge->processIndexes('actions'); + + // check that the two foreign keys exist + $this->assertCount(2, $this->db->getForeignKeyData('actions')); + + // drop tables to avoid any future conflicts + $this->forge->dropTable('actions', true); + $this->forge->dropTable('user2', true); + } + private function createUser2TableWithKeys(): void { $fields = [ diff --git a/user_guide_src/source/changelogs/v4.4.2.rst b/user_guide_src/source/changelogs/v4.4.2.rst index 55cf33b0df25..2911638c2cae 100644 --- a/user_guide_src/source/changelogs/v4.4.2.rst +++ b/user_guide_src/source/changelogs/v4.4.2.rst @@ -42,6 +42,8 @@ Bugs Fixed Page Not Found. - **Spark:** Fixed a bug that caused spark to not display exceptions in the production mode or to display backtrace in json when an exception occurred. +- **Forge:** Fixed a bug where adding a Primary Key to an existing table was + ignored if there were no other Keys added too. See the repo's `CHANGELOG.md `_ From 7c9c2054d3c76dcf6979803acf3f070aeb123ddc Mon Sep 17 00:00:00 2001 From: michalsn Date: Wed, 11 Oct 2023 10:03:20 +0200 Subject: [PATCH 2/3] replace checks that use empty() --- system/Database/Forge.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/system/Database/Forge.php b/system/Database/Forge.php index bebf5c63aa7a..ae26f6e7457d 100644 --- a/system/Database/Forge.php +++ b/system/Database/Forge.php @@ -1073,7 +1073,7 @@ public function processIndexes(string $table): bool $sqls = []; $fk = $this->foreignKeys; - if (empty($this->fields)) { + if ($this->fields === []) { $this->fields = array_flip(array_map( static fn ($columnName) => $columnName->name, $this->db->getFieldData($this->db->DBPrefix . $table) @@ -1082,18 +1082,18 @@ public function processIndexes(string $table): bool $fields = $this->fields; - if (! empty($this->keys)) { + if ($this->keys !== []) { $sqls = $this->_processIndexes($this->db->DBPrefix . $table, true); } - if (! empty($this->primaryKeys)) { + if ($this->primaryKeys !== []) { $sqls[] = $this->_processPrimaryKeys($table, true); } $this->foreignKeys = $fk; $this->fields = $fields; - if (! empty($this->foreignKeys)) { + if ($this->foreignKeys !== []) { $sqls = array_merge($sqls, $this->_processForeignKeys($table, true)); } From c61d8e7fd4ea99f21d8c2f8388e65aaf50b11d20 Mon Sep 17 00:00:00 2001 From: michalsn Date: Wed, 11 Oct 2023 10:15:25 +0200 Subject: [PATCH 3/3] update phpstan-baseline.php --- phpstan-baseline.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 87be8ceb2377..a45da9b79c44 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -1238,7 +1238,7 @@ ]; $ignoreErrors[] = [ 'message' => '#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#', - 'count' => 16, + 'count' => 13, 'path' => __DIR__ . '/system/Database/Forge.php', ]; $ignoreErrors[] = [